GCE: healthcheck: add description param to ex_create_healthcheck(); t…

…argetpool: multiple bugfixes

Fix apache#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 apache#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 apache#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

@@ -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,13 +970,18 @@ 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`
         """
         hc_data = {}
         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:

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",

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(