Merge "Add retries to Nova client methods for connection errors"

heat/common/config.py

@@ -125,6 +125,11 @@ engine_opts = [
                help=_('Number of times to retry to bring a '
                       'resource to a non-error state. Set to 0 to disable '
                       'retries.')),
+    cfg.IntOpt('client_retry_limit',
+               default=2,
+               help=_('Number of times to retry when a client encounters an '
+                      'expected intermittent error. Set to 0 to disable '
+                      'retries.')),
     cfg.IntOpt('event_purge_batch_size',
                default=10,
                help=_("Controls how many events will be pruned whenever a "

heat/engine/clients/client_plugin.py

@@ -22,11 +22,14 @@ from keystoneclient.auth.identity import v3
 from keystoneclient import exceptions
 from keystoneclient import session
 from oslo_config import cfg
+import requests
 import six
 
 from heat.common import config
 from heat.common.i18n import _
 
+cfg.CONF.import_opt('client_retry_limit', 'heat.common.config')
+
 
 class ExceptionFilter(object):
     """A context manager that prevents some exceptions from being raised.
@@ -282,3 +285,7 @@ class ClientPlugin(object):
             return True
         except exceptions.EndpointNotFound:
             return False
+
+
+def retry_if_connection_err(exception):
+    return isinstance(exception, requests.ConnectionError)

heat/engine/clients/os/nova.py

@@ -25,6 +25,7 @@ from oslo_config import cfg
 from oslo_log import log as logging
 from oslo_serialization import jsonutils
 from oslo_utils import uuidutils
+from retrying import retry
 import six
 from six.moves.urllib import parse as urlparse
 
@@ -105,6 +106,8 @@ class NovaClientPlugin(client_plugin.ClientPlugin):
         return (isinstance(ex, exceptions.ClientException) and
                 http_status == 422)
 
+    @retry(stop_max_attempt_number=max(cfg.CONF.client_retry_limit + 1, 0),
+           retry_on_exception=client_plugin.retry_if_connection_err)
     def get_server(self, server):
         """Return fresh server object.
 
@@ -549,6 +552,8 @@ echo -e '%s\tALL=(ALL)\tNOPASSWD: ALL' >> /etc/sudoers
                 if len(server.networks[n]) > 0:
                     return server.networks[n][0]
 
+    @retry(stop_max_attempt_number=max(cfg.CONF.client_retry_limit + 1, 0),
+           retry_on_exception=client_plugin.retry_if_connection_err)
     def absolute_limits(self):
         """Return the absolute limits as a dictionary."""
         limits = self.client().limits.get()

heat/engine/resources/openstack/nova/server.py

@@ -1096,7 +1096,7 @@ class Server(stack_user.StackUser, sh.SchedulerHintsMixin,
         server = None
 
         if self.METADATA in prop_diff:
-            server = self.client().servers.get(self.resource_id)
+            server = self.client_plugin().get_server(self.resource_id)
             self.client_plugin().meta_update(server,
                                              prop_diff[self.METADATA])
 
@@ -1107,12 +1107,12 @@ class Server(stack_user.StackUser, sh.SchedulerHintsMixin,
             updaters.append(self._update_image(prop_diff))
         elif self.ADMIN_PASS in prop_diff:
             if not server:
-                server = self.client().servers.get(self.resource_id)
+                server = self.client_plugin().get_server(self.resource_id)
             server.change_password(prop_diff[self.ADMIN_PASS])
 
         if self.NAME in prop_diff:
             if not server:
-                server = self.client().servers.get(self.resource_id)
+                server = self.client_plugin().get_server(self.resource_id)
             self.client_plugin().rename(server, prop_diff[self.NAME])
 
         if self.NETWORKS in prop_diff:

heat/tests/clients/test_nova_client.py

@@ -21,6 +21,7 @@ from novaclient import exceptions as nova_exceptions
 from oslo_config import cfg
 from oslo_serialization import jsonutils as json
 from oslo_utils import encodeutils
+import requests
 import six
 
 from heat.common import exception
@@ -215,6 +216,44 @@ class NovaClientPluginTests(NovaClientPluginTestCase):
         observed = self.nova_plugin.get_status(server)
         self.assertEqual('ACTIVE', observed)
 
+    def _absolute_limits(self):
+        max_personality = self.m.CreateMockAnything()
+        max_personality.name = 'maxPersonality'
+        max_personality.value = 5
+        max_personality_size = self.m.CreateMockAnything()
+        max_personality_size.name = 'maxPersonalitySize'
+        max_personality_size.value = 10240
+        max_server_meta = self.m.CreateMockAnything()
+        max_server_meta.name = 'maxServerMeta'
+        max_server_meta.value = 3
+        yield max_personality
+        yield max_personality_size
+        yield max_server_meta
+
+    def test_absolute_limits_success(self):
+        limits = mock.Mock()
+        limits.absolute = self._absolute_limits()
+        self.nova_client.limits.get.return_value = limits
+        self.nova_plugin.absolute_limits()
+
+    def test_absolute_limits_retry(self):
+        limits = mock.Mock()
+        limits.absolute = self._absolute_limits()
+        self.nova_client.limits.get.side_effect = [
+            requests.ConnectionError, requests.ConnectionError,
+            limits]
+        self.nova_plugin.absolute_limits()
+        self.assertEqual(3, self.nova_client.limits.get.call_count)
+
+    def test_absolute_limits_failure(self):
+        limits = mock.Mock()
+        limits.absolute = self._absolute_limits()
+        self.nova_client.limits.get.side_effect = [
+            requests.ConnectionError, requests.ConnectionError,
+            requests.ConnectionError]
+        self.assertRaises(requests.ConnectionError,
+                          self.nova_plugin.absolute_limits)
+
 
 class NovaClientPluginRefreshServerTests(NovaClientPluginTestCase):
     msg = ("ClientException: The server has either erred or is "

heat/tests/openstack/nova/test_server.py

@@ -21,6 +21,7 @@ from neutronclient.v2_0 import client as neutronclient
 from novaclient import exceptions as nova_exceptions
 from oslo_serialization import jsonutils
 from oslo_utils import uuidutils
+import requests
 import six
 from six.moves.urllib import parse as urlparse
 
@@ -3903,6 +3904,55 @@ class ServersTest(common.HeatTestCase):
 
         self.m.VerifyAll()
 
+    def test_server_validate_connection_error_retry_successful(self):
+        stack_name = 'srv_val'
+        (tmpl, stack) = self._setup_test_stack(stack_name)
+        tmpl.t['Resources']['WebServer']['Properties'][
+            'personality'] = {"/fake/path1": "a" * 10}
+
+        resource_defns = tmpl.resource_definitions(stack)
+        server = servers.Server('server_create_image_err',
+                                resource_defns['WebServer'], stack)
+
+        self.m.StubOutWithMock(nova.NovaClientPlugin, '_create')
+        nova.NovaClientPlugin._create().AndReturn(self.fc)
+        self._mock_get_image_id_success('F17-x86_64-gold', 'image_id')
+        self._mock_validate_flavor_image_success()
+
+        self.m.StubOutWithMock(self.fc.limits, 'get')
+        self.fc.limits.get().AndRaise(requests.ConnectionError())
+        self.fc.limits.get().AndReturn(self.limits)
+        self.m.ReplayAll()
+
+        self.assertIsNone(server.validate())
+
+        self.m.VerifyAll()
+
+    def test_server_validate_connection_error_retry_failure(self):
+        stack_name = 'srv_val'
+        (tmpl, stack) = self._setup_test_stack(stack_name)
+        tmpl.t['Resources']['WebServer']['Properties'][
+            'personality'] = {"/fake/path1": "a" * 10}
+
+        resource_defns = tmpl.resource_definitions(stack)
+        server = servers.Server('server_create_image_err',
+                                resource_defns['WebServer'], stack)
+
+        self.m.StubOutWithMock(nova.NovaClientPlugin, '_create')
+        nova.NovaClientPlugin._create().AndReturn(self.fc)
+        self._mock_get_image_id_success('F17-x86_64-gold', 'image_id')
+        self._mock_validate_flavor_image_success()
+
+        self.m.StubOutWithMock(self.fc.limits, 'get')
+        self.fc.limits.get().AndRaise(requests.ConnectionError())
+        self.fc.limits.get().AndRaise(requests.ConnectionError())
+        self.fc.limits.get().AndRaise(requests.ConnectionError())
+        self.m.ReplayAll()
+
+        self.assertRaises(requests.ConnectionError, server.validate)
+
+        self.m.VerifyAll()
+
     def test_server_restore(self):
         t = template_format.parse(ns_template)
         tmpl = template.Template(t, files={'a_file': 'the content'})

requirements.txt

@@ -51,6 +51,7 @@ python-zaqarclient>=0.3.0 # Apache-2.0
 pytz>=2013.6 # MIT
 PyYAML>=3.1.0 # MIT
 requests!=2.9.0,>=2.8.1 # Apache-2.0
+retrying>=1.2.3,!=1.3.0 # Apache-2.0
 Routes!=2.0,!=2.1,>=1.12.3;python_version=='2.7' # MIT
 Routes!=2.0,>=1.12.3;python_version!='2.7' # MIT
 six>=1.9.0 # MIT