From 6fef96480dd5fd76e4f77ad1b757b987ec7ff17e Mon Sep 17 00:00:00 2001 From: Jenia Peimer <86722603+jpeimer@users.noreply.github.com> Date: Mon, 6 Jul 2026 16:48:25 +0300 Subject: [PATCH 1/2] [Storage] Use source_dict for DataVolume source specification, refactor create_dv (#5443) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `DataVolume` `source` argument is deprecated and needs to be replaced by the `source_dict` Refactor `create_dv` and all `DataVolume` instantiations to use `source_dict=` with `construct_datavolume_source_dict()` instead of the legacy `source=`. **Why:** The old `create_dv` interface mixed source specification, artifactory lifecycle, and annotation handling into a single flat parameter list. This made it unclear which parameters applied to which source types and coupled every caller to artifactory internals. **Changes:** - Extract `construct_datavolume_source_dict()` as a pure function for building DataVolume source dicts from typed parameters (`http`, `registry`, `pvc`, `blank`, `upload`) - Refactor `create_dv` to accept `source_dict=` directly, own artifactory lifecycle in `try/finally`, use generic `annotations` dict instead of `multus_annotation`/`bind_immediate`, and add full type hints - Rename parameters for consistency: `secret` → `secret_name`, `cert_configmap` → `cert_configmap_name`, `source_pvc` → `source_pvc_name`, `source_namespace` → `source_pvc_namespace` - Remove unused `hostpath_node` parameter - Migrate all 36 caller files to the new interface ##### Which issue(s) this PR fixes: ##### Special notes for reviewer: - The two commits are stacked: first refactors `create_dv` and updates callers using `create_dv`, second migrates direct `DataVolume()` instantiations to use `construct_datavolume_source_dict()` - No behavioral changes — all callers produce the same `DataVolume` specs as before - This change is needed as preparation for specifying the `architecture` in DV spec. * **New Features** * Added a shared helper to standardize DataVolume `source` setup for HTTP, registry, blank, PVC, and upload scenarios. * Standardized bind-immediate behavior using DataVolume annotations. * **Bug Fixes** * Corrected TLS/auth wiring by consistently using named secret and cert config-map references. * Improved Artifactory-backed image handling by enabling Artifactory mode where applicable. * Updated source PVC identification to use explicit name/namespace fields. * **Tests** * Refreshed many fixtures and helpers to use the unified `source_dict` flow and new parameter names. * Added unit tests validating the new source configuration helper, including URL validation behavior. --------- Signed-off-by: Jenia Peimer Co-authored-by: Claude Opus 4.6 --- tests/chaos/conftest.py | 11 +- tests/chaos/snapshot/conftest.py | 11 +- tests/conftest.py | 14 +- tests/data_protection/oadp/conftest.py | 20 +- .../test_ssp_common_templates_boot_sources.py | 15 +- .../test_ssp_data_sources.py | 16 +- .../infrastructure/instance_types/conftest.py | 15 +- .../instance_types/supported_os/conftest.py | 12 +- tests/observability/metrics/utils.py | 11 +- tests/scale/test_scale_benchmark.py | 12 +- tests/storage/cdi_clone/test_clone.py | 13 +- tests/storage/cdi_import/conftest.py | 14 +- tests/storage/cdi_import/test_import_http.py | 6 +- .../cross_cluster_live_migration/conftest.py | 11 +- .../storage/golden_image/test_golden_image.py | 4 +- tests/storage/hpp/utils.py | 2 + tests/storage/online_resize/conftest.py | 3 +- .../online_resize/test_online_resize.py | 6 +- .../restricted_namespace_cloning/conftest.py | 18 +- .../test_restricted_namespace_cloning.py | 12 +- .../restricted_namespace_cloning/utils.py | 8 +- tests/storage/storage_migration/conftest.py | 23 +- tests/storage/test_cdi_certificate.py | 3 +- tests/storage/test_cdi_resources.py | 3 +- tests/storage/test_priority_class.py | 7 +- tests/storage/test_wffc.py | 3 +- tests/storage/utils.py | 13 +- tests/storage/vm_export/utils.py | 4 +- tests/utils.py | 21 +- tests/virt/cluster/longevity_tests/utils.py | 12 +- .../cluster/vm_cloning/test_vm_cloning.py | 3 +- tests/virt/upgrade/conftest.py | 2 + tests/virt/upgrade_custom/vgpu/conftest.py | 2 + tests/virt/utils.py | 2 + utilities/oadp.py | 15 +- utilities/storage.py | 249 +++++++++++++----- utilities/unittests/conftest.py | 7 +- utilities/unittests/test_storage.py | 149 +++++++++++ 38 files changed, 548 insertions(+), 204 deletions(-) create mode 100644 utilities/unittests/test_storage.py diff --git a/tests/chaos/conftest.py b/tests/chaos/conftest.py index 5a2da68652..f112c90a63 100644 --- a/tests/chaos/conftest.py +++ b/tests/chaos/conftest.py @@ -43,6 +43,7 @@ utility_daemonset_for_custom_tests, wait_for_node_status, ) +from utilities.storage import construct_datavolume_source_dict from utilities.virt import VirtualMachineForTests, running_vm LOGGER = logging.getLogger(__name__) @@ -93,16 +94,18 @@ def chaos_dv_rhel9( artifactory_config_map_chaos_namespace_scope_module, ): yield DataVolume( - source="http", + source_dict=construct_datavolume_source_dict( + source="http", + url=rhel9_http_image_url, + secret_name=artifactory_secret_chaos_namespace_scope_module.name, + cert_configmap_name=artifactory_config_map_chaos_namespace_scope_module.name, + ), name="chaos-dv", api_name="storage", namespace=chaos_namespace.name, - url=rhel9_http_image_url, size=Images.Rhel.DEFAULT_DV_SIZE, storage_class=request.param["storage_class"], client=admin_client, - secret=artifactory_secret_chaos_namespace_scope_module, - cert_configmap=artifactory_config_map_chaos_namespace_scope_module.name, ) diff --git a/tests/chaos/snapshot/conftest.py b/tests/chaos/snapshot/conftest.py index 25203959b3..76fb03c841 100644 --- a/tests/chaos/snapshot/conftest.py +++ b/tests/chaos/snapshot/conftest.py @@ -3,6 +3,7 @@ from tests.chaos.snapshot.utils import VirtualMachineSnapshotWithDeadline from utilities.constants import OS_FLAVOR_RHEL, TIMEOUT_8MIN, TIMEOUT_10MIN, Images +from utilities.storage import construct_datavolume_source_dict from utilities.virt import VirtualMachineForTests @@ -16,16 +17,18 @@ def chaos_dv_rhel9_for_snapshot( artifactory_config_map_chaos_namespace_scope_module, ): yield DataVolume( - source="http", + source_dict=construct_datavolume_source_dict( + source="http", + url=rhel9_http_image_url, + secret_name=artifactory_secret_chaos_namespace_scope_module.name, + cert_configmap_name=artifactory_config_map_chaos_namespace_scope_module.name, + ), name="chaos-dv", api_name="storage", namespace=chaos_namespace.name, - url=rhel9_http_image_url, size=Images.Rhel.DEFAULT_DV_SIZE, storage_class=[*storage_class_matrix_snapshot_matrix__function__][0], client=admin_client, - secret=artifactory_secret_chaos_namespace_scope_module, - cert_configmap=artifactory_config_map_chaos_namespace_scope_module.name, ) diff --git a/tests/conftest.py b/tests/conftest.py index d58008a0b5..d33b0cbe92 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -105,6 +105,7 @@ OC_ADM_LOGS_COMMAND, OS_FLAVOR_RHEL, OVS_BRIDGE, + BIND_IMMEDIATE_ANNOTATION, POD_SECURITY_NAMESPACE_LABELS, PREFERENCE_STR, RHEL9_STR, @@ -181,6 +182,7 @@ from utilities.sanity import cluster_sanity from utilities.ssp import get_data_import_crons, get_ssp_resource from utilities.storage import ( + construct_datavolume_source_dict, create_or_update_data_source, data_volume, get_default_storage_class, @@ -2483,13 +2485,15 @@ def dvs_for_upgrade( client=admin_client, name=f"dv-for-product-upgrade-{storage_class}", namespace=golden_images_namespace_name, - source="http", + source_dict=construct_datavolume_source_dict( + source="http", + url=rhel_latest_os_params["rhel_image_path"], + secret_name=artifactory_secret.name, + cert_configmap_name=artifactory_config_map.name, + ), storage_class=storage_class, - secret=artifactory_secret, - cert_configmap=artifactory_config_map.name, - url=rhel_latest_os_params["rhel_image_path"], size=rhel_latest_os_params["rhel_dv_size"], - bind_immediate_annotation=True, + annotations=BIND_IMMEDIATE_ANNOTATION, api_name="storage", ) dv.create() diff --git a/tests/data_protection/oadp/conftest.py b/tests/data_protection/oadp/conftest.py index 7f0aa27cee..acef2f8394 100644 --- a/tests/data_protection/oadp/conftest.py +++ b/tests/data_protection/oadp/conftest.py @@ -35,6 +35,7 @@ ) from utilities.storage import ( check_upload_virtctl_result, + construct_datavolume_source_dict, create_dv, create_vm_from_dv, get_downloaded_artifact, @@ -54,10 +55,12 @@ def imported_dv_in_progress_second_namespace( with create_dv( dv_name="imported-dv", namespace=namespace_for_backup2.name, + source="http", url=rhel9_http_image_url, size=Images.Rhel.DEFAULT_DV_SIZE, storage_class=storage_class_for_snapshot, client=namespace_for_backup2.client, + use_artifactory=True, ) as dv: yield dv @@ -171,16 +174,18 @@ def windows_vm_with_data_volume_template( name="oadp-windows-dv", namespace=namespace_for_backup.name, storage_class=snapshot_storage_class_name_scope_module, - source="registry", - url=( - f"{get_test_artifact_server_url(schema='registry')}/" - f"{py_config['latest_windows_os_dict'][CONTAINER_DISK_IMAGE_PATH_STR]}" + source_dict=construct_datavolume_source_dict( + source="registry", + url=( + f"{get_test_artifact_server_url(schema='registry')}/" + f"{py_config['latest_windows_os_dict'][CONTAINER_DISK_IMAGE_PATH_STR]}" + ), + secret_name=artifactory_secret.name, + cert_configmap_name=artifactory_config_map.name, ), size=Images.Windows.CONTAINER_DISK_DV_SIZE, client=admin_client, api_name="storage", - secret=artifactory_secret, - cert_configmap=artifactory_config_map.name, ) dv.to_dict() @@ -305,7 +310,8 @@ def cloned_rhel_dv(imported_dv_second_namespace): dv_name="cloned-dv", namespace=imported_dv_second_namespace.namespace, size=imported_dv_second_namespace.size, - source_pvc=imported_dv_second_namespace.name, + source_pvc_name=imported_dv_second_namespace.name, + source_pvc_namespace=imported_dv_second_namespace.namespace, storage_class=imported_dv_second_namespace.storage_class, client=imported_dv_second_namespace.client, ) as cdv: diff --git a/tests/infrastructure/golden_images/update_boot_source/test_ssp_common_templates_boot_sources.py b/tests/infrastructure/golden_images/update_boot_source/test_ssp_common_templates_boot_sources.py index 9246799097..3a9032c80d 100644 --- a/tests/infrastructure/golden_images/update_boot_source/test_ssp_common_templates_boot_sources.py +++ b/tests/infrastructure/golden_images/update_boot_source/test_ssp_common_templates_boot_sources.py @@ -11,10 +11,18 @@ assert_missing_golden_image_pvc, assert_os_version_mismatch_in_vm, ) -from utilities.constants import DEFAULT_FEDORA_REGISTRY_URL, OS_FLAVOR_FEDORA, TIMEOUT_5MIN, TIMEOUT_5SEC, Images +from utilities.constants import ( + BIND_IMMEDIATE_ANNOTATION, + DEFAULT_FEDORA_REGISTRY_URL, + OS_FLAVOR_FEDORA, + TIMEOUT_5MIN, + TIMEOUT_5SEC, + Images, +) from utilities.infra import ( validate_os_info_vmi_vs_linux_os, ) +from utilities.storage import construct_datavolume_source_dict from utilities.virt import VirtualMachineForTestsFromTemplate, running_vm LOGGER = logging.getLogger(__name__) @@ -111,11 +119,10 @@ def imported_fedora_dv(admin_client, golden_images_namespace, fedora_data_source name=fedora_data_source.name, namespace=golden_images_namespace.name, api_name="storage", - source="registry", + source_dict=construct_datavolume_source_dict(source="registry", url=DEFAULT_FEDORA_REGISTRY_URL), size=Images.Fedora.DEFAULT_DV_SIZE, storage_class=py_config["default_storage_class"], - url=DEFAULT_FEDORA_REGISTRY_URL, - bind_immediate_annotation=True, + annotations=BIND_IMMEDIATE_ANNOTATION, ) as dv: dv.wait_for_dv_success() yield dv diff --git a/tests/infrastructure/golden_images/update_boot_source/test_ssp_data_sources.py b/tests/infrastructure/golden_images/update_boot_source/test_ssp_data_sources.py index 2ba5cff507..d7debbe670 100644 --- a/tests/infrastructure/golden_images/update_boot_source/test_ssp_data_sources.py +++ b/tests/infrastructure/golden_images/update_boot_source/test_ssp_data_sources.py @@ -24,6 +24,7 @@ get_http_image_url, ) from utilities.constants import ( + BIND_IMMEDIATE_ANNOTATION, DATA_SOURCE_NAME, DEFAULT_FEDORA_REGISTRY_URL, QUARANTINED, @@ -33,6 +34,7 @@ ) from utilities.exceptions import ResourceValueError from utilities.ssp import wait_for_condition_message_value +from utilities.storage import construct_datavolume_source_dict LOGGER = logging.getLogger(__name__) @@ -51,14 +53,16 @@ def dv_for_data_source(name, data_source, admin_client): client=admin_client, name=name, namespace=data_source.namespace, - # underlying OS is not relevant - url=get_http_image_url(image_directory=Images.Cirros.DIR, image_name=Images.Cirros.QCOW2_IMG), - source="http", - secret=artifactory_secret, - cert_configmap=artifactory_config_map.name, + source_dict=construct_datavolume_source_dict( + # underlying OS is not relevant + source="http", + url=get_http_image_url(image_directory=Images.Cirros.DIR, image_name=Images.Cirros.QCOW2_IMG), + secret_name=artifactory_secret.name, + cert_configmap_name=artifactory_config_map.name, + ), size=Images.Cirros.DEFAULT_DV_SIZE, storage_class=py_config["default_storage_class"], - bind_immediate_annotation=True, + annotations=BIND_IMMEDIATE_ANNOTATION, api_name="storage", ) as dv: dv.wait_for_dv_success() diff --git a/tests/infrastructure/instance_types/conftest.py b/tests/infrastructure/instance_types/conftest.py index 133974c36c..59c2d9cc72 100644 --- a/tests/infrastructure/instance_types/conftest.py +++ b/tests/infrastructure/instance_types/conftest.py @@ -27,6 +27,7 @@ Images, ) from utilities.storage import ( + construct_datavolume_source_dict, create_dummy_first_consumer_pod, data_volume_template_with_source_ref_dict, generate_data_source_dict, @@ -124,13 +125,17 @@ def latest_windows_data_volume( name="latest-windows", namespace=namespace.name, api_name="storage", - source="registry", + source_dict=construct_datavolume_source_dict( + source="registry", + url=( + f"{get_test_artifact_server_url(schema='registry')}/" + f"{py_config['latest_windows_os_dict'][CONTAINER_DISK_IMAGE_PATH_STR]}" + ), + secret_name=secret.name, + cert_configmap_name=cert.name, + ), size=Images.Windows.CONTAINER_DISK_DV_SIZE, storage_class=default_sc.name, - url=f"{get_test_artifact_server_url(schema='registry')}/" - f"{py_config['latest_windows_os_dict'][CONTAINER_DISK_IMAGE_PATH_STR]}", - secret=secret, - cert_configmap=cert.name, ) as win_dv: if sc_volume_binding_mode_is_wffc(sc=default_sc.name, client=win_dv.client): create_dummy_first_consumer_pod(pvc=win_dv.pvc) diff --git a/tests/infrastructure/instance_types/supported_os/conftest.py b/tests/infrastructure/instance_types/supported_os/conftest.py index fbc6a5a05e..d5462d84ee 100644 --- a/tests/infrastructure/instance_types/supported_os/conftest.py +++ b/tests/infrastructure/instance_types/supported_os/conftest.py @@ -18,7 +18,7 @@ RHEL8_PREFERENCE, Images, ) -from utilities.storage import get_test_artifact_server_url +from utilities.storage import construct_datavolume_source_dict, get_test_artifact_server_url from utilities.virt import VirtualMachineForTests @@ -103,12 +103,14 @@ def windows_data_volume_template( name=f"{os_matrix_key}-dv", namespace=namespace.name, api_name="storage", - source="registry", + source_dict=construct_datavolume_source_dict( + source="registry", + url=f"{get_test_artifact_server_url(schema='registry')}/{os_params[CONTAINER_DISK_IMAGE_PATH_STR]}", + secret_name=secret.name, + cert_configmap_name=cert.name, + ), size=Images.Windows.CONTAINER_DISK_DV_SIZE, storage_class=py_config["default_storage_class"], - url=f"{get_test_artifact_server_url(schema='registry')}/{os_params[CONTAINER_DISK_IMAGE_PATH_STR]}", - secret=secret, - cert_configmap=cert.name, ) win_dv.to_dict() yield win_dv diff --git a/tests/observability/metrics/utils.py b/tests/observability/metrics/utils.py index 7f8b1c9a6a..3f17595fd9 100644 --- a/tests/observability/metrics/utils.py +++ b/tests/observability/metrics/utils.py @@ -54,6 +54,7 @@ Images, ) from utilities.monitoring import get_metrics_value +from utilities.storage import construct_datavolume_source_dict from utilities.virt import VirtualMachineForTests, running_vm LOGGER = logging.getLogger(__name__) @@ -657,12 +658,14 @@ def create_windows11_wsl2_vm( name=dv_name, namespace=namespace, api_name="storage", - source=REGISTRY_STR, + source_dict=construct_datavolume_source_dict( + source=REGISTRY_STR, + url=f"{get_test_artifact_server_url(schema=REGISTRY_STR)}/docker-local/windows-qe/win_11:virtio", + secret_name=artifactory_secret.name, + cert_configmap_name=artifactory_config_map.name, + ), size=Images.Windows.CONTAINER_DISK_DV_SIZE, storage_class=storage_class, - url=f"{get_test_artifact_server_url(schema=REGISTRY_STR)}/docker-local/windows-qe/win_11:virtio", - secret=artifactory_secret, - cert_configmap=artifactory_config_map.name, ) dv.to_dict() base_preference = VirtualMachineClusterPreference(client=client, name=windows_preference_name) diff --git a/tests/scale/test_scale_benchmark.py b/tests/scale/test_scale_benchmark.py index 2dfd6a2ff8..0cafe1da26 100644 --- a/tests/scale/test_scale_benchmark.py +++ b/tests/scale/test_scale_benchmark.py @@ -44,7 +44,7 @@ create_ns, ) from utilities.must_gather import run_must_gather -from utilities.storage import generate_data_source_dict, get_test_artifact_server_url +from utilities.storage import construct_datavolume_source_dict, generate_data_source_dict, get_test_artifact_server_url from utilities.virt import ( VirtualMachineForTestsFromTemplate, verify_vm_migrated, @@ -272,12 +272,14 @@ def _delete_resources(): namespace=golden_images_namespace.name, storage_class=SCALE_STORAGE_TYPES[storage_type], api_name="storage", - url=f"{get_test_artifact_server_url()}{dv_info['url']}", + source_dict=construct_datavolume_source_dict( + source="http", + url=f"{get_test_artifact_server_url()}{dv_info['url']}", + secret_name=artifactory_secret.name, + cert_configmap_name=artifactory_config_map.name, + ), size=dv_info["size"], client=admin_client, - source="http", - secret=artifactory_secret, - cert_configmap=artifactory_config_map.name, ) golden_images_scale_dv.deploy() dvs_list.append(golden_images_scale_dv) diff --git a/tests/storage/cdi_clone/test_clone.py b/tests/storage/cdi_clone/test_clone.py index a6667224e9..1c01170568 100644 --- a/tests/storage/cdi_clone/test_clone.py +++ b/tests/storage/cdi_clone/test_clone.py @@ -89,7 +89,8 @@ def test_successful_clone_of_large_image( dv_name="dv-target", namespace=namespace.name, size=data_volume_multi_storage_scope_function.size, - source_pvc=data_volume_multi_storage_scope_function.name, + source_pvc_name=data_volume_multi_storage_scope_function.name, + source_pvc_namespace=data_volume_multi_storage_scope_function.namespace, storage_class=data_volume_multi_storage_scope_function.storage_class, client=namespace.client, ) as cdv: @@ -177,7 +178,8 @@ def test_successful_vm_from_cloned_dv_windows( dv_name="dv-target", namespace=data_volume_multi_storage_scope_function.namespace, size=data_volume_multi_storage_scope_function.size, - source_pvc=data_volume_multi_storage_scope_function.name, + source_pvc_name=data_volume_multi_storage_scope_function.name, + source_pvc_namespace=data_volume_multi_storage_scope_function.namespace, storage_class=data_volume_multi_storage_scope_function.storage_class, ) as cdv: cdv.wait_for_dv_success(timeout=WINDOWS_CLONE_TIMEOUT) @@ -202,7 +204,7 @@ def test_successful_vm_from_cloned_dv_windows( ), pytest.param( { - "dv_name": "dv-source-win", + "dv_name": f"dv-source-{OS_FLAVOR_WINDOWS}", "image": f"{Images.Windows.DIR}/{Images.Windows.WIN11_IMG}", "dv_size": Images.Windows.DEFAULT_DV_SIZE, }, @@ -224,11 +226,12 @@ def test_successful_snapshot_clone( dv_name="dv-target", namespace=namespace, size=data_volume_snapshot_capable_storage_scope_function.size, - source_pvc=data_volume_snapshot_capable_storage_scope_function.name, + source_pvc_name=data_volume_snapshot_capable_storage_scope_function.name, + source_pvc_namespace=data_volume_snapshot_capable_storage_scope_function.namespace, storage_class=storage_class, ) as cdv: cdv.wait_for_dv_success() - if OS_FLAVOR_WINDOWS not in data_volume_snapshot_capable_storage_scope_function.url.split("/")[-1]: + if OS_FLAVOR_WINDOWS not in data_volume_snapshot_capable_storage_scope_function.name: with create_vm_from_dv( client=unprivileged_client, dv=cdv, diff --git a/tests/storage/cdi_import/conftest.py b/tests/storage/cdi_import/conftest.py index 54aba85672..4106f3d9db 100644 --- a/tests/storage/cdi_import/conftest.py +++ b/tests/storage/cdi_import/conftest.py @@ -7,6 +7,7 @@ import pytest from ocp_resources.datavolume import DataVolume +from ocp_resources.resource import Resource from pytest_testconfig import py_config from tests.storage.cdi_import.utils import wait_dv_and_get_importer, wait_for_multus_network_status @@ -27,7 +28,7 @@ ) from utilities.infra import NON_EXIST_URL from utilities.network import network_device, network_nad -from utilities.storage import create_dv, sc_volume_binding_mode_is_wffc +from utilities.storage import construct_datavolume_source_dict, create_dv, sc_volume_binding_mode_is_wffc from utilities.virt import VirtualMachineForTests LOGGER = logging.getLogger(__name__) @@ -66,6 +67,7 @@ def dv_non_exist_url(namespace, storage_class_name_scope_module): with create_dv( dv_name=f"cnv-876-{storage_class_name_scope_module}", namespace=namespace.name, + source="http", url=NON_EXIST_URL, size=DEFAULT_DV_SIZE, storage_class=storage_class_name_scope_module, @@ -85,12 +87,13 @@ def dv_from_http_import( with create_dv( dv_name=f"{request.param.get('dv_name', 'http-dv')}-{storage_class_name_scope_module}", namespace=namespace.name, + source="http", url=get_file_url( url=images_internal_http_server[request.param.get("source", HTTP)], file_name=request.param["file_name"], ), content_type=request.param.get("content_type", DataVolume.ContentType.KUBEVIRT), - cert_configmap=request.param.get("configmap_name"), + cert_configmap_name=request.param.get("configmap_name"), size=request.param.get("size", DEFAULT_DV_SIZE), storage_class=storage_class_name_scope_module, client=namespace.client, @@ -122,7 +125,7 @@ def created_blank_dv_list(unprivileged_client, namespace, storage_class_name_sco for dv_index in range(number_of_dvs): dv = DataVolume( client=unprivileged_client, - source="blank", + source_dict=construct_datavolume_source_dict(source="blank"), name=f"dv-{dv_index}", namespace=namespace.name, size=Images.Fedora.DEFAULT_DV_SIZE, @@ -176,10 +179,9 @@ def dvs_and_vms_from_public_registry(unprivileged_client, namespace, storage_cla for name in ("dv1", "dv2", "dv3"): dv = DataVolume( client=unprivileged_client, - source=REGISTRY_STR, + source_dict=construct_datavolume_source_dict(source=REGISTRY_STR, url=QUAY_FEDORA_CONTAINER_IMAGE), name=f"import-public-registry-quay-{name}", namespace=namespace.name, - url=QUAY_FEDORA_CONTAINER_IMAGE, size=Images.Fedora.DEFAULT_DV_SIZE, storage_class=storage_class_name_scope_function, api_name="storage", @@ -219,7 +221,7 @@ def importer_pod_annotations(admin_client, namespace, linux_nad): url=QUAY_FEDORA_CONTAINER_IMAGE, size=Images.Fedora.DEFAULT_DV_SIZE, storage_class=py_config["default_storage_class"], - multus_annotation=linux_nad.name, + annotations={f"{Resource.ApiGroup.K8S_V1_CNI_CNCF_IO}/networks": linux_nad.name}, client=namespace.client, ) as dv: importer_pod = wait_dv_and_get_importer(dv=dv, admin_client=admin_client) diff --git a/tests/storage/cdi_import/test_import_http.py b/tests/storage/cdi_import/test_import_http.py index e23c3585b8..9d5fe5244e 100644 --- a/tests/storage/cdi_import/test_import_http.py +++ b/tests/storage/cdi_import/test_import_http.py @@ -79,6 +79,7 @@ def test_empty_url(namespace, storage_class_name_scope_module, unprivileged_clie client=unprivileged_client, dv_name=f"cnv-674-{storage_class_name_scope_module}", namespace=namespace.name, + source="http", url="", size=DEFAULT_DV_SIZE, storage_class=storage_class_name_scope_module, @@ -188,10 +189,11 @@ def test_successful_import_basic_auth( client=admin_client, dv_name="import-http-dv", namespace=namespace.name, + source="http", url=get_file_url(url=images_internal_http_server["http_auth"], file_name=file_name), content_type=content_type, size=DEFAULT_DV_SIZE, - secret=internal_http_secret, + secret_name=internal_http_secret.name, storage_class=storage_class_name_scope_module, ) as dv: dv.wait_for_dv_success() @@ -274,7 +276,7 @@ def test_certconfigmap_incorrect_cert( "source": HTTP, "image": ALPINE_QCOW2_IMG, "dv_size": DEFAULT_DV_SIZE, - "cert_configmap": "wrong_name", + "cert_configmap_name": "wrong_name", "wait": False, }, marks=pytest.mark.polarion("CNV-2815"), diff --git a/tests/storage/cross_cluster_live_migration/conftest.py b/tests/storage/cross_cluster_live_migration/conftest.py index e62305a044..90e3a8f1cf 100644 --- a/tests/storage/cross_cluster_live_migration/conftest.py +++ b/tests/storage/cross_cluster_live_migration/conftest.py @@ -52,6 +52,7 @@ ) from utilities.infra import create_ns, get_hyperconverged_resource from utilities.storage import ( + construct_datavolume_source_dict, data_volume_template_with_source_ref_dict, write_file, ) @@ -497,12 +498,14 @@ def vm_for_cclm_windows_with_instance_type( name="dv-windows", namespace=remote_cluster_source_test_namespace.name, api_name="storage", - source="registry", + source_dict=construct_datavolume_source_dict( + source="registry", + url=f"{get_test_artifact_server_url(schema='registry')}/{WINDOWS_2022[CONTAINER_DISK_IMAGE_PATH_STR]}", + secret_name=remote_cluster_artifactory_secret_scope_class.name, + cert_configmap_name=remote_cluster_artifactory_config_map_scope_class.name, + ), size=Images.Windows.CONTAINER_DISK_DV_SIZE, storage_class=py_config["default_storage_class"], - url=f"{get_test_artifact_server_url(schema='registry')}/{WINDOWS_2022[CONTAINER_DISK_IMAGE_PATH_STR]}", - secret=remote_cluster_artifactory_secret_scope_class, - cert_configmap=remote_cluster_artifactory_config_map_scope_class.name, ) dv.to_dict() dv.res["metadata"].pop("namespace", None) diff --git a/tests/storage/golden_image/test_golden_image.py b/tests/storage/golden_image/test_golden_image.py index 251bc89092..743ace9bed 100644 --- a/tests/storage/golden_image/test_golden_image.py +++ b/tests/storage/golden_image/test_golden_image.py @@ -157,8 +157,8 @@ def test_regular_user_cant_clone_dv_in_ns( namespace=golden_images_namespace, source=PVC, size=golden_image_data_volume_scope_module.size, - source_pvc=golden_image_data_volume_scope_module.pvc.name, - source_namespace=golden_images_namespace, + source_pvc_name=golden_image_data_volume_scope_module.pvc.name, + source_pvc_namespace=golden_images_namespace, client=unprivileged_client, storage_class=storage_class, ): diff --git a/tests/storage/hpp/utils.py b/tests/storage/hpp/utils.py index d16ea9b249..a4bef9c8c9 100644 --- a/tests/storage/hpp/utils.py +++ b/tests/storage/hpp/utils.py @@ -113,10 +113,12 @@ def cirros_dv_on_hpp(dv_name, storage_class, namespace): with create_dv( dv_name=dv_name, namespace=namespace.name, + source="http", url=get_http_image_url(image_directory=Images.Cirros.DIR, image_name=Images.Cirros.QCOW2_IMG), size=Images.Cirros.DEFAULT_DV_SIZE, storage_class=storage_class, client=namespace.client, + use_artifactory=True, ) as dv: yield dv diff --git a/tests/storage/online_resize/conftest.py b/tests/storage/online_resize/conftest.py index d726e1e83b..4173acc0ec 100644 --- a/tests/storage/online_resize/conftest.py +++ b/tests/storage/online_resize/conftest.py @@ -68,7 +68,8 @@ def second_rhel_dv_for_online_resize(rhel_dv_for_online_resize, unprivileged_cli client=unprivileged_client, size=rhel_dv_for_online_resize.size, storage_class=rhel_dv_for_online_resize.storage_class, - source_pvc=rhel_dv_for_online_resize.name, + source_pvc_name=rhel_dv_for_online_resize.name, + source_pvc_namespace=rhel_dv_for_online_resize.namespace, ) as rhel_dv: yield rhel_dv diff --git a/tests/storage/online_resize/test_online_resize.py b/tests/storage/online_resize/test_online_resize.py index e792dea4e7..a4a0b54412 100644 --- a/tests/storage/online_resize/test_online_resize.py +++ b/tests/storage/online_resize/test_online_resize.py @@ -97,7 +97,8 @@ def test_disk_expand_then_clone_fail( client=unprivileged_client, size=RHEL_DV_SIZE, storage_class=rhel_dv_for_online_resize.storage_class, - source_pvc=rhel_dv_for_online_resize.name, + source_pvc_name=rhel_dv_for_online_resize.name, + source_pvc_namespace=rhel_dv_for_online_resize.namespace, ) as dv: for sample in TimeoutSampler( wait_timeout=TIMEOUT_1MIN, @@ -142,7 +143,8 @@ def test_disk_expand_then_clone_success( client=unprivileged_client, size=rhel_dv_for_online_resize.pvc.instance.spec.resources.requests.storage, storage_class=rhel_dv_for_online_resize.storage_class, - source_pvc=rhel_dv_for_online_resize.name, + source_pvc_name=rhel_dv_for_online_resize.name, + source_pvc_namespace=rhel_dv_for_online_resize.namespace, ) as cdv: cdv.wait_for_condition( condition=DataVolume.Condition.Type.READY, diff --git a/tests/storage/restricted_namespace_cloning/conftest.py b/tests/storage/restricted_namespace_cloning/conftest.py index 2c32aed135..37d7994250 100644 --- a/tests/storage/restricted_namespace_cloning/conftest.py +++ b/tests/storage/restricted_namespace_cloning/conftest.py @@ -32,7 +32,7 @@ ) from utilities.constants import OS_FLAVOR_FEDORA, PVC, UNPRIVILEGED_USER, Images from utilities.infra import create_ns -from utilities.storage import create_dv, get_dv_size_from_datasource +from utilities.storage import construct_datavolume_source_dict, create_dv, get_dv_size_from_datasource from utilities.virt import VirtualMachineForTests, running_vm @@ -95,10 +95,12 @@ def data_volume_clone_settings(destination_namespace, dv_cloned_from_datasource) dv = DataVolume( name=f"{TARGET_DV}-{storage_class}", namespace=destination_namespace.name, - source=PVC, + source_dict=construct_datavolume_source_dict( + source=PVC, + source_pvc_name=dv_cloned_from_datasource.name, + source_pvc_namespace=dv_cloned_from_datasource.namespace, + ), size=dv_cloned_from_datasource.size, - source_pvc=dv_cloned_from_datasource.name, - source_namespace=dv_cloned_from_datasource.namespace, storage_class=storage_class, api_name="storage", ) @@ -281,8 +283,8 @@ def dv_cloned_by_unprivileged_user_in_the_same_namespace( namespace=namespace, source=PVC, size=dv_cloned_from_datasource.size, - source_pvc=dv_cloned_from_datasource.pvc.name, - source_namespace=namespace, + source_pvc_name=dv_cloned_from_datasource.pvc.name, + source_pvc_namespace=namespace, client=unprivileged_client, storage_class=storage_class_name_scope_module, ) as cdv: @@ -304,8 +306,8 @@ def dv_destination_cloned_from_pvc( namespace=destination_namespace.name, source=PVC, size=dv_cloned_from_datasource.size, - source_pvc=dv_cloned_from_datasource.pvc.name, - source_namespace=dv_cloned_from_datasource.namespace, + source_pvc_name=dv_cloned_from_datasource.pvc.name, + source_pvc_namespace=dv_cloned_from_datasource.namespace, client=unprivileged_client, storage_class=storage_class_name_scope_module, ) as cdv: diff --git a/tests/storage/restricted_namespace_cloning/test_restricted_namespace_cloning.py b/tests/storage/restricted_namespace_cloning/test_restricted_namespace_cloning.py index d1d009b6f4..e56aaa12ac 100644 --- a/tests/storage/restricted_namespace_cloning/test_restricted_namespace_cloning.py +++ b/tests/storage/restricted_namespace_cloning/test_restricted_namespace_cloning.py @@ -141,8 +141,8 @@ def test_user_permissions_negative( namespace=destination_namespace.name, storage_class=storage_class_name_scope_module, size=dv_cloned_from_datasource.size, - source_pvc=dv_cloned_from_datasource.pvc.name, - source_namespace=dv_cloned_from_datasource.namespace, + source_pvc_name=dv_cloned_from_datasource.pvc.name, + source_pvc_namespace=dv_cloned_from_datasource.namespace, unprivileged_client=unprivileged_client, ) @@ -170,8 +170,8 @@ def test_unprivileged_user_clone_same_namespace_negative( namespace=namespace.name, storage_class=storage_class_name_scope_module, size=dv_cloned_from_datasource.size, - source_pvc=dv_cloned_from_datasource.pvc.name, - source_namespace=dv_cloned_from_datasource.namespace, + source_pvc_name=dv_cloned_from_datasource.pvc.name, + source_pvc_namespace=dv_cloned_from_datasource.namespace, unprivileged_client=unprivileged_client, ) @@ -202,7 +202,7 @@ def test_user_permissions_only_for_dst_ns_negative( namespace=destination_namespace.name, storage_class=storage_class_name_scope_module, size=dv_cloned_from_datasource.size, - source_pvc=dv_cloned_from_datasource.pvc.name, - source_namespace=dv_cloned_from_datasource.namespace, + source_pvc_name=dv_cloned_from_datasource.pvc.name, + source_pvc_namespace=dv_cloned_from_datasource.namespace, unprivileged_client=unprivileged_client, ) diff --git a/tests/storage/restricted_namespace_cloning/utils.py b/tests/storage/restricted_namespace_cloning/utils.py index aca9da6a6a..900262debf 100644 --- a/tests/storage/restricted_namespace_cloning/utils.py +++ b/tests/storage/restricted_namespace_cloning/utils.py @@ -31,8 +31,8 @@ def create_dv_negative( namespace: str, storage_class: str, size: str, - source_pvc: str, - source_namespace: str, + source_pvc_name: str, + source_pvc_namespace: str, unprivileged_client: DynamicClient, ) -> None: with pytest.raises( @@ -44,8 +44,8 @@ def create_dv_negative( namespace=namespace, source=PVC, size=size, - source_pvc=source_pvc, - source_namespace=source_namespace, + source_pvc_name=source_pvc_name, + source_pvc_namespace=source_pvc_namespace, client=unprivileged_client, storage_class=storage_class, ): diff --git a/tests/storage/storage_migration/conftest.py b/tests/storage/storage_migration/conftest.py index ae877c0d4b..ef601720dd 100644 --- a/tests/storage/storage_migration/conftest.py +++ b/tests/storage/storage_migration/conftest.py @@ -37,6 +37,7 @@ ) from utilities.infra import create_ns from utilities.storage import ( + construct_datavolume_source_dict, create_dv, data_volume_template_with_source_ref_dict, virtctl_volume, @@ -175,13 +176,15 @@ def vm_for_storage_class_migration_from_template_with_dv( dv = DataVolume( name="dv-rhel-imported", namespace=namespace.name, - source="http", - url=rhel_latest_os_params["rhel_image_path"], + source_dict=construct_datavolume_source_dict( + source="http", + url=rhel_latest_os_params["rhel_image_path"], + secret_name=artifactory_secret_scope_module.name, + cert_configmap_name=artifactory_config_map_scope_module.name, + ), size=Images.Rhel.DEFAULT_DV_SIZE, storage_class=source_storage_class, api_name="storage", - secret=artifactory_secret_scope_module, - cert_configmap=artifactory_config_map_scope_module.name, ) dv.to_dict() with VirtualMachineForTests( @@ -371,14 +374,16 @@ def windows_vm_with_vtpm_for_storage_migration( name="windows-11-dv", namespace=namespace.name, storage_class=source_storage_class, - source="http", - # Using WSL image to avoid the issue of the Windows VM not being able to boot - url=get_http_image_url(image_directory=Images.Windows.DIR, image_name=Images.Windows.WIN11_WSL2_IMG), + source_dict=construct_datavolume_source_dict( + source="http", + # Using WSL image to avoid the issue of the Windows VM not being able to boot + url=get_http_image_url(image_directory=Images.Windows.DIR, image_name=Images.Windows.WIN11_WSL2_IMG), + secret_name=artifactory_secret_scope_module.name, + cert_configmap_name=artifactory_config_map_scope_module.name, + ), size=Images.Windows.DEFAULT_DV_SIZE, client=unprivileged_client, api_name="storage", - secret=artifactory_secret_scope_module, - cert_configmap=artifactory_config_map_scope_module.name, ) dv.to_dict() with VirtualMachineForTests( diff --git a/tests/storage/test_cdi_certificate.py b/tests/storage/test_cdi_certificate.py index 528fb03628..ab424453d0 100644 --- a/tests/storage/test_cdi_certificate.py +++ b/tests/storage/test_cdi_certificate.py @@ -234,7 +234,8 @@ def test_import_clone_after_certs_renewal( dv_name="dv-target", namespace=namespace.name, size=data_volume_multi_storage_scope_module.size, - source_pvc=data_volume_multi_storage_scope_module.name, + source_pvc_name=data_volume_multi_storage_scope_module.name, + source_pvc_namespace=data_volume_multi_storage_scope_module.namespace, storage_class=data_volume_multi_storage_scope_module.storage_class, ) as cdv: cdv.wait_for_dv_success(timeout=TIMEOUT_3MIN) diff --git a/tests/storage/test_cdi_resources.py b/tests/storage/test_cdi_resources.py index df7a165719..3afe973efc 100644 --- a/tests/storage/test_cdi_resources.py +++ b/tests/storage/test_cdi_resources.py @@ -234,7 +234,8 @@ def test_cloner_pods_cdi_label( dv_name="dv-target", namespace=data_volume_without_snapshot_capability_scope_function.namespace, size=data_volume_without_snapshot_capability_scope_function.size, - source_pvc=data_volume_without_snapshot_capability_scope_function.name, + source_pvc_name=data_volume_without_snapshot_capability_scope_function.name, + source_pvc_namespace=data_volume_without_snapshot_capability_scope_function.namespace, storage_class=data_volume_without_snapshot_capability_scope_function.storage_class, ) as cdv: cdv.wait_for_status(status=DataVolume.Status.CLONE_IN_PROGRESS, timeout=TIMEOUT_10MIN) diff --git a/tests/storage/test_priority_class.py b/tests/storage/test_priority_class.py index 91fa1294d9..623f41b8be 100644 --- a/tests/storage/test_priority_class.py +++ b/tests/storage/test_priority_class.py @@ -8,6 +8,7 @@ from tests.storage.utils import get_importer_pod from utilities.artifactory import get_test_artifact_server_url from utilities.constants import Images +from utilities.storage import construct_datavolume_source_dict from utilities.virt import VirtualMachineForTests, fedora_vm_body @@ -31,10 +32,12 @@ def priority_class(request, admin_client): @pytest.fixture() def dv_dict(namespace, priority_class, unprivileged_client): dv = DataVolume( - source="http", + source_dict=construct_datavolume_source_dict( + source="http", + url=f"{get_test_artifact_server_url()}{RHEL_LATEST['image_path']}", + ), name="priority-dv", namespace=namespace.name, - url=f"{get_test_artifact_server_url()}{RHEL_LATEST['image_path']}", size=RHEL_LATEST["dv_size"], storage_class=py_config["default_storage_class"], client=unprivileged_client, diff --git a/tests/storage/test_wffc.py b/tests/storage/test_wffc.py index 8ea6928f98..a794fa730c 100644 --- a/tests/storage/test_wffc.py +++ b/tests/storage/test_wffc.py @@ -19,6 +19,7 @@ add_dv_to_vm, check_disk_count_in_vm, check_upload_virtctl_result, + construct_datavolume_source_dict, create_dv, create_vm_from_dv, data_volume_template_with_source_ref_dict, @@ -63,7 +64,7 @@ def blank_dv_template_wffc_scope_function(request, namespace, wffc_storage_class blank_dv_template = DataVolume( name=f"dv-{request.param['dv_name']}", namespace=namespace.name, - source="blank", + source_dict=construct_datavolume_source_dict(source="blank"), size=DEFAULT_BLANK_DV_SIZE, storage_class=wffc_storage_class_name_scope_module, api_name="storage", diff --git a/tests/storage/utils.py b/tests/storage/utils.py index 7459b45192..bbd8a556c1 100644 --- a/tests/storage/utils.py +++ b/tests/storage/utils.py @@ -47,6 +47,7 @@ from utilities.ssp import validate_os_info_vmi_vs_windows_os from utilities.storage import ( PodWithPVC, + construct_datavolume_source_dict, create_dv, get_containers_for_pods_with_pvc, ) @@ -81,7 +82,7 @@ def import_image_to_dv( dv_name=dv_name, namespace=configmap.namespace, url=url, - cert_configmap=configmap.name, + cert_configmap_name=configmap.name, storage_class=py_config["default_storage_class"], client=client, ) as dv: @@ -364,13 +365,15 @@ def create_windows19_vm(dv_name, namespace, client, vm_name, cpu_model, storage_ name=dv_name, namespace=namespace, storage_class=storage_class, - source="http", - url=get_http_image_url(image_directory=Images.Windows.UEFI_WIN_DIR, image_name=Images.Windows.WIN2k19_IMG), + source_dict=construct_datavolume_source_dict( + source="http", + url=get_http_image_url(image_directory=Images.Windows.UEFI_WIN_DIR, image_name=Images.Windows.WIN2k19_IMG), + secret_name=artifactory_secret.name, + cert_configmap_name=artifactory_config_map.name, + ), size=Images.Windows.DEFAULT_DV_SIZE, client=client, api_name="storage", - secret=artifactory_secret, - cert_configmap=artifactory_config_map.name, ) dv.to_dict() with VirtualMachineForTestsFromTemplate( diff --git a/tests/storage/vm_export/utils.py b/tests/storage/vm_export/utils.py index 100c1af67f..eee8435f95 100644 --- a/tests/storage/vm_export/utils.py +++ b/tests/storage/vm_export/utils.py @@ -15,7 +15,7 @@ from pyhelper_utils.shell import run_command from pytest_testconfig import config as py_config -from utilities.constants import TIMEOUT_1MIN +from utilities.constants import BIND_IMMEDIATE_ANNOTATION, TIMEOUT_1MIN from utilities.storage import create_dv LOGGER = logging.getLogger(__name__) @@ -57,7 +57,7 @@ def create_blank_dv_by_specific_user( size="1Gi", storage_class=py_config["default_storage_class"], consume_wffc=False, - bind_immediate=True, + annotations=BIND_IMMEDIATE_ANNOTATION, client=client, ) as dv: dv.wait_for_dv_success(timeout=TIMEOUT_1MIN) diff --git a/tests/utils.py b/tests/utils.py index 6b26904d47..981c39c19a 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -62,6 +62,7 @@ ExecCommandOnPod, ) from utilities.os_utils import get_windows_container_disk_path +from utilities.storage import construct_datavolume_source_dict from utilities.virt import ( VirtualMachineForTests, fedora_vm_body, @@ -565,14 +566,16 @@ def create_cirros_vm( dv = DataVolume( name=dv_name, namespace=namespace, - source="http", - url=get_http_image_url(image_directory=Images.Cirros.DIR, image_name=Images.Cirros.QCOW2_IMG), + source_dict=construct_datavolume_source_dict( + source="http", + url=get_http_image_url(image_directory=Images.Cirros.DIR, image_name=Images.Cirros.QCOW2_IMG), + secret_name=artifactory_secret.name, + cert_configmap_name=artifactory_config_map.name, + ), storage_class=storage_class, size=Images.Cirros.DEFAULT_DV_SIZE, api_name="storage", volume_mode=volume_mode, - secret=artifactory_secret, - cert_configmap=artifactory_config_map.name, ) dv.to_dict() dv_metadata = dv.res["metadata"] @@ -732,13 +735,15 @@ def create_windows2022_dv_from_registry( name=dv_name, namespace=namespace, storage_class=storage_class, - source="registry", - url=f"{get_test_artifact_server_url(schema='registry')}/{get_windows_container_disk_path(os_value=WIN_2K22)}", + source_dict=construct_datavolume_source_dict( + source="registry", + url=f"{get_test_artifact_server_url(schema='registry')}/{get_windows_container_disk_path(os_value=WIN_2K22)}", + secret_name=artifactory_secret.name, + cert_configmap_name=artifactory_config_map.name, + ), size=Images.Windows.CONTAINER_DISK_DV_SIZE, client=client, api_name="storage", - secret=artifactory_secret, - cert_configmap=artifactory_config_map.name, ) dv.to_dict() diff --git a/tests/virt/cluster/longevity_tests/utils.py b/tests/virt/cluster/longevity_tests/utils.py index e94e3bf281..2f596f359e 100644 --- a/tests/virt/cluster/longevity_tests/utils.py +++ b/tests/virt/cluster/longevity_tests/utils.py @@ -23,7 +23,7 @@ get_artifactory_secret, ) from utilities.constants import TCP_TIMEOUT_30SEC, TIMEOUT_5MIN, TIMEOUT_30MIN, TIMEOUT_40MIN, TIMEOUT_60MIN, WIN_10 -from utilities.storage import get_test_artifact_server_url +from utilities.storage import construct_datavolume_source_dict, get_test_artifact_server_url from utilities.virt import ( VirtualMachineForTests, VirtualMachineForTestsFromTemplate, @@ -347,13 +347,15 @@ def create_multi_dvs(namespace, client, dv_params): name=dv_name, client=client, namespace=namespace_name, - source="http", + source_dict=construct_datavolume_source_dict( + source="http", + url=f"{get_test_artifact_server_url()}{dv[dv_name].get('image_path')}", + secret_name=artifactory_secret.name, + cert_configmap_name=artifactory_config_map.name, + ), size=dv[dv_name].get("dv_size"), storage_class=dv[dv_name].get("storage_class"), - url=f"{get_test_artifact_server_url()}{dv[dv_name].get('image_path')}", api_name="storage", - secret=artifactory_secret, - cert_configmap=artifactory_config_map.name, ) yield from deploy_and_wait_for_dvs(dv_dict=dvs) diff --git a/tests/virt/cluster/vm_cloning/test_vm_cloning.py b/tests/virt/cluster/vm_cloning/test_vm_cloning.py index c3b862ad9f..d08194530a 100644 --- a/tests/virt/cluster/vm_cloning/test_vm_cloning.py +++ b/tests/virt/cluster/vm_cloning/test_vm_cloning.py @@ -19,6 +19,7 @@ from utilities.storage import ( add_dv_to_vm, check_disk_count_in_vm, + construct_datavolume_source_dict, ) from utilities.virt import ( VirtualMachineForCloning, @@ -47,7 +48,7 @@ def dummy_dv_dict_for_vm_cloning(client, namespace): name="dummy-dv-for-clone", client=client, namespace=namespace.name, - source="blank", + source_dict=construct_datavolume_source_dict(source="blank"), size="10Gi", storage_class=py_config["default_storage_class"], api_name="storage", diff --git a/tests/virt/upgrade/conftest.py b/tests/virt/upgrade/conftest.py index 5bb8cea2c4..776858f5a5 100644 --- a/tests/virt/upgrade/conftest.py +++ b/tests/virt/upgrade/conftest.py @@ -257,11 +257,13 @@ def windows_vm( client=admin_client, dv_name=latest_windows_dict["os_version"], namespace=py_config["golden_images_namespace"], + source="http", url=f"{get_test_artifact_server_url()}{latest_windows_dict['image_path']}", storage_class=py_config["default_storage_class"], access_modes=py_config["default_access_mode"], volume_mode=py_config["default_volume_mode"], size=latest_windows_dict["dv_size"], + use_artifactory=True, ) as dv: dv.wait_for_dv_success(timeout=TIMEOUT_30MIN) with DataSource( diff --git a/tests/virt/upgrade_custom/vgpu/conftest.py b/tests/virt/upgrade_custom/vgpu/conftest.py index 49a597644e..9584263349 100644 --- a/tests/virt/upgrade_custom/vgpu/conftest.py +++ b/tests/virt/upgrade_custom/vgpu/conftest.py @@ -42,11 +42,13 @@ def rhel_data_volume( client=admin_client, dv_name=RHEL_LATEST_OS, namespace=py_config["golden_images_namespace"], + source="http", url=f"{get_test_artifact_server_url()}{RHEL_LATEST['image_path']}", storage_class=py_config["default_storage_class"], access_modes=py_config["default_access_mode"], volume_mode=py_config["default_volume_mode"], size=RHEL_LATEST["dv_size"], + use_artifactory=True, ) as dv: dv.wait_for_dv_success(timeout=TIMEOUT_30MIN) yield dv diff --git a/tests/virt/utils.py b/tests/virt/utils.py index e81ca96814..a640d99c50 100644 --- a/tests/virt/utils.py +++ b/tests/virt/utils.py @@ -458,10 +458,12 @@ def get_or_create_golden_image_data_source( with create_dv( dv_name=data_source_name, namespace=golden_images_namespace.name, + source="http", storage_class=py_config["default_storage_class"], url=f"{get_test_artifact_server_url()}{os_dict['image_path']}", size=os_dict["dv_size"], client=admin_client, + use_artifactory=True, ) as dv: dv.wait_for_dv_success(timeout=TIMEOUT_30MIN) yield from create_or_update_data_source(admin_client=admin_client, dv=dv) diff --git a/utilities/oadp.py b/utilities/oadp.py index 896f175821..d70ef52507 100644 --- a/utilities/oadp.py +++ b/utilities/oadp.py @@ -33,6 +33,7 @@ get_pod_by_name_prefix, unique_name, ) +from utilities.storage import construct_datavolume_source_dict from utilities.virt import VirtualMachineForTests, running_vm LOGGER = logging.getLogger(__name__) @@ -159,17 +160,19 @@ def create_rhel_vm( dv = DataVolume( name=dv_name, namespace=namespace, - source="http", - url=get_http_image_url( - image_directory=Images.Rhel.DIR, - image_name=rhel_image, + source_dict=construct_datavolume_source_dict( + source="http", + url=get_http_image_url( + image_directory=Images.Rhel.DIR, + image_name=rhel_image, + ), + secret_name=artifactory_secret.name, + cert_configmap_name=artifactory_config_map.name, ), storage_class=storage_class, size=Images.Rhel.DEFAULT_DV_SIZE, api_name="storage", volume_mode=volume_mode, - secret=artifactory_secret, - cert_configmap=artifactory_config_map.name, ) dv.to_dict() dv_metadata = dv.res["metadata"] diff --git a/utilities/storage.py b/utilities/storage.py index 1e676c8748..9a3dc5b691 100644 --- a/utilities/storage.py +++ b/utilities/storage.py @@ -35,6 +35,7 @@ from utilities import console from utilities.artifactory import get_test_artifact_server_url from utilities.constants import ( + BIND_IMMEDIATE_ANNOTATION, CDI_LABEL, HOTPLUG_DISK_SERIAL, HPP_POOL, @@ -105,76 +106,177 @@ def create_dummy_first_consumer_pod(volume_mode=DataVolume.VolumeMode.FILE, dv=N ) +def construct_datavolume_source_dict( + source: str, + url: str | None = None, + secret_name: str | None = None, + cert_configmap_name: str | None = None, + source_pvc_name: str | None = None, + source_pvc_namespace: str | None = None, +) -> dict[str, Any]: + """ + Build a DataVolume source_dict. + + Args: + source: Source type ("http", "registry", "pvc", "blank", "upload"). + url: URL for http/registry sources. + secret_name: Optional Secret name for authentication (http/registry sources). + cert_configmap_name: Optional ConfigMap name for TLS certificates (http/registry sources). + source_pvc_name: PVC name for pvc source type. + source_pvc_namespace: Namespace of the source PVC. + + Returns: + dict[str, Any]: The constructed source_dict for DataVolume. + """ + if source == "http": + if not utilities.infra.url_excluded_from_validation(url): + validate_file_exists_in_url(url=url) + source_spec: dict[str, Any] = {"http": {"url": url}} + elif source == "registry": + source_spec = {"registry": {"url": url}} + elif source == "pvc": + pvc_spec: dict[str, Any] = {"name": source_pvc_name} + if source_pvc_namespace is not None: + pvc_spec["namespace"] = source_pvc_namespace + source_spec = {"pvc": pvc_spec} + elif source == "blank": + source_spec = {"blank": {}} + elif source == "upload": + source_spec = {"upload": {}} + else: + raise ValueError(f"Unsupported source type: {source}") + + if source in ("http", "registry"): + if secret_name: + source_spec[source]["secretRef"] = secret_name + if cert_configmap_name: + source_spec[source]["certConfigMap"] = cert_configmap_name + + return source_spec + + @contextmanager def create_dv( - dv_name, - namespace, - storage_class, - client, - volume_mode=None, - url=None, - source="http", - content_type=None, - size="5Gi", - secret=None, - cert_configmap=None, - hostpath_node=None, - access_modes=None, - source_pvc=None, - source_namespace=None, - multus_annotation=None, - teardown=True, - consume_wffc=True, - bind_immediate=None, - preallocation=None, - api_name="storage", - source_ref=None, -): + dv_name: str, + namespace: str, + client: DynamicClient, + storage_class: str | None = None, + access_modes: str | None = None, + volume_mode: str | None = None, + url: str | None = None, + source: str | None = None, + content_type: str | None = None, + size: str = "5Gi", + secret_name: str | None = None, + cert_configmap_name: str | None = None, + source_pvc_name: str | None = None, + source_pvc_namespace: str | None = None, + annotations: dict[str, str] | None = None, + teardown: bool = True, + consume_wffc: bool = True, + preallocation: bool | None = None, + api_name: str = "storage", + source_ref: dict[str, Any] | None = None, + source_dict: dict[str, Any] | None = None, + use_artifactory: bool = False, +) -> Generator[DataVolume]: + """ + Create and manage a DataVolume with optional Artifactory resource lifecycle. + + Context manager that constructs a DataVolume from either a pre-built ``source_dict``/``source_ref`` + or by building one via ``construct_datavolume_source_dict`` from the ``source`` parameter. + When ``use_artifactory`` is True for http/registry sources, creates namespace-scoped + Artifactory Secret and ConfigMap resources that are cleaned up on exit. + + Args: + dv_name: Name for the DataVolume resource. + namespace: Target Kubernetes namespace. + client: Kubernetes dynamic client. + storage_class: StorageClass name. + access_modes: PVC access mode (e.g. "ReadWriteOnce"). + volume_mode: PVC volume mode ("Block" or "Filesystem"). + url: Source URL for http/registry sources. + source: Source type ("http", "registry", "pvc", "blank", "upload"). + Used to construct ``source_dict`` when neither ``source_dict`` nor ``source_ref`` is provided. + content_type: CDI content type (e.g. "kubevirt"). + size: PVC size (default "5Gi"). + secret_name: Pre-existing Secret name for source authentication. + cert_configmap_name: Pre-existing ConfigMap name for TLS certificates. + source_pvc_name: PVC name for clone sources. + source_pvc_namespace: Namespace of the source PVC for clone sources. + annotations: Annotations dict to apply to the DataVolume. + teardown: Whether to delete the DataVolume on context exit. + consume_wffc: Whether to create a dummy consumer pod for WaitForFirstConsumer storage classes. + preallocation: Whether to preallocate the target PVC. + api_name: CDI API group name (default "storage"). + source_ref: Pre-built sourceRef dict for DataVolume. + source_dict: Pre-built source dict for DataVolume. + use_artifactory: Whether to create Artifactory Secret/ConfigMap for http/registry sources. + + Yields: + DataVolume: The created DataVolume resource. + + Raises: + ValueError: If ``source`` is not provided when ``source_dict`` and ``source_ref`` are both None. + """ artifactory_secret = None - cert_created = None + artifactory_config_map = None - if source_ref: - source = None - if source in ("http", "https"): - if not utilities.infra.url_excluded_from_validation(url): - # Make sure URL exists - validate_file_exists_in_url(url=url) - if not secret: - secret = utilities.artifactory.get_artifactory_secret(namespace=namespace) - artifactory_secret = secret - if not cert_configmap: - cert_created = utilities.artifactory.get_artifactory_config_map(namespace=namespace) - cert_configmap = cert_created.name - - with DataVolume( - source=source, - name=dv_name, - namespace=namespace, - url=url, - content_type=content_type, - size=size, - storage_class=storage_class, - cert_configmap=cert_configmap, - volume_mode=volume_mode, - hostpath_node=hostpath_node, - access_modes=access_modes, - secret=secret, - client=client, - source_pvc=source_pvc, - source_namespace=source_namespace, - bind_immediate_annotation=bind_immediate, - multus_annotation=multus_annotation, - teardown=teardown, - preallocation=preallocation, - api_name=api_name, - source_ref=source_ref, - ) as dv: - if sc_volume_binding_mode_is_wffc(sc=storage_class, client=client) and consume_wffc: - create_dummy_first_consumer_pod(dv=dv) - yield dv - utilities.artifactory.cleanup_artifactory_secret_and_config_map( - artifactory_secret=artifactory_secret, artifactory_config_map=cert_created - ) + try: + if source_dict is None and source_ref is None: + if not source: + raise ValueError("'source' is required when 'source_dict' and 'source_ref' are not provided") + + LOGGER.info("No 'source_dict' or 'source_ref' provided - will construct the 'source_dict'") + + if source in ("http", "registry") and use_artifactory: + LOGGER.info(f"Creating artifactory resources for DV '{dv_name}' in namespace '{namespace}'") + LOGGER.info(f"DV source is '{source}' with url: {url}") + + if not secret_name: + artifactory_secret = utilities.artifactory.get_artifactory_secret( + namespace=namespace, client=client + ) + secret_name = artifactory_secret.name + if not cert_configmap_name: + artifactory_config_map = utilities.artifactory.get_artifactory_config_map( + namespace=namespace, client=client + ) + cert_configmap_name = artifactory_config_map.name + + source_dict = construct_datavolume_source_dict( + source=source, + url=url, + secret_name=secret_name, + cert_configmap_name=cert_configmap_name, + source_pvc_name=source_pvc_name, + source_pvc_namespace=source_pvc_namespace, + ) + + with DataVolume( + name=dv_name, + namespace=namespace, + client=client, + content_type=content_type, + size=size, + storage_class=storage_class, + access_modes=access_modes, + volume_mode=volume_mode, + annotations=annotations, + teardown=teardown, + preallocation=preallocation, + api_name=api_name, + source_ref=source_ref, + source_dict=source_dict, + ) as dv: + if storage_class and sc_volume_binding_mode_is_wffc(sc=storage_class, client=client) and consume_wffc: + create_dummy_first_consumer_pod(dv=dv) + yield dv + + finally: + utilities.artifactory.cleanup_artifactory_secret_and_config_map( + artifactory_secret=artifactory_secret, artifactory_config_map=artifactory_config_map + ) def data_volume( @@ -256,13 +358,14 @@ def data_volume( "access_modes": params_dict.get("access_modes"), "volume_mode": params_dict.get("volume_mode"), "consume_wffc": consume_wffc, - "bind_immediate": bind_immediate, + "annotations": BIND_IMMEDIATE_ANNOTATION if bind_immediate else None, "preallocation": params_dict.get("preallocation", None), "url": url, "client": client, + "use_artifactory": source == "http", } - if params_dict.get("cert_configmap"): - dv_kwargs["cert_configmap"] = params_dict.get("cert_configmap") + if params_dict.get("cert_configmap_name"): + dv_kwargs["cert_configmap_name"] = params_dict["cert_configmap_name"] # Create dv with create_dv(**{k: v for k, v in dv_kwargs.items() if v is not None}) as dv: if params_dict.get("wait", True): @@ -551,12 +654,14 @@ def data_volume_template_dict( dv = DataVolume( name=target_dv_name, namespace=target_dv_namespace, - source="pvc", + source_dict=construct_datavolume_source_dict( + source="pvc", + source_pvc_name=source_dv.name, + source_pvc_namespace=source_dv.namespace, + ), storage_class=storage_class or source_dv_pvc_spec.storageClassName, volume_mode=volume_mode or source_dv_pvc_spec.volumeMode, size=size or source_dv.size, - source_pvc=source_dv.name, - source_namespace=source_dv.namespace, api_name=source_dv.api_name, ) dv.to_dict() diff --git a/utilities/unittests/conftest.py b/utilities/unittests/conftest.py index 716551300f..6259758bfb 100644 --- a/utilities/unittests/conftest.py +++ b/utilities/unittests/conftest.py @@ -13,8 +13,6 @@ from ocp_resources import resource from pytest_testconfig import config as py_config -import utilities - os.environ["OPENSHIFT_VIRTUALIZATION_TEST_IMAGES_ARCH"] = "amd64" # Add utilities to Python path for imports @@ -34,6 +32,7 @@ def _mock_get_client(*args: Any, **kwargs: Any) -> MagicMock: # type: ignore[mi # Note: utilities.hco is mocked here but test_hco.py will clear it and import real module mock_hco = MagicMock() mock_infra = MagicMock() +mock_virt = MagicMock() mock_data_collector = MagicMock() mock_data_collector.get_data_collector_base_directory = MagicMock(return_value="/tmp/data") mock_data_collector.get_data_collector_base = MagicMock(return_value="/tmp/data/") @@ -44,12 +43,16 @@ def _mock_get_client(*args: Any, **kwargs: Any) -> MagicMock: # type: ignore[mi sys.modules["utilities.hco"] = mock_hco sys.modules["utilities.infra"] = mock_infra +sys.modules["utilities.virt"] = mock_virt sys.modules["utilities.data_collector"] = mock_data_collector sys.modules["jira"] = mock_jira +import utilities + # Also set them as attributes of the utilities module for tests that need them utilities.hco = mock_hco # type: ignore[attr-defined] utilities.infra = mock_infra # type: ignore[attr-defined] +utilities.virt = mock_virt # type: ignore[attr-defined] utilities.data_collector = mock_data_collector # type: ignore[attr-defined] diff --git a/utilities/unittests/test_storage.py b/utilities/unittests/test_storage.py new file mode 100644 index 0000000000..f85a753ce2 --- /dev/null +++ b/utilities/unittests/test_storage.py @@ -0,0 +1,149 @@ +"""Unit tests for construct_datavolume_source_dict in utilities/storage.py""" + +import importlib +import sys +from unittest.mock import patch + +import pytest + +# Other test modules (test_hco, test_ssp) mock utilities.storage in sys.modules. +# Clear the mock and reimport the real module to test actual behavior. +if "utilities.storage" in sys.modules: + del sys.modules["utilities.storage"] + +import utilities.storage + +importlib.reload(utilities.storage) + +from utilities.storage import construct_datavolume_source_dict + + +class TestConstructDatavolumeSourceDictHttp: + @patch("utilities.storage.validate_file_exists_in_url") + @patch("utilities.infra.url_excluded_from_validation", return_value=False) + def test_http_source(self, mock_excluded, mock_validate): + result = construct_datavolume_source_dict(source="http", url="https://example.com/image.qcow2") + assert result == {"http": {"url": "https://example.com/image.qcow2"}} + mock_validate.assert_called_once_with(url="https://example.com/image.qcow2") + + @patch("utilities.storage.validate_file_exists_in_url") + @patch("utilities.infra.url_excluded_from_validation", return_value=True) + def test_http_source_excluded_from_validation(self, mock_excluded, mock_validate): + result = construct_datavolume_source_dict(source="http", url="https://internal.example.com/image.qcow2") + assert result == {"http": {"url": "https://internal.example.com/image.qcow2"}} + mock_validate.assert_not_called() + + @patch("utilities.storage.validate_file_exists_in_url") + @patch("utilities.infra.url_excluded_from_validation", return_value=False) + def test_http_source_with_secret(self, mock_excluded, mock_validate): + result = construct_datavolume_source_dict( + source="http", + url="https://example.com/image.qcow2", + secret_name="my-secret", + ) + assert result == {"http": {"url": "https://example.com/image.qcow2", "secretRef": "my-secret"}} + + @patch("utilities.storage.validate_file_exists_in_url") + @patch("utilities.infra.url_excluded_from_validation", return_value=False) + def test_http_source_with_cert_configmap(self, mock_excluded, mock_validate): + result = construct_datavolume_source_dict( + source="http", + url="https://example.com/image.qcow2", + cert_configmap_name="my-cert-cm", + ) + assert result == {"http": {"url": "https://example.com/image.qcow2", "certConfigMap": "my-cert-cm"}} + + @patch("utilities.storage.validate_file_exists_in_url") + @patch("utilities.infra.url_excluded_from_validation", return_value=False) + def test_http_source_with_secret_and_cert(self, mock_excluded, mock_validate): + result = construct_datavolume_source_dict( + source="http", + url="https://example.com/image.qcow2", + secret_name="my-secret", + cert_configmap_name="my-cert-cm", + ) + assert result == { + "http": {"url": "https://example.com/image.qcow2", "secretRef": "my-secret", "certConfigMap": "my-cert-cm"} + } + + +class TestConstructDatavolumeSourceDictRegistry: + def test_registry_source(self): + result = construct_datavolume_source_dict(source="registry", url="docker://registry.example.com/image:latest") + assert result == {"registry": {"url": "docker://registry.example.com/image:latest"}} + + def test_registry_source_with_secret(self): + result = construct_datavolume_source_dict( + source="registry", + url="docker://registry.example.com/image:latest", + secret_name="registry-secret", + ) + assert result == { + "registry": {"url": "docker://registry.example.com/image:latest", "secretRef": "registry-secret"} + } + + def test_registry_source_with_cert_configmap(self): + result = construct_datavolume_source_dict( + source="registry", + url="docker://registry.example.com/image:latest", + cert_configmap_name="registry-cert-cm", + ) + assert result == { + "registry": {"url": "docker://registry.example.com/image:latest", "certConfigMap": "registry-cert-cm"} + } + + def test_registry_source_with_secret_and_cert(self): + result = construct_datavolume_source_dict( + source="registry", + url="docker://registry.example.com/image:latest", + secret_name="registry-secret", + cert_configmap_name="registry-cert-cm", + ) + assert result == { + "registry": { + "url": "docker://registry.example.com/image:latest", + "secretRef": "registry-secret", + "certConfigMap": "registry-cert-cm", + } + } + + +class TestConstructDatavolumeSourceDictPvc: + def test_pvc_source_with_namespace(self): + result = construct_datavolume_source_dict( + source="pvc", + source_pvc_name="my-pvc", + source_pvc_namespace="my-namespace", + ) + assert result == {"pvc": {"name": "my-pvc", "namespace": "my-namespace"}} + + def test_pvc_source_without_namespace(self): + result = construct_datavolume_source_dict(source="pvc", source_pvc_name="my-pvc") + assert result == {"pvc": {"name": "my-pvc"}} + assert "namespace" not in result["pvc"] + + def test_pvc_source_with_empty_namespace(self): + result = construct_datavolume_source_dict( + source="pvc", + source_pvc_name="my-pvc", + source_pvc_namespace="", + ) + assert result == {"pvc": {"name": "my-pvc", "namespace": ""}} + + +class TestConstructDatavolumeSourceDictBlank: + def test_blank_source(self): + result = construct_datavolume_source_dict(source="blank") + assert result == {"blank": {}} + + +class TestConstructDatavolumeSourceDictUpload: + def test_upload_source(self): + result = construct_datavolume_source_dict(source="upload") + assert result == {"upload": {}} + + +class TestConstructDatavolumeSourceDictUnsupported: + def test_unsupported_source_raises_value_error(self): + with pytest.raises(ValueError, match="Unsupported source type: ftp"): + construct_datavolume_source_dict(source="ftp") From 8d2f9993b675f48ad882867741a0ff5391824612 Mon Sep 17 00:00:00 2001 From: Jenia Peimer Date: Mon, 6 Jul 2026 18:28:52 +0300 Subject: [PATCH 2/2] Fix E402 lint errors by centralizing sys.modules mocking in unittests conftest Centralize utilities.storage and utilities.ssp mocking in conftest.py, removing redundant mocking from individual test files. This allows imports to be at the top of each module, fixing ruff E402 violations. Also removes stale noqa comments and replaces deprecated IOError with OSError. Assisted-by: Claude Signed-off-by: Jenia Peimer --- tests/conftest.py | 2 +- utilities/unittests/conftest.py | 12 +++++++-- utilities/unittests/test_data_collector.py | 2 +- utilities/unittests/test_database.py | 2 +- utilities/unittests/test_guest_support.py | 11 +------- utilities/unittests/test_hco.py | 27 +------------------ utilities/unittests/test_oadp.py | 15 ----------- utilities/unittests/test_operator.py | 2 +- .../unittests/test_pytest_matrix_utils.py | 2 +- utilities/unittests/test_ssp.py | 19 ++----------- utilities/unittests/test_storage.py | 5 ---- 11 files changed, 19 insertions(+), 80 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index d33b0cbe92..40108480ac 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -81,6 +81,7 @@ ARM_64, ARQ_QUOTA_HARD_SPEC, AUDIT_LOGS_PATH, + BIND_IMMEDIATE_ANNOTATION, CDI_KUBEVIRT_HYPERCONVERGED, CLUSTER, CNV_TEST_SERVICE_ACCOUNT, @@ -105,7 +106,6 @@ OC_ADM_LOGS_COMMAND, OS_FLAVOR_RHEL, OVS_BRIDGE, - BIND_IMMEDIATE_ANNOTATION, POD_SECURITY_NAMESPACE_LABELS, PREFERENCE_STR, RHEL9_STR, diff --git a/utilities/unittests/conftest.py b/utilities/unittests/conftest.py index 6259758bfb..34729526ea 100644 --- a/utilities/unittests/conftest.py +++ b/utilities/unittests/conftest.py @@ -13,6 +13,8 @@ from ocp_resources import resource from pytest_testconfig import config as py_config +import utilities + os.environ["OPENSHIFT_VIRTUALIZATION_TEST_IMAGES_ARCH"] = "amd64" # Add utilities to Python path for imports @@ -33,6 +35,10 @@ def _mock_get_client(*args: Any, **kwargs: Any) -> MagicMock: # type: ignore[mi mock_hco = MagicMock() mock_infra = MagicMock() mock_virt = MagicMock() +mock_storage = MagicMock() +mock_ssp = MagicMock() +mock_ssp.wait_for_ssp = MagicMock() +mock_ssp.validate_os_info_vmi_vs_windows_os = MagicMock() mock_data_collector = MagicMock() mock_data_collector.get_data_collector_base_directory = MagicMock(return_value="/tmp/data") mock_data_collector.get_data_collector_base = MagicMock(return_value="/tmp/data/") @@ -44,15 +50,17 @@ def _mock_get_client(*args: Any, **kwargs: Any) -> MagicMock: # type: ignore[mi sys.modules["utilities.hco"] = mock_hco sys.modules["utilities.infra"] = mock_infra sys.modules["utilities.virt"] = mock_virt +sys.modules["utilities.storage"] = mock_storage +sys.modules["utilities.ssp"] = mock_ssp sys.modules["utilities.data_collector"] = mock_data_collector sys.modules["jira"] = mock_jira -import utilities - # Also set them as attributes of the utilities module for tests that need them utilities.hco = mock_hco # type: ignore[attr-defined] utilities.infra = mock_infra # type: ignore[attr-defined] utilities.virt = mock_virt # type: ignore[attr-defined] +utilities.storage = mock_storage # type: ignore[attr-defined] +utilities.ssp = mock_ssp # type: ignore[attr-defined] utilities.data_collector = mock_data_collector # type: ignore[attr-defined] diff --git a/utilities/unittests/test_data_collector.py b/utilities/unittests/test_data_collector.py index 6bedf87a44..58ffa727ca 100644 --- a/utilities/unittests/test_data_collector.py +++ b/utilities/unittests/test_data_collector.py @@ -277,7 +277,7 @@ def test_write_to_file_custom_mode(self, mock_file_open, mock_makedirs): mock_file_open.assert_called_once_with("/test/dir/test.txt", "a") @patch("os.makedirs") - @patch("builtins.open", side_effect=IOError("Permission denied")) + @patch("builtins.open", side_effect=OSError("Permission denied")) @patch("utilities.data_collector.LOGGER") def test_write_to_file_exception_handling(self, mock_logger, mock_file_open, mock_makedirs): """Test write_to_file handles exceptions gracefully""" diff --git a/utilities/unittests/test_database.py b/utilities/unittests/test_database.py index 02b2f52b71..40024a5664 100644 --- a/utilities/unittests/test_database.py +++ b/utilities/unittests/test_database.py @@ -9,7 +9,7 @@ # Add utilities to Python path for imports sys.path.insert(0, str(Path(__file__).parent.parent)) -from database import CNV_TEST_DB, Base, CnvTestTable, Database # noqa: E402 +from database import CNV_TEST_DB, Base, CnvTestTable, Database class TestCnvTestTable: diff --git a/utilities/unittests/test_guest_support.py b/utilities/unittests/test_guest_support.py index 1108fe13d8..ac00a488fa 100644 --- a/utilities/unittests/test_guest_support.py +++ b/utilities/unittests/test_guest_support.py @@ -3,21 +3,12 @@ """Unit tests for guest_support module""" import json -import sys from unittest.mock import MagicMock, patch import pytest from timeout_sampler import TimeoutExpiredError -# Need to mock circular imports for guest_support -import utilities - -mock_virt = MagicMock() -sys.modules["utilities.virt"] = mock_virt -utilities.virt = mock_virt - -# Import after setting up mocks to avoid circular dependency -from utilities.guest_support import ( # noqa: E402 +from utilities.guest_support import ( assert_windows_efi, check_vm_xml_hyperv, check_windows_vm_hvinfo, diff --git a/utilities/unittests/test_hco.py b/utilities/unittests/test_hco.py index dd1a2aa3ba..da8b78b42f 100644 --- a/utilities/unittests/test_hco.py +++ b/utilities/unittests/test_hco.py @@ -4,42 +4,17 @@ import json import sys -from pathlib import Path from unittest.mock import MagicMock, patch import pytest from kubernetes.dynamic.exceptions import ResourceNotFoundError from timeout_sampler import TimeoutExpiredError -# Add utilities to Python path for imports -sys.path.insert(0, str(Path(__file__).parent.parent)) - -# Mock circular imports at module level to avoid circular dependencies # conftest.py mocks utilities.hco, but we need the real module for these tests -import utilities - -# First, set up mocks for hco.py's dependencies -# hco -> ssp -> storage -> virt -> console -> data_collector -mock_virt = MagicMock() -mock_storage = MagicMock() -mock_ssp = MagicMock() -# SSP needs these attributes for hco.py imports -mock_ssp.wait_for_ssp = MagicMock() -mock_ssp.validate_os_info_vmi_vs_windows_os = MagicMock() - -sys.modules["utilities.virt"] = mock_virt -sys.modules["utilities.storage"] = mock_storage -sys.modules["utilities.ssp"] = mock_ssp -utilities.virt = mock_virt -utilities.storage = mock_storage -utilities.ssp = mock_ssp - -# Remove the mock for utilities.hco from conftest.py so we can import the real module if "utilities.hco" in sys.modules: del sys.modules["utilities.hco"] -# Import after setting up mocks to avoid circular dependency -from utilities.hco import ( # noqa: E402 +from utilities.hco import ( CDI, DEFAULT_HCO_PROGRESSING_CONDITIONS, HCO_JSONPATCH_ANNOTATION_COMPONENT_DICT, diff --git a/utilities/unittests/test_oadp.py b/utilities/unittests/test_oadp.py index 48525fba6d..54024f019b 100644 --- a/utilities/unittests/test_oadp.py +++ b/utilities/unittests/test_oadp.py @@ -2,27 +2,12 @@ """Unit tests for oadp module""" -# flake8: noqa: E402 -import sys from re import escape from shlex import quote from unittest.mock import MagicMock, patch import pexpect import pytest - -# Need to mock circular imports for oadp -import utilities - -# mock must be before importing oadp to prevent circular import -mock_virt = MagicMock() -mock_infra = MagicMock() -sys.modules["utilities.virt"] = mock_virt -sys.modules["utilities.infra"] = mock_infra -utilities.virt = mock_virt -utilities.infra = mock_infra - -# Import after setting up mocks to avoid circular dependency from kubernetes.dynamic.exceptions import ResourceNotFoundError from ocp_resources.exceptions import ResourceTeardownError diff --git a/utilities/unittests/test_operator.py b/utilities/unittests/test_operator.py index b930685fe9..d38c398c29 100644 --- a/utilities/unittests/test_operator.py +++ b/utilities/unittests/test_operator.py @@ -9,7 +9,7 @@ from timeout_sampler import TimeoutExpiredError # Import after setting up mocks to avoid circular dependency -from utilities.operator import ( # noqa: E402 +from utilities.operator import ( approve_install_plan, cluster_with_icsp, collect_mcp_data_on_update_timeout, diff --git a/utilities/unittests/test_pytest_matrix_utils.py b/utilities/unittests/test_pytest_matrix_utils.py index 772ade5f47..9f806abfeb 100644 --- a/utilities/unittests/test_pytest_matrix_utils.py +++ b/utilities/unittests/test_pytest_matrix_utils.py @@ -7,7 +7,7 @@ import pytest -from utilities.pytest_matrix_utils import ( # noqa: E402 +from utilities.pytest_matrix_utils import ( hpp_matrix, immediate_matrix, online_resize_matrix, diff --git a/utilities/unittests/test_ssp.py b/utilities/unittests/test_ssp.py index 296f89d519..a2b940f291 100644 --- a/utilities/unittests/test_ssp.py +++ b/utilities/unittests/test_ssp.py @@ -9,26 +9,11 @@ from kubernetes.dynamic.exceptions import NotFoundError from timeout_sampler import TimeoutExpiredError -# Need to mock additional circular imports for ssp -import utilities - -mock_virt = MagicMock() -mock_storage = MagicMock() -mock_infra = MagicMock() -sys.modules["utilities.virt"] = mock_virt -sys.modules["utilities.storage"] = mock_storage -sys.modules["utilities.infra"] = mock_infra -utilities.virt = mock_virt -utilities.storage = mock_storage -utilities.infra = mock_infra - -# Clear any mock of utilities.ssp from other test modules (e.g., test_hco.py) -# to ensure we can import the real module for testing +# conftest.py mocks utilities.ssp, but we need the real module for these tests if "utilities.ssp" in sys.modules: del sys.modules["utilities.ssp"] -# Import after setting up mocks to avoid circular dependency -from utilities.ssp import ( # noqa: E402 +from utilities.ssp import ( cluster_instance_type_for_hot_plug, create_custom_template_from_url, get_cim_instance_json, diff --git a/utilities/unittests/test_storage.py b/utilities/unittests/test_storage.py index f85a753ce2..e420aa4fb6 100644 --- a/utilities/unittests/test_storage.py +++ b/utilities/unittests/test_storage.py @@ -1,6 +1,5 @@ """Unit tests for construct_datavolume_source_dict in utilities/storage.py""" -import importlib import sys from unittest.mock import patch @@ -11,10 +10,6 @@ if "utilities.storage" in sys.modules: del sys.modules["utilities.storage"] -import utilities.storage - -importlib.reload(utilities.storage) - from utilities.storage import construct_datavolume_source_dict