* Address reviewer comments

Signed-off-by: Alina Buzachis <[email protected]>
ansible-collections · Mar 9, 2021 · 851b6c9 · 851b6c9
1 parent 75b7abf
commit 851b6c9
Show file tree

Hide file tree

Showing 2 changed files with 85 additions and 63 deletions.
diff --git a/plugins/modules/ec2_vpc_nat_gateway.py b/plugins/modules/ec2_vpc_nat_gateway.py
@@ -498,7 +498,7 @@ def get_eip_allocation_id_by_address(client, eip_address, check_mode=False):
     err_msg = ""
 
     try:
-        allocations = client.describe_addresses(**params)['Addresses']
+        allocations = client.describe_addresses(aws_retry=True, **params)['Addresses']
         if len(allocations) == 1:
             allocation = allocations[0]
         else:
@@ -553,7 +553,7 @@ def allocate_eip_address(client, check_mode=False):
         return ip_allocated, err_msg, new_eip
 
     try:
-        new_eip = client.allocate_address(**params)['AllocationId']
+        new_eip = client.allocate_address(aws_retry=True, **params)['AllocationId']
         ip_allocated = True
         err_msg = 'eipalloc id {0} created'.format(new_eip)
 
@@ -768,7 +768,7 @@ def pre_create(client, module, subnet_id, tags, purge_tags, allocation_id=None,
     success = False
     changed = False
     err_msg = ""
-    results = list()
+    results = {}
 
     if not allocation_id and not eip_address:
         existing_gateways, allocation_id_exists = (gateway_in_subnet_exists(client, subnet_id, check_mode=check_mode))
@@ -951,7 +951,7 @@ def ensure_tags(client, module, nat_gw_id, tags, purge_tags, check_mode):
     changed = False
 
     if check_mode and nat_gw_id is None:
-        # We can't describe tags without an option id, we might get here when creating a new option set in check_mode
+        # We can't describe tags without an EIP id, we might get here when creating a new EIP in check_mode
         return final_tags, changed
 
     filters = ansible_dict_to_boto3_filter_list({'resource-id': nat_gw_id, 'resource-type': 'natgateway'})

diff --git a/tests/integration/targets/ec2_vpc_nat_gateway/tasks/main.yml b/tests/integration/targets/ec2_vpc_nat_gateway/tasks/main.yml
@@ -10,6 +10,7 @@
     - amazon.aws
 
   block:
+
   # ============================================================
   - name: Create a VPC
     ec2_vpc_net:
@@ -22,6 +23,14 @@
     assert:
       that:
           - vpc_result is successful
+          - '"vpc" in vpc_result'
+          - '"cidr_block" in vpc_result.vpc'
+          - vpc_result.vpc.cidr_block == vpc_cidr
+          - '"id" in vpc_result.vpc'
+          - vpc_result.vpc.id.startswith("vpc-")
+          - '"state" in vpc_result.vpc'
+          - vpc_result.vpc.state == 'available'
+          - '"tags" in vpc_result.vpc'
 
   - name: "set fact: VPC ID"
     set_fact:
@@ -40,7 +49,9 @@
     assert:
       that:
         - eip_result is successful
+        - '"allocation_id" in eip_result'
         - 'eip_result.allocation_id.startswith("eipalloc-")'
+        - '"public_ip" in eip_result'
 
   - name: "set fact: EIP allocation ID and EIP public IP"
     set_fact:
@@ -60,9 +71,14 @@
     assert:
       that:
         - subnet_result is successful
-        - subnet_result.subnet.id.startswith("subnet-")
+        - '"subnet" in subnet_result'
+        - '"cidr_block" in subnet_result.subnet'
         - subnet_result.subnet.cidr_block == subnet_cidr
+        - '"id" in subnet_result.subnet'
+        - subnet_result.subnet.id.startswith("subnet-")
+        - '"state" in subnet_result.subnet'
         - subnet_result.subnet.state == 'available'
+        - '"tags" in subnet_result.subnet'
         - subnet_result.subnet.vpc_id == vpc_id
 
   - name: "set fact: VPC subnet ID"
@@ -185,7 +201,6 @@
     vars:
       first_ngw: '{{ avalaible_ngws.result[0] }}'
 
-
   # ============================================================
   - name: Trying this again for idempotency - create new NAT gateway with eip allocation-id - CHECK_MODE
     ec2_vpc_nat_gateway:
@@ -236,6 +251,57 @@
         - '"tags" in create_ngw'
         - '"vpc_id" in create_ngw'
         - create_ngw.vpc_id == vpc_id
+
+
+  # ============================================================
+  - name: Create new NAT gateway only if one does not exist already - CHECK_MODE
+    ec2_vpc_nat_gateway:
+      if_exist_do_not_create: yes
+      subnet_id: "{{ subnet_id }}"
+      wait: yes
+    register: create_ngw
+    check_mode: yes
+
+  - name: Assert recreation would do nothing (expected changed=false) - CHECK_MODE
+    assert:
+      that:
+        - not create_ngw.changed
+        - '"create_time" in create_ngw'
+        - '"nat_gateway_addresses" in create_ngw'
+        - '"nat_gateway_id" in create_ngw'
+        - create_ngw.nat_gateway_addresses[0].allocation_id == allocation_id
+        - create_ngw.nat_gateway_id.startswith("nat-")
+        - '"state" in create_ngw'
+        - create_ngw.state == 'available'
+        - '"subnet_id" in create_ngw'
+        - create_ngw.subnet_id == subnet_id
+        - '"tags" in create_ngw'
+        - '"vpc_id" in create_ngw'
+        - create_ngw.vpc_id == vpc_id
+
+  - name: Create new NAT gateway only if one does not exist already
+    ec2_vpc_nat_gateway:
+      if_exist_do_not_create: yes
+      subnet_id: "{{ subnet_id }}"
+      wait: yes
+    register: create_ngw
+
+  - name: Assert recreation would do nothing (expected changed=false)
+    assert:
+      that:
+        - not create_ngw.changed
+        - '"create_time" in create_ngw'
+        - '"nat_gateway_addresses" in create_ngw'
+        - '"nat_gateway_id" in create_ngw'
+        - create_ngw.nat_gateway_addresses[0].allocation_id == allocation_id
+        - create_ngw.nat_gateway_id.startswith("nat-")
+        - '"state" in create_ngw'
+        - create_ngw.state == 'available'
+        - '"subnet_id" in create_ngw'
+        - create_ngw.subnet_id == subnet_id
+        - '"tags" in create_ngw'
+        - '"vpc_id" in create_ngw'
+        - create_ngw.vpc_id == vpc_id
 
 
   # ============================================================
@@ -250,14 +316,15 @@
     assert:
       that:
         - eip_result is successful
+        - '"allocation_id" in eip_result'
         - 'eip_result.allocation_id.startswith("eipalloc-")'
+        - '"public_ip" in eip_result'
 
   - name: "set fact: EIP allocation ID and EIP public IP"
     set_fact:
       second_eip_address: "{{ eip_result.public_ip }}"
       second_allocation_id: "{{ eip_result.allocation_id }}"
 
-
   - name: Create new nat gateway with eip address - CHECK_MODE
     ec2_vpc_nat_gateway:
       subnet_id: "{{ subnet_id }}"
@@ -277,7 +344,7 @@
        eip_address: "{{ second_eip_address }}"
        wait: yes
     register: create_ngw
-  
+
   - name: Assert creation happened (expected changed=true)
     assert:
       that:
@@ -297,7 +364,7 @@
 
 
   # ============================================================
-  - name: Trying this again for idempotency - create new nat gateway with eip address - CHECK_MODE
+  - name: Trying this again for idempotency - create new NAT gateway with eip address - CHECK_MODE
     ec2_vpc_nat_gateway:
       subnet_id: "{{ subnet_id }}"
       eip_address: "{{ second_eip_address }}"
@@ -322,7 +389,7 @@
          - '"vpc_id" in create_ngw'
          - create_ngw.vpc_id == vpc_id
 
-  - name: Trying this again for idempotency -  create new nat gateway with eip address
+  - name: Trying this again for idempotency -  create new NAT gateway with eip address
     ec2_vpc_nat_gateway:
       subnet_id: "{{ subnet_id }}"
       eip_address: "{{ second_eip_address }}"
@@ -348,55 +415,6 @@
 
 
   # ============================================================
-  - name: Create new nat gateway only if one does not exist already - CHECK_MODE
-    ec2_vpc_nat_gateway:
-      if_exist_do_not_create: yes
-      subnet_id: "{{ subnet_id }}"
-      wait: yes
-    register: create_ngw
-    check_mode: yes
-
-  - name: Assert recreation would do nothing (expected changed=false) - CHECK_MODE
-    assert:
-      that:
-        - not create_ngw.changed
-        - '"create_time" in create_ngw'
-        - '"nat_gateway_addresses" in create_ngw'
-        - '"nat_gateway_id" in create_ngw'
-        - create_ngw.nat_gateway_addresses[0].allocation_id == second_allocation_id
-        - create_ngw.nat_gateway_id.startswith("nat-")
-        - '"state" in create_ngw'
-        - create_ngw.state == 'available'
-        - '"subnet_id" in create_ngw'
-        - create_ngw.subnet_id == subnet_id
-        - '"tags" in create_ngw'
-        - '"vpc_id" in create_ngw'
-        - create_ngw.vpc_id == vpc_id
-
-  - name: Create new NAT gateway only if one does not exist already
-    ec2_vpc_nat_gateway:
-      if_exist_do_not_create: yes
-      subnet_id: "{{ subnet_id }}"
-      wait: yes
-    register: create_ngw
-
-  - name: Assert recreation would do nothing (expected changed=false)
-    assert:
-      that:
-        - not create_ngw.changed
-        - '"create_time" in create_ngw'
-        - '"nat_gateway_addresses" in create_ngw'
-        - '"nat_gateway_id" in create_ngw'
-        - create_ngw.nat_gateway_addresses[0].allocation_id == second_allocation_id
-        - create_ngw.nat_gateway_id.startswith("nat-")
-        - '"state" in create_ngw'
-        - create_ngw.state == 'available'
-        - '"subnet_id" in create_ngw'
-        - create_ngw.subnet_id == subnet_id
-        - '"tags" in create_ngw'
-        - '"vpc_id" in create_ngw'
-        - create_ngw.vpc_id == vpc_id
-
   - name: Fetch NAT gateway by ID (list)
     ec2_vpc_nat_gateway_info:
       nat_gateway_ids:
@@ -566,10 +584,11 @@
 
 
   # ============================================================
-  - name: Gather information about a filtered list of NAT Gateways using tags - CHECK_MODE
+  - name: Gather information about a filtered list of NAT Gateways using tags and state - CHECK_MODE
     ec2_vpc_nat_gateway_info:
       filters:
         "tag:Tag Two": 'two {{ resource_prefix }}'
+        state: ['available']
     register: ngw_info
     check_mode: yes
 
@@ -597,10 +616,11 @@
     vars:
       second_ngw: '{{ ngw_info.result[0] }}'
 
-  - name: Gather information about a filtered list of NAT Gateways using tags
+  - name: Gather information about a filtered list of NAT Gateways using tags and state 
     ec2_vpc_nat_gateway_info:
       filters:
         "tag:Tag Two": 'two {{ resource_prefix }}'
+        state: ['available']
     register: ngw_info
 
   - name: Assert success
@@ -682,10 +702,11 @@
 
 
   # ============================================================
-  - name: Gather information about a filtered list of NAT Gateways using tags (no match) - CHECK_MODE
+  - name: Gather information about a filtered list of NAT Gateways using tags and state (no match) - CHECK_MODE
     ec2_vpc_nat_gateway_info:
       filters:
         "tag:tag_one": '{{ resource_prefix }} One'
+        state: ['available']
     register: ngw_info
     check_mode: yes
 
@@ -695,10 +716,11 @@
           - ngw_info is successful
           - ngw_info.result | length == 0
 
-  - name: Gather information about a filtered list of NAT Gateways using tags (no match)
+  - name: Gather information about a filtered list of NAT Gateways using tags and state (no match)
     ec2_vpc_nat_gateway_info:
       filters:
         "tag:tag_one": '{{ resource_prefix }} One'
+        state: ['available']
     register: ngw_info
 
   - name: Assert success