Slave: add 'info' dictionary

Make the 'info' dictionary more public and provide hooks to update it and set it

master/buildbot/buildslave/base.py

@@ -175,25 +175,10 @@ def _lockReleased(self):
         """One of the locks for this slave was released; try scheduling
         builds."""
         if not self.botmaster:
-            return  # oh well..
+            return # oh well..
         self.botmaster.maybeStartBuildsForSlave(self.slavename)
 
-    def _applySlaveInfo(self, info):
-        if not info:
-            return
-
-        self.slave_status.setAdmin(info.get("admin"))
-        self.slave_status.setHost(info.get("host"))
-        self.slave_status.setAccessURI(info.get("access_uri"))
-        self.slave_status.setVersion(info.get("version"))
-
-    def _saveSlaveInfoDict(self):
-        slaveinfo = {
-            'admin': self.slave_status.getAdmin(),
-            'host': self.slave_status.getHost(),
-            'access_uri': self.slave_status.getAccessURI(),
-            'version': self.slave_status.getVersion(),
-        }
+    def _saveSlaveInfoDict(self, slaveinfo):
         return self.master.db.buildslaves.updateBuildslave(
             name=self.slavename,
             slaveinfo=slaveinfo,
@@ -207,7 +192,7 @@ def applyInfo(buildslave):
             if buildslave is None:
                 return
 
-            self._applySlaveInfo(buildslave.get('slaveinfo'))
+            self.slave_status.updateInfo(**buildslave['slaveinfo'])
 
         return d
 
@@ -220,6 +205,7 @@ def setServiceParent(self, parent):
     def startService(self):
         self.updateLocks()
         self.startMissingTimer()
+        self.slave_status.addInfoWatcher(self._saveSlaveInfoDict)
         d = self._getSlaveInfo()
         d.addCallback(lambda _: service.MultiService.startService(self))
         return d
@@ -274,6 +260,7 @@ def reconfigService(self, new_config):
                                                                 new_config)
 
     def stopService(self):
+        self.slave_status.removeInfoWatcher(self._saveSlaveInfoDict)
         if self.registration:
             self.registration.unregister()
             self.registration = None
@@ -477,7 +464,12 @@ def _commands_unavailable(why):
         def _accept_slave(res):
             self.slave_status.setConnected(True)
 
-            self._applySlaveInfo(state)
+            self.slave_status.updateInfo(
+                admin=state.get("admin"),
+                host=state.get("host"),
+                access_uri=state.get("access_uri"),
+                version=state.get("version"),
+            )
 
             self.slave_commands = state.get("slave_commands")
             self.slave_environ = state.get("slave_environ")
@@ -495,8 +487,6 @@ def _accept_slave(res):
             self.stopMissingTimer()
             self.botmaster.master.status.slaveConnected(self.slavename)
 
