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, fujii.hironori, 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+

Aakash Jain
Reported 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.
Attachments
Patch (2.45 MB, patch)
2020-10-21 09:14 PDT, Aakash Jain 		no flags
DetailsFormatted DiffDiff
Patch (2.45 MB, patch)
2020-10-27 09:23 PDT, Aakash Jain 		jbedard: review+
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Aakash Jain
Comment 1 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.
Jonathan Bedard
Comment 2 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.
Radar WebKit Bug Importer
Comment 3 2020-10-21 09:13:06 PDT
<rdar://problem/70531568>
Aakash Jain
Comment 4 2020-10-21 09:14:30 PDT
Created attachment 411994 [details] Patch
Aakash Jain
Comment 5 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.
Aakash Jain
Comment 6 2020-10-27 09:18:57 PDT
After discussing with few folks, "CISupport" seems like a better name for this directory.
Aakash Jain
Comment 7 2020-10-27 09:23:38 PDT
Created attachment 412428 [details] Patch
Jonathan Bedard
Comment 8 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?
Aakash Jain
Comment 9 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
Jonathan Bedard
Comment 10 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.
Aakash Jain
Comment 11 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!
Aakash Jain
Comment 12 2020-10-27 10:39:20 PDT
Committed r269052: <https://trac.webkit.org/changeset/269052>
Aakash Jain
Comment 13 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.
Aakash Jain
Comment 14 2020-10-29 16:04:08 PDT
Restarted build.webkit.org to pick up this change. Seems to be working fine.
Note You need to log in before you can comment on or make changes to this bug.