diff --git a/requirements.txt b/requirements.txt index 099b50bf16..06fe95df75 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.17.1 # 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 b1aba0a50d..234cda95bc 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 vmware_nsx.api_client import client as nsx_client from vmware_nsx.api_client import eventlet_client @@ -46,6 +49,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)