schedulers/manager.py: make it a BuildbotServiceManager

To do so, this commit implements three refactorings
- make ClusteredService become a BuildbotService
- create measured_service, to factorize the metrics
  method calls during reconfigServiceWithBuildbotConfig
- make schedulers/manager.py (and buildslave/manager.py)
  become a measured_service.MeasuredBuildbotServiceManager

master/buildbot/buildslave/manager.py

@@ -14,7 +14,7 @@
 # Copyright Buildbot Team Members
 
 from buildbot.buildslave.protocols import pb as bbpb
-from buildbot.process import metrics
+from buildbot.process.measured_service import MeasuredBuildbotServiceManager
 from buildbot.util import misc
 from buildbot.util import service
 from twisted.internet import defer
@@ -54,9 +54,11 @@ def getPBPort(self):
         return self.pbReg.getPort()
 
 
-class BuildslaveManager(service.BuildbotServiceManager):
+class BuildslaveManager(MeasuredBuildbotServiceManager):
+
+    name = "BuildslaveManager"
+    managed_services_name = "buildslaves"
 
-    name = "buildslaves"
     config_attr = "slaves"
     PING_TIMEOUT = 10
     reconfig_priority = 127
@@ -73,20 +75,6 @@ def __init__(self, master):
         # connection objects keyed by buildslave name
         self.connections = {}
 
-    @defer.inlineCallbacks
-    def reconfigServiceWithBuildbotConfig(self, new_config):
-
-        timer = metrics.Timer("BuildSlaveManager.reconfigServiceSlaves")
-        timer.start()
-
-        yield service.BuildbotServiceManager.reconfigServiceWithBuildbotConfig(self,
-                                                                               new_config)
-
-        metrics.MetricCountEvent.log("num_slaves",
-                                     len(self.slaves), absolute=True)
-
-        timer.stop()
-
     @property
     def slaves(self):
         # self.slaves contains a ready BuildSlave instance for each

master/buildbot/changes/base.py

@@ -22,9 +22,12 @@
 from buildbot.util.poll import method as poll_method
 
 
-class ChangeSource(service.ClusteredService):
+class ChangeSource(service.ClusteredBuildbotService):
     implements(IChangeSource)
 
+    def __init__(self, name):
+        super(ChangeSource, self).__init__(name=name)
+
     def describe(self):
         pass
 

master/buildbot/changes/pb.py

@@ -96,7 +96,7 @@ def perspective_addChange(self, changedict):
         return d
 
 
-class PBChangeSource(service.ReconfigurableServiceMixin, base.ChangeSource):
+class PBChangeSource(base.ChangeSource):
     compare_attrs = ("user", "passwd", "port", "prefix", "port")
 
     def __init__(self, user="change", passwd="changepw", port=None,

master/buildbot/master.py

@@ -147,7 +147,7 @@ def create_child_services(self):
         self.botmaster = BotMaster(self)
         self.botmaster.setServiceParent(self)
 
-        self.scheduler_manager = SchedulerManager(self)
+        self.scheduler_manager = SchedulerManager()
         self.scheduler_manager.setServiceParent(self)
 
         self.user_manager = UserManagerManager(self)

master/buildbot/process/measured_service.py

@@ -0,0 +1,32 @@
+# This file is part of Buildbot.  Buildbot is free software: you can
+# redistribute it and/or modify it under the terms of the GNU General Public
+# License as published by the Free Software Foundation, version 2.
+#
+# This program is distributed in the hope that it will be useful, but WITHOUT
+# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS
+# FOR A PARTICULAR PURPOSE.  See the GNU General Public License for more
+# details.
+#
+# You should have received a copy of the GNU General Public License along with
+# this program; if not, write to the Free Software Foundation, Inc., 51
+# Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+#
+# Copyright Buildbot Team Members
+
+from twisted.internet import defer
+
+from buildbot.process import metrics
+from buildbot.util.service import BuildbotServiceManager
+
+
+class MeasuredBuildbotServiceManager(BuildbotServiceManager):
+    managed_services_name = "services"
+
+    @defer.inlineCallbacks
+    def reconfigServiceWithBuildbotConfig(self, new_config):
+        timer = metrics.Timer("{}.reconfigServiceWithBuildbotConfig".format(self.name))
+        timer.start()
+        yield super(MeasuredBuildbotServiceManager, self).reconfigServiceWithBuildbotConfig(new_config)
+        metrics.MetricCountEvent.log("num_{}".format(self.managed_services_name),
+                                     len(list(self)), absolute=True)
+        timer.stop()

master/buildbot/schedulers/base.py

@@ -17,26 +17,26 @@
 from buildbot import interfaces
 from buildbot.changes import changes
 from buildbot.process.properties import Properties
-from buildbot.util.service import ClusteredService
+from buildbot.util.service import ClusteredBuildbotService
 from buildbot.util.state import StateMixin
 from twisted.internet import defer
 from twisted.python import failure
 from twisted.python import log
 from zope.interface import implements
 
 
-class BaseScheduler(ClusteredService, StateMixin):
+class BaseScheduler(ClusteredBuildbotService, StateMixin):
 
     implements(interfaces.IScheduler)
 
     DEFAULT_CODEBASES = {'': {}}
 
-    compare_attrs = ClusteredService.compare_attrs + \
+    compare_attrs = ClusteredBuildbotService.compare_attrs + \
         ('builderNames', 'properties', 'codebases')
 
     def __init__(self, name, builderNames, properties=None,
                  codebases=DEFAULT_CODEBASES):
-        ClusteredService.__init__(self, name)
+        super(BaseScheduler, self).__init__(name=name)
 
         ok = True
         if not isinstance(builderNames, (list, tuple)):
@@ -58,7 +58,6 @@ def __init__(self, name, builderNames, properties=None,
         self.properties.update(properties, "Scheduler")
         self.properties.setProperty("scheduler", name, "Scheduler")
         self.objectid = None
-        self.master = None
 
         # Set the codebases that are necessary to process the changes
         # These codebases will always result in a sourcestamp with or without
@@ -321,3 +320,9 @@ def addBuildsetForSourceStamps(self, waited_for=False, sourcestamps=None,
             waited_for=waited_for, properties=properties_dict, builderids=builderids,
             external_idstring=external_idstring, **kw)
         defer.returnValue((bsid, brids))
+
+    @defer.inlineCallbacks
+    def reconfigService(self, *args, **kwargs):
+        if self.running:
+            yield self.stopService()
+        yield self.startService()

master/buildbot/schedulers/manager.py

@@ -13,87 +13,10 @@
 #
 # Copyright Buildbot Team Members
 
-from buildbot import util
-from buildbot.process import metrics
-from buildbot.util import service as util_service
-from twisted.application import service
-from twisted.internet import defer
-from twisted.python import log
-from twisted.python import reflect
+from buildbot.process.measured_service import MeasuredBuildbotServiceManager
 
 
-class SchedulerManager(util_service.ReconfigurableServiceMixin,
-                       util_service.AsyncMultiService):
-
-    def __init__(self, master):
-        service.MultiService.__init__(self)
-        self.setName('scheduler_manager')
-        self.master = master
-
-    @defer.inlineCallbacks
-    def reconfigServiceWithBuildbotConfig(self, new_config):
-        timer = metrics.Timer("SchedulerManager.reconfigServiceWithBuildbotConfig")
-        timer.start()
-
-        old_by_name = dict((sch.name, sch) for sch in self)
-        old_set = set(old_by_name.iterkeys())
-        new_by_name = new_config.schedulers
-        new_set = set(new_by_name.iterkeys())
-
-        removed_names, added_names = util.diffSets(old_set, new_set)
-
-        # find any schedulers that don't know how to reconfig, and, if they
-        # have changed, add them to both removed and added, so that we
-        # run the new version.  While we're at it, find any schedulers whose
-        # fully qualified class name has changed, and consider those a removal
-        # and re-add as well.
-        for n in old_set & new_set:
-            old = old_by_name[n]
-            new = new_by_name[n]
-            # detect changed class name
-            if reflect.qual(old.__class__) != reflect.qual(new.__class__):
-                removed_names.add(n)
-                added_names.add(n)
-
-            # compare using ComparableMixin if they don't support reconfig
-            elif not hasattr(old, 'reconfigServiceWithBuildbotConfig'):
-                if old != new:
-                    removed_names.add(n)
-                    added_names.add(n)
-
-        # removals first
-
-        for sch_name in removed_names:
-            log.msg("removing scheduler '%s'" % (sch_name,))
-            sch = old_by_name[sch_name]
-            yield defer.maybeDeferred(lambda:
-                                      sch.disownServiceParent())
-            sch.master = None
-
-        # .. then additions
-
-        for sch_name in added_names:
-            log.msg("adding scheduler '%s'" % (sch_name,))
-            sch = new_by_name[sch_name]
-
-            # get the scheduler's objectid
-            class_name = '%s.%s' % (sch.__class__.__module__,
-                                    sch.__class__.__name__)
-            objectid = yield self.master.db.state.getObjectId(
-                sch.name, class_name)
-
-            # set up the scheduler
-            sch.objectid = objectid
-            sch.master = self.master
-
-            # *then* attach and start it
-            yield sch.setServiceParent(self)
-
-        metrics.MetricCountEvent.log("num_schedulers", len(list(self)),
-                                     absolute=True)
-
-        # reconfig any newly-added schedulers, as well as existing
-        yield util_service.ReconfigurableServiceMixin.reconfigServiceWithBuildbotConfig(self,
-                                                                                        new_config)
-
-        timer.stop()
+class SchedulerManager(MeasuredBuildbotServiceManager):
+    name = "SchedulerManager"
+    managed_services_name = "schedulers"
+    config_attr = "schedulers"

master/buildbot/test/unit/test_changes_base.py

@@ -32,8 +32,6 @@ class Subclass(base.ChangeSource):
     def setUp(self):
         yield self.setUpChangeSource()
 
-        self.attachChangeSource(self.Subclass(name="DummyCS"))
-
     def tearDown(self):
         return self.tearDownChangeSource()
 
@@ -47,7 +45,7 @@ def test_activation(self):
         self.attachChangeSource(cs)
         self.setChangeSourceToMaster(self.OTHER_MASTER_ID)
 
-        cs.startService()
+        yield cs.startService()
         cs.clock.advance(cs.POLL_INTERVAL_SEC / 2)
         cs.clock.advance(cs.POLL_INTERVAL_SEC / 5)
         cs.clock.advance(cs.POLL_INTERVAL_SEC / 5)
@@ -57,7 +55,10 @@ def test_activation(self):
         self.assertEqual(cs.serviceid, self.DUMMY_CHANGESOURCE_ID)
 
         # clear that masterid
+        yield cs.stopService()
         self.setChangeSourceToMaster(None)
+
+        yield cs.startService()
         cs.clock.advance(cs.POLL_INTERVAL_SEC)
         self.assertTrue(cs.activate.called)
         self.assertFalse(cs.deactivate.called)
@@ -102,7 +103,6 @@ def test_loop_loops(self):
 
         self.changesource.pollInterval = 5
         self.startChangeSource()
-
         d = defer.Deferred()
         d.addCallback(self.runClockFor, 12)