CherryPicked: [4.22] eus upgrade alignment#5168
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? |
📝 WalkthroughWalkthroughThe PR removes the ChangesEUS Upgrade Orchestration Rework
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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-2
requested review from
RoniKishner and
vsibirsk
openshift-virtualization-qe-bot-2
requested review from
SamAlber,
dshchedr and
rnetser
openshift-virtualization-qe-bot-2
requested review from
OhadRevah,
akri3i,
albarker-rh and
rlobillo
Signed-off-by: Harel Meir <hmeir@redhat.com>
hmeir
force-pushed
the
4.22-eus-upgrade-update
branch
from
74371d9 to
e220bea
Compare
openshift-virtualization-qe-bot
added
tox:verify-tc-requirement-polarion:running
|
/retest all Auto-triggered: Files in this PR were modified by merged PR #5156. Overlapping filestests/conftest.py |
|
/verified
|
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 Changed ==="
gh pr view 5168 --repo RedHatQE/openshift-virtualization-tests --json files --jq '.files[].path'
echo ""
echo "=== PR Diff ==="
gh pr diff 5168 --repo RedHatQE/openshift-virtualization-testsLength of output: 36094 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "=== Smoke-marked test files ==="
rg -l '`@pytest.mark.smoke`' tests/
echo ""
echo "=== Gating-marked test files ==="
rg -l '`@pytest.mark.gating`' tests/
echo ""
echo "=== Direct callers of wait_for_mcp_update_completion in tests/ ==="
rg -rn 'wait_for_mcp_update_completion' tests/
echo ""
echo "=== Direct callers of wait_for_mcp_update_end in tests/ ==="
rg -rn 'wait_for_mcp_update_end' tests/
echo ""
echo "=== Callers of wait_for_mcp_update_completion in utilities/ ==="
rg -rn 'wait_for_mcp_update_completion' utilities/
echo ""
echo "=== Who imports eus_target_cnv_version (removed fixture) ==="
rg -rn 'eus_target_cnv_version' tests/
echo ""
echo "=== Who imports eus_hco_target_csv_name (removed fixture) ==="
rg -rn 'eus_hco_target_csv_name' tests/
echo ""
echo "=== Who imports get_virt_launcher_image_from_csv (renamed function) ==="
rg -rn 'get_virt_launcher_image_from_csv' tests/
echo ""
echo "=== Who uses virt_launcher_from_csv_before_upgrade fixture ==="
rg -rn 'virt_launcher_from_csv_before_upgrade' tests/
echo ""
echo "=== Who uses cnv_channel fixture ==="
rg -rn 'cnv_channel' tests/
echo ""
echo "=== Who calls validate_vms_pod_updated ==="
rg -rn 'validate_vms_pod_updated' tests/
echo ""
echo "=== EUS upgrade test file markers ==="
rg -n 'pytest.mark' tests/install_upgrade_operators/product_upgrade/test_eus_upgrade.py
echo ""
echo "=== Virt upgrade test file markers ==="
rg -n 'pytest.mark' tests/virt/upgrade/test_upgrade_virt.py | head -30Length of output: 5567 🏁 Script executed: #!/bin/bash
set -euo pipefail
# Get the PR details to find the commit SHA for inline comment
gh api repos/RedHatQE/openshift-virtualization-tests/pulls/5168 --jq '{head_sha: .head.sha, base_sha: .base.sha}'
echo ""
echo "=== List commits in PR ==="
gh api repos/RedHatQE/openshift-virtualization-tests/pulls/5168/commits --jq '.[].sha'Length of output: 561 🏁 Script executed: #!/bin/bash
set -euo pipefail
# Post inline review comment on the first changed line of conftest.py (line 24, the added `from packaging.version import Version`)
gh api \
--method POST \
repos/RedHatQE/openshift-virtualization-tests/pulls/5168/reviews \
--field commit_id="e220bea976cdefc37faa051e83f8881087e403e7" \
--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 smoke-marked test file traces through the modified symbols (\`wait_for_mcp_update_completion\`, \`wait_for_mcp_update_end\`, EUS fixture chain). Smoke files (e.g. \`tests/install_upgrade_operators/csv/test_csv.py\`) do not call into upgrade-path utilities.
**Run gating tests: True**
\`tests/virt/upgrade/test_upgrade_virt.py\` → \`virt_launcher_images_from_csv_before_upgrade\` fixture (renamed from \`virt_launcher_from_csv_before_upgrade\`) → \`get_virt_launcher_images_from_csv\` (changed from single image to set). This file carries \`@pytest.mark.gating\`.
**Affected tests to run**
- \`tests/install_upgrade_operators/product_upgrade/test_eus_upgrade.py\` — directly modified; EUS upgrade path completely refactored (\`eus_cnv_upgrade_path\` now calls \`build_eus_upgrade_path_dict\`, removed \`eus_target_cnv_version\`/\`eus_hco_target_csv_name\` fixtures, \`created_target_hco_csv\` replaces \`eus_created_target_hco_csv\`)
- \`tests/virt/upgrade/test_upgrade_virt.py\` — \`virt_launcher_images_from_csv_before_upgrade\` fixture rename + \`csv_after_upgrade\` fixture no longer falls back to \`eus_hco_target_csv_name\` (gating)
- \`utilities/unittests/test_operator.py\` — unit test updated for \`wait_for_mcp_update_end\` new \`timeout\` default (already updated in this PR; run to confirm)
**Real tests (cluster required)**
\`pytest_cmdline_main\` now validates EUS args at session startup — must verify both paths on a real cluster:
Error path (odd minor version rejected):
\`\`\`
pytest --tc-file=tests/global_config.py --upgrade=eus --cnv-version=4.21.0 --cnv-image=NA --eus-ocp-images=NA,NA --collect-only
\`\`\`
Expected: \`ValueError: EUS target version 4.21.0 must have an even minor version\`
Happy path (even minor passes validation):
\`\`\`
pytest --tc-file=tests/global_config.py --upgrade=eus --cnv-version=4.22.0 --cnv-image=NA --eus-ocp-images=NA,NA --collect-only
\`\`\`
Expected: session starts normally, no ValueError
Full EUS upgrade suite:
\`\`\`
pytest -m eus_upgrade --upgrade eus --cnv-version <target_version> --cnv-image <cnv_image> --eus-ocp-images <4.y+1.z_image>,<4.y+2.z_image>
\`\`\`"Length of output: 4486 |
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@conftest.py`:
- Line 24: This PR has an unresolved review thread that is blocking merge.
Navigate to the pull request review comments and locate the unresolved
conversation thread. Address all feedback, concerns, or questions raised in that
thread by either making the requested code changes, providing clarification, or
reaching consensus with the reviewer. Once the conversation is fully addressed
and the thread is marked as resolved, the can-be-merged check should pass and
the PR can proceed to merge.
In `@tests/install_upgrade_operators/product_upgrade/utils.py`:
- Around line 656-668: The else block in the upgrade path handling is creating a
fallback direct upgrade to the intermediate version without validation when
find_path_with_start_version returns None. This bypasses the version-explorer
path validation and allows unsupported upgrade sequences to proceed. Remove the
else block that creates the fallback non_eus_images dictionary and instead raise
an exception or fail the operation when intermediate_step is None, ensuring that
only validated upgrade paths from GetUpgradePath are used.
🪄 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: 15c8923c-993a-4b12-939d-a2f4b5582dc4
📒 Files selected for processing (12)
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.pyutilities/unittests/test_operator.py
openshift-virtualization-qe-bot-3
added
lgtm-coderabbitai[bot]
and removed
changes-requested-coderabbitai[bot]
labels
|
/check-can-merge |
|
/lgtm |
|
/approve |
|
/retest security-committer-identity /retest security-suspicious-paths |
|
New container for quay.io/openshift-cnv/openshift-virtualization-tests:cnv-4.22 published |
10 participants
Short description:
Cherry-pick from #5055
More details:
What this PR does / why we need it:
Which issue(s) this PR fixes:
Special notes for reviewer:
jira-ticket:
https://redhat.atlassian.net/browse/CNV-89570
Summary by CodeRabbit
New Features
--cnv-version,--cnv-image,--cnv-channel, and--eus-ocp-imagesparameters.Documentation
Bug Fixes