Add AWSRetry backoff logic to route53_zone and route53_info (ansible-…

…collections#865)

Add AWSRetry backoff logic to route53_zone and route53_info

SUMMARY
Add AWSRetry backoff logic to route53_zone and route53_info. Currently from time to time I've been hitting AWS throttling errors leading to ansible failures:
An exception occurred during task execution. To see the full traceback, use -vvv. The error was: botocore.exceptions.ClientError: An error occurred (Throttling) when calling the ListHostedZones operation (reached max retries: 4): Rate exceeded
fatal: [localhost_staging -> 127.0.0.1]: FAILED! => changed=false
  boto3_version: 1.20.34
  botocore_version: 1.23.34
  error:
    code: Throttling
    message: Rate exceeded
    type: Sender
  msg: 'Could not list current hosted zones: An error occurred (Throttling) when calling the ListHostedZones operation (reached max retries: 4): Rate exceeded'
  response_metadata:
    http_headers:
      connection: close
      content-length: '255'
      content-type: text/xml
      date: Fri, 14 Jan 2022 12:09:35 GMT
      x-amzn-requestid: xxxxxxx
    http_status_code: 400
    max_attempts_reached: true
    request_id: xxxxxxx
    retry_attempts: 4
ISSUE TYPE

Bugfix Pull Request

COMPONENT NAME
route53_zone
route53_info
ADDITIONAL INFORMATION
I've added the standard backoff retry logic and split out the paginators.

Reviewed-by: Alina Buzachis <None>
Reviewed-by: Markus Bergholz <[email protected]>

This commit was initially merged in https://github.com/ansible-collections/community.aws
See: ansible-collections/community.aws@ca1d33f

plugins/modules/route53_info.py

@@ -212,9 +212,17 @@
 from ansible.module_utils._text import to_native
 
 from ansible_collections.amazon.aws.plugins.module_utils.core import AnsibleAWSModule
+from ansible_collections.amazon.aws.plugins.module_utils.ec2 import AWSRetry
 
 
-def get_hosted_zone(client, module):
+# Split out paginator to allow for the backoff decorator to function
+@AWSRetry.jittered_backoff()
+def _paginated_result(paginator_name, **params):
+    paginator = client.get_paginator(paginator_name)
+    return paginator.paginate(**params).build_full_result()
+
+
+def get_hosted_zone():
     params = dict()
 
     if module.params.get('hosted_zone_id'):
@@ -225,7 +233,7 @@ def get_hosted_zone(client, module):
     return client.get_hosted_zone(**params)
 
 
-def reusable_delegation_set_details(client, module):
+def reusable_delegation_set_details():
     params = dict()
 
     if not module.params.get('delegation_set_id'):
@@ -246,7 +254,7 @@ def reusable_delegation_set_details(client, module):
     return results
 
 
-def list_hosted_zones(client, module):
+def list_hosted_zones():
     params = dict()
 
     # Set PaginationConfig with max_items
@@ -261,15 +269,15 @@ def list_hosted_zones(client, module):
     if module.params.get('delegation_set_id'):
         params['DelegationSetId'] = module.params.get('delegation_set_id')
 
-    paginator = client.get_paginator('list_hosted_zones')
-    zones = paginator.paginate(**params).build_full_result()['HostedZones']
+    zones = _paginated_result('list_hosted_zones', **params)['HostedZones']
+
     return {
         "HostedZones": zones,
         "list": zones,
     }
 
 
-def list_hosted_zones_by_name(client, module):
+def list_hosted_zones_by_name():
     params = dict()
 
     if module.params.get('hosted_zone_id'):
@@ -287,7 +295,7 @@ def list_hosted_zones_by_name(client, module):
     return client.list_hosted_zones_by_name(**params)
 
 
-def change_details(client, module):
+def change_details():
     params = dict()
 
     if module.params.get('change_id'):
@@ -299,11 +307,11 @@ def change_details(client, module):
     return results
 
 
-def checker_ip_range_details(client, module):
+def checker_ip_range_details():
     return client.get_checker_ip_ranges()
 
 
-def get_count(client, module):
+def get_count():
     if module.params.get('query') == 'health_check':
         results = client.get_health_check_count()
     else:
@@ -312,7 +320,7 @@ def get_count(client, module):
     return results
 
 
-def get_health_check(client, module):
+def get_health_check():
     params = dict()
 
     if not module.params.get('health_check_id'):
@@ -330,7 +338,7 @@ def get_health_check(client, module):
     return results
 
 
-def get_resource_tags(client, module):
+def get_resource_tags():
     params = dict()
 
     if module.params.get('resource_id'):
@@ -346,7 +354,7 @@ def get_resource_tags(client, module):
     return client.list_tags_for_resources(**params)
 
 
-def list_health_checks(client, module):
+def list_health_checks():
     params = dict()
 
     if module.params.get('next_marker'):
@@ -358,15 +366,15 @@ def list_health_checks(client, module):
             MaxItems=module.params.get('max_items')
         )
 
