Bug 89391 - Would like a way to force a clean build
Summary: Would like a way to force a clean build
Status: RESOLVED DUPLICATE of bug 123559
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-06-18 15:18 PDT by Lucas Forschler
Modified: 2016-03-18 10:01 PDT (History)
8 users (show)

See Also:


Attachments
Changes to buildbot (16.40 KB, patch)
2012-07-16 10:25 PDT, Josh Hawn
no flags Details | Formatted Diff | Diff
DB migration script (1023 bytes, text/x-python-script)
2012-07-16 16:53 PDT, Josh Hawn
no flags Details
Changes to Buildbot (Take 2) (5.54 KB, application/octet-stream)
2012-07-19 12:30 PDT, Josh Hawn
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Lucas Forschler 2012-06-18 15:18:21 PDT
Occasionally we run into issues where we need to force a clean buildbot.  Currently, we do this manually.
It would be great to have an option to force a clean build on any machine.  

I can think of a couple ways to do this, but ideally it would in a way that makes maintenance easy and avoids a buildmaster restart when things are changed.

One way this could be done is to add a Clean Factory.  This factory would call out to a script which would do the work of cleaning the build.  (this could involve several steps, and would probably differ based on OS/platform/Botconfig , and where the bot lived).  The script would live in ./Tools/OpenSource/Scripts.

It would be great to have a button on the bot page which would force a clean build ASAP.  (putting it ahead of any other builds in the queue)
This button would need some kind of authz.

If we choose not to use a button on the buildbot page, we'd need some way to trigger the bot to be cleaned.  This could work by checking for the existence of a 'please-clean-me' file, strategically placed on the file system somewhere.  Bot Admins would have to do the work to place this file in the right place.  Everytime the bot ran, it would check for this file, and act appropriately.  

