From 7c68d320a789fcd6dcc6e5be24d252687b23e553 Mon Sep 17 00:00:00 2001 From: Alina Buzachis Date: Thu, 21 Jan 2021 12:01:54 +0100 Subject: [PATCH] add tests - idempotency issue when recreating gateway with tags for update --- plugins/modules/ec2_vpc_nat_gateway.py | 117 +++++++++++------- .../ec2_vpc_nat_gateway/tasks/main.yml | 14 ++- 2 files changed, 85 insertions(+), 46 deletions(-) diff --git a/plugins/modules/ec2_vpc_nat_gateway.py b/plugins/modules/ec2_vpc_nat_gateway.py index ee93218d572..733a4c609c8 100644 --- a/plugins/modules/ec2_vpc_nat_gateway.py +++ b/plugins/modules/ec2_vpc_nat_gateway.py @@ -460,6 +460,7 @@ def wait_for_status(client, wait_timeout, nat_gateway_id, status, ######################################### +@q def gateway_in_subnet_exists(client, module, subnet_id, tags, purge_tags, allocation_id=None, check_mode=False): """Retrieve all NAT Gateways for a subnet. @@ -501,7 +502,7 @@ def gateway_in_subnet_exists(client, module, subnet_id, tags, purge_tags, alloca Tuple (list, bool) """ allocation_id_exists = False - changed = False + update_tags = False gateways = [] states = ['available', 'pending'] @@ -515,20 +516,25 @@ def gateway_in_subnet_exists(client, module, subnet_id, tags, purge_tags, alloca ) ) if not gws_retrieved: - return gateways, allocation_id_exists + return gateways, allocation_id_exists, changed for gw in gws: for address in gw['nat_gateway_addresses']: if allocation_id: if address.get('allocation_id') == allocation_id: + ################################################### - gw['tags'], changed = ensure_tags(client, module, gw['nat_gateway_id'], tags, purge_tags, check_mode) + gw['tags'], update_tags = ensure_tags(client, module, gw['nat_gateway_id'], tags, purge_tags, check_mode) ################################################### allocation_id_exists = True gateways.append(gw) else: + q("ELSE") gateways.append(gw) - return gateways, allocation_id_exists, changed + + q("gateways") + q(gateways) + return gateways, allocation_id_exists, update_tags def get_eip_allocation_id_by_address(client, eip_address, check_mode=False): @@ -671,7 +677,8 @@ def release_address(client, allocation_id, check_mode=False): #################################### -def create(client, subnet_id, allocation_id, client_token=None, +@q +def create(client, module, subnet_id, allocation_id, tags, purge_tags, client_token=None, wait=False, wait_timeout=0, if_exist_do_not_create=False, check_mode=False): """Create an Amazon NAT Gateway. @@ -735,7 +742,7 @@ def create(client, subnet_id, allocation_id, client_token=None, params['ClientToken'] = client_token ################################################### - # check_input_tags(tags) + check_input_tags(tags) ################################################### try: @@ -751,10 +758,6 @@ def create(client, subnet_id, allocation_id, client_token=None, changed = True create_time = result['create_time'].replace(tzinfo=None) - ################################################### - # result['tags'], changed = ensure_tags(client, nat_gw_id=result['nat_gateway_id'], tags=tags, purge_tags=purge_tags, check_mode=check_mode) - ################################################## - if token_provided and (request_time > create_time): changed = False elif wait: @@ -769,6 +772,10 @@ def create(client, subnet_id, allocation_id, client_token=None, 'NAT gateway {0} created'.format(result['nat_gateway_id']) ) + ################################################### + result['tags'], _ = ensure_tags(client, module, nat_gw_id=result['nat_gateway_id'], tags=tags, purge_tags=purge_tags, check_mode=check_mode) + ################################################## + except is_boto3_error_code('IdempotentParameterMismatch'): err_msg = ( 'NAT Gateway does not support update and token has already been provided: ' + err_msg @@ -781,10 +788,13 @@ def create(client, subnet_id, allocation_id, client_token=None, success = False changed = False result = None - + + q("GW CREATED") + q(result) return success, changed, err_msg, result ###################################### +@q def pre_create(client, module, subnet_id, tags, purge_tags, allocation_id=None, eip_address=None, if_exist_do_not_create=False, wait=False, wait_timeout=0, client_token=None, check_mode=False): @@ -842,14 +852,26 @@ def pre_create(client, module, subnet_id, tags, purge_tags, allocation_id=None, err_msg = "" results = list() + if not allocation_id and not eip_address: - existing_gateways, allocation_id_exists, changed = ( + existing_gateways, allocation_id_exists, tags_update_exists = ( gateway_in_subnet_exists(client, module, subnet_id, tags, purge_tags, check_mode=check_mode) ) - if len(existing_gateways) > 0 and if_exist_do_not_create: + results = existing_gateways[0] + if tags_update_exists: + success = True + changed = True + err_msg = ( + 'NAT gateway {0} already exists in subnet_id {1}. Tags update was successful' + .format( + existing_gateways[0]['nat_gateway_id'], subnet_id + ) + ) + return success, changed, err_msg, results + success = True - #changed = False + changed = False results = existing_gateways[0] err_msg = ( 'NAT Gateway {0} already exists in subnet_id {1}' @@ -876,16 +898,33 @@ def pre_create(client, module, subnet_id, tags, purge_tags, allocation_id=None, success = False changed = False return success, changed, err_msg, dict() - - existing_gateways, allocation_id_exists, changed = ( + + existing_gateways, allocation_id_exists, tags_update_exists = ( gateway_in_subnet_exists( client, module, subnet_id, tags, purge_tags, allocation_id, check_mode=check_mode ) ) + q("existing_gateways") + q(existing_gateways) + q("tags_update_exists") + q(tags_update_exists) + if len(existing_gateways) > 0 and (allocation_id_exists or if_exist_do_not_create): - success = True - #changed = False results = existing_gateways[0] + if tags_update_exists: + success = True + changed = True + err_msg = ( + 'NAT gateway {0} already exists in subnet_id {1}. Tags update was successful' + .format( + existing_gateways[0]['nat_gateway_id'], subnet_id + ) + ) + return success, changed, err_msg, results + + success = True + changed = False + err_msg = ( 'NAT Gateway {0} already exists in subnet_id {1}' .format( @@ -895,7 +934,7 @@ def pre_create(client, module, subnet_id, tags, purge_tags, allocation_id=None, return success, changed, err_msg, results success, changed, err_msg, results = create( - client, subnet_id, allocation_id, client_token, + client, module, subnet_id, allocation_id, tags, purge_tags, client_token, wait, wait_timeout, if_exist_do_not_create, check_mode=check_mode ) @@ -1011,44 +1050,36 @@ def check_input_tags(tags): err_msg = ( 'One or more tags contain non-string values: {0}'.format(nonstring_tags) ) -################################################################# ################################################################# @q def ensure_tags(client, module, nat_gw_id, tags, purge_tags, check_mode): final_tags = [] results = False - q("Ensure Tags") - filters = ansible_dict_to_boto3_filter_list({'resource-id': nat_gw_id, 'resource-type': 'nat-gateway'}) + filters = ansible_dict_to_boto3_filter_list({'resource-id': nat_gw_id, 'resource-type': 'natgateway'}) cur_tags = None try: - q('try') - q('filters are: ', filters) - q("foo?") + + q('filters: ', filters) cur_tags = client.describe_tags(aws_retry=True, Filters=filters) - cur_tags = AWSRetry.jittered_backoff()(client.describe_tags)(Filters=filters) - q('bar?') - q(cur_tags) - q('done trying') + q("cur_tags", cur_tags) except (botocore.exceptions.ClientError, botocore.exceptions.BotoCoreError) as e: - #err_msg = "Couldn't describe tags: {}".format(str(e)) module.fail_json_aws(e, 'Couldnt describe tags') - q("Tags", tags) - if tags is None: - q('why are we here if not done?') - return boto3_tag_list_to_ansible_dict(cur_tags['Tags']), results #boto2_tag_list_to_ansible_dict(cur_tags.get('Tags')) + + q("tags", tags) - q(cur_tags) + if tags is None: + q("Tags are none") + q("results", results) + return boto3_tag_list_to_ansible_dict(cur_tags['Tags']), results to_update, to_delete = compare_aws_tags(boto3_tag_list_to_ansible_dict(cur_tags.get('Tags')), tags, purge_tags) final_tags = boto3_tag_list_to_ansible_dict(cur_tags.get('Tags')) - - q("Tag control") - q("To update:", to_update) + + q("to_update:", to_update) q("to_delete:", to_delete) - q("changed", results) if to_update: try: @@ -1064,7 +1095,7 @@ def ensure_tags(client, module, nat_gw_id, tags, purge_tags, check_mode): results = True except (botocore.exceptions.ClientError, botocore.exceptions.BotoCoreError) as e: - err_msg = "Couldn't create tags: {}".format(str(e)) + module.fail_json_aws(e, "Couldn't create tags") if to_delete: try: @@ -1081,16 +1112,14 @@ def ensure_tags(client, module, nat_gw_id, tags, purge_tags, check_mode): results = True except (botocore.exceptions.ClientError, botocore.exceptions.BotoCoreError) as e: - err_msg = "Couldn't delete tags: {}".format(str(e)) + module.fail_json_aws(e,"Couldn't delete tags") if not check_mode and (to_update or to_delete): try: response = client.describe_tags(aws_retry=True, Filters=filters) final_tags = boto3_tag_list_to_ansible_dict(response.get('Tags')) except (botocore.exceptions.ClientError, botocore.exceptions.BotoCoreError) as e: - err_msg = "Couldn't describe tags: {}".format(str(e)) - - q("changed", results) + module.fail_json_aws(e,"Couldn't describe tags") return final_tags, results ################################################################# diff --git a/tests/integration/targets/ec2_vpc_nat_gateway/tasks/main.yml b/tests/integration/targets/ec2_vpc_nat_gateway/tasks/main.yml index a673e0095cb..1349e1eb2f5 100644 --- a/tests/integration/targets/ec2_vpc_nat_gateway/tasks/main.yml +++ b/tests/integration/targets/ec2_vpc_nat_gateway/tasks/main.yml @@ -139,7 +139,7 @@ register: nat_gateway check_mode: yes - - name: Assert tag update would happen (expected changed=true) - CHECK_MODE + - name: Assert tag update would happen (expected changed=true) - check_mode assert: that: - nat_gateway.changed @@ -157,7 +157,7 @@ register: nat_gateway check_mode: yes - - name: Assert tags would be added - CHECK_MODE + - name: Assert tags would be added - check_mode assert: that: - nat_gateway.changed @@ -175,3 +175,13 @@ assert: that: - nat_gateway.changed + + # ============================================================ + always: + - name: tidy up nat gateway + ec2_vpc_nat_gateway: + nat_gateway_id: "{{ nat_gateway.nat_gateway_id }}" + state: absent + wait: yes + check_mode: yes + ignore_errors: true