From 7911570004292991b8579d3ba5190dd393b5b8c3 Mon Sep 17 00:00:00 2001 From: Evgeny Egorochkin Date: Mon, 30 Jun 2014 17:12:35 +0300 Subject: [PATCH] GCE: healthcheck: add description param to ex_create_healthcheck(); targetpool: multiple bugfixes Fix #1: targetpool = driver.ex_get_targetpool('tpname') targetpool.add_node(node) node.destroy() # targetpool still contains the node targetpool = driver.ex_get_targetpool('tpname') # targetpool.nodes contains node uri string (in addition to possible other node objects) # as produced by _to_targetpool because the node is in the pool but is destroyed targetpool.remove_node(node) # raises an exception, removes the node nevertheless. Expected behavior: remove the node, return true Fix #2: targetpool.add(node) targetpool.add(node) # targetpool.nodes contains 2 copies of node # actual targetpool resource on the GCE side doesn't contain 2 copies Expected behavior: no duplicates in targetpool.nodes, the node list matches GCE side Fix/Improvement #3: Allow specifying nodes by fully-qualified node uri in add_node and remove_node. if tp.nodes: tp.remove_node(tp.nodes[0]) # fails if the node in the list doesn't exist exoected behavior: should be able to remove any node from the list GCE allows adding non-existent nodes to the targetpool and doesn't automatically remove nodes from the pool if you delete them. libcloud should support doing the same. --- libcloud/compute/drivers/gce.py | 41 +++++++++++++++---- ...global_httpHealthChecks_lchealthcheck.json | 1 + libcloud/test/compute/test_gce.py | 18 +++++++- 3 files changed, 50 insertions(+), 10 deletions(-) diff --git a/libcloud/compute/drivers/gce.py b/libcloud/compute/drivers/gce.py index 991432478a..30e0776525 100644 --- a/libcloud/compute/drivers/gce.py +++ b/libcloud/compute/drivers/gce.py @@ -938,7 +938,8 @@ def ex_create_address(self, name, region=None): def ex_create_healthcheck(self, name, host=None, path=None, port=None, interval=None, timeout=None, unhealthy_threshold=None, - healthy_threshold=None): + healthy_threshold=None, + description=None): """ Create an Http Health Check. @@ -969,6 +970,9 @@ def ex_create_healthcheck(self, name, host=None, path=None, port=None, healthy. Defaults to 2. :type healthy_threshold: ``int`` + :keyword description: The description of the check. Defaults to None. + :type description: ``str`` + :return: Health Check object :rtype: :class:`GCEHealthCheck` """ @@ -976,6 +980,8 @@ def ex_create_healthcheck(self, name, host=None, path=None, port=None, hc_data['name'] = name if host: hc_data['host'] = host + if description: + hc_data['description'] = description # As of right now, the 'default' values aren't getting set when called # through the API, so set them explicitly hc_data['requestPath'] = path or '/' @@ -1613,16 +1619,25 @@ def ex_targetpool_add_node(self, targetpool, node): """ if not hasattr(targetpool, 'name'): targetpool = self.ex_get_targetpool(targetpool) - if not hasattr(node, 'name'): - node = self.ex_get_node(node, 'all') + if hasattr(node, 'name'): + node_uri = node.extra['selfLink'] + else: + if node.startswith('https://'): + node_uri = node + else: + node = self.ex_get_node(node, 'all') + node_uri = node.extra['selfLink'] - targetpool_data = {'instances': [{'instance': node.extra['selfLink']}]} + targetpool_data = {'instances': [{'instance': node_uri}]} request = '/regions/%s/targetPools/%s/addInstance' % ( targetpool.region.name, targetpool.name) self.connection.async_request(request, method='POST', data=targetpool_data) - targetpool.nodes.append(node) + if all((node_uri != n) and + (not hasattr(n, 'extra') or n.extra['selfLink'] != node_uri) + for n in targetpool.nodes): + targetpool.nodes.append(node) return True def ex_targetpool_add_healthcheck(self, targetpool, healthcheck): @@ -1667,10 +1682,17 @@ def ex_targetpool_remove_node(self, targetpool, node): """ if not hasattr(targetpool, 'name'): targetpool = self.ex_get_targetpool(targetpool) - if not hasattr(node, 'name'): - node = self.ex_get_node(node, 'all') - targetpool_data = {'instances': [{'instance': node.extra['selfLink']}]} + if hasattr(node, 'name'): + node_uri = node.extra['selfLink'] + else: + if node.startswith('https://'): + node_uri = node + else: + node = self.ex_get_node(node, 'all') + node_uri = node.extra['selfLink'] + + targetpool_data = {'instances': [{'instance': node_uri}]} request = '/regions/%s/targetPools/%s/removeInstance' % ( targetpool.region.name, targetpool.name) @@ -1679,7 +1701,8 @@ def ex_targetpool_remove_node(self, targetpool, node): # Remove node object from node list index = None for i, nd in enumerate(targetpool.nodes): - if nd.name == node.name: + if nd == node_uri or (hasattr(nd, 'extra') and + nd.extra['selfLink'] == node_uri): index = i break if index is not None: diff --git a/libcloud/test/compute/fixtures/gce/global_httpHealthChecks_lchealthcheck.json b/libcloud/test/compute/fixtures/gce/global_httpHealthChecks_lchealthcheck.json index 80c9aebfe7..6c4a8ce7c0 100644 --- a/libcloud/test/compute/fixtures/gce/global_httpHealthChecks_lchealthcheck.json +++ b/libcloud/test/compute/fixtures/gce/global_httpHealthChecks_lchealthcheck.json @@ -1,6 +1,7 @@ { "checkIntervalSec": 10, "creationTimestamp": "2013-09-02T22:18:01.180-07:00", + "description": "test healthcheck", "healthyThreshold": 3, "host": "lchost", "id": "06860603312991823381", diff --git a/libcloud/test/compute/test_gce.py b/libcloud/test/compute/test_gce.py index 74cc8b64e5..d4bdd3e4c9 100644 --- a/libcloud/test/compute/test_gce.py +++ b/libcloud/test/compute/test_gce.py @@ -231,13 +231,16 @@ def test_ex_create_healthcheck(self): 'interval': 10, 'timeout': 10, 'unhealthy_threshold': 4, - 'healthy_threshold': 3} + 'healthy_threshold': 3, + 'description': 'test healthcheck'} hc = self.driver.ex_create_healthcheck(healthcheck_name, **kwargs) self.assertTrue(isinstance(hc, GCEHealthCheck)) self.assertEqual(hc.name, healthcheck_name) self.assertEqual(hc.path, '/lc') self.assertEqual(hc.port, 8000) self.assertEqual(hc.interval, 10) + self.assertEqual(hc.extra['host'], 'lchost') + self.assertEqual(hc.extra['description'], 'test healthcheck') def test_ex_create_firewall(self): firewall_name = 'lcfirewall' @@ -413,10 +416,23 @@ def test_ex_targetpool_remove_add_node(self): self.assertTrue(remove_node) self.assertEqual(len(targetpool.nodes), 1) + add_node = self.driver.ex_targetpool_add_node(targetpool, node.extra['selfLink']) + self.assertTrue(add_node) + self.assertEqual(len(targetpool.nodes), 2) + + remove_node = self.driver.ex_targetpool_remove_node(targetpool, node.extra['selfLink']) + self.assertTrue(remove_node) + self.assertEqual(len(targetpool.nodes), 1) + add_node = self.driver.ex_targetpool_add_node(targetpool, node) self.assertTrue(add_node) self.assertEqual(len(targetpool.nodes), 2) + # check that duplicates are filtered + add_node = self.driver.ex_targetpool_add_node(targetpool, node.extra['selfLink']) + self.assertTrue(add_node) + self.assertEqual(len(targetpool.nodes), 2) + def test_ex_targetpool_remove_add_healthcheck(self): targetpool = self.driver.ex_get_targetpool('lctargetpool') healthcheck = self.driver.ex_get_healthcheck(