[Storage] Remove skip_block_volumemode from archive import test

[jpeimer]

What this PR does / why we need it:

• Archive import only supports Filesystem volume mode, so explicitly set it on the DV instead of skipping Block volume mode at runtime
• Remove the unused skip_block_volumemode_scope_module fixture.
• Use PVC volume mode in running_pod_with_dv_pvc fixture
• Ignore 'lost+found' file that may be created by some storage providers

Assisted-by: Claude noreply@anthropic.com

Which issue(s) this PR fixes:

Remove programmatic skip

Special notes for reviewer:

jira-ticket:

Summary by CodeRabbit

• Tests
  • Enhanced test configuration to support volume mode settings for HTTP import scenarios.
  • Simplified test fixture dependencies to improve maintainability.

[qodo-code-review]

Qodo reviews are paused for this user.

Troubleshooting steps vary by plan Learn more →

On a Teams plan?
Reviews resume once this user has a paid seat and their Git account is linked in Qodo.
Link Git account →

Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center?
These require an Enterprise plan - Contact us
Contact us →

[coderabbitai]

📝 Walkthrough

Walkthrough

Fixtures now accept and propagate a per-test volume_mode for HTTP DataVolume imports; tests specify FILE for archive imports; running pod fixture reads volumeMode from the created DV PVC; test file-counting helper filters ls output before asserting.

Changes

HTTP Import Volume Mode Configuration

Layer / File(s)	Summary
Fixture: forward volume_mode into DataVolume creation tests/storage/cdi_import/conftest.py	dv_from_http_import reads volume_mode from request.param and passes it into create_dv.
Fixture: derive pod volume_mode from DV PVC tests/storage/cdi_import/conftest.py	running_pod_with_dv_pvc now consumes dv_from_http_import and sets create_pod_for_pvc's volume_mode from dv_from_http_import.pvc.instance.spec.volumeMode.
Tests: declare FILE volume_mode and remove unused fixture tests/storage/cdi_import/test_import_http.py	CNV-2338 test config adds "volume_mode": DataVolume.VolumeMode.FILE for archive import; test_successful_import_secure_archive removes the skip_block_volumemode_scope_module fixture from its signature.
Test util: robust file counting in pod tests/storage/utils.py	assert_num_files_in_pod now parses ls -1 /pvc into a filtered file list (excluding empty lines and lost+found) and asserts its length matches expected.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related issues

• [Storage] Remove skip_block_volumemode from archive import test - 5099 #5100 — same test case and volume_mode fixture changes appear connected to the issue's objective.

Suggested reviewers

• kshvaika
• acinko-rh
• Ahmad-Hafe
• josemacassan
• ema-aka-young
• kgoldbla
• dalia-frank

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and accurately describes the main change: removing the skip_block_volumemode fixture from the archive import test.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Stp Link Required	✅ Passed	No new test files (test_.py) or new test functions (def test_) added; only modifications to existing test function test_successful_import_secure_archive.
Description check	✅ Passed	Pull request description covers key changes and reasoning but lacks a Jira ticket link despite the template requirement.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[openshift-virtualization-qe-bot-4]

Report bugs in Issues

Welcome! 🎉

This pull request will be automatically processed with the following features:

🔄 Automatic Actions

• Reviewer Assignment: Reviewers are automatically assigned based on the OWNERS file in the repository root
• Size Labeling: PR size labels (XS, S, M, L, XL, XXL) are automatically applied based on changes
• Issue Creation: A tracking issue is created for this PR and will be closed when the PR is merged or closed
• Branch Labeling: Branch-specific labels are applied to track the target branch
• Auto-verification: Auto-verified users have their PRs automatically marked as verified
• Labels: Enabled categories: branch, can-be-merged, cherry-pick, has-conflicts, hold, needs-rebase, size, verified, wip

📋 Available Commands

PR Status Management

• /wip - Mark PR as work in progress (adds WIP: prefix to title)
• /wip cancel - Remove work in progress status
• /hold - Block PR merging (approvers only)
• /hold cancel - Unblock PR merging
• /verified - Mark PR as verified
• /verified cancel - Remove verification status
• /reprocess - Trigger complete PR workflow reprocessing (useful if webhook failed or configuration changed)
• /regenerate-welcome - Regenerate this welcome message

Review & Approval