-    paginator = client.get_paginator('list_health_checks')
-    health_checks = paginator.paginate(**params).build_full_result()['HealthChecks']
+    health_checks = _paginated_result('list_health_checks', **params)['HealthChecks']
+
     return {
         "HealthChecks": health_checks,
         "list": health_checks,
     }
 
 
-def record_sets_details(client, module):
+def record_sets_details():
     params = dict()
 
     if module.params.get('hosted_zone_id'):
@@ -390,16 +398,15 @@ def record_sets_details(client, module):
             MaxItems=module.params.get('max_items')
         )
 
-    paginator = client.get_paginator('list_resource_record_sets')
-    record_sets = paginator.paginate(**params).build_full_result()['ResourceRecordSets']
+    record_sets = _paginated_result('list_resource_record_sets', **params)['ResourceRecordSets']
 
     return {
         "ResourceRecordSets": record_sets,
         "list": record_sets,
     }
 
 
-def health_check_details(client, module):
+def health_check_details():
     health_check_invocations = {
         'list': list_health_checks,
         'details': get_health_check,
@@ -409,11 +416,11 @@ def health_check_details(client, module):
         'tags': get_resource_tags,
     }
 
-    results = health_check_invocations[module.params.get('health_check_method')](client, module)
+    results = health_check_invocations[module.params.get('health_check_method')]()
     return results
 
 
-def hosted_zone_details(client, module):
+def hosted_zone_details():
     hosted_zone_invocations = {
         'details': get_hosted_zone,
         'list': list_hosted_zones,
@@ -422,11 +429,14 @@ def hosted_zone_details(client, module):
         'tags': get_resource_tags,
     }
 
-    results = hosted_zone_invocations[module.params.get('hosted_zone_method')](client, module)
+    results = hosted_zone_invocations[module.params.get('hosted_zone_method')]()
     return results
 
 
 def main():
