Skip to content

infra: Update dv_for_data_source to create the dv from public quay #5592

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
RoniKishner wants to merge 2 commits into
base: main
Choose a base branch
from
Open

infra: Update dv_for_data_source to create the dv from public quay #5592

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7b6f9c0
infra: Update dv_for_data_source to create the dv from public quay
RoniKishner Jul 12, 2026
5b75ca9
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Jul 16, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 9 additions & 23 deletions tests/infrastructure/golden_images/update_boot_source/test_ssp_data_sources.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,20 +17,15 @@
PVC_NOT_FOUND_ERROR,
)
from tests.utils import get_parameters_from_template
from utilities.artifactory import (
cleanup_artifactory_secret_and_config_map,
get_artifactory_config_map,
get_artifactory_secret,
get_http_image_url,
)
from utilities.constants import Images

@coderabbitai coderabbitai Bot Jul 16, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Test Execution Plan

  • Run smoke tests: False — the only changed module is marked post_upgrade; it has no smoke marker, and this PR does not modify shared utilities/, libs/, or conftest.py code that could transitively affect smoke coverage.
  • Run gating tests: False — the changed module has no gating marker, and there are no shared-code changes that could affect gating tests.
  • Affected tests to run:
    • tests/infrastructure/golden_images/update_boot_source/test_ssp_data_sources.py::test_upload_dv_for_auto_update_dangling_data_sources (post_upgrade) — exercises data_source_by_name_scope_function → uploaded_dv_for_dangling_data_source_scope_function → dv_for_data_source, including the new public-Quay Fedora registry import.
    • tests/infrastructure/golden_images/update_boot_source/test_ssp_data_sources.py::test_data_source_with_existing_golden_image_pvc (post_upgrade) — exercises data_source_by_name_scope_function → created_dv_for_data_import_cron_managed_data_source_scope_function → dv_for_data_source and the reduced unchanged-reference timeout.

Real tests (cluster required)

Error/fix path (public registry import):
pytest tests/infrastructure/golden_images/update_boot_source/test_ssp_data_sources.py::test_upload_dv_for_auto_update_dangling_data_sources
Expected: the Fedora DataVolume imports from public Quay and the DataSource condition reaches the parameterized expected state.

Happy path (existing golden-image PVC):
pytest tests/infrastructure/golden_images/update_boot_source/test_ssp_data_sources.py::test_data_source_with_existing_golden_image_pvc
Expected: the manually created Fedora DataVolume is consumed and the DataSource reference remains unchanged throughout the 30-second verification window.

from utilities.constants.hco import DATA_SOURCE_NAME
from utilities.constants.images import DEFAULT_FEDORA_REGISTRY_URL
from utilities.constants.pytest import QUARANTINED
from utilities.constants.storage import BIND_IMMEDIATE_ANNOTATION
from utilities.constants.storage import BIND_IMMEDIATE_ANNOTATION, REGISTRY_STR
from utilities.constants.timeouts import (
TIMEOUT_5MIN,
TIMEOUT_10MIN,
TIMEOUT_30SEC,
)
from utilities.exceptions import ResourceValueError
from utilities.ssp import wait_for_condition_message_value
Expand All @@ -41,26 +36,20 @@
TESTS_AUTO_UPDATE_BOOT_SOURCE_NAME = [*py_config["auto_update_data_source_matrix"][0]][0]
DUMMY_VOLUME_NAME = "dummy"
DATA_SOURCE_MANAGED_BY_CDI_LABEL = f"{DataSource.ApiGroup.CDI_KUBEVIRT_IO}/dataImportCron"
RECONCILE_TRIGGER_ANNOTATION = "reconcile-trigger"

@geetikakay geetikakay Jul 16, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Leftover ?


pytestmark = pytest.mark.post_upgrade


