[CCLM] Add cross-storage cross-cluster migration tests#4650
Conversation
|
Warning Review limit reached
More reviews will be available in 42 minutes and 34 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughCentralizes storage-class constants and the migration validation helper into shared ChangesShared utils consolidation and CCLM updates
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Suggested labels
Suggested reviewers
Caution Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional.
❌ Failed checks (2 errors)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsLinked repositories: Your configuration references 1 linked repositories, but your current plan allows 0. Analyzed ``, skipped 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 |
|
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
Label Management
✅ Merge RequirementsThis PR will be automatically approved when the following conditions are met:
📊 Review ProcessApprovers and ReviewersApprovers:
Reviewers:
Available Labels
AI Features
💡 Tips
For more information, please refer to the project documentation or contact the maintainers. |
openshift-virtualization-qe-bot-5
requested review from
dalia-frank and
ema-aka-young
openshift-virtualization-qe-bot-5
requested review from
josemacassan,
kgoldbla and
kshvaika
openshift-virtualization-qe-bot
added
tox:verify-tc-requirement-polarion:running
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/storage/cross_cluster_live_migration/test_cclm.py (1)
1-17:⚠️ Potential issue | 🟠 MajorHIGH: Add module-level traceability for the new CCLM suite.
This file adds a new feature test suite, but there is no module docstring with an STP link or Jira/RFE fallback. The repo requires that traceability for new feature tests so coverage can be tracked. As per coding guidelines, new feature tests MUST include an STP link in the module docstring; if no STP exists, include the Jira/RFE link instead.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/storage/cross_cluster_live_migration/test_cclm.py` around lines 1 - 17, The module lacks the required traceability docstring; add a module-level docstring at the top of the file (above imports) that includes the STP link for this CCLM test suite (or a Jira/RFE link if no STP exists) so the new feature tests (e.g., TESTS_CLASS_NAME_SEVERAL_VMS, TESTS_CLASS_NAME_WINDOWS_VM, TESTS_CLASS_NAME_STORAGE_A_TO_B) are traceable; ensure the docstring is succinct and clearly labeled as the STP/Jira reference for this test module.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@tests/storage/cross_cluster_live_migration/test_cclm.py`:
- Around line 1-17: The module lacks the required traceability docstring; add a
module-level docstring at the top of the file (above imports) that includes the
STP link for this CCLM test suite (or a Jira/RFE link if no STP exists) so the
new feature tests (e.g., TESTS_CLASS_NAME_SEVERAL_VMS,
TESTS_CLASS_NAME_WINDOWS_VM, TESTS_CLASS_NAME_STORAGE_A_TO_B) are traceable;
ensure the docstring is succinct and clearly labeled as the STP/Jira reference
for this test module.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: f86f8ea7-076e-4166-bebd-0c5d30ab449c
📒 Files selected for processing (8)
tests/storage/constants.pytests/storage/cross_cluster_live_migration/conftest.pytests/storage/cross_cluster_live_migration/test_cclm.pytests/storage/storage_migration/conftest.pytests/storage/storage_migration/constants.pytests/storage/storage_migration/test_storage_class_migration.pytests/storage/storage_migration/utils.pytests/storage/utils.py
💤 Files with no reviewable changes (2)
- tests/storage/storage_migration/constants.py
- tests/storage/storage_migration/utils.py
|
/build-and-push-container |
|
New container for quay.io/openshift-cnv/openshift-virtualization-tests:pr-4650 published |
|
/lgtm |
|
/verified openshift-virtualization-tests-runner/5084/ |
|
Polarion was down, now when it's up, I re-triggered the check manually, it passed: |
…zation-tests into cclm_cross_storage
|
Clean rebase detected — no code changes compared to previous head ( |
|
/retest all Auto-triggered: Files in this PR were modified by merged PR #5099. Overlapping filestests/storage/utils.py |
|
/retest all Auto-triggered: Files in this PR were modified by merged PR #5120. Overlapping filestests/storage/utils.py |
…zation-tests into cclm_cross_storage
|
/retest all Auto-triggered: Files in this PR were modified by merged PR #4556. Overlapping filestests/storage/utils.py |
Assisted-by: Claude <noreply@anthropic.com> Signed-off-by: Jenia Peimer <jpeimer@redhat.com>
…zation-tests into cclm_cross_storage
@coderabbitai |
|
To force an immediate re-evaluation of all checks against the current HEAD, you can request a full review: Or I can trigger it right now — just say the word. ✅ Action performedReview finished.
|
There was a problem hiding this comment.
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/cross_cluster_live_migration/test_cclm.py`:
- Around line 1-5: The module docstring in test_cclm.py currently references the
Jira epic as CNV-50823, but the PR description and commit message indicate the
correct epic is CNV-60016. Update the Jira epic URL in the module docstring at
the beginning of the file to reference CNV-60016 instead of CNV-50823 to ensure
consistency across the PR documentation and commit history.
🪄 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: d4003228-ffbb-455f-beaf-e87ab3cb2a89
📒 Files selected for processing (2)
tests/storage/cross_cluster_live_migration/test_cclm.pytests/storage/utils.py
The Jira link is an epic tracker, not a bug — skip the verify-bugs-are-open check so it is not flagged as unresolved. Assisted-by: Claude <noreply@anthropic.com> Signed-off-by: Jenia Peimer <jpeimer@redhat.com>
|
/build-and-push-container |
|
New container for quay.io/openshift-cnv/openshift-virtualization-tests:pr-4650 published |
|
/verified All tests passed for PR #4650. 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: |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
# Get HEAD commit SHA
git rev-parse HEAD
# Get first changed line of tests/storage/constants.py
git diff origin/main --unified=0 -- tests/storage/constants.py | head -20
# Check smoke tests
echo "=== Smoke test files ==="
rg -l '`@pytest.mark.smoke`' tests/
# Check gating tests
echo "=== Gating test files ==="
rg -l '`@pytest.mark.gating`' tests/
# Find callers of get_storage_class_for_storage_migration
echo "=== Callers of get_storage_class_for_storage_migration ==="
rg -n 'get_storage_class_for_storage_migration' tests/ --type=py -B2 -A2
# Check if any smoke/gating tests import from tests.storage.utils or tests.storage.constants
echo "=== Smoke/gating files importing from tests.storage.utils ==="
rg -l 'from tests.storage.utils|from tests\.storage import utils' tests/ --type=py
# Check storage_migration conftest callers
echo "=== storage_migration/conftest.py content for source/target storage class ==="
cat tests/storage/storage_migration/conftest.py | grep -A5 "storage_class"
# Check CCLM conftest for fixture names used by new tests
echo "=== CCLM conftest fixtures ==="
cat tests/storage/cross_cluster_live_migration/conftest.pyLength of output: 42417 🏁 Script executed: #!/bin/bash
gh api \
--method POST \
/repos/RedHatQE/openshift-virtualization-tests/pulls/4650/reviews \
--field commit_id="c8d00583cbf6b3a09a725b66fb118f951e1d0e8c" \
--field event="COMMENT" \
--field "comments[][path]=tests/storage/constants.py" \
--field "comments[][line]=25" \
--field "comments[][side]=RIGHT" \
--field "comments[][body]=**Test Execution Plan**
- **Run smoke tests: False**
Smoke tests (test_clone.py, test_upload_virtctl.py) import from \`tests.storage.utils\` but do not call \`get_storage_class_for_storage_migration\`. No traced dependency path from any smoke test to the modified/added symbols.
- **Run gating tests: False**
Gating tests (test_hotplug.py, test_clone.py, test_upload_virtctl.py, test_import_http.py) import from \`tests.storage.utils\` but do not call \`get_storage_class_for_storage_migration\`. No traced dependency path.
- **Affected tests to run:**
- \`-m cclm\` — covers \`TestCCLMFromStorageAtoB\` (new class), updated \`TestCCLMSeveralVMs\` and \`TestCCLMWindowsWithVTPM\` parametrizations in \`tests/storage/cross_cluster_live_migration/test_cclm.py\`; exercises new \`remote_cluster_source_storage_class\` / \`local_cluster_target_storage_class\` fixtures and updated \`local_cluster_mtv_storage_map\`
- \`tests/storage/storage_migration/test_storage_class_migration.py\` — import path for \`STORAGE_CLASS_A\`/\`STORAGE_CLASS_B\` moved from \`tests.storage.storage_migration.constants\` → \`tests.storage.constants\`"Length of output: 3282 |
|
/lgtm |
|
/check-can-merge |
|
/retest all |
1 similar comment
|
/retest all |
|
/security-override |
|
Security checks overridden by @rnetser. Security check runs set to pass. |
|
Successfully removed PR tag: quay.io/openshift-cnv/openshift-virtualization-tests:pr-4650. |
12 participants
Short description:
Jira: https://redhat.atlassian.net/browse/CNV-60016
split to: https://redhat.atlassian.net/browse/CNV-79287
More details:
What this PR does / why we need it:
Which issue(s) this PR fixes:
Special notes for reviewer:
jira-ticket:
Summary by CodeRabbit
New Features
Tests
Assisted-by: Claude noreply@anthropic.com