488411 - Simplify slave listings

Status: RESOLVED FIXED

(Release Engineering :: General, defect)

Description

[Chris AtLee [:catlee]]

There have been several cases when new slaves are added to one section in config.py, but not to others.  We need to have a simpler way of listing these slaves in our configuration.

Comment 1

[Chris AtLee [:catlee]]

Attachment: Simplify slave listings

BuildSlaves.py would also have to be updated to contain this:
SlavePasswords = {
        'linux': 'password',
        'linux64': 'password',
        'win32': 'password',
        'macosx': 'password',
    }

Comment 2

[[e-mail] (:bhearsum)]

Comment on attachment 372763 [details] [diff] [review]
Simplify slave listings

>diff --git a/mozilla2-staging/config.py b/mozilla2-staging/config.py
>--- a/mozilla2-staging/config.py
>+++ b/mozilla2-staging/config.py
>@@ -30,6 +30,20 @@
> # talos master should result in a warning
> TALOS_MASTERS = [('qm-buildbot01.mozilla.org:9987', False)]
> 
>+SLAVES = {
>+    'linux': ['moz2-linux-slave%02i' % x for x in range(1,26)],
>+    'linux64': ['moz2-linux64-slave%02i' % x for x in range(1,2)],
>+    'win32': ['moz2-win32-slave%02i' % x for x in range(1,30)],
>+    'macosx': ['moz2-darwin9-slave%02i' % x for x in range(1,13)] + [
>+        'bm-xserve12',
>+        'bm-xserve16',
>+        'bm-xserve17',
>+        'bm-xserve18',
>+        'bm-xserve19',
>+        'bm-xserve22',
>+        ],
>+}

I'm torn on this. This is fine for staging, and readable enough, but in production we skip slaves on every platform. I wonder if it would just be better if we list all of the slaves explicitly at the top. What do you think?


> import BuildSlaves
> reload(BuildSlaves)
>-c['slaves'] = BuildSlaves.SlaveList
> 
> # most of the config is in an external file
> import config
> reload(config)
> from config import *
> 
>+from buildbot.buildslave import BuildSlave
>+c['slaves'] = []
>+for platform, names in SLAVES.items():
>+    c['slaves'].extend(BuildSlave(name, BuildSlaves.SlavePasswords[platform], max_builds=1) for name in names)

This line is pretty opaque. I think:
for name in names:
    c['slaves'].extend(...)

would be easier to read.

Comment 3

[Chris AtLee [:catlee]]

(In reply to comment #2)
> (From update of attachment 372763 [details] [diff] [review])
> >diff --git a/mozilla2-staging/config.py b/mozilla2-staging/config.py
> >--- a/mozilla2-staging/config.py
> >+++ b/mozilla2-staging/config.py
> >@@ -30,6 +30,20 @@
> > # talos master should result in a warning
> > TALOS_MASTERS = [('qm-buildbot01.mozilla.org:9987', False)]
> > 
> >+SLAVES = {
> >+    'linux': ['moz2-linux-slave%02i' % x for x in range(1,26)],
> >+    'linux64': ['moz2-linux64-slave%02i' % x for x in range(1,2)],
> >+    'win32': ['moz2-win32-slave%02i' % x for x in range(1,30)],
> >+    'macosx': ['moz2-darwin9-slave%02i' % x for x in range(1,13)] + [
> >+        'bm-xserve12',
> >+        'bm-xserve16',
> >+        'bm-xserve17',
> >+        'bm-xserve18',
> >+        'bm-xserve19',
> >+        'bm-xserve22',
> >+        ],
> >+}
> 
> I'm torn on this. This is fine for staging, and readable enough, but in
> production we skip slaves on every platform. I wonder if it would just be
> better if we list all of the slaves explicitly at the top. What do you think?

Here's what it could look like on production:
SLAVES = {
    'linux': ['moz2-linux-slave%02i' % x for x in (set(range(1,26)) - set([3,4,17]))],
    'linux64': ['moz2-linux64-slave%02i' % x for x in range(1,2)],
    'win32': ['moz2-win32-slave%02i' % x for x in (set(range(1,30)) - set([3,4,21]))],
    'macosx': ['moz2-darwin9-slave%02i' % x for x in (set(range(1,8)) - set([1,3,4]))] + [
               'bm-xserve%02i' for x in [12,16,17,18,19,22]],
}

It's a bit messier than the staging version, but not too bad.  We could also have a separate listing of the excluded slaves.

In any case, having all the slaves listed in one place is better than having them repeated 5 times!

Comment 4

[Chris AtLee [:catlee]]

Attachment: Simplify slave listings

I also took the opportunity to have a subset of the slaves used for l10n.   8 slaves per OS seemed like a good number to start with.

Comment 5

[Axel Hecht [:Pike]]

Comment on attachment 373946 [details] [diff] [review]
Simplify slave listings

I'd prefer to keep the information about the local slave config in BuildSlaves instead of hard-coding it in config.py. It'd be nice if all the info about the local setup would be factored in one file for folks without access to the staging and production masters, possibly with a example in the tree.

I'm also surprised to see more slaves in staging than in production.

Comment 6

[Chris AtLee [:catlee]]

(In reply to comment #5)
> (From update of attachment 373946 [details] [diff] [review])
> I'd prefer to keep the information about the local slave config in BuildSlaves
> instead of hard-coding it in config.py. It'd be nice if all the info about the
> local setup would be factored in one file for folks without access to the
> staging and production masters, possibly with a example in the tree.

Not sure what you mean by this.  We could include a template BuildSlaves.py if that's what you need.  Mainly we want to keep the buildbot slave passwords out of public source control.

> I'm also surprised to see more slaves in staging than in production.

We list all the slaves in staging to make it easier to move slaves back into staging from production if a machine needs to be examined.

Comment 7

[[e-mail] (:bhearsum)]

Comment on attachment 373946 [details] [diff] [review]
Simplify slave listings

>diff --git a/mozilla2-staging/config.py b/mozilla2-staging/config.py
>--- a/mozilla2-staging/config.py
>+++ b/mozilla2-staging/config.py
>@@ -30,6 +30,20 @@
> # talos master should result in a warning
> TALOS_MASTERS = [('qm-buildbot01.mozilla.org:9987', False)]
> 
>+SLAVES = {
>+    'linux': ['moz2-linux-slave%02i' % x for x in range(1,26)],
>+    'linux64': ['moz2-linux64-slave%02i' % x for x in range(1,2)],
>+    'win32': ['moz2-win32-slave%02i' % x for x in range(1,30)],
>+    'macosx': ['moz2-darwin9-slave%02i' % x for x in range(1,13)] + [
>+               'bm-xserve%02i' % x for x in [12,16,17,18,19,22]],
>+}
>+
>+L10N_SLAVES = {
>+    'linux': SLAVES['linux'][:8],
>+    'win32': SLAVES['win32'][:8],
>+    'macosx': SLAVES['macosx'][:8],
>+}

When you land this, can you make sure you add a comment to bug 489333 about this happening here? In the future, it's probably best to keep changes like this separate, though.

Comment 8

[Chris AtLee [:catlee]]

Comment on attachment 373946 [details] [diff] [review]
Simplify slave listings

changeset:   1104:5e9a6efbfc5f

Comment 9

[Chris AtLee [:catlee]]

Now live in production