+    global module
+    global client
+
     argument_spec = dict(
         query=dict(choices=[
             'change',
@@ -475,7 +485,7 @@ def main():
     )
 
     try:
-        route53 = module.client('route53')
+        client = module.client('route53', retry_decorator=AWSRetry.jittered_backoff())
     except (botocore.exceptions.ClientError, botocore.exceptions.BotoCoreError) as e:
         module.fail_json_aws(e, msg='Failed to connect to AWS')
 
@@ -490,7 +500,7 @@ def main():
 
     results = dict(changed=False)
     try:
-        results = invocations[module.params.get('query')](route53, module)
+        results = invocations[module.params.get('query')]()
     except (botocore.exceptions.ClientError, botocore.exceptions.BotoCoreError) as e:
         module.fail_json(msg=to_native(e))
 

plugins/modules/route53_zone.py

@@ -5,7 +5,7 @@
 from __future__ import absolute_import, division, print_function
 __metaclass__ = type
 
-DOCUMENTATION = '''
+DOCUMENTATION = r'''
 module: route53_zone
 short_description: add or delete Route53 zones
 version_added: 1.0.0
@@ -65,7 +65,7 @@
 author: "Christopher Troup (@minichate)"
 '''
 
-EXAMPLES = '''
+EXAMPLES = r'''
 - name: create a public zone
   community.aws.route53_zone:
     zone: example.com
@@ -105,7 +105,7 @@
     purge_tags: true
 '''
 
-RETURN = '''
+RETURN = r'''
 comment:
     description: optional hosted zone comment
     returned: when hosted zone exists
@@ -149,6 +149,7 @@
 
 import time
 from ansible_collections.amazon.aws.plugins.module_utils.core import AnsibleAWSModule
+from ansible_collections.amazon.aws.plugins.module_utils.ec2 import AWSRetry
 from ansible_collections.community.aws.plugins.module_utils.route53 import manage_tags
 from ansible_collections.community.aws.plugins.module_utils.route53 import get_tags
 
@@ -158,10 +159,15 @@
     pass  # caught by AnsibleAWSModule
 
 
-def find_zones(module, client, zone_in, private_zone):
+@AWSRetry.jittered_backoff()
+def _list_zones():
+    paginator = client.get_paginator('list_hosted_zones')
+    return paginator.paginate().build_full_result()
+
+
+def find_zones(zone_in, private_zone):
     try:
-        paginator = client.get_paginator('list_hosted_zones')
-        results = paginator.paginate().build_full_result()
+        results = _list_zones()
     except (BotoCoreError, ClientError) as e:
         module.fail_json_aws(e, msg="Could not list current hosted zones")
     zones = []
@@ -176,7 +182,7 @@ def find_zones(module, client, zone_in, private_zone):
     return zones
 
 
-def create(module, client, matching_zones):
+def create(matching_zones):
     zone_in = module.params.get('zone').lower()
     vpc_id = module.params.get('vpc_id')
     vpc_region = module.params.get('vpc_region')
@@ -201,9 +207,9 @@ def create(module, client, matching_zones):
     }
 
     if private_zone:
-        changed, result = create_or_update_private(module, client, matching_zones, record)
+        changed, result = create_or_update_private(matching_zones, record)
     else:
-        changed, result = create_or_update_public(module, client, matching_zones, record)
+        changed, result = create_or_update_public(matching_zones, record)
 
     zone_id = result.get('zone_id')
     if zone_id:
@@ -216,7 +222,7 @@ def create(module, client, matching_zones):
     return changed, result
 
 
-def create_or_update_private(module, client, matching_zones, record):
+def create_or_update_private(matching_zones, record):
     for z in matching_zones:
         try:
             result = client.get_hosted_zone(Id=z['Id'])  # could be in different regions or have different VPCids
@@ -275,7 +281,7 @@ def create_or_update_private(module, client, matching_zones, record):
     return changed, record
 
 
-def create_or_update_public(module, client, matching_zones, record):
+def create_or_update_public(matching_zones, record):
     zone_details, zone_delegation_set_details = None, {}
     for matching_zone in matching_zones:
         try:
@@ -332,7 +338,7 @@ def create_or_update_public(module, client, matching_zones, record):
     return changed, record
 
 
-def delete_private(module, client, matching_zones, vpc_id, vpc_region):
+def delete_private(matching_zones, vpc_id, vpc_region):
     for z in matching_zones:
         try:
             result = client.get_hosted_zone(Id=z['Id'])
@@ -360,7 +366,7 @@ def delete_private(module, client, matching_zones, vpc_id, vpc_region):
     return False, "The vpc_id and the vpc_region do not match a private hosted zone."
 
 
-def delete_public(module, client, matching_zones):
+def delete_public(matching_zones):
     if len(matching_zones) > 1:
         changed = False
         msg = "There are multiple zones that match. Use hosted_zone_id to specify the correct zone."
@@ -375,7 +381,7 @@ def delete_public(module, client, matching_zones):
     return changed, msg
 
 
-def delete_hosted_id(module, client, hosted_zone_id, matching_zones):
+def delete_hosted_id(hosted_zone_id, matching_zones):
     if hosted_zone_id == "all":
         deleted = []
         for z in matching_zones:
@@ -401,7 +407,7 @@ def delete_hosted_id(module, client, hosted_zone_id, matching_zones):
     return changed, msg
 
 
-def delete(module, client, matching_zones):
+def delete(matching_zones):
     zone_in = module.params.get('zone').lower()
     vpc_id = module.params.get('vpc_id')
     vpc_region = module.params.get('vpc_region')
@@ -414,12 +420,12 @@ def delete(module, client, matching_zones):
 
     if zone_in in [z['Name'] for z in matching_zones]:
         if hosted_zone_id:
-            changed, result = delete_hosted_id(module, client, hosted_zone_id, matching_zones)
+            changed, result = delete_hosted_id(hosted_zone_id, matching_zones)
         else:
             if private_zone:
-                changed, result = delete_private(module, client, matching_zones, vpc_id, vpc_region)
+                changed, result = delete_private(matching_zones, vpc_id, vpc_region)
             else:
-                changed, result = delete_public(module, client, matching_zones)
+                changed, result = delete_public(matching_zones)
     else:
         changed = False
         result = "No zone to delete."
@@ -428,6 +434,9 @@ def delete(module, client, matching_zones):
 
 
 def main():
+    global module
+    global client
+
     argument_spec = dict(
         zone=dict(required=True),
         state=dict(default='present', choices=['present', 'absent']),
@@ -461,13 +470,13 @@ def main():
 
     private_zone = bool(vpc_id and vpc_region)
 
-    client = module.client('route53')
+    client = module.client('route53', retry_decorator=AWSRetry.jittered_backoff())
 
-    zones = find_zones(module, client, zone_in, private_zone)
+    zones = find_zones(zone_in, private_zone)
     if state == 'present':
-        changed, result = create(module, client, matching_zones=zones)
+        changed, result = create(matching_zones=zones)
     elif state == 'absent':
-        changed, result = delete(module, client, matching_zones=zones)
+        changed, result = delete(matching_zones=zones)
 
     if isinstance(result, dict):
         module.exit_json(changed=changed, result=result, **result)