@contextmanager
def dv_for_data_source(name, data_source, admin_client):
artifactory_secret = get_artifactory_secret(namespace=data_source.namespace)
artifactory_config_map = get_artifactory_config_map(namespace=data_source.namespace)
with DataVolume(
client=admin_client,
name=name,
namespace=data_source.namespace,
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,
# underlying OS is not relevant
source_dict=construct_datavolume_source_dict(source=REGISTRY_STR, url=DEFAULT_FEDORA_REGISTRY_URL),
size=Images.Fedora.DEFAULT_DV_SIZE,
Comment on lines +51 to +52

@jpeimer jpeimer Jul 13, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't it be better to clone from a golden image to reduce the dependency on quay?

@RoniKishner RoniKishner Jul 13, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You mean from a DataSource?
It's not possible, the test idea is to create a DV while the boot sources are down, and then to bring them up again after the PVC is created.
So there no source to clone from when creating the DV.

Comment on lines 44 to +52

@coderabbitai coderabbitai Bot Jul 16, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win

Move local helpers and fixtures to dedicated modules.

HIGH severity. Defining helper functions and fixtures inside test_*.py files mixes test definitions with test infrastructure. To ensure clean test modules and reusability, they must be separated.

As per coding guidelines: "Do not define helper functions, utility functions, or classes in conftest.py or test_*.py; place them in dedicated utility modules... keep local helpers in <feature_dir>/utils.py, and local fixtures in <feature_dir>/conftest.py."

Please relocate these to their appropriate files. When moving the helpers, ensure they include Google-format docstrings as required for public functions.

  • tests/infrastructure/golden_images/update_boot_source/test_ssp_data_sources.py#L44-L52: Move the dv_for_data_source helper function to <feature_dir>/utils.py.
  • tests/infrastructure/golden_images/update_boot_source/test_ssp_data_sources.py#L89-L89: Move the wait_for_data_source_unchanged_referenced_volume helper function to <feature_dir>/utils.py.
  • tests/infrastructure/golden_images/update_boot_source/test_ssp_data_sources.py#L254-L255: Move the data_source_by_name_scope_function fixture to <feature_dir>/conftest.py.
📍 Affects 1 file
  • tests/infrastructure/golden_images/update_boot_source/test_ssp_data_sources.py#L44-L52 (this comment)
  • tests/infrastructure/golden_images/update_boot_source/test_ssp_data_sources.py#L89-L89
  • tests/infrastructure/golden_images/update_boot_source/test_ssp_data_sources.py#L254-L255
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@tests/infrastructure/golden_images/update_boot_source/test_ssp_data_sources.py`
around lines 44 - 52, Move dv_for_data_source and
wait_for_data_source_unchanged_referenced_volume from
tests/infrastructure/golden_images/update_boot_source/test_ssp_data_sources.py
(lines 44-52 and 89-89) into
tests/infrastructure/golden_images/update_boot_source/utils.py, adding
Google-format docstrings; move the data_source_by_name_scope_function fixture
from
tests/infrastructure/golden_images/update_boot_source/test_ssp_data_sources.py
(lines 254-255) into
tests/infrastructure/golden_images/update_boot_source/conftest.py, and update
imports or fixture discovery so existing tests continue using them.

Source: Coding guidelines

storage_class=py_config["default_storage_class"],
annotations=BIND_IMMEDIATE_ANNOTATION,
api_name="storage",
Expand All @@ -71,9 +60,6 @@ def dv_for_data_source(name, data_source, admin_client):
expected_message=DATA_SOURCE_READY_FOR_CONSUMPTION_MESSAGE,
)
yield dv
cleanup_artifactory_secret_and_config_map(
artifactory_secret=artifactory_secret, artifactory_config_map=artifactory_config_map
)


def opt_in_status_str(opt_in):
Expand All @@ -100,7 +86,7 @@ def wait_for_data_source_reconciliation_after_update(
def wait_for_data_source_unchanged_referenced_volume(data_source, volume_name):
try:
for sample in TimeoutSampler(
wait_timeout=TIMEOUT_10MIN,
wait_timeout=TIMEOUT_30SEC,
sleep=5,
func=lambda: data_source.source.name != volume_name,
):
Expand Down Expand Up @@ -265,8 +251,8 @@ def data_sources_from_templates_scope_function(admin_client, data_sources_names_


@pytest.fixture()
def data_source_by_name_scope_function(request, unprivileged_client, golden_images_namespace):
return DataSource(client=unprivileged_client, name=request.param, namespace=golden_images_namespace.name)
def data_source_by_name_scope_function(request, admin_client, golden_images_namespace):
return DataSource(client=admin_client, name=request.param, namespace=golden_images_namespace.name)


@pytest.fixture(scope="class")
Expand Down