Merge pull request #1234 from openstack-charmers/test-cinder-client-retrier

Add ObjectRetrier to CinderaBackupTests

Author: wolsen

requirements.txt

@@ -50,7 +50,10 @@ python-novaclient
 python-octaviaclient
 python-swiftclient
 python-watcherclient
-tenacity
+# Due to https://github.com/jd/tenacity/pull/479 the strategy for mocking out tenacity
+# waits/times/etc no longer works.  Pin to 8.4.1 until it is solved.
+# Bug in tenacity tracking issue: https://github.com/jd/tenacity/issues/482
+tenacity<8.4.2
 paramiko
 
 # Documentation requirements

unit_tests/utilities/test_utilities.py

@@ -113,6 +113,28 @@ def func(self):
 
         mock_sleep.assert_not_called()
 
+    @mock.patch("time.sleep")
+    def test_object_wrap_multilevel_with_exception(self, mock_sleep):
+
+        class A:
+
+            def func(self):
+                raise SomeException()
+
+        class B:
+
+            def __init__(self):
+                self.a = A()
+
+        b = B()
+        # retry on a specific exception
+        wrapped_b = utilities.ObjectRetrierWraps(
+            b, num_retries=1, retry_exceptions=[SomeException])
+        with self.assertRaises(SomeException):
+            wrapped_b.a.func()
+
+        mock_sleep.assert_called_once_with(5)
+
     @mock.patch("time.sleep")
     def test_log_called(self, mock_sleep):
 
@@ -128,9 +150,7 @@ def func(self):
         with self.assertRaises(SomeException):
             wrapped_a.func()
 
-        # there should be two calls; one for the single retry and one for the
-        # failure.
-        self.assertEqual(mock_log.call_count, 2)
+        mock_log.assert_called()
 
     @mock.patch("time.sleep")
     def test_back_off_maximum(self, mock_sleep):

unit_tests/utilities/test_zaza_utilities_openstack.py

@@ -99,6 +99,9 @@ def setUp(self):
         self.neutronclient.list_agents.return_value = self.agents
         self.neutronclient.list_bgp_speaker_on_dragent.return_value = \
             self.bgp_speakers
+        self.patch("zaza.openstack.utilities.ObjectRetrierWraps",
+                   name="_object_retrier_wraps",
+                   new=lambda x, *_, **__: x)
 
     def test_create_port(self):
         self.patch_object(openstack_utils, "get_net_uuid")

zaza/openstack/charm_tests/cinder_backup/tests.py

@@ -22,6 +22,7 @@
 
 import zaza.model
 import zaza.openstack.charm_tests.test_utils as test_utils
+from zaza.openstack.utilities import retry_on_connect_failure
 import zaza.openstack.utilities.ceph as ceph_utils
 import zaza.openstack.utilities.openstack as openstack_utils
 
@@ -35,8 +36,9 @@ class CinderBackupTest(test_utils.OpenStackBaseTest):
     def setUpClass(cls):
         """Run class setup for running Cinder Backup tests."""
         super(CinderBackupTest, cls).setUpClass()
-        cls.cinder_client = openstack_utils.get_cinder_session_client(
-            cls.keystone_session)
+        cls.cinder_client = retry_on_connect_failure(
+            openstack_utils.get_cinder_session_client(cls.keystone_session),
+            log=logging.warn)
 
     @property
     def services(self):
@@ -101,7 +103,7 @@ def test_410_cinder_vol_create_backup_delete_restore_pool_inspect(self):
                     self.cinder_client.volumes,
                     cinder_vol.id,
                     wait_iteration_max_time=180,
-                    stop_after_attempt=15,
+                    stop_after_attempt=30,
                     expected_status='available',
                     msg='ceph-backed cinder volume')
 
@@ -123,7 +125,7 @@ def test_410_cinder_vol_create_backup_delete_restore_pool_inspect(self):
                     self.cinder_client.backups,
                     vol_backup.id,
                     wait_iteration_max_time=180,
-                    stop_after_attempt=15,
+                    stop_after_attempt=30,
                     expected_status='available',
                     msg='Backup volume')
 

zaza/openstack/charm_tests/manila/tests.py

@@ -359,7 +359,7 @@ def test_manila_share(self):
             self.manila_client.shares,
             share.id,
             wait_iteration_max_time=120,
