CherryPicked: [5.0] eus upgrade alignment#5167
Conversation
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
📝 WalkthroughWalkthroughReplaces implicit CNV-target derivation with explicit channel-aware upgrade-path builders, enforces new CLI flags/validation for CNV/EUS upgrades, rewrites product-upgrade fixtures to iterate phased CNV upgrades, shifts virt-launcher validation to image-set comparisons, and makes MCP wait timeouts configurable. ChangesEUS upgrade infrastructure overhaul
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
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
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-3
requested review from
SamAlber,
akri3i,
albarker-rh,
geetikakay,
rnetser and
vsibirsk
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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/install_upgrade_operators/product_upgrade/test_eus_upgrade.py`:
- Around line 37-38: Remove the unused fixtures from the test function signature
and register them as ordering-only fixtures via pytest.mark.usefixtures: add
'eus_unpaused_workload_update' and 'eus_unpaused_worker_mcp' to the existing
pytestmark usefixtures list so they still run but their return values aren’t
expected, and then delete those two parameters from the test function definition
in tests/install_upgrade_operators/product_upgrade/test_eus_upgrade.py.
In `@tests/install_upgrade_operators/product_upgrade/utils.py`:
- Around line 659-666: Add a brief clarifying comment and/or validation for the
direct-upgrade fallback where intermediate_step is None: in the block that
assigns non_eus_images (the else branch that currently sets non_eus_images =
{intermediate_version: {"cnv_image_url": intermediate_iib, "channel":
"stable"}}), annotate the assumption that a direct upgrade from current_version
to intermediate_version is allowed (e.g., adjacent z-stream direct upgrade) and,
if desired, add optional verification logic using build_version_images or
version explorer APIs to confirm current_version is a valid startVersion for
intermediate_version; reference intermediate_step, non_eus_images,
build_version_images, intermediate_version, and intermediate_iib when locating
where to add the comment/validation.
In `@tests/virt/upgrade/conftest.py`:
- Line 180: The call to wait_for_automatic_vm_migrations is missing the required
admin_client argument; update the call in the fixture so it passes the fixture's
admin_client into wait_for_automatic_vm_migrations (which expects vm_list and
admin_client) so the function receives both vm_list and admin_client parameters.
In `@tests/virt/upgrade/utils.py`:
- Around line 37-40: The error message in get_virt_launcher_images_from_csv
should use plural "digests" to match the set return type; update the ValueError
message thrown when no images are found from "Image digest for {VIRT_LAUNCHER}
not found" to "Image digests for {VIRT_LAUNCHER} not found" (leave VIRT_LAUNCHER
interpolation and surrounding logic unchanged).
🪄 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: 94a13f7e-ffad-4d83-9b59-e6025907f427
📒 Files selected for processing (11)
conftest.pydocs/INSTALL_AND_UPGRADE.mdtests/conftest.pytests/install_upgrade_operators/product_upgrade/conftest.pytests/install_upgrade_operators/product_upgrade/test_eus_upgrade.pytests/install_upgrade_operators/product_upgrade/utils.pytests/virt/upgrade/conftest.pytests/virt/upgrade/test_upgrade_virt.pytests/virt/upgrade/utils.pytox.iniutilities/operator.py
👮 Files not reviewed due to content moderation or server errors (3)
- tests/conftest.py
- tests/install_upgrade_operators/product_upgrade/conftest.py
- utilities/operator.py
hmeir
force-pushed
the
5.0-eus-upgrade-update
branch
from
a1dc447 to
ddbfb18
Compare
openshift-virtualization-qe-bot
added
tox:verify-tc-requirement-polarion:running
openshift-virtualization-qe-bot-3
removed
the
changes-requested-coderabbitai[bot]
label
|
/verified upgrade path detected correctly(latest we can get):
|
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: |
openshift-virtualization-qe-bot
added
lgtm-coderabbitai[bot]
and removed
changes-requested-coderabbitai[bot]
labels
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
# Find smoke-marked test files
echo "=== SMOKE TESTS ==="
rg -l '`@pytest.mark.smoke`' tests/
echo ""
echo "=== GATING TESTS ==="
rg -l '`@pytest.mark.gating`' tests/
echo ""
echo "=== Callers of wait_for_mcp_update_completion ==="
rg -rn 'wait_for_mcp_update_completion' tests/ --type py
echo ""
echo "=== Callers of wait_for_mcp_update_end ==="
rg -rn 'wait_for_mcp_update_end' tests/ --type py
echo ""
echo "=== Callers of validate_vms_pod_updated ==="
rg -rn 'validate_vms_pod_updated' tests/ --type py
echo ""
echo "=== Callers of get_virt_launcher_image_from_csv / get_virt_launcher_images_from_csv ==="
rg -rn 'get_virt_launcher_image' tests/ --type py
echo ""
echo "=== Callers of build_eus_upgrade_path_dict ==="
rg -rn 'build_eus_upgrade_path_dict' tests/ --type py
echo ""
echo "=== Callers of perform_cnv_upgrade ==="
rg -rn 'perform_cnv_upgrade' tests/ --type py
echo ""
echo "=== Callers of get_build_info_by_version ==="
rg -rn 'get_build_info_by_version' tests/ --type py
echo ""
echo "=== Callers of get_upgrade_path ==="
rg -rn 'get_upgrade_path' tests/ --type py
echo ""
echo "=== First file in the PR diff (first changed line) ==="
git diff HEAD~1 --name-only | head -5
echo ""
echo "=== First line changed in conftest.py ==="
git diff HEAD~1 -- conftest.py | head -40
echo ""
echo "=== Check virt upgrade test for markers ==="
rg '`@pytest.mark`' tests/virt/upgrade/test_upgrade_virt.py
echo ""
echo "=== Check eus upgrade test for markers ==="
rg '`@pytest.mark`' tests/install_upgrade_operators/product_upgrade/test_eus_upgrade.py
echo ""
echo "=== Smoke tests in virt/upgrade/ hierarchy ==="
rg '`@pytest.mark.smoke`' tests/virt/upgrade/ tests/install_upgrade_operators/
echo ""
echo "=== Gating tests in affected test dirs ==="
rg '`@pytest.mark.gating`' tests/virt/upgrade/ tests/install_upgrade_operators/Length of output: 15032 🏁 Script executed: #!/bin/bash
# Get exact line number of first changed line in conftest.py
git diff HEAD~1 -- conftest.py | head -60
echo "---"
# Check the actual line number
grep -n 'from packaging.version import Version' conftest.py
echo "---"
# Get PR info for posting inline review
gh pr view 5167 --json number,headRefOid --repo RedHatQE/openshift-virtualization-testsLength of output: 3306 🏁 Script executed: #!/bin/bash
# Post inline review comment on conftest.py line 24 (first changed line)
gh api \
--method POST \
-H "Accept: application/vnd.github+json" \
/repos/RedHatQE/openshift-virtualization-tests/pulls/5167/reviews \
--field commit_id="082480f8600f74432327aa888273bcc286247710" \
--field event="COMMENT" \
--field "body=" \
--field "comments[][path]=conftest.py" \
--field "comments[][line]=24" \
--field "comments[][side]=RIGHT" \
--field "comments[][body]=## Test Execution Plan
**Run smoke tests: False**
No traced path from any smoke-marked test to the modified symbols. The \`conftest.py\` root-level changes are scoped to upgrade-lane startup validation (\`pytest_cmdline_main\`, \`pytest_addoption\`), which only activate under \`--upgrade eus/cnv\`. The smoke tests in \`tests/install_upgrade_operators/csv/test_csv.py\` and other smoke files do not invoke those code paths.
**Run gating tests: True**
Traced dependency paths:
- \`tests/virt/upgrade/test_upgrade_virt.py\` (gating) → \`virt_launcher_images_from_csv_before_upgrade\` fixture (\`tests/virt/upgrade/conftest.py\`) → \`get_virt_launcher_images_from_csv()\` (\`tests/virt/upgrade/utils.py\`) — function renamed from \`get_virt_launcher_image_from_csv\` and now returns \`set[str]\` instead of a single string
- \`tests/install_upgrade_operators/product_upgrade/test_upgrade.py\` (gating) → CNV upgrade fixtures in \`tests/install_upgrade_operators/product_upgrade/conftest.py\` → \`perform_cnv_upgrade()\` (\`tests/install_upgrade_operators/product_upgrade/utils.py\`) — signature extended with optional \`subscription\`, \`subscription_source\`, \`subscription_channel\` params
**Affected tests to run**
- \`tests/install_upgrade_operators/product_upgrade/test_eus_upgrade.py\` — directly modified; EUS fixture wiring fully rewritten (\`eus_cnv_upgrade_path\`, \`source_eus_to_non_eus_cnv_upgraded\`, \`non_eus_to_target_eus_cnv_upgraded\` all reworked)
- \`tests/virt/upgrade/test_upgrade_virt.py\` (gating) — fixture \`virt_launcher_images_from_csv_before_upgrade\` renamed and refactored; \`validate_vms_pod_updated\` now takes \`set[str]\` instead of single image
- \`tests/install_upgrade_operators/product_upgrade/test_upgrade.py\` (gating) — uses CNV upgrade conftest fixtures calling \`perform_cnv_upgrade\` with new optional params
**Real tests (cluster required)**
*Error path (verifying old \`--eus-cnv-target-version\` is gone, new contract enforced):*
\`\`\`
pytest tests/install_upgrade_operators/product_upgrade/test_eus_upgrade.py \
--upgrade eus --cnv-version=4.22.0 --cnv-image=<iib_url> --eus-ocp-images=<img1>,<img2> \
--collect-only
\`\`\`
Expected: collection succeeds; no \`ValueError\` about missing flags; old \`--eus-cnv-target-version\` flag should cause an error if passed
*Odd-minor-version guard:*
\`\`\`
pytest tests/install_upgrade_operators/product_upgrade/test_eus_upgrade.py \
--upgrade eus --cnv-version=4.21.0 --cnv-image=<iib_url> --eus-ocp-images=<img1>,<img2>
\`\`\`
Expected: \`ValueError: EUS target version 4.21.0 must have an even minor version\`
*Happy path (CNV upgrade lane regression):*
\`\`\`
pytest tests/install_upgrade_operators/product_upgrade/test_upgrade.py \
--upgrade cnv --cnv-version=<ver> --cnv-image=<iib_url> --collect-only
\`\`\`
Expected: session starts normally without errors"Length of output: 5138 |
|
/lgtm |
|
/approve |
|
/retest all Auto-triggered: Files in this PR were modified by merged PR #4808. Overlapping filestests/conftest.py |
|
/retest tox |
|
/check-can-merge |
|
New container for quay.io/openshift-cnv/openshift-virtualization-tests:latest published |
10 participants
Short description:
Cherry-pick of #5055 to align EUS upgrade automation with the new
--cnv-version/--cnv-imageCLI contract,replacing the removed
--eus-cnv-target-versionflag and updating fixture wiring and docs accordingly.jira-ticket:
https://redhat.atlassian.net/browse/CNV-89569
Summary by CodeRabbit
New Features
Bug Fixes / Validation
Documentation
Refactor