• /lgtm - Approve changes (looks good to me)
• /approve - Approve PR (approvers only)
• /assign-reviewers - Assign reviewers based on OWNERS file
• /assign-reviewer @username - Assign specific reviewer
• /check-can-merge - Check if PR meets merge requirements

Testing & Validation

• /retest tox - Run Python test suite with tox
• /retest build-container - Rebuild and test container image
• /retest verify-bugs-are-open - verify-bugs-are-open
• /retest all - Run all available tests

Container Operations

• /build-and-push-container - Build and push container image (tagged with PR number)
  • Supports additional build arguments: /build-and-push-container --build-arg KEY=value

Cherry-pick Operations

• /cherry-pick <branch> - Schedule cherry-pick to target branch when PR is merged
  • Multiple branches: /cherry-pick branch1 branch2 branch3

Label Management

• /<label-name> - Add a label to the PR
• /<label-name> cancel - Remove a label from the PR

✅ Merge Requirements

This PR will be automatically approved when the following conditions are met:

1. Approval: /approve from at least one approver
2. LGTM Count: Minimum 2 /lgtm from reviewers
3. Status Checks: All required status checks must pass
4. No Blockers: No wip, hold, has-conflicts labels and PR must be mergeable (no conflicts)
5. Verified: PR must be marked as verified

📊 Review Process

Approvers and Reviewers

Approvers:

• jpeimer

Reviewers:

• Ahmad-Hafe
• acinko-rh
• dalia-frank
• ema-aka-young
• josemacassan
• jpeimer
• kgoldbla
• kshvaika

Available Labels

• hold
• verified
• wip
• lgtm
• approve

AI Features

• Cherry-Pick Conflict Resolution: Enabled (claude/claude-opus-4-6[1m])

💡 Tips

• WIP Status: Use /wip when your PR is not ready for review
• Verification: The verified label is removed on new commits unless the push is detected as a clean rebase
• Cherry-picking: Cherry-pick labels are processed when the PR is merged
• Container Builds: Container images are automatically tagged with the PR number
• Permission Levels: Some commands require approver permissions
• Auto-verified Users: Certain users have automatic verification and merge privileges

For more information, please refer to the project documentation or contact the maintainers.

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with 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.

Inline comments:
In `@tests/storage/cdi_import/test_import_http.py`:
- Line 135: The test test_successful_import_secure_archive forces the DataVolume
to Filesystem but running_pod_with_dv_pvc still uses the storage-class default
volume_mode when calling create_pod_for_pvc, causing consumer/producer mismatch;
update the test so the pod creation uses the actual PVC/DV volume_mode (e.g.,
retrieve the DataVolume or resulting PVC volumeMode after create_dv and pass
that explicit value into create_pod_for_pvc) or explicitly pass
volume_mode='Filesystem' into create_pod_for_pvc to ensure create_dv and
create_pod_for_pvc agree on the PVC mode.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 8f770483-a447-4e23-90a1-070b8ba67e5f

📥 Commits

Reviewing files that changed from the base of the PR and between b1117fc and 36afeb0.

📒 Files selected for processing (3)

• tests/storage/cdi_import/conftest.py
• tests/storage/cdi_import/test_import_http.py
• tests/storage/conftest.py

💤 Files with no reviewable changes (1)

• tests/storage/conftest.py

[openshift-virtualization-qe-bot]

/build-and-push-container

[openshift-virtualization-qe-bot-3]

New container for quay.io/openshift-cnv/openshift-virtualization-tests:pr-5099 published

[openshift-virtualization-qe-bot]

Verification failed for PR #5099 (image: openshift-virtualization-tests:pr-5099).
Result: UNSTABLE
Job: openshift-virtualization-tests-runner #5451

[coderabbitai]

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Find smoke and gating marked tests
echo "=== SMOKE TESTS ==="
rg -l '`@pytest.mark.smoke`' tests/ 2>/dev/null

echo ""
echo "=== GATING TESTS ==="
rg -l '`@pytest.mark.gating`' tests/ 2>/dev/null

echo ""
echo "=== assert_num_files_in_pod callers ==="
rg -n 'assert_num_files_in_pod' tests/ --type py -C2

echo ""
echo "=== running_pod_with_dv_pvc callers ==="
rg -n 'running_pod_with_dv_pvc' tests/ --type py -C2