-            stop_after_attempt=2,
+            stop_after_attempt=10,
             expected_status="available",
             msg="Waiting for a share to become available")
 

zaza/openstack/utilities/__init__.py

@@ -15,10 +15,30 @@
 """Collection of utilities to support zaza tests etc."""
 
 
+import logging
 import time
 
 from keystoneauth1.exceptions.connection import ConnectFailure
 
+NEVER_RETRY_EXCEPTIONS = (
+    AssertionError,
+    AttributeError,
+    ImportError,
+    IndexError,
+    KeyError,
+    NotImplementedError,
+    OverflowError,
+    RecursionError,
+    ReferenceError,
+    RuntimeError,
+    SyntaxError,
+    IndentationError,
+    SystemExit,
+    TypeError,
+    UnicodeError,
+    ZeroDivisionError,
+)
+
 
 class ObjectRetrierWraps(object):
     """An automatic retrier for an object.
@@ -76,35 +96,39 @@ def __init__(self, obj, num_retries=3, initial_interval=5.0, backoff=1.0,
             If a list, then it will only retry if the exception is one of the
             ones in the list.
         :type retry_exceptions: List[Exception]
+        :param log: If False, disable logging; if None (the default) or True,
+            use logging.warn; otherwise use the passed param `log`.
+        :type param: None | Boolean | Callable
         """
         # Note we use semi-private variable names that shouldn't clash with any
         # on the actual object.
         self.__obj = obj
+        if log in (None, True):
+            _log = logging.warning
+        elif log is False:
+            _log = lambda *_, **__: None  # noqa
+        else:
+            _log = log
         self.__kwargs = {
             'num_retries': num_retries,
             'initial_interval': initial_interval,
             'backoff': backoff,
             'max_interval': max_interval,
             'total_wait': total_wait,
             'retry_exceptions': retry_exceptions,
-            'log': log or (lambda x: None),
+            'log': _log,
         }
+        _log(f"ObjectRetrierWraps: wrapping {self.__obj}")
 
     def __getattr__(self, name):
         """Get attribute; delegates to wrapped object."""
-        # Note the above may generate an attribute error; we expect this and
-        # will fail with an attribute error.
-        attr = getattr(self.__obj, name)
-        if callable(attr) or hasattr(attr, "__getattr__"):
+        obj = self.__obj
+        attr = getattr(obj, name)
+        if callable(attr):
             return ObjectRetrierWraps(attr, **self.__kwargs)
-        else:
+        if attr.__class__.__module__ == 'builtins':
             return attr
-        # TODO(ajkavanagh): Note detecting a property is a bit trickier.  we
-        # can do isinstance(attr, property), but then the act of accessing it
-        # is what calls it.  i.e. it would fail at the getattr(self.__obj,
-        # name) stage.  The solution is to check first, and if it's a property,
-        # then treat it like the retrier.  However, I think this is too
-        # complex for the first go, and to use manual retries in that instance.
+        return ObjectRetrierWraps(attr, **self.__kwargs)
 
     def __call__(self, *args, **kwargs):
         """Call the object; delegates to the wrapped object."""
@@ -126,16 +150,20 @@ def __call__(self, *args, **kwargs):
                 # is not in the list of retries, then raise an exception
                 # immediately.  This means that if retry_exceptions is None,
                 # then the method is always retried.
+                if isinstance(e, NEVER_RETRY_EXCEPTIONS):
+                    log("ObjectRetrierWraps: error {} is never caught; raising"
+                        .format(str(e)))
+                    raise
                 if (retry_exceptions is not None and
                         type(e) not in retry_exceptions):
                     raise
                 retry += 1
                 if retry > num_retries:
-                    log("{}: exceeded number of retries, so erroring out"
-                        .format(str(obj)))
+                    log("ObjectRetrierWraps: exceeded number of retries, "
+                        "so erroring out")
                     raise e
-                log("{}: call failed: retrying in {} seconds"
-                    .format(str(obj), wait))
+                log("ObjectRetrierWraps: call failed: retrying in {} "
+                    "seconds" .format(wait))
                 time.sleep(wait)
                 wait_so_far += wait
                 if wait_so_far >= total_wait: