611149 - make buildbot-tac.py optionally download .tac from an allocator

Status: RESOLVED INVALID

(Release Engineering :: General, defect)

Description

[Dustin J. Mitchell [:dustin] (he/him)]

The idea is that /etc/init.d/buildbot-tac will be improved to also send a URL prefix to buildbot-tac.py, and buildbot-tac.py will attempt to download the .tac file from this URL, falling back to the usual tac-generation process if that fails.

Is it worth handling passwords specially in all of this?  For example, the allocator on the other end of the URL could return a tac file containing

  passwd = %%PASSWORD%%

and buildbot-tac.py could substitute that appropriately -- pretty easy.

Comment 1

[Dustin J. Mitchell [:dustin] (he/him)]

Attachment: 611149-buildbot-tac-py.patch

This is the patch to buildbot-tac.py.  Unless I've screwed something up, this can land now -- if it's not called with --allocator-url, it will do the same thing as always.  Of course, I did some refactoring so I might have broken something.  Hence the need for review!

I've tested the --allocator-url using
  http://people.mozilla.com/~dmitchell/allocator/SLAVE
and it seems to work quite well.  I'm not sure if I've missed any necessary features, though?  Of course, in the final analysis we can use a URL like http://build.mozilla.org/slave-allocator/tac?slave=SLAVE

Comment 2

[Chris AtLee [:catlee]]

Comment on attachment 489665 [details] [diff] [review]
611149-buildbot-tac-py.patch

>-    slavename = options.slavename or socket.gethostname().split('.')[0]
>-    tacOptions,header,footer = get_default_options(slavename)
>-    tacOptions['passwd'] = quote_option(options.password)
>-    if options.basedir:
>-        tacOptions['basedir'] = quote_option(options.basedir, raw=True)
>-    if options.buildmaster:
>-        tacOptions['buildmaster'] = quote_option(options.buildmaster)
>+    if not options.allocator_url:
>+        if not options.password:
>+            parser.error("--password is required")

Looks good!  We always need to be specifying a password though, so just this bit needs to be adjusted.

Comment 3

[Dustin J. Mitchell [:dustin] (he/him)]

Landing this will involve rolling new packages and deploying them with puppet, so I'll need to figure out how to do that.  If buildbot-tac is also part of the package, then I should roll that in, too.

Comment 5

[Dustin J. Mitchell [:dustin] (he/him)]

OK, well, that patch won't work at all, review or no.

It looks like the talos windows slaves aren't managed at all.  Do talos slaves need to support dynamic master allocation?

Comment 6

[Dustin J. Mitchell [:dustin] (he/him)]

OK, back to the drawing board here..

Comment 7

[[e-mail] (:bhearsum)]

Which slaves did you find do not have buildbot-tac.py? I thought they all did. If any of them don't, they should!

Comment 8

[Dustin J. Mitchell [:dustin] (he/him)]

For the record, it is only Windows 7 test slaves that are not managed by OPSI, and even those have buildbot-tac.py installed manually.

Comment 9

[[e-mail] (:bhearsum)]

(In reply to comment #8)
> For the record, it is only Windows 7 test slaves that are not managed by OPSI,
> and even those have buildbot-tac.py installed manually.

Only 64-bit Windows 7 has buildbot-tac.py, actually. 32-bit ones do not.