8872: Fixups from code review.

[arvados.git] / services / nodemanager / arvnodeman / computenode / driver / __init__.py

diff --git a/services/nodemanager/arvnodeman/computenode/driver/__init__.py b/services/nodemanager/arvnodeman/computenode/driver/__init__.py
index 06b532ac5618d79915478930d05072278ad3753f..95b6fa8e0ce962d67b43b26f8106469d9d692faf 100644 (file)
--- a/services/nodemanager/arvnodeman/computenode/driver/__init__.py
+++ b/services/nodemanager/arvnodeman/computenode/driver/__init__.py
@@ -2,14 +2,16 @@
 
 from __future__ import absolute_import, print_function
 
 
 from __future__ import absolute_import, print_function
 

+import logging

 from operator import attrgetter
 
 import libcloud.common.types as cloud_types
 from libcloud.compute.base import NodeDriver, NodeAuthSSHKey
 
 from ...config import NETWORK_ERRORS
 from operator import attrgetter
 
 import libcloud.common.types as cloud_types
 from libcloud.compute.base import NodeDriver, NodeAuthSSHKey
 
 from ...config import NETWORK_ERRORS

+from .. import RetryMixin

 
 

-class BaseComputeNodeDriver(object):
+class BaseComputeNodeDriver(RetryMixin):

     """Abstract base class for compute node drivers.
 
     libcloud drivers abstract away many of the differences between
     """Abstract base class for compute node drivers.
 
     libcloud drivers abstract away many of the differences between


@@ -24,7 +26,16 @@ class BaseComputeNodeDriver(object):
     """
     CLOUD_ERRORS = NETWORK_ERRORS + (cloud_types.LibcloudError,)
 
     """
     CLOUD_ERRORS = NETWORK_ERRORS + (cloud_types.LibcloudError,)
 

-    def __init__(self, auth_kwargs, list_kwargs, create_kwargs, driver_class):
+    @RetryMixin._retry()
+    def _create_driver(self, driver_class, **auth_kwargs):
+        return driver_class(**auth_kwargs)
+
+    @RetryMixin._retry()
+    def _set_sizes(self):
+        self.sizes = {sz.id: sz for sz in self.real.list_sizes()}
+
+    def __init__(self, auth_kwargs, list_kwargs, create_kwargs,
+                 driver_class, retry_wait=1, max_retry_wait=180):

         """Base initializer for compute node drivers.
 
         Arguments:
         """Base initializer for compute node drivers.
 
         Arguments:


@@ -37,7 +48,12 @@ class BaseComputeNodeDriver(object):
           libcloud driver's create_node method to create a new compute node.
         * driver_class: The class of a libcloud driver to use.
         """
           libcloud driver's create_node method to create a new compute node.
         * driver_class: The class of a libcloud driver to use.
         """

-        self.real = driver_class(**auth_kwargs)
+
+        super(BaseComputeNodeDriver, self).__init__(retry_wait, max_retry_wait,
+                                         logging.getLogger(self.__class__.__name__),
+                                         type(self),
+                                         None)
+        self.real = self._create_driver(driver_class, **auth_kwargs)

         self.list_kwargs = list_kwargs
         self.create_kwargs = create_kwargs
         # Transform entries in create_kwargs.  For each key K, if this class
         self.list_kwargs = list_kwargs
         self.create_kwargs = create_kwargs
         # Transform entries in create_kwargs.  For each key K, if this class


@@ -53,7 +69,7 @@ class BaseComputeNodeDriver(object):
                 if new_pair is not None:
                     self.create_kwargs[new_pair[0]] = new_pair[1]
 
                 if new_pair is not None:
                     self.create_kwargs[new_pair[0]] = new_pair[1]
 

-        self.sizes = {sz.id: sz for sz in self.real.list_sizes()}
+        self._set_sizes()

 
     def _init_ping_host(self, ping_host):
         self.ping_host = ping_host
 
     def _init_ping_host(self, ping_host):
         self.ping_host = ping_host


@@ -63,7 +79,7 @@ class BaseComputeNodeDriver(object):
             key = NodeAuthSSHKey(ssh_file.read())
         return 'auth', key
 
             key = NodeAuthSSHKey(ssh_file.read())
         return 'auth', key
 

-    def search_for(self, term, list_method, key=attrgetter('id'), **kwargs):
+    def search_for_now(self, term, list_method, key=attrgetter('id'), **kwargs):

         """Return one matching item from a list of cloud objects.
 
         Raises ValueError if the number of matching objects is not exactly 1.
         """Return one matching item from a list of cloud objects.
 
         Raises ValueError if the number of matching objects is not exactly 1.


@@ -76,22 +92,45 @@ class BaseComputeNodeDriver(object):
           value search for a `term` match on each item.  Returns the
           object's 'id' attribute by default.
         """
           value search for a `term` match on each item.  Returns the
           object's 'id' attribute by default.
         """

