[Buildbot-devel] Slave concurrency?

Dustin J. Mitchell dustin at zmanda.com
Sat Jun 23 05:44:23 UTC 2007


Updated patches based on Jean-Paul's comments attached -- I'll avoid
spamming the list with each in its own message this time.

Any other feedback?

Dustin

-- 
        Dustin J. Mitchell
        Storage Software Engineer, Zmanda, Inc.
        http://www.zmanda.com/
-------------- next part --------------
2007-06-23  Dustin J. Mitchell <dustin at zmanda.com>
    This patch changes the bot configuration to use objects instead of
    tuples.  This will allow us to move some bot-specific functionality
    into BuildSlave, and also allow users to subclass BuildSlave for their
    own purposes.
    * buildbot/buildslave.py: add new BuildSlave class, suitable for use
      in master.cfg
    * buildbot/master.py: use the BotPerspective subclass, BuildSlave, 
      as a configuration source; support consumption of successors' souls
      as a reconfiguration technique
    * docs/buildbot.texinfo buildbot/scripts/sample.cfg: documentation
    * buildbot/test/test_config.py: update to use new bot configuration

Index: wip/buildbot/buildslave.py
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ wip/buildbot/buildslave.py	2007-06-22 18:15:07.570996764 -0500
@@ -0,0 +1,29 @@
+import string
+
+from twisted.python import log
+
+from buildbot.master import BotPerspective
+
+class BuildSlave(BotPerspective):
+    """I represent a build slave -- a connection from a remote machine capable of
+    running builds.  I am instantiated by the configuration file, and can be
+    subclassed to add extra functionality."""
+
+    def __init__(self, name, password):
+        BotPerspective.__init__(self, name)
+        self.password = password
+
+    def consumeTheSoulOfYourSuccessor(self, new):
+        """
+        Given a new BuildSlave, configure this one identically.  Because
+        BuildSlave objects are remotely referenced, we can't replace them
+        without disconnecting the slave, yet there's no reason to do that.
+        """
+        BotPerspective.consumeTheSoulOfYourSuccessor(self, new)
+        self.password = new.password
+
+    def __repr__(self):
+        builders = self.botmaster.getBuildersForSlave(self.slavename)
+        return "<BuildSlave '%s', current builders: %s>" % \
+               (self.slavename,
+                string.join(map(lambda b: b.name, builders), ','))
Index: wip/buildbot/master.py
===================================================================
--- wip.orig/buildbot/master.py	2007-06-22 18:15:05.227080613 -0500
+++ wip/buildbot/master.py	2007-06-22 18:15:07.570996764 -0500
@@ -42,13 +42,21 @@
     reference to this instance. The BotMaster object is stashed as the
     .service attribute."""
 
-    def __init__(self, name, botmaster):
+    def __init__(self, name):
         self.slavename = name
-        self.botmaster = botmaster
+        self.botmaster = None # as-yet unowned
         self.slave_status = SlaveStatus(name)
         self.slave = None # a RemoteReference to the Bot, when connected
         self.slave_commands = None
 
+    def consumeTheSoulOfYourSuccessor(self, new):
+        # the reconfiguration logic should guarantee this:
+        assert self.slavename == new.slavename
+
+    def setBotmaster(self, botmaster):
+        assert not self.botmaster, "BotPerspective already has a botmaster"
+        self.botmaster = botmaster
+
     def updateSlave(self):
         """Called to add or remove builders after the slave has connected.
 
@@ -301,9 +309,9 @@
         return defer.succeed(None)
 
 
-    def addSlave(self, slavename):
-        slave = BotPerspective(slavename, self)
-        self.slaves[slavename] = slave
+    def addSlave(self, slave):
+        slave.setBotmaster(self)
+        self.slaves[slave.slavename] = slave
 
     def removeSlave(self, slavename):
         d = self.slaves[slavename].disconnect()
@@ -530,7 +538,7 @@
 
         self.statusTargets = []
 
-        self.bots = []
+        self.bots = {}
         # this ChangeMaster is a dummy, only used by tests. In the real
         # buildmaster, where the BuildMaster instance is activated
         # (startService is called) by twistd, this attribute is overwritten.
@@ -678,9 +686,14 @@
             log.msg("leaving old configuration in place")
             raise
 
