Improve efs_facts (#31817)

Avoid an infinite loop when no EFS resources are present

Use standard ansible approaches to pagination, retries,
exception handling, tag processing

lib/ansible/modules/cloud/amazon/efs_facts.py

@@ -39,10 +39,9 @@ options:
         default: None
     targets:
         description:
-            - "List of mounted targets. It should be a list of dictionaries, every dictionary should include next attributes:
+          - list of targets on which to filter the returned results
-                    - SubnetId - Mandatory. The ID of the subnet to add the mount target in.
+          - result must match all of the specified targets, each of which can be
-                    - IpAddress - Optional. A valid IPv4 address within the address range of the specified subnet.
+            a security group ID, a subnet ID or an IP address
-                    - SecurityGroups - Optional. List of security group IDs, of the form 'sg-xxxxxxxx'. These must be for the same VPC as subnet specified."
         required: false
         default: None
 extends_documentation_fragment:
@@ -158,16 +157,15 @@ tags:
 
 
 from collections import defaultdict
-from time import sleep
 
 try:
-    from botocore.exceptions import ClientError
+    import botocore
 except ImportError:
-    pass  # caught by imported HAS_BOTO3
+    pass  # caught by AnsibleAWSModule
 
-from ansible.module_utils.basic import AnsibleModule
+from ansible.module_utils.aws.core import AnsibleAWSModule
-from ansible.module_utils.ec2 import (boto3_conn, get_aws_connection_info, ec2_argument_spec,
+from ansible.module_utils.ec2 import boto3_conn, get_aws_connection_info, ec2_argument_spec, AWSRetry
-                                      camel_dict_to_snake_dict, HAS_BOTO3)
+from ansible.module_utils.ec2 import camel_dict_to_snake_dict, boto3_tag_list_to_ansible_dict
 
 
 class EFSConnection(object):
@@ -181,97 +179,89 @@ class EFSConnection(object):
             self.connection = boto3_conn(module, conn_type='client',
                                          resource='efs', region=region,
                                          **aws_connect_params)
+            self.module = module
         except Exception as e:
             module.fail_json(msg="Failed to connect to AWS: %s" % str(e))
 
         self.region = region
 
-    def get_file_systems(self, **kwargs):
+    @AWSRetry.exponential_backoff()
+    def list_file_systems(self, **kwargs):
         """
          Returns generator of file systems including all attributes of FS
         """
-        items = iterate_all(
+        paginator = self.connection.get_paginator('describe_file_systems')
-            'FileSystems',
+        return paginator.paginate(**kwargs).build_full_result()['FileSystems']
-            self.connection.describe_file_systems,
-            **kwargs
-        )
-        for item in items:
-            item['CreationTime'] = str(item['CreationTime'])
-            """
-            Suffix of network path to be used as NFS device for mount. More detail here:
-            http://docs.aws.amazon.com/efs/latest/ug/gs-step-three-connect-to-ec2-instance.html
-            """
-            item['MountPoint'] = '.%s.efs.%s.amazonaws.com:/' % (item['FileSystemId'], self.region)
-            if 'Timestamp' in item['SizeInBytes']:
-                item['SizeInBytes']['Timestamp'] = str(item['SizeInBytes']['Timestamp'])
-            if item['LifeCycleState'] == self.STATE_AVAILABLE:
-                item['Tags'] = self.get_tags(FileSystemId=item['FileSystemId'])
-                item['MountTargets'] = list(self.get_mount_targets(FileSystemId=item['FileSystemId']))
-            else:
-                item['Tags'] = {}
-                item['MountTargets'] = []
-            yield item
 
-    def get_tags(self, **kwargs):
+    @AWSRetry.exponential_backoff()
+    def get_tags(self, file_system_id):
         """
          Returns tag list for selected instance of EFS
         """
-        tags = iterate_all(
+        paginator = self.connection.get_paginator('describe_tags')
-            'Tags',
+        return boto3_tag_list_to_ansible_dict(paginator.paginate(FileSystemId=file_system_id).build_full_result()['Tags'])
-            self.connection.describe_tags,
+
-            **kwargs
+    @AWSRetry.exponential_backoff()
-        )
+    def get_mount_targets(self, file_system_id):
-        return dict((tag['Key'], tag['Value']) for tag in tags)
-
-    def get_mount_targets(self, **kwargs):
         """
          Returns mount targets for selected instance of EFS
         """
-        targets = iterate_all(
+        paginator = self.connection.get_paginator('describe_mount_targets')
-            'MountTargets',
+        return paginator.paginate(FileSystemId=file_system_id).build_full_result()['MountTargets']
-            self.connection.describe_mount_targets,
-            **kwargs
-        )
-        for target in targets:
-            if target['LifeCycleState'] == self.STATE_AVAILABLE:
-                target['SecurityGroups'] = list(self.get_security_groups(
-                    MountTargetId=target['MountTargetId']
-                ))
-            else:
-                target['SecurityGroups'] = []
-            yield target
 
-    def get_security_groups(self, **kwargs):
+    @AWSRetry.exponential_backoff()
+    def get_security_groups(self, mount_target_id):
         """
          Returns security groups for selected instance of EFS
         """
-        return iterate_all(
+        return self.connection.describe_mount_target_security_groups(MountTargetId=mount_target_id)['SecurityGroups']
-            'SecurityGroups',
+
-            self.connection.describe_mount_target_security_groups,
+    def get_file_systems(self, file_system_id=None, creation_token=None):
-            **kwargs
+        kwargs = dict()
-        )
+        if file_system_id:
-
+            kwargs['FileSystemId'] = file_system_id
-
+        if creation_token:
-def iterate_all(attr, map_method, **kwargs):
+            kwargs['CreationToken'] = creation_token
-    """
-     Method creates iterator from boto result set
-    """
-    args = dict((key, value) for (key, value) in kwargs.items() if value is not None)
-    wait = 1
-    while True:
         try:
-            data = map_method(**args)
+            file_systems = self.list_file_systems(**kwargs)
-            for elm in data[attr]:
+        except (botocore.exceptions.ClientError, botocore.exceptions.BotoCoreError) as e:
-                yield elm
+            self.module.fail_json_aws(e, msg="Couldn't get EFS file systems")
-            if 'NextMarker' in data:
+
-                args['Marker'] = data['Nextmarker']
+        results = list()
-                continue
+        for item in file_systems:
-            break
+            item['CreationTime'] = str(item['CreationTime'])
-        except ClientError as e:
+            """
-            if e.response['Error']['Code'] == "ThrottlingException" and wait < 600:
+            Suffix of network path to be used as NFS device for mount. More detail here:
-                sleep(wait)
+            http://docs.aws.amazon.com/efs/latest/ug/gs-step-three-connect-to-ec2-instance.html
-                wait = wait * 2
+            """
-                continue
+            item['MountPoint'] = '.%s.efs.%s.amazonaws.com:/' % (item['FileSystemId'], self.region)
+            if 'Timestamp' in item['SizeInBytes']:
+                item['SizeInBytes']['Timestamp'] = str(item['SizeInBytes']['Timestamp'])
+            if item['LifeCycleState'] == self.STATE_AVAILABLE:
+                try:
+                    item['MountTargets'] = self.get_mount_targets(item['FileSystemId'])
+                except (botocore.exceptions.ClientError, botocore.exceptions.BotoCoreError) as e:
+                    self.module.fail_json_aws(e, msg="Couldn't get EFS targets")
+                for target in item['MountTargets']:
+                    if target['LifeCycleState'] == self.STATE_AVAILABLE:
+                        try:
+                            target['SecurityGroups'] = self.get_security_groups(target['MountTargetId'])
+                        except (botocore.exceptions.ClientError, botocore.exceptions.BotoCoreError) as e:
+                            self.module.fail_json_aws(e, msg="Couldn't get EFS security groups")
+                    else:
+                        target['SecurityGroups'] = []
+            else:
+                item['tags'] = {}
+                item['mount_targets'] = []
+            result = camel_dict_to_snake_dict(item)
+            # Set tags *after* doing camel to snake
+            if result['life_cycle_state'] == self.STATE_AVAILABLE:
+                try:
+                    result['tags'] = self.get_tags(result['file_system_id'])
+                except (botocore.exceptions.ClientError, botocore.exceptions.BotoCoreError) as e:
+                    self.module.fail_json_aws(e, msg="Couldn't get EFS tags")
+            results.append(result)
+        return results
 
 
 def prefix_to_attr(attr_id):
@@ -279,18 +269,13 @@ def prefix_to_attr(attr_id):
      Helper method to convert ID prefix to mount target attribute
     """
     attr_by_prefix = {
-        'fsmt-': 'MountTargetId',
+        'fsmt-': 'mount_target_id',
-        'subnet-': 'SubnetId',
+        'subnet-': 'subnet_id',
-        'eni-': 'NetworkInterfaceId',
+        'eni-': 'network_interface_id',
-        'sg-': 'SecurityGroups'
+        'sg-': 'security_groups'
     }
-    prefix = first_or_default(filter(
+    return first_or_default([attr_name for (prefix, attr_name) in attr_by_prefix.items()
-        lambda pref: str(attr_id).startswith(pref),
+                             if str(attr_id).startswith(prefix)], 'ip_address')
-        attr_by_prefix.keys()
-    ))
-    if prefix:
-        return attr_by_prefix[prefix]
-    return 'IpAddress'
 
 
 def first_or_default(items, default=None):
@@ -314,7 +299,7 @@ def has_tags(available, required):
 
 def has_targets(available, required):
     """
-    Helper method to determine if mount tager requested already exists
+    Helper method to determine if mount target requested already exists
     """
     grouped = group_list_of_dict(available)
     for (value, field) in required:
@@ -346,11 +331,8 @@ def main():
         targets=dict(type="list", default=[])
     ))
 
-    module = AnsibleModule(argument_spec=argument_spec,
+    module = AnsibleAWSModule(argument_spec=argument_spec,
-                           supports_check_mode=True)
+                              supports_check_mode=True)
-
-    if not HAS_BOTO3:
-        module.fail_json(msg='boto3 required for this module')
 
     region, _, aws_connect_params = get_aws_connection_info(module, boto3=True)
     connection = EFSConnection(module, region, **aws_connect_params)
@@ -360,17 +342,15 @@ def main():
     tags = module.params.get('tags')
     targets = module.params.get('targets')
 
-    file_systems_info = connection.get_file_systems(FileSystemId=fs_id, CreationToken=name)
+    file_systems_info = connection.get_file_systems(fs_id, name)
 
     if tags:
-        file_systems_info = filter(lambda item: has_tags(item['Tags'], tags), file_systems_info)
+        file_systems_info = [item for item in file_systems_info if has_tags(item['tags'], tags)]
 
     if targets:
         targets = [(item, prefix_to_attr(item)) for item in targets]
-        file_systems_info = filter(lambda item:
+        file_systems_info = [item for item in file_systems_info if has_targets(item['mount_targets'], targets)]
-                                   has_targets(item['MountTargets'], targets), file_systems_info)
 
-    file_systems_info = [camel_dict_to_snake_dict(x) for x in file_systems_info]
     module.exit_json(changed=False, ansible_facts={'efs': file_systems_info})