From 4c39723fd175b60329056fd2814c4dcb8b546172 Mon Sep 17 00:00:00 2001 From: Jason Dunsmore Date: Wed, 21 Oct 2015 15:56:54 -0500 Subject: [PATCH] Add retries to Nova client methods for connection errors Connection errors can be recoverable so retry if any are encountered. Also, add a new config option client_retry_limit. Closes-Bug: #1509071 Change-Id: I2c09184d255898a35c194b33027e141c547e1308 --- heat/common/config.py | 5 ++ heat/engine/clients/client_plugin.py | 7 +++ heat/engine/clients/os/nova.py | 5 ++ .../engine/resources/openstack/nova/server.py | 6 +-- heat/tests/clients/test_nova_client.py | 39 +++++++++++++++ heat/tests/openstack/nova/test_server.py | 50 +++++++++++++++++++ requirements.txt | 1 + 7 files changed, 110 insertions(+), 3 deletions(-) diff --git a/heat/common/config.py b/heat/common/config.py index b17086ea13..7b2152868c 100644 --- a/heat/common/config.py +++ b/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 " diff --git a/heat/engine/clients/client_plugin.py b/heat/engine/clients/client_plugin.py index 9a5790c57b..7680e92e31 100644 --- a/heat/engine/clients/client_plugin.py +++ b/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) diff --git a/heat/engine/clients/os/nova.py b/heat/engine/clients/os/nova.py index fb9c7848c2..a2e0ded8a2 100644 --- a/heat/engine/clients/os/nova.py +++ b/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() diff --git a/heat/engine/resources/openstack/nova/server.py b/heat/engine/resources/openstack/nova/server.py index 1140814ae7..20b90639ea 100644 --- a/heat/engine/resources/openstack/nova/server.py +++ b/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: diff --git a/heat/tests/clients/test_nova_client.py b/heat/tests/clients/test_nova_client.py index 3dd73b4f35..84e1485f20 100644 --- a/heat/tests/clients/test_nova_client.py +++ b/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 " diff --git a/heat/tests/openstack/nova/test_server.py b/heat/tests/openstack/nova/test_server.py index 3026541eea..d7013d41db 100644 --- a/heat/tests/openstack/nova/test_server.py +++ b/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 @@ -3908,6 +3909,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'}) diff --git a/requirements.txt b/requirements.txt index cf64bcb8f6..baeff194da 100644 --- a/requirements.txt +++ b/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