From 028d6a8b686dd7a49d9a22319f82505b248d2c07 Mon Sep 17 00:00:00 2001 From: Adit Sarfaty Date: Mon, 12 Sep 2016 10:07:04 +0300 Subject: [PATCH] Replace retrying with tenacity We are replacing all usages of the retrying package with tenacity with an end goal of removing the retrying package from our requirements. This patch also demonstrate how to use the new api to retry only for some of the exception error codes Change-Id: Ie1b082848ac6153d29af7779de914071dc8c1ba5 --- requirements.txt | 2 +- vmware_nsx/common/utils.py | 41 +++++++++++++++++++ vmware_nsx/nsxlib/v3/utils.py | 13 +++--- .../plugins/nsx_v/vshield/common/constants.py | 2 + vmware_nsx/plugins/nsx_v/vshield/vcns.py | 34 +++++++++++---- vmware_nsx/tests/unit/__init__.py | 8 ++++ vmware_nsx/tests/unit/nsx_v/test_misc.py | 32 +++++++++++++-- 7 files changed, 113 insertions(+), 19 deletions(-) diff --git a/requirements.txt b/requirements.txt index fc4bcea7e9..549e183372 100644 --- a/requirements.txt +++ b/requirements.txt @@ -7,7 +7,7 @@ enum34;python_version=='2.7' or python_version=='2.6' or python_version=='3.3' # eventlet!=0.18.3,>=0.18.2 # MIT httplib2>=0.7.5 # MIT netaddr!=0.7.16,>=0.7.13 # BSD -retrying!=1.3.0,>=1.2.3 # Apache-2.0 +tenacity>=3.1.1 # Apache-2.0 SQLAlchemy<1.1.0,>=1.0.10 # MIT six>=1.9.0 # MIT stevedore>=1.16.0 # Apache-2.0 diff --git a/vmware_nsx/common/utils.py b/vmware_nsx/common/utils.py index b7d795ef9f..c209d6ab9e 100644 --- a/vmware_nsx/common/utils.py +++ b/vmware_nsx/common/utils.py @@ -19,6 +19,8 @@ import hashlib import eventlet import six +import tenacity +import xml.etree.ElementTree as et from neutron import version as n_version from neutron_lib.api import validators @@ -106,6 +108,45 @@ def check_and_truncate(display_name): return display_name or '' +def _get_bad_request_error_code(e): + """Get the error code out of the exception""" + try: + desc = et.fromstring(e.response) + return int(desc.find('errorCode').text) + except Exception: + pass + + +def retry_upon_exception_exclude_error_codes( + exc, excluded_errors, delay, max_delay, max_attempts): + """Retry with the configured exponential delay, unless the exception error + code is in the given list + """ + def retry_if_not_error_codes(e): + # return True only for BadRequests without error codes or with error + # codes not in the exclude list + if isinstance(e, exc): + error_code = _get_bad_request_error_code(e) + if error_code and error_code not in excluded_errors: + return True + return False + + return tenacity.retry(reraise=True, + retry=tenacity.retry_if_exception( + retry_if_not_error_codes), + wait=tenacity.wait_exponential( + multiplier=delay, max=max_delay), + stop=tenacity.stop_after_attempt(max_attempts)) + + +def retry_upon_exception(exc, delay, max_delay, max_attempts): + return tenacity.retry(reraise=True, + retry=tenacity.retry_if_exception_type(exc), + wait=tenacity.wait_exponential( + multiplier=delay, max=max_delay), + stop=tenacity.stop_after_attempt(max_attempts)) + + def read_file(path): try: with open(path) as file: diff --git a/vmware_nsx/nsxlib/v3/utils.py b/vmware_nsx/nsxlib/v3/utils.py index d94836aae7..75d7252398 100644 --- a/vmware_nsx/nsxlib/v3/utils.py +++ b/vmware_nsx/nsxlib/v3/utils.py @@ -13,7 +13,7 @@ # License for the specific language governing permissions and limitations # under the License. -import retrying +import tenacity from neutron_lib import exceptions from oslo_log import log @@ -63,12 +63,13 @@ def update_v3_tags(current_tags, tags_update): return tags -def retry_upon_exception(exc, delay=500, max_delay=2000, +def retry_upon_exception(exc, delay=0.5, max_delay=2, max_attempts=DEFAULT_MAX_ATTEMPTS): - return retrying.retry(retry_on_exception=lambda e: isinstance(e, exc), - wait_exponential_multiplier=delay, - wait_exponential_max=max_delay, - stop_max_attempt_number=max_attempts) + return tenacity.retry(reraise=True, + retry=tenacity.retry_if_exception_type(exc), + wait=tenacity.wait_exponential( + multiplier=delay, max=max_delay), + stop=tenacity.stop_after_attempt(max_attempts)) def list_match(list1, list2): diff --git a/vmware_nsx/plugins/nsx_v/vshield/common/constants.py b/vmware_nsx/plugins/nsx_v/vshield/common/constants.py index 4401246166..79be131603 100644 --- a/vmware_nsx/plugins/nsx_v/vshield/common/constants.py +++ b/vmware_nsx/plugins/nsx_v/vshield/common/constants.py @@ -57,6 +57,8 @@ NSX_ERROR_DHCP_DUPLICATE_MAC = 12518 NSX_ERROR_IPAM_ALLOCATE_ALL_USED = 120051 NSX_ERROR_IPAM_ALLOCATE_IP_USED = 120056 +NSX_ERROR_ALREADY_HAS_SG_POLICY = 120508 + SUFFIX_LENGTH = 8 #Edge size diff --git a/vmware_nsx/plugins/nsx_v/vshield/vcns.py b/vmware_nsx/plugins/nsx_v/vshield/vcns.py index e638d076b8..9cc53b09fb 100644 --- a/vmware_nsx/plugins/nsx_v/vshield/vcns.py +++ b/vmware_nsx/plugins/nsx_v/vshield/vcns.py @@ -19,12 +19,13 @@ import time from oslo_config import cfg from oslo_log import log as logging from oslo_serialization import jsonutils -import retrying import six import xml.etree.ElementTree as et from vmware_nsx._i18n import _LE from vmware_nsx.common import nsxv_constants +from vmware_nsx.common import utils +from vmware_nsx.plugins.nsx_v.vshield.common import constants from vmware_nsx.plugins.nsx_v.vshield.common import exceptions from vmware_nsx.plugins.nsx_v.vshield.common import VcnsApiClient @@ -84,12 +85,27 @@ CERTIFICATE = "certificate" NETWORK_TYPES = ['Network', 'VirtualWire', 'DistributedVirtualPortgroup'] -def retry_upon_exception(exc, delay=500, max_delay=4000, - max_attempts=cfg.CONF.nsxv.retries): - return retrying.retry(retry_on_exception=lambda e: isinstance(e, exc), - wait_exponential_multiplier=delay, - wait_exponential_max=max_delay, - stop_max_attempt_number=max_attempts) +def _get_bad_request_error_code(e): + """Get the error code out of the exception""" + try: + desc = et.fromstring(e.response) + return int(desc.find('errorCode').text) + except Exception: + pass + + +def retry_upon_exception_exclude_error_codes( + exc, excluded_errors, delay=0.5, max_delay=4, max_attempts=0): + if not max_attempts: + max_attempts = cfg.CONF.nsxv.retries + return utils.retry_upon_exception_exclude_error_codes( + exc, excluded_errors, delay, max_delay, max_attempts) + + +def retry_upon_exception(exc, delay=0.5, max_delay=4, max_attempts=0): + if not max_attempts: + max_attempts = cfg.CONF.nsxv.retries + return utils.retry_upon_exception(exc, delay, max_delay, max_attempts) class Vcns(object): @@ -676,7 +692,8 @@ class Vcns(object): }) return {'__enforcementPoints': e_point_list} - @retry_upon_exception(exceptions.RequestBad) + @retry_upon_exception_exclude_error_codes( + exceptions.RequestBad, [constants.NSX_ERROR_ALREADY_HAS_SG_POLICY]) def create_spoofguard_policy(self, enforcement_points, name, enable): uri = '%s/policies/' % SPOOFGUARD_PREFIX @@ -797,7 +814,6 @@ class Vcns(object): uri = '%s/usermgmt/scopingobjects' % SERVICES_PREFIX h, so_list = self.do_request(HTTP_GET, uri, decode=False, format='xml') - root = et.fromstring(so_list) for obj in root.iter('object'): if (obj.find('objectTypeName').text in type_names and diff --git a/vmware_nsx/tests/unit/__init__.py b/vmware_nsx/tests/unit/__init__.py index cd4e83d217..c47dc969f8 100644 --- a/vmware_nsx/tests/unit/__init__.py +++ b/vmware_nsx/tests/unit/__init__.py @@ -14,7 +14,10 @@ # License for the specific language governing permissions and limitations # under the License. +import eventlet +import mock import os +import time from networking_l2gw.db.l2gateway import l2gateway_models # noqa @@ -48,6 +51,11 @@ VCNSAPI_NAME = '%s.%s' % (vcns_api_helper.__module__, vcns_api_helper.__name__) EDGE_MANAGE_NAME = '%s.%s' % (edge_manage_class.__module__, edge_manage_class.__name__) +# Mock for the tenacity retrying sleeping method +eventlet.monkey_patch() +mocked_retry_sleep = mock.patch.object(time, 'sleep') +mocked_retry_sleep.start() + def get_fake_conf(filename): return os.path.join(STUBS_PATH, filename) diff --git a/vmware_nsx/tests/unit/nsx_v/test_misc.py b/vmware_nsx/tests/unit/nsx_v/test_misc.py index b0cecefffc..e5763fb9a7 100644 --- a/vmware_nsx/tests/unit/nsx_v/test_misc.py +++ b/vmware_nsx/tests/unit/nsx_v/test_misc.py @@ -31,23 +31,49 @@ def raise_until_attempt(attempt, exception): class TestMisc(base.BaseTestCase): + response = """ +
Dummy
1 + core-services + """ + def test_retry_on_exception_one_attempt(self): success_on_first_attempt = raise_until_attempt( 1, exceptions.RequestBad(uri='', response='')) should_return_one = vcns.retry_upon_exception( - exceptions.RequestBad, max_attempts=1)(success_on_first_attempt) + exceptions.RequestBad, + max_attempts=1)(success_on_first_attempt) self.assertEqual(1, should_return_one()) def test_retry_on_exception_five_attempts(self): success_on_fifth_attempt = raise_until_attempt( 5, exceptions.RequestBad(uri='', response='')) should_return_five = vcns.retry_upon_exception( - exceptions.RequestBad, max_attempts=10)(success_on_fifth_attempt) + exceptions.RequestBad, + max_attempts=10)(success_on_fifth_attempt) self.assertEqual(5, should_return_five()) def test_retry_on_exception_exceed_attempts(self): success_on_fifth_attempt = raise_until_attempt( 5, exceptions.RequestBad(uri='', response='')) should_raise = vcns.retry_upon_exception( - exceptions.RequestBad, max_attempts=4)(success_on_fifth_attempt) + exceptions.RequestBad, + max_attempts=4)(success_on_fifth_attempt) + self.assertRaises(exceptions.RequestBad, should_raise) + + def test_retry_on_exception_exclude_error_codes_retry(self): + success_on_fifth_attempt = raise_until_attempt( + 5, exceptions.RequestBad(uri='', response=self.response)) + # excluding another error code, so should retry + should_return_five = vcns.retry_upon_exception_exclude_error_codes( + exceptions.RequestBad, [2], + max_attempts=10)(success_on_fifth_attempt) + self.assertEqual(5, should_return_five()) + + def test_retry_on_exception_exclude_error_codes_raise(self): + success_on_fifth_attempt = raise_until_attempt( + 5, exceptions.RequestBad(uri='', response=self.response)) + # excluding the returned error code, so no retries are expected + should_raise = vcns.retry_upon_exception_exclude_error_codes( + exceptions.RequestBad, [1], + max_attempts=10)(success_on_fifth_attempt) self.assertRaises(exceptions.RequestBad, should_raise)