From 7551e8c921da5b117c6f8fa3c6f575eeba9a77f8 Mon Sep 17 00:00:00 2001 From: Sloane Hertel Date: Tue, 22 Aug 2017 15:31:20 -0400 Subject: [PATCH] AWSRetry: allow retrying on additional ClientError exceptions (#28483) * Added the ability to extend the exception list in CloudRetry * AWSRetry boto and boto compatible * Updated tests to reflect boto/boto3 * Added boto to shippable requirements * Have base_class and added_exceptions default to None in CloudRetry AWSRetry - only retry on boto3 exceptions and remove boto requirement from tests * Make requested changes. --- lib/ansible/module_utils/cloud.py | 18 +++++++++--------- lib/ansible/module_utils/ec2.py | 9 ++++----- test/units/module_utils/ec2/test_aws.py | 16 ++++++++++++++++ 3 files changed, 29 insertions(+), 14 deletions(-) diff --git a/lib/ansible/module_utils/cloud.py b/lib/ansible/module_utils/cloud.py index 96024588c8..0d29071fe1 100644 --- a/lib/ansible/module_utils/cloud.py +++ b/lib/ansible/module_utils/cloud.py @@ -116,7 +116,7 @@ class CloudRetry(object): pass @staticmethod - def found(response_code): + def found(response_code, catch_extra_error_codes=None): """ Return True if the Response Code to retry on was found. Args: response_code (str): This is the Response Code that is being matched against. @@ -124,7 +124,7 @@ class CloudRetry(object): pass @classmethod - def _backoff(cls, backoff_strategy): + def _backoff(cls, backoff_strategy, catch_extra_error_codes=None): """ Retry calling the Cloud decorated function using the provided backoff strategy. Args: @@ -141,7 +141,7 @@ class CloudRetry(object): except Exception as e: if isinstance(e, cls.base_class): response_code = cls.status_code_from_exception(e) - if cls.found(response_code): + if cls.found(response_code, catch_extra_error_codes): msg = "{0}: Retrying in {1} seconds...".format(str(e), delay) syslog.syslog(syslog.LOG_INFO, msg) time.sleep(delay) @@ -158,7 +158,7 @@ class CloudRetry(object): return deco @classmethod - def exponential_backoff(cls, retries=10, delay=3, backoff=2, max_delay=60): + def exponential_backoff(cls, retries=10, delay=3, backoff=2, max_delay=60, catch_extra_error_codes=None): """ Retry calling the Cloud decorated function using an exponential backoff. @@ -174,10 +174,10 @@ class CloudRetry(object): default=60 """ return cls._backoff(_exponential_backoff( - retries=retries, delay=delay, backoff=backoff, max_delay=max_delay)) + retries=retries, delay=delay, backoff=backoff, max_delay=max_delay), catch_extra_error_codes) @classmethod - def jittered_backoff(cls, retries=10, delay=3, max_delay=60): + def jittered_backoff(cls, retries=10, delay=3, max_delay=60, catch_extra_error_codes=None): """ Retry calling the Cloud decorated function using a jittered backoff strategy. More on this strategy here: @@ -193,10 +193,10 @@ class CloudRetry(object): default=60 """ return cls._backoff(_full_jitter_backoff( - retries=retries, delay=delay, max_delay=max_delay)) + retries=retries, delay=delay, max_delay=max_delay), catch_extra_error_codes) @classmethod - def backoff(cls, tries=10, delay=3, backoff=1.1): + def backoff(cls, tries=10, delay=3, backoff=1.1, catch_extra_error_codes=None): """ Retry calling the Cloud decorated function using an exponential backoff. @@ -214,4 +214,4 @@ class CloudRetry(object): default=1.1 """ return cls.exponential_backoff( - retries=tries - 1, delay=delay, backoff=backoff, max_delay=None) + retries=tries - 1, delay=delay, backoff=backoff, max_delay=None, catch_extra_error_codes=catch_extra_error_codes) diff --git a/lib/ansible/module_utils/ec2.py b/lib/ansible/module_utils/ec2.py index b7ecaac1af..9aebc4d9ad 100644 --- a/lib/ansible/module_utils/ec2.py +++ b/lib/ansible/module_utils/ec2.py @@ -72,7 +72,7 @@ class AWSRetry(CloudRetry): return error.response['Error']['Code'] @staticmethod - def found(response_code): + def found(response_code, catch_extra_error_codes): # This list of failures is based on this API Reference # http://docs.aws.amazon.com/AWSEC2/latest/APIReference/errors-overview.html # @@ -88,12 +88,11 @@ class AWSRetry(CloudRetry): 'InternalFailure', 'InternalError', 'TooManyRequestsException', 'Throttling' ] + if catch_extra_error_codes: + retry_on.extend(catch_extra_error_codes) not_found = re.compile(r'^\w+.NotFound') - if response_code in retry_on or not_found.search(response_code): - return True - else: - return False + return response_code in retry_on or not_found.search(response_code) def boto3_conn(module, conn_type=None, resource=None, region=None, endpoint=None, **params): diff --git a/test/units/module_utils/ec2/test_aws.py b/test/units/module_utils/ec2/test_aws.py index 347eb4e688..5610d424c4 100644 --- a/test/units/module_utils/ec2/test_aws.py +++ b/test/units/module_utils/ec2/test_aws.py @@ -44,6 +44,22 @@ class RetryTestCase(unittest.TestCase): r = no_failures() self.assertEqual(self.counter, 1) + def test_extend_boto3_failures(self): + self.counter = 0 + err_msg = {'Error': {'Code': 'MalformedPolicyDocument'}} + + @AWSRetry.backoff(tries=2, delay=0.1, catch_extra_error_codes=['MalformedPolicyDocument']) + def extend_failures(): + self.counter += 1 + if self.counter < 2: + raise botocore.exceptions.ClientError(err_msg, 'Could not find you') + else: + return 'success' + + r = extend_failures() + self.assertEqual(r, 'success') + self.assertEqual(self.counter, 2) + def test_retry_once(self): self.counter = 0 err_msg = {'Error': {'Code': 'InstanceId.NotFound'}}