433315 - clean-up mozilla2 configs + steps

Status: RESOLVED FIXED

(Release Engineering :: General, defect, P3)

Description

[[e-mail] (:bhearsum)]

With debug+leak tests being enabled on mozilla-central very soon the master.cfg and some steps have gotten a little ugly. This bug is to track cleaning them up and making them more maintainable. bug 429670 may end up as a dependent - more investigation is needed first.

Some of the clean up will involve simplifying the code in buildbotcustom.steps.test.py and removing conditionals in the master.cfg.

Another idea I have is to put the 'BRANCHES' array into a separate file.

Ideas/input are welcomed.

Comment 1

[[e-mail] (:bhearsum)]

Attachment: [checked in] phase 1: move config section to an external file + reformat

This is phase 1 of clean-up: getting the main configuration to an external file.

Mostly this is just a logical separation, but I also think it will make it simpler to add new branches in the future.

I'm sure the PGO patch is going to land before so I built it on-top of that (which is why you see the 'profiled_build' options).

Comment 2

[[e-mail] (:bhearsum)]

Attachment: phase 2: clean-up builder names a bit

(This one's built ontop of the previous one, though it'd probably apply cleanly to the current master.cfg.)

This patch simply makes the builder names a little bit nicer. Talos will need a patch to cope with this.

The Firefox tree uses the following format for build machines:
$OS [$osversion] $hostname [dep] [nightly|debug]

It doesn't make sense for us to display hostname since many machines share the same column so I subbed in 'firefox'. I was originally going to use $branch (eg, mozilla-central), but given that branches are separated in different Tinderbox pages I think 'firefox' is more useful/makes more sense.

I'd like to leave $osversion out to avoid putting it in the config. If there's a reason to have it printed we can put it in the log somehow.

The only other difference is that '-debug' will be appended to the platform for debug builders, rather than it being at the end. The only reason for this is to simplify the code.


No rush here.

Comment 3

[Nick Thomas [:nthomas] (UTC+12)]

Comment on attachment 321125 [details] [diff] [review]
[checked in] phase 1: move config section to an external file + reformat

>diff -r 6bb82fa0ee20 mozilla2/config.py
...
>+BRANCHES['mozilla-central']['platforms']['win32-debug']['profiled_build'] = True

This one is False in the current master config. A deliberate change or just bitrot ? r+ for the general approach, but please resolve this discrepancy and check for other lines that have gotten out of sync before landing.


Some other things I noticed along the way ....

>+BRANCHES['mozilla-central']['platforms']['linux']['slaves'] = [
>+    'moz2-linux-slave1',
>+    'moz2-linux-slave02',
>+    'moz2-linux-slave03'
>+]

A random comment - I guess we should rename moz2-linux-slave1 to moz2-linux-slave01 at some point.

>+BRANCHES['mozilla-central']['platforms']['macosx']['slaves'] = [
>+    'moz2-mac-slave1',
>+    'bm-xserve18'
>+]

Another random comment - rename bm-xserve18 for consistency ?

