From 31cfa72ef77279dde1138d442258ced55327ea46 Mon Sep 17 00:00:00 2001 From: Jose Manuel Castano <73908807+josemacassan@users.noreply.github.com> Date: Mon, 25 May 2026 18:28:26 +0200 Subject: [PATCH 1/4] Storage: Change Cirros images for RHEL from DataSource into upgrade tests. (#2917) Change use of Cirros images coming from artifactory to RHEL coming from DataSource into upgrade tests. `tests/storage/upgrade`. We are refactoring tests by using RHEL images from DataSource, for this I have changed all the Cirros resources into upgrade tests as well as RHEL utilities and config functions I have added a function called `write_file_via_ssh` that writes a file using `run_ssh_commands` function. It has an internal import to avoid circular import errors I was facing. Programmatically I think is not a very good practice so I appreciate some comments specially in there (perhaps `utilities/storage.py` is not the best place to locate it, and other place might avoid those errors). https://issues.redhat.com/browse/CNV-69123 --------- Signed-off-by: Jose Manuel Castano Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> --- tests/storage/upgrade/conftest.py | 69 ++++++++++++--- tests/storage/upgrade/constants.py | 9 ++ tests/storage/upgrade/test_upgrade_storage.py | 84 ++++++++++++++----- tests/storage/upgrade/utils.py | 42 +++++++--- utilities/storage.py | 8 +- 5 files changed, 160 insertions(+), 52 deletions(-) create mode 100644 tests/storage/upgrade/constants.py diff --git a/tests/storage/upgrade/conftest.py b/tests/storage/upgrade/conftest.py index 8c1936e037..9614506dc6 100644 --- a/tests/storage/upgrade/conftest.py +++ b/tests/storage/upgrade/conftest.py @@ -24,18 +24,65 @@ @pytest.fixture(scope="session") -def cirros_vm_for_upgrade_a( +def skip_if_less_than_two_storage_classes(cluster_storage_classes): + if len(cluster_storage_classes) < 2: + pytest.skip("Need two Storage Classes at least.") + + +@pytest.fixture(scope="session") +def storage_class_for_updating_cdiconfig_scratch( + skip_if_less_than_two_storage_classes, cdi_config, cluster_storage_classes +): + """ + Choose one StorageClass which is not the current one for scratch space. + """ + current_sc_for_scratch = cdi_config.scratch_space_storage_class_from_status + LOGGER.info(f"The current StorageClass for scratch space on CDIConfig is: {current_sc_for_scratch}") + for sc in cluster_storage_classes: + if sc.instance.metadata.get("name") != current_sc_for_scratch: + LOGGER.info(f"Candidate StorageClass: {sc.instance.metadata.name}") + return sc + + +@pytest.fixture(scope="session") +def override_cdiconfig_scratch_spec( + hyperconverged_resource_scope_session, + cdi_config, + storage_class_for_updating_cdiconfig_scratch, +): + """ + Change spec.scratchSpaceStorageClass to the selected StorageClass on CDIConfig. + """ + if storage_class_for_updating_cdiconfig_scratch: + new_sc = storage_class_for_updating_cdiconfig_scratch.name + + with update_scratch_space_sc( + cdi_config=cdi_config, new_sc=new_sc, hco=hyperconverged_resource_scope_session + ) as edited_cdi_config: + yield edited_cdi_config + + +@pytest.fixture(scope="session") +def skip_if_not_override_cdiconfig_scratch_space(override_cdiconfig_scratch_spec): + if not override_cdiconfig_scratch_spec: + pytest.skip("Skip test because the scratch space was not changed.") + + +@pytest.fixture(scope="session") +def rhel_vm_for_upgrade_a( upgrade_namespace_scope_session, admin_client, storage_class_for_snapshot, - cluster_common_node_cpu, + modern_cpu_for_migration, + rhel10_data_source_scope_session, ): with create_vm_for_snapshot_upgrade_tests( vm_name="snapshot-upgrade-a", namespace=upgrade_namespace_scope_session.name, client=admin_client, storage_class_for_snapshot=storage_class_for_snapshot, - cpu_model=cluster_common_node_cpu, + cpu_model=modern_cpu_for_migration, + data_source=rhel10_data_source_scope_session, ) as vm: yield vm @@ -43,25 +90,27 @@ def cirros_vm_for_upgrade_a( @pytest.fixture(scope="session") def snapshots_for_upgrade_a( admin_client, - cirros_vm_for_upgrade_a, + rhel_vm_for_upgrade_a, ): - with create_snapshot_for_upgrade(vm=cirros_vm_for_upgrade_a, client=admin_client) as snapshot: + with create_snapshot_for_upgrade(vm=rhel_vm_for_upgrade_a, client=admin_client) as snapshot: yield snapshot @pytest.fixture(scope="session") -def cirros_vm_for_upgrade_b( +def rhel_vm_for_upgrade_b( upgrade_namespace_scope_session, admin_client, storage_class_for_snapshot, - cluster_common_node_cpu, + modern_cpu_for_migration, + rhel10_data_source_scope_session, ): with create_vm_for_snapshot_upgrade_tests( vm_name="snapshot-upgrade-b", namespace=upgrade_namespace_scope_session.name, client=admin_client, storage_class_for_snapshot=storage_class_for_snapshot, - cpu_model=cluster_common_node_cpu, + cpu_model=modern_cpu_for_migration, + data_source=rhel10_data_source_scope_session, ) as vm: yield vm @@ -69,9 +118,9 @@ def cirros_vm_for_upgrade_b( @pytest.fixture(scope="session") def snapshots_for_upgrade_b( admin_client, - cirros_vm_for_upgrade_b, + rhel_vm_for_upgrade_b, ): - with create_snapshot_for_upgrade(vm=cirros_vm_for_upgrade_b, client=admin_client) as snapshot: + with create_snapshot_for_upgrade(vm=rhel_vm_for_upgrade_b, client=admin_client) as snapshot: yield snapshot diff --git a/tests/storage/upgrade/constants.py b/tests/storage/upgrade/constants.py new file mode 100644 index 0000000000..eb35caf34d --- /dev/null +++ b/tests/storage/upgrade/constants.py @@ -0,0 +1,9 @@ +""" +Upgrade storage test constants +""" + +# Upgrade snapshot test file constants +UPGRADE_FIRST_FILE_NAME = "first-file.txt" +UPGRADE_FIRST_FILE_CONTENT = "first-file" +UPGRADE_SECOND_FILE_NAME = "second-file.txt" +UPGRADE_SECOND_FILE_CONTENT = "second-file" diff --git a/tests/storage/upgrade/test_upgrade_storage.py b/tests/storage/upgrade/test_upgrade_storage.py index 75be7573f5..81095b038d 100644 --- a/tests/storage/upgrade/test_upgrade_storage.py +++ b/tests/storage/upgrade/test_upgrade_storage.py @@ -7,6 +7,11 @@ from ocp_resources.virtual_machine_restore import VirtualMachineRestore from tests.storage.utils import assert_disk_bus +from tests.storage.upgrade.constants import ( + UPGRADE_FIRST_FILE_CONTENT, + UPGRADE_FIRST_FILE_NAME, + UPGRADE_SECOND_FILE_NAME, +) from tests.upgrade_params import ( HOTPLUG_VM_AFTER_UPGRADE_NODE_ID, IUO_UPGRADE_TEST_DEPENDENCY_NODE_ID, @@ -19,10 +24,10 @@ from utilities.storage import ( assert_disk_serial, assert_hotplugvolume_nonexist, - 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 +from utilities.virt import migrate_vm_and_verify, running_vm if TYPE_CHECKING: from kubernetes.dynamic import DynamicClient @@ -52,22 +57,33 @@ def test_vm_snapshot_restore_before_upgrade( self, admin_client, skip_if_no_storage_class_for_snapshot, - cirros_vm_for_upgrade_a, + rhel_vm_for_upgrade_a, snapshots_for_upgrade_a, ): with VirtualMachineRestore( - name=f"restore-snapshot-{cirros_vm_for_upgrade_a.name}", + client=admin_client, + name=f"restore-snapshot-{rhel_vm_for_upgrade_a.name}", namespace=snapshots_for_upgrade_a.namespace, - vm_name=cirros_vm_for_upgrade_a.name, + vm_name=rhel_vm_for_upgrade_a.name, snapshot_name=snapshots_for_upgrade_a.name, client=admin_client, ) as vm_restore: + if rhel_vm_for_upgrade_a.ready: + rhel_vm_for_upgrade_a.stop(wait=True) vm_restore.wait_restore_done() - cirros_vm_for_upgrade_a.start(wait=True) - run_command_on_cirros_vm_and_check_output( - vm=cirros_vm_for_upgrade_a, - command=LS_COMMAND, - expected_result="1", + running_vm(vm=rhel_vm_for_upgrade_a) + # Verify first file exists (created before snapshot) + run_command_on_vm_and_check_output( + vm=rhel_vm_for_upgrade_a, + command=f"cat {UPGRADE_FIRST_FILE_NAME}", + expected_result=UPGRADE_FIRST_FILE_CONTENT, + ) + + # Verify second file does NOT exist (created after snapshot) + run_command_on_vm_and_check_output( + vm=rhel_vm_for_upgrade_a, + command=f"test ! -f {UPGRADE_SECOND_FILE_NAME} && echo 'file not found'", + expected_result="file not found", ) @pytest.mark.sno @@ -117,12 +133,21 @@ def test_vm_with_hotplug_before_upgrade( ) def test_vm_snapshot_restore_check_after_upgrade( self, - cirros_vm_for_upgrade_a, + rhel_vm_for_upgrade_a, ): - run_command_on_cirros_vm_and_check_output( - vm=cirros_vm_for_upgrade_a, - command=LS_COMMAND, - expected_result="1", + running_vm(vm=rhel_vm_for_upgrade_a) + # Verify first file exists (created before snapshot, should still be there after upgrade) + run_command_on_vm_and_check_output( + vm=rhel_vm_for_upgrade_a, + command=f"cat {UPGRADE_FIRST_FILE_NAME}", + expected_result=UPGRADE_FIRST_FILE_CONTENT, + ) + + # Verify second file does NOT exist (was created after snapshot, should not be present after restore) + run_command_on_vm_and_check_output( + vm=rhel_vm_for_upgrade_a, + command=f"test ! -f {UPGRADE_SECOND_FILE_NAME} && echo 'file not found'", + expected_result="file not found", ) @pytest.mark.sno @@ -137,21 +162,34 @@ def test_vm_snapshot_restore_check_after_upgrade( scope=DEPENDENCY_SCOPE_SESSION, ) def test_vm_snapshot_restore_create_after_upgrade( - self, admin_client, cirros_vm_for_upgrade_b, snapshots_for_upgrade_b + self, admin_client, rhel_vm_for_upgrade_b, snapshots_for_upgrade_b ): with VirtualMachineRestore( - name=f"restore-snapshot-{cirros_vm_for_upgrade_b.name}", + client=admin_client, + name=f"restore-snapshot-{rhel_vm_for_upgrade_b.name}", namespace=snapshots_for_upgrade_b.namespace, - vm_name=cirros_vm_for_upgrade_b.name, + vm_name=rhel_vm_for_upgrade_b.name, snapshot_name=snapshots_for_upgrade_b.name, client=admin_client, ) as vm_restore: + if rhel_vm_for_upgrade_b.ready: + rhel_vm_for_upgrade_b.stop(wait=True) vm_restore.wait_restore_done() - cirros_vm_for_upgrade_b.start(wait=True) - run_command_on_cirros_vm_and_check_output( - vm=cirros_vm_for_upgrade_b, - command=LS_COMMAND, - expected_result="1", + + running_vm(vm=rhel_vm_for_upgrade_b) + + # Verify first file exists (created before snapshot) + run_command_on_vm_and_check_output( + vm=rhel_vm_for_upgrade_b, + command=f"cat {UPGRADE_FIRST_FILE_NAME}", + expected_result=UPGRADE_FIRST_FILE_CONTENT, + ) + + # Verify second file does NOT exist (created after snapshot) + run_command_on_vm_and_check_output( + vm=rhel_vm_for_upgrade_b, + command=f"test ! -f {UPGRADE_SECOND_FILE_NAME} && echo 'file not found'", + expected_result="file not found", ) @pytest.mark.polarion("CNV-5310") diff --git a/tests/storage/upgrade/utils.py b/tests/storage/upgrade/utils.py index 1c2673c4a3..76b57032a7 100644 --- a/tests/storage/upgrade/utils.py +++ b/tests/storage/upgrade/utils.py @@ -1,9 +1,19 @@ from contextlib import contextmanager +from ocp_resources.virtual_machine import VirtualMachine +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 tests.utils import create_cirros_vm -from utilities.storage import write_file +from tests.storage.upgrade.constants import ( + UPGRADE_FIRST_FILE_CONTENT, + UPGRADE_FIRST_FILE_NAME, + UPGRADE_SECOND_FILE_CONTENT, + UPGRADE_SECOND_FILE_NAME, +) +from utilities.constants import OS_FLAVOR_RHEL, RHEL10_PREFERENCE, U1_SMALL +from utilities.storage import data_volume_template_with_source_ref_dict, write_file_via_ssh +from utilities.virt import VirtualMachineForTests, running_vm @contextmanager @@ -13,19 +23,27 @@ def create_vm_for_snapshot_upgrade_tests( client, storage_class_for_snapshot, cpu_model, + data_source, ): - with create_cirros_vm( - storage_class=storage_class_for_snapshot, + with VirtualMachineForTests( + name=f"vm-{vm_name}", namespace=namespace, client=client, - dv_name=f"dv-{vm_name}", - vm_name=f"vm-{vm_name}", + os_flavor=OS_FLAVOR_RHEL, + vm_instance_type=VirtualMachineClusterInstancetype(client=client, name=U1_SMALL), + vm_preference=VirtualMachineClusterPreference(client=client, name=RHEL10_PREFERENCE), + data_volume_template=data_volume_template_with_source_ref_dict( + data_source=data_source, + storage_class=storage_class_for_snapshot, + ), + run_strategy=VirtualMachine.RunStrategy.ALWAYS, cpu_model=cpu_model, ) as vm: - write_file( + running_vm(vm=vm) + write_file_via_ssh( vm=vm, - filename="first-file.txt", - content="first-file", + filename=UPGRADE_FIRST_FILE_NAME, + content=UPGRADE_FIRST_FILE_CONTENT, ) yield vm @@ -40,9 +58,9 @@ def create_snapshot_for_upgrade(vm, client): client=client, ) as vm_snapshot: vm_snapshot.wait_snapshot_done() - write_file( + write_file_via_ssh( vm=vm, - filename="second-file.txt", - content="second-file", + filename=UPGRADE_SECOND_FILE_NAME, + content=UPGRADE_SECOND_FILE_CONTENT, ) yield vm_snapshot diff --git a/utilities/storage.py b/utilities/storage.py index ca25c49362..9185104744 100644 --- a/utilities/storage.py +++ b/utilities/storage.py @@ -686,13 +686,7 @@ def run_command_on_vm_and_check_output( ) -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) - - -def assert_disk_serial(vm, command=_DEFAULT_DISK_SERIAL_COMMAND): +def assert_disk_serial(vm, command=shlex.split("sudo ls /dev/disk/by-id")): assert ( HOTPLUG_DISK_SERIAL in run_ssh_commands(host=vm.ssh_exec, commands=command, wait_timeout=TIMEOUT_2MIN, sleep=TIMEOUT_5SEC)[0] From 487e52c6a4cec52864b61b3eedca8992fb94871e Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Tue, 26 May 2026 09:39:56 +0000 Subject: [PATCH 2/4] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- tests/storage/upgrade/test_upgrade_storage.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/storage/upgrade/test_upgrade_storage.py b/tests/storage/upgrade/test_upgrade_storage.py index 81095b038d..094b9d76ef 100644 --- a/tests/storage/upgrade/test_upgrade_storage.py +++ b/tests/storage/upgrade/test_upgrade_storage.py @@ -20,7 +20,7 @@ SNAPSHOT_RESTORE_CREATE_AFTER_UPGRADE, STORAGE_NODE_ID_PREFIX, ) -from utilities.constants import DEPENDENCY_SCOPE_SESSION, HOTPLUG_DISK_VIRTIO_BUS, LS_COMMAND +from utilities.constants import DEPENDENCY_SCOPE_SESSION, HOTPLUG_DISK_VIRTIO_BUS from utilities.storage import ( assert_disk_serial, assert_hotplugvolume_nonexist, From 3e249fffc5331294f347aed0e25b552f46cbeb04 Mon Sep 17 00:00:00 2001 From: Jose Manuel Castano Date: Thu, 28 May 2026 09:40:31 +0200 Subject: [PATCH 3/4] Storage: Remove dead code and fix syntax errors in upgrade tests Remove unused fixtures that reference non-existent update_scratch_space_sc function (removed in fbda8ef1). Fix duplicate client keyword arguments in VirtualMachineRestore constructor calls. Signed-off-by: Jose Manuel Castano Co-Authored-By: Claude Sonnet 4.5 --- tests/storage/upgrade/conftest.py | 45 ------------------- tests/storage/upgrade/test_upgrade_storage.py | 4 +- 2 files changed, 1 insertion(+), 48 deletions(-) diff --git a/tests/storage/upgrade/conftest.py b/tests/storage/upgrade/conftest.py index 9614506dc6..e2982ba9cb 100644 --- a/tests/storage/upgrade/conftest.py +++ b/tests/storage/upgrade/conftest.py @@ -23,51 +23,6 @@ LOGGER = logging.getLogger(__name__) -@pytest.fixture(scope="session") -def skip_if_less_than_two_storage_classes(cluster_storage_classes): - if len(cluster_storage_classes) < 2: - pytest.skip("Need two Storage Classes at least.") - - -@pytest.fixture(scope="session") -def storage_class_for_updating_cdiconfig_scratch( - skip_if_less_than_two_storage_classes, cdi_config, cluster_storage_classes -): - """ - Choose one StorageClass which is not the current one for scratch space. - """ - current_sc_for_scratch = cdi_config.scratch_space_storage_class_from_status - LOGGER.info(f"The current StorageClass for scratch space on CDIConfig is: {current_sc_for_scratch}") - for sc in cluster_storage_classes: - if sc.instance.metadata.get("name") != current_sc_for_scratch: - LOGGER.info(f"Candidate StorageClass: {sc.instance.metadata.name}") - return sc - - -@pytest.fixture(scope="session") -def override_cdiconfig_scratch_spec( - hyperconverged_resource_scope_session, - cdi_config, - storage_class_for_updating_cdiconfig_scratch, -): - """ - Change spec.scratchSpaceStorageClass to the selected StorageClass on CDIConfig. - """ - if storage_class_for_updating_cdiconfig_scratch: - new_sc = storage_class_for_updating_cdiconfig_scratch.name - - with update_scratch_space_sc( - cdi_config=cdi_config, new_sc=new_sc, hco=hyperconverged_resource_scope_session - ) as edited_cdi_config: - yield edited_cdi_config - - -@pytest.fixture(scope="session") -def skip_if_not_override_cdiconfig_scratch_space(override_cdiconfig_scratch_spec): - if not override_cdiconfig_scratch_spec: - pytest.skip("Skip test because the scratch space was not changed.") - - @pytest.fixture(scope="session") def rhel_vm_for_upgrade_a( upgrade_namespace_scope_session, diff --git a/tests/storage/upgrade/test_upgrade_storage.py b/tests/storage/upgrade/test_upgrade_storage.py index 094b9d76ef..d004c1cd82 100644 --- a/tests/storage/upgrade/test_upgrade_storage.py +++ b/tests/storage/upgrade/test_upgrade_storage.py @@ -6,12 +6,12 @@ import pytest from ocp_resources.virtual_machine_restore import VirtualMachineRestore -from tests.storage.utils import assert_disk_bus from tests.storage.upgrade.constants import ( UPGRADE_FIRST_FILE_CONTENT, UPGRADE_FIRST_FILE_NAME, UPGRADE_SECOND_FILE_NAME, ) +from tests.storage.utils import assert_disk_bus from tests.upgrade_params import ( HOTPLUG_VM_AFTER_UPGRADE_NODE_ID, IUO_UPGRADE_TEST_DEPENDENCY_NODE_ID, @@ -66,7 +66,6 @@ def test_vm_snapshot_restore_before_upgrade( namespace=snapshots_for_upgrade_a.namespace, vm_name=rhel_vm_for_upgrade_a.name, snapshot_name=snapshots_for_upgrade_a.name, - client=admin_client, ) as vm_restore: if rhel_vm_for_upgrade_a.ready: rhel_vm_for_upgrade_a.stop(wait=True) @@ -170,7 +169,6 @@ def test_vm_snapshot_restore_create_after_upgrade( namespace=snapshots_for_upgrade_b.namespace, vm_name=rhel_vm_for_upgrade_b.name, snapshot_name=snapshots_for_upgrade_b.name, - client=admin_client, ) as vm_restore: if rhel_vm_for_upgrade_b.ready: rhel_vm_for_upgrade_b.stop(wait=True) From 99ec8efbb4a6dbb8a790828122105e97af2b450d Mon Sep 17 00:00:00 2001 From: Jose Manuel Castano Date: Wed, 3 Jun 2026 10:51:40 +0200 Subject: [PATCH 4/4] fix: use module constant for default disk serial command Fixes B008 ruff violation by using _DEFAULT_DISK_SERIAL_COMMAND instead of calling shlex.split() in function argument default. Co-Authored-By: Claude Sonnet 4.5 Signed-off-by: Jose Manuel Castano --- utilities/storage.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/utilities/storage.py b/utilities/storage.py index 9185104744..e7a505b862 100644 --- a/utilities/storage.py +++ b/utilities/storage.py @@ -686,7 +686,7 @@ def run_command_on_vm_and_check_output( ) -def assert_disk_serial(vm, command=shlex.split("sudo ls /dev/disk/by-id")): +def assert_disk_serial(vm, command=_DEFAULT_DISK_SERIAL_COMMAND): assert ( HOTPLUG_DISK_SERIAL in run_ssh_commands(host=vm.ssh_exec, commands=command, wait_timeout=TIMEOUT_2MIN, sleep=TIMEOUT_5SEC)[0]