Merge "Replace retrying with tenacity"

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

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:

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),
+    return tenacity.retry(reraise=True,
-                          wait_exponential_multiplier=delay,
+                          retry=tenacity.retry_if_exception_type(exc),
-                          wait_exponential_max=max_delay,
+                          wait=tenacity.wait_exponential(
-                          stop_max_attempt_number=max_attempts)
+                                multiplier=delay, max=max_delay),
+                          stop=tenacity.stop_after_attempt(max_attempts))
 
 
 def list_match(list1, list2):

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

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,
+def _get_bad_request_error_code(e):
-                         max_attempts=cfg.CONF.nsxv.retries):
+    """Get the error code out of the exception"""
-    return retrying.retry(retry_on_exception=lambda e: isinstance(e, exc),
+    try:
-                          wait_exponential_multiplier=delay,
+        desc = et.fromstring(e.response)
-                          wait_exponential_max=max_delay,
+        return int(desc.find('errorCode').text)
-                          stop_max_attempt_number=max_attempts)
+    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

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)

vmware_nsx/tests/unit/nsx_v/test_misc.py

@@ -31,23 +31,49 @@ def raise_until_attempt(attempt, exception):
 
 
 class TestMisc(base.BaseTestCase):
+    response = """
+        <error><details>Dummy</details><errorCode>1</errorCode>
+        <moduleName>core-services</moduleName></error>
+        """
+
     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)