Add ODF lp-interop variant and CR view for OCP 4.22 - #3850
Conversation
|
Pipeline controller notification For optional jobs, comment This repository is configured in: automatic mode |
WalkthroughChangesLP Interop readiness
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested reviewers: Caution Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional.
❌ Failed checks (1 error)
✅ Passed checks (20 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 |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: amiskin94 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
openshift-ci
Bot
added
the
ready-for-human-review
|
Scheduling required tests: |
8894574 to
9d065a7
Compare
Adds layeredProductPatterns entry for ODF lp-interop CR jobs, a 4.22-LP-Interop--lpGA Component Readiness view, and a test case for the new job name pattern. Part of INTEROP-9280. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
9d065a7 to
8a8430e
Compare
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)
pkg/variantregistry/ocp.go (1)
854-855: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAdd regression cases for all new pattern mappings.
pkg/variantregistry/ocp_test.gocovers the ACM and ODF additions, but not-vcf-migrationor-coo-. Add table entries assertingVariantJobTier == "candidate"andVariantLayeredProduct == "lp-interop-coo"respectively.As per coding guidelines, new or modified functionality should include test coverage.
Also applies to: 1371-1374
🤖 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 854 - 855, Add regression table entries in the variant registry tests for both new mappings: assert the “-vcf-migration” pattern resolves to VariantJobTier “candidate”, and the “-coo-” pattern resolves to VariantLayeredProduct “lp-interop-coo”. Extend the existing test table and reuse its established assertion structure.Source: Coding guidelines
🤖 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.
Outside diff comments:
In `@pkg/variantregistry/ocp.go`:
- Around line 854-855: Add regression table entries in the variant registry
tests for both new mappings: assert the “-vcf-migration” pattern resolves to
VariantJobTier “candidate”, and the “-coo-” pattern resolves to
VariantLayeredProduct “lp-interop-coo”. Extend the existing test table and reuse
its established assertion structure.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: 894550b2-9c52-4394-a72a-949ca6f230df
📒 Files selected for processing (3)
config/views.yamlpkg/variantregistry/ocp.gopkg/variantregistry/ocp_test.go
|
Scheduling required tests: |
The openshift/release PR renamed the test from cr--odf--aws to cr--aws (removing redundant product name from test). Update the variant registry pattern and test case to match the new job name. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
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`:
- Line 1371: Narrow the ODF pattern in the setLayeredProduct match table so
“-lpga-lp-interop-cr--” does not classify every LP Interop job as ODF. Make the
entry explicitly ODF-specific, or move the later “-coo-”, “-acm-cnv-”, and
“-acm-virt-” product patterns ahead of it while preserving their intended
matches.
🪄 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: 7eee6795-3f1b-4a85-abba-6edff4e46141
📒 Files selected for processing (2)
pkg/variantregistry/ocp.gopkg/variantregistry/ocp_test.go
🚧 Files skipped from review as they are similar to previous changes (1)
- pkg/variantregistry/ocp_test.go
| {"-lpga-lp-ocp-compat-cr--oadp-", "lp-ocp-compat--oadp--lpGA"}, | ||
| {"-lpga-lp-ocp-compat-cr--servicemesh-", "lp-ocp-compat--servicemesh--lpGA"}, | ||
| {"-lpga-lp-ocp-compat-cr--operator-e2e-", "lp-ocp-compat--serverless--lpGA"}, | ||
| {"-lpga-lp-interop-cr--", "lp-interop--odf--lpGA"}, |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 3 \
-e 'lpga-lp-interop-cr--' \
-e 'cr--odf' \
-e 'cr--aws' \
-e 'lp-interop' \
-e 'coo' \
-e 'acm' \
.Repository: openshift/sippy
Length of output: 50372
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== candidate files =="
git ls-files | rg '(^|/)pkg/variantregistry/ocp\.go$|ocp.*test.*\.go$' | sed -n '1,120p'
echo
echo "== relevant ocp.go section =="
sed -n '1330,1390p' pkg/variantregistry/ocp.go
echo
echo "== related tests mentioning layered product variants/patterns =="
rg -n -C 3 'setLayeredProduct|lp-interop|odf|lpga-lp-interop-cr|cr--odf|cr--' pkg/variantregistry -g '*_test.go' | sed -n '1,240p'
echo
echo "== exact pattern entries in ocp.go around lp-interop =="
sed -n '1350,1380p' pkg/variantregistry/ocp.goRepository: openshift/sippy
Length of output: 7199
Narrow the ODF match before assigning lp-interop--odf--lpGA.
setLayeredProduct returns on the first match, and the -lpga-lp-interop-cr-- entry comes before the later -coo-, -acm-cnv-, and -acm-virt- product matches. Any LP Interop job with that suffix is assigned ODF, including periodic-ci-red-hat-storage-ocs-ci-master-odf-ocp-4.22-lpGA-lp-interop-cr--aws from the test. Make this entry ODF-specific, or reorder the patterns so product-specific entries take precedence.
🤖 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` at line 1371, Narrow the ODF pattern in the
setLayeredProduct match table so “-lpga-lp-interop-cr--” does not classify every
LP Interop job as ODF. Make the entry explicitly ODF-specific, or move the later
“-coo-”, “-acm-cnv-”, and “-acm-virt-” product patterns ahead of it while
preserving their intended matches.
|
Scheduling required tests: |
|
@amiskin94: 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. |
|
Closing this PR in favor of a new Sippy PR that will add layered product patterns for the existing multi-LP OPP job ( Why: The team pivoted from single-LP testing to leveraging the existing OPP job which already tests ACM + ODF + Quay together (genuine multi-LP interop). Per-product test routing will be handled via What carries forward: Thank you @amiskin94 for the foundational work — the following patterns from this PR directly inform the new approach:
The new Sippy PR will add Tracked by: INTEROP-9280 | Epic: INTEROP-9202 AI-generated. Review for accuracy. |
|
Closing in favor of #3865, which registers the multi-LP OPP variant ( Carry-forward contributions from this PR:
Tracked by: INTEROP-9397 | Epic: INTEROP-9202 |
2 participants
Summary
layeredProductPatternsentry for ODF lp-interop CR jobs (lp-interop--odf--lpGA)4.22-LP-Interop--lpGAComponent Readiness view withOwner: mpiitDetails
Part of Epic INTEROP-9202 (Integrate Component Readiness Reporting into OPP Testing).
Changes:
pkg/variantregistry/ocp.go: New pattern{"-lpga-lp-interop-cr--odf--", "lp-interop--odf--lpGA"}config/views.yaml: New4.22-LP-Interop--lpGAview (base: 4.21 GA, sample: 4.22 now)pkg/variantregistry/ocp_test.go: Test case forperiodic-ci-red-hat-storage-ocs-ci-master-odf-ocp-4.22-lpGA-lp-interop-cr--odf--awsCompanion to openshift/release#82653.
Jira: INTEROP-9280
Test plan
TestVariantSyncerpasses with new test caseTestVariantsSnapshotunaffected (no existing jobs match new pattern)🤖 Generated with Claude Code
Summary by CodeRabbit