-
Notifications
You must be signed in to change notification settings - Fork 97
CMP-3987: Add Component Readiness dashboard for Compliance Operator #3137
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
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 |
WalkthroughAdds lp-interop-compliance readiness configuration blocks, registers a "Compliance-lp-interop" test suite, maps compliance-related job name patterns to the lp-interop-compliance layered product, and updates periodic CI job snapshots to use Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 5 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
📜 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 (4)
🔇 Additional comments (4)
Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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: 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: 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. |
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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)
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
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. |
|
@neisw @deepsm007 Could you please help to review this PR, thanks. |
2d2365c to
7a16b9d
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. |
7a16b9d to
6d5a4e4
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @config/views.yaml:
- Around line 2922-2923: Resolve the merge conflict by setting
regression_tracking.enabled to false for the COO views listed: update the
entries for 4.19-lp-Interop-coo, 4.18-lp-Interop-coo, 4.17-lp-Interop-coo, and
4.16-lp-Interop-coo so that each has regression_tracking: enabled: false
(replace the conflicting true values from the feature branch), ensure the final
YAML contains only the false value for those view blocks, and remove any
leftover conflict markers.
- Line 1678: The file contains unresolved Git conflict markers ("<<<<<<< HEAD",
"=======", ">>>>>>> 7a16b9d6") that break YAML parsing; rebase onto main,
resolve the conflict by selecting or merging the intended changes between the
two versions (remove the conflict markers and keep the correct YAML nodes),
commit the resolved content, and validate the file with a YAML linter to ensure
no syntax errors remain.
📜 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 (1)
config/views.yaml
6d5a4e4 to
10cea42
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. |
10cea42 to
a8d66f2
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @config/views.yaml:
- Around line 1677-1679: The YAML comment is indented too far and causes a
parsing error; move the comment so it aligns with the surrounding keys (same
indentation level as relative_start and relative_end) or remove it entirely;
update the comment line that currently precedes relative_start so its leading
spaces match the keys in this block (referencing the keys relative_start and
relative_end) to fix the YAML syntax.
🧹 Nitpick comments (1)
config/views.yaml (1)
2250-2261: Consider adding JobTier filter to ensure job stability.The compliance views don't specify a
JobTierfilter ininclude_variants, unlike standard views (e.g., lines 646-649 for4.21-main). Without this filter, the views will include jobs of all tiers, potentially includingcandidateorhiddenjobs that may not be stable enough for component readiness tracking.♻️ Suggested addition
include_variants: Architecture: - amd64 - arm64 + JobTier: + - blocking + - informing + - standard LayeredProduct: - lp-interop-complianceApply this same pattern to all six compliance view blocks (4.21, 4.20, 4.19, 4.18, 4.17, 4.16).
📜 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.yamlpkg/db/suites.gopkg/variantregistry/ocp.gopkg/variantregistry/snapshot.yaml
🧰 Additional context used
🪛 YAMLlint (1.37.1)
config/views.yaml
[error] 1678-1678: syntax error: expected , but found ''
(syntax)
🔇 Additional comments (3)
pkg/db/suites.go (1)
72-72: LGTM! Test suite addition aligns with compliance operator support.The new "Compliance-lp-interop" test suite follows the established naming convention for lp-interop suites and integrates correctly with the LayeredProduct mappings added in
pkg/variantregistry/ocp.go.pkg/variantregistry/ocp.go (1)
1111-1113: LGTM! Compliance job mappings follow the established pattern.The three new LayeredProduct mappings correctly route compliance-related jobs to
lp-interop-compliance. The ordering is appropriate, with specific patterns (-complianceascode-,-compliance-destructive) checked before the broader-compliancepattern. This ensures precise matching for specialized compliance jobs while catching all other compliance jobs with the general pattern.config/views.yaml (1)
2232-2501: Compliance view configurations are well-structured and address previous feedback.The six compliance view blocks (4.21, 4.20, 4.19, 4.18, 4.17, 4.16) correctly include both
engandqein the Owner filter, addressing the past review concern about downstream QE-owned compliance jobs. Theregression_tracking: enabled: falsesetting aligns with reviewer guidance. The view structure is consistent with other LP-Interop views.
|
@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. |
Fix the check failures Fix yaml lint failure Update regression_tracking and base_release per comments
a8d66f2 to
a84e3cf
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. |
|
Scheduling required tests: |
|
@xiaojiey: 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. |
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
New Features
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.