+        # keep compatibility with old 'bots' format
+        if bots and type(bots[0]) in (type(()), type([])):
+            from buildbot.buildslave import BuildSlave # work around circular import
+            bots = [ BuildSlave(name, pw) for name, pw in bots ]
+
         # do some validation first
-        for name, passwd in bots:
-            if name in ("debug", "change", "status"):
+        for bot in bots:
+            if bot.slavename in ("debug", "change", "status"):
                 raise KeyError, "reserved name '%s' used for a bot" % name
         if config.has_key('interlocks'):
             raise KeyError("c['interlocks'] is no longer accepted")
@@ -698,7 +711,7 @@
         for s in status:
             assert interfaces.IStatusReceiver(s, None)
 
-        slavenames = [name for name,pw in bots]
+        slavenames = [ bot.slavename for bot in bots ]
         buildernames = []
         dirnames = []
         for b in builders:
@@ -848,19 +861,32 @@
 
     def loadConfig_Slaves(self, bots):
         # set up the Checker with the names and passwords of all valid bots
+
+        # update bot authentication
         self.checker.users = {} # violates abstraction, oh well
-        for user, passwd in bots:
-            self.checker.addUser(user, passwd)
+        for bot in bots:
+            self.checker.addUser(bot.slavename, bot.password)
         self.checker.addUser("change", "changepw")
 
         # identify new/old bots
-        old = self.bots; oldnames = [name for name,pw in old]
-        new = bots; newnames = [name for name,pw in new]
+        old = self.bots; oldnames = [ bot.slavename for bot in old ]
+        new = bots; newnames = [ bot.slavename for bot in new ]
+
         # removeSlave will hang up on the old bot
-        dl = [self.botmaster.removeSlave(name)
-              for name in oldnames if name not in newnames]
-        [self.botmaster.addSlave(name)
-         for name in newnames if name not in oldnames]
+        dl = [self.botmaster.removeSlave(botname)
+              for botname in oldnames if botname not in newnames]
+
+        # add all of the new bots
+        [self.botmaster.addSlave(bot)
+              for bot in new if bot.slavename not in oldnames]
+
+        # and reconfigure any existing bots, and slot the *old*
+        # object into 'bots'.
+        for oldbot in old:
+            for newbot in new:
+                if oldbot.slavename == newbot.slavename:
+                    oldbot.consumeTheSoulOfYourSuccessor(newbot)
+                    bots[bots.index(newbot)] = oldbot
 
         # all done
         self.bots = bots
Index: wip/buildbot/scripts/sample.cfg
===================================================================
--- wip.orig/buildbot/scripts/sample.cfg	2007-06-22 18:15:05.227080613 -0500
+++ wip/buildbot/scripts/sample.cfg	2007-06-22 18:15:07.570996764 -0500
@@ -17,9 +17,14 @@
 ####### BUILDSLAVES
 
 # the 'bots' list defines the set of allowable buildslaves. Each element is a
-# tuple of bot-name and bot-password. These correspond to values given to the
-# buildslave's mktap invocation.
-c['bots'] = [("bot1name", "bot1passwd")]
+# BuildSlave object containing the bot-name and bot-password. These correspond to
+# values given to the buildslave's mktap invocation.  A subclass can be used here
+# to allow further bot-specific configuration.
+from buildbot.buildslave import BuildSlave
+c['bots'] = []
+c['bots'].append(
+    BuildSlave("bot1name", "bot1passwd")
+)
 
 
 # 'slavePortnum' defines the TCP port to listen on. This must match the value
Index: wip/docs/buildbot.texinfo
===================================================================
--- wip.orig/docs/buildbot.texinfo	2007-06-22 18:15:05.227080613 -0500
+++ wip/docs/buildbot.texinfo	2007-06-22 18:15:07.574996621 -0500
@@ -2214,14 +2214,16 @@
 @bcindex c['bots']
 
 The @code{c['bots']} key is a list of known buildslaves. Each
-buildslave is defined by a tuple of (slavename, slavepassword). These
+buildslave is defined by a @code{BuildSlave} instance.  The
+ at code{BuildSlave} constructor requires a botname and password.  These
 are the same two values that need to be provided to the buildslave
 administrator when they create the buildslave.
 
 @example
