CMP-3987: Add Component Readiness dashboard for Compliance Operator #3137
Conversation
|
Pipeline controller notification For optional jobs, comment This repository is configured in: automatic mode |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: xiaojiey 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 |
WalkthroughThis PR introduces support for a new "lp-interop-compliance" layered product by adding configuration blocks for compliance readiness tracking, creating a new test suite, mapping compliance-related job patterns to the layered product, and updating periodic CI jobs to reference it. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (6 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Cache: Disabled due to data retention organization setting Knowledge base: Disabled due to data retention organization setting 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to data retention organization setting
📒 Files selected for processing (4)
config/views.yaml(1 hunks)pkg/db/suites.go(1 hunks)pkg/variantregistry/ocp.go(1 hunks)pkg/variantregistry/snapshot.yaml(66 hunks)
🔇 Additional comments (5)
pkg/db/suites.go (1)
60-71: NewCompliance-lp-interopsuite entry looks correctAdding
"Compliance-lp-interop"alongside the other*-lp-interopsuites cleanly extends the set of suites we persist into the DB and aligns with the new compliance readiness flow. There are no behavioral concerns with this change itself.One thing to double-check: ensure the underlying JUnit data (BigQuery
testsuitevalues) for the new compliance jobs actually uses this exact string; any mismatch would mean those results are still filtered out here.pkg/variantregistry/snapshot.yaml (1)
1173-1728: Verify that the snapshot changes align with the supporting configuration updates.The changes update 66 CI job entries, changing
LayeredProduct: nonetoLayeredProduct: lp-interop-compliance. While the bulk pattern is consistent, confirm:
Is this the correct change location? Verify whether
snapshot.yamlis manually maintained or auto-generated. If auto-generated, ensure the generation process (potentially triggered by changes toconfig/views.yaml,pkg/variantregistry/ocp.go, orpkg/db/suites.go) is part of this PR workflow.Job coverage completeness. Ensure all and only the correct compliance-related jobs are being categorized under
lp-interop-complianceto prevent missing jobs from the compliance dashboard.Integration validation. Verify that these snapshot updates, combined with supporting configuration changes, enable the Component Readiness dashboard for Compliance Operator as intended.
config/views.yaml (2)
1507-1507: Note: Lower minimum_failure threshold for new dashboard.The
minimum_failureis set to 2, which is lower than the standard threshold of 3 used in main views (e.g., line 68). This lower threshold means tests will be flagged as problematic with fewer failures, which could be appropriate for a new compliance dashboard to catch issues early, but may also result in more false positives if compliance jobs are less stable.This appears intentional for initial compliance tracking sensitivity, similar to other specialized views like
4.21-LP-Interop(line 334).
1497-1506: Verify the intentionally limited platform and owner scope.The compliance view is restricted to:
- Platform: only
aws(line 1505)- Owner: only
eng(line 1503)This is notably more restrictive than other views, which typically include multiple platforms (aws, azure, gcp, metal, rosa, vsphere) and owners (eng, service-delivery). Additionally, the view lacks filters for Installer, Network, Topology, and FeatureSet that are standard in other component readiness views.
If this is the initial rollout scope, consider documenting this in the PR description or a comment. Otherwise, verify whether this limited scope is intentional.
pkg/variantregistry/ocp.go (1)
1107-1109: Verify the generic-compliancepattern doesn't over-match unintended jobs.The
-compliancepattern on line 1109 uses substring matching and will categorize any job containing "compliance" (e.g., "pre-compliance-check", "non-compliance-test") underlp-interop-compliance. While the pattern order is correct—more specific patterns (-complianceascode-,-compliance-destructive) are checked first—confirm that no unintended compliance-related jobs are being routed to this product. If broader matching is undesired, consider anchoring the pattern with additional delimiters (e.g.,compliance-instead of justcompliance).
|
Scheduling required tests: |
xiaojiey
changed the title
|
@xiaojiey: This pull request references CMP-3987 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 story to target the "4.21.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. |
openshift-ci-robot
added
the
jira/valid-reference
|
@xiaojiey: This pull request references CMP-3987 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 story to target the "4.21.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. |
xiaojiey
force-pushed
the
compliance-dashboard
branch
from
d1bdd22 to
d8e9a70
Compare
|
@xiaojiey: This pull request references CMP-3987 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 story to target the "4.22.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. |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (6)
config/views.yaml (6)
1743-1786: Missing JobTier filter may include unstable jobs.This compliance view block lacks a
JobTierfilter ininclude_variants, which means it will include jobs of all tiers (blocking, informing, standard, candidate, hidden). This is the same concern previously raised in past review comments.For consistency with main release views (e.g., 4.21-main at lines 39-42) and to ensure only stable jobs are tracked, consider adding:
include_variants: Architecture: - amd64 - arm64 + JobTier: + - blocking + - informing + - standard LayeredProduct: - lp-interop-compliance Owner: - eng Platform: - azure - gcp - awsNote: If downstream QE jobs default to
JobTier: hidden(as mentioned in the Owner filter concern), you may need to either includehiddenin the filter or opt-in specific compliance QE jobs to a different tier inpkg/variantregistry/ocp.go.
1787-1830: Missing JobTier filter may include unstable jobs.Same issue as the 4.21 block - this view lacks a
JobTierfilter and may include unstable candidate or hidden jobs.
1831-1874: Missing JobTier filter may include unstable jobs.Same issue as the 4.21 and 4.20 blocks - this view lacks a
JobTierfilter.
1875-1918: Missing JobTier filter may include unstable jobs.Same issue as other compliance blocks - this view lacks a
JobTierfilter.
1919-1962: Missing JobTier filter may include unstable jobs.Same issue as other compliance blocks - this view lacks a
JobTierfilter.
1963-2006: Missing JobTier filter may include unstable jobs.Same issue as other compliance blocks - this view lacks a
JobTierfilter.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to data retention organization setting
📒 Files selected for processing (3)
config/views.yaml(1 hunks)pkg/db/suites.go(1 hunks)pkg/variantregistry/ocp.go(1 hunks)
🔇 Additional comments (1)
pkg/db/suites.go (1)
71-71: LGTM!The new test suite "Compliance-lp-interop" is correctly added to the testSuites list, following the established pattern for other layered product interop suites.
|
@xiaojiey: This pull request references CMP-3987 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 story to target the "4.22.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. |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
config/views.yaml (1)
1743-2012: Add JobTier filter to compliance-readiness blocks to exclude unstable job tiers.All six new
lp-interop-complianceblocks (4.21 through 4.16) are missing aJobTierfilter in theirinclude_variantssections. Without this filter, the views may include candidate or hidden jobs that are not stable enough for component readiness tracking. This mirrors the concern raised in the previous review.Apply this diff to add JobTier filtering to each block (example shown for 4.21-lp-interop-compliance; repeat for all six):
include_variants: Architecture: - amd64 - arm64 LayeredProduct: - lp-interop-compliance + JobTier: + - blocking + - informing + - standard Owner: - eng - qe Platform: - azure - gcp - aws
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to data retention organization setting
📒 Files selected for processing (2)
config/views.yaml(1 hunks)pkg/variantregistry/snapshot.yaml(66 hunks)
| Installer: ipi | ||
| JobTier: informing | ||
| LayeredProduct: none | ||
| LayeredProduct: lp-interop-compliance |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Extract job names and verify compliance pattern matching
# Get a few sample hunks to see job names
echo "=== Sample job names from snapshot.yaml around LayeredProduct updates ==="
rg -B 30 "LayeredProduct: lp-interop-compliance" pkg/variantregistry/snapshot.yaml | grep -E '"name":|periodic-ci-' | head -30
# Check if lp-interop-compliance is registered in suites.go
echo -e "\n=== Checking lp-interop-compliance registration in suites.go ==="
rg "lp-interop-compliance|interop.compliance" pkg/db/suites.go
# Check mapping in ocp.go
echo -e "\n=== Checking lp-interop-compliance in ocp.go ==="
rg "lp-interop-compliance" pkg/variantregistry/ocp.go
# Check for ComplianceAsCode pattern mapping in ocp.go
echo -e "\n=== Checking ComplianceAsCode patterns in ocp.go ==="
rg -i "ComplianceAsCode|compliance" pkg/variantregistry/ocp.go | head -20Repository: openshift/sippy
Length of output: 3545
🏁 Script executed:
#!/bin/bash
# Check the structure and content of suites.go to understand how test suites are registered
echo "=== Checking suites.go for test suite definitions ==="
head -100 pkg/db/suites.go
echo -e "\n=== Searching for any reference to 'interop' or 'compliance' in suites.go ==="
rg -i "interop|compliance" pkg/db/suites.go
echo -e "\n=== Checking total content size and structure of suites.go ==="
wc -l pkg/db/suites.go
echo -e "\n=== Sample of suite definitions in suites.go ==="
rg "lp-" pkg/db/suites.go | head -20Repository: openshift/sippy
Length of output: 2963
The LayeredProduct value lp-interop-compliance in snapshot.yaml is not registered in pkg/db/suites.go.
All 66 hunks correctly target compliance jobs (names contain "-ComplianceAsCode-", "-compliance", or "-compliance-destructive"), and the mapping in pkg/variantregistry/ocp.go correctly routes these jobs to lp-interop-compliance. However, suites.go only defines "Compliance-lp-interop" as a test suite, not "lp-interop-compliance". This name mismatch will cause the system to fail when processing jobs assigned to the non-existent lp-interop-compliance suite. Either add "lp-interop-compliance" to the testSuites array in suites.go, or update the snapshot and ocp.go mappings to use the existing "Compliance-lp-interop" name.
🤖 Prompt for AI Agents
In pkg/variantregistry/snapshot.yaml at line 1104 the LayeredProduct is set to
"lp-interop-compliance" but pkg/db/suites.go only defines
"Compliance-lp-interop" in testSuites; add "lp-interop-compliance" to the
testSuites array in pkg/db/suites.go so the suite name used by snapshot.yaml and
pkg/variantregistry/ocp.go is registered, or alternatively change the
LayeredProduct in snapshot.yaml (and the mapping in ocp.go) to
"Compliance-lp-interop" so names are consistent across snapshot, ocp.go, and
suites.go.
b682638 to
dd3edd0
Compare
|
@xiaojiey: This pull request references CMP-3987 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 story to target the "4.22.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. |
|
@xiaojiey: This pull request references CMP-3987 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 story to target the "4.22.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. |
|
@xiaojiey: This pull request references CMP-3987 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 story to target the "4.22.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. |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
sippy-ng/src/component_readiness/JobArtifactQuery.js (1)
594-601: AlignvalidateURLimplementations to avoid subtle divergenceYou now have three
validateURLimplementations: two withtry/catch(Lines 594-601, 804-811) and one without (Lines 624-630). To keep behavior consistent and avoid future drift, consider giving the JAQResultTable version the sametry/catchsemantics (or extracting a shared helper in this file if Snyk still accepts it). This keeps all URL checks robust against malformed input while preserving thehttp/httpsprotocol restriction.Also applies to: 624-630, 804-811
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to data retention organization setting
📒 Files selected for processing (2)
chat/prompts/payload-analysis.yaml(1 hunks)sippy-ng/src/component_readiness/JobArtifactQuery.js(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
sippy-ng/src/component_readiness/JobArtifactQuery.js (1)
sippy-ng/src/helpers.js (1)
url(338-338)
🔇 Additional comments (2)
chat/prompts/payload-analysis.yaml (1)
106-112: Revert text formatting change looks goodSwitching the high-confidence revert notice to
"** REVERT RECOMMENDED **"keeps the instruction explicit while avoiding emoji; no behavioral impact on the workflow.sippy-ng/src/component_readiness/JobArtifactQuery.js (1)
594-601: URL validation aroundwindow.openis correctly tightenedThe new
validateURLhelpers (Lines 594-601 and 804-811) and the guards aroundwindow.open(Lines 603-608 and 813-822) safely restrict navigation tohttp/httpsURLs and handle parse errors viatry/catch. This meaningfully hardens these paths without changing intended behavior.Also applies to: 603-608, 804-822
xiaojiey
force-pushed
the
compliance-dashboard
branch
from
d9b5bd5 to
2d2365c
Compare
|
@xiaojiey: This pull request references CMP-3987 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 story to target the "4.22.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. |
|
@xiaojiey: 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. |
|
@neisw @deepsm007 Could you please help to review this PR, thanks. |
2 participants
Overview
This PR adds support for tracking Component Readiness for the Compliance Operator as a separate layered product dashboard. The Compliance Operator includes both upstream (ComplianceAsCode) and downstream testing, and we need dedicated dashboards to track readiness across these components.
Component Readiness views are added for Compliance across actively supported OCP versions (4.16-4.21).
Changes
Added Component Readiness views for Compliance across OCP versions:
Related Jobs
This tracks the following Compliance periodic jobs:
Upstream (ComplianceAsCode) Jobs:
Downstream Jobs:
Coverage includes versions 4.12-4.21 across amd64, arm64, and multi architectures on platforms: aws, azure, gcp, baremetal, vsphere.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.