+        items = getattr(self.real, list_method)(**kwargs)
+        results = [item for item in items if key(item) == term]
+        count = len(results)
+        if count != 1:
+            raise ValueError("{} returned {} results for {!r}".format(
+                    list_method, count, term))
+        return results[0]
+
+    def search_for(self, term, list_method, key=attrgetter('id'), **kwargs):
+        """Return one cached matching item from a list of cloud objects.
+
+        See search_for_now() for details of arguments and exceptions.
+        This method caches results, so it's good to find static cloud objects
+        like node sizes, regions, etc.
+        """

         cache_key = (list_method, term)
         if cache_key not in self.SEARCH_CACHE:
         cache_key = (list_method, term)
         if cache_key not in self.SEARCH_CACHE:

-            items = getattr(self.real, list_method)(**kwargs)
-            results = [item for item in items
-                       if key(item) == term]
-            count = len(results)
-            if count != 1:
-                raise ValueError("{} returned {} results for '{}'".format(
-                        list_method, count, term))
-            self.SEARCH_CACHE[cache_key] = results[0]
+            self.SEARCH_CACHE[cache_key] = self.search_for_now(
+                term, list_method, key, **kwargs)

         return self.SEARCH_CACHE[cache_key]
 
         return self.SEARCH_CACHE[cache_key]
 

-    def list_nodes(self):
-        return self.real.list_nodes(**self.list_kwargs)
+    def list_nodes(self, **kwargs):
+        l = self.list_kwargs.copy()
+        l.update(kwargs)
+        return self.real.list_nodes(**l)
+
+    def create_cloud_name(self, arvados_node):
+        """Return a cloud node name for the given Arvados node record.
+
+        Subclasses must override this method.  It should return a string
+        that can be used as the name for a newly-created cloud node,
+        based on identifying information in the Arvados node record.

 
 

-    def arvados_create_kwargs(self, arvados_node):
+        Arguments:
+        * arvados_node: This Arvados node record to seed the new cloud node.
+        """
+        raise NotImplementedError("BaseComputeNodeDriver.create_cloud_name")
+
+    def arvados_create_kwargs(self, size, arvados_node):

         """Return dynamic keyword arguments for create_node.
 
         Subclasses must override this method.  It should return a dictionary
         """Return dynamic keyword arguments for create_node.
 
         Subclasses must override this method.  It should return a dictionary


@@ -100,6 +139,7 @@ class BaseComputeNodeDriver(object):
         create_kwargs.
 
         Arguments:
         create_kwargs.
 
         Arguments:

+        * size: The node size that will be created (libcloud NodeSize object)

         * arvados_node: The Arvados node record that will be associated
           with this cloud node, as returned from the API server.
         """
         * arvados_node: The Arvados node record that will be associated
           with this cloud node, as returned from the API server.
         """


@@ -114,11 +154,27 @@ class BaseComputeNodeDriver(object):
             self.ping_host, arvados_node['uuid'],
             arvados_node['info']['ping_secret'])
 
             self.ping_host, arvados_node['uuid'],
             arvados_node['info']['ping_secret'])
 

+    @staticmethod
+    def _name_key(cloud_object):
+        return cloud_object.name
+

     def create_node(self, size, arvados_node):
     def create_node(self, size, arvados_node):

-        kwargs = self.create_kwargs.copy()
-        kwargs.update(self.arvados_create_kwargs(arvados_node))
-        kwargs['size'] = size
-        return self.real.create_node(**kwargs)
+        try:
+            kwargs = self.create_kwargs.copy()
+            kwargs.update(self.arvados_create_kwargs(size, arvados_node))
+            kwargs['size'] = size
+            return self.real.create_node(**kwargs)
+        except self.CLOUD_ERRORS as create_error:
+            # Workaround for bug #6702: sometimes the create node request
+            # succeeds but times out and raises an exception instead of
+            # returning a result.  If this happens, we get stuck in a retry
+            # loop forever because subsequent create_node attempts will fail
+            # due to node name collision.  So check if the node we intended to
+            # create shows up in the cloud node list and return it if found.
+            try:
+                return self.search_for_now(kwargs['name'], 'list_nodes', self._name_key)
+            except ValueError:
+                raise create_error

 
     def post_create_node(self, cloud_node):
         # ComputeNodeSetupActor calls this method after the cloud node is
 
     def post_create_node(self, cloud_node):
         # ComputeNodeSetupActor calls this method after the cloud node is


@@ -161,6 +217,11 @@ class BaseComputeNodeDriver(object):
             lambda self, value: setattr(self.real, attr_name, value),
             doc=getattr(getattr(NodeDriver, attr_name), '__doc__', None))
 
             lambda self, value: setattr(self.real, attr_name, value),
             doc=getattr(getattr(NodeDriver, attr_name), '__doc__', None))
 

+    # node id
+    @classmethod
+    def node_id(cls):
+        raise NotImplementedError("BaseComputeNodeDriver.node_id")
+

     _locals = locals()
     for _attr_name in dir(NodeDriver):
         if (not _attr_name.startswith('_')) and (_attr_name not in _locals):
     _locals = locals()
     for _attr_name in dir(NodeDriver):
         if (not _attr_name.startswith('_')) and (_attr_name not in _locals):