-c['bots'] = [('bot-solaris', 'solarispasswd'),
-             ('bot-bsd', 'bsdpasswd'),
-            ]
+from buildbot.buildslave import BuildSlave
+c['bots'] = []
+c['bots'].append(BuildSlave("bot-solaris", "solarispasswd"))
+c['bots'].append(BuildSlave("bot-bsd", "bsdpasswd"))
 @end example
 
 The slavenames must be unique, of course. The password exists to
Index: wip/buildbot/test/test_config.py
===================================================================
--- wip.orig/buildbot/test/test_config.py	2007-06-22 18:00:12.007282137 -0500
+++ wip/buildbot/test/test_config.py	2007-06-22 18:22:07.744015541 -0500
@@ -46,11 +46,17 @@
 c['buildbotURL'] = 'http://dummy.example.com/buildbot'
 """
 
+oldBotCfg = emptyCfg + \
+"""
+c['bots'].append(('bot1', 'pw1'))
+"""
+
 buildersCfg = \
 """
 from buildbot.process.factory import BasicBuildFactory
+from buildbot.buildslave import BuildSlave
 BuildmasterConfig = c = {}
-c['bots'] = [('bot1', 'pw1')]
+c['bots'] = [BuildSlave('bot1', 'pw1')]
 c['sources'] = []
 c['schedulers'] = []
 c['slavePortnum'] = 9999
@@ -158,8 +164,9 @@
 interlockCfgBad = \
 """
 from buildbot.process.factory import BasicBuildFactory
+from buildbot.buildslave import BuildSlave
 c = {}
-c['bots'] = [('bot1', 'pw1')]
+c['bots'] = [BuildSlave('bot1', 'pw1')]
 c['sources'] = []
 c['schedulers'] = []
 f1 = BasicBuildFactory('cvsroot', 'cvsmodule')
@@ -181,8 +188,9 @@
 from buildbot.steps.dummy import Dummy
 from buildbot.process.factory import BuildFactory, s
 from buildbot.locks import MasterLock
+from buildbot.buildslave import BuildSlave
 c = {}
-c['bots'] = [('bot1', 'pw1')]
+c['bots'] = [BuildSlave('bot1', 'pw1')]
 c['sources'] = []
 c['schedulers'] = []
 l1 = MasterLock('lock1')
@@ -203,8 +211,9 @@
 from buildbot.steps.dummy import Dummy
 from buildbot.process.factory import BuildFactory, s
 from buildbot.locks import MasterLock, SlaveLock
+from buildbot.buildslave import BuildSlave
 c = {}
-c['bots'] = [('bot1', 'pw1')]
+c['bots'] = [BuildSlave('bot1', 'pw1')]
 c['sources'] = []
 c['schedulers'] = []
 l1 = MasterLock('lock1')
@@ -225,8 +234,9 @@
 from buildbot.steps.dummy import Dummy
 from buildbot.process.factory import BuildFactory, s
 from buildbot.locks import MasterLock
+from buildbot.buildslave import BuildSlave
 c = {}
-c['bots'] = [('bot1', 'pw1')]
+c['bots'] = [BuildSlave('bot1', 'pw1')]
 c['sources'] = []
 c['schedulers'] = []
 l1 = MasterLock('lock1')
@@ -247,8 +257,9 @@
 """
 from buildbot.process.factory import BasicBuildFactory
 from buildbot.locks import MasterLock
+from buildbot.buildslave import BuildSlave
 c = {}
-c['bots'] = [('bot1', 'pw1')]
+c['bots'] = [BuildSlave('bot1', 'pw1')]
 c['sources'] = []
 c['schedulers'] = []
 f1 = BasicBuildFactory('cvsroot', 'cvsmodule')
