Merge remote-tracking branch 'upstream/master'

master/NEWS

@@ -0,0 +1 @@
+docs/release-notes.txt

master/buildbot/buildslave.py

@@ -26,13 +26,15 @@
 
 from buildbot.status.slave import SlaveStatus
 from buildbot.status.mail import MailNotifier
-from buildbot.process import metrics
+from buildbot.process import metrics, botmaster
 from buildbot.interfaces import IBuildSlave, ILatentBuildSlave
 from buildbot.process.properties import Properties
 from buildbot.locks import LockAccess
 from buildbot.util import subscription
+from buildbot import config
 
-class AbstractBuildSlave(pb.Avatar, service.MultiService):
+class AbstractBuildSlave(config.ReconfigurableServiceMixin, pb.Avatar,
+                        service.MultiService):
     """This is the master-side representative for a remote buildbot slave.
     There is exactly one for each slave described in the config file (the
     c['slaves'] list). When buildbots connect in (.attach), they get a
@@ -67,7 +69,16 @@ def __init__(self, name, password, max_builds=None,
         service.MultiService.__init__(self)
         self.slavename = name
         self.password = password
-        self.botmaster = None # no buildmaster yet
+
+        # PB registration
+        self.registration = None
+        self.registered_port = None
+
+        # these are set when the service is started, and unset when it is
+        # stopped
+        self.botmaster = None
+        self.master = None
+
         self.slave_status = SlaveStatus(name)
         self.slave = None # a RemoteReference to the Bot, when connected
         self.slave_commands = None
@@ -86,7 +97,9 @@ def __init__(self, name, password, max_builds=None,
             notify_on_missing = [notify_on_missing]
         self.notify_on_missing = notify_on_missing
         for i in notify_on_missing:
-            assert isinstance(i, str)
+            if not isinstance(i, str):
+                raise config.ConfigErrors([
+                    'notify_on_missing arg %r is not a string' % (i,) ])
         self.missing_timeout = missing_timeout
         self.missing_timer = None
         self.keepalive_interval = keepalive_interval
@@ -95,48 +108,8 @@ def __init__(self, name, password, max_builds=None,
 
         self._old_builder_list = None
 
-    def identity(self):
-        """
-        Return a tuple describing this slave.  After reconfiguration a
-        new slave with the same identity will update this one, rather
-        than replacing it, thereby avoiding an interruption of current
-        activity.
-        """
-        return (self.slavename, self.password, 
-                '%s.%s' % (self.__class__.__module__,
-                           self.__class__.__name__))
-
-    def update(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.
-        """
-        # the reconfiguration logic should guarantee this:
-        assert self.slavename == new.slavename
-        assert self.password == new.password
-        assert self.identity() == new.identity()
-        self.max_builds = new.max_builds
-        self.access = new.access
-        self.notify_on_missing = new.notify_on_missing
-        self.missing_timeout = new.missing_timeout
-        self.keepalive_interval = new.keepalive_interval
-
-        self.properties = Properties()
-        self.properties.updateFromProperties(new.properties)
-
-        if self.botmaster:
-            self.updateLocks()
-
     def __repr__(self):
-        if self.botmaster:
-            builders = self.botmaster.getBuildersForSlave(self.slavename)
-            return "<%s '%s', current builders: %s>" % \
-               (self.__class__.__name__, self.slavename,
-                ','.join(map(lambda b: b.name, builders)))
-        else:
-            return "<%s '%s', (no builders yet)>" % \
-                (self.__class__.__name__, self.slavename)
+        return "<%s %r>" % (self.__class__.__name__, self.slavename)
 
     def updateLocks(self):
         # convert locks into their real form
@@ -184,23 +157,98 @@ def releaseLocks(self):
         for lock, access in self.locks:
             lock.release(self, access)
 
-    def setBotmaster(self, botmaster):
-        assert not self.botmaster, "BuildSlave already has a botmaster"
-        self.botmaster = botmaster
+    def startService(self):
         self.updateLocks()
         self.startMissingTimer()
+        return service.MultiService.startService(self)
 
-    def stopMissingTimer(self):
-        if self.missing_timer:
-            self.missing_timer.cancel()
-            self.missing_timer = None
+    def reconfigService(self, new_config):
+        # 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.
+        new = self.findNewSlaveInstance(new_config)
+
+        assert self.slavename == new.slavename
+
+        # do we need to re-register?
+        if (not self.registration or
+            self.password != new.password or
+            new_config.slavePortnum != self.registered_port):
+            if self.registration:
+                self.registration.unregister()
+            self.password = new.password
+            self.registered_port = new_config.slavePortnum
+            self.registration = self.master.pbmanager.register(
+                    self.registered_port, self.slavename,
+                    self.password, self.getPerspective)
+
+        # adopt new instance's configuration parameters
+        self.max_builds = new.max_builds
+        self.access = new.access
+        self.notify_on_missing = new.notify_on_missing
+        self.keepalive_interval = new.keepalive_interval
+
+        if self.missing_timeout != new.missing_timeout:
+            running_missing_timer = self.missing_timer
+            self.stopMissingTimer()
+            self.missing_timeout = new.missing_timeout
+            if running_missing_timer:
+                self.startMissingTimer()
+
+        self.properties = Properties()
+        self.properties.updateFromProperties(new.properties)
+
+        self.updateLocks()
+
+        # update the attached slave's notion of which builders are attached
+        d = self.updateSlave()
+
+        # and chain up
+        d.addCallback(lambda _ :
+            config.ReconfigurableServiceMixin.reconfigService(self,
+                                                            new_config))
+
+        return d
+
+    def stopService(self):
+        self.stopMissingTimer()
+        return service.MultiService.stopService(self)
+
+    def findNewSlaveInstance(self, new_config):
+        # TODO: called multiple times per reconfig; use 1-element cache?
+        for sl in new_config.slaves:
+            if sl.name == self.name:
+                return sl
+        assert 0, "no new slave named '%s'" % self.name
 
     def startMissingTimer(self):
         if self.notify_on_missing and self.missing_timeout and self.parent:
             self.stopMissingTimer() # in case it's already running
             self.missing_timer = reactor.callLater(self.missing_timeout,
                                                 self._missing_timer_fired)
 
