Bug 218026

Summary: Rename BuildSlaveSupport to CISupport
Product: WebKit Reporter: Aakash Jain <aakash_jain>
Component: Tools / TestsAssignee: Aakash Jain <aakash_jain>
Status: RESOLVED FIXED    
Severity: Normal CC: aakash_jain, ap, bdakin, clopez, dean_johnson, dewei_zhu, dpino, Hironori.Fujii, jbedard, lingho, ryanhaddad, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: Other   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=218302
https://bugs.webkit.org/show_bug.cgi?id=227644
Bug Depends on:    
Bug Blocks: 213092    
Attachments:
Description Flags
Patch
none
Patch jbedard: review+

Description Aakash Jain 2020-10-21 08:31:33 PDT 
We should rename BuildSlaveSupport to BuildAutomation. This is for following purposes:

1) using more inclusive name and avoiding the 'slave' terminology

2) we are planning to upgrade build.webkit.org to latest Buildbot soon which uses "worker" terminology instead of "buildslave", it would be nice to have uniformity in directory naming as well

3) making the naming/directory-structure more consistent with some of our other projects


Note that some of the scripts inside this folder might be used in various places, including other repositories and other buildbot instances, we would need to make sure that we update those places as well. Also the servers like build.webkit.org, ews-build.webkit.org and ews.webkit.org would need to be updated accordingly as they are running the production services from inside this directory.
Comment 1 Aakash Jain 2020-10-21 08:46:21 PDT 
Buildbot instances might fail certain builds for certain revisions after this change, depending on when the Buildbot is restarted to pick up this change. 

For e.g.: if this change is landed in r1000. Until Buildbot instance (e.g.: for build.webkit.org) is restarted, it would keep executing scripts from old directory (e.g.: Tools/BuildSlaveSupport/kill-old-processes). If the revision being tested is after r1000, it would keep failing until Buildbot is restarted (since BuildSlaveSupport wouldn't exist in newer revisions). If Buildbot is restarted immediately after committing r1000, Buildbot instance would fail the builds which were testing any revision prior to r1000.

To solve this issue, we can consider adding a symlink named BuildSlaveSupport (pointing to BuildAutomation). We can then remove the symlink after few days/weeks.
Comment 2 Jonathan Bedard 2020-10-21 08:51:25 PDT 
(In reply to Aakash Jain from comment #1)
> Buildbot instances might fail certain builds for certain revisions after
> this change, depending on when the Buildbot is restarted to pick up this
> change. 
> 
> For e.g.: if this change is landed in r1000. Until Buildbot instance (e.g.:
> for build.webkit.org) is restarted, it would keep executing scripts from old
> directory (e.g.: Tools/BuildSlaveSupport/kill-old-processes). If the
> revision being tested is after r1000, it would keep failing until Buildbot
> is restarted (since BuildSlaveSupport wouldn't exist in newer revisions). If
> Buildbot is restarted immediately after committing r1000, Buildbot instance
> would fail the builds which were testing any revision prior to r1000.
> 
> To solve this issue, we can consider adding a symlink named
> BuildSlaveSupport (pointing to BuildAutomation). We can then remove the
> symlink after few days/weeks.

I think the symlink is a really good idea.
Comment 3 Radar WebKit Bug Importer 2020-10-21 09:13:06 PDT 
<rdar://problem/70531568>
Comment 4 Aakash Jain 2020-10-21 09:14:30 PDT 
Created attachment 411994 [details]
Patch
Comment 5 Aakash Jain 2020-10-21 09:19:33 PDT 
svn-apply isn't able to handle this patch, probably because it has a lot of svn property changes. Would land it manually once approved.
Comment 6 Aakash Jain 2020-10-27 09:18:57 PDT 
After discussing with few folks, "CISupport" seems like a better name for this directory.
Comment 7 Aakash Jain 2020-10-27 09:23:38 PDT 
Created attachment 412428 [details]
Patch
Comment 8 Jonathan Bedard 2020-10-27 09:57:20 PDT 
Patch review is obviously tough for a change like this. To verify, we're moving Tools/BuildSlaveSupport to Tools/CISupport and creating a symlink in place of Tools/BuildSlaveSupport, correct?
Comment 9 Aakash Jain 2020-10-27 10:03:09 PDT 
(In reply to Jonathan Bedard from comment #8)
> Patch review is obviously tough for a change like this. To verify, we're moving Tools/BuildSlaveSupport to Tools/CISupport and creating a symlink in place of Tools/BuildSlaveSupport, correct?
That's correct
Comment 10 Jonathan Bedard 2020-10-27 10:07:26 PDT 
Comment on attachment 412428 [details]
Patch

Seems like now is as good a time as any to land this. Always going to be a bit risky to land something like this.
Comment 11 Aakash Jain 2020-10-27 10:12:10 PDT 
(In reply to Jonathan Bedard from comment #10)
> Seems like now is as good a time as any to land this. Always going to be a bit risky to land something like this.
Agree!
Comment 12 Aakash Jain 2020-10-27 10:39:20 PDT 
Committed r269052: <https://trac.webkit.org/changeset/269052>
Comment 13 Aakash Jain 2020-10-27 11:19:05 PDT 
symlink seems to be working fine. For e.g.: in https://build.webkit.org/builders/Apple-Mojave-Release-Build/builds/20794 checked out r269054 (which is after r269052). It correctly renamed the directory and created the BuildSlaveSupport symlink pointing to CISupport (I also verified by sshing in to the bot). Subsequent steps of running Tools/BuildSlaveSupport/kill-old-processes worked fine.
Comment 14 Aakash Jain 2020-10-29 16:04:08 PDT 
Restarted build.webkit.org to pick up this change. Seems to be working fine.