@@ -268,8 +279,9 @@
 """
 from buildbot.process.factory import BasicBuildFactory
 from buildbot.locks import MasterLock
+from buildbot.buildslave import BuildSlave
 c = {}
-c['bots'] = [('bot1', 'pw1')]
+c['bots'] = [BuildSlave('bot1', 'pw1')]
 c['sources'] = []
 c['schedulers'] = []
 f1 = BasicBuildFactory('cvsroot', 'cvsmodule')
@@ -291,8 +303,9 @@
 from buildbot.steps.dummy import Dummy
 from buildbot.process.factory import BuildFactory, s
 from buildbot.locks import MasterLock
+from buildbot.buildslave import BuildSlave
 c = {}
-c['bots'] = [('bot1', 'pw1')]
+c['bots'] = [BuildSlave('bot1', 'pw1')]
 c['sources'] = []
 c['schedulers'] = []
 l1 = MasterLock('lock1')
@@ -315,8 +328,9 @@
 from buildbot.steps.dummy import Dummy
 from buildbot.process.factory import BuildFactory, s
 from buildbot.locks import MasterLock
+from buildbot.buildslave import BuildSlave
 c = {}
-c['bots'] = [('bot1', 'pw1')]
+c['bots'] = [BuildSlave('bot1', 'pw1')]
 c['sources'] = []
 c['schedulers'] = []
 l1 = MasterLock('lock1')
@@ -339,8 +353,9 @@
 from buildbot.steps.dummy import Dummy
 from buildbot.process.factory import BuildFactory, s
 from buildbot.locks import MasterLock
+from buildbot.buildslave import BuildSlave
 c = {}
-c['bots'] = [('bot1', 'pw1')]
+c['bots'] = [BuildSlave('bot1', 'pw1')]
 c['sources'] = []
 c['schedulers'] = []
 l1 = MasterLock('lock1')
@@ -362,8 +377,9 @@
 """
 from buildbot.scheduler import Scheduler, Dependent
 from buildbot.process.factory import BasicBuildFactory
+from buildbot.buildslave import BuildSlave
 c = {}
