From 2aa3c3001179fc586e2c2249221b5dcf67b92f95 Mon Sep 17 00:00:00 2001 From: Mark Woolley Date: Fri, 4 Feb 2022 16:10:43 +0000 Subject: [PATCH] Fix IOPs io1 DB instance updates and integration tests also (#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 Reviewed-by: Markus Bergholz Reviewed-by: Alina Buzachis This commit was initially merged in https://github.com/ansible-collections/community.aws See: https://github.com/ansible-collections/community.aws/commit/45e79ed2e8a60d87b20f77cfbef4ba8a27da1260 --- plugins/modules/rds_instance.py | 19 ++++- .../integration/targets/rds_instance/aliases | 1 - .../targets/rds_instance/inventory | 17 ++-- .../integration/targets/rds_instance/main.yml | 2 +- .../roles/rds_instance/defaults/main.yml | 6 +- ...st_modify_complex.yml => test_complex.yml} | 30 +++---- ...cessor_features.yml => test_processor.yml} | 0 ...test_read_replica.yml => test_replica.yml} | 0 ..._restore_instance.yml => test_restore.yml} | 0 ...c_security_groups.yml => test_sgroups.yml} | 1 - .../roles/rds_instance/tasks/test_states.yml | 30 ------- .../roles/rds_instance/tasks/test_tagging.yml | 30 +++++++ .../roles/rds_instance/tasks/test_upgrade.yml | 82 +++++++++++++++++++ 13 files changed, 163 insertions(+), 55 deletions(-) rename tests/integration/targets/rds_instance/roles/rds_instance/tasks/{test_modify_complex.yml => test_complex.yml} (82%) rename tests/integration/targets/rds_instance/roles/rds_instance/tasks/{test_processor_features.yml => test_processor.yml} (100%) rename tests/integration/targets/rds_instance/roles/rds_instance/tasks/{test_read_replica.yml => test_replica.yml} (100%) rename tests/integration/targets/rds_instance/roles/rds_instance/tasks/{test_restore_instance.yml => test_restore.yml} (100%) rename tests/integration/targets/rds_instance/roles/rds_instance/tasks/{test_vpc_security_groups.yml => test_sgroups.yml} (98%) create mode 100644 tests/integration/targets/rds_instance/roles/rds_instance/tasks/test_upgrade.yml diff --git a/plugins/modules/rds_instance.py b/plugins/modules/rds_instance.py index 92d5e257cf0..742a7266c5e 100644 --- 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 index 67aa5c052e2..e30a1801b1e 100644 --- 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 index 9daf5db1e07..e0443d829d6 100644 --- 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 index 1b33dab5076..7d0dd4f8990 100644 --- 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 index 35d5dd7bdac..10f39d1ee22 100644 --- 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/tests/integration/targets/rds_instance/roles/rds_instance/tasks/test_modify_complex.yml b/tests/integration/targets/rds_instance/roles/rds_instance/tasks/test_complex.yml similarity index 82% rename from tests/integration/targets/rds_instance/roles/rds_instance/tasks/test_modify_complex.yml rename to tests/integration/targets/rds_instance/roles/rds_instance/tasks/test_complex.yml index 066d35c11d9..c9d8b3a4c5f 100644 --- a/tests/integration/targets/rds_instance/roles/rds_instance/tasks/test_modify_complex.yml +++ b/tests/integration/targets/rds_instance/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/tests/integration/targets/rds_instance/roles/rds_instance/tasks/test_processor_features.yml b/tests/integration/targets/rds_instance/roles/rds_instance/tasks/test_processor.yml similarity index 100% rename from tests/integration/targets/rds_instance/roles/rds_instance/tasks/test_processor_features.yml rename to tests/integration/targets/rds_instance/roles/rds_instance/tasks/test_processor.yml diff --git a/tests/integration/targets/rds_instance/roles/rds_instance/tasks/test_read_replica.yml b/tests/integration/targets/rds_instance/roles/rds_instance/tasks/test_replica.yml similarity index 100% rename from tests/integration/targets/rds_instance/roles/rds_instance/tasks/test_read_replica.yml rename to tests/integration/targets/rds_instance/roles/rds_instance/tasks/test_replica.yml diff --git a/tests/integration/targets/rds_instance/roles/rds_instance/tasks/test_restore_instance.yml b/tests/integration/targets/rds_instance/roles/rds_instance/tasks/test_restore.yml similarity index 100% rename from tests/integration/targets/rds_instance/roles/rds_instance/tasks/test_restore_instance.yml rename to tests/integration/targets/rds_instance/roles/rds_instance/tasks/test_restore.yml diff --git a/tests/integration/targets/rds_instance/roles/rds_instance/tasks/test_vpc_security_groups.yml b/tests/integration/targets/rds_instance/roles/rds_instance/tasks/test_sgroups.yml similarity index 98% rename from tests/integration/targets/rds_instance/roles/rds_instance/tasks/test_vpc_security_groups.yml rename to tests/integration/targets/rds_instance/roles/rds_instance/tasks/test_sgroups.yml index d797d965323..82e63578554 100644 --- a/tests/integration/targets/rds_instance/roles/rds_instance/tasks/test_vpc_security_groups.yml +++ b/tests/integration/targets/rds_instance/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 index dd13d55e164..c0d36b85943 100644 --- 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 index 0cdd9c1b7a5..bb84a63d95d 100644 --- 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 new file mode 100644 index 00000000000..90833bfd487 --- /dev/null +++ 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