zuul
/
nodepool
Code Issues Proposed changes

Clarify provider manager vs provider config 

There are places where we handle both the provider config and provider
manager objects. We had a bug where we called a manager method on the
config object which failed. Since python doesn't type check ahead of
time let us try and make this easier on developers and very explicitly
name the provider manager provider_manager and the provider config
provider_config.

Change-Id: I36919672cdbf9a5c66297ff49f304b9040eee1cb
This commit is contained in:
Clark Boylan 2018-01-07 12:44:15 -08:00 committed by Tristan Cacqueray
parent 38e1d2afdb
commit be4b8f1416
4 changed files with 31 additions and 26 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
nodepool/driver/__init__.py
View File

@ -258,7 +258,7 @@ class NodeLaunchManager(object):
self._threads = [] self._threads = []
self._zk = zk self._zk = zk
self._pool = pool self._pool = pool
self._manager = provider_manager self._provider_manager = provider_manager
self._requestor = requestor self._requestor = requestor
@property @property

40
nodepool/driver/openstack/handler.py
View File

@ -52,23 +52,24 @@ class NodeLauncher(threading.Thread, stats.StatsReporter):
self.log = logging.getLogger("nodepool.NodeLauncher-%s" % node.id) self.log = logging.getLogger("nodepool.NodeLauncher-%s" % node.id)
self._zk = zk self._zk = zk
self._label = provider_label self._label = provider_label
self._manager = provider_manager self._provider_manager = provider_manager
self._node = node self._node = node
self._retries = retries self._retries = retries
self._image_name = None self._image_name = None
self._requestor = requestor self._requestor = requestor
self._pool = self._label.pool self._pool = self._label.pool
self._provider = self._pool.provider self._provider_config = self._pool.provider
if self._label.diskimage: if self._label.diskimage:
self._diskimage = self._provider.diskimages[self._label.diskimage.name] self._diskimage = \
self._provider_config.diskimages[self._label.diskimage.name]
else: else:
self._diskimage = None self._diskimage = None
def logConsole(self, server_id, hostname): def logConsole(self, server_id, hostname):
if not self._label.console_log: if not self._label.console_log:
return return
console = self._manager.getServerConsole(server_id) console = self._provider_manager.getServerConsole(server_id)
if console: if console:
self.log.debug('Console log from hostname %s:' % hostname) self.log.debug('Console log from hostname %s:' % hostname)
for line in console.splitlines(): for line in console.splitlines():
@ -78,12 +79,12 @@ class NodeLauncher(threading.Thread, stats.StatsReporter):
if self._label.diskimage: if self._label.diskimage:
# launch using diskimage # launch using diskimage
cloud_image = self._zk.getMostRecentImageUpload( cloud_image = self._zk.getMostRecentImageUpload(
self._diskimage.name, self._provider.name) self._diskimage.name, self._provider_config.name)
if not cloud_image: if not cloud_image:
raise exceptions.LaunchNodepoolException( raise exceptions.LaunchNodepoolException(
"Unable to find current cloud image %s in %s" % "Unable to find current cloud image %s in %s" %
(self._diskimage.name, self._provider.name) (self._diskimage.name, self._provider_config.name)
) )
config_drive = self._diskimage.config_drive config_drive = self._diskimage.config_drive
@ -107,12 +108,13 @@ class NodeLauncher(threading.Thread, stats.StatsReporter):
username = self._label.cloud_image.username username = self._label.cloud_image.username
connection_type = self._label.cloud_image.connection_type connection_type = self._label.cloud_image.connection_type
hostname = self._provider.hostname_format.format( hostname = self._provider_config.hostname_format.format(
label=self._label, provider=self._provider, node=self._node label=self._label, provider=self._provider_config, node=self._node
) )
self.log.info("Creating server with hostname %s in %s from image %s " self.log.info("Creating server with hostname %s in %s from image %s "
"for node id: %s" % (hostname, self._provider.name, "for node id: %s" % (hostname,
self._provider_config.name,
image_name, image_name,
self._node.id)) self._node.id))
@ -121,7 +123,7 @@ class NodeLauncher(threading.Thread, stats.StatsReporter):
# because that isn't available in ZooKeeper until after the server is # because that isn't available in ZooKeeper until after the server is
# active, which could cause a race in leak detection. # active, which could cause a race in leak detection.
server = self._manager.createServer( server = self._provider_manager.createServer(
hostname, hostname,
image=image_external, image=image_external,
min_ram=self._label.min_ram, min_ram=self._label.min_ram,
@ -148,8 +150,8 @@ class NodeLauncher(threading.Thread, stats.StatsReporter):
self.log.debug("Waiting for server %s for node id: %s" % self.log.debug("Waiting for server %s for node id: %s" %
(server.id, self._node.id)) (server.id, self._node.id))
server = self._manager.waitForServer( server = self._provider_manager.waitForServer(
server, self._provider.launch_timeout, server, self._provider_config.launch_timeout,
auto_ip=self._pool.auto_floating_ip) auto_ip=self._pool.auto_floating_ip)
if server.status != 'ACTIVE': if server.status != 'ACTIVE':
@ -196,7 +198,7 @@ class NodeLauncher(threading.Thread, stats.StatsReporter):
try: try:
self.log.debug("Gathering host keys for node %s", self._node.id) self.log.debug("Gathering host keys for node %s", self._node.id)
host_keys = utils.keyscan( host_keys = utils.keyscan(
interface_ip, timeout=self._provider.boot_timeout) interface_ip, timeout=self._provider_config.boot_timeout)
if not host_keys: if not host_keys:
raise exceptions.LaunchKeyscanException( raise exceptions.LaunchKeyscanException(
"Unable to gather host keys") "Unable to gather host keys")
@ -220,8 +222,10 @@ class NodeLauncher(threading.Thread, stats.StatsReporter):
attempts, self._retries, self._node.id) attempts, self._retries, self._node.id)
# If we created an instance, delete it. # If we created an instance, delete it.
if self._node.external_id: if self._node.external_id:
self._manager.cleanupNode(self._node.external_id) self._provider_manager.cleanupNode(self._node.external_id)
self._manager.waitForNodeCleanup(self._node.external_id) self._provider_manager.waitForNodeCleanup(
self._node.external_id
)
self._node.external_id = None self._node.external_id = None
self._node.public_ipv4 = None self._node.public_ipv4 = None
self._node.public_ipv6 = None self._node.public_ipv6 = None
@ -232,7 +236,7 @@ class NodeLauncher(threading.Thread, stats.StatsReporter):
# Invalidate the quota cache if we encountered a quota error. # Invalidate the quota cache if we encountered a quota error.
if 'quota exceeded' in str(e).lower(): if 'quota exceeded' in str(e).lower():
self.log.info("Quota exceeded, invalidating quota cache") self.log.info("Quota exceeded, invalidating quota cache")
self._manager.invalidateQuotaCache() self._provider_manager.invalidateQuotaCache()
attempts += 1 attempts += 1
self._node.state = zk.READY self._node.state = zk.READY
@ -261,7 +265,7 @@ class NodeLauncher(threading.Thread, stats.StatsReporter):
self.recordLaunchStats(statsd_key, dt, self._image_name, self.recordLaunchStats(statsd_key, dt, self._image_name,
self._node.provider, self._node.az, self._node.provider, self._node.az,
self._requestor) self._requestor)
self.updateNodeStats(self._zk, self._provider) self.updateNodeStats(self._zk, self._provider_config)
except Exception: except Exception:
self.log.exception("Exception while reporting stats:") self.log.exception("Exception while reporting stats:")
@ -279,7 +283,7 @@ class OpenStackNodeLaunchManager(NodeLaunchManager):
''' '''
self._nodes.append(node) self._nodes.append(node)
provider_label = self._pool.labels[node.type] provider_label = self._pool.labels[node.type]
t = NodeLauncher(self._zk, provider_label, self._manager, t = NodeLauncher(self._zk, provider_label, self._provider_manager,
self._requestor, node, self._retries) self._requestor, node, self._retries)
t.start() t.start()
self._threads.append(t) self._threads.append(t)

