484203 - Clobberer clobbered builds need to be clearly identified

Status: RESOLVED FIXED

(Mozilla Messaging Graveyard :: Release Engineering, defect)

Description

[Philippe M. Chiasson (:gozer)]

So they can be automatically identified, the buildstep that runs clobberer.py needs to :
 - Set a BuildProperty indicating if it clobbered or not
 - A TinderboxPrint: clobbered so it shows up visibly on the waterfall

Comment 1

[Philippe M. Chiasson (:gozer)]

Attachment: Proof of concept MozillaClobberer

This is a proof-of-concept of what could be buildbotcustom.steps.MozillaClobberer.

It's again our unittest master right now, but that's so I can test it out.

Comment 2

[Philippe M. Chiasson (:gozer)]

Works! I forced clobbered a Linux comm-1.9.1 check build just now, and the tinderbox waterfall correctly shows 'clobber forced'

Comment 3

[Philippe M. Chiasson (:gozer)]

Attachment: buildbot.steps.misc.MozillaClobberer buildbot step

Attached is a new buildbot.steps.misc step that takes care of running clobberer.py and identifies builds that have been clobbered by :
 - Setting some build properties
 - TinderboxPrint'ing so they show up on the waterfall.

Comment 4

[[e-mail] (:bhearsum)]

Comment on attachment 368952 [details] [diff] [review]
buildbot.steps.misc.MozillaClobberer buildbot step

>diff -d -u misc.py
>--- misc.py	20 Jan 2009 21:25:54 -0000	1.10
>+++ misc.py	23 Mar 2009 20:27:29 -0000
>@@ -26,7 +26,7 @@
> 
> import buildbot
> from buildbot.process.buildstep import LoggedRemoteCommand, LoggingBuildStep
>-from buildbot.steps.shell import ShellCommand
>+from buildbot.steps.shell import ShellCommand, WithProperties
> from buildbot.status.builder import FAILURE, SUCCESS
> 
> class CreateDir(ShellCommand):
>@@ -181,3 +181,49 @@
>         except:
>             return FAILURE
>         return SUCCESS
>+
>+import re

Please keep all the imports at the top.

>+class MozillaClobberer(ShellCommand):
>+    def __init__(self, branch, clobber_url, clobberer_path, **kwargs):
>+        ShellCommand.__init__(self,
>+                              description=['checking','clobber','times'],
>+                              command = ['python', clobberer_path,
>+                                         '-s', 'tools',
>+                                         clobber_url, branch,
>+                                         WithProperties("%(buildername)s"),
>+                                         WithProperties("%(slavename)s")
>+                                        ],
>+                                **kwargs)
>+    def createSummary(self, log):
>+
>+        # Server is forcing a clobber
>+        forcedClobberRe = re.compile('Server is forcing a clobber')
>+        # Clobbering...
>+        ClobberingRe = re.compile('Clobbering...')
>+        # More than 7 days, 0:00:00 have passed since our last clobber

Can you note that this is just an example output?

>+        periodicClobberRe = re.compile('More than (\d+) days, (\d+:\d+:\d+) have passed since our last clobber')

Seems OK. If for some reason clobber is based a weird fraction of an hour that doesn't divide evenly into seconds this will fail....but I don't think we need to concern ourselves with that situation.

>+        self.setProperty('forced_clobber', False, MozillaClobberer)
>+        self.setProperty('clobbered', False, MozillaClobberer)
>+        self.setProperty('periodic_clobber', False, 'MozillaClobberer')

You probably want to use a quoted 'MozillaClobberer' for all of these.

>+
>+        clobberType = None
>+        clobbered   = False
>+        for line in log.readlines():
>+            m = forcedClobberRe.search(line)
>+            if m:
>+                self.setProperty('forced_clobber', True, 'MozillaClobberer')
>+                clobberType = "forced"
>+            m = ClobberingRe.search(line)
>+            if m:
>+                self.setProperty('clobbered', True, 'MozillaClobberer')
>+                clobbered = True
>+            m = periodicClobberRe.search(line)
>+            if m:
>+                self.setProperty('periodic_clobber', True, 'MozillaClobberer')
>+                clobberType = "periodic"
>+
>+        if clobberType != None:
>+            summary = "TinderboxPrint: %s clobber" % clobberType
>+            self.addCompleteLog('clobberer', summary)
>+

Two overall comments:
1) Isn't the 'clobbered' stuff redundant? Eg, if force_clobber or periodic_clobber happens it is *always* the case that we clobbered, right? And if neither of those is set, we haven't clobbered. I may be missing something here though...

