From 5f80f023bff8e54bab20539c20a2092385abfdfd Mon Sep 17 00:00:00 2001 From: abikouo Date: Wed, 18 Sep 2024 10:49:55 +0200 Subject: [PATCH 1/8] Code completed --- ...stance-upgrade-downgrade-instance-type.yml | 3 + plugins/modules/ec2_instance.py | 129 ++++++++++++------ 2 files changed, 89 insertions(+), 43 deletions(-) create mode 100644 changelogs/fragments/20240917-ec2_instance-upgrade-downgrade-instance-type.yml diff --git a/changelogs/fragments/20240917-ec2_instance-upgrade-downgrade-instance-type.yml b/changelogs/fragments/20240917-ec2_instance-upgrade-downgrade-instance-type.yml new file mode 100644 index 00000000000..82b866d1bf9 --- /dev/null +++ b/changelogs/fragments/20240917-ec2_instance-upgrade-downgrade-instance-type.yml @@ -0,0 +1,3 @@ +--- +minor_changes: + - ec2_instance - add the possibility to upgrade / downgrade existing ec2 instance type (https://github.com/ansible-collections/amazon.aws/issues/469). \ No newline at end of file diff --git a/plugins/modules/ec2_instance.py b/plugins/modules/ec2_instance.py index bf6bd438306..dab9fd64a15 100644 --- a/plugins/modules/ec2_instance.py +++ b/plugins/modules/ec2_instance.py @@ -52,9 +52,10 @@ description: - Instance type to use for the instance, see U(https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/instance-types.html). - - Only required when instance is not already present. - At least one of O(instance_type) or O(launch_template) must be specificed when launching an instance. + - When the instance is present and the O(instance_type) specified value is different from the current value, + the instance will be stopped and the instance type will be updated. type: str count: description: @@ -1881,6 +1882,7 @@ def value_wrapper(v): param_mappings = [ ParamMapper("ebs_optimized", "EbsOptimized", "ebsOptimized", value_wrapper), ParamMapper("termination_protection", "DisableApiTermination", "disableApiTermination", value_wrapper), + ParamMapper("instance_type", "InstanceType", "instanceType", value_wrapper), # user data is an immutable property # ParamMapper('user_data', 'UserData', 'userData', value_wrapper), ] @@ -2327,6 +2329,43 @@ def determine_iam_role(module: AnsibleAWSModule, name_or_arn: Optional[str]) -> ) +def modify_instance_type( + client, + module: AnsibleAWSModule, + state: str, + instance_id: str, + changes: Dict[str, Dict[str, str]], +) -> None: + filters = { + "instance-id": [instance_id], + } + # Ensure that the instance is stopped before changing the instance type + ensure_instance_state(client, module, "stopped", filters) + + # force wait for the instance to be stopped + await_instances(client, module, ids=[instance_id], desired_module_state="stopped", force_wait=True) + + # Modify instance type + modify_instance_attribute(client, instance_id=instance_id, **changes) + + # Ensure instance state + desired_module_state = "running" if state == "present" else state + ensure_instance_state(client, module, desired_module_state, filters) + + +def modify_ec2_instance_attribute(client, module: AnsibleAWSModule, state: str, changes: List[Dict[str, Any]]) -> None: + if not module.check_mode: + for c in changes: + instance_id = c.pop("InstanceId") + try: + if "InstanceType" in c: + modify_instance_type(client, module, state, instance_id, c) + else: + modify_instance_attribute(client, instance_id=instance_id, **c) + except AnsibleEC2Error as e: + module.fail_json_aws(e, msg=f"Could not apply change {str(c)} to existing instance.") + + def handle_existing( client, module: AnsibleAWSModule, @@ -2356,13 +2395,9 @@ def handle_existing( changed |= change_instance_metadata_options(client, module, instance) changes = diff_instance_and_params(client, module, instance) - for c in changes: - if not module.check_mode: - try: - instance_id = c.pop("InstanceId") - modify_instance_attribute(client, instance_id=instance_id, **c) - except AnsibleEC2Error as e: - module.fail_json_aws(e, msg=f"Could not apply change {str(c)} to existing instance.") + # modify instance attributes + modify_ec2_instance_attribute(client, module, state, changes) + all_changes.extend(changes) changed |= bool(changes) changed |= add_or_update_instance_profile(client, module, instance, module.params.get("iam_instance_profile")) @@ -2394,10 +2429,12 @@ def enforce_count( current_count = len(existing_matches) if current_count == exact_count: - if desired_module_state != "present": + if desired_module_state not in ("absent", "terminated"): + results = handle_existing(client, module, existing_matches, desired_module_state, filters) + else: results = ensure_instance_state(client, module, desired_module_state, filters) - if results["changed"]: - return results + if results["changed"]: + return results return dict( changed=False, instances=[pretty_instance(i) for i in existing_matches], @@ -2407,43 +2444,52 @@ def enforce_count( if current_count < exact_count: # launch instances - return ensure_present( + results = ensure_present( client, module, existing_matches=existing_matches, desired_module_state=desired_module_state, current_count=current_count, ) + else: + # terminate instances + to_terminate = current_count - exact_count + # sort the instances from least recent to most recent based on launch time + existing_matches = sorted(existing_matches, key=lambda inst: inst["LaunchTime"]) + # get the instance ids of instances with the count tag on them + all_instance_ids = [x["InstanceId"] for x in existing_matches] + terminate_ids = all_instance_ids[0:to_terminate] + if module.check_mode: + return dict( + changed=True, + terminated_ids=terminate_ids, + instance_ids=all_instance_ids, + msg=f"Would have terminated following instances if not in check mode {terminate_ids}", + ) + # terminate instances + try: + terminate_instances(client, terminate_ids) + except AnsibleAWSError as e: + module.fail_json(e, msg="Unable to terminate instances") + await_instances(client, module, terminate_ids, desired_module_state="terminated", force_wait=True) - to_terminate = current_count - exact_count - # sort the instances from least recent to most recent based on launch time - existing_matches = sorted(existing_matches, key=lambda inst: inst["LaunchTime"]) - # get the instance ids of instances with the count tag on them - all_instance_ids = [x["InstanceId"] for x in existing_matches] - terminate_ids = all_instance_ids[0:to_terminate] - if module.check_mode: - return dict( + # include data for all matched instances in addition to the list of terminations + # allowing for recovery of metadata from the destructive operation + results = dict( changed=True, + msg="Successfully terminated instances.", terminated_ids=terminate_ids, instance_ids=all_instance_ids, - msg=f"Would have terminated following instances if not in check mode {terminate_ids}", + instances=existing_matches, ) - # terminate instances - try: - terminate_instances(client, terminate_ids) - except AnsibleAWSError as e: - module.fail_json(e, msg="Unable to terminate instances") - await_instances(client, module, terminate_ids, desired_module_state="terminated", force_wait=True) - - # include data for all matched instances in addition to the list of terminations - # allowing for recovery of metadata from the destructive operation - return dict( - changed=True, - msg="Successfully terminated instances.", - terminated_ids=terminate_ids, - instance_ids=all_instance_ids, - instances=existing_matches, - ) + + # Find instances + existing_matches = find_instances(client, module, filters=filters) + # Update instance attributes + updated_results = handle_existing(client, module, existing_matches, desired_module_state, filters) + if updated_results["changed"]: + results = updated_results + return results def ensure_present( @@ -2495,12 +2541,9 @@ def ensure_present( except AnsibleEC2Error as e: module.fail_json_aws(e, msg="Failed to fetch status of new EC2 instance") changes = diff_instance_and_params(client, module, ins, skip=["UserData", "EbsOptimized"]) - for c in changes: - try: - instance_id = c.pop("InstanceId") - modify_instance_attribute(client, instance_id=instance_id, **c) - except AnsibleEC2Error as e: - module.fail_json_aws(e, msg=f"Could not apply change {str(c)} to new instance.") + # modify instance attributes + modify_ec2_instance_attribute(client, module, desired_module_state, changes) + if existing_matches: # If we came from enforce_count, create a second list to distinguish # between existing and new instances when returning the entire cohort From 6335f1ad64185d303db8b25a20f18d4e91de0dbc Mon Sep 17 00:00:00 2001 From: abikouo Date: Wed, 18 Sep 2024 10:51:23 +0200 Subject: [PATCH 2/8] added basic integration tests --- .../targets/ec2_instance_type/aliases | 4 + .../ec2_instance_type/defaults/main.yml | 4 + .../targets/ec2_instance_type/meta/main.yml | 4 + .../targets/ec2_instance_type/tasks/main.yml | 82 +++++++++++++++++++ 4 files changed, 94 insertions(+) create mode 100644 tests/integration/targets/ec2_instance_type/aliases create mode 100644 tests/integration/targets/ec2_instance_type/defaults/main.yml create mode 100644 tests/integration/targets/ec2_instance_type/meta/main.yml create mode 100644 tests/integration/targets/ec2_instance_type/tasks/main.yml diff --git a/tests/integration/targets/ec2_instance_type/aliases b/tests/integration/targets/ec2_instance_type/aliases new file mode 100644 index 00000000000..b141f3fce20 --- /dev/null +++ b/tests/integration/targets/ec2_instance_type/aliases @@ -0,0 +1,4 @@ +time=7m +cloud/aws +ec2_instance_info +ec2_instance diff --git a/tests/integration/targets/ec2_instance_type/defaults/main.yml b/tests/integration/targets/ec2_instance_type/defaults/main.yml new file mode 100644 index 00000000000..5ecd9041a9d --- /dev/null +++ b/tests/integration/targets/ec2_instance_type/defaults/main.yml @@ -0,0 +1,4 @@ +--- +ec2_instance_type_initial: t2.micro +ec2_instance_type_updated: t3.nano +ec2_instance_name: "{{ resource_prefix }}-test-instance-type" diff --git a/tests/integration/targets/ec2_instance_type/meta/main.yml b/tests/integration/targets/ec2_instance_type/meta/main.yml new file mode 100644 index 00000000000..ffffe5089c3 --- /dev/null +++ b/tests/integration/targets/ec2_instance_type/meta/main.yml @@ -0,0 +1,4 @@ +--- +dependencies: + - role: setup_ec2_facts + - role: setup_ec2_instance_env diff --git a/tests/integration/targets/ec2_instance_type/tasks/main.yml b/tests/integration/targets/ec2_instance_type/tasks/main.yml new file mode 100644 index 00000000000..80e30ec84e8 --- /dev/null +++ b/tests/integration/targets/ec2_instance_type/tasks/main.yml @@ -0,0 +1,82 @@ +--- +- module_defaults: + group/aws: + access_key: "{{ aws_access_key }}" + secret_key: "{{ aws_secret_key }}" + session_token: "{{ security_token | default(omit) }}" + region: "{{ aws_region }}" + block: + - name: Make instance in the testing subnet created in the test VPC + amazon.aws.ec2_instance: + state: present + name: "{{ ec2_instance_name }}" + image_id: "{{ ec2_ami_id }}" + availability_zone: "{{ subnet_b_az }}" + instance_type: "{{ ec2_instance_type_initial }}" + wait: false + register: _instances + + - name: Define instance ids + ansible.builtin.set_fact: + ec2_instance_ids: "{{ _instances.instance_ids }}" + + - name: Update instance type (check mode) + amazon.aws.ec2_instance: + state: present + instance_ids: "{{ ec2_instance_ids }}" + instance_type: "{{ ec2_instance_type_updated }}" + wait: false + register: update_check_mode + check_mode: true + + - name: Gather ec2 instances info + amazon.aws.ec2_instance_info: + instance_ids: "{{ ec2_instance_ids }}" + register: _instances_info + + - name: Ensure module reported change while the instance type was not changed (check_mode) + ansible.builtin.assert: + that: + - update_check_mode is changed + - _instances_info.instances | map(attribute='instance_type') | unique | list == [ec2_instance_type_initial] + + - name: Update instance type + amazon.aws.ec2_instance: + state: present + instance_ids: "{{ ec2_instance_ids }}" + instance_type: "{{ ec2_instance_type_updated }}" + wait: false + register: _update_instance_type + + - name: Gather ec2 instances info + amazon.aws.ec2_instance_info: + instance_ids: "{{ ec2_instance_ids }}" + register: _instances_info + + - name: Ensure module reported change while the instance type was not changed (check_mode) + ansible.builtin.assert: + that: + - _update_instance_type is changed + - _instances_info.instances | map(attribute='instance_type') | unique | list == [ec2_instance_type_updated] + + - name: Update instance type (idempotency) + amazon.aws.ec2_instance: + state: present + instance_ids: "{{ ec2_instance_ids }}" + instance_type: "{{ ec2_instance_type_updated }}" + wait: false + register: _update_idempotency + + - name: Validate idempotency did not reported change + ansible.builtin.assert: + that: + - _update_idempotency is not changed + + always: + - name: Delete instance created for tests + amazon.aws.ec2_instance: + state: absent + instance_ids: "{{ ec2_instance_ids }}" + wait: false + ignore_errors: true + when: ec2_instance_ids is defined \ No newline at end of file From 7a809f0635ef8ef6aa6ce5b1b416755b8eade34b Mon Sep 17 00:00:00 2001 From: abikouo Date: Wed, 18 Sep 2024 14:58:04 +0200 Subject: [PATCH 3/8] add units tests --- .../ec2_instance/test_modify_instance_type.py | 84 +++++++++++++++++++ 1 file changed, 84 insertions(+) create mode 100644 tests/unit/plugins/modules/ec2_instance/test_modify_instance_type.py diff --git a/tests/unit/plugins/modules/ec2_instance/test_modify_instance_type.py b/tests/unit/plugins/modules/ec2_instance/test_modify_instance_type.py new file mode 100644 index 00000000000..8866331581a --- /dev/null +++ b/tests/unit/plugins/modules/ec2_instance/test_modify_instance_type.py @@ -0,0 +1,84 @@ +# (c) 2024 Red Hat Inc. +# +# This file is part of Ansible +# GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt) + +from unittest.mock import ANY +from unittest.mock import MagicMock +from unittest.mock import call +from unittest.mock import patch + +import pytest + +from ansible_collections.amazon.aws.plugins.modules import ec2_instance + +module_path = "ansible_collections.amazon.aws.plugins.modules.ec2_instance" + + +@pytest.fixture +def ansible_aws_module(): + module = MagicMock() + module.params = {} + module.check_mode = False + module.fail_json.side_effect = SystemExit(1) + module.fail_json_aws.side_effect = SystemExit(1) + return module + + +@pytest.mark.parametrize("state", ["present", "started", "running", "stopped", "restarted", "rebooted"]) +@patch(module_path + ".modify_instance_attribute") +@patch(module_path + ".await_instances") +@patch(module_path + ".ensure_instance_state") +def test_modify_instance_type( + m_ensure_instance_state, m_await_instances, m_modify_instance_attribute, ansible_aws_module, state +): + instance_id = MagicMock() + client = MagicMock() + state = "present" + changes = MagicMock() + desired_module_state = "running" if state == "present" else state + + ec2_instance.modify_instance_type(client, ansible_aws_module, state, instance_id, changes) + m_await_instances.assert_called_once_with( + client, ansible_aws_module, ids=[instance_id], desired_module_state="stopped", force_wait=True + ) + m_modify_instance_attribute.assert_called_once_with(client, instance_id=instance_id, **changes) + filters = {"instance-id": [instance_id]} + m_ensure_instance_state.assert_has_calls( + [ + call(client, ansible_aws_module, "stopped", filters), + call(client, ansible_aws_module, desired_module_state, filters), + ] + ) + + +@pytest.mark.parametrize("check_mode", [True, False]) +@pytest.mark.parametrize("attribute", ["InstanceType", "Ramdisk", "DisableApiTermination"]) +@patch(module_path + ".modify_instance_attribute") +@patch(module_path + ".modify_instance_type") +def test_modify_ec2_instance_attribute( + m_modify_instance_type, m_modify_instance_attribute, ansible_aws_module, check_mode, attribute +): + client = MagicMock() + state = MagicMock() + instance_id = MagicMock() + attribute_value = MagicMock() + + ansible_aws_module.check_mode = check_mode + + ec2_instance.modify_ec2_instance_attribute( + client, ansible_aws_module, state, [{"InstanceId": instance_id, attribute: attribute_value}] + ) + if check_mode: + m_modify_instance_type.assert_not_called() + m_modify_instance_attribute.assert_not_called() + elif "InstanceType" == attribute: + m_modify_instance_type.assert_called_once_with( + client, ansible_aws_module, state, instance_id, {attribute: attribute_value} + ) + m_modify_instance_attribute.assert_not_called() + else: + m_modify_instance_type.assert_not_called() + m_modify_instance_attribute.assert_called_once_with( + client, instance_id=instance_id, **{attribute: attribute_value} + ) From 46eab72db522cd5517c0e00b84cfda8b00d41fac Mon Sep 17 00:00:00 2001 From: abikouo Date: Wed, 18 Sep 2024 19:02:55 +0200 Subject: [PATCH 4/8] update ec2_instance_type tests --- .../targets/ec2_instance_type/aliases | 2 +- .../ec2_instance_type/defaults/main.yml | 1 - .../targets/ec2_instance_type/tasks/main.yml | 109 ++++++------------ .../tasks/single_instance.yml | 77 +++++++++++++ .../tasks/update_instance_type.yml | 77 +++++++++++++ 5 files changed, 190 insertions(+), 76 deletions(-) create mode 100644 tests/integration/targets/ec2_instance_type/tasks/single_instance.yml create mode 100644 tests/integration/targets/ec2_instance_type/tasks/update_instance_type.yml diff --git a/tests/integration/targets/ec2_instance_type/aliases b/tests/integration/targets/ec2_instance_type/aliases index b141f3fce20..efed61553cc 100644 --- a/tests/integration/targets/ec2_instance_type/aliases +++ b/tests/integration/targets/ec2_instance_type/aliases @@ -1,4 +1,4 @@ -time=7m +time=15m cloud/aws ec2_instance_info ec2_instance diff --git a/tests/integration/targets/ec2_instance_type/defaults/main.yml b/tests/integration/targets/ec2_instance_type/defaults/main.yml index 5ecd9041a9d..2c868c389fc 100644 --- a/tests/integration/targets/ec2_instance_type/defaults/main.yml +++ b/tests/integration/targets/ec2_instance_type/defaults/main.yml @@ -1,4 +1,3 @@ --- ec2_instance_type_initial: t2.micro ec2_instance_type_updated: t3.nano -ec2_instance_name: "{{ resource_prefix }}-test-instance-type" diff --git a/tests/integration/targets/ec2_instance_type/tasks/main.yml b/tests/integration/targets/ec2_instance_type/tasks/main.yml index 80e30ec84e8..70b3d585065 100644 --- a/tests/integration/targets/ec2_instance_type/tasks/main.yml +++ b/tests/integration/targets/ec2_instance_type/tasks/main.yml @@ -6,77 +6,38 @@ session_token: "{{ security_token | default(omit) }}" region: "{{ aws_region }}" block: - - name: Make instance in the testing subnet created in the test VPC - amazon.aws.ec2_instance: - state: present - name: "{{ ec2_instance_name }}" - image_id: "{{ ec2_ami_id }}" - availability_zone: "{{ subnet_b_az }}" - instance_type: "{{ ec2_instance_type_initial }}" - wait: false - register: _instances - - - name: Define instance ids - ansible.builtin.set_fact: - ec2_instance_ids: "{{ _instances.instance_ids }}" - - - name: Update instance type (check mode) - amazon.aws.ec2_instance: - state: present - instance_ids: "{{ ec2_instance_ids }}" - instance_type: "{{ ec2_instance_type_updated }}" - wait: false - register: update_check_mode - check_mode: true - - - name: Gather ec2 instances info - amazon.aws.ec2_instance_info: - instance_ids: "{{ ec2_instance_ids }}" - register: _instances_info - - - name: Ensure module reported change while the instance type was not changed (check_mode) - ansible.builtin.assert: - that: - - update_check_mode is changed - - _instances_info.instances | map(attribute='instance_type') | unique | list == [ec2_instance_type_initial] - - - name: Update instance type - amazon.aws.ec2_instance: - state: present - instance_ids: "{{ ec2_instance_ids }}" - instance_type: "{{ ec2_instance_type_updated }}" - wait: false - register: _update_instance_type - - - name: Gather ec2 instances info - amazon.aws.ec2_instance_info: - instance_ids: "{{ ec2_instance_ids }}" - register: _instances_info - - - name: Ensure module reported change while the instance type was not changed (check_mode) - ansible.builtin.assert: - that: - - _update_instance_type is changed - - _instances_info.instances | map(attribute='instance_type') | unique | list == [ec2_instance_type_updated] - - - name: Update instance type (idempotency) - amazon.aws.ec2_instance: - state: present - instance_ids: "{{ ec2_instance_ids }}" - instance_type: "{{ ec2_instance_type_updated }}" - wait: false - register: _update_idempotency - - - name: Validate idempotency did not reported change - ansible.builtin.assert: - that: - - _update_idempotency is not changed - - always: - - name: Delete instance created for tests - amazon.aws.ec2_instance: - state: absent - instance_ids: "{{ ec2_instance_ids }}" - wait: false - ignore_errors: true - when: ec2_instance_ids is defined \ No newline at end of file + - include_tasks: single_instance.yml + vars: + ec2_instance_name: "{{ resource_prefix }}-test-instance-type-single" + + - name: "Test update instance type using exact_count" + vars: + ec2_instance_name: "{{ resource_prefix }}-test-instance-type-multiple" + block: + - name: Create multiple ec2 instances + amazon.aws.ec2_instance: + state: present + name: "{{ ec2_instance_name }}" + image_id: "{{ ec2_ami_id }}" + subnet_id: "{{ testing_subnet_a.subnet.id }}" + instance_type: "{{ ec2_instance_type_initial }}" + wait: false + exact_count: 2 + + - name: Test upgrade instance type with various number of instances + include_tasks: update_instance_type.yml + with_items: + - new_instance_type: "{{ ec2_instance_type_updated }}" + new_instance_count: 2 + - new_instance_type: "{{ ec2_instance_type_initial }}" + new_instance_count: 3 + - new_instance_type: "{{ ec2_instance_type_updated }}" + new_instance_count: 2 + + always: + - name: Delete ec2 instances + amazon.aws.ec2_instance: + state: absent + instance_ids: "{{ _instances_info.instances | map(attribute='instance_id') | list }}" + wait: false + when: _instances_info is defined diff --git a/tests/integration/targets/ec2_instance_type/tasks/single_instance.yml b/tests/integration/targets/ec2_instance_type/tasks/single_instance.yml new file mode 100644 index 00000000000..bdf2bc7eaa2 --- /dev/null +++ b/tests/integration/targets/ec2_instance_type/tasks/single_instance.yml @@ -0,0 +1,77 @@ +--- +- name: Test upgrade instance type from a single created instance + block: + - name: Create ec2 instance + amazon.aws.ec2_instance: + state: present + name: "{{ ec2_instance_name }}" + image_id: "{{ ec2_ami_id }}" + subnet_id: "{{ testing_subnet_a.subnet.id }}" + instance_type: "{{ ec2_instance_type_initial }}" + wait: false + register: _instances + + - name: Define instance ids + ansible.builtin.set_fact: + ec2_instance_ids: "{{ _instances.instance_ids }}" + + - name: Update instance type (check mode) + amazon.aws.ec2_instance: + state: present + instance_ids: "{{ ec2_instance_ids }}" + instance_type: "{{ ec2_instance_type_updated }}" + wait: false + register: update_check_mode + check_mode: true + + - name: Gather ec2 instances info + amazon.aws.ec2_instance_info: + instance_ids: "{{ ec2_instance_ids }}" + register: _instances_info + + - name: Ensure module reported change while the instance type was not changed (check_mode) + ansible.builtin.assert: + that: + - update_check_mode is changed + - _instances_info.instances | map(attribute='instance_type') | unique | list == [ec2_instance_type_initial] + + - name: Update instance type + amazon.aws.ec2_instance: + state: present + instance_ids: "{{ ec2_instance_ids }}" + instance_type: "{{ ec2_instance_type_updated }}" + wait: false + register: _update_instance_type + + - name: Gather ec2 instances info + amazon.aws.ec2_instance_info: + instance_ids: "{{ ec2_instance_ids }}" + register: _instances_info + + - name: Ensure module reported change while the instance type was not changed (check_mode) + ansible.builtin.assert: + that: + - _update_instance_type is changed + - _instances_info.instances | map(attribute='instance_type') | unique | list == [ec2_instance_type_updated] + + - name: Update instance type (idempotency) + amazon.aws.ec2_instance: + state: present + instance_ids: "{{ ec2_instance_ids }}" + instance_type: "{{ ec2_instance_type_updated }}" + wait: false + register: _update_idempotency + + - name: Validate idempotency did not reported change + ansible.builtin.assert: + that: + - _update_idempotency is not changed + + always: + - name: Delete instance created for tests + amazon.aws.ec2_instance: + state: absent + instance_ids: "{{ ec2_instance_ids }}" + wait: false + ignore_errors: true + when: ec2_instance_ids is defined \ No newline at end of file diff --git a/tests/integration/targets/ec2_instance_type/tasks/update_instance_type.yml b/tests/integration/targets/ec2_instance_type/tasks/update_instance_type.yml new file mode 100644 index 00000000000..dfbc10ba30b --- /dev/null +++ b/tests/integration/targets/ec2_instance_type/tasks/update_instance_type.yml @@ -0,0 +1,77 @@ +--- +- name: Gather current ec2 instances info + amazon.aws.ec2_instance_info: + filters: + "tag:Name": "{{ ec2_instance_name }}" + instance-state-name: [pending, running] + register: _current_instances_info + +# Test update using check_mode +- name: Update instance type using check mode + amazon.aws.ec2_instance: + state: present + name: "{{ ec2_instance_name }}" + image_id: "{{ ec2_ami_id }}" + subnet_id: "{{ testing_subnet_a.subnet.id }}" + instance_type: "{{ item.new_instance_type }}" + wait: false + exact_count: "{{ item.new_instance_count }}" + register: run_check_mode + check_mode: true + +- name: Gather ec2 instances info + amazon.aws.ec2_instance_info: + filters: + "tag:Name": "{{ ec2_instance_name }}" + instance-state-name: [pending, running] + register: _instances_info + +- name: Ensure module reported change while the instance type was not updated (check_mode=true) + ansible.builtin.assert: + that: + - run_check_mode is changed + - _instances_info.instances | length == _current_instances_info.instances | length + - _instances_info.instances | map(attribute='instance_type') | list == _current_instances_info.instances | map(attribute='instance_type') | list + +# Run update +- name: Update instance type + amazon.aws.ec2_instance: + state: present + name: "{{ ec2_instance_name }}" + image_id: "{{ ec2_ami_id }}" + subnet_id: "{{ testing_subnet_a.subnet.id }}" + instance_type: "{{ item.new_instance_type }}" + wait: false + exact_count: "{{ item.new_instance_count }}" + register: _run_update + +- name: Gather ec2 instances info + amazon.aws.ec2_instance_info: + filters: + "tag:Name": "{{ ec2_instance_name }}" + instance-state-name: [pending, running] + register: _instances_info + +- name: Ensure module reported change and the instance type changed + ansible.builtin.assert: + that: + - _run_update is changed + - _instances_info.instances | length == item.new_instance_count + - _instances_info.instances | map(attribute='instance_type') | unique | list == [item.new_instance_type] + +# Test idempotency +- name: Update instance type once again (idempotency) + amazon.aws.ec2_instance: + state: present + name: "{{ ec2_instance_name }}" + image_id: "{{ ec2_ami_id }}" + subnet_id: "{{ testing_subnet_a.subnet.id }}" + instance_type: "{{ item.new_instance_type }}" + wait: false + exact_count: "{{ item.new_instance_count }}" + register: _run_idempotency + +- name: Validate idempotency + ansible.builtin.assert: + that: + - _run_idempotency is not changed From eac7c04f24270aaa9ad0532c1b8166f969d9b33e Mon Sep 17 00:00:00 2001 From: abikouo Date: Thu, 19 Sep 2024 15:36:16 +0200 Subject: [PATCH 5/8] additional fixes --- plugins/modules/ec2_instance.py | 49 ++++++++++--------- .../tasks/main.yml | 26 ++++++++-- 2 files changed, 46 insertions(+), 29 deletions(-) diff --git a/plugins/modules/ec2_instance.py b/plugins/modules/ec2_instance.py index dab9fd64a15..78939e5b90c 100644 --- a/plugins/modules/ec2_instance.py +++ b/plugins/modules/ec2_instance.py @@ -2459,36 +2459,37 @@ def enforce_count( # get the instance ids of instances with the count tag on them all_instance_ids = [x["InstanceId"] for x in existing_matches] terminate_ids = all_instance_ids[0:to_terminate] - if module.check_mode: - return dict( - changed=True, - terminated_ids=terminate_ids, - instance_ids=all_instance_ids, - msg=f"Would have terminated following instances if not in check mode {terminate_ids}", - ) - # terminate instances - try: - terminate_instances(client, terminate_ids) - except AnsibleAWSError as e: - module.fail_json(e, msg="Unable to terminate instances") - await_instances(client, module, terminate_ids, desired_module_state="terminated", force_wait=True) - - # include data for all matched instances in addition to the list of terminations - # allowing for recovery of metadata from the destructive operation results = dict( changed=True, - msg="Successfully terminated instances.", terminated_ids=terminate_ids, instance_ids=all_instance_ids, - instances=existing_matches, + msg=f"Would have terminated following instances if not in check mode {terminate_ids}", ) + if not module.check_mode: + # terminate instances + try: + terminate_instances(client, terminate_ids) + except AnsibleAWSError as e: + module.fail_json(e, msg="Unable to terminate instances") + await_instances(client, module, terminate_ids, desired_module_state="terminated", force_wait=True) + + # include data for all matched instances in addition to the list of terminations + # allowing for recovery of metadata from the destructive operation + results = dict( + changed=True, + msg="Successfully terminated instances.", + terminated_ids=terminate_ids, + instance_ids=all_instance_ids, + instances=existing_matches, + ) - # Find instances - existing_matches = find_instances(client, module, filters=filters) - # Update instance attributes - updated_results = handle_existing(client, module, existing_matches, desired_module_state, filters) - if updated_results["changed"]: - results = updated_results + if not module.check_mode: + # Find instances + existing_matches = find_instances(client, module, filters=filters) + # Update instance attributes + updated_results = handle_existing(client, module, existing_matches, desired_module_state, filters) + if updated_results["changed"]: + results = updated_results return results diff --git a/tests/integration/targets/ec2_instance_instance_multiple/tasks/main.yml b/tests/integration/targets/ec2_instance_instance_multiple/tasks/main.yml index 735562370c2..6e7e4dac6bf 100644 --- a/tests/integration/targets/ec2_instance_instance_multiple/tasks/main.yml +++ b/tests/integration/targets/ec2_instance_instance_multiple/tasks/main.yml @@ -15,6 +15,7 @@ region: "{{ aws_region }}" image_id: "{{ ec2_ami_id }}" state: present + purge_tags: false tags: TestId: "{{ ec2_instance_tag_TestId }}" filters: @@ -38,9 +39,10 @@ state: present tags: TestId: "{{ ec2_instance_tag_TestId }}" + purge_tags: false filters: tag:TestId: "{{ ec2_instance_tag_TestId }}" - wait: true + wait: false register: create_multiple_instances - ansible.builtin.assert: @@ -121,6 +123,7 @@ region: "{{ aws_region }}" name: "{{ resource_prefix }}-test-enf_cnt" image_id: "{{ ec2_ami_id }}" + purge_tags: false tags: TestId: "{{ ec2_instance_tag_TestId }}" register: create_multiple_instances @@ -140,9 +143,10 @@ region: "{{ aws_region }}" image_id: "{{ ec2_ami_id }}" name: "{{ resource_prefix }}-test-enf_cnt" + purge_tags: false tags: TestId: "{{ ec2_instance_tag_TestId }}" - wait: true + wait: false register: create_multiple_instances - ansible.builtin.assert: @@ -160,6 +164,7 @@ region: "{{ aws_region }}" image_id: "{{ ec2_ami_id }}" name: "{{ resource_prefix }}-test-enf_cnt" + purge_tags: false tags: TestId: "{{ ec2_instance_tag_TestId }}" register: create_multiple_instances @@ -180,6 +185,7 @@ region: "{{ aws_region }}" image_id: "{{ ec2_ami_id }}" name: "{{ resource_prefix }}-test-enf_cnt" + purge_tags: false tags: TestId: "{{ ec2_instance_tag_TestId }}" wait: true @@ -200,6 +206,7 @@ region: "{{ aws_region }}" image_id: "{{ ec2_ami_id }}" name: "{{ resource_prefix }}-test-enf_cnt" + purge_tags: false tags: TestId: "{{ ec2_instance_tag_TestId }}" register: terminate_multiple_instances @@ -222,9 +229,10 @@ region: "{{ aws_region }}" image_id: "{{ ec2_ami_id }}" name: "{{ resource_prefix }}-test-enf_cnt" + purge_tags: false tags: TestId: "{{ ec2_instance_tag_TestId }}" - wait: true + wait: false register: terminate_multiple_instances - ansible.builtin.assert: @@ -243,6 +251,7 @@ region: "{{ aws_region }}" image_id: "{{ ec2_ami_id }}" name: "{{ resource_prefix }}-test-enf_cnt" + purge_tags: false tags: TestId: "{{ ec2_instance_tag_TestId }}" register: terminate_multiple_instances @@ -264,6 +273,7 @@ region: "{{ aws_region }}" image_id: "{{ ec2_ami_id }}" name: "{{ resource_prefix }}-test-enf_cnt" + purge_tags: false tags: TestId: "{{ ec2_instance_tag_TestId }}" register: terminate_multiple_instances @@ -285,6 +295,7 @@ region: "{{ aws_region }}" image_id: "{{ ec2_ami_id }}" name: "{{ resource_prefix }}-test-enf_cnt" + purge_tags: false tags: TestId: "{{ ec2_instance_tag_TestId }}" register: restart_multiple_instances @@ -310,9 +321,10 @@ region: "{{ aws_region }}" image_id: "{{ ec2_ami_id }}" name: "{{ resource_prefix }}-test-enf_cnt" + purge_tags: false tags: TestId: "{{ ec2_instance_tag_TestId }}" - wait: true + wait: false register: restart_multiple_instances - ansible.builtin.assert: @@ -335,6 +347,7 @@ region: "{{ aws_region }}" image_id: "{{ ec2_ami_id }}" name: "{{ resource_prefix }}-test-enf_cnt" + purge_tags: false tags: TestId: "{{ ec2_instance_tag_TestId }}" wait: true @@ -358,9 +371,10 @@ region: "{{ aws_region }}" image_id: "{{ ec2_ami_id }}" name: "{{ resource_prefix }}-test-enf_cnt" + purge_tags: false tags: TestId: "{{ ec2_instance_tag_TestId }}" - wait: true + wait: false register: create_multiple_instances - name: debug is here @@ -381,6 +395,7 @@ region: "{{ aws_region }}" image_id: "{{ ec2_ami_id }}" name: "{{ resource_prefix }}-test-enf_cnt" + purge_tags: false tags: TestId: "{{ ec2_instance_tag_TestId }}" wait: true @@ -403,6 +418,7 @@ region: "{{ aws_region }}" image_id: "{{ ec2_ami_id }}" name: "{{ resource_prefix }}-test-enf_cnt" + purge_tags: false tags: TestId: "{{ ec2_instance_tag_TestId }}" wait: true From 6285f8db41a7b61a6abfebb6b9a675e597b35d92 Mon Sep 17 00:00:00 2001 From: abikouo Date: Thu, 19 Sep 2024 15:43:06 +0200 Subject: [PATCH 6/8] fix sanity tests --- .../plugins/modules/ec2_instance/test_modify_instance_type.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/unit/plugins/modules/ec2_instance/test_modify_instance_type.py b/tests/unit/plugins/modules/ec2_instance/test_modify_instance_type.py index 8866331581a..b83f96aadac 100644 --- a/tests/unit/plugins/modules/ec2_instance/test_modify_instance_type.py +++ b/tests/unit/plugins/modules/ec2_instance/test_modify_instance_type.py @@ -3,7 +3,6 @@ # This file is part of Ansible # GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt) -from unittest.mock import ANY from unittest.mock import MagicMock from unittest.mock import call from unittest.mock import patch From c61d6ed08a9cdd245c300514dbdbecc7214f1525 Mon Sep 17 00:00:00 2001 From: abikouo Date: Thu, 26 Sep 2024 16:12:45 +0200 Subject: [PATCH 7/8] Wait for instances --- .../targets/ec2_instance_instance_multiple/tasks/main.yml | 2 +- .../targets/ec2_instance_type/tasks/single_instance.yml | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/integration/targets/ec2_instance_instance_multiple/tasks/main.yml b/tests/integration/targets/ec2_instance_instance_multiple/tasks/main.yml index 6e7e4dac6bf..a0516f40480 100644 --- a/tests/integration/targets/ec2_instance_instance_multiple/tasks/main.yml +++ b/tests/integration/targets/ec2_instance_instance_multiple/tasks/main.yml @@ -232,7 +232,7 @@ purge_tags: false tags: TestId: "{{ ec2_instance_tag_TestId }}" - wait: false + wait: true register: terminate_multiple_instances - ansible.builtin.assert: diff --git a/tests/integration/targets/ec2_instance_type/tasks/single_instance.yml b/tests/integration/targets/ec2_instance_type/tasks/single_instance.yml index bdf2bc7eaa2..5b0a735aa89 100644 --- a/tests/integration/targets/ec2_instance_type/tasks/single_instance.yml +++ b/tests/integration/targets/ec2_instance_type/tasks/single_instance.yml @@ -48,7 +48,7 @@ instance_ids: "{{ ec2_instance_ids }}" register: _instances_info - - name: Ensure module reported change while the instance type was not changed (check_mode) + - name: Ensure module reported change and the instance type was updated ansible.builtin.assert: that: - _update_instance_type is changed @@ -62,7 +62,7 @@ wait: false register: _update_idempotency - - name: Validate idempotency did not reported change + - name: Validate idempotency did not report change ansible.builtin.assert: that: - _update_idempotency is not changed From 9cef8d26dbd8dbac7ef2a2ea1b1f9c9f04ed3afc Mon Sep 17 00:00:00 2001 From: abikouo Date: Fri, 27 Sep 2024 14:16:47 +0200 Subject: [PATCH 8/8] set back waiter --- .../targets/ec2_instance_instance_multiple/tasks/main.yml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/integration/targets/ec2_instance_instance_multiple/tasks/main.yml b/tests/integration/targets/ec2_instance_instance_multiple/tasks/main.yml index a0516f40480..898f45f3de4 100644 --- a/tests/integration/targets/ec2_instance_instance_multiple/tasks/main.yml +++ b/tests/integration/targets/ec2_instance_instance_multiple/tasks/main.yml @@ -42,7 +42,7 @@ purge_tags: false filters: tag:TestId: "{{ ec2_instance_tag_TestId }}" - wait: false + wait: true register: create_multiple_instances - ansible.builtin.assert: @@ -146,7 +146,7 @@ purge_tags: false tags: TestId: "{{ ec2_instance_tag_TestId }}" - wait: false + wait: true register: create_multiple_instances - ansible.builtin.assert: @@ -324,7 +324,7 @@ purge_tags: false tags: TestId: "{{ ec2_instance_tag_TestId }}" - wait: false + wait: true register: restart_multiple_instances - ansible.builtin.assert: @@ -374,7 +374,7 @@ purge_tags: false tags: TestId: "{{ ec2_instance_tag_TestId }}" - wait: false + wait: true register: create_multiple_instances - name: debug is here