From 9bfd2a1e616d8a810766ea1a901ecbd712cdfab7 Mon Sep 17 00:00:00 2001 From: Ana Zobec Date: Tue, 16 May 2023 14:58:45 +0200 Subject: [PATCH 1/7] Update return of vm_snapshot_attach_disk module. --- plugins/modules/vm_snapshot_attach_disk.py | 25 +++++++++++++++++----- 1 file changed, 20 insertions(+), 5 deletions(-) diff --git a/plugins/modules/vm_snapshot_attach_disk.py b/plugins/modules/vm_snapshot_attach_disk.py index 326171cf7..a4a7215a3 100644 --- a/plugins/modules/vm_snapshot_attach_disk.py +++ b/plugins/modules/vm_snapshot_attach_disk.py @@ -101,6 +101,12 @@ description: VM unique identifier. type: str sample: e18ec6af-9dd2-41dc-89af-8ce637171524 +vm_rebooted: + description: + - Info if reboot of the VM was performed. + returned: success + type: bool + sample: true """ @@ -118,7 +124,7 @@ def attach_disk( module: AnsibleModule, rest_client: RestClient -) -> Tuple[bool, Optional[Dict[Any, Any]], TypedDiff]: +) -> Tuple[bool, Optional[Dict[Any, Any]], TypedDiff, bool]: # =============== SAVE PARAMS VALUES =============== # destination vm_name = module.params["vm_name"] @@ -162,11 +168,12 @@ def attach_disk( rest_client=rest_client, ) if before_disk is not None: - # changed, after, diff + # changed, after, diff, vm_rebooted return ( False, before_disk, dict(before=before_disk, after=None), + False, # destination vm wasn't rebooted up to this point ) # First power off the destination VM @@ -208,7 +215,14 @@ def attach_disk( ) # Restart the previously running VM (destination) + vm_state_before = vm_object.power_state vm_object.vm_power_up(module, rest_client) # type: ignore + vm_state_after = vm_object.power_state + + # This is True if the destination VM was indeed rebooted + vm_destination_reboot = \ + (vm_state_before != vm_state_after) and \ + vm_state_before in ("stop", "stopped", "shutdown") # return changed, after, diff return ( @@ -218,12 +232,13 @@ def attach_disk( dict( before=None, after=created_disk ), # before, we ofcourse, didn't have that new block device, and after we should have it + vm_destination_reboot, ) def run( module: AnsibleModule, rest_client: RestClient -) -> Tuple[bool, Optional[Dict[Any, Any]], TypedDiff]: +) -> Tuple[bool, Optional[Dict[Any, Any]], TypedDiff, bool]: return attach_disk(module, rest_client) @@ -267,8 +282,8 @@ def main() -> None: try: client = Client.get_client(module.params["cluster_instance"]) rest_client = RestClient(client) - changed, record, diff = run(module, rest_client) - module.exit_json(changed=changed, record=record, diff=diff) + changed, record, diff, reboot = run(module, rest_client) + module.exit_json(changed=changed, record=record, diff=diff, vm_rebooted=reboot) except errors.ScaleComputingError as e: module.fail_json(msg=str(e)) From 72c5f6a7125bcf02175ad33252257d5b8292c129 Mon Sep 17 00:00:00 2001 From: Justin Cinkelj Date: Tue, 16 May 2023 16:45:04 +0200 Subject: [PATCH 2/7] Use VM .reboot to detect if VM reboot was needed Signed-off-by: Justin Cinkelj --- plugins/modules/vm_snapshot_attach_disk.py | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/plugins/modules/vm_snapshot_attach_disk.py b/plugins/modules/vm_snapshot_attach_disk.py index a4a7215a3..c499275fe 100644 --- a/plugins/modules/vm_snapshot_attach_disk.py +++ b/plugins/modules/vm_snapshot_attach_disk.py @@ -215,14 +215,7 @@ def attach_disk( ) # Restart the previously running VM (destination) - vm_state_before = vm_object.power_state vm_object.vm_power_up(module, rest_client) # type: ignore - vm_state_after = vm_object.power_state - - # This is True if the destination VM was indeed rebooted - vm_destination_reboot = \ - (vm_state_before != vm_state_after) and \ - vm_state_before in ("stop", "stopped", "shutdown") # return changed, after, diff return ( @@ -232,7 +225,7 @@ def attach_disk( dict( before=None, after=created_disk ), # before, we ofcourse, didn't have that new block device, and after we should have it - vm_destination_reboot, + vm_object.reboot, ) From 52d3ddf3b2c083fc186d342dad9befcd34534b2f Mon Sep 17 00:00:00 2001 From: Justin Cinkelj Date: Tue, 16 May 2023 17:33:22 +0200 Subject: [PATCH 3/7] Test vm_rebooted return value Signed-off-by: Justin Cinkelj --- .../vm_snapshot/tasks/03_vm_snapshot_attach_disk.yml | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tests/integration/targets/vm_snapshot/tasks/03_vm_snapshot_attach_disk.yml b/tests/integration/targets/vm_snapshot/tasks/03_vm_snapshot_attach_disk.yml index cb68f4ef5..988379f95 100644 --- a/tests/integration/targets/vm_snapshot/tasks/03_vm_snapshot_attach_disk.yml +++ b/tests/integration/targets/vm_snapshot/tasks/03_vm_snapshot_attach_disk.yml @@ -83,6 +83,7 @@ - ansible.builtin.assert: that: - result is changed + - result.vm_rebooted is false - include_tasks: helper_check_vm_state.yml vars: vm_name: "{{ vm_2 }}" @@ -117,6 +118,7 @@ - ansible.builtin.assert: that: - result is not changed + - result.vm_rebooted is false - include_tasks: helper_check_vm_state.yml vars: vm_name: "{{ vm_2 }}" @@ -146,6 +148,7 @@ - ansible.builtin.assert: that: - result is changed + - result.vm_rebooted is true - include_tasks: helper_check_vm_state.yml vars: vm_name: "{{ vm_2 }}" @@ -174,6 +177,7 @@ - ansible.builtin.assert: that: - result is not changed + - result.vm_rebooted is false - include_tasks: helper_check_vm_state.yml vars: vm_name: "{{ vm_2 }}" @@ -203,6 +207,7 @@ - ansible.builtin.assert: that: - result is changed + - result.vm_rebooted is true - include_tasks: helper_check_vm_state.yml vars: vm_name: "{{ vm_1 }}" @@ -235,6 +240,7 @@ - ansible.builtin.assert: that: - result is not changed + - result.vm_rebooted is false - ansible.builtin.assert: *test-virtio-2 # ---------- Cleanup ------------ From 72530d2524deecdad046331b65bef16dcf19b9ff Mon Sep 17 00:00:00 2001 From: Justin Cinkelj Date: Tue, 16 May 2023 18:16:58 +0200 Subject: [PATCH 4/7] Test returned record Signed-off-by: Justin Cinkelj --- .../tasks/03_vm_snapshot_attach_disk.yml | 21 ++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/tests/integration/targets/vm_snapshot/tasks/03_vm_snapshot_attach_disk.yml b/tests/integration/targets/vm_snapshot/tasks/03_vm_snapshot_attach_disk.yml index 988379f95..b45dac3a1 100644 --- a/tests/integration/targets/vm_snapshot/tasks/03_vm_snapshot_attach_disk.yml +++ b/tests/integration/targets/vm_snapshot/tasks/03_vm_snapshot_attach_disk.yml @@ -14,6 +14,12 @@ force_reboot: true # allow forced vm shutdown + # disk size gets rounded by HC3: + # 0.1 GB to 107374182 B + # 0.2 GB to 214958080 B + size_0_1_GB: 107374182 + size_0_2_GB: 214958080 + block: - name: Create a stopped VM "{{ vm_2 }}" scale_computing.hypercore.vm: @@ -90,8 +96,11 @@ expected_state: stopped - ansible.builtin.assert: &test-virtio that: - - result.record.type == "virtio_disk" + - "result.record.keys() | sort == ['disk_slot', 'size', 'type', 'uuid', 'vm_uuid']" - result.record.disk_slot == slot_a + - result.record.size == size_0_2_GB + - result.record.type == "virtio_disk" + - result.record.uuid | length == 36 - result.record.vm_uuid == vm_2_info.records[0].uuid - name: Start VM "{{ vm_2 }}" @@ -155,8 +164,11 @@ expected_state: started - ansible.builtin.assert: &test-not-virtio that: - - result.record.type == "ide_disk" + - "result.record.keys() | sort == ['disk_slot', 'size', 'type', 'uuid', 'vm_uuid']" - result.record.disk_slot == slot_b + - result.record.size == size_0_2_GB + - result.record.type == "ide_disk" + - result.record.uuid | length == 36 - result.record.vm_uuid == vm_2_info.records[0].uuid - name: >- @@ -214,8 +226,11 @@ expected_state: started - ansible.builtin.assert: &test-virtio-2 that: - - result.record.type == "virtio_disk" + - "result.record.keys() | sort == ['disk_slot', 'size', 'type', 'uuid', 'vm_uuid']" - result.record.disk_slot == slot_vm_1_virtio + - result.record.size == size_0_2_GB + - result.record.type == "virtio_disk" + - result.record.uuid | length == 36 - result.record.vm_uuid == vm_1_info.records[0].uuid - name: >- From 0b60ff2b1229ad858790ee8663326097c1d7fa62 Mon Sep 17 00:00:00 2001 From: Justin Cinkelj Date: Tue, 16 May 2023 18:29:49 +0200 Subject: [PATCH 5/7] Test returned record is consistent with data from vm_info Signed-off-by: Justin Cinkelj --- .../tasks/03_vm_snapshot_attach_disk.yml | 25 +++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/tests/integration/targets/vm_snapshot/tasks/03_vm_snapshot_attach_disk.yml b/tests/integration/targets/vm_snapshot/tasks/03_vm_snapshot_attach_disk.yml index b45dac3a1..93cb75e67 100644 --- a/tests/integration/targets/vm_snapshot/tasks/03_vm_snapshot_attach_disk.yml +++ b/tests/integration/targets/vm_snapshot/tasks/03_vm_snapshot_attach_disk.yml @@ -102,6 +102,13 @@ - result.record.type == "virtio_disk" - result.record.uuid | length == 36 - result.record.vm_uuid == vm_2_info.records[0].uuid + # Test returned record is consistent with data from vm_info + - vm_info.records[0].disks | length == 3 + - vm_info.records[0].disks[2].disk_slot == slot_a + - vm_info.records[0].disks[2].size == size_0_2_GB + - vm_info.records[0].disks[2].type == "virtio_disk" + - vm_info.records[0].disks[2].uuid == result.record.uuid + - vm_info.records[0].disks[2].vm_uuid == vm_2_info.records[0].uuid - name: Start VM "{{ vm_2 }}" scale_computing.hypercore.vm_params: @@ -170,6 +177,17 @@ - result.record.type == "ide_disk" - result.record.uuid | length == 36 - result.record.vm_uuid == vm_2_info.records[0].uuid + - vm_info.records[0].disks | length == 4 + - vm_info.records[0].disks[2].disk_slot == slot_a + - vm_info.records[0].disks[2].size == size_0_2_GB + - vm_info.records[0].disks[2].type == "virtio_disk" + # - vm_info.records[0].disks[2].uuid == result.record.uuid + - vm_info.records[0].disks[2].vm_uuid == vm_2_info.records[0].uuid + - vm_info.records[0].disks[3].disk_slot == slot_b + - vm_info.records[0].disks[3].size == size_0_2_GB + - vm_info.records[0].disks[3].type == "ide_disk" + - vm_info.records[0].disks[3].uuid == result.record.uuid + - vm_info.records[0].disks[3].vm_uuid == vm_2_info.records[0].uuid - name: >- IDEMPOTENCE - Attach "snap-0" from VM "{{ vm_1 }}" @@ -232,6 +250,13 @@ - result.record.type == "virtio_disk" - result.record.uuid | length == 36 - result.record.vm_uuid == vm_1_info.records[0].uuid + - vm_info.records[0].disks | length == 3 + - vm_info.records[0].disks[2].disk_slot == slot_vm_1_virtio + - vm_info.records[0].disks[2].size == size_0_2_GB + - vm_info.records[0].disks[2].type == "virtio_disk" + - vm_info.records[0].disks[2].uuid == result.record.uuid + - vm_info.records[0].disks[2].vm_uuid == vm_1_info.records[0].uuid + - name: >- IDEMPOTENCE - Attach "snap-0" from VM "{{ vm_1 }}" From dc58c0d87014f80461cd974c62a4e69455754fd3 Mon Sep 17 00:00:00 2001 From: Justin Cinkelj Date: Tue, 16 May 2023 18:55:31 +0200 Subject: [PATCH 6/7] Update unit tests Signed-off-by: Justin Cinkelj --- .../modules/test_vm_snapshot_attach_disk.py | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/tests/unit/plugins/modules/test_vm_snapshot_attach_disk.py b/tests/unit/plugins/modules/test_vm_snapshot_attach_disk.py index e0bcb9719..99011f90a 100644 --- a/tests/unit/plugins/modules/test_vm_snapshot_attach_disk.py +++ b/tests/unit/plugins/modules/test_vm_snapshot_attach_disk.py @@ -155,10 +155,10 @@ def setup_method(self): @pytest.mark.parametrize( ("destination_vm_disk_info", "expected_return"), [ - (None, (True, BLOCK_DEVICE, dict(before=None, after=BLOCK_DEVICE))), + (None, (True, BLOCK_DEVICE, dict(before=None, after=BLOCK_DEVICE), True)), ( BLOCK_DEVICE, - (False, BLOCK_DEVICE, dict(before=BLOCK_DEVICE, after=None)), + (False, BLOCK_DEVICE, dict(before=BLOCK_DEVICE, after=None), False), ), ], ) @@ -222,7 +222,7 @@ def test_attach_disk_is_change( check_mode=False, ) - changed, record, diff = vm_snapshot_attach_disk.attach_disk(module, rest_client) + changed, record, diff, vm_rebooted = vm_snapshot_attach_disk.attach_disk(module, rest_client) if destination_vm_disk_info is None: rest_client.create_record.assert_any_call(**called_with_dict) @@ -235,15 +235,16 @@ def test_attach_disk_is_change( assert changed == expected_return[0] assert record == expected_return[1] assert diff == expected_return[2] + assert vm_rebooted == expected_return[3] class TestMain: - def test_fail(self, run_main): - success, result = run_main(vm_snapshot_attach_disk) + def test_fail(self, run_main_with_reboot): + success, result = run_main_with_reboot(vm_snapshot_attach_disk) assert success is False assert "missing required arguments" in result["msg"] - def test_params(self, run_main): - success, result = run_main(vm_snapshot_attach_disk, PARAMS) + def test_params(self, run_main_with_reboot): + success, result = run_main_with_reboot(vm_snapshot_attach_disk, PARAMS) assert success is True From 6f7da9ce6168bf4ba71b67535b60e65b8ef5b02b Mon Sep 17 00:00:00 2001 From: Justin Cinkelj Date: Tue, 16 May 2023 18:57:44 +0200 Subject: [PATCH 7/7] format Signed-off-by: Justin Cinkelj --- tests/unit/plugins/modules/test_vm_snapshot_attach_disk.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/unit/plugins/modules/test_vm_snapshot_attach_disk.py b/tests/unit/plugins/modules/test_vm_snapshot_attach_disk.py index 99011f90a..7847d8a20 100644 --- a/tests/unit/plugins/modules/test_vm_snapshot_attach_disk.py +++ b/tests/unit/plugins/modules/test_vm_snapshot_attach_disk.py @@ -222,7 +222,9 @@ def test_attach_disk_is_change( check_mode=False, ) - changed, record, diff, vm_rebooted = vm_snapshot_attach_disk.attach_disk(module, rest_client) + changed, record, diff, vm_rebooted = vm_snapshot_attach_disk.attach_disk( + module, rest_client + ) if destination_vm_disk_info is None: rest_client.create_record.assert_any_call(**called_with_dict)