ec2_vol: Fix incorrectly returned changed result (#483)

ec2_vol: Fix incorrectly returned changed result

SUMMARY
When modify_volume is used but no new disk is being attached, the module incorrectly reports that no change has occurred even when disks have been modified (iops, throughput, type, etc.). This is due to the attach function overwriting the changed variable even if no disks are attached.
Fixes #482
The integration test has been fixed so that when the gp3 modifications are tested, the volume is already in an attached state (previously detached) when reporting back changed. The detach tests are moved further down now, allowing this case to be properly covered by the existing assert:

  
    
      amazon.aws/tests/integration/targets/ec2_vol/tasks/tests.yml
    
    
        Lines 384 to 387
      in
      e8df917
    
    
    
    

        
          
           - name: check that volume_type has changed 
        

        
          
             assert: 
        

        
          
               that: 
        

        
          
                 - changed_gp3_volume.changed 
        
    
  


ISSUE TYPE


Bugfix Pull Request

COMPONENT NAME
ec2_vol

Reviewed-by: Alina Buzachis <None>
Reviewed-by: None <None>
Reviewed-by: Mark Chappell <None>
Reviewed-by: None <None>

changelogs/fragments/483-ec2_vol_fix_returned_changed_var.yml

@@ -0,0 +1,6 @@
+bugfixes:
+- >-
+  ec2_vol - Fixes ``changed`` status when ``modify_volume`` is used, but no new 
+  disk is being attached.  The module incorrectly reported that no change had 
+  occurred even when disks had been modified (iops, throughput, type, etc.).
+  (https://github.com/ansible-collections/amazon.aws/issues/482).

plugins/modules/ec2_vol.py

@@ -821,12 +821,12 @@ def main():
         if detach_vol_flag:
             volume, changed = detach_volume(module, ec2_conn, volume_dict=volume)
         elif inst is not None:
-            volume, changed = attach_volume(module, ec2_conn, volume_dict=volume, instance_dict=inst, device_name=device_name)
+            volume, vol_attached = attach_volume(module, ec2_conn, volume_dict=volume, instance_dict=inst, device_name=device_name)
 
         # Add device, volume_id and volume_type parameters separately to maintain backward compatibility
         volume_info = get_volume_info(module, volume, tags=final_tags)
 
-        if tags_changed:
+        if tags_changed or vol_attached:
             changed = True
 
         module.exit_json(changed=changed, volume=volume_info, device=device_name,

tests/integration/targets/ec2_vol/tasks/tests.yml

@@ -338,29 +338,6 @@
         that:
           - ec2_vol_info.volumes | length == 4
 
-    - name: detach volume from the instance
-      ec2_vol:
-        id: "{{ new_vol_attach_result.volume_id }}"
-        instance: ""
-      register: new_vol_attach_result
-
-    - name: check task return attributes
-      assert:
-        that:
-          - new_vol_attach_result.changed
-          - new_vol_attach_result.volume.status == 'available'
-
-    - name: detach volume from the instance (idempotent)
-      ec2_vol:
-        id: "{{ new_vol_attach_result.volume_id }}"
-        instance: ""
-      register: new_vol_attach_result_idem
-
-    - name: check task return attributes
-      assert:
-        that:
-          - not new_vol_attach_result_idem.changed
-
     - name: must not change because of missing parameter modify_volume
       ec2_vol:
         id: "{{ new_vol_attach_result.volume_id }}"
@@ -454,6 +431,29 @@
       vars:
         v: "{{ verify_gp3_change.volumes[0] }}"
 
+    - name: detach volume from the instance
+      ec2_vol:
+        id: "{{ new_vol_attach_result.volume_id }}"
+        instance: ""
+      register: new_vol_attach_result
+
+    - name: check task return attributes
+      assert:
+        that:
+          - new_vol_attach_result.changed
+          - new_vol_attach_result.volume.status == 'available'
+
+    - name: detach volume from the instance (idempotent)
+      ec2_vol:
+        id: "{{ new_vol_attach_result.volume_id }}"
+        instance: ""
+      register: new_vol_attach_result_idem
+
+    - name: check task return attributes
+      assert:
+        that:
+          - not new_vol_attach_result_idem.changed
+
     - name: delete volume
       ec2_vol:
         id: "{{ volume2.volume_id }}"