12
nodepool/launcher.py
View File

@ -44,12 +44,12 @@ SUSPEND_WAIT_TIME = 30 # How long to wait between checks for ZooKeeper
class NodeDeleter(threading.Thread, stats.StatsReporter): class NodeDeleter(threading.Thread, stats.StatsReporter):
log = logging.getLogger("nodepool.NodeDeleter") log = logging.getLogger("nodepool.NodeDeleter")
def __init__(self, zk, manager, node): def __init__(self, zk, provider_manager, node):
threading.Thread.__init__(self, name='NodeDeleter for %s %s' % threading.Thread.__init__(self, name='NodeDeleter for %s %s' %
(node.provider, node.external_id)) (node.provider, node.external_id))
stats.StatsReporter.__init__(self) stats.StatsReporter.__init__(self)
self._zk = zk self._zk = zk
self._manager = manager self._provider_manager = provider_manager
self._node = node self._node = node
@staticmethod @staticmethod
@ -60,8 +60,8 @@ class NodeDeleter(threading.Thread, stats.StatsReporter):
This is a class method so we can support instantaneous deletes. This is a class method so we can support instantaneous deletes.
:param ZooKeeper zk_conn: A ZooKeeper object to use. :param ZooKeeper zk_conn: A ZooKeeper object to use.
:param ProviderManager manager: ProviderManager object to use for :param ProviderManager provider_manager: ProviderManager object to
deleting the server. use fo deleting the server.
:param Node node: A locked Node object that describes the server to :param Node node: A locked Node object that describes the server to
delete. delete.
:param bool node_exists: True if the node actually exists in ZooKeeper. :param bool node_exists: True if the node actually exists in ZooKeeper.
@ -101,10 +101,10 @@ class NodeDeleter(threading.Thread, stats.StatsReporter):
else: else:
node_exists = True node_exists = True
self.delete(self._zk, self._manager, self._node, node_exists) self.delete(self._zk, self._provider_manager, self._node, node_exists)
try: try:
self.updateNodeStats(self._zk, self._manager.provider) self.updateNodeStats(self._zk, self._provider_manager.provider)
except Exception: except Exception:
self.log.exception("Exception while reporting stats:") self.log.exception("Exception while reporting stats:")

3
nodepool/tests/test_nodelaunchmanager.py
View File

@ -61,7 +61,8 @@ class TestNodeLaunchManager(tests.DBTestCase):
time.sleep(0) time.sleep(0)
self.assertEqual(len(mgr.ready_nodes), 1) self.assertEqual(len(mgr.ready_nodes), 1)
self.assertEqual(len(mgr.failed_nodes), 0) self.assertEqual(len(mgr.failed_nodes), 0)
self.assertEqual(mgr._manager.listNodes()[0]['metadata']['groups'], nodes = mgr._provider_manager.listNodes()
self.assertEqual(nodes[0]['metadata']['groups'],
'fake-provider,fake-image,fake-label') 'fake-provider,fake-image,fake-label')
@mock.patch('nodepool.driver.openstack.handler.NodeLauncher._launchNode') @mock.patch('nodepool.driver.openstack.handler.NodeLauncher._launchNode')