Fix IOPs io1 DB instance updates and integration tests also (ansible-…

…collections#878) Fix IOPs io1 DB instance updates and integration tests also SUMMARY Primary this PR is to fix updates when updating iops or allocated_storage on io1 DB instances when only one param is changing. Secondarily this fixes up the tests again and is test against some improvements to the waiter configuration see linked PR. IOPs error on update attempts if only one param is being updated: error: code: InvalidParameterCombination message: You must specify both the storage size and iops when modifying the storage size or iops on a DB instance that has iops. type: Sender msg: 'Unable to modify DB instance: An error occurred (InvalidParameterCombination) when calling the ModifyDBInstance operation: You must specify both the storage size and iops when modifying the storage size or iops on a DB instance that has iops.' ISSUE TYPE Bugfix Pull Request COMPONENT NAME rds_instance ADDITIONAL INFORMATION These tests are very slow and still a little flakey but generally all pass as expected now locally. Reviewed-by: Mark Woolley <[email protected]> Reviewed-by: Markus Bergholz <[email protected]> Reviewed-by: Alina Buzachis <None> This commit was initially merged in https://github.com/ansible-collections/community.aws See: ansible-collections/community.aws@45e79ed
alinabuzachis · Sep 21, 2022 · 8121323 · 8121323
1 parent 40d8808
commit 8121323
Show file tree

Hide file tree

Showing 13 changed files with 163 additions and 55 deletions.
diff --git a/plugins/modules/rds_instance.py b/plugins/modules/rds_instance.py
@@ -467,10 +467,15 @@
 
 RETURN = r'''
 allocated_storage:
-  description: The allocated storage size in gibibytes. This is always 1 for aurora database engines.
+  description: The allocated storage size in gigabytes. This is always 1 for aurora database engines.
   returned: always
   type: int
   sample: 20
+associated_roles:
+  description: The list of currently associated roles.
+  returned: always
+  type: list
+  sample: []
 auto_minor_version_upgrade:
   description: Whether minor engine upgrades are applied automatically to the DB instance during the maintenance window.
   returned: always
@@ -890,6 +895,17 @@ def get_options_with_changing_values(client, module, parameters):
     updated_parameters.update(get_changing_options_with_consistent_keys(parameters, instance))
     parameters = updated_parameters
 
+    if instance.get('StorageType') == 'io1':
+        # Bundle Iops and AllocatedStorage while updating io1 RDS Instance
+        current_iops = instance.get('PendingModifiedValues', {}).get('Iops', instance['Iops'])
+        current_allocated_storage = instance.get('PendingModifiedValues', {}).get('AllocatedStorage', instance['AllocatedStorage'])
+        new_iops = module.params.get('iops')
+        new_allocated_storage = module.params.get('allocated_storage')
+
+        if current_iops != new_iops or current_allocated_storage != new_allocated_storage:
+            parameters['AllocatedStorage'] = new_allocated_storage
+            parameters['Iops'] = new_iops
+
     if parameters.get('NewDBInstanceIdentifier') and instance.get('PendingModifiedValues', {}).get('DBInstanceIdentifier'):
         if parameters['NewDBInstanceIdentifier'] == instance['PendingModifiedValues']['DBInstanceIdentifier'] and not apply_immediately:
             parameters.pop('NewDBInstanceIdentifier')
@@ -1179,6 +1195,7 @@ def main():
         ('engine', 'aurora', ('db_cluster_identifier',)),
         ('engine', 'aurora-mysql', ('db_cluster_identifier',)),
         ('engine', 'aurora-postresql', ('db_cluster_identifier',)),
+        ('storage_type', 'io1', ('iops', 'allocated_storage')),
         ('creation_source', 'snapshot', ('snapshot_identifier', 'engine')),
         ('creation_source', 's3', (
             's3_bucket_name', 'engine', 'master_username', 'master_user_password',

diff --git a/tests/integration/targets/rds_instance/aliases b/tests/integration/targets/rds_instance/aliases
@@ -1,4 +1,3 @@
-disabled # something is broken
 slow
 
 cloud/aws
diff --git a/tests/integration/targets/rds_instance/inventory b/tests/integration/targets/rds_instance/inventory
@@ -1,12 +1,19 @@
+# inventory names shortened down to fit resource name length limits
 [tests]
+# processor feature tests
+processor
+# restore instance tests
+restore
+# security groups db tests
+sgroups
+# modify complex tests
+complex
+# other tests
 states
 modify
-modify_complex
-processor_features
-read_replica
-vpc_security_groups
-restore_instance
 tagging
+replica
+upgrade
 
 # TODO: uncomment after adding rds_cluster module
 # aurora

diff --git a/tests/integration/targets/rds_instance/main.yml b/tests/integration/targets/rds_instance/main.yml
@@ -6,6 +6,6 @@
 - hosts: all
   gather_facts: no
   strategy: free
-  serial: 8
+  serial: 9
   roles:
     - rds_instance
diff --git a/tests/integration/targets/rds_instance/roles/rds_instance/defaults/main.yml b/tests/integration/targets/rds_instance/roles/rds_instance/defaults/main.yml
@@ -1,5 +1,5 @@
 ---
-instance_id: "ansible-test-{{ tiny_prefix }}"
+instance_id: "ansible-test-{{ inventory_hostname | replace('_','-') }}{{ tiny_prefix }}"
 modified_instance_id: "{{ instance_id }}-updated"
 username: test
 password: test12345678
@@ -8,8 +8,12 @@ storage_encrypted_db_instance_class: db.t3.small
 modified_db_instance_class: db.t3.medium
 allocated_storage: 20
 modified_allocated_storage: 30
+io1_allocated_storage: 100
+io1_modified_allocated_storage: 110
 monitoring_interval: 60
 preferred_maintenance_window: "mon:06:20-mon:06:50"
+storage_type: io1
+iops: 1000
 
 # For aurora tests
 cluster_id: "{{ instance_id }}-cluster"

diff --git a/...ds_instance/tasks/test_modify_complex.yml → ...roles/rds_instance/tasks/test_complex.yml b/...ds_instance/tasks/test_modify_complex.yml → ...roles/rds_instance/tasks/test_complex.yml
@@ -31,7 +31,9 @@
         username: "{{ username }}"
         password: "{{ password }}"
         db_instance_class: "{{ db_instance_class }}"
-        allocated_storage: "{{ allocated_storage }}"
+        allocated_storage: "{{ io1_allocated_storage }}"
+        storage_type: "{{ storage_type }}"
+        iops: "{{ iops }}"
       register: result
 
     - assert:
@@ -48,47 +50,46 @@
       rds_instance:
         id: "{{ instance_id }}"
         state: present
-        allocated_storage: 30
+        allocated_storage: "{{ io1_modified_allocated_storage }}"
+        storage_type: "{{ storage_type }}"
         db_instance_class: "{{ modified_db_instance_class }}"
         backup_retention_period: 2
         preferred_backup_window: "05:00-06:00"
         preferred_maintenance_window: "{{ preferred_maintenance_window }}"
-        engine_version: "{{ mariadb_engine_version_2 }}"
-        allow_major_version_upgrade: true
         auto_minor_version_upgrade: false
         monitoring_interval: "{{ monitoring_interval }}"
         monitoring_role_arn: "{{ enhanced_monitoring_role.arn }}"
+        iops: "{{ iops }}"
         port: 1150
-        max_allocated_storage: 100
+        max_allocated_storage: 150
         apply_immediately: True
       register: result
 
     - assert:
         that:
           - result.changed
-          - '"allocated_storage" in result.pending_modified_values or result.allocated_storage == 30'
-          - '"max_allocated_storage" in result.pending_modified_values or result.max_allocated_storage == 100'
+          - '"allocated_storage" in result.pending_modified_values or result.allocated_storage == io1_modified_allocated_storage'
+          - '"max_allocated_storage" in result.pending_modified_values or result.max_allocated_storage == 150'
           - '"port" in result.pending_modified_values or result.endpoint.port == 1150'
           - '"db_instance_class" in result.pending_modified_values or result.db_instance_class == modified_db_instance_class'
-          - '"engine_version" in result.pending_modified_values or result.engine_version == mariadb_engine_version_2'
           - '"monitoring_interval" in result.pending_modified_values or result.monitoring_interval == monitoring_interval'
 
     - name: Idempotence modifying several pending attributes
       rds_instance:
         id: "{{ instance_id }}"
         state: present
-        allocated_storage: 30
+        allocated_storage: "{{ io1_modified_allocated_storage }}"
+        storage_type: "{{ storage_type }}"
         db_instance_class: "{{ modified_db_instance_class }}"
         backup_retention_period: 2
         preferred_backup_window: "05:00-06:00"
         preferred_maintenance_window: "{{ preferred_maintenance_window }}"
-        engine_version: "{{ mariadb_engine_version_2 }}"
-        allow_major_version_upgrade: true
         auto_minor_version_upgrade: false
         monitoring_interval: "{{ monitoring_interval }}"
         monitoring_role_arn: "{{ enhanced_monitoring_role.arn }}"
+        iops: "{{ iops }}"
         port: 1150
-        max_allocated_storage: 100
+        max_allocated_storage: 150
       register: result
       retries: 30
       delay: 10
@@ -97,11 +98,10 @@
     - assert:
         that:
           - not result.changed
-          - '"allocated_storage" in result.pending_modified_values or result.allocated_storage == 30'
-          - '"max_allocated_storage" in result.pending_modified_values or result.max_allocated_storage == 100'
+          - '"allocated_storage" in result.pending_modified_values or result.allocated_storage == io1_modified_allocated_storage'
+          - '"max_allocated_storage" in result.pending_modified_values or result.max_allocated_storage == 150'
           - '"port" in result.pending_modified_values or result.endpoint.port == 1150'
           - '"db_instance_class" in result.pending_modified_values or result.db_instance_class == modified_db_instance_class'
-          - '"engine_version" in result.pending_modified_values or result.engine_version == mariadb_engine_version_2'
 
   always:
     - name: Delete the instance

diff --git a/...nstance/tasks/test_processor_features.yml → ...les/rds_instance/tasks/test_processor.yml b/...nstance/tasks/test_processor_features.yml → ...les/rds_instance/tasks/test_processor.yml
diff --git a/.../rds_instance/tasks/test_read_replica.yml → ...roles/rds_instance/tasks/test_replica.yml b/.../rds_instance/tasks/test_read_replica.yml → ...roles/rds_instance/tasks/test_replica.yml
diff --git a/..._instance/tasks/test_restore_instance.yml → ...roles/rds_instance/tasks/test_restore.yml b/..._instance/tasks/test_restore_instance.yml → ...roles/rds_instance/tasks/test_restore.yml
diff --git a/...stance/tasks/test_vpc_security_groups.yml → ...roles/rds_instance/tasks/test_sgroups.yml b/...stance/tasks/test_vpc_security_groups.yml → ...roles/rds_instance/tasks/test_sgroups.yml
@@ -28,7 +28,6 @@
           - {"cidr": "10.122.122.128/28", "zone": "{{ aws_region }}a"}
           - {"cidr": "10.122.122.144/28", "zone": "{{ aws_region }}b"}
           - {"cidr": "10.122.122.160/28", "zone": "{{ aws_region }}c"}
-          - {"cidr": "10.122.122.176/28", "zone": "{{ aws_region }}d"}
 
       - name: Create security groups
         ec2_group:

diff --git a/tests/integration/targets/rds_instance/roles/rds_instance/tasks/test_states.yml b/tests/integration/targets/rds_instance/roles/rds_instance/tasks/test_states.yml
@@ -119,37 +119,7 @@
         that:
           - result.changed
 
-    # Test final snapshot on deletion
-    - name: Delete the DB instance
-      rds_instance:
-        id: "{{ instance_id }}"
-        state: absent
-        final_snapshot_identifier: "{{ instance_id }}"
-      register: result
-
-    - assert:
-        that:
-          - result.changed
-          - "result.final_snapshot.db_instance_identifier == '{{ instance_id }}'"
-
-    - name: Check that snapshot exists
-      rds_snapshot_info:
-        db_snapshot_identifier: "{{ instance_id }}"
-      register: result
-
-    - assert:
-        that:
-          - "result.snapshots | length == 1"
-          - "result.snapshots.0.engine == 'mariadb'"
-
   always:
-    - name: remove snapshot
-      rds_instance_snapshot:
-        db_snapshot_identifier: "{{ resource_prefix }}-test-snapshot"
-        state: absent
-        wait: false
-      ignore_errors: yes
-
     - name: Remove DB instance
       rds_instance:
         id: "{{ instance_id }}"

diff --git a/tests/integration/targets/rds_instance/roles/rds_instance/tasks/test_tagging.yml b/tests/integration/targets/rds_instance/roles/rds_instance/tasks/test_tagging.yml
@@ -119,7 +119,37 @@
           - "result.tags | length == 2"
           - "result.tags.Name == '{{ instance_id }}-new'"
 
+    # Test final snapshot on deletion
+    - name: Delete the DB instance
+      rds_instance:
+        id: "{{ instance_id }}"
+        state: absent
+        final_snapshot_identifier: "{{ instance_id }}"
+      register: result
+
+    - assert:
+        that:
+          - result.changed
+          - "result.final_snapshot.db_instance_identifier == '{{ instance_id }}'"
+
+    - name: Check that snapshot exists
+      rds_snapshot_info:
+        db_snapshot_identifier: "{{ instance_id }}"
+      register: result
+
+    - assert:
+        that:
+          - "result.snapshots | length == 1"
+          - "result.snapshots.0.engine == 'mariadb'"
+
   always:
+    - name: remove snapshot
+      rds_instance_snapshot:
+        db_snapshot_identifier: "{{ tiny_prefix }}-test-snapshot"
+        state: absent
+        wait: false
+      ignore_errors: yes
+
     - name: Remove DB instance
       rds_instance:
         id: "{{ instance_id }}"

diff --git a/tests/integration/targets/rds_instance/roles/rds_instance/tasks/test_upgrade.yml b/tests/integration/targets/rds_instance/roles/rds_instance/tasks/test_upgrade.yml
@@ -0,0 +1,82 @@
+---
+- block:
+    - name: Ensure the resource doesn't exist
+      rds_instance:
+        id: "{{ instance_id }}"
+        state: absent
+        skip_final_snapshot: True
+      register: result
+
+    - assert:
+        that:
+          - not result.changed
+      ignore_errors: yes
+
+    - name: Create a mariadb instance
+      rds_instance:
+        id: "{{ instance_id }}"
+        state: present
+        engine: mariadb
+        engine_version: "{{ mariadb_engine_version }}"
+        allow_major_version_upgrade: true
+        username: "{{ username }}"
+        password: "{{ password }}"
+        db_instance_class: "{{ db_instance_class }}"
+        allocated_storage: "{{ allocated_storage }}"
+      register: result
+
+    - assert:
+        that:
+          - result.changed
+          - "result.db_instance_identifier == '{{ instance_id }}'"
+
+    # Test upgrade of DB instance
+
+    - name: Uprade a mariadb instance
+      rds_instance:
+        id: "{{ instance_id }}"
+        state: present
+        engine: mariadb
+        engine_version: "{{ mariadb_engine_version_2 }}"
+        allow_major_version_upgrade: true
+        username: "{{ username }}"
+        password: "{{ password }}"
+        db_instance_class: "{{ db_instance_class }}"
+        allocated_storage: "{{ allocated_storage }}"
+        apply_immediately: True
+      register: result
+
+    - assert:
+        that:
+          - result.changed
+          - '"engine_version" in result.pending_modified_values or result.engine_version == mariadb_engine_version_2'
+
+    - name: Idempotence uprading a mariadb instance
+      rds_instance:
+        id: "{{ instance_id }}"
+        state: present
+        engine: mariadb
+        engine_version: "{{ mariadb_engine_version_2 }}"
+        allow_major_version_upgrade: true
+        username: "{{ username }}"
+        password: "{{ password }}"
+        db_instance_class: "{{ db_instance_class }}"
+        allocated_storage: "{{ allocated_storage }}"
+      register: result
+      retries: 30
+      delay: 10
+      until: result is not failed
+
+    - assert:
+        that:
+          - not result.changed
+          - '"engine_version" in result.pending_modified_values or result.engine_version == mariadb_engine_version_2'
+
+  always:
+    - name: Delete the instance
+      rds_instance:
+        id: "{{ instance_id }}"
+        state: absent
+        skip_final_snapshot: True
+        wait: false
+      ignore_errors: yes