[Buildbot-devel] Extending ShellCommand

[Brian Warner]

> From: "Fred L. Drake, Jr." <fdrake at acm.org>
> 
> The "bots" configuration key is currently iterated over (twice) when the 
> configuration is loaded.  I'd like to be able to start a master and add 
> slaves "on the fly" without affecting the general status display or active 
> slave connections.

For what it's worth, that's how it ought to work right now. The ['bots']
configuration entry is only used when a buildslave connects. Changing it (and
reloading the config file with a SIGHUP or the debug tool) will not cause any
change to existing builders.

The same is true with pretty much everything in the config file. The
['builders'] list is examined with an algorithm that strives to leave
unmodified entries alone. Existing Builders which have not been changed in
the config file are not touched: builds keep running, histories are left
intact. If you change some aspect of the Builder (like its factory), then the
Builder is deleted and re-instantiated (keeping the build history). If you
delete the Builder altogether, then the history is deleted too.

The things I have marked as TODO include:

 when a bot is removed from ['bots'], hang up on it

 when a Builder is changed, wait until the current Build has finished before
 enacting the change

 when a Builder is deleted, tell the slave to halt the current build before
 hanging up on it

My goal is to allow slaves or the buildmaster to be stopped and restarted
without losing too much progress on any build that's in process. It has been
a lot harder to do this than I originally thought, so I've kept putting it
off. (some steps can be re-started, like 'make all', but interrupting
something like a CVS export means you have to start again from scratch..
which means you need a way to express this distinction to the Build so it can
know how to handle an interruption. You also need unique step-ID-values so
you can tell if a slave is still running the command you told it to work on
before you got disconnected).

> Perhaps a nicer approach would be to allow an alternate checker to be used;
> this would be easier if the checker could be passed in to the BuildMaster
> constructor or if there was a factory method that the constructor used to
> actually create the checker.

I agree, and that would let you use more complex or centralized
authentication mechanisms (DB-backed schemes, RADIUS or something). My
concern would be how to keep it easy to use the simple
InMemoryUsernamePasswordDatabaseDontUse-style checker. Maybe ['bots'] could
either be a dict or an instance which supports IChecker.

At some point in the near future (probably not in the next release, but
probably the one after that) I'm going to be working on the "Dispatcher",
with the goal of making it possible to put multiple PB-based services on the
same port. Currently we use PB for the debug port, the PBChangeSource inlet,
and the slave connections, and you have to use a different TCP port for each
one. By having those listeners coordinate username/password information, it
should be possible to have them share a port, which would make configuration
more straightforward. Part of that upgrade will involve playing around with
multiple Checkers on a given port, and that will probably be a good time to
permit this config-file-supplied Checker stuff.

> If I could use an arbitrary checker, I'd be able to reference an external
> file that contains the sensitive client -> passwd mapping. That would also
> make it easier to separate general configuration from security-sensitive
> information (a specific goal for me).

Just so you know, you can split them out into separate pieces right now, by
doing an 'import private' in your config file. I use this for the Twisted
buildmaster: its master.cfg file is included in the Buildbot tarball in
docs/examples/twisted.cfg, but we leave out the "private.py" that sits next
to master.cfg in the actual Twisted buildmaster's runtime directory. That
file contains the bot passwords and the debug password. The buildmaster has
to be able to read them both, but for documentation purposes the master.cfg
file by itself is sufficient.


> I'm not sure of the best way to wire things together since I'm not familiar
> with the "mktap" system at all.

One advantage of moving to the reloadable master.cfg file is that mktap is
barely involved. You have to set the base directory and the name of the
config file at mktap time, but after that everything is controlled by the
config file. So changes like this don't require anything Twisted-specific:
the logic is contained entirely within BuildMaster.loadConfig() .

cheers,
 -Brian