2) Please add in support for overriding the clobber time. We use these here: http://hg.mozilla.org/build/buildbotcustom/file/tip/process/factory.py#l167.

This looks good, otherwise. Fix the inline nits and let me know how you feel about removing 'clobbered'.

Comment 5

[Philippe M. Chiasson (:gozer)]

Attachment: buildbot.steps.misc.MozillaClobberer buildbot step v2

Here is a new version that addresses all the comments. Yes, clobberer was a redundant proprety, it was originally in the patch to help identify clobberer builds only, but that's truly not needed anymore.

I've also added support for clobberTimes, so this should be all that's needed.

Comment 6

[Philippe M. Chiasson (:gozer)]

Attachment: buildbot.steps.misc.MozillaClobberer buildbot step v3

Correct a small typo in patch v2

Comment 7

[Philippe M. Chiasson (:gozer)]

Attachment: buildbot.steps.misc.MozillaClobberer buildbot step v4

And this would be the last typo in that patch, apologies for the bugspam

Comment 8

[[e-mail] (:bhearsum)]

Comment on attachment 372066 [details] [diff] [review]
buildbot.steps.misc.MozillaClobberer buildbot step v4

>diff -u misc.py
>--- steps/misc.py	20 Jan 2009 21:25:54 -0000	1.10
>+++ steps/misc.py	10 Apr 2009 16:34:14 -0000
>@@ -25,8 +25,9 @@
> # ***** END LICENSE BLOCK *****
> 
> import buildbot
>+import re
> from buildbot.process.buildstep import LoggedRemoteCommand, LoggingBuildStep
>-from buildbot.steps.shell import ShellCommand
>+from buildbot.steps.shell import ShellCommand, WithProperties
> from buildbot.status.builder import FAILURE, SUCCESS
> 
> class CreateDir(ShellCommand):
>@@ -181,3 +182,42 @@
>         except:
>             return FAILURE
>         return SUCCESS
>+
>+class MozillaClobberer(ShellCommand):
>+    def __init__(self, branch, clobber_url, clobberer_path, clobberTime=None, **kwargs):
>+        clobber_time = []
>+        if clobberTime is not None:
>+            clobber_time = [ '-t', str(clobberTime) ]

This tripped me up when I read it. I thought "why is clobberTime an array?". Can you rename it to clobber_time_arg, or something? You could also construct command separately and pass command=command to ShellCommand.__init__().

r=bhearsum with one of those changes

Comment 9

[[e-mail] (:bhearsum)]

Attachment: [checked in] clobberer step, my own nits fixed

What I meant about the arguments was to put them in a class level spot, like flunkOnFailure and description are. For some reason though, timeout and workdir refuse to function properly like that. Perhaps ShellCommand doesn't work well that way...

In any case, I gave this a good test and it seems to work OK. Does it serve your purpose, Gozer?

Comment 10

[[e-mail] (:bhearsum)]

Attachment: [checked in] use clobberer step for Firefox builds

(Hope you don't mind me using this bug for this.)

Here's the buildbot-config code I tested this with. Worked fine for me. Catlee, how do you feel about switching to this? The only change should be a '(forced|periodic) clobber' TinderboxPrint when one of those events happens.

Comment 11

[Philippe M. Chiasson (:gozer)]

Comment on attachment 372469 [details] [diff] [review]
[checked in] clobberer step, my own nits fixed

I like it, command.extend() makes it cleaner than my attempt.

Comment 12

[[e-mail] (:bhearsum)]

Comment on attachment 372469 [details] [diff] [review]
[checked in] clobberer step, my own nits fixed

changeset:   249:918bba9e5cbc

Comment 13

[[e-mail] (:bhearsum)]

Comment on attachment 372470 [details] [diff] [review]
[checked in] use clobberer step for Firefox builds

changeset:   250:cd2b80c3d9b8

Comment 14

[[e-mail] (:bhearsum)]

Comment on attachment 372470 [details] [diff] [review]
[checked in] use clobberer step for Firefox builds

production-master/moz2-master has been reconfig'ed for this. Gozer - do you still need this bug open for your own deployment?

Comment 15

[Philippe M. Chiasson (:gozer)]

(In reply to comment #14)
> (From update of attachment 372470 [details] [diff] [review])
> production-master/moz2-master has been reconfig'ed for this.
>
> Gozer - do you still need this bug open for your own deployment?

Nope, go ahead and close this. Thanks for the legwork!

Comment 16

[Philippe M. Chiasson (:gozer)]

Closing, production MoMo buildbots are running this too.