echo ""
echo "=== dv_from_http_import callers ==="
rg -n 'dv_from_http_import' tests/ --type py -C2

echo ""
echo "=== PR diff to find first changed line ==="
git diff HEAD~2..HEAD -- tests/storage/cdi_import/conftest.py | head -60

Length of output: 12064

---

🏁 Script executed:

#!/bin/bash
# Get the actual diff
git log --oneline -5
echo "---"
git diff HEAD~1..HEAD -- tests/storage/cdi_import/conftest.py | head -80
echo "=== test_import_http markers ==="
rg -n '`@pytest.mark`\.' tests/storage/cdi_import/test_import_http.py
echo "=== conftest.py full relevant section ==="
grep -n '' tests/storage/cdi_import/conftest.py | head -120

Length of output: 5422

---

🏁 Script executed:

#!/bin/bash
# Get PR details and diff to find the right commit SHA and line for inline comment
gh pr view 5099 --json commits,headRefOid,baseRefOid | jq '{headRefOid, baseRefOid, commits: [.commits[] | {oid, messageHeadline}]}'
echo "==="
# Get files changed in PR
gh pr diff 5099 -- tests/storage/cdi_import/conftest.py 2>/dev/null | head -60

Length of output: 1078

---

🏁 Script executed:

#!/bin/bash
# Get the full PR diff
gh api repos/RedHatQE/openshift-virtualization-tests/pulls/5099/files \
  --jq '.[] | select(.filename | contains("cdi_import/conftest")) | {filename, patch: .patch[0:500]}' 

echo "==="
# Get the raw diff between base and head
git fetch origin main b1117fc37b9d071233f9d43518c46c1102404c4a 2>/dev/null || true
git diff b1117fc37b9d071233f9d43518c46c1102404c4a 0455bd443f42fe66e53787e8465c661df9bc0ee0 -- tests/storage/cdi_import/conftest.py 2>/dev/null | head -80

Length of output: 2371

---

🏁 Script executed:

#!/bin/bash
# Post inline review comment on the first changed line of tests/storage/cdi_import/conftest.py
BODY="**Test Execution Plan**

**Run smoke tests: False** — No smoke-marked tests use the modified fixtures or \`assert_num_files_in_pod\`.

**Run gating tests: True** — \`dv_from_http_import\` fixture now forwards \`volume_mode\` to \`create_dv\`; two gating tests depend on it directly:
- \`test_successful_import_image\` → \`dv_from_http_import\` (fixture changed: \`volume_mode=request.param.get(\"volume_mode\")\` added to \`create_dv\` call)
- \`test_successful_import_secure_image\` → \`dv_from_http_import\` (same path)

**Affected tests to run:**
- \`tests/storage/cdi_import/test_import_http.py::test_successful_import_image\` (gating)
- \`tests/storage/cdi_import/test_import_http.py::test_successful_import_secure_image\` (gating)
- \`tests/storage/cdi_import/test_import_http.py::test_successful_import_secure_archive\` — fixture chain changed (\`running_pod_with_dv_pvc\` now reads \`volumeMode\` from actual PVC spec; \`assert_num_files_in_pod\` now filters \`lost+found\`)"

gh api \
  repos/RedHatQE/openshift-virtualization-tests/pulls/5099/comments \
  -X POST \
  -f body="$BODY" \
  -f commit_id="0455bd443f42fe66e53787e8465c661df9bc0ee0" \
  -f path="tests/storage/cdi_import/conftest.py" \
  -f side="RIGHT" \
  -F line=95

Length of output: 5174

---

---

[kshvaika]

/lgtm

[openshift-virtualization-qe-bot-3]

/retest all

Auto-triggered: Files in this PR were modified by merged PR #4985.

Overlapping files

tests/storage/conftest.py

[josemacassan]

/lgtm

[jpeimer]

polarion check was missing, ran it manually:
cnv-tests-tox-executor/28777

@rnetser FYI

[rnetser]

/approve
/lgtm

[rnetser]

/approve
/lgtm

[openshift-virtualization-qe-bot-2]

Successfully removed PR tag: quay.io/openshift-cnv/openshift-virtualization-tests:pr-5099.

[openshift-virtualization-qe-bot-2]

New container for quay.io/openshift-cnv/openshift-virtualization-tests:latest published