Have buildZooKeeperHosts accept a config object

Add a new ZooKeeperServerConnection class which represents the
information needed to connect to a server, and then have the
config parser create that object instead of its own internal
representation of the connection.  Modify the ZooKeeper class
to accept this new object to specify what connection(s) it
should use.

The new class implements the equality operator so that it may
still be used in the same way as other ConfigValue objects (that
is, checking to see if the configuration has been updated).

Change-Id: I4cfa3866e3b0ae2a730c1e624570cd7025901808

nodepool/config.py

@@ -21,6 +21,7 @@ from six.moves import configparser as ConfigParser
 import yaml
 
 import fakeprovider
+import zk
 
 
 class ConfigValue(object):
@@ -102,10 +103,6 @@ class GearmanServer(ConfigValue):
     pass
 
 
-class ZooKeeperServer(ConfigValue):
-    pass
-
-
 class DiskImage(ConfigValue):
     pass
 
@@ -161,12 +158,11 @@ def loadConfig(config_path):
         newconfig.gearman_servers[g.name] = g
 
     for server in config.get('zookeeper-servers', []):
-        z = ZooKeeperServer()
+        z = zk.ZooKeeperConnectionConfig(server['host'],
-        z.host = server['host']
+                                         server.get('port', 2181),
-        z.port = server.get('port', 2181)
+                                         server.get('chroot', None))
-        z.chroot = server.get('chroot', '')
+        name = z.host + '_' + str(z.port)
-        z.name = z.host + '_' + str(z.port)
+        newconfig.zookeeper_servers[name] = z
-        newconfig.zookeeper_servers[z.name] = z
 
     for provider in config.get('providers', []):
         p = Provider()

nodepool/tests/test_zk.py

@@ -26,15 +26,18 @@ class TestZooKeeper(tests.ZKTestCase):
 
     def test_buildZooKeeperHosts_single(self):
         hosts = [
-            dict(host='127.0.0.1', port=2181, chroot='/test1')
+            zk.ZooKeeperConnectionConfig('127.0.0.1', port=2181,
+                                         chroot='/test1')
         ]
         self.assertEqual('127.0.0.1:2181/test1',
                          zk.buildZooKeeperHosts(hosts))
 
     def test_buildZooKeeperHosts_multiple(self):
         hosts = [
-            dict(host='127.0.0.1', port=2181, chroot='/test1'),
+            zk.ZooKeeperConnectionConfig('127.0.0.1', port=2181,
-            dict(host='127.0.0.2', port=2182, chroot='/test2')
+                                         chroot='/test1'),
+            zk.ZooKeeperConnectionConfig('127.0.0.2', port=2182,
+                                         chroot='/test2')
         ]
         self.assertEqual('127.0.0.1:2181/test1,127.0.0.2:2182/test2',
                          zk.buildZooKeeperHosts(hosts))
@@ -89,9 +92,9 @@ class TestZooKeeper(tests.ZKTestCase):
 
     def test_imageBuildLock_exception_nonblocking(self):
         zk2 = zk.ZooKeeper()
-        zk2.connect([{'host': self.zookeeper_host,
+        zk2.connect([zk.ZooKeeperConnectionConfig(self.zookeeper_host,
-                      'port': self.zookeeper_port,
+                                                  port=self.zookeeper_port,
-                      'chroot': self.chroot_path}])
+                                                  chroot=self.chroot_path)])
         with zk2.imageBuildLock("ubuntu-trusty", blocking=False):
             with testtools.ExpectedException(npe.ZKLockException):
                 with self.zk.imageBuildLock("ubuntu-trusty", blocking=False):
@@ -100,9 +103,9 @@ class TestZooKeeper(tests.ZKTestCase):
 
     def test_imageBuildLock_exception_blocking(self):
         zk2 = zk.ZooKeeper()
-        zk2.connect([{'host': self.zookeeper_host,
+        zk2.connect([zk.ZooKeeperConnectionConfig(self.zookeeper_host,
-                      'port': self.zookeeper_port,
+                                                  port=self.zookeeper_port,
-                      'chroot': self.chroot_path}])
+                                                  chroot=self.chroot_path)])
         with zk2.imageBuildLock("ubuntu-trusty", blocking=False):
             with testtools.ExpectedException(npe.TimeoutException):
                 with self.zk.imageBuildLock("ubuntu-trusty",

nodepool/zk.py

@@ -22,30 +22,49 @@ from kazoo.recipe.lock import Lock
 from nodepool import exceptions as npe
 
 
+class ZooKeeperConnectionConfig(object):
+    '''
+    Represents the connection parameters for a ZooKeeper server.
+    '''
+
+    def __eq__(self, other):
+        if isinstance(other, ZooKeeperConnectionConfig):
+            if other.__dict__ == self.__dict__:
+                return True
+        return False
+
+    def __init__(self, host, port=2181, chroot=None):
+        '''Initialize the ZooKeeperConnectionConfig object.
+
+        :param str host: The hostname of the ZooKeeper server.
+        :param int port: The port on which ZooKeeper is listening.
+            Optional, default: 2181.
+        :param str chroot: A chroot for this connection.  All
+            ZooKeeper nodes will be underneath this root path.
+            Optional, default: None.
+
+        (one per server) defining the ZooKeeper cluster servers. Only
+        the 'host' attribute is required.'.
+
+        '''
+        self.host = host
+        self.port = port
+        self.chroot = chroot or ''
+
+
 def buildZooKeeperHosts(host_list):
     '''
     Build the ZK cluster host list for client connections.
 
-    :param list host_list: A list of dicts (one per server) defining
+    :param list host_list: A list of
-        the ZooKeeper cluster servers. Keys for 'host', 'port', and
+        :py:class:`~nodepool.zk.ZooKeeperConnectionConfig` objects (one
-        'chroot' are expected. Only 'host' is required.'. E.g.::
+        per server) defining the ZooKeeper cluster servers.
-
-            [
-              dict(host='192.168.0.2'),
-              dict(host='192.168.0.3', port=2181, chroot='/junk')
-            ]
     '''
     if not isinstance(host_list, list):
         raise Exception("'host_list' must be a list")
     hosts = []
     for host_def in host_list:
-        host = host_def['host']
+        host = '%s:%s%s' % (host_def.host, host_def.port, host_def.chroot)
-        if 'port' in host_def:
-            host = host + ":%s" % host_def['port']
-        else:
-            host = host + ":2181"
-        if 'chroot' in host_def:
-            host = host + host_def['chroot']
         hosts.append(host)
     return ",".join(hosts)
 
@@ -162,10 +181,12 @@ class ZooKeeper(object):
         Convenience method if a pre-existing ZooKeeper connection is not
         supplied to the ZooKeeper object at instantiation time.
 
-        :param list host_list: A list of dicts (one per server) defining
+        :param list host_list: A list of
-            the ZooKeeper cluster servers.
+            :py:class:`~nodepool.zk.ZooKeeperConnectionConfig` objects
+            (one per server) defining the ZooKeeper cluster servers.
 
         :param bool read_only: If True, establishes a read-only connection.
+
         '''
         if not self.client:
             hosts = buildZooKeeperHosts(host_list)