+    def stopMissingTimer(self):
+        if self.missing_timer:
+            self.missing_timer.cancel()
+            self.missing_timer = None
+
+    def getPerspective(self, mind, slavename):
+        assert slavename == self.slavename
+        metrics.MetricCountEvent.log("attached_slaves", 1)
+
+        # record when this connection attempt occurred
+        if self.slave_status:
+            self.slave_status.recordConnectTime()
+
+
+        if self.isConnected():
+            # duplicate slave - send it to arbitration
+            arb = botmaster.DuplicateSlaveArbitrator(self)
+            return arb.getPerspective(mind, slavename)
+        else:
+            log.msg("slave '%s' attaching from %s" % (slavename, mind.broker.transport.getPeer()))
+            return self
+
     def doKeepalive(self):
         self.keepalive_timer = reactor.callLater(self.keepalive_interval,
                                                 self.doKeepalive)
@@ -219,10 +267,6 @@ def startKeepaliveTimer(self):
                                         (self.slavename, ))
         self.doKeepalive()
 
-    def recordConnectTime(self):
-        if self.slave_status:
-            self.slave_status.recordConnectTime()
-
     def isConnected(self):
         return self.slave
 
@@ -473,9 +517,8 @@ def _disconnected(rref):
 
     def sendBuilderList(self):
         our_builders = self.botmaster.getBuildersForSlave(self.slavename)
-        blist = [(b.name, b.slavebuilddir) for b in our_builders]
+        blist = [(b.name, b.config.slavebuilddir) for b in our_builders]
         if blist == self._old_builder_list:
-            log.msg("Builder list is unchanged; not calling setBuilderList")
             return defer.succeed(None)
 
         d = self.slave.callRemote("setBuilderList", blist)

master/buildbot/changes/manager.py

@@ -14,12 +14,13 @@
 # Copyright Buildbot Team Members
 
 from zope.interface import implements
+from twisted.python import log
 from twisted.internet import defer
 from twisted.application import service
+from buildbot import interfaces, config, util
+from buildbot.process import metrics
 
-from buildbot import interfaces
-
-class ChangeManager(service.MultiService):
+class ChangeManager(config.ReconfigurableServiceMixin, service.MultiService):
     """
     This is the master-side service which receives file change notifications
     from version-control systems.
@@ -31,29 +32,47 @@ class ChangeManager(service.MultiService):
 
     implements(interfaces.IEventSource)
 
-    lastPruneChanges = None
     name = "changemanager"
 
-    def __init__(self):
+    def __init__(self, master):
         service.MultiService.__init__(self)
-        self.master = None
-        self.lastPruneChanges = 0
-
-    def startService(self):
-        service.MultiService.startService(self)
-        self.master = self.parent
-
-    def addSource(self, source):
-        assert interfaces.IChangeSource.providedBy(source)
-        assert service.IService.providedBy(source)
-        source.master = self.master
-        source.setServiceParent(self)
-
-    def removeSource(self, source):
-        assert source in self
-        d = defer.maybeDeferred(source.disownServiceParent)
-        def unset_master(x):
-            source.master = None
-            return x
-        d.addBoth(unset_master)
-        return d
+        self.setName('change_manager')
+        self.master = master
+
+    @defer.deferredGenerator
+    def reconfigService(self, new_config):
+        timer = metrics.Timer("ChangeManager.reconfigService")
+        timer.start()
+
+        removed, added = util.diffSets(
+                set(self),
+                new_config.change_sources)
+
+        if removed or added:
+            log.msg("adding %d new changesources, removing %d" %
+                    (len(added), len(removed)))
+
+            for src in removed:
+                wfd = defer.waitForDeferred(
+                    defer.maybeDeferred(
+                        src.disownServiceParent))
+                yield wfd
+                wfd.getResult()
+                src.master = None
+
+            for src in added:
+                src.master = self.master
+                src.setServiceParent(self)
+
+        num_sources = len(list(self))
+        assert num_sources == len(new_config.change_sources)
+        metrics.MetricCountEvent.log("num_sources", num_sources, absolute=True)
+
+        # reconfig any newly-added change sources, as well as existing
+        wfd = defer.waitForDeferred(
+            config.ReconfigurableServiceMixin.reconfigService(self,
+                                                        new_config))
+        yield wfd
+        wfd.getResult()
+
+        timer.stop()