-c['bots'] = [('bot1', 'pw1')]
+c['bots'] = [BuildSlave('bot1', 'pw1')]
 c['sources'] = []
 f1 = BasicBuildFactory('cvsroot', 'cvsmodule')
 b1 = {'name':'builder1', 'slavename':'bot1',
@@ -452,6 +468,15 @@
         self.failUnlessEqual(master.buildbotURL,
                              "http://dummy.example.com/buildbot")
 
+    def testOldBotConfig(self):
+        # covers slavePortnum, base checker passwords
+        master = self.buildmaster
+        master.loadChanges()
+
+        master.loadConfig(oldBotCfg)
+        self.failUnlessEqual(master.checker.users,
+                             {"bot1":"pw1", "change": "changepw"})
+
     def testSlavePortnum(self):
         master = self.buildmaster
         master.loadChanges()
@@ -481,8 +506,9 @@
         self.failUnlessEqual(master.botmaster.builders, {})
         self.failUnlessEqual(master.checker.users,
                              {"change": "changepw"})
-        botsCfg = (emptyCfg +
-                   "c['bots'] = [('bot1', 'pw1'), ('bot2', 'pw2')]\n")
+        botsCfg = (emptyCfg +
+                   "from buildbot.buildslave import BuildSlave\n" +
+                   "c['bots'] = [BuildSlave('bot1', 'pw1'), BuildSlave('bot2', 'pw2')]\n")
         master.loadConfig(botsCfg)
         self.failUnlessEqual(master.checker.users,
                              {"change": "changepw",
@@ -1107,8 +1133,9 @@
 from buildbot.process.factory import BuildFactory, s
 from buildbot.steps.shell import ShellCommand
 from buildbot.steps.source import Darcs
+from buildbot.buildslave import BuildSlave
 BuildmasterConfig = c = {}
-c['bots'] = [('bot1', 'pw1')]
+c['bots'] = [BuildSlave('bot1', 'pw1')]
 c['sources'] = []
 c['schedulers'] = []
 c['slavePortnum'] = 9999
-------------- next part --------------
2007-06-23  Dustin J. Mitchell <dustin at zmanda.com>
    * buildbot/buildslave.py: add canStartBuild()
    * buildbot/process/builder.py: call BuildSlave's canStartBuild()
    * buildbot/test/test_run.py: corresponding tests

Index: wip/buildbot/buildslave.py
===================================================================
--- wip.orig/buildbot/buildslave.py	2007-06-22 23:28:40.937399649 -0500
+++ wip/buildbot/buildslave.py	2007-06-23 00:39:19.771787015 -0500
@@ -5,7 +5,7 @@
 from buildbot.master import BotPerspective
 
 class BuildSlave(BotPerspective):
-    """I represent a build slave -- a connection from a remote machine capable of
+    """I represent a build slave -- a remote machine capable of
     running builds.  I am instantiated by the configuration file, and can be
     subclassed to add extra functionality."""
 
@@ -27,3 +27,11 @@
         return "<BuildSlave '%s', current builders: %s>" % \
                (self.slavename,
                 string.join(map(lambda b: b.name, builders), ','))
+
+    def canStartBuild(self):
+        """
+        I am called when a build is requested to see if this buildslave is
+        can start a build.  This function can be used to limit overall
+        concurrency on the buildslave.
+        """
+        return True
Index: wip/buildbot/process/builder.py
===================================================================
--- wip.orig/buildbot/process/builder.py	2007-06-22 23:28:40.941399518 -0500
+++ wip/buildbot/process/builder.py	2007-06-23 00:39:19.771787015 -0500
@@ -38,11 +38,27 @@
         return self.remoteCommands.get(command)
 
     def isAvailable(self):
-        if self.state == IDLE:
-            return True
+        # if this SlaveBuilder is busy, then it's definitely not available
+        if self.state != IDLE:
+            return False
+
+        # otherwise, check in with the BuildSlave
+        if self.slave:
+            return self.slave.canStartBuild()
+
+        # no slave? not very available.
         return False
 
     def attached(self, slave, remote, commands):
+        """
+        @type  slave: L{buildbot.buildslave.BuildSlave}
+        @param slave: the BuildSlave that represents the buildslave as a
+                      whole
+        @type  remote: L{twisted.spread.pb.RemoteReference}
+        @param remote: a reference to the L{buildbot.slave.bot.SlaveBuilder}
+        @type  commands: dict: string -> string, or None
+        @param commands: provides the slave's version of each RemoteCommand
+        """
         self.slave = slave
         self.remote = remote
         self.remoteCommands = commands # maps command name to version
@@ -392,8 +408,8 @@
         """This is invoked by the BotPerspective when the self.slavename bot
         registers their builder.
 
-        @type  slave: L{buildbot.master.BotPerspective}
-        @param slave: the BotPerspective that represents the buildslave as a
+        @type  slave: L{buildbot.buildslave.BuildSlave}
+        @param slave: the BuildSlave that represents the buildslave as a
                       whole
         @type  remote: L{twisted.spread.pb.RemoteReference}
         @param remote: a reference to the L{buildbot.slave.bot.SlaveBuilder}
@@ -502,7 +518,7 @@
         if not self.buildable:
             self.updateBigStatus()
             return # nothing to do
-        # find the first idle slave
+        # find the first slave that can run this build
         for sb in self.slaves:
             if sb.isAvailable():
                 break
Index: wip/buildbot/test/test_run.py
===================================================================
--- wip.orig/buildbot/test/test_run.py	2007-06-22 23:28:40.941399518 -0500
+++ wip/buildbot/test/test_run.py	2007-06-23 00:39:19.739787973 -0500
@@ -39,6 +39,23 @@
 c['schedulers'] = [Scheduler('quick', None, 120, ['quick'])]
 """
 
+config_can_build = config_base + """
+from buildbot.buildslave import BuildSlave
+c['bots'] = [ BuildSlave('bot1', 'sekrit') ]
+
+from buildbot.scheduler import Scheduler
+c['schedulers'] = [Scheduler('dummy', None, 0.1, ['dummy'])]
+
+c['builders'] = [{'name': 'dummy', 'slavename': 'bot1',
+                  'builddir': 'dummy1', 'factory': f2}]
+"""
+
+config_cant_build = config_can_build + """
+class MyBuildSlave(BuildSlave):
+    def canStartBuild(self): return False
+c['bots'] = [ MyBuildSlave('bot1', 'sekrit') ]
+"""
+
 config_2 = config_base + """
 c['builders'] = [{'name': 'dummy', 'slavename': 'bot1',
                   'builddir': 'dummy1', 'factory': f2},
@@ -90,6 +107,52 @@
         d = defer.maybeDeferred(m.stopService)
         return d
 
+class CanStartBuild(RunMixin, unittest.TestCase):
+    def rmtree(self, d):
+        rmtree(d)
+
+    def testCanStartBuild(self):
+        return self.do_test(config_can_build, True)
+
+    def testCantStartBuild(self):
+        return self.do_test(config_cant_build, False)
+
+    def do_test(self, config, builder_should_run):
+        self.master.loadConfig(config)
+        self.master.readConfig = True
+        self.master.startService()
+        d = self.connectSlave()
+
+        # send a change
+        cm = self.master.change_svc
+        c = changes.Change("bob", ["Makefile", "foo/bar.c"], "changed stuff")
+        cm.addChange(c)
+
+        d.addCallback(self._do_test1, builder_should_run)
+
+        return d
+
+    def _do_test1(self, res, builder_should_run):
+        # delay a little bit
+        d = defer.Deferred()
+        reactor.callLater(.5, d.callback, builder_should_run)
+        d.addCallback(self._do_test2)
+        return d
+
+    def _do_test2(self, builder_should_run):
+        b = self.master.botmaster.builders['dummy']
+        self.failUnless(len(b.slaves) == 1)
+
+        bs = b.slaves[0]
+        from buildbot.process.builder import IDLE, BUILDING
+        if builder_should_run:
+            self.failUnlessEqual(bs.state, BUILDING)
+        else:
+            self.failUnlessEqual(bs.state, IDLE)
+
+        d = defer.maybeDeferred(self.master.stopService)
+        return d
+
 class Ping(RunMixin, unittest.TestCase):
     def testPing(self):
         self.master.loadConfig(config_2)
-------------- next part --------------
2007-06-23  Dustin J. Mitchell <dustin at zmanda.com>
    * buildbot/buildslave.py: add addSlaveBuilder() and removeSlaveBuilder()
    * buildbot/process/builder.py: call BuildSlave's addSlaveBuilder() and 
      removeSlaveBuilder()
    * test/test_slaves.py: corresponding tests

Index: wip/buildbot/buildslave.py
===================================================================
--- wip.orig/buildbot/buildslave.py	2007-06-22 18:28:10.261843969 -0500
+++ wip/buildbot/buildslave.py	2007-06-22 23:27:13.284263232 -0500
@@ -12,6 +12,7 @@
     def __init__(self, name, password):
         BotPerspective.__init__(self, name)
         self.password = password
+        self.slavebuilders = []
 
     def consumeTheSoulOfYourSuccessor(self, new):
         """
@@ -28,6 +29,15 @@
                (self.slavename,
                 string.join(map(lambda b: b.name, builders), ','))
 
+    def addSlaveBuilder(self, sb):
+        log.msg("%s adding %s" % (self, sb))
+        self.slavebuilders.append(sb)
+
+    def removeSlaveBuilder(self, sb):
+        log.msg("%s removing %s" % (self, sb))
+        if sb in self.slavebuilders:
+            self.slavebuilders.remove(sb)
+
     def canStartBuild(self):
         """
         I am called when a build is requested to see if this buildslave is
Index: wip/buildbot/process/builder.py
===================================================================
--- wip.orig/buildbot/process/builder.py	2007-06-22 18:28:10.261843969 -0500
+++ wip/buildbot/process/builder.py	2007-06-22 19:13:49.986217652 -0500
@@ -62,6 +62,7 @@
         self.slave = slave
         self.remote = remote
         self.remoteCommands = commands # maps command name to version
+        self.slave.addSlaveBuilder(self)
         log.msg("Buildslave %s attached to %s" % (slave.slavename,
                                                   self.builder_name))
         d = self.remote.callRemote("setMaster", self)
@@ -89,6 +90,7 @@
     def detached(self):
         log.msg("Buildslave %s detached from %s" % (self.slave.slavename,
                                                     self.builder_name))
+        if self.slave: self.slave.removeSlaveBuilder(self)
         self.slave = None
         self.remote = None
         self.remoteCommands = None
Index: wip/buildbot/test/test_slaves.py
===================================================================
--- wip.orig/buildbot/test/test_slaves.py	2006-12-11 02:23:29.000000000 -0600
+++ wip/buildbot/test/test_slaves.py	2007-06-22 23:27:31.527667382 -0500
@@ -40,6 +40,7 @@
 
 """
 
+
 class Slave(RunMixin, unittest.TestCase):
 
     def setUp(self):
@@ -430,3 +431,34 @@
         reasons = [build.getReason() for build in builds]
         self.failUnlessEqual(reasons, ["first", "second"])
 
+config_multi_builders = config_1 + """
+c['builders'] = [
+    {'name': 'dummy', 'slavenames': ['bot1','bot2','bot3'],
+     'builddir': 'b1', 'factory': f2},
+    {'name': 'dummy2', 'slavenames': ['bot1','bot2','bot3'],
+     'builddir': 'b2', 'factory': f2},
+    {'name': 'dummy3', 'slavenames': ['bot1','bot2','bot3'],
+     'builddir': 'b3', 'factory': f2},
+    ]
+
+"""
+
+class BuildSlave(RunMixin, unittest.TestCase):
+    def test_BuildSlave_tracking_Builders(self):
+        self.master.loadConfig(config_multi_builders)
+        self.master.readConfig = True
+        self.master.startService()
+        d = self.connectSlave()
+        d.addCallback(self._test_BuildSlave_tracking_Builders1)
+        return d
+
+    def _test_BuildSlave_tracking_Builders1(self, res):
+        b = self.master.botmaster.builders['dummy']
+        self.failUnless(len(b.slaves) == 1) # just bot1
+
+        bs = b.slaves[0].slave
+        self.failUnless(len(bs.slavebuilders) == 3)
+
+        d = defer.maybeDeferred(self.master.stopService)
+        return d
+
-------------- next part --------------
2007-06-23  Dustin J. Mitchell <dustin at zmanda.com>
    * buildbot/buildslave.py buildbot/process/builder.py: Add a concurrency
      limit to the BuildSlave class, and use that to limit the total
      number of concurrent builds on a given BuildSlave.
    * buildbot/scripts/sample.cfg: describe new concurrency_limit keyword arg
    * buildbot/process/builder.py: call the botmaster's maybeStartAllBuilds
      when a build is finished, to potentially trigger a build from another
      builder.
    * buildbot/test/test_run.py: corresponding tests
    * docs/buildbot.texinfo: documentation

Index: wip/buildbot/buildslave.py
===================================================================
--- wip.orig/buildbot/buildslave.py	2007-06-22 23:29:37.723543495 -0500
+++ wip/buildbot/buildslave.py	2007-06-22 23:29:38.247526364 -0500
@@ -9,10 +9,18 @@
     running builds.  I am instantiated by the configuration file, and can be
     subclassed to add extra functionality."""
 
-    def __init__(self, name, password):
+    def __init__(self, name, password, concurrency_limit=None):
+        """
+        @param name: botname this machine will supply when it connects
+        @param password: password this machine will supply when
+            it connects
+        @param concurrency_limit: maximum number of simultaneous builds
+            (default: None for no limit)
+        """
         BotPerspective.__init__(self, name)
         self.password = password
         self.slavebuilders = []
+        self.concurrency_limit = concurrency_limit
 
     def consumeTheSoulOfYourSuccessor(self, new):
         """
@@ -22,6 +30,7 @@
         """
         BotPerspective.consumeTheSoulOfYourSuccessor(self, new)
         self.password = new.password
+        self.concurrency_limit = new.concurrency_limit
 
     def __repr__(self):
         builders = self.botmaster.getBuildersForSlave(self.slavename)
@@ -44,4 +53,11 @@
         can start a build.  This function can be used to limit overall
         concurrency on the buildslave.
         """
+        if self.concurrency_limit:
+            num_active_builders = len([
+                sb for sb in self.slavebuilders
+                if sb.isBusy()])
+            if num_active_builders >= self.concurrency_limit:
+                return False
+
         return True
Index: wip/buildbot/process/builder.py
===================================================================
--- wip.orig/buildbot/process/builder.py	2007-06-22 23:29:37.723543495 -0500
+++ wip/buildbot/process/builder.py	2007-06-22 23:29:38.247526364 -0500
@@ -39,7 +39,7 @@
 
     def isAvailable(self):
         # if this SlaveBuilder is busy, then it's definitely not available
-        if self.state != IDLE:
+        if self.isBusy():
             return False
 
         # otherwise, check in with the BuildSlave
@@ -49,6 +49,9 @@
         # no slave? not very available.
         return False
 
+    def isBusy(self):
+        return self.state in (PINGING, BUILDING)
+
     def attached(self, slave, remote, commands):
         """
         @type  slave: L{buildbot.buildslave.BuildSlave}
@@ -100,7 +103,7 @@
 
     def buildFinished(self):
         self.state = IDLE
-        reactor.callLater(0, self.builder.maybeStartBuild)
+        reactor.callLater(0, self.builder.botmaster.maybeStartAllBuilds)
 
     def ping(self, timeout, status=None):
         """Ping the slave to make sure it is still there. Returns a Deferred
Index: wip/buildbot/scripts/sample.cfg
===================================================================
--- wip.orig/buildbot/scripts/sample.cfg	2007-06-22 23:28:36.717537551 -0500
+++ wip/buildbot/scripts/sample.cfg	2007-06-22 23:29:38.251526233 -0500
@@ -26,6 +26,10 @@
     BuildSlave("bot1name", "bot1passwd")
 )
 
+# to limit to two concurrent builds on a slave, use
+#  c['bots'].append(
+#      BuildSlave("bot1name", "bot1passwd", concurrency_limit=2)
+#  )
 
 # 'slavePortnum' defines the TCP port to listen on. This must match the value
 # configured into the buildslaves (with their --master option)
Index: wip/docs/buildbot.texinfo
===================================================================
--- wip.orig/docs/buildbot.texinfo	2007-06-22 23:28:36.721537421 -0500
+++ wip/docs/buildbot.texinfo	2007-06-22 23:29:38.251526233 -0500
@@ -2235,6 +2235,18 @@
 will be rejected when they attempt to connect, and a message
 describing the problem will be put in the log file (see @ref{Logfiles}).
 
+The @code{BuildSlave} constructor can take an optional
+ at code{concurrency_limit} parameter to limit the number of builds that
+it will execute simultaneously:
+
+ at example
+from buildbot.buildslave import BuildSlave
+c['bots'] = []
+c['bots'].append(BuildSlave("bot-linux", "linuxpassword", concurrency_limit=2))
+ at end example
+
+The @code{BuildSlave} class can be subclassed to implement more
+sophisticated functionality at the level of a single buildslave.
 
 @node Defining Builders, Defining Status Targets, Buildslave Specifiers, Configuration
 @section Defining Builders
Index: wip/buildbot/test/test_run.py
===================================================================
--- wip.orig/buildbot/test/test_run.py	2007-06-22 23:29:36.251591618 -0500
+++ wip/buildbot/test/test_run.py	2007-06-22 23:44:54.206051076 -0500
@@ -56,6 +56,19 @@
 c['bots'] = [ MyBuildSlave('bot1', 'sekrit') ]
 """
 
+config_concurrency = config_base + """
+from buildbot.buildslave import BuildSlave
+c['bots'] = [ BuildSlave('bot1', 'sekrit', concurrency_limit=1) ]
+
+from buildbot.scheduler import Scheduler
+c['schedulers'] = [Scheduler('dummy', None, 0.1, ['dummy', 'dummy2'])]
+
+c['builders'].append({'name': 'dummy', 'slavename': 'bot1',
+                      'builddir': 'dummy', 'factory': f2})
+c['builders'].append({'name': 'dummy2', 'slavename': 'bot1',
+                      'builddir': 'dummy2', 'factory': f2})
+"""
+
 config_2 = config_base + """
 c['builders'] = [{'name': 'dummy', 'slavename': 'bot1',
                   'builddir': 'dummy1', 'factory': f2},
@@ -153,6 +166,45 @@
         d = defer.maybeDeferred(self.master.stopService)
         return d
 
+class ConcurrencyLimit(RunMixin, unittest.TestCase):
+    def rmtree(self, d):
+        rmtree(d)
+
+    def testConcurrencyLimit(self):
+        self.master.loadConfig(config_concurrency)
+        self.master.readConfig = True
+        self.master.startService()
+        d = self.connectSlave()
+
+        # send a change
+        cm = self.master.change_svc
+        c = changes.Change("bob", ["Makefile", "foo/bar.c"], "changed stuff")
+        cm.addChange(c)
+
+        d.addCallback(self._do_test1)
+
+        return d
+
+    def _do_test1(self, res):
+        # delay a little bit
+        d = defer.Deferred()
+        reactor.callLater(1, d.callback, None)
+        d.addCallback(self._do_test2)
+        return d
+
+    def _do_test2(self, res):
+        builders = [ self.master.botmaster.builders[bn] for bn in ('dummy', 'dummy2') ]
+        for builder in builders:
+            self.failUnless(len(builder.slaves) == 1)
+
+        from buildbot.process.builder import IDLE, BUILDING
+        building_bs = [ builder for builder in builders if builder.slaves[0].state == BUILDING ]
+
+        self.failUnlessEqual(len(building_bs), 1)
+
+        d = defer.maybeDeferred(self.master.stopService)
+        return d
+
 class Ping(RunMixin, unittest.TestCase):
     def testPing(self):
         self.master.loadConfig(config_2)


More information about the devel mailing list