INTEROP-9255: Register ACM+Virt interop jobs in Sippy#3830
Conversation
Add layered product patterns for ACM+Virt interop jobs so they get
classified as 'lp-interop--acm-virt' instead of the generic 'virt':
- {-acm-cnv-} matches P2P upgrade job
- {-acm-virt-} matches live migration job
Both patterns placed before generic -virt/-cnv entries (first match wins).
Add 4.22-LP-Interop--lpMainline view for Component Readiness dashboard,
filtering by Owner: mpiit to show all MPIIT interop jobs.
Regenerated snapshot.yaml via make update-variants.
Co-authored-by: Cursor <cursoragent@cursor.com>
|
Pipeline controller notification For optional jobs, comment This repository is configured in: automatic mode |
|
@sg-rh: This pull request references INTEROP-9255 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. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: sg-rh The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
WalkthroughAdds layered-product classification for ACM interoperability jobs and introduces a ChangesLP interoperability readiness
Estimated code review effort: 3 (Moderate) | ~20 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 20 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (20 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ast-grep (0.44.1)pkg/variantregistry/snapshot.yamlast-grep skipped this file: it is too large to scan (8415954 bytes) 🔧 Checkov (3.3.8)pkg/variantregistry/snapshot.yamlCheckov skipped this file: it is too large to scan (8415954 bytes) 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 |
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 `@pkg/variantregistry/ocp.go`:
- Around line 1366-1367: Add focused regression tests in the OCP variant
registry tests for the new -acm-cnv- and -acm-virt- mappings, verifying each
resolves to lp-interop--acm-virt and takes precedence over the generic -virt and
-cnv mappings. Keep the cases scoped to the modified variant-classification
behavior.
🪄 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: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: 797380b7-ad34-4c17-bdc7-4a395c9d9aea
📒 Files selected for processing (3)
config/views.yamlpkg/variantregistry/ocp.gopkg/variantregistry/snapshot.yaml
| {"-acm-cnv-", "lp-interop--acm-virt"}, | ||
| {"-acm-virt-", "lp-interop--acm-virt"}, |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate files =="
git ls-files | rg '(^|/)(ocp\.go|ocp_test\.go)$|pkg/variantregistry' || true
echo "== relevant mapping lines =="
if [ -f pkg/variantregistry/ocp.go ]; then
nl -ba pkg/variantregistry/ocp.go | sed -n '1330,1385p'
fi
echo "== search for relevant test cases =="
if [ -f pkg/variantregistry/ocp_test.go ]; then
rg -n "lp-interop|acm-cnv|acm-virt|acm|cnv|virt" pkg/variantregistry/ocp_test.go || true
echo "== file size =="
wc -l pkg/variantregistry/ocp_test.go
echo "== outline/test function names (top-level relevant) =="
rg -n "func Test|func .*Layer|func .*Variant|acm|lp-interop|cnv|virt" pkg/variantregistry/ocp_test.go || true
else
echo "pkg/variantregistry/ocp_test.go not found"
fi
echo "== define mapping-related function outline =="
if [ -f pkg/variantregistry/ocp.go ]; then
ast-grep outline pkg/variantregistry/ocp.go --match "map" --view expanded || true
ast-grep outline pkg/variantregistry/ocp.go --match "func" --view expanded || true
fiRepository: openshift/sippy
Length of output: 502
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== relevant mapping lines =="
awk 'NR>=1330 && NR<=1385 {printf "%6d\t%s\n", NR, $0}' pkg/variantregistry/ocp.go
echo "== search for relevant test cases =="
awk 'BEGIN{IGNORECASE=1} /lp-interop|acm-cnv|acm-virt|acm|cnv|virt/' pkg/variantregistry/ocp_test.go || true
echo "== test function/function name lines and relevant counts =="
awk '/func Test|func .*Layer|func .*Variant|acm|lp-interop|cnv|virt/ {printf "%6d\t%s\n", NR, $0}' pkg/variantregistry/ocp_test.go || true
echo "== file sizes =="
wc -l pkg/variantregistry/ocp_test.go pkg/variantregistry/ocp.go
echo "== structural/context for mapping slice =="
awk '/acm-cnv-|acm-virt-|-virt-|-cnv-|layer|Layer|Job|Variant|variant/ {printf "%6d\t%s\n", NR, $0}' pkg/variantregistry/ocp.go | sed -n '1,240p'Repository: openshift/sippy
Length of output: 21248
Add regression coverage for the new ACM layered-product mappings.
Add focused test cases for -acm-cnv- and -acm-virt- in pkg/variantregistry/ocp_test.go, including a case that proves precedence over the generic -virt/-cnv mappings. This is needed to meet the Go test-coverage requirement for modified variant classification behavior.
🤖 Prompt for 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.
In `@pkg/variantregistry/ocp.go` around lines 1366 - 1367, Add focused regression
tests in the OCP variant registry tests for the new -acm-cnv- and -acm-virt-
mappings, verifying each resolves to lp-interop--acm-virt and takes precedence
over the generic -virt and -cnv mappings. Keep the cases scoped to the modified
variant-classification behavior.
Source: Coding guidelines
Summary
Register ACM+Virt interop jobs in Sippy so they get their own layered product classification (
lp-interop--acm-virt) instead of falling into the genericvirtcategory, and add a 4.22 Component Readiness dashboard view.Changes
1.
pkg/variantregistry/ocp.go— Layered product patternsAdded two entries to
layeredProductPatternsbefore the generic-virt/-cnventries (first match wins):-acm-cnv-...-acm-cnv-ocp-4.22-p2p-lp-interop-aws)lp-interop--acm-virt-acm-virt-...-acm-virt-ocp4.22-p2p-cclm-liv-mig-lp-interop-aws)lp-interop--acm-virt2.
config/views.yaml— New CR dashboard viewAdded
4.22-LP-Interop--lpMainlineview:Owner: mpiit,LayeredProduct: [](any),Network: ovn3.
pkg/variantregistry/snapshot.yaml— RegeneratedLive migration job reclassified:
LayeredProduct: virt→LayeredProduct: lp-interop--acm-virtRelated
Made with Cursor
Summary by CodeRabbit