From 76cbdb467b315f58e2778c7dbfdad48163ae3867 Mon Sep 17 00:00:00 2001 From: Andrey Pavlov Date: Mon, 20 Apr 2015 16:21:50 +0300 Subject: [PATCH] Adding ability to retry clients calls * added execute_with_retries method * added retries_number and retry_after config opts * added unit tests that check that only specified errors will be retried partially implements bp: clients-calls-retry Change-Id: I59c27b1b9891900d9222fbe4303a33a31d749493 --- sahara/config.py | 8 +- sahara/exceptions.py | 11 ++ .../tests/unit/utils/openstack/test_base.py | 111 ++++++++++++++++++ sahara/utils/openstack/base.py | 51 ++++++++ 4 files changed, 179 insertions(+), 2 deletions(-) diff --git a/sahara/config.py b/sahara/config.py index cae8e6fcb2..f393c09619 100644 --- a/sahara/config.py +++ b/sahara/config.py @@ -127,6 +127,7 @@ def list_opts(): from sahara.service.edp import job_utils from sahara.service import periodic from sahara.utils import cluster_progress_ops as cpo + from sahara.utils.openstack import base from sahara.utils.openstack import heat from sahara.utils.openstack import neutron from sahara.utils.openstack import nova @@ -151,7 +152,8 @@ def list_opts(): periodic.periodic_opts, proxy.opts, cpo.event_log_opts, - wsgi.wsgi_opts)), + wsgi.wsgi_opts, + base.opts)), (poll_utils.timeouts.name, itertools.chain(poll_utils.timeouts_opts)), (api.conductor_group.name, @@ -167,7 +169,9 @@ def list_opts(): (swift.swift_group.name, itertools.chain(swift.opts)), (keystone.keystone_group.name, - itertools.chain(keystone.ssl_opts)) + itertools.chain(keystone.ssl_opts)), + (base.retries.name, + itertools.chain(base.opts)) ] diff --git a/sahara/exceptions.py b/sahara/exceptions.py index c295a50df6..17894a5eab 100644 --- a/sahara/exceptions.py +++ b/sahara/exceptions.py @@ -359,3 +359,14 @@ class UpdateFailedException(SaharaException): self.message = message self.message = self.message % value super(UpdateFailedException, self).__init__() + + +class MaxRetriesExceeded(SaharaException): + code = "MAX_RETRIES_EXCEEDED" + message = _("Operation %(operation)s wasn't executed correctly after " + "%(attempts)d attempts") + + def __init__(self, attempts, operation): + self.message = self.message % {'operation': operation, + 'attempts': attempts} + super(MaxRetriesExceeded, self).__init__() diff --git a/sahara/tests/unit/utils/openstack/test_base.py b/sahara/tests/unit/utils/openstack/test_base.py index 5918f21000..84efe0b11c 100644 --- a/sahara/tests/unit/utils/openstack/test_base.py +++ b/sahara/tests/unit/utils/openstack/test_base.py @@ -13,6 +13,15 @@ # See the License for the specific language governing permissions and # limitations under the License. + +from cinderclient import exceptions as cinder_exc +from heatclient import exc as heat_exc +from keystoneclient import exceptions as keystone_exc +import mock +from neutronclient.common import exceptions as neutron_exc +from novaclient import exceptions as nova_exc + +from sahara import exceptions as sahara_exc from sahara.tests.unit import base as testbase from sahara.utils.openstack import base @@ -82,3 +91,105 @@ class AuthUrlTest(testbase.SaharaTestCase): _assert("https://127.0.0.1:8080/v3/") _assert("https://127.0.0.1:8080/v42") _assert("https://127.0.0.1:8080/v42/") + + +class ExecuteWithRetryTest(testbase.SaharaTestCase): + + def setUp(self): + super(ExecuteWithRetryTest, self).setUp() + self.fake_client_call = mock.MagicMock() + self.fake_client_call.__name__ = 'fake_client_call' + self.override_config('retries_number', 2, 'retries') + + @mock.patch('sahara.context.sleep') + def _check_error_without_retry(self, error, code, m_sleep): + self.fake_client_call.side_effect = error(code) + + self.assertRaises(error, base.execute_with_retries, + self.fake_client_call) + self.assertEqual(1, self.fake_client_call.call_count) + self.fake_client_call.reset_mock() + + @mock.patch('sahara.context.sleep') + def _check_error_with_retry(self, error, code, m_sleep): + self.fake_client_call.side_effect = error(code) + + self.assertRaises(sahara_exc.MaxRetriesExceeded, + base.execute_with_retries, self.fake_client_call) + self.assertEqual(3, self.fake_client_call.call_count) + self.fake_client_call.reset_mock() + + def test_novaclient_calls_without_retry(self): + # check that following errors will not be retried + self._check_error_without_retry(nova_exc.BadRequest, 400) + self._check_error_without_retry(nova_exc.Unauthorized, 401) + self._check_error_without_retry(nova_exc.Forbidden, 403) + self._check_error_without_retry(nova_exc.NotFound, 404) + self._check_error_without_retry(nova_exc.MethodNotAllowed, 405) + self._check_error_without_retry(nova_exc.Conflict, 409) + self._check_error_without_retry(nova_exc.HTTPNotImplemented, 501) + + def test_novaclient_calls_with_retry(self): + # check that following errors will be retried + self._check_error_with_retry(nova_exc.OverLimit, 413) + self._check_error_with_retry(nova_exc.RateLimit, 429) + + def test_cinderclient_calls_without_retry(self): + # check that following errors will not be retried + self._check_error_without_retry(cinder_exc.BadRequest, 400) + self._check_error_without_retry(cinder_exc.Unauthorized, 401) + self._check_error_without_retry(cinder_exc.Forbidden, 403) + self._check_error_without_retry(cinder_exc.NotFound, 404) + self._check_error_without_retry(nova_exc.HTTPNotImplemented, 501) + + def test_cinderclient_calls_with_retry(self): + # check that following error will be retried + self._check_error_with_retry(cinder_exc.OverLimit, 413) + + def test_neutronclient_calls_without_retry(self): + # check that following errors will not be retried + self._check_error_without_retry(neutron_exc.BadRequest, 400) + self._check_error_without_retry(neutron_exc.Forbidden, 403) + self._check_error_without_retry(neutron_exc.NotFound, 404) + self._check_error_without_retry(neutron_exc.Conflict, 409) + + def test_neutronclient_calls_with_retry(self): + # check that following errors will be retried + self._check_error_with_retry(neutron_exc.InternalServerError, 500) + self._check_error_with_retry(neutron_exc.ServiceUnavailable, 503) + + def test_heatclient_calls_without_retry(self): + # check that following errors will not be retried + self._check_error_without_retry(heat_exc.HTTPBadRequest, 400) + self._check_error_without_retry(heat_exc.HTTPUnauthorized, 401) + self._check_error_without_retry(heat_exc.HTTPForbidden, 403) + self._check_error_without_retry(heat_exc.HTTPNotFound, 404) + self._check_error_without_retry(heat_exc.HTTPMethodNotAllowed, 405) + self._check_error_without_retry(heat_exc.HTTPConflict, 409) + self._check_error_without_retry(heat_exc.HTTPUnsupported, 415) + self._check_error_without_retry(heat_exc.HTTPNotImplemented, 501) + + def test_heatclient_calls_with_retry(self): + # check that following errors will be retried + self._check_error_with_retry(heat_exc.HTTPInternalServerError, 500) + self._check_error_with_retry(heat_exc.HTTPBadGateway, 502) + self._check_error_with_retry(heat_exc.HTTPServiceUnavailable, 503) + + def test_keystoneclient_calls_without_retry(self): + # check that following errors will not be retried + self._check_error_without_retry(keystone_exc.BadRequest, 400) + self._check_error_without_retry(keystone_exc.Unauthorized, 401) + self._check_error_without_retry(keystone_exc.Forbidden, 403) + self._check_error_without_retry(keystone_exc.NotFound, 404) + self._check_error_without_retry(keystone_exc.MethodNotAllowed, 405) + self._check_error_without_retry(keystone_exc.Conflict, 409) + self._check_error_without_retry(keystone_exc.UnsupportedMediaType, 415) + self._check_error_without_retry(keystone_exc.HttpNotImplemented, 501) + + def test_keystoneclient_calls_with_retry(self): + # check that following errors will be retried + self._check_error_with_retry(keystone_exc.RequestTimeout, 408) + self._check_error_with_retry(keystone_exc.InternalServerError, 500) + self._check_error_with_retry(keystone_exc.BadGateway, 502) + self._check_error_with_retry(keystone_exc.ServiceUnavailable, 503) + self._check_error_with_retry(keystone_exc.GatewayTimeout, 504) diff --git a/sahara/utils/openstack/base.py b/sahara/utils/openstack/base.py index f48b6ae30b..b1b0d7576d 100644 --- a/sahara/utils/openstack/base.py +++ b/sahara/utils/openstack/base.py @@ -14,14 +14,37 @@ # limitations under the License. from oslo_config import cfg +from oslo_log import log as logging from oslo_serialization import jsonutils as json from six.moves.urllib import parse as urlparse from sahara import context from sahara import exceptions as ex from sahara.i18n import _ +from sahara.i18n import _LE +from sahara.i18n import _LW + +LOG = logging.getLogger(__name__) + +# List of the errors, that can be retried +ERRORS_TO_RETRY = [408, 413, 429, 500, 502, 503, 504] + +opts = [ + cfg.IntOpt('retries_number', + default=5, + help='Number of times to retry the request to client before ' + 'failing'), + cfg.IntOpt('retry_after', + default=10, + help='Time between the retries to client (in seconds).') +] + +retries = cfg.OptGroup(name='retries', + title='OpenStack clients calls retries') CONF = cfg.CONF +CONF.register_group(retries) +CONF.register_opts(opts, group=retries) def url_for(service_catalog, service_type, admin=False, endpoint_type=None): @@ -84,3 +107,31 @@ def retrieve_auth_url(): version = 'v3' if CONF.use_identity_api_v3 else 'v2.0' return "%s://%s:%s/%s/" % (info.scheme, info.hostname, info.port, version) + + +def execute_with_retries(method, *args, **kwargs): + attempts = CONF.retries.retries_number + 1 + while attempts > 0: + try: + return method(*args, **kwargs) + except Exception as e: + error_code = getattr(e, 'http_status', None) or getattr( + e, 'status_code', None) or getattr(e, 'code', None) + if error_code in ERRORS_TO_RETRY: + LOG.warning(_LW('Occasional error occured during "{method}" ' + 'execution: {error_msg} ({error_code}). ' + 'Operation will be retried.').format( + method=method.__name__, + error_msg=e, + error_code=error_code)) + attempts -= 1 + retry_after = getattr(e, 'retry_after', 0) + context.sleep(max(retry_after, CONF.retries.retry_after)) + else: + LOG.error(_LE('Permanent error occured during "{method}" ' + 'execution: {error_msg}.').format( + method=method.__name__, + error_msg=e)) + raise e + else: + raise ex.MaxRetriesExceeded(attempts, method.__name__)