Merge pull request #1237 from openstack-charmers/retrier-fixes-master
[main] ObjectRetrierWraps fixes (cherry-pick #1234)
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -113,6 +113,28 @@ class TestObjectRetrierWraps(ut_utils.BaseTestCase):
|
||||
|
||||
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 @@ class TestObjectRetrierWraps(ut_utils.BaseTestCase):
|
||||
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):
|
||||
|
||||
@@ -99,6 +99,9 @@ class TestOpenStackUtils(ut_utils.BaseTestCase):
|
||||
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")
|
||||
|
||||
@@ -22,6 +22,7 @@ import tenacity
|
||||
|
||||
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 @@ class CinderBackupTest(test_utils.OpenStackBaseTest):
|
||||
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 @@ class CinderBackupTest(test_utils.OpenStackBaseTest):
|
||||
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')
|
||||
|
||||
|
||||
@@ -359,7 +359,7 @@ packages:
|
||||
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")
|
||||
|
||||
|
||||
@@ -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,10 +96,19 @@ class ObjectRetrierWraps(object):
|
||||
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,
|
||||
@@ -87,24 +116,19 @@ class ObjectRetrierWraps(object):
|
||||
'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 @@ class ObjectRetrierWraps(object):
|
||||
# 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:
|
||||
|
||||
Reference in New Issue
Block a user