make the integration test work

new services, and deleted services works as expected on reconfig

Signed-off-by: Pierre Tardy <pierre.tardy@intel.com>

master/buildbot/config.py

@@ -591,17 +591,13 @@ def load_services(self, filename, config_dict):
         if 'services' not in config_dict:
             return
         self.services = {}
-        for serviceFactory in config_dict['services']:
-            if not isinstance(serviceFactory, util_service.CustomServiceFactory):
-                error("%r object should be an instance of buildbot.util.service.CustomServiceFactory" %
-                      (type(serviceFactory)))
+        for _service in config_dict['services']:
+            if not isinstance(_service, util_service.ReconfigurableService):
+                error("%r object should be an instance of buildbot.util.service.ReconfigurableService" %
+                      (type(_service)))
                 continue
 
-            if not issubclass(serviceFactory.klass, util_service.CustomService):
-                error("%r should be an instance of buildbot.util.service.CustomService" %
-                      (serviceFactory.klass))
-                continue
-            self.services[serviceFactory.name] = serviceFactory
+            self.services[_service.name] = _service
 
     def check_single_master(self):
         # check additional problems that are only valid in a single-master

master/buildbot/master.py

@@ -253,7 +253,7 @@ def sigusr1(*args):
             yield self.doMasterHouseKeeping(self.masterid)
 
             for serviceFactory in self.config.services.values():
-                yield serviceFactory.createService(self)
+                yield serviceFactory.setServiceParent(self)
 
             # call the parent method
             yield service.AsyncMultiService.startService(self)
@@ -342,10 +342,19 @@ def doReconfig(self):
         changes_made = False
         failed = False
         try:
+            old_config = self.config
             new_config = config.MasterConfig.loadConfig(self.basedir,
                                                         self.configFileName)
             changes_made = True
             self.config = new_config
+            for name in old_config.services.keys():
+                if name not in new_config.services:
+                    self.namedServices[name].disownServiceParent()
+
+            for name in new_config.services.keys():
+                if name not in old_config.services:
+                    new_config.services[name].setServiceParent(self)
+
             yield self.reconfigService(new_config)
 
         except config.ConfigErrors, e:

master/buildbot/test/integration/test_customservices.py

@@ -62,6 +62,20 @@ def test_customService(self):
         self.failUnlessEqual(myService.num_reconfig, 2)
         self.failUnlessEqual(build['steps'][0]['state_string'], 'num reconfig: 2')
 
+        yield m.reconfig()
+
+        myService2 = m.namedServices['myService2']
+
+        self.failUnlessEqual(myService2.num_reconfig, 3)
+        self.failUnlessEqual(myService.num_reconfig, 3)
+
+        yield m.reconfig()
+
+        # second service removed
+        self.failIfIn('myService2', m.namedServices)
+        self.failUnlessEqual(myService2.num_reconfig, 3)
+        self.failUnlessEqual(myService.num_reconfig, 4)
+
 
 # master configuration
 
@@ -104,4 +118,6 @@ def reconfigServiceWithConstructorArgs(self, num_reconfig):
                       factory=f)]
 
     c['services'] = [MyService(num_reconfig=num_reconfig)]
+    if num_reconfig == 3:
+        c['services'].append(MyService(name="myService2", num_reconfig=num_reconfig))
     return c

master/buildbot/util/service.py

@@ -226,21 +226,24 @@ def addService(self, service):
             return defer.succeed(None)
 
 
-class ReconfigurableService(AsyncMultiService, config.configuredMixin,
+class ReconfigurableService(AsyncMultiService, config.ConfiguredMixin,
                             ReconfigurableServiceMixin, util.ComparableMixin):
     compare_attrs = ('name', 'config', 'config_attr')
     config_attr = "services"
     name = None
+    configured = False
 
     def __init__(self, name=None, *args, **kwargs):
         if name is not None:
             self.name = name
         if self.name is None:
-            config.error("%s: should pass a name to constructor" % (type(self),))
+            config.error("%s: must pass a name to constructor" % (type(self),))
             return
         self.config_args = args
         self.config_kwargs = kwargs
 
+        AsyncMultiService.__init__(self)
+
     def getConfigDict(self):
         _type = type(self)
         return {'name': self.name,
@@ -253,8 +256,11 @@ def reconfigService(self, new_config):
         config_sibling = getattr(new_config, self.config_attr)[self.name]
 
         # only reconfigure if different as ComparableMixin says.
-        if config_sibling != self:
-            return self.reconfigServiceWithConstructorArgs(*self.config_args, **self.config_kwargs)
+        if self.configured and config_sibling == self:
+            return defer.succeed(None)
+        self.configured = True
+        return self.reconfigServiceWithConstructorArgs(*config_sibling.config_args,
+                                                       **config_sibling.config_kwargs)
 
     def setServiceParent(self, parent):
         self.master = parent