>+BRANCHES['mozilla-central']['platforms']['linux']['env'] = {

Yet another random comment - Are we setting MOZILLA_OFFICIAL and BUILD_OFFICIAL anywhere ? It changes a few things in the build (eg safe browsing request, search urls, rebasing win32 symbols) see
  http://mxr.mozilla.org/seamonkey/search?string=MOZILLA_OFFICIAL
  http://mxr.mozilla.org/seamonkey/search?string=BUILD_OFFICIAL
Tinderbox sets it even for nightlies so we have to be a bit careful not to introduce a divergence between nightly and release builds when moving to buildbot. Strangely I still see 
 [Crash Reporter]
 Enabled=1
in the application.ini from a linux nightly, despite the ifdef in browser/app/application.ini. Would be worth following up with someone like Ted.

Comment 4

[Nick Thomas [:nthomas] (UTC+12)]

Comment on attachment 321156 [details] [diff] [review]
phase 2: clean-up builder names a bit

>-        builders.append('%s-%s' % (name, platform))
>+        builders.append('%s firefox dep' % platform)

Not so keen on firefox for a couple of reasons. At this stage we're not planning on having multiple apps on one Tinderbox tree (right ?), so firefox would be as redundant as mozilla-central; you could argue it's building Minefield too.

Instead, I wonder if we should use something to differentiate from talos and unittest boxes, since I'm forever trying to pick out the box I care about in the sea of column headers. "build" would probably do, if you can stand the overloading. Any better suggestions spring to mind ?

Comment 5

[Rob Campbell [:rc] (:robcee)]

(In reply to comment #4)
> Instead, I wonder if we should use something to differentiate from talos and
> unittest boxes, since I'm forever trying to pick out the box I care about in
> the sea of column headers. "build" would probably do, if you can stand the
> overloading. Any better suggestions spring to mind ? 

We've talked about doing something like that before. I've brought up having separate trees for tracking performance and unittests from the mainline builders, but it was always rejected. I think we want to minimize the number of pages developers have to visit to track their patches through the various testing cycles. More clicks mean more chances that someone won't navigate to a page to see the results of a patch.

Comment 6

[[e-mail] (:bhearsum)]

Comment on attachment 321125 [details] [diff] [review]
[checked in] phase 1: move config section to an external file + reformat

Landed in changeset:   71:878ed42400f1 and changeset:   72:0ce07417b108

Comment 7

[Nick Thomas [:nthomas] (UTC+12)]

(In reply to comment #5)
Yeah, I agree with that. Just saying that if we have (the likes of)
   Linux mozilla-central qm-centos5-moz2-01 dep unit test
   Linux talos mozilla-central qm-plinux-trunk01
on the Mozilla2 tree, then 
   linux build dep
would be better than
   linux firefox dep

Comment 8

[[e-mail] (:bhearsum)]

(In reply to comment #7)
> (In reply to comment #5)
> Yeah, I agree with that. Just saying that if we have (the likes of)
>    Linux mozilla-central qm-centos5-moz2-01 dep unit test
>    Linux talos mozilla-central qm-plinux-trunk01
> on the Mozilla2 tree, then 
>    linux build dep
> would be better than
>    linux firefox dep
> 

This makes sense to me. I wonder if we should circulate this around m.d.platform or a moz2 meeting before we change it.

Comment 9

[[e-mail] (:bhearsum)]

Sorry Nick, I checked in that first patch before I read your review comments :(. I fixed the profiled_build one.

I'll set up a CNAME, moz2-linux-slave01 -> moz2-linux-slave01 (and win32). I've just added bm-xserve16...so should probably rename moz2-mac-slave1 -> bm-xserve17...

MOZILLA_OFFICIAL is in the mozconfigs. Shall we set BUILD_OFFICIAL, too?

Comment 10

[[e-mail] (:bhearsum)]

Alright, I've added the CNAMEs.

Comment 11

[[e-mail] (:bhearsum)]

Attachment: [checked in] renamed moz2-mac-slave1 -> bm-xserve17

We'll have to update BuildSlaves.py on production-master when we make this change, as well as the buildbot.tac file on the slave.

Comment 12

[[e-mail] (:bhearsum)]

Comment on attachment 321771 [details] [diff] [review]
[checked in] renamed moz2-mac-slave1 -> bm-xserve17

landed in changeset:   79:d5d628e27bf7

Comment 13

[Nick Thomas [:nthomas] (UTC+12)]

Thanks for the machine naming changes.

> This makes sense to me. I wonder if we should circulate this around
> m.d.platform or a moz2 meeting before we change it.

Could be useful. The current names are quite verbose and not very easy to parse quickly, but they are complete.

Comment 14

[Nick Thomas [:nthomas] (UTC+12)]

(In reply to comment #9)
> MOZILLA_OFFICIAL is in the mozconfigs. Shall we set BUILD_OFFICIAL, too?

Ah, ok. Running through the uses of BUILD_OFFICIAL according to mxr
* config/mkdepend/Makefile.in - unsetting the var for this dir
* config/version.mk - looks like either BUILD_OFFICIAL or MOZILLA_OFFICIAL is sufficient
* config/autoconf.mk.in - just a substitution
* tools/tinderbox/build-seamonkey-util.pl - tinderbox setting the var when ReleaseBuild is true
[* seamonkey setting it for good measure]
* browser/components/search/Makefile.in - either will do
* browser/components/safebrowsing/Makefile.in - either will do
* security/coreconf/ruleset.mk - changes error handling
* configure, configure.in - substitutions

So it looks like BUILD_OFFICIAL only makes a difference to security/ if MOZILLA_OFFICIAL is already set. It doesn't really make sense to me to ignore errors only for release builds, but go figure.

Comment 15

[[e-mail] (:bhearsum)]

Attachment: [checked in] phase 2.1: rename builders, take 2

This is much more in-line with the existing names on 1.8/1.9, and addresses the comments in the m.d.planning thread.

Comment 16

[Nick Thomas [:nthomas] (UTC+12)]

Comment on attachment 322244 [details] [diff] [review]
[checked in] phase 2.1: rename builders, take 2

Looks ok to me. Are you coordinating the Talos change with Alice ?

Comment 17

[[e-mail] (:bhearsum)]

That's a good point, I'll do that.

Comment 18

[[e-mail] (:bhearsum)]

Attachment: have talos watch the newly-named columns

Incidentally, I think this will resolve bug 433863.

Comment 19

[[e-mail] (:bhearsum)]

Attachment: have talos watch the newly-named columns

Incidentally, I think this will resolve bug 433863.

Comment 20

[[e-mail] (:bhearsum)]

Comment on attachment 323370 [details] [diff] [review]
have talos watch the newly-named columns

wth?

Comment 21

[[e-mail] (:bhearsum)]

Attachment: [checked in] only watch a single column, for now

As per IRC, only watch the regular dep builds for now. My previous patch will be tested in talos staging for a few days, and once we know it's good we'll get it on production talos.

Comment 22

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

Comment on attachment 323406 [details] [diff] [review]
[checked in] only watch a single column, for now

Looks good.

Comment 23

[[e-mail] (:bhearsum)]

Comment on attachment 322244 [details] [diff] [review]
[checked in] phase 2.1: rename builders, take 2

landed in changeset:   100:b3b62b95e2cd

Comment 24

[[e-mail] (:bhearsum)]

Comment on attachment 323406 [details] [diff] [review]
[checked in] only watch a single column, for now

Checking in master.cfg;
/cvsroot/mozilla/tools/buildbot-configs/testing/talos/perfmaster/master.cfg,v  <--  master.cfg
new revision: 1.65; previous revision: 1.64
done

Comment 25

[[e-mail] (:bhearsum)]

Okay, the moz2 & talos masters have been reconfig'ed with these patches. Any new builds should be in new columns now. I'll keep an eye on things for a bit.

Comment 26

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

The test patch to watch multiple columns has been failing on stage.  I'll attempt to determine what the problem is and report back a new patch.

Comment 27

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

Attachment: have talos watch the newly-named columns

Comment 28

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

Checking in master.cfg;
/cvsroot/mozilla/tools/buildbot-configs/testing/talos/perfmaster/master.cfg,v  <--  master.cfg
new revision: 1.68; previous revision: 1.67
done

Comment 29

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

Just need to let this bake overnight to see if mozilla-central nightlies are correctly picked up by talos - that will show that the patch to check multiple columns is working correctly.

Comment 30

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

Nightlies are successfully being picked up and tested.

Is there anything left to do for this bug?

Comment 31

[[e-mail] (:bhearsum)]

Still want to do more clean-up, part of which is in the open dependent bug. Not going to get to this anytime soon, though.

Comment 32

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

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