From 49da9c0c92df80b5d45dfe378c5490e0dd739910 Mon Sep 17 00:00:00 2001 From: "James E. Blair" Date: Thu, 18 Aug 2016 09:48:18 -0700 Subject: [PATCH] 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 | 16 +++++------- nodepool/tests/test_zk.py | 21 ++++++++------- nodepool/zk.py | 55 +++++++++++++++++++++++++++------------ 3 files changed, 56 insertions(+), 36 deletions(-) diff --git a/nodepool/config.py b/nodepool/config.py index 46d64ddf4..6115cade6 100644 --- a/nodepool/config.py +++ b/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.host = server['host'] - z.port = server.get('port', 2181) - z.chroot = server.get('chroot', '') - z.name = z.host + '_' + str(z.port) - newconfig.zookeeper_servers[z.name] = z + z = zk.ZooKeeperConnectionConfig(server['host'], + server.get('port', 2181), + server.get('chroot', None)) + name = z.host + '_' + str(z.port) + newconfig.zookeeper_servers[name] = z for provider in config.get('providers', []): p = Provider() diff --git a/nodepool/tests/test_zk.py b/nodepool/tests/test_zk.py index 8d6201a0e..19be8ac11 100644 --- a/nodepool/tests/test_zk.py +++ b/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'), - dict(host='127.0.0.2', port=2182, chroot='/test2') + zk.ZooKeeperConnectionConfig('127.0.0.1', port=2181, + 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, - 'port': self.zookeeper_port, - 'chroot': self.chroot_path}]) + zk2.connect([zk.ZooKeeperConnectionConfig(self.zookeeper_host, + port=self.zookeeper_port, + 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, - 'port': self.zookeeper_port, - 'chroot': self.chroot_path}]) + zk2.connect([zk.ZooKeeperConnectionConfig(self.zookeeper_host, + port=self.zookeeper_port, + chroot=self.chroot_path)]) with zk2.imageBuildLock("ubuntu-trusty", blocking=False): with testtools.ExpectedException(npe.TimeoutException): with self.zk.imageBuildLock("ubuntu-trusty", diff --git a/nodepool/zk.py b/nodepool/zk.py index a7959870b..6e9ddceb1 100644 --- a/nodepool/zk.py +++ b/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 - the ZooKeeper cluster servers. Keys for 'host', 'port', and - 'chroot' are expected. Only 'host' is required.'. E.g.:: - - [ - dict(host='192.168.0.2'), - dict(host='192.168.0.3', port=2181, chroot='/junk') - ] + :param list host_list: A list of + :py:class:`~nodepool.zk.ZooKeeperConnectionConfig` objects (one + per server) defining the ZooKeeper cluster servers. ''' 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'] - 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'] + host = '%s:%s%s' % (host_def.host, host_def.port, 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 - the ZooKeeper cluster servers. + :param list host_list: A list of + :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)