Thoughts?
Comment 1 Ryosuke Niwa 2012-06-18 15:19:50 PDT
We should just add a button.
Comment 2 Eric Seidel (no email) 2012-06-19 16:46:14 PDT
There used to be a button.  It was removed long ago due to spam-bot abuse, iirc?
Comment 3 Ryosuke Niwa 2012-06-19 16:53:07 PDT
(In reply to comment #2)
> There used to be a button.  It was removed long ago due to spam-bot abuse, iirc?

I don't think this will be an issue now that we have authentication.
Comment 4 Mark Rowe (bdash) 2012-06-19 16:54:59 PDT
There used to be a force build button that could be abused to make a clean build happen by telling Buildbot to build a branch or revision that didn't exist.
Comment 5 Eric Seidel (no email) 2012-06-19 16:59:22 PDT
My current solution to forcing a clean build is to force a build, and then immediately abort it while it's in the "svn" step.  That leaves an svn lock around, if you then force a second build, it will fail to svn up, and buildbot will automatically nuke the client directory from orbit.
Comment 6 Josh Hawn 2012-07-16 10:24:22 PDT
I've come up with a working solution for this, though it required a change to my buildbot installation.

A "Clean Build" button was be added to the builders/$buildername page, with one button adjacent to all of the listed buildslaves.
This button acts much like the "Force Build" button, except that it schedules a build only on the slave that was specified, rather than any available slave for this builder.

I've created a new type of scheduler called a CleanScheduler, and (like ForceScheduler) the button is only available for builders that are given to this scheduler.

Because the new build request is for a specific slave, I needed to add a new attribute to build requests currently called clean_slavename. It is None by default, but is set to the name of the slave that is to be cleaned when the build request is caused by pressing the "Clean Build" button.
I added a new file to db migrations to add this column to the buildrequests table.

These build requests are given highest priority and are guaranteed to run only on the slave that was specified.

Because the clean steps are different than the normal build steps, the builder must be given a separate "clean_factory" object which will send the steps to the remote slave to clean the build directory for that builder.

Here's an excerpt from an example config file:

...
# create a Clean Scheduler with a list of builders that may need cleaning.
c['schedulers'].append(CleanScheduler(name="clean", builderNames=["builder1"]))

...
# make the usual build factory, and a clean factory with the steps for performing a clean.
factory = BuildFactory()
factory.addStep(ShellCommand(command=["make", "debug"]))

clean_factory = BuildFactory()
clean_factory.addStep(ShellCommand(command=["make", "clean"]))

...
# add builders, making sure that the builders listed in the clean scheduler have a 'clean_factory' specified.
# notice how "builder1" has a clean factory because it was listed in the clean scheduler, but "builder2" does not.
c['builders'].append(
    BuilderConfig(name="builder1",
      slavenames=["slave1", "slave2", "slave3", "slave4"],
      factory=factory,
      clean_factory=clean_factory))
c['builders'].append(
    BuilderConfig(name="builder2",
      slavenames=["slave3", "slave4", "slave5"],
      factory=factory))

...
# make sure that only authorized users can use the new button, notice the new 'cleanBuild' keyword argument
authz_cfg=authz.Authz(
    # change any of these to True to enable; see the manual for more
    # options
    auth=auth.BasicAuth([("username", "password")]),
    gracefulShutdown = False,
    forceBuild = 'auth',
    cleanBuild = 'auth', # the 'clean' button should appear on the builders page
    forceAllBuilds = False,
    pingBuilder = False,
    stopBuild = False,
    stopAllBuilds = False,
    cancelPendingBuild = False,
)
c['status'].append(html.WebStatus(http_port=8010, authz=authz_cfg))

I'll attach a diff off all of the changes I made to buildbot as well. I know it's probably not the best design yet, so let me know what you think.
Comment 7 Josh Hawn 2012-07-16 10:25:21 PDT
Created attachment 152557 [details]
Changes to buildbot
Comment 8 Josh Hawn 2012-07-16 16:53:34 PDT
Created attachment 152643 [details]
DB migration script

Add this file to buildbot/db/migrate/versions/ so that the buildrequests table has the clean_slavename column
Comment 9 Lucas Forschler 2012-07-18 13:04:37 PDT
Ryosuke, Ojan or Eric, do you have any thoughts on this implementation?
Comment 10 Mark Rowe (bdash) 2012-07-18 13:16:02 PDT
(In reply to comment #6)
> Because the new build request is for a specific slave, I needed to add a new attribute to build requests currently called clean_slavename. It is None by default, but is set to the name of the slave that is to be cleaned when the build request is caused by pressing the "Clean Build" button.
> I added a new file to db migrations to add this column to the buildrequests table.
> 

Is there some reason that this information isn't stored in the properties dictionary of the build set rather than adding a new column to the build request in the database?
Comment 11 Josh Hawn 2012-07-18 14:34:50 PDT
(In reply to comment #10)

> Is there some reason that this information isn't stored in the properties dictionary of the build set rather than adding a new column to the build request in the database?

I hadn't considered using buildset properties to store this information.

Currently, buildbot gets the buildset properties from the database only after already deciding which slave to use, but I could reorder it so that I check properties of a build request first. I'll work on changing this.

Please let me know any other thoughts you have about my changes.
Comment 12 Ojan Vafai 2012-07-18 14:37:27 PDT
(In reply to comment #9)
> Ryosuke, Ojan or Eric, do you have any thoughts on this implementation?

I don't know the buildbot code at all. So, no. :)
Comment 13 Josh Hawn 2012-07-19 11:53:04 PDT
Okay, I've got a new plan!

Forget all of the above changes to Buildbot.

The way that a Builder currently chooses which build to start is by picking a slavebuilder and build requests from a list of available slaves and a list of build requests.

First, it chooses which slavebuilder to use. Users can currently customize how they want to choose builders by specifying a 'nextSlave' function in their configuration for a builder. It defaults to using random.choice to pick any of the available slaves.

Then (only after choosing the slave) it chooses which build request to use. Users can also customize which build request is chosen by specifying a 'nextBuild' function in their configuration for a builder. It defaults to using the earliest submitted request.

Finally, (after choosing the slave and build request) it tries to merge build requests to submit for one build. Users can even customize this by specifying a 'mergeRequests' function in their configuration for a builder! It defaults to merging builds with compatible source stamps (check the code/documentation for the details).

The issue here is that we would like to be able to do all three of these things at once: choose a slave, build request, and merge compatible requests.

When we have a build request to clean a slave, we need to choose the request first so that we know which slave to choose (if available). The current implementation doesn't allow that (it chooses build requests independently from choosing the slave, and only after choosing the slave).

So, I propose a simple (much simpler than my last suggestion) change to Buildbot Builder configurations. A configuration option called "chooseSlaveAndMergeBuildRequests". It is the combination of the currently available options: "nextSlave", "nextBuild", and "mergeRequests".

The "chooseSlaveAndMergeBuildRequests" function will take two arguments: a list of available slavebuilders and a list of buildrequest objects. It will return a tuple of (selected_slavebuilder, [buildrequest, ...]) which is a single chosen slavebuilder and a list of build requests to be merged. You can still implement the logic of your "nextSlave", "nextBuild" and "mergeRequests" configuration inside this function, but now it gives you more control to choose build requests that depend on a particular slavebuilder.

This will allow us to inspect the properties of a build request to see if it was scheduled by a certain 'clean' scheduler! Another property set by the scheduler will specify the 'clean_slavename', and we can choose that slave if it is available. We can also merge 'clean' requests for the same buildslave.

Now, about getting that "Clean" button on the builder page...

The clean button can be implemented as a simple ForceScheduler that sets a property for which slave to clean.
Normally, ForceScheduler buttons are listed separately on the page, but we want them listed in the "Buildslaves" section of the page.
We've always been able to customize templates for pages, so we just need to customize the "builder.html" template to put the form for the clean scheduler in the right place. Customizing the "forms.html" template is also possible, so we can make a macro for "clean_scheduler(url, scheduler, slavename)" that just places a button on the page that submits a form to force a build with the 'clean_slavename' property set to the given slavename.

Oh, no, but we still need a different set of steps for this "clean" build!

That can be easily taken care of in a user's configuration as well! 

Each 'BuildFactory' instance has a method called 'newBuild' which takes a list of BuildRequest objects and returns a new 'Build' object that has all of the steps needed to complete the build.
Users can override this method in a subclass of 'BuildFactory' and check the properties of the BuildRequest objects to see if they came from the 'clean' scheduler. It can then return a Build object that has been instantiated with a different set of 'clean steps' instead of the normal build steps. A user would have to specify these 'clean steps' in the configuration.

I've got all of these changes applied on a test master I'm running, and it provides all of the features of my last proposed change, but with minimal changes to Buildbot itself and most changes done in the master.cfg file.

Now there's no need to add new columns to database tables, or have separate factories for each Builder! The "chooseSlaveAndMergeBuildRequests" builder config option allows for more control, and everything else is done in master.cfg and custom templates!

Let me know what you guys think.
Comment 14 Josh Hawn 2012-07-19 12:30:33 PDT
Created attachment 153320 [details]
Changes to Buildbot (Take 2)
Comment 15 Csaba Osztrogonác 2016-03-18 10:01:45 PDT
already fixed 2.5 years ago - http://trac.webkit.org/changeset/158393

*** This bug has been marked as a duplicate of bug 123559 ***