INTEROP-9317: Add OPP LP interop variant and CR view for OCP 4.22 - #3865
Conversation
Add layered product pattern for OPP multi-LP interop jobs (ACM + ODF + Quay), CR dashboard view 4.22-LP-Interop--OPP, and unit tests for job name classification. Tracked by: INTEROP-9317 | Epic: INTEROP-9202 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Pipeline controller notification For optional jobs, comment This repository is configured in: automatic mode |
|
@amp-rh: This pull request references INTEROP-9317 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "5.0.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
Skipping CI for Draft Pull Request. |
WalkthroughThe change classifies ChangesOPP interoperability support
Estimated code review effort: 2 (Simple) | ~15 minutes Suggested reviewers: Caution Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional.
❌ Failed checks (1 error, 1 warning)
✅ Passed checks (19 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 |
|
/test all |
Review — INTEROP-9317: Add OPP LP interop variant and CR view for OCP 4.22Overall this is a clean, well-structured PR that follows existing conventions in the codebase. The four changes (pattern matcher, tests, snapshot, view config) are internally consistent. A few observations and suggestions below. ✅ Looks Good
💡 Suggestions / Questions
SummaryThe code changes are correct and follow established patterns. The suggestions above are minor improvements to test coverage and completeness — none are blockers. Nice work! 👍 AI-generated. Review for accuracy. |
amp-rh
left a comment
There was a problem hiding this comment.
Chai Bot Review
Automated review via Chai Bot
Summary
This PR correctly adds OPP (ACM + ODF + Quay) multi-LP interop job classification and a Component Readiness dashboard view for OCP 4.22.
What Works Well
- Pattern placement is correct.
"-interop-opp-"is inserted before-virt/-cnv/-kubevirtin the first-match-wins list, preventing OPP jobs from being misclassified. - Trailing-dash guard. The trailing dash in
"-interop-opp-"prevents false matches on hypothetical jobs like-interop-oppsec-. - Naming consistency.
lp-interop--OPPfollows established LP interop conventions. - JobTier change is correct.
informing→candidateis a correct side effect ofadjustJobTierBasedOnView, not a manual override. - View structure mirrors
4.22-LP-Interop--lpMainlineappropriately.
Concerns
The BQ query uses LIKE 'periodic-ci-%%-lp-interop-%%' which catches new-style names (with lp- prefix). The snapshot also contains older-style names like periodic-ci-stolostron-...-interop-opp-aws (no lp- prefix) that wouldn't match. This is a pre-existing issue, not introduced by this PR, but worth confirming whether old-style jobs are still active.
ℹ️ Low: Missing 5.0 view?
The snapshot includes OPP jobs for both 4.22 and 5.0, but only a 4.22 view is added. If 5.0 OPP jobs should be visible in CR, a 5.0-LP-Interop--OPP view would be needed. This might be intentional if 5.0 isn't ready yet.
ℹ️ Low: Test coverage for old-style job names
Unit tests cover only the new lp-interop-opp naming. A test case with the old-style name (interop-opp, no lp- prefix) would verify both conventions work.
Suggestions
- Verify whether old-style
interop-opp(nolp-prefix) job names still exist in CI. If so, consider adding a BQ SQL pattern. - Consider adding a test case with the old naming convention for documentation and coverage.
- Consider a
5.0-LP-Interop--OPPview in this PR or a follow-up.
Overall Assessment
✅ This PR looks good and is safe to merge. Pattern logic is correct, ordering is safe, test coverage is solid for the primary use case, view structure follows conventions, and snapshot changes are correct side effects of proper LP classification.
|
/pj-rehearse ack |
1 similar comment
|
/pj-rehearse ack |
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: amp-rh, xueqzhan The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/pj-rehearse ack |
|
/test e2e |
|
/retest |
|
/test e2e |
1 similar comment
|
/test e2e |
|
/retest AI-generated. Review for accuracy. |
|
/retest |
|
/retest |
8 similar comments
|
/retest |
|
/retest |
|
/retest |
|
/retest |
|
/retest |
|
/retest |
|
/retest |
|
/retest |
|
/test e2e |
|
/retest |
|
/retest-required |
|
@amp-rh: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
Add layered product pattern for OPP multi-LP interop jobs (ACM + ODF + Quay), CR dashboard view 4.22-LP-Interop--OPP, and unit tests for job name classification.
Tracked by: INTEROP-9317 | Epic: INTEROP-9202
Summary by CodeRabbit
New Features
lp-interop--OPPand updated their release status to candidate.Tests