-        d.addCallback(lambda _: self._saveSlaveInfoDict())
-
         d.addCallback(lambda _: self.updateSlave())
 
         d.addCallback(lambda _:

master/buildbot/status/slave.py

@@ -16,18 +16,15 @@
 import time
 
 from buildbot import interfaces
-from buildbot.util import ascii2unicode
+from buildbot.util import ascii2unicode, json
 from buildbot.util.eventual import eventually
 from zope.interface import implements
+import copy
 
 
 class SlaveStatus:
     implements(interfaces.ISlaveStatus)
 
-    admin = None
-    host = None
-    access_uri = None
-    version = None
     connected = False
     graceful_shutdown = False
     paused = False
@@ -37,22 +34,24 @@ def __init__(self, name):
         self._lastMessageReceived = 0
         self.runningBuilds = []
         self.graceful_callbacks = []
+        self.info = {}
+        self.info_change_callbacks = []
         self.connect_times = []
 
     def getName(self):
         return self.name
 
     def getAdmin(self):
-        return self.admin
+        return self.getInfo('admin')
 
     def getHost(self):
-        return self.host
+        return self.getInfo('host')
 
     def getAccessURI(self):
-        return self.access_uri
+        return self.getInfo('access_uri')
 
     def getVersion(self):
-        return self.version
+        return self.getInfo('version')
 
     def isConnected(self):
         return self.connected
@@ -71,16 +70,16 @@ def getConnectCount(self):
         return len([t for t in self.connect_times if t > then])
 
     def setAdmin(self, admin):
-        self.admin = ascii2unicode(admin)
+        self.updateInfo(admin=admin)
 
     def setHost(self, host):
-        self.host = ascii2unicode(host)
+        self.updateInfo(host=host)
 
     def setAccessURI(self, access_uri):
-        self.access_uri = access_uri
+        self.updateInfo(access_uri=access_uri)
 
     def setVersion(self, version):
-        self.version = version
+        self.updateInfo(version=version)
 
     def setConnected(self, isConnected):
         self.connected = isConnected
@@ -124,6 +123,50 @@ def removeGracefulWatcher(self, watcher):
         if watcher in self.graceful_callbacks:
             self.graceful_callbacks.remove(watcher)
 
+    def getInfoAsDict(self):
+        return copy.deepcopy(self.info)
+
+    def setInfoDict(self, info):
+        self.info = info
+
+    def getInfo(self, key, default=None):
+        return self.info.get(key, default)
+
+    def updateInfo(self, **kwargs):
+        # round-trip the value through json to 'normalize' it and
+        # to ensure bad values dont get stuffed into the dictionary
+        new_values = json.loads(json.dumps(kwargs))
+
+        for special_key in ['admin', 'host']:
+            if special_key in new_values:
+                new_values[special_key] = ascii2unicode(new_values[special_key])
+
+        # try to see if anything changed (so we should inform watchers)
+        for k, v in new_values.iteritems():
+            if k not in self.info:
+                break
+            if self.info[k]!=v:
+                break
+        else:
+            # nothing changed so just bail now
+            return
+
+        self.info.update(new_values)
+
+        for watcher in self.info_change_callbacks:
+            eventually(watcher, self.getInfoAsDict())
+
+    def hasInfo(self, key):
+        return key in self.info
+
+    def addInfoWatcher(self, watcher):
+        if not watcher in self.info_change_callbacks:
+            self.info_change_callbacks.append(watcher)
+
+    def removeInfoWatcher(self, watcher):
+        if watcher in self.info_change_callbacks:
+            self.info_change_callbacks.remove(watcher)
+
     def asDict(self):
         result = {}
         # Constant
@@ -136,4 +179,5 @@ def asDict(self):
         result['version'] = self.getVersion()
         result['connected'] = self.isConnected()
         result['runningBuilds'] = [b.asDict() for b in self.getRunningBuilds()]
+        result['info'] = self.getInfoAsDict()
         return result

master/buildbot/test/fake/fakedb.py

@@ -27,6 +27,7 @@
 from buildbot.util import json
 from twisted.internet import defer
 from twisted.internet import reactor
+from copy import deepcopy
 
 # Fake DB Rows
 
@@ -864,10 +865,17 @@ def insertTestData(self, rows):
                 })
 
     def getBuildslaves(self):
-        return defer.succeed([])
+        return defer.succeed([{
+                    'name':    s['name'],
+                    'slaveid': s['slaveid'],
+        } for s in self.buildslaves])
 
     def getBuildslaveByName(self, name):
-        return defer.succeed(self._getBuildslaveByName(name))
+        buildslave = self._getBuildslaveByName(name)
+        if buildslave is not None:
+            # XX: make a deep-copy to avoid side effects
+            buildslave = deepcopy(buildslave)
+        return defer.succeed(buildslave)
 
     def _getBuildslaveByName(self, name):
         for slave in self.buildslaves:
@@ -876,6 +884,7 @@ def _getBuildslaveByName(self, name):
         return None
 
     def updateBuildslave(self, name, slaveinfo):
