From caa6e32e6b2f4acdd6f497b8296872c4947217f8 Mon Sep 17 00:00:00 2001 From: Mark Chappell Date: Mon, 4 Oct 2021 14:58:34 +0200 Subject: [PATCH 1/4] ec2_eni - Move tagging to module_utils tagging code --- changelogs/fragments/522-ec2_eni-tagging.yml | 2 + plugins/modules/ec2_eni.py | 70 ++++++-------------- 2 files changed, 24 insertions(+), 48 deletions(-) create mode 100644 changelogs/fragments/522-ec2_eni-tagging.yml diff --git a/changelogs/fragments/522-ec2_eni-tagging.yml b/changelogs/fragments/522-ec2_eni-tagging.yml new file mode 100644 index 00000000000..3af277f5c0d --- /dev/null +++ b/changelogs/fragments/522-ec2_eni-tagging.yml @@ -0,0 +1,2 @@ +minor_changes: +- ec2_eni - use module_util helper for tagging ENIs (https://github.com/ansible-collections/amazon.aws/pull/522). diff --git a/plugins/modules/ec2_eni.py b/plugins/modules/ec2_eni.py index 640130e1312..17ebc6ac427 100644 --- a/plugins/modules/ec2_eni.py +++ b/plugins/modules/ec2_eni.py @@ -302,10 +302,10 @@ from ..module_utils.core import AnsibleAWSModule from ..module_utils.core import is_boto3_error_code from ..module_utils.ec2 import AWSRetry -from ..module_utils.ec2 import ansible_dict_to_boto3_tag_list from ..module_utils.ec2 import get_ec2_security_group_ids_from_names -from ..module_utils.ec2 import boto3_tag_list_to_ansible_dict -from ..module_utils.ec2 import compare_aws_tags +from ..module_utils.tagging import boto3_tag_list_to_ansible_dict +from ..module_utils.tagging import boto3_tag_specifications +from ..module_utils.tagging import ensure_ec2_tags from ..module_utils.waiters import get_waiter @@ -336,17 +336,11 @@ def get_eni_info(interface): } if "TagSet" in interface: - tags = {} - name = None - for tag in interface["TagSet"]: - tags[tag["Key"]] = tag["Value"] - if tag["Key"] == "Name": - name = tag["Value"] + tags = boto3_tag_list_to_ansible_dict(interface["TagSet"]) + if "Name" in tags: + interface_info["name"] = tags["Name"] interface_info["tags"] = tags - if name is not None: - interface_info["name"] = name - if "Attachment" in interface: interface_info['attachment'] = { 'attachment_id': interface["Attachment"].get("AttachmentId"), @@ -429,9 +423,12 @@ def create_eni(connection, vpc_id, module): secondary_private_ip_addresses = module.params.get("secondary_private_ip_addresses") secondary_private_ip_address_count = module.params.get("secondary_private_ip_address_count") changed = False - tags = module.params.get("tags") + + tags = module.params.get("tags") or dict() name = module.params.get("name") - purge_tags = module.params.get("purge_tags") + # Make sure that the 'name' parameter sets the Name tag + if name: + tags['Name'] = name try: args = {"SubnetId": subnet_id} @@ -441,6 +438,9 @@ def create_eni(connection, vpc_id, module): args["Description"] = description if len(security_groups) > 0: args["Groups"] = security_groups + if tags: + args["TagSpecifications"] = boto3_tag_specifications(tags, types='network-interface') + eni_dict = connection.create_network_interface(aws_retry=True, **args) eni = eni_dict["NetworkInterface"] # Once we have an ID make sure we're always modifying the same object @@ -483,8 +483,6 @@ def create_eni(connection, vpc_id, module): connection.delete_network_interface(aws_retry=True, NetworkInterfaceId=eni_id) raise - manage_tags(eni, name, tags, purge_tags, connection) - # Refresh the eni data eni = describe_eni(connection, module, eni_id) changed = True @@ -633,7 +631,7 @@ def modify_eni(connection, module, eni): changed |= detach_eni(connection, eni, module) get_waiter(connection, 'network_interface_available').wait(NetworkInterfaceIds=[eni_id]) - changed |= manage_tags(eni, name, tags, purge_tags, connection) + changed |= manage_tags(connection, module, eni, name, tags, purge_tags) except (botocore.exceptions.ClientError, botocore.exceptions.BotoCoreError) as e: module.fail_json_aws(e, "Failed to modify eni {0}".format(eni_id)) @@ -786,42 +784,18 @@ def _get_vpc_id(connection, module, subnet_id): module.fail_json_aws(e, "Failed to get vpc_id for {0}".format(subnet_id)) -def manage_tags(eni, name, new_tags, purge_tags, connection): - changed = False - - if "TagSet" in eni: - old_tags = boto3_tag_list_to_ansible_dict(eni['TagSet']) - elif new_tags: - old_tags = {} - else: - # No new tags and nothing in TagSet - return False - +def manage_tags(connection, module, eni, name, tags, purge_tags): # Do not purge tags unless tags is not None - if new_tags is None: + if tags is None: purge_tags = False - new_tags = {} + tags = {} if name: - new_tags['Name'] = name + tags['Name'] = name - tags_to_set, tags_to_delete = compare_aws_tags( - old_tags, new_tags, - purge_tags=purge_tags, - ) - if tags_to_set: - connection.create_tags( - aws_retry=True, - Resources=[eni['NetworkInterfaceId']], - Tags=ansible_dict_to_boto3_tag_list(tags_to_set)) - changed |= True - if tags_to_delete: - delete_with_current_values = dict((k, old_tags.get(k)) for k in tags_to_delete) - connection.delete_tags( - aws_retry=True, - Resources=[eni['NetworkInterfaceId']], - Tags=ansible_dict_to_boto3_tag_list(delete_with_current_values)) - changed |= True + eni_id = eni['NetworkInterfaceId'] + + changed = ensure_ec2_tags(connection, module, eni_id, tags=tags, purge_tags=purge_tags) return changed From 46ff6d875af0435f7058d11b57b901ff1e487f3d Mon Sep 17 00:00:00 2001 From: Mark Chappell Date: Mon, 4 Oct 2021 14:58:54 +0200 Subject: [PATCH 2/4] Refactor detach wait/timeout code --- plugins/modules/ec2_eni.py | 20 ++++++++++++------- .../ec2_eni/tasks/test_attachment.yaml | 8 ++++++++ 2 files changed, 21 insertions(+), 7 deletions(-) diff --git a/plugins/modules/ec2_eni.py b/plugins/modules/ec2_eni.py index 17ebc6ac427..b66346348e1 100644 --- a/plugins/modules/ec2_eni.py +++ b/plugins/modules/ec2_eni.py @@ -640,6 +640,16 @@ def modify_eni(connection, module, eni): module.exit_json(changed=changed, interface=get_eni_info(eni)) +def _wait_for_detach(connection, module, eni_id): + try: + get_waiter(connection, 'network_interface_available').wait( + NetworkInterfaceIds=[eni_id], + WaiterConfig={'Delay': 5, 'MaxAttempts': 80}, + ) + except botocore.exceptions.WaiterError as e: + module.fail_json_aws(e, "Timeout waiting for ENI {0} to detach".format(eni_id)) + + def delete_eni(connection, module): eni = uniquely_find_eni(connection, module) @@ -655,10 +665,9 @@ def delete_eni(connection, module): connection.detach_network_interface( aws_retry=True, AttachmentId=eni["Attachment"]["AttachmentId"], - Force=True + Force=True, ) - # Wait to allow detachment to finish - get_waiter(connection, 'network_interface_available').wait(NetworkInterfaceIds=[eni_id]) + _wait_for_detach(connection, module, eni_id) connection.delete_network_interface(aws_retry=True, NetworkInterfaceId=eni_id) changed = True else: @@ -684,10 +693,7 @@ def detach_eni(connection, eni, module): AttachmentId=eni["Attachment"]["AttachmentId"], Force=force_detach, ) - get_waiter(connection, 'network_interface_available').wait( - NetworkInterfaceIds=[eni_id], - WaiterConfig={'Delay': 5, 'MaxAttempts': 80}, - ) + _wait_for_detach(connection, module, eni_id) return True return False diff --git a/tests/integration/targets/ec2_eni/tasks/test_attachment.yaml b/tests/integration/targets/ec2_eni/tasks/test_attachment.yaml index bb0e13362ca..a58af1dfe69 100644 --- a/tests/integration/targets/ec2_eni/tasks/test_attachment.yaml +++ b/tests/integration/targets/ec2_eni/tasks/test_attachment.yaml @@ -1,4 +1,12 @@ # ============================================================ +- name: Ensure test instances are running + ec2_instance: + state: running + instance_ids: + - "{{ instance_id_1 }}" + - "{{ instance_id_2 }}" + wait: True + - name: attach the network interface to instance 1 ec2_eni: instance_id: "{{ instance_id_1 }}" From 9b021233e9b79b25105a89038590e3a807413f9c Mon Sep 17 00:00:00 2001 From: Mark Chappell Date: Mon, 4 Oct 2021 17:34:08 +0200 Subject: [PATCH 3/4] Fix ec2_eni integration tests - ensure VM state --- tests/integration/targets/ec2_eni/aliases | 3 +-- .../targets/ec2_eni/tasks/test_attachment.yaml | 12 ++++++++++-- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/tests/integration/targets/ec2_eni/aliases b/tests/integration/targets/ec2_eni/aliases index 4fc9a91e159..9adce456758 100644 --- a/tests/integration/targets/ec2_eni/aliases +++ b/tests/integration/targets/ec2_eni/aliases @@ -1,4 +1,3 @@ -unstable - cloud/aws + ec2_eni_info diff --git a/tests/integration/targets/ec2_eni/tasks/test_attachment.yaml b/tests/integration/targets/ec2_eni/tasks/test_attachment.yaml index a58af1dfe69..333b9f538a8 100644 --- a/tests/integration/targets/ec2_eni/tasks/test_attachment.yaml +++ b/tests/integration/targets/ec2_eni/tasks/test_attachment.yaml @@ -1,7 +1,8 @@ # ============================================================ -- name: Ensure test instances are running +# If we don't stop the instances they can get stuck "detaching" +- name: Ensure test instances are stopped ec2_instance: - state: running + state: stopped instance_ids: - "{{ instance_id_1 }}" - "{{ instance_id_2 }}" @@ -174,6 +175,13 @@ - '"currently in use" in result.msg' # ============================================================ +- name: Ensure test instances is running (will block non-forced detachment) + ec2_instance: + state: running + instance_ids: + - "{{ instance_id_2 }}" + wait: True + - name: delete an attached network interface with force_detach ec2_eni: force_detach: True From f52424d70989d2b36ec8a9e8fbf795fa65d2187f Mon Sep 17 00:00:00 2001 From: Mark Chappell Date: Wed, 6 Oct 2021 13:07:16 +0200 Subject: [PATCH 4/4] fix import - too aggressive Co-authored-by: Alina Buzachis --- plugins/modules/ec2_eni.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/modules/ec2_eni.py b/plugins/modules/ec2_eni.py index b66346348e1..fe7b3981555 100644 --- a/plugins/modules/ec2_eni.py +++ b/plugins/modules/ec2_eni.py @@ -305,7 +305,7 @@ from ..module_utils.ec2 import get_ec2_security_group_ids_from_names from ..module_utils.tagging import boto3_tag_list_to_ansible_dict from ..module_utils.tagging import boto3_tag_specifications -from ..module_utils.tagging import ensure_ec2_tags +from ..module_utils.ec2 import ensure_ec2_tags from ..module_utils.waiters import get_waiter