422747 - clean up master.cfg of the talos buildmaster for more code re-use

Status: RESOLVED FIXED

(Release Engineering :: General, defect, P2)

Description

[alice nodelman [:alice] [:anode]]

The talos buildbot master.cfg is massive and full of repeated code chunks.  The problem is that we have entirely separate buildbot factories for each set of talos machines.  These factories share 90% of their build steps, but then have unique steps per-platform/per-set.  We'd need to find a way to share most steps and be able to add unique steps on a per-factory basis.

Comment 1

[Mikeal Rogers [:mikeal] (mrogers@)]

I'll be doing this as soon as https://bugzilla.mozilla.org/attachment.cgi?id=308737 is checked in.

Comment 2

[Mikeal Rogers [:mikeal] (mrogers@)]

Accidentally hit the re-assign option, taking it back.

Comment 3

[alice nodelman [:alice] [:anode]]

Attachment: compress down master.cfg for talos & talos stage

Will also need patches for talos try master and talos try perfrunner - but that can wait till we see if this works out for talos production/stage.

Comment 4

[alice nodelman [:alice] [:anode]]

Attachment: compress down master.cfg for talos & talos stage (take 2)

fixing typo.

Comment 5

[alice nodelman [:alice] [:anode]]

The risk with this patch is assigning the wrong branch number to a given build factory (ie, identifying a branch builder as 1.9 instead of 1.8) - this will cause issues on the graph server as the branch number is used as part of the identifying key for a set of data.

I've gone over it several times to make sure that each factory is using the correct config file, branch number and other identifying information but I'm starting to go a little cross-eyed over it.  

Comment 6

[[e-mail] (:bhearsum)]

Comment on attachment 328974 [details] [diff] [review]
compress down master.cfg for talos & talos stage (take 2)

>Index: perfmaster2/master.cfg
>===================================================================
This file looks OK to me, except as noted below, you shouldn't need to call addTalosSteps. I looked carefully at all of the branches/config file parameters and I'm pretty sure it's fine.


>Index: perfmaster2/perfrunner.py
>===================================================================
>+class TalosFactory(BuildFactory):

Can we put this in buildbotcustom.process.factory, pretty please?

>+  """Create working talos build factory"""

Need this at the same indentation level as the 'def's.

>+
>+    def __init__(self, **kwargs):
>+        BuildFactory.__init__(self, **kwargs)
>+
>+    def addTalosSteps(self, OS, envName, cleanCmd, buildBranch, configFile, buildSearchString, buildDir, buildPath, talosCmd, customManifest=''):

Is there a reason you can't add all of these in __init__()? Like this: http://mxr.mozilla.org/mozilla/source/tools/buildbotcustom/process/factory.py#5

Also, i think it'd be fine to define the cleanCmd in here based on the OS, since it is constant across each OS. Might be able to do that for buildPath too. It's up to, though.

>+        self.addStep(ShellCommand,
>+                           workdir=".",
>+                           description="Cleanup",
>+                           command=cleanCmd,
>+                           env=MozillaEnvironments[envName])
>+        self.addStep(ShellCommand,
>+                           command=["cvs", "-d", CVSROOT, "co", "-d", "talos",

CVSROOT technically works here, because it is defined in the master.cfg, but good practice would be to pass it in.

The rest of this factory looks OK.




>Index: perf-staging/master.cfg
>===================================================================

>+
>+winClean       = ["rm", "-rf", "*.zip", "talos/", "firefox/"]
>+macClean       = "rm -vrf *"
>+linuxClean     = ["rm", "-rf", "*.bz2", "*.gz", "talos/", "firefox/"]
>+

As mentioned above, consider putting these in the factory.



I'll admit, I only scanned the rest of this file, but it looked OK.


r- specifically because you should be adding steps in TalosFactory.__init__() and the CVSROOT. I'd really like this to go in buildbotcustom, but since the rest of the Talos stuff isn't there yet I won't require it.

This is really great, I'm glad it's getting done.

Comment 7

[alice nodelman [:alice] [:anode]]

Attachment: [checked in]compress down master.cfg for talos & talos stage (take 3)

Moved the cvsRoot and cleanCmd into TalosFactory.  I can't fold in firefox path as it can change per branch (look at the mac builders that use GrandParadiso, Minefield & Bonecho in paths).  

I think that pretty much everything that could be moved into the factory is now there.

Where does the buildbot custom stuff live?  I can put together a patch for that as well.

Comment 8

[[e-mail] (:bhearsum)]

Comment on attachment 329111 [details] [diff] [review]
[checked in]compress down master.cfg for talos & talos stage (take 3)

>Index: perfmaster2/perfrunner.py
>===================================================================
>+class TalosFactory(BuildFactory):

Like we talked about on IRC we should put this in buildbotcustom (mozilla/tools/buildbotcustom in CVS) at some point. This is fine for now though.

Comment 9

[alice nodelman [:alice] [:anode]]

Comment on attachment 329111 [details] [diff] [review]
[checked in]compress down master.cfg for talos & talos stage (take 3)

Checking in perfmaster2/master.cfg;
/cvsroot/mozilla/tools/buildbot-configs/testing/talos/perfmaster/master.cfg,v  <--  master.cfg
new revision: 1.73; previous revision: 1.72
done
Checking in perfmaster2/perfrunner.py;
/cvsroot/mozilla/tools/buildbot-configs/testing/talos/perfmaster/perfrunner.py,v  <--  perfrunner.py
new revision: 1.20; previous revision: 1.19
done
Checking in perf-staging/master.cfg;
/cvsroot/mozilla/tools/buildbot-configs/testing/talos/perf-staging/master.cfg,v  <--  master.cfg
new revision: 1.18; previous revision: 1.17
done

Comment 10

[alice nodelman [:alice] [:anode]]

Checked in bustage fix for perfrunny.py (malformed init function in TalosFactory).

Comment 11

[alice nodelman [:alice] [:anode]]

Attachment: [Checked in]integrate changes into try talos set up

You are going to hate this - since this just duplicates the code in the production talos setup into the try talos setup.  I figure that this is the half way step - hopefully quickly followed by integration into buildbot custom to reduce code repetition.

Comment 12

[alice nodelman [:alice] [:anode]]

In response to a question in irc: yes, this is just a direct copy of the TalosFactory from the talos production perfrunner.py.

Comment 13

[[e-mail] (:bhearsum)]

Comment on attachment 331160 [details] [diff] [review]
[Checked in]integrate changes into try talos set up

hello, meet my friend stampy.

Comment 14

[alice nodelman [:alice] [:anode]]

Comment on attachment 331160 [details] [diff] [review]
[Checked in]integrate changes into try talos set up

Checking in perfrunner.py;
/cvsroot/mozilla/tools/buildbot-configs/testing/talos/tryperfmaster/perfrunner.py,v  <--  perfrunner.py
new revision: 1.5; previous revision: 1.4
done
Checking in master.cfg;
/cvsroot/mozilla/tools/buildbot-configs/testing/talos/tryperfmaster/master.cfg,v  <--  master.cfg
new revision: 1.4; previous revision: 1.3
done

Comment 15

[alice nodelman [:alice] [:anode]]

The buildbot custom issue can be tracked in bug 448047.

Otherwise this bug is done, the master.cfg for talos set ups is a lot smaller and there is a *lot* less duplicate code.

Comment 16

[Chris Cooper [:coop] (he/him)]

Moving closed Future bugs into Release Engineering in preparation for removing the Future component.