+        slaveinfo = deepcopy(slaveinfo)
         slave = self._getBuildslaveByName(name)
         if slave is None:
             self.insertTestData([

master/buildbot/test/unit/test_buildslave_base.py

@@ -22,6 +22,7 @@
 from buildbot.test.fake import fakemaster
 from buildbot.test.fake import pbmanager
 from buildbot.test.fake.botmaster import FakeBotMaster
+from buildbot.util import eventual
 from twisted.internet import defer
 from twisted.internet import reactor
 from twisted.internet import task
@@ -41,6 +42,9 @@ def setUp(self):
         self.patch(reactor, 'callLater', self.clock.callLater)
         self.patch(reactor, 'seconds', self.clock.seconds)
 
+    def tearDown(self):
+        self.clock.pump([0])
+
     def createBuildslave(self, name='bot', password='pass', **kwargs):
         slave = self.ConcreteBuildSlave(name, password, **kwargs)
         slave.master = self.master
@@ -262,7 +266,8 @@ def test_startService_getSlaveInfo_fromDb(self):
                 'admin': 'TheAdmin',
                 'host': 'TheHost',
                 'access_uri': 'TheURI',
-                'version': 'TheVersion'
+                'version': 'TheVersion',
+                'key': 'value',
             })
         ])
         slave = self.createBuildslave()
@@ -273,6 +278,35 @@ def test_startService_getSlaveInfo_fromDb(self):
         self.assertEqual(slave.slave_status.getHost(), 'TheHost')
         self.assertEqual(slave.slave_status.getAccessURI(), 'TheURI')
         self.assertEqual(slave.slave_status.getVersion(), 'TheVersion')
+        self.assertEqual(slave.slave_status.getInfo('key'), 'value')
+
+    @defer.inlineCallbacks
+    def test_startService_setSlaveInfo_UpdatesDb(self):
+        self.master.db.insertTestData([
+            fakedb.Buildslave(name='bot', info={ 
+                'admin': 'TheAdmin',
+                'host': 'TheHost',
+                'access_uri': 'TheURI',
+                'version': 'TheVersion',
+                'key': 'value',
+            })
+        ])
+        slave = self.createBuildslave()
+        yield slave.startService()
+
+        # change a value
+        slave.slave_status.updateInfo(key='new-value')
+        self.clock.pump([0]) # we overrode the reactor, so gotta force the calls
+        yield eventual.flushEventualQueue()
+
+        # and the db is updated too:
+        slave_db = yield self.master.db.buildslaves.getBuildslaveByName("bot")
+
+        self.assertEqual(slave_db['slaveinfo']['admin'], 'TheAdmin')
+        self.assertEqual(slave_db['slaveinfo']['host'], 'TheHost')
+        self.assertEqual(slave_db['slaveinfo']['access_uri'], 'TheURI')
+        self.assertEqual(slave_db['slaveinfo']['version'], 'TheVersion')
+        self.assertEqual(slave_db['slaveinfo']['key'], 'new-value' )
 
     def createRemoteBot(self):
         class Bot():
@@ -403,7 +437,8 @@ def test_attached_slaveInfoUpdates(self):
                 'admin': 'WrongAdmin',
                 'host': 'WrongHost',
                 'access_uri': 'WrongURI',
-                'version': 'WrongVersion'
+                'version': 'WrongVersion',
+                'key': 'value',
             })
         ])
         slave = self.createBuildslave()
@@ -422,6 +457,10 @@ def test_attached_slaveInfoUpdates(self):
         self.assertEqual(slave.slave_status.getHost(), 'TheHost')
         self.assertEqual(slave.slave_status.getAccessURI(), 'TheURI')
         self.assertEqual(slave.slave_status.getVersion(), 'TheVersion')
+        self.assertEqual(slave.slave_status.getInfo('key'), 'value')
+
+        self.clock.pump([0]) # we overrode the reactor, so gotta force the calls
+        yield eventual.flushEventualQueue()
 
         # and the db is updated too:
         buildslave = yield self.master.db.buildslaves.getBuildslaveByName("bot")