From 59bbdcc5ee6ad858466e37ae67d54baa230b5540 Mon Sep 17 00:00:00 2001 From: Kate Shvaika Date: Fri, 28 Nov 2025 12:05:07 +0100 Subject: [PATCH 1/6] datesources to storage/snapshot tests --- tests/storage/conftest.py | 77 +++++------ tests/storage/snapshots/test_snapshots.py | 126 +++++++++--------- tests/storage/upgrade/test_upgrade_storage.py | 8 +- utilities/storage.py | 2 +- 4 files changed, 101 insertions(+), 112 deletions(-) diff --git a/tests/storage/conftest.py b/tests/storage/conftest.py index c5b512abdc..ee4c2a25e4 100644 --- a/tests/storage/conftest.py +++ b/tests/storage/conftest.py @@ -21,6 +21,12 @@ from ocp_resources.route import Route from ocp_resources.secret import Secret from ocp_resources.storage_class import StorageClass +from ocp_resources.virtual_machine_cluster_instancetype import ( + VirtualMachineClusterInstancetype, +) +from ocp_resources.virtual_machine_cluster_preference import ( + VirtualMachineClusterPreference, +) from ocp_resources.virtual_machine_snapshot import VirtualMachineSnapshot from pytest_testconfig import config as py_config from timeout_sampler import TimeoutExpiredError, TimeoutSampler @@ -45,9 +51,11 @@ CDI_UPLOADPROXY, CNV_TEST_SERVICE_ACCOUNT, CNV_TESTS_CONTAINER, + OS_FLAVOR_RHEL, SECURITY_CONTEXT, TIMEOUT_1MIN, TIMEOUT_5SEC, + U1_SMALL, Images, ) from utilities.hco import ( @@ -60,7 +68,7 @@ ) from utilities.jira import is_jira_open from utilities.storage import ( - create_cirros_dv_for_snapshot_dict, + data_volume_template_with_source_ref_dict, get_downloaded_artifact, write_file, ) @@ -419,7 +427,7 @@ def hpp_daemonset_scope_module(hco_namespace, hpp_cr_suffix_scope_module): @pytest.fixture() -def cirros_vm_name(request): +def rhel_vm_name(request): return request.param["vm_name"] @@ -450,59 +458,40 @@ def artifactory_config_map_scope_module(namespace): @pytest.fixture() -def cirros_dv_for_snapshot_dict( - namespace, - cirros_vm_name, - storage_class_matrix_snapshot_matrix__module__, - artifactory_secret_scope_module, - artifactory_config_map_scope_module, -): - yield create_cirros_dv_for_snapshot_dict( - name=cirros_vm_name, - namespace=namespace.name, - storage_class=[*storage_class_matrix_snapshot_matrix__module__][0], - artifactory_secret=artifactory_secret_scope_module, - artifactory_config_map=artifactory_config_map_scope_module, - ) - - -@pytest.fixture() -def cirros_vm_for_snapshot( +def rhel_vm_for_snapshot( admin_client, namespace, - cirros_vm_name, - cirros_dv_for_snapshot_dict, + rhel_vm_name, + rhel10_data_source_scope_session, + snapshot_storage_class_name_scope_module, ): - """ - Create a VM with a DV that supports snapshots - """ - dv_metadata = cirros_dv_for_snapshot_dict["metadata"] + """Create a RHEL VM with using DataSource that supports snapshots""" with VirtualMachineForTests( + name=rhel_vm_name, + namespace=namespace.name, client=admin_client, - name=cirros_vm_name, - namespace=dv_metadata["namespace"], - os_flavor=Images.Cirros.OS_FLAVOR, - memory_guest=Images.Cirros.DEFAULT_MEMORY_SIZE, - data_volume_template={ - "metadata": dv_metadata, - "spec": cirros_dv_for_snapshot_dict["spec"], - }, + os_flavor=OS_FLAVOR_RHEL, + vm_instance_type=VirtualMachineClusterInstancetype(name=U1_SMALL), + vm_preference=VirtualMachineClusterPreference(name="rhel.10"), + data_volume_template=data_volume_template_with_source_ref_dict( + data_source=rhel10_data_source_scope_session, + storage_class=snapshot_storage_class_name_scope_module, + ), ) as vm: yield vm @pytest.fixture() -def snapshots_with_content( +def snapshot_with_content( request, namespace, admin_client, - cirros_vm_for_snapshot, + rhel_vm_for_snapshot, ): """ Creates a requested number of snapshots with content The default behavior of the fixture is creating an offline - snapshot unless {online_vm = True} declared in the test - """ + snapshot unless {online_vm = True} declared in the test""" vm_snapshots = [] is_online_test = request.param.get("online_vm", False) for idx in range(request.param["number_of_snapshots"]): @@ -511,16 +500,16 @@ def snapshots_with_content( index = idx + 1 before_snap_index = f"before-snap-{index}" write_file( - vm=cirros_vm_for_snapshot, + vm=rhel_vm_for_snapshot, filename=f"{before_snap_index}.txt", content=before_snap_index, ) if is_online_test: - cirros_vm_for_snapshot.start(wait=True) + rhel_vm_for_snapshot.start(wait=True) with VirtualMachineSnapshot( - name=f"snapshot-{cirros_vm_for_snapshot.name}-number-{index}", - namespace=cirros_vm_for_snapshot.namespace, - vm_name=cirros_vm_for_snapshot.name, + name=f"snapshot-{rhel_vm_for_snapshot.name}-number-{index}", + namespace=rhel_vm_for_snapshot.namespace, + vm_name=rhel_vm_for_snapshot.name, client=admin_client, teardown=False, ) as vm_snapshot: @@ -528,7 +517,7 @@ def snapshots_with_content( vm_snapshot.wait_snapshot_done() after_snap_index = f"after-snap-{index}" write_file( - vm=cirros_vm_for_snapshot, + vm=rhel_vm_for_snapshot, filename=f"{after_snap_index}.txt", content=after_snap_index, ) diff --git a/tests/storage/snapshots/test_snapshots.py b/tests/storage/snapshots/test_snapshots.py index bd7791df34..5336fb3455 100644 --- a/tests/storage/snapshots/test_snapshots.py +++ b/tests/storage/snapshots/test_snapshots.py @@ -26,7 +26,7 @@ ) from tests.storage.utils import assert_windows_directory_existence from utilities.constants import LS_COMMAND, TIMEOUT_1MIN, TIMEOUT_10SEC -from utilities.storage import run_command_on_cirros_vm_and_check_output +from utilities.storage import run_command_on_vm_and_check_output LOGGER = logging.getLogger(__name__) @@ -48,7 +48,7 @@ def test_snapshot_feature_gate_present(kubevirt_feature_gates): class TestRestoreSnapshots: @pytest.mark.parametrize( - "cirros_vm_name, snapshots_with_content, expected_results, snapshots_to_restore_idx", + "rhel_vm_name, snapshot_with_content, expected_results, snapshots_to_restore_idx", [ pytest.param( {"vm_name": "vm-cnv-4789"}, @@ -110,12 +110,12 @@ class TestRestoreSnapshots: id="test_restore_all_snapshots", ), ], - indirect=["cirros_vm_name", "snapshots_with_content"], + indirect=["rhel_vm_name", "snapshot_with_content"], ) def test_restore_snapshots( self, - cirros_vm_for_snapshot, - snapshots_with_content, + rhel_vm_for_snapshot, + snapshot_with_content, expected_results, snapshots_to_restore_idx, ): @@ -123,21 +123,21 @@ def test_restore_snapshots( snap_idx = snapshots_to_restore_idx[idx] with VirtualMachineRestore( name=f"restore-snapshot-{snap_idx}", - namespace=cirros_vm_for_snapshot.namespace, - vm_name=cirros_vm_for_snapshot.name, - snapshot_name=snapshots_with_content[snap_idx].name, + namespace=rhel_vm_for_snapshot.namespace, + vm_name=rhel_vm_for_snapshot.name, + snapshot_name=snapshot_with_content[snap_idx].name, ) as vm_restore: vm_restore.wait_restore_done() - cirros_vm_for_snapshot.start(wait=True) - run_command_on_cirros_vm_and_check_output( - vm=cirros_vm_for_snapshot, + rhel_vm_for_snapshot.start(wait=True) + run_command_on_vm_and_check_output( + vm=rhel_vm_for_snapshot, command=LS_COMMAND, expected_result=expected_results[idx], ) - cirros_vm_for_snapshot.stop(wait=True) + rhel_vm_for_snapshot.stop(wait=True) @pytest.mark.parametrize( - "cirros_vm_name, snapshots_with_content", + "rhel_vm_name, snapshot_with_content", [ pytest.param( {"vm_name": "vm-cnv-5048"}, @@ -149,19 +149,19 @@ def test_restore_snapshots( ) def test_restore_snapshot_while_vm_is_running( self, - cirros_vm_for_snapshot, - snapshots_with_content, + rhel_vm_for_snapshot, + snapshot_with_content, ): - cirros_vm_for_snapshot.start(wait=True) + rhel_vm_for_snapshot.start(wait=True) # snapshot restore with online VM should create vmstore object # with 'status.complete=False', 'status.conditions.ready="False"' # and 'status.conditions.progress="False"' with VirtualMachineRestore( name="restore-snapshot-cnv-5048", - namespace=cirros_vm_for_snapshot.namespace, - vm_name=cirros_vm_for_snapshot.name, - snapshot_name=snapshots_with_content[0].name, + namespace=rhel_vm_for_snapshot.namespace, + vm_name=rhel_vm_for_snapshot.name, + snapshot_name=snapshot_with_content[0].name, ) as vmrestore: try: for sampler in TimeoutSampler( @@ -179,11 +179,11 @@ def test_restore_snapshot_while_vm_is_running( LOGGER.error("Snapshot restore should not succeed with running VM") raise # Snapshot restore should be successful once the VM is stopped - cirros_vm_for_snapshot.stop(wait=True) + rhel_vm_for_snapshot.stop(wait=True) vmrestore.wait_restore_done() @pytest.mark.parametrize( - "cirros_vm_name, snapshots_with_content, namespace", + "rhel_vm_name, snapshot_with_content, namespace", [ pytest.param( {"vm_name": "vm-cnv-5049"}, @@ -196,8 +196,8 @@ def test_restore_snapshot_while_vm_is_running( ) def test_fail_restore_vm_with_unprivileged_client( self, - cirros_vm_for_snapshot, - snapshots_with_content, + rhel_vm_for_snapshot, + snapshot_with_content, unprivileged_client, ): with pytest.raises( @@ -207,15 +207,15 @@ def test_fail_restore_vm_with_unprivileged_client( with VirtualMachineRestore( client=unprivileged_client, name="restore-snapshot-cnv-5049-unprivileged", - namespace=cirros_vm_for_snapshot.namespace, - vm_name=cirros_vm_for_snapshot.name, - snapshot_name=snapshots_with_content[0].name, + namespace=rhel_vm_for_snapshot.namespace, + vm_name=rhel_vm_for_snapshot.name, + snapshot_name=snapshot_with_content[0].name, ): return @pytest.mark.sno @pytest.mark.parametrize( - "cirros_vm_name, snapshots_with_content", + "rhel_vm_name, snapshot_with_content", [ pytest.param( {"vm_name": "vm-cnv-5084"}, @@ -228,33 +228,33 @@ def test_fail_restore_vm_with_unprivileged_client( ) def test_restore_same_snapshot_twice( self, - cirros_vm_for_snapshot, - snapshots_with_content, + rhel_vm_for_snapshot, + snapshot_with_content, ): with VirtualMachineRestore( name="restore-snapshot-cnv-5084-first", - namespace=cirros_vm_for_snapshot.namespace, - vm_name=cirros_vm_for_snapshot.name, - snapshot_name=snapshots_with_content[0].name, + namespace=rhel_vm_for_snapshot.namespace, + vm_name=rhel_vm_for_snapshot.name, + snapshot_name=snapshot_with_content[0].name, ) as first_restore: first_restore.wait_restore_done() with VirtualMachineRestore( name="restore-snapshot-cnv-5084-second", - namespace=cirros_vm_for_snapshot.namespace, - vm_name=cirros_vm_for_snapshot.name, - snapshot_name=snapshots_with_content[0].name, + namespace=rhel_vm_for_snapshot.namespace, + vm_name=rhel_vm_for_snapshot.name, + snapshot_name=snapshot_with_content[0].name, ) as second_restore: second_restore.wait_restore_done() - cirros_vm_for_snapshot.start(wait=True) - run_command_on_cirros_vm_and_check_output( - vm=cirros_vm_for_snapshot, + rhel_vm_for_snapshot.start(wait=True) + run_command_on_vm_and_check_output( + vm=rhel_vm_for_snapshot, command=LS_COMMAND, expected_result=expected_output_after_restore(1), ) @pytest.mark.parametrize( - "cirros_vm_name, snapshots_with_content", + "rhel_vm_name, snapshot_with_content", [ pytest.param( {"vm_name": "vm-cnv-4866"}, @@ -265,16 +265,16 @@ def test_restore_same_snapshot_twice( indirect=True, ) def test_remove_vm_with_snapshots( - cirros_vm_for_snapshot, - snapshots_with_content, + rhel_vm_for_snapshot, + snapshot_with_content, ): - cirros_vm_for_snapshot.delete(wait=True) - for snapshot in snapshots_with_content: + rhel_vm_for_snapshot.delete(wait=True) + for snapshot in snapshot_with_content: assert snapshot.instance.status.readyToUse @pytest.mark.parametrize( - "cirros_vm_name, snapshots_with_content, expected_result", + "rhel_vm_name, snapshot_with_content, expected_result", [ pytest.param( {"vm_name": "vm-cnv-4870"}, @@ -283,24 +283,24 @@ def test_remove_vm_with_snapshots( marks=pytest.mark.polarion("CNV-4870"), ), ], - indirect=["cirros_vm_name", "snapshots_with_content"], + indirect=["rhel_vm_name", "snapshot_with_content"], ) def test_remove_snapshots_while_vm_is_running( - cirros_vm_for_snapshot, - snapshots_with_content, + rhel_vm_for_snapshot, + snapshot_with_content, expected_result, ): - cirros_vm_for_snapshot.start(wait=True) - for idx in range(len(snapshots_with_content)): - snapshots_with_content[idx].delete(wait=True) - run_command_on_cirros_vm_and_check_output( - vm=cirros_vm_for_snapshot, + rhel_vm_for_snapshot.start(wait=True) + for idx in range(len(snapshot_with_content)): + snapshot_with_content[idx].delete(wait=True) + run_command_on_vm_and_check_output( + vm=rhel_vm_for_snapshot, command=LS_COMMAND, expected_result=expected_result, ) - cirros_vm_for_snapshot.restart(wait=True) - run_command_on_cirros_vm_and_check_output( - vm=cirros_vm_for_snapshot, + rhel_vm_for_snapshot.restart(wait=True) + run_command_on_vm_and_check_output( + vm=rhel_vm_for_snapshot, command=LS_COMMAND, expected_result=expected_result, ) @@ -335,7 +335,7 @@ def test_unprivileged_client_fails_to_list_resources(namespace, unprivileged_cli @pytest.mark.parametrize( - "cirros_vm_name, namespace", + "rhel_vm_name, namespace", [ pytest.param( {"vm_name": "vm-cnv-4867"}, @@ -347,19 +347,19 @@ def test_unprivileged_client_fails_to_list_resources(namespace, unprivileged_cli ) @pytest.mark.s390x def test_fail_to_snapshot_with_unprivileged_client_no_permissions( - cirros_vm_for_snapshot, + rhel_vm_for_snapshot, unprivileged_client, ): fail_to_create_snapshot_no_permissions( snapshot_name="snapshot-cnv-4867-unprivileged", - namespace=cirros_vm_for_snapshot.namespace, - vm_name=cirros_vm_for_snapshot.name, + namespace=rhel_vm_for_snapshot.namespace, + vm_name=rhel_vm_for_snapshot.name, client=unprivileged_client, ) @pytest.mark.parametrize( - "cirros_vm_name, namespace", + "rhel_vm_name, namespace", [ pytest.param( {"vm_name": "vm-cnv-4868"}, @@ -371,14 +371,14 @@ def test_fail_to_snapshot_with_unprivileged_client_no_permissions( ) @pytest.mark.s390x def test_fail_to_snapshot_with_unprivileged_client_dv_permissions( - cirros_vm_for_snapshot, + rhel_vm_for_snapshot, permissions_for_dv, unprivileged_client, ): fail_to_create_snapshot_no_permissions( snapshot_name="snapshot-cnv-4868-unprivileged", - namespace=cirros_vm_for_snapshot.namespace, - vm_name=cirros_vm_for_snapshot.name, + namespace=rhel_vm_for_snapshot.namespace, + vm_name=rhel_vm_for_snapshot.name, client=unprivileged_client, ) diff --git a/tests/storage/upgrade/test_upgrade_storage.py b/tests/storage/upgrade/test_upgrade_storage.py index 19a1bf378b..5d88e6d643 100644 --- a/tests/storage/upgrade/test_upgrade_storage.py +++ b/tests/storage/upgrade/test_upgrade_storage.py @@ -16,7 +16,7 @@ from utilities.storage import ( assert_disk_serial, assert_hotplugvolume_nonexist_optional_restart, - run_command_on_cirros_vm_and_check_output, + run_command_on_vm_and_check_output, wait_for_vm_volume_ready, ) from utilities.virt import migrate_vm_and_verify @@ -72,7 +72,7 @@ def test_vm_snapshot_restore_before_upgrade( ) as vm_restore: vm_restore.wait_restore_done() cirros_vm_for_upgrade_a.start(wait=True) - run_command_on_cirros_vm_and_check_output( + run_command_on_vm_and_check_output( vm=cirros_vm_for_upgrade_a, command=LS_COMMAND, expected_result="1", @@ -147,7 +147,7 @@ def test_vm_snapshot_restore_check_after_upgrade( self, cirros_vm_for_upgrade_a, ): - run_command_on_cirros_vm_and_check_output( + run_command_on_vm_and_check_output( vm=cirros_vm_for_upgrade_a, command=LS_COMMAND, expected_result="1", @@ -173,7 +173,7 @@ def test_vm_snapshot_restore_create_after_upgrade(self, cirros_vm_for_upgrade_b, ) as vm_restore: vm_restore.wait_restore_done() cirros_vm_for_upgrade_b.start(wait=True) - run_command_on_cirros_vm_and_check_output( + run_command_on_vm_and_check_output( vm=cirros_vm_for_upgrade_b, command=LS_COMMAND, expected_result="1", diff --git a/utilities/storage.py b/utilities/storage.py index 891cf2145a..a768c0a4c9 100644 --- a/utilities/storage.py +++ b/utilities/storage.py @@ -634,7 +634,7 @@ def write_file(vm, filename, content, stop_vm=True): vm.stop(wait=True) -def run_command_on_cirros_vm_and_check_output(vm, command, expected_result): +def run_command_on_vm_and_check_output(vm, command, expected_result): with console.Console(vm=vm) as vm_console: vm_console.sendline(command) vm_console.expect(expected_result, timeout=20) From 83e0e5a184df4ee6d67c547fb7e31e66246a8744 Mon Sep 17 00:00:00 2001 From: Kate Shvaika Date: Fri, 12 Dec 2025 11:27:14 +0100 Subject: [PATCH 2/6] resolved the comments from review --- tests/storage/conftest.py | 44 +++++++++---------- tests/storage/snapshots/test_snapshots.py | 51 +++++++++++++++-------- tests/storage/snapshots/utils.py | 8 ++++ tests/storage/utils.py | 8 ++++ utilities/storage.py | 18 ++++++++ 5 files changed, 90 insertions(+), 39 deletions(-) diff --git a/tests/storage/conftest.py b/tests/storage/conftest.py index ee4c2a25e4..81cffd79d2 100644 --- a/tests/storage/conftest.py +++ b/tests/storage/conftest.py @@ -43,6 +43,7 @@ get_hpp_daemonset, hpp_cr_suffix, is_hpp_cr_legacy, + ensure_vm_running, ) from tests.utils import create_cirros_vm from utilities.artifactory import get_artifactory_config_map, get_artifactory_secret @@ -51,7 +52,7 @@ CDI_UPLOADPROXY, CNV_TEST_SERVICE_ACCOUNT, CNV_TESTS_CONTAINER, - OS_FLAVOR_RHEL, + RHEL10_PREFERENCE, SECURITY_CONTEXT, TIMEOUT_1MIN, TIMEOUT_5SEC, @@ -70,9 +71,9 @@ from utilities.storage import ( data_volume_template_with_source_ref_dict, get_downloaded_artifact, - write_file, + write_file_via_ssh ) -from utilities.virt import VirtualMachineForTests +from utilities.virt import VirtualMachineForTests, running_vm LOGGER = logging.getLogger(__name__) LOCAL_PATH = f"/tmp/{Images.Cdi.QCOW2_IMG}" @@ -470,14 +471,15 @@ def rhel_vm_for_snapshot( name=rhel_vm_name, namespace=namespace.name, client=admin_client, - os_flavor=OS_FLAVOR_RHEL, - vm_instance_type=VirtualMachineClusterInstancetype(name=U1_SMALL), - vm_preference=VirtualMachineClusterPreference(name="rhel.10"), + os_flavor=RHEL10_PREFERENCE, + vm_instance_type=VirtualMachineClusterInstancetype(client=admin_client, name=U1_SMALL), + vm_preference=VirtualMachineClusterPreference(client=admin_client, name=RHEL10_PREFERENCE), data_volume_template=data_volume_template_with_source_ref_dict( data_source=rhel10_data_source_scope_session, storage_class=snapshot_storage_class_name_scope_module, ), ) as vm: + running_vm(vm=vm) yield vm @@ -491,21 +493,20 @@ def snapshot_with_content( """ Creates a requested number of snapshots with content The default behavior of the fixture is creating an offline - snapshot unless {online_vm = True} declared in the test""" + snapshot unless {online_vm = True} declared in the test + """ vm_snapshots = [] is_online_test = request.param.get("online_vm", False) + stop_vm = not is_online_test for idx in range(request.param["number_of_snapshots"]): - # write_file check if the vm is running and if not, start the vm - # after the file have been written the function stops the vm index = idx + 1 before_snap_index = f"before-snap-{index}" - write_file( - vm=rhel_vm_for_snapshot, - filename=f"{before_snap_index}.txt", - content=before_snap_index, - ) - if is_online_test: - rhel_vm_for_snapshot.start(wait=True) + with ensure_vm_running(vm=rhel_vm_for_snapshot, stop_vm=stop_vm) as vm: + write_file_via_ssh( + vm=vm, + filename=f"{before_snap_index}.txt", + content=before_snap_index + ) with VirtualMachineSnapshot( name=f"snapshot-{rhel_vm_for_snapshot.name}-number-{index}", namespace=rhel_vm_for_snapshot.namespace, @@ -516,11 +517,12 @@ def snapshot_with_content( vm_snapshots.append(vm_snapshot) vm_snapshot.wait_snapshot_done() after_snap_index = f"after-snap-{index}" - write_file( - vm=rhel_vm_for_snapshot, - filename=f"{after_snap_index}.txt", - content=after_snap_index, - ) + with ensure_vm_running(vm=rhel_vm_for_snapshot, stop_vm=stop_vm) as vm: + write_file_via_ssh( + vm=vm, + filename=f"{after_snap_index}.txt", + content=after_snap_index + ) check_snapshot_indication(snapshot=vm_snapshot, is_online=is_online_test) yield vm_snapshots diff --git a/tests/storage/snapshots/test_snapshots.py b/tests/storage/snapshots/test_snapshots.py index 5336fb3455..5c4e0a163b 100644 --- a/tests/storage/snapshots/test_snapshots.py +++ b/tests/storage/snapshots/test_snapshots.py @@ -24,9 +24,10 @@ fail_to_create_snapshot_no_permissions, start_windows_vm_after_restore, ) -from tests.storage.utils import assert_windows_directory_existence +from tests.storage.utils import assert_windows_directory_existence, ensure_vm_running from utilities.constants import LS_COMMAND, TIMEOUT_1MIN, TIMEOUT_10SEC -from utilities.storage import run_command_on_vm_and_check_output +from tests.storage.snapshots.utils import run_command_on_vm_and_check_output +from utilities.virt import restart_vm_wait_for_running_vm, running_vm LOGGER = logging.getLogger(__name__) @@ -114,27 +115,30 @@ class TestRestoreSnapshots: ) def test_restore_snapshots( self, + admin_client, rhel_vm_for_snapshot, snapshot_with_content, expected_results, snapshots_to_restore_idx, ): + if rhel_vm_for_snapshot.ready: + rhel_vm_for_snapshot.stop(wait=True) for idx in range(len(snapshots_to_restore_idx)): snap_idx = snapshots_to_restore_idx[idx] with VirtualMachineRestore( + client=admin_client, name=f"restore-snapshot-{snap_idx}", namespace=rhel_vm_for_snapshot.namespace, vm_name=rhel_vm_for_snapshot.name, snapshot_name=snapshot_with_content[snap_idx].name, ) as vm_restore: vm_restore.wait_restore_done() - rhel_vm_for_snapshot.start(wait=True) - run_command_on_vm_and_check_output( - vm=rhel_vm_for_snapshot, - command=LS_COMMAND, - expected_result=expected_results[idx], - ) - rhel_vm_for_snapshot.stop(wait=True) + with ensure_vm_running(vm=rhel_vm_for_snapshot, stop_vm=True) as vm: + run_command_on_vm_and_check_output( + vm=vm, + command=LS_COMMAND, + expected_result=expected_results[idx], + ) @pytest.mark.parametrize( "rhel_vm_name, snapshot_with_content", @@ -149,15 +153,17 @@ def test_restore_snapshots( ) def test_restore_snapshot_while_vm_is_running( self, + admin_client, rhel_vm_for_snapshot, snapshot_with_content, ): - rhel_vm_for_snapshot.start(wait=True) + running_vm(vm=rhel_vm_for_snapshot) # snapshot restore with online VM should create vmstore object # with 'status.complete=False', 'status.conditions.ready="False"' # and 'status.conditions.progress="False"' with VirtualMachineRestore( + client=admin_client, name="restore-snapshot-cnv-5048", namespace=rhel_vm_for_snapshot.namespace, vm_name=rhel_vm_for_snapshot.name, @@ -200,6 +206,8 @@ def test_fail_restore_vm_with_unprivileged_client( snapshot_with_content, unprivileged_client, ): + if rhel_vm_for_snapshot.ready: + rhel_vm_for_snapshot.stop(wait=True) with pytest.raises( ApiException, match=ERROR_MSG_USER_CANNOT_CREATE_VM_RESTORE, @@ -228,10 +236,14 @@ def test_fail_restore_vm_with_unprivileged_client( ) def test_restore_same_snapshot_twice( self, + admin_client, rhel_vm_for_snapshot, snapshot_with_content, ): + if rhel_vm_for_snapshot.ready: + rhel_vm_for_snapshot.stop(wait=True) with VirtualMachineRestore( + client=admin_client, name="restore-snapshot-cnv-5084-first", namespace=rhel_vm_for_snapshot.namespace, vm_name=rhel_vm_for_snapshot.name, @@ -239,18 +251,19 @@ def test_restore_same_snapshot_twice( ) as first_restore: first_restore.wait_restore_done() with VirtualMachineRestore( + client=admin_client, name="restore-snapshot-cnv-5084-second", namespace=rhel_vm_for_snapshot.namespace, vm_name=rhel_vm_for_snapshot.name, snapshot_name=snapshot_with_content[0].name, ) as second_restore: second_restore.wait_restore_done() - rhel_vm_for_snapshot.start(wait=True) - run_command_on_vm_and_check_output( - vm=rhel_vm_for_snapshot, - command=LS_COMMAND, - expected_result=expected_output_after_restore(1), - ) + with ensure_vm_running(vm=rhel_vm_for_snapshot, stop_vm=False) as vm: + run_command_on_vm_and_check_output( + vm=vm, + command=LS_COMMAND, + expected_result=expected_output_after_restore(1), + ) @pytest.mark.parametrize( @@ -268,6 +281,8 @@ def test_remove_vm_with_snapshots( rhel_vm_for_snapshot, snapshot_with_content, ): + if rhel_vm_for_snapshot.ready: + rhel_vm_for_snapshot.stop(wait=True) rhel_vm_for_snapshot.delete(wait=True) for snapshot in snapshot_with_content: assert snapshot.instance.status.readyToUse @@ -290,7 +305,7 @@ def test_remove_snapshots_while_vm_is_running( snapshot_with_content, expected_result, ): - rhel_vm_for_snapshot.start(wait=True) + running_vm(vm=rhel_vm_for_snapshot) for idx in range(len(snapshot_with_content)): snapshot_with_content[idx].delete(wait=True) run_command_on_vm_and_check_output( @@ -298,7 +313,7 @@ def test_remove_snapshots_while_vm_is_running( command=LS_COMMAND, expected_result=expected_result, ) - rhel_vm_for_snapshot.restart(wait=True) + restart_vm_wait_for_running_vm(vm=rhel_vm_for_snapshot, check_ssh_connectivity=True) run_command_on_vm_and_check_output( vm=rhel_vm_for_snapshot, command=LS_COMMAND, diff --git a/tests/storage/snapshots/utils.py b/tests/storage/snapshots/utils.py index 5a17f2b19a..c22fb7bddd 100644 --- a/tests/storage/snapshots/utils.py +++ b/tests/storage/snapshots/utils.py @@ -1,6 +1,7 @@ import pytest from kubernetes.client.rest import ApiException from ocp_resources.virtual_machine_snapshot import VirtualMachineSnapshot +from pyhelper_utils.shell import run_ssh_commands from tests.storage.snapshots.constants import ERROR_MSG_USER_CANNOT_CREATE_VM_SNAPSHOTS from utilities.constants import TIMEOUT_10MIN @@ -44,3 +45,10 @@ def fail_to_create_snapshot_no_permissions(snapshot_name, namespace, vm_name, cl def start_windows_vm_after_restore(vm_restore, windows_vm): vm_restore.wait_restore_done(timeout=TIMEOUT_10MIN) running_vm(vm=windows_vm) + + +def run_command_on_vm_and_check_output(vm, command, expected_result): + """Run command on RHEL VM via SSH and verify expected result is in output.""" + cmd_output = run_ssh_commands(host=vm.ssh_exec, commands=["bash", "-c", command])[0].strip() + expected_result = expected_result.strip() + assert expected_result in cmd_output, f"Expected '{expected_result}' in output '{cmd_output}'" diff --git a/tests/storage/utils.py b/tests/storage/utils.py index 01e3a64deb..8b66c87ad3 100644 --- a/tests/storage/utils.py +++ b/tests/storage/utils.py @@ -116,6 +116,14 @@ def upload_token_request(storage_ns_name, pvc_name, data, client): break +@contextmanager +def ensure_vm_running(vm, stop_vm=True): + running_vm(vm=vm) + yield vm + if stop_vm: + vm.stop(wait=True) + + def create_windows_vm_validate_guest_agent_info( dv, namespace, diff --git a/utilities/storage.py b/utilities/storage.py index a768c0a4c9..7217f5b8e3 100644 --- a/utilities/storage.py +++ b/utilities/storage.py @@ -634,6 +634,24 @@ def write_file(vm, filename, content, stop_vm=True): vm.stop(wait=True) +def write_file_via_ssh(vm: "VirtualMachineForTests", filename: str, content: str) -> None: + """ + Write content to a file in VM using SSH connection. + + Args: + vm: VirtualMachine instance with SSH connectivity + filename: Path to the file to write in the VM + content: Content to write to the file + + Raises: + TimeoutExpiredError: If SSH connectivity cannot be established + SSHException: If SSH command execution fails + """ + + cmd = shlex.split(f"echo '{content}' > {filename} && sync") + run_ssh_commands(host=vm.ssh_exec, commands=cmd) + + def run_command_on_vm_and_check_output(vm, command, expected_result): with console.Console(vm=vm) as vm_console: vm_console.sendline(command) From 595f4ad1e75a7dda122ce1b65455d1bcefd1c681 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Fri, 12 Dec 2025 14:45:58 +0000 Subject: [PATCH 3/6] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- tests/storage/conftest.py | 20 ++++---------------- tests/storage/snapshots/test_snapshots.py | 2 +- tests/storage/utils.py | 2 +- utilities/storage.py | 2 +- 4 files changed, 7 insertions(+), 19 deletions(-) diff --git a/tests/storage/conftest.py b/tests/storage/conftest.py index 81cffd79d2..f179200dfb 100644 --- a/tests/storage/conftest.py +++ b/tests/storage/conftest.py @@ -40,10 +40,10 @@ from tests.storage.utils import ( HttpService, check_snapshot_indication, + ensure_vm_running, get_hpp_daemonset, hpp_cr_suffix, is_hpp_cr_legacy, - ensure_vm_running, ) from tests.utils import create_cirros_vm from utilities.artifactory import get_artifactory_config_map, get_artifactory_secret @@ -68,11 +68,7 @@ ExecCommandOnPod, ) from utilities.jira import is_jira_open -from utilities.storage import ( - data_volume_template_with_source_ref_dict, - get_downloaded_artifact, - write_file_via_ssh -) +from utilities.storage import data_volume_template_with_source_ref_dict, get_downloaded_artifact, write_file_via_ssh from utilities.virt import VirtualMachineForTests, running_vm LOGGER = logging.getLogger(__name__) @@ -502,11 +498,7 @@ def snapshot_with_content( index = idx + 1 before_snap_index = f"before-snap-{index}" with ensure_vm_running(vm=rhel_vm_for_snapshot, stop_vm=stop_vm) as vm: - write_file_via_ssh( - vm=vm, - filename=f"{before_snap_index}.txt", - content=before_snap_index - ) + write_file_via_ssh(vm=vm, filename=f"{before_snap_index}.txt", content=before_snap_index) with VirtualMachineSnapshot( name=f"snapshot-{rhel_vm_for_snapshot.name}-number-{index}", namespace=rhel_vm_for_snapshot.namespace, @@ -518,11 +510,7 @@ def snapshot_with_content( vm_snapshot.wait_snapshot_done() after_snap_index = f"after-snap-{index}" with ensure_vm_running(vm=rhel_vm_for_snapshot, stop_vm=stop_vm) as vm: - write_file_via_ssh( - vm=vm, - filename=f"{after_snap_index}.txt", - content=after_snap_index - ) + write_file_via_ssh(vm=vm, filename=f"{after_snap_index}.txt", content=after_snap_index) check_snapshot_indication(snapshot=vm_snapshot, is_online=is_online_test) yield vm_snapshots diff --git a/tests/storage/snapshots/test_snapshots.py b/tests/storage/snapshots/test_snapshots.py index 5c4e0a163b..30fb370a5b 100644 --- a/tests/storage/snapshots/test_snapshots.py +++ b/tests/storage/snapshots/test_snapshots.py @@ -22,11 +22,11 @@ from tests.storage.snapshots.utils import ( expected_output_after_restore, fail_to_create_snapshot_no_permissions, + run_command_on_vm_and_check_output, start_windows_vm_after_restore, ) from tests.storage.utils import assert_windows_directory_existence, ensure_vm_running from utilities.constants import LS_COMMAND, TIMEOUT_1MIN, TIMEOUT_10SEC -from tests.storage.snapshots.utils import run_command_on_vm_and_check_output from utilities.virt import restart_vm_wait_for_running_vm, running_vm LOGGER = logging.getLogger(__name__) diff --git a/tests/storage/utils.py b/tests/storage/utils.py index 8b66c87ad3..ffd2d5e25b 100644 --- a/tests/storage/utils.py +++ b/tests/storage/utils.py @@ -122,7 +122,7 @@ def ensure_vm_running(vm, stop_vm=True): yield vm if stop_vm: vm.stop(wait=True) - + def create_windows_vm_validate_guest_agent_info( dv, diff --git a/utilities/storage.py b/utilities/storage.py index 7217f5b8e3..1fc0572f9b 100644 --- a/utilities/storage.py +++ b/utilities/storage.py @@ -649,7 +649,7 @@ def write_file_via_ssh(vm: "VirtualMachineForTests", filename: str, content: str """ cmd = shlex.split(f"echo '{content}' > {filename} && sync") - run_ssh_commands(host=vm.ssh_exec, commands=cmd) + run_ssh_commands(host=vm.ssh_exec, commands=cmd) def run_command_on_vm_and_check_output(vm, command, expected_result): From 501a6bf8bb94a174a4a61645dfac3308e490467a Mon Sep 17 00:00:00 2001 From: Kate Shvaika Date: Fri, 12 Dec 2025 17:17:56 +0100 Subject: [PATCH 4/6] Reverted the func name to origin one --- tests/storage/upgrade/test_upgrade_storage.py | 8 ++++---- utilities/storage.py | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/storage/upgrade/test_upgrade_storage.py b/tests/storage/upgrade/test_upgrade_storage.py index 5d88e6d643..19a1bf378b 100644 --- a/tests/storage/upgrade/test_upgrade_storage.py +++ b/tests/storage/upgrade/test_upgrade_storage.py @@ -16,7 +16,7 @@ from utilities.storage import ( assert_disk_serial, assert_hotplugvolume_nonexist_optional_restart, - run_command_on_vm_and_check_output, + run_command_on_cirros_vm_and_check_output, wait_for_vm_volume_ready, ) from utilities.virt import migrate_vm_and_verify @@ -72,7 +72,7 @@ def test_vm_snapshot_restore_before_upgrade( ) as vm_restore: vm_restore.wait_restore_done() cirros_vm_for_upgrade_a.start(wait=True) - run_command_on_vm_and_check_output( + run_command_on_cirros_vm_and_check_output( vm=cirros_vm_for_upgrade_a, command=LS_COMMAND, expected_result="1", @@ -147,7 +147,7 @@ def test_vm_snapshot_restore_check_after_upgrade( self, cirros_vm_for_upgrade_a, ): - run_command_on_vm_and_check_output( + run_command_on_cirros_vm_and_check_output( vm=cirros_vm_for_upgrade_a, command=LS_COMMAND, expected_result="1", @@ -173,7 +173,7 @@ def test_vm_snapshot_restore_create_after_upgrade(self, cirros_vm_for_upgrade_b, ) as vm_restore: vm_restore.wait_restore_done() cirros_vm_for_upgrade_b.start(wait=True) - run_command_on_vm_and_check_output( + run_command_on_cirros_vm_and_check_output( vm=cirros_vm_for_upgrade_b, command=LS_COMMAND, expected_result="1", diff --git a/utilities/storage.py b/utilities/storage.py index 1fc0572f9b..36a2116285 100644 --- a/utilities/storage.py +++ b/utilities/storage.py @@ -652,7 +652,7 @@ def write_file_via_ssh(vm: "VirtualMachineForTests", filename: str, content: str run_ssh_commands(host=vm.ssh_exec, commands=cmd) -def run_command_on_vm_and_check_output(vm, command, expected_result): +def run_command_on_cirros_vm_and_check_output(vm, command, expected_result): with console.Console(vm=vm) as vm_console: vm_console.sendline(command) vm_console.expect(expected_result, timeout=20) From c10cf4d287a3eb4baa2c51581801ce99fa9ef636 Mon Sep 17 00:00:00 2001 From: Kate Shvaika Date: Fri, 12 Dec 2025 18:13:02 +0100 Subject: [PATCH 5/6] handle circular imports --- utilities/storage.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/utilities/storage.py b/utilities/storage.py index 36a2116285..a0df349edd 100644 --- a/utilities/storage.py +++ b/utilities/storage.py @@ -634,7 +634,7 @@ def write_file(vm, filename, content, stop_vm=True): vm.stop(wait=True) -def write_file_via_ssh(vm: "VirtualMachineForTests", filename: str, content: str) -> None: +def write_file_via_ssh(vm: virt_util.VirtualMachineForTests, filename: str, content: str) -> None: """ Write content to a file in VM using SSH connection. From 3a9105d3dc27d23887191292692add3393fb7650 Mon Sep 17 00:00:00 2001 From: Kate Shvaika Date: Fri, 12 Dec 2025 18:27:00 +0100 Subject: [PATCH 6/6] solved coderabbitai's comments --- utilities/storage.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/utilities/storage.py b/utilities/storage.py index a0df349edd..b5ff159cb4 100644 --- a/utilities/storage.py +++ b/utilities/storage.py @@ -645,10 +645,9 @@ def write_file_via_ssh(vm: virt_util.VirtualMachineForTests, filename: str, cont Raises: TimeoutExpiredError: If SSH connectivity cannot be established - SSHException: If SSH command execution fails """ - cmd = shlex.split(f"echo '{content}' > {filename} && sync") + cmd = shlex.split(f"echo {shlex.quote(content)} > {shlex.quote(filename)} && sync") run_ssh_commands(host=vm.ssh_exec, commands=cmd)