605267 - update clobberer for buildbot 0.8.0 releases

Status: RESOLVED FIXED

(Release Engineering :: General, defect, P3)

Description

[[e-mail] (:bhearsum)]

Clobberer needs to be updated to know that release-* are release builders -- we can probably just use that glob for it rather than trying to spell them out. That'll be helpful when we add new builders, too.

Comment 1

[Syed Albiz [:salbiz]]

Attachment: clobber release-* directories through clobber page

Tested this with a snapshot of the clobberer.db, with release-{builder_name} for a bunch of release builder names.

Comment 2

[[e-mail] (:bhearsum)]

Comment on attachment 485183 [details] [diff] [review]
clobber release-* directories through clobber page

>diff --git a/clobberer/index.php b/clobberer/index.php
>--- a/clobberer/index.php
>+++ b/clobberer/index.php
>@@ -14,16 +14,20 @@ http://hg.mozilla.org/build/buildbotcust
> 
> $RELEASE_BUILDERS = array(
>   'tag',
>   'source',
>   'updates',
>   'major_update',
>   'final_verification',
> );
>+$RELEASE_PREFIXES = array(
>+  'release-',
>+);
>+
> foreach ($PLATFORMS as $platform){
>   $RELEASE_BUILDERS[] = "${platform}_build";
>   $RELEASE_BUILDERS[] = "${platform}_repack";
>   $RELEASE_BUILDERS[] = "${platform}_l10n_verification";
>   $RELEASE_BUILDERS[] = "${platform}_update_verify";
>   $RELEASE_BUILDERS[] = "${platform}_major_update_verify";
>   $RELEASE_BUILDERS[] = "${platform}_test mochitests";
>   $RELEASE_BUILDERS[] = "${platform}_test mochitest-other";
>@@ -197,27 +201,39 @@ if ($_POST['form_submitted']) {
>     if ($k == "master") {
>       if (isSpecial($user)) {
>         $branch = $_POST['branch'];
>         if ($branch != '') {
>           $branch = e($branch);
>         } else {
>           $branch = 'NULL';
>         }
>-        $builddir = $_POST['builddir'];
>-        if ($builddir != '') {
>-            $builders = array($builddir);
>-        } else {
>-            $builders = $RELEASE_BUILDERS;
>-        }
>+
>         if ($v != '') {
>           $master = e($v);
>         } else {
>           $master = 'NULL';
>         }
>+
>+        $builddir = $_POST['builddir'];
>+        if (in_array($builddir, $RELEASE_PREFIXES)) {

Hmmm, I'm not sure I follow what's going on here. It seems like you meant to say "if builddir has a release- prefix". But even if that's the case, you don't want to clobber anything except that specific directory. This code is executed when someone submits a clobber, not when a build machine is looking for them. Basically, I don't think you need to add this section at all.



>+            $q = "SELECT DISTINCT builddir FROM builds "
>+                ."WHERE "
>+                ."builddir like '". $builddir . "%'";

You want to do a query like the one above when somebody requests "all builders" being clobbered, but it's a bit more complicated than that. If they also select "any release" the below query is fine, but if they limit it to a specific one you'll have to refine the query further to only catch builders from that branch. This part belongs in the final else {} block AFAICT.

Does that make sense?

Comment 3

[Syed Albiz [:salbiz]]

Attachment: refactor to silently look up 'release-' prefixes and insert those for clobbering

changed my approach to look up valid builddirs with the release- prefix and submit them for clobbering if they exist whenever we submit a request to clobber release builders. This does not affect how we clobber non 0.8.0 builders.

Comment 4

[[e-mail] (:bhearsum)]

Comment on attachment 485358 [details] [diff] [review]
refactor to silently look up 'release-' prefixes and insert those for clobbering

This looks good to me

Comment 5

[Syed Albiz [:salbiz]]

Comment on attachment 485358 [details] [diff] [review]
refactor to silently look up 'release-' prefixes and insert those for clobbering

Only affects release-builder clobbers, so should be safe to land.

Comment 6

[[e-mail] (:bhearsum)]

Comment on attachment 485358 [details] [diff] [review]
refactor to silently look up 'release-' prefixes and insert those for clobbering

Adjusted the wrong flag, whoops

Comment 7

[Chris AtLee [:catlee]]

Comment on attachment 485358 [details] [diff] [review]
refactor to silently look up 'release-' prefixes and insert those for clobbering

There are unittests for clobberer in test_clobberer.py.  Do these still pass, and do they need updating as well?

Comment 8

[Syed Albiz [:salbiz]]

Attachment: add testcase for index.php

The unit tests all pass, and still cover the same functionality as before. All previous tests covering scheduling clobbers look like they're done by direct insertion into the clobber.db, although the builds table looks like it is updated through the php code using the test harness. I have added a test to verify that scheduling clobbers through the php code works as expected for release builders.

In the future, we may want to add some more tests to validate the php insertion code further, as the unit tests here don't seem to do that.

Comment 9

[[e-mail] (:bhearsum)]

I noticed that nothing gets marked for a clobber until the builder checks in one time -- this won't be an issue if this lands in sync with the first 0.8.0 production release, if it doesn't we'll need to fake out all of the builders in the clobberer db.

canSee() needs to be updated to ignore the new style release builders.

Comment 10

[[e-mail] (:bhearsum)]

Comment on attachment 485358 [details] [diff] [review]
refactor to silently look up 'release-' prefixes and insert those for clobbering

I'll land this once Beta 7 is clear.

Comment 11

[[e-mail] (:bhearsum)]

Comment on attachment 487516 [details] [diff] [review]
add testcase for index.php

I had trouble making these run locally, but it seems fine.

Comment 12

[[e-mail] (:bhearsum)]

I'm finding that release-* builders are showing up in the main table on stage-clobberer....Syed, any idea why that's happening?

Comment 13

[[e-mail] (:bhearsum)]

Comment on attachment 485358 [details] [diff] [review]
refactor to silently look up 'release-' prefixes and insert those for clobbering

Landed this in production this morning.

Comment 14

[[e-mail] (:bhearsum)]

Comment on attachment 485358 [details] [diff] [review]
refactor to silently look up 'release-' prefixes and insert those for clobbering

Forgot to commit this...now landed in:
changeset:   1328:86aab0d25b33

Comment 15

[Syed Albiz [:salbiz]]

Now that a fix for bug 615192 has landed this should be all done.