[main][storage] test coverage dataimportcron source pvc uptodated #597

Ahmad-Hafe · 2025-03-26T13:39:39Z

Short description:

qe coverage for new feature for dataimportcron, the feature is about:
A PVC from any namespace can be the source for a DataImportCron
Update detection when PVC is replaced

More details:

A PVC from any namespace can also be the source for a DataImportCron. The source digest is based on the PVC UID, which is polled according to the schedule, so when a new PVC is detected it will be imported.

What this PR does / why we need it:

adding test coverage to Verify_DataImportCron_with_PVC_source_is_ready after importing/cloning pvc

Which issue(s) this PR fixes:

addint test coverage for T2

Special notes for reviewer:

jira-ticket:

https://issues.redhat.com/browse/CNV-58329

cnv-qe-bot1 · 2025-03-26T13:39:57Z

Report bugs in Issues

The following are automatically added:

Add reviewers from OWNER file (in the root of the repository) under reviewers section.
Set PR size label.
New issue is created for the PR. (Closed when PR is merged/closed)
Run pre-commit if .pre-commit-config.yaml exists in the repo.

Available user actions:

To mark PR as WIP comment /wip to the PR, To remove it from the PR comment /wip cancel to the PR.
To block merging of PR comment /hold, To un-block merging of PR comment /hold cancel.
To mark PR as verified comment /verified to the PR, to un-verify comment /verified cancel to the PR.
verified label removed on each new commit push.
To cherry pick a merged PR comment /cherry-pick <target branch to cherry-pick to> in the PR.
- Multiple target branches can be cherry-picked, separated by spaces. (/cherry-pick branch1 branch2)
- Cherry-pick will be started when PR is merged
To build and push container image command /build-and-push-container in the PR (tag will be the PR number).
- You can add extra args to the Podman build command
  - Example: /build-and-push-container --build-arg OPENSHIFT_PYTHON_WRAPPER_COMMIT=<commit_hash>
To add a label by comment use /<label name>, to remove, use /<label name> cancel
To assign reviewers based on OWNERS file use /assign-reviewers
To check if PR can be merged use /check-can-merge
to assign reviewer to PR use /assign-reviewer @<reviewer>

Supported /retest check runs

/retest tox: Retest tox
/retest build-container: Retest build-container
/retest all: Retest all

Supported labels

hold
verified
wip
lgtm

geetikakay · 2025-03-26T13:42:12Z

tests/infrastructure/golden_images/update_boot_source/test_ssp_data_import_crons.py

-        unprivileged_client=unprivileged_client,
-        name="data-import-cron-using-default-sc",
-    )
+# @pytest.fixture()


geetikakay · 2025-03-26T13:42:39Z

tests/storage/cdi_upload/test_upload_virtctl.py

-            create_dummy_first_consumer_pod(pvc=pvc)
-        pvc.wait_for_status(status=PersistentVolumeClaim.Status.BOUND, timeout=60)
-        yield pvc
+# @pytest.fixture()


ditto..remove?

Ahmad-Hafe · 2025-03-26T13:46:08Z

/hold

redhat-cnv-qe-bot1 · 2025-03-26T13:46:26Z

Ahmad-Hafe is not part of the approver, only approvers can mark pull request with hold

polarion-jenkins

D/S test tox -e verify-bugs-are-open failed: cnv-tests-tox-executor/9375

jpeimer · 2025-04-03T13:02:19Z

tests/storage/data_import_cron/conftest.py

+        dv.wait_for_status(
+            status=DataVolume.Status.IMPORT_IN_PROGRESS,
+            timeout=TIMEOUT_5MIN,
+            stop_status=DataVolume.Status.SUCCEEDED,
+        )


To let it work with the WaitForFirstConsumer storage, let's remove this wait_for_status
And make sure your DV wasn't consumed (first consumer pod wasn't created)

jpeimer · 2025-04-03T13:09:28Z

tests/storage/data_import_cron/conftest.py

+                "sourceFormat": "pvc",
+                "volumeMode": storage_class_matrix__module__[storage_class_name_scope_module]["volume_mode"],
+                "storage": {
+                    "storageClassName": storage_class_name_scope_module,
+                    "accessModes": [storage_class_matrix__module__[storage_class_name_scope_module]["access_mode"]],
+                    "resources": {"requests": {"storage": "20Gi"}},


You can replace all this with only:

storage: storageClassName: storage_class_name_scope_module resources: requests: storage: dv_source_for_data_import_cron.size status: {}

jpeimer · 2025-04-03T13:09:44Z

tests/storage/data_import_cron/conftest.py

+@pytest.fixture()
+def dv_vm_for_data_import_cron(
+    data_import_cron_pvc_source_namespace, storage_class_name_scope_module, rhel9_http_image_url
+):


Please make separate fixtures for the DV and for the VM
You'll need the VM to write the data there
And you'll need the DV to access the PVC

jpeimer · 2025-04-03T13:11:32Z

tests/storage/data_import_cron/conftest.py

+def data_import_cron_pvc_source(
+    unprivileged_client,
+    data_import_cron_pvc_target_namespace,
+    storage_class_with_filesystem_volume_mode,


jpeimer · 2025-04-03T13:14:16Z

tests/storage/data_import_cron/conftest.py

+            }
+        },
+    ) as data_import_cron:
+        wait_for_condition_message_value(resource=data_import_cron, expected_message=LAST_IMPORT_IS_UP_TO_DATE_MESSAGE)


Can you use wait_for_condition? Something like

data_import_cron.wait_for_condition( condition=UpToDate, status=data_import_cron.Condition.Status.TRUE, )

Ahmad-Hafe · 2025-04-03T13:14:48Z

/wip cancel

jpeimer · 2025-04-03T13:18:27Z

tests/storage/utils.py

+
+def get_data_import_cron_conditions(namespace, name):
+    data_import_cron = DataImportCron(namespace=namespace, name=name)
+    for condition in data_import_cron.instance.status.get("conditions"):
+        if condition["type"] == "UpToDate":
+            return condition["status"]


If we will need this function, please move it to tests/storage/data_import_cron/utils.py

jpeimer · 2025-04-03T13:27:22Z

tests/storage/data_import_cron/conftest.py

+
+
+@pytest.fixture()
+def data_import_cron_pvc_source_namespace(unprivileged_client):


We can just use the namespace fixture for one of the namespaces

cnv-qe-bot1 added the size/L label Mar 26, 2025

cnv-qe-bot1 requested a review from dbasunag March 26, 2025 13:39

cnv-qe-bot1 mentioned this pull request Mar 26, 2025

test coverage dataimportcron source pvc uptodated - 597 #598

Open

cnv-qe-bot1 added the branch-main label Mar 26, 2025

cnv-qe-bot1 requested review from geetikakay, jpeimer, dshchedr, vsibirsk, duyanyan and RoniKishner March 26, 2025 13:39

polarion-jenkins added tox:verify-tc-requirement-polarion:running D/S Testing tox:verify-bugs-are-open:running D/S Testing tox:verify-tc-requirement-polarion:passed D/S Testing and removed tox:verify-tc-requirement-polarion:running D/S Testing labels Mar 26, 2025

geetikakay reviewed Mar 26, 2025

View reviewed changes

polarion-jenkins added tox:verify-bugs-are-open:passed D/S Testing and removed tox:verify-bugs-are-open:running D/S Testing labels Mar 26, 2025

polarion-jenkins previously approved these changes Mar 26, 2025

View reviewed changes

redhat-cnv-qe-bot1 added the commented-geetikakay label Mar 26, 2025

geetikakay reviewed Mar 26, 2025

View reviewed changes

redhat-cnv-qe-bot1 added the lgtm-polarion-jenkins label Mar 26, 2025

Ahmad-Hafe dismissed polarion-jenkins’s stale review via 6f8a0e9 March 26, 2025 13:45

Ahmad-Hafe force-pushed the PVC_SOURCE_DataImportCron_Test_coverage branch from 1b78428 to 6f8a0e9 Compare March 26, 2025 13:45

redhat-cnv-qe-bot1 removed commented-geetikakay lgtm-polarion-jenkins labels Mar 26, 2025

redhat-cnv-qe-bot1 requested a review from geetikakay March 26, 2025 13:45

Ahmad-Hafe changed the title ~~test coverage dataimportcron source pvc uptodated~~ [DO NOT REVIEW] test coverage dataimportcron source pvc uptodated Mar 26, 2025

polarion-jenkins requested changes Apr 3, 2025

View reviewed changes

redhat-cnv-qe-bot added the changes-requested-polarion-jenkins label Apr 3, 2025

jpeimer requested changes Apr 3, 2025

View reviewed changes

Ahmad-Hafe changed the title ~~[DO NOT REVIEW][storage] test coverage dataimportcron source pvc uptodated~~ [main][storage] test coverage dataimportcron source pvc uptodated Apr 3, 2025

redhat-cnv-qe-bot1 added the changes-requested-jpeimer label Apr 3, 2025

test coverage dataimportcron source pvc uptodated

0f1f66b

Ahmad-Hafe force-pushed the PVC_SOURCE_DataImportCron_Test_coverage branch from e2b742d to 0f1f66b Compare April 3, 2025 13:14

redhat-cnv-qe-bot removed changes-requested-jpeimer changes-requested-polarion-jenkins labels Apr 3, 2025

redhat-cnv-qe-bot requested a review from jpeimer April 3, 2025 13:15

polarion-jenkins approved these changes Apr 3, 2025

View reviewed changes

redhat-cnv-qe-bot added the lgtm-polarion-jenkins label Apr 3, 2025

jpeimer requested changes Apr 3, 2025

View reviewed changes

cnv-qe-bot1 added the changes-requested-jpeimer label Apr 3, 2025

jpeimer requested changes Apr 3, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[main][storage] test coverage dataimportcron source pvc uptodated #597

[main][storage] test coverage dataimportcron source pvc uptodated #597

Ahmad-Hafe commented Mar 26, 2025 •

edited

Loading

cnv-qe-bot1 commented Mar 26, 2025

geetikakay Mar 26, 2025

geetikakay Mar 26, 2025

Ahmad-Hafe commented Mar 26, 2025

redhat-cnv-qe-bot1 commented Mar 26, 2025

polarion-jenkins left a comment

jpeimer Apr 3, 2025

jpeimer Apr 3, 2025

jpeimer Apr 3, 2025

jpeimer Apr 3, 2025

jpeimer Apr 3, 2025

Ahmad-Hafe commented Apr 3, 2025

jpeimer Apr 3, 2025

jpeimer Apr 3, 2025



		@pytest.fixture()
		def data_import_cron_pvc_source_namespace(unprivileged_client):

[main][storage] test coverage dataimportcron source pvc uptodated #597

Are you sure you want to change the base?

[main][storage] test coverage dataimportcron source pvc uptodated #597

Conversation

Ahmad-Hafe commented Mar 26, 2025 • edited Loading

Short description:

More details:

What this PR does / why we need it:

Which issue(s) this PR fixes:

Special notes for reviewer:

jira-ticket:

cnv-qe-bot1 commented Mar 26, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Ahmad-Hafe commented Mar 26, 2025

redhat-cnv-qe-bot1 commented Mar 26, 2025

polarion-jenkins left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Ahmad-Hafe commented Apr 3, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Ahmad-Hafe commented Mar 26, 2025 •

edited

Loading