[Storage] Refactor golden_image tests to use Fedora DataSource - #5922
Conversation
Signed-off-by: Emanuele Prella <eprella@redhat.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🔗 Linked repositories identifiedCodeRabbit considers these linked repositories for cross-repo context during reviews:
💤 Files with no reviewable changes (1)
📜 Recent review details🧰 Additional context used🔀 Multi-repo contextLinked repositories findingsRedHatQE/openshift-virtualization-tests-design-docs
No cross-repository consumers or API mismatches were identified. 📝 WalkthroughWalkthroughChangesThe golden image tests now use a module-scoped DataVolume cloned from the Fedora DataSource. The obsolete module-scoped fixture and RHEL parameterization were removed. Golden image tests
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 6✅ Passed checks (6 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
openshift-virtualization-qe-bot
added
tox:verify-tc-requirement-polarion:running
|
Report bugs in Issues Welcome! 🎉This pull request will be automatically processed with the following features: 🔄 Automatic Actions
📋 Available CommandsPR Status Management
Review & Approval
Testing & Validation
Container Operations
Cherry-pick Operations
Branch Management
Label Management
✅ Merge RequirementsThis PR will be automatically approved when the following conditions are met:
📊 Review ProcessApprovers and ReviewersApprovers:
Reviewers:
Available Labels
AI Features
Security Checks
💡 Tips
📌 Additional InformationCustom Commands:
For more information, please refer to the project documentation or contact the maintainers. |
openshift-virtualization-qe-bot
added
tox:verify-tc-requirement-polarion:passed
|
/retest all Auto-triggered: Files in this PR were modified by merged PR #5898. Overlapping filestests/conftest.py |
|
/approve |
|
/build-and-push-container |
|
New container for quay.io/openshift-cnv/openshift-virtualization-tests:pr-5922 published |
|
/verified All tests passed for PR #5922. Execution details |
Test execution plan request detailsCRITICAL: You MUST post an inline review comment on the first changed line of the first file. As an expert software testing engineer, analyze all modified files in this PR and create a targeted test execution plan. Analysis Requirements:
Output rules: Your deliverable: Test Execution Plan
Use these formats:
Real test commands (MANDATORY when changes affect session/runtime code): When the affected code runs at session/collection time (conftest fixtures, pytest plugins,
Example output for a session-startup fix: Guidelines:
Hardware-Related Checks (SR-IOV, GPU, DPDK): When PR modifies fixtures for hardware-specific resources:
CRITICAL WORKFLOW COMPLETION RULES: When responding to this test execution plan request, you MUST follow these rules EXACTLY:
ABSOLUTE PROHIBITIONS (violating these creates empty/meaningless reviews):
Remember: The pull request review is visible to users. Additional comments are redundant noise. CRITICAL — Verification rules for smoke and gating decisions: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '=== PR files and diff ==='
gh pr diff 5922 --name-only
gh pr diff 5922 -- tests/conftest.py tests/storage/golden_image/test_golden_image.py
echo '=== Symbol locations and consumers ==='
rg -n -C 5 '\b(golden_image_dv_from_fedora_datasource_scope_module|golden_image_data_volume_scope_module)\b' tests
echo '=== Fixture and test outlines ==='
ast-grep outline tests/storage/golden_image/test_golden_image.py --items all --type function --match 'golden_image|test_regular_user'
ast-grep outline tests/conftest.py --items all --type function --match 'golden_image_data_volume_scope_module'
echo '=== Markers in affected test module ==='
rg -n -C 3 '`@pytest`\.mark\.(smoke|gating)|def test_' tests/storage/golden_image/test_golden_image.py
echo '=== Marker file discovery ==='
printf '%s\n' '-- smoke --'
rg -l '`@pytest`\.mark\.smoke' tests/ || true
printf '%s\n' '-- gating --'
rg -l '`@pytest`\.mark\.gating' tests/ || trueLength of output: 1447 🏁 Script executed: #!/bin/bash
set -euo pipefail
base_ref="$(gh pr view 5922 --json baseRefName --jq '.baseRefName')"
git fetch origin "$base_ref" --quiet
base="origin/$base_ref"
echo '=== Changed files ==='
git diff --name-only "$base...HEAD"
echo '=== Changed hunks ==='
git diff --find-renames --unified=12 "$base...HEAD" -- \
tests/conftest.py \
tests/storage/golden_image/test_golden_image.py
echo '=== Fixture declarations and consumers ==='
rg -n -C 6 '\b(golden_image_dv_from_fedora_datasource_scope_module|golden_image_data_volume_scope_module)\b' tests
echo '=== Affected test functions and markers ==='
rg -n -C 4 '`@pytest`\.mark\.(smoke|gating)|^\s*def test_' \
tests/storage/golden_image/test_golden_image.py
echo '=== Smoke marker files ==='
rg -l '`@pytest`\.mark\.smoke' tests/ || true
echo '=== Gating marker files ==='
rg -l '`@pytest`\.mark\.gating' tests/ || true
echo '=== First changed line metadata ==='
gh api "repos/RedHatQE/openshift-virtualization-tests/pulls/5922/files?per_page=100" \
--jq '.[] | {filename, status, patch}'Length of output: 1468 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '=== Server-side PR file patches ==='
gh api --paginate "repos/RedHatQE/openshift-virtualization-tests/pulls/5922/files?per_page=100" \
--jq '.[] | "FILE: \(.filename)\nSTATUS: \(.status)\nPATCH:\n\(.patch // "<patch unavailable>")\n---"'
echo '=== Current fixture declarations and direct consumers ==='
rg -n -C 8 '\b(golden_image_dv_from_fedora_datasource_scope_module|golden_image_data_volume_scope_module)\b' tests
echo '=== Complete affected test module function and marker map ==='
ast-grep outline tests/storage/golden_image/test_golden_image.py --items all --type function
rg -n -C 4 '`@pytest`\.mark\.(smoke|gating)|^\s*def test_' \
tests/storage/golden_image/test_golden_image.py
echo '=== Shared conftest fixture context ==='
rg -n -C 12 '^\s*def (golden_image_data_volume_scope_module|golden_image_data_volume)\b' tests/conftest.py
echo '=== Marker file discovery ==='
printf '%s\n' '-- smoke --'
rg -l '`@pytest`\.mark\.smoke' tests/ || true
printf '%s\n' '-- gating --'
rg -l '`@pytest`\.mark\.gating' tests/ || trueLength of output: 17298 🏁 Script executed: #!/bin/bash
set -euo pipefail
repo="RedHatQE/openshift-virtualization-tests"
pr=5922
head_sha="$(gh pr view "$pr" --repo "$repo" --json headRefOid --jq '.headRefOid')"
body="$(cat <<'EOF'
**Test Execution Plan**
- **Run smoke tests: False** — No changed `utilities/` or `libs/` symbol has a verified dependency path to a smoke-marked test.
- **Run gating tests: False** — `test_regular_user_can_create_dv_in_ns_given_proper_rolebinding` is the only gating test in this module. It does not depend on `golden_image_dv_from_fedora_datasource_scope_module`.
**Affected tests to run**
- `tests/storage/golden_image/test_golden_image.py::test_regular_user_cant_delete_dv_from_cloned_dv` (`sno`, `s390x`)
- `tests/storage/golden_image/test_golden_image.py::test_regular_user_can_list_all_pvc_in_ns` (`sno`, `s390x`)
- `tests/storage/golden_image/test_golden_image.py::test_regular_user_cant_clone_dv_in_ns` (`sno`, `s390x`)
The three tests use `golden_image_dv_from_fedora_datasource_scope_module`. This module-scoped fixture creates a DataVolume from `fedora_data_source_scope_module`. The removed shared fixture no longer provides their setup.
**Real tests (cluster required)**
Error/fix path:
`pytest -s -o log_cli=true -m tier2 --jira tests/storage/golden_image/test_golden_image.py -k 'test_regular_user_cant_delete_dv_from_cloned_dv or test_regular_user_can_list_all_pvc_in_ns or test_regular_user_cant_clone_dv_in_ns' --storage-class-matrix=ocs-storagecluster-ceph-rbd-virtualization,hostpath-csi-pvc-block`
Expected: each test creates its source DataVolume from the Fedora DataSource. No Artifactory image URL is required.
Happy path:
`pytest -s -o log_cli=true -m tier2 --jira tests/storage/golden_image/test_golden_image.py::test_regular_user_cant_create_dv_in_ns --storage-class-matrix=ocs-storagecluster-ceph-rbd-virtualization,hostpath-csi-pvc-block`
Expected: the existing Fedora DataSource permission-denial test still runs successfully.
EOF
)"
gh api --method POST "repos/$repo/pulls/$pr/comments" \
-f body="$body" \
-f commit_id="$head_sha" \
-f path="tests/conftest.py" \
-F line=865 \
-f side="LEFT" \
--jq '{id, path, line, side}'Length of output: 2429
You are interacting with an AI system. |
|
/approve |
|
Successfully removed PR tag: quay.io/openshift-cnv/openshift-virtualization-tests:pr-5922. |
|
New container for quay.io/openshift-cnv/openshift-virtualization-tests:latest published |
|
/cherry-pick cnv-4.22 |
|
Cherry-pick conflicts were resolved by AI Cherry-picked PR [Storage] Refactor golden_image tests to use Fedora DataSource into cnv-4.22: #5977 @ema-aka-young Manual verification is required — please review the changes and test before merging. |
9 participants
What this PR does / why we need it:
This PR removes the remaining artifactory usage from the golden images.
The artifactory usage was indirect in utilities/storage.py ->
def data_volume where it constructed url = f"{get_test_artifact_server_url()}{image}" if source == "http" else NoneNew fixture golden_image_dv_from_fedora_datasource_scope_module uses source_ref (DataSource) instead of url (artifactory).
Unused golden_image_data_volume_scope_module was removed.
Which issue(s) this PR fixes:
Special notes for reviewer:
Based on #5160
jira-ticket:
Summary by CodeRabbit