MPIIT: Update Variants and Views Ownership#3535
Conversation
|
Pipeline controller notification For optional jobs, comment This repository is configured in: automatic mode |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThis PR updates LP (Layered Product) view block definitions in config and supporting variant registry logic. Version 5.0 migrates from a single LP-Interop block to separate Chaos, Interop, and OCP-compat mainline/GA blocks. Version 4.22 replaces LP-Interop with OCP-compat mainline and GA blocks. Registry code updates job filtering patterns and product inference mappings accordingly. ChangesLP Variant Block Configuration and Registry Updates
🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 13 | ❌ 4❌ Failed checks (4 warnings)
✅ Passed checks (13 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: oharan2 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
|
/test security |
|
Scheduling required tests: |
openshift-ci
Bot
added
the
do-not-merge/work-in-progress
openshift-ci
Bot
removed
the
do-not-merge/work-in-progress
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
pkg/variantregistry/ocp.go (2)
526-535:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
setOwnermisclassifies LP chaos/interop jobs due to pattern order and stale owner value.
-lp-chaos-currently matches the earlier generic-chaos-rule, and LP interop/chaos still map tompiiteven though downstream expectations arempex.Suggested fix
ownerPatterns := []struct { substring string owner string }{ + {"-lp-chaos-", "mpex"}, // LP chaos + {"-lp-interop-", "mpex"}, // LP interop + {"-lp-ocp-compat-", "lp"}, // LP OCP compat {"-osd", "service-delivery"}, {"-rosa", "service-delivery"}, {"-openshift-online", "service-delivery"}, {"-telco5g", "cnf"}, {"-perfscale", "perfscale"}, {"-chaos-", "chaos"}, {"-azure-aro-hcp", "aro"}, {"-qe", "qe"}, // Keep this one below perfscale {"-openshift-tests-private", "qe"}, {"-openshift-verification-tests", "qe"}, {"-openshift-distributed-tracing", "qe"}, {"-oadp-", "oadp"}, - {"-lp-chaos-", "mpiit"}, // MPEX Integrity Engineering Chaos Team - {"-lp-interop-", "mpiit"}, // MPEX Integrity Engineering Interop Team - {"-lp-ocp-compat-", "lp"}, // Layered Product Teams }🤖 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 526 - 535, The pattern list in setOwner misclassifies LP chaos/interop jobs because the generic "-chaos-" entry appears before the more specific "-lp-chaos-" and "-lp-interop-" entries and the LP mappings still use the stale "mpiit" owner; reorder the entries so "-lp-chaos-" and "-lp-interop-" appear before the generic "-chaos-" pattern and update their mapped owner from "mpiit" to "mpex" (update the entries referenced as "-lp-chaos-" and "-lp-interop-" in the pattern list within setOwner).
125-134:⚠️ Potential issue | 🟠 Major | ⚡ Quick winOuter WHERE clause is missing the new LP job patterns.
-lp-chaos-and-lp-ocp-compat-were added inRecentSuccessfulJobsbut not in the mainj.prowjob_job_namefilter, so those jobs can be dropped from final results.As per coding guidelines, "When modifying any data provider (BigQuery or PostgreSQL), ensure parity between both implementations. Changes to query logic, filtering, or returned data in one provider must be reflected in the other".Suggested fix
((j.prowjob_job_name LIKE 'periodic-ci-openshift-%%' OR j.prowjob_job_name LIKE 'periodic-ci-shiftstack-%%' OR j.prowjob_job_name LIKE 'periodic-ci-redhat-chaos-prow-scripts-main-cr-%%' OR j.prowjob_job_name LIKE 'periodic-ci-Azure-ARO-HCP-%%' OR j.prowjob_job_name LIKE 'release-%%' OR j.prowjob_job_name LIKE 'periodic-ci-%%-lp-interop-%%' + OR j.prowjob_job_name LIKE 'periodic-ci-%%-lp-chaos-%%' + OR j.prowjob_job_name LIKE 'periodic-ci-%%-lp-ocp-compat-%%' OR j.prowjob_job_name LIKE 'periodic-ci-%%-quay-cr-%%' OR j.prowjob_job_name LIKE 'aggregator-%%')🤖 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 125 - 134, The outer WHERE job-name filter in pkg/variantregistry/ocp.go is missing the LP patterns added in RecentSuccessfulJobs, so add the same patterns (those containing "-lp-chaos-" and "-lp-ocp-compat-") into the j.prowjob_job_name OR list used in the main WHERE clause; update the j.prowjob_job_name LIKE conditions to include LIKE 'periodic-ci-%%-lp-chaos-%%' and LIKE 'periodic-ci-%%-lp-ocp-compat-%%' (matching the same naming used in RecentSuccessfulJobs) to ensure parity between the two query implementations.
♻️ Duplicate comments (1)
config/views.yaml (1)
587-589:⚠️ Potential issue | 🟠 Major | ⚡ Quick winOwner still set to
mpiitin LP views, conflicting with PR ownership migration tompex.These entries look out of sync with the PR objective and downstream expected owner behavior. Please update the touched LP owner filters to
mpexfor consistency.Suggested fix
Owner: - - mpiit + - mpexAlso applies to: 632-633, 2550-2551, 3143-3144
🤖 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 `@config/views.yaml` around lines 587 - 589, Update the LP view Owner entries that are still set to "mpiit" to "mpex": locate the YAML blocks where the key "Owner:" has the list entry "- mpiit" (these LP view owner filter sections) and replace the value "mpiit" with "mpex" in each occurrence (including the other similar blocks referenced in the review). Ensure you only change the owner string and preserve surrounding indentation and other keys such as "advanced_options".
🤖 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 1273-1274: The ACS layered-product patterns use "lpMainline" and
"lpGA" but matching is done against jobNameLower (all lowercase), so those
patterns never match; update the pattern strings to use lowercase ("lpmainline",
"lpga") or alternatively perform the comparisons against the original-cased
jobName instead of jobNameLower. Locate the pattern entries containing
"lpMainline" and "lpGA" in pkg/variantregistry/ocp.go and change them to
lowercase ("lpmainline" and "lpga") or switch the matching code to use jobName
(not jobNameLower) so ACS LP OCP-compat jobs are classified correctly.
---
Outside diff comments:
In `@pkg/variantregistry/ocp.go`:
- Around line 526-535: The pattern list in setOwner misclassifies LP
chaos/interop jobs because the generic "-chaos-" entry appears before the more
specific "-lp-chaos-" and "-lp-interop-" entries and the LP mappings still use
the stale "mpiit" owner; reorder the entries so "-lp-chaos-" and "-lp-interop-"
appear before the generic "-chaos-" pattern and update their mapped owner from
"mpiit" to "mpex" (update the entries referenced as "-lp-chaos-" and
"-lp-interop-" in the pattern list within setOwner).
- Around line 125-134: The outer WHERE job-name filter in
pkg/variantregistry/ocp.go is missing the LP patterns added in
RecentSuccessfulJobs, so add the same patterns (those containing "-lp-chaos-"
and "-lp-ocp-compat-") into the j.prowjob_job_name OR list used in the main
WHERE clause; update the j.prowjob_job_name LIKE conditions to include LIKE
'periodic-ci-%%-lp-chaos-%%' and LIKE 'periodic-ci-%%-lp-ocp-compat-%%'
(matching the same naming used in RecentSuccessfulJobs) to ensure parity between
the two query implementations.
---
Duplicate comments:
In `@config/views.yaml`:
- Around line 587-589: Update the LP view Owner entries that are still set to
"mpiit" to "mpex": locate the YAML blocks where the key "Owner:" has the list
entry "- mpiit" (these LP view owner filter sections) and replace the value
"mpiit" with "mpex" in each occurrence (including the other similar blocks
referenced in the review). Ensure you only change the owner string and preserve
surrounding indentation and other keys such as "advanced_options".
🪄 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: Enterprise
Run ID: b5ab94ab-18b2-4595-9823-9f2156195123
📒 Files selected for processing (2)
config/views.yamlpkg/variantregistry/ocp.go
oharan2
changed the title
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 535-537: The LP-specific chaos entry {"-lp-chaos-", "mpict"} is
placed after the more generic {"-chaos-", "chaos"} pattern so it never matches;
move the {"-lp-chaos-", "mpict"} mapping so it appears before the generic
"-chaos-" mapping in the same slice/array (the list used by the matcher in
pkg/variantregistry/ocp.go) so "-lp-chaos-" is matched first and LP chaos jobs
are classified as "mpict" rather than "chaos".
🪄 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: Enterprise
Run ID: f0944922-f81e-478d-ac41-90d2f598e676
📒 Files selected for processing (2)
config/views.yamlpkg/variantregistry/ocp.go
| {"-lp-chaos-", "mpict"}, // MPEX Integrity Engineering Chaos Team | ||
| {"-lp-interop-", "mpiit"}, // MPEX Integrity Engineering Interop Team | ||
| {"-lp-ocp-compat-", "lp"}, // Layered Product Teams |
There was a problem hiding this comment.
Place -lp-chaos- before generic -chaos- to avoid shadowing.
Line 535 is currently unreachable because Line 528 matches first; LP chaos jobs will be classified as chaos instead of mpict.
💡 Proposed fix
ownerPatterns := []struct {
substring string
owner string
}{
{"-osd", "service-delivery"},
{"-rosa", "service-delivery"},
{"-openshift-online", "service-delivery"},
{"-telco5g", "cnf"},
{"-perfscale", "perfscale"},
+ {"-lp-chaos-", "mpict"}, // MPEX Integrity Engineering Chaos Team
{"-chaos-", "chaos"},
{"-azure-aro-hcp", "aro"},
{"-qe", "qe"}, // Keep this one below perfscale
{"-openshift-tests-private", "qe"},
{"-openshift-verification-tests", "qe"},
{"-openshift-distributed-tracing", "qe"},
{"-oadp-", "oadp"},
- {"-lp-chaos-", "mpict"}, // MPEX Integrity Engineering Chaos Team
{"-lp-interop-", "mpiit"}, // MPEX Integrity Engineering Interop Team
{"-lp-ocp-compat-", "lp"}, // Layered Product Teams
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| {"-lp-chaos-", "mpict"}, // MPEX Integrity Engineering Chaos Team | |
| {"-lp-interop-", "mpiit"}, // MPEX Integrity Engineering Interop Team | |
| {"-lp-ocp-compat-", "lp"}, // Layered Product Teams | |
| ownerPatterns := []struct { | |
| substring string | |
| owner string | |
| }{ | |
| {"-osd", "service-delivery"}, | |
| {"-rosa", "service-delivery"}, | |
| {"-openshift-online", "service-delivery"}, | |
| {"-telco5g", "cnf"}, | |
| {"-perfscale", "perfscale"}, | |
| {"-lp-chaos-", "mpict"}, // MPEX Integrity Engineering Chaos Team | |
| {"-chaos-", "chaos"}, | |
| {"-azure-aro-hcp", "aro"}, | |
| {"-qe", "qe"}, // Keep this one below perfscale | |
| {"-openshift-tests-private", "qe"}, | |
| {"-openshift-verification-tests", "qe"}, | |
| {"-openshift-distributed-tracing", "qe"}, | |
| {"-oadp-", "oadp"}, | |
| {"-lp-interop-", "mpiit"}, // MPEX Integrity Engineering Interop Team | |
| {"-lp-ocp-compat-", "lp"}, // Layered Product Teams | |
| } |
🤖 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 535 - 537, The LP-specific chaos
entry {"-lp-chaos-", "mpict"} is placed after the more generic {"-chaos-",
"chaos"} pattern so it never matches; move the {"-lp-chaos-", "mpict"} mapping
so it appears before the generic "-chaos-" mapping in the same slice/array (the
list used by the matcher in pkg/variantregistry/ocp.go) so "-lp-chaos-" is
matched first and LP chaos jobs are classified as "mpict" rather than "chaos".
|
@oharan2: The following test failed, say
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. |
2 participants
Summary
Aligns MPIIT-owned Component Readiness views and variant classification with the layered-product CI migration from
lp-interop/lp-interop-cr-*job naming tolp-ocp-compat/lp-ocp-compat-cr--*, including separate lpMainline vs lpGA ACS tracking.Variant registry (
pkg/variantregistry/ocp.go)periodic-ci-*-lp-chaos-*andperiodic-ci-*-lp-ocp-compat-*in the recent-successful-jobs query (alongside existinglp-interop).-lp-chaos-→mpict(MPEX Integrity Engineering Chaos)-lp-interop-→mpiit(MPEX Integrity Engineering Interop)-lp-ocp-compat-→lp(layered product teams)lp-interop-cr-*patterns withlp-ocp-compat-cr--*equivalents (virt, quay, pipelines, ODF, gitops, fusion-access, MTA, OADP, servicemesh, serverless, etc.).-lpMainline-lp-ocp-compat-cr--acs-and-lpGA-lp-ocp-compat-cr--acs-.Component Readiness views (
config/views.yaml)*-LP-Interopviews (which listedlp-interop-*products undermpex/qe) with focused views for 5.0 and 4.22 only. Older releases (4.21, 4.20, …) are unchanged.5.0-LP-Chaos--lpMainlinempict[](onboarding shell)5.0-LP-Interop--lpMainlinempiit[](onboarding shell)5.0-LP-OCP-Compat--lpMainlinelplp-ocp-compat--acs--lpMainline5.0-LP-OCP-Compat--lpGAlp4.22-LP-OCP-Compat--lpMainlinelplp-ocp-compat--acs--lpMainline4.22-LP-OCP-Compat--lpGAlpTo ensure the best practices,
regression_tracking.enabled: trueis set on these views so regressions sync back to the DB.Summary by CodeRabbit