Require all Prow CI checks in branch protection for SRE operators#81412
Conversation
|
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 change updates Prow branch-protection requirements for several OpenShift repositories and removes ChangesProw branch protection
CI job config
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 15✅ Passed checks (15 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
c3dbb3b to
103333a
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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
`@ci-operator/step-registry/rosa/cluster-lease/health/rosa-cluster-lease-health-commands.sh`:
- Around line 126-139: The cluster lease health check currently defaults missing
ocm-env to staging in the CLUSTER_OCM_ENV assignment, which can misclassify
leases from the script’s default OCM environment. Update the handling around
CLUSTER_OCM_ENV and ocm_ensure_env so missing ocm-env falls back to
OCM_LOGIN_ENV (or aborts the health check before any error patch is applied)
instead of staging. Keep the existing OCM_STATUS evaluation and patching logic
in rosa-cluster-lease-health-commands.sh gated on the corrected environment
resolution.
- Around line 45-50: The login flow in the cluster lease health script updates
CURRENT_OCM_ENV even when no ocm login actually happened, which can leave the
previous session active and make later checks use the wrong environment. Move
the CURRENT_OCM_ENV assignment so it only runs inside the successful login
branches in the rosa-cluster-lease-health-commands.sh logic, using the existing
SSO_CLIENT_ID/SSO_CLIENT_SECRET and OCM_TOKEN paths as the only valid places to
set it.
🪄 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: de2654fb-1bce-435e-91b0-bbb2f13e226a
📒 Files selected for processing (7)
ci-operator/step-registry/rosa/cluster-lease/controller/rosa-cluster-lease-controller-commands.shci-operator/step-registry/rosa/cluster-lease/health/rosa-cluster-lease-health-commands.shcore-services/prow/02_config/openshift/cloud-ingress-operator/_prowconfig.yamlcore-services/prow/02_config/openshift/configure-alertmanager-operator/_prowconfig.yamlcore-services/prow/02_config/openshift/custom-domains-operator/_prowconfig.yamlcore-services/prow/02_config/openshift/pagerduty-operator/_prowconfig.yamlcore-services/prow/02_config/openshift/splunk-forwarder-operator/_prowconfig.yaml
103333a to
21fc531
Compare
21fc531 to
e1f985e
Compare
|
/hold |
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
`@core-services/prow/02_config/openshift/cloud-ingress-operator/_prowconfig.yaml`:
- Around line 12-17: Remove the hardcoded ci/prow/coverage, ci/prow/lint,
ci/prow/test, and ci/prow/validate entries from the cloud-ingress-operator prow
config so it matches the PR’s intended design. In _prowconfig.yaml, keep only
the truly required always-on checks and rely on the existing
required-if-present-contexts wiring already defined for this repo in the shared
prow config. Use the cloud-ingress-operator required_status_checks section as
the target for cleanup.
🪄 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: f91ff484-8912-45aa-836b-033ecca107da
📒 Files selected for processing (1)
core-services/prow/02_config/openshift/cloud-ingress-operator/_prowconfig.yaml
e1f985e to
2252b24
Compare
There was a problem hiding this comment.
Actionable comments posted: 9
🤖 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
`@core-services/prow/02_config/openshift/aws-account-operator/_prowconfig.yaml`:
- Around line 12-17: Remove the duplicated ci/prow/coverage, ci/prow/lint,
ci/prow/prek, ci/prow/test, and ci/prow/validate entries from the
required_status_checks.contexts block in _prowconfig.yaml. These checks are
already configured as required-if-present via the shared Prow config in
_config.yaml, so this local config should not hard-require them; keep only the
genuinely required contexts here to stay consistent with checkconfig --strict.
In
`@core-services/prow/02_config/openshift/configure-alertmanager-operator/_prowconfig.yaml`:
- Around line 12-17: Remove the `skip_if_only_changed`-gated contexts from
`required_status_checks.contexts` in `configure-alertmanager-operator`’s
`_prowconfig.yaml`; the affected entries are the ones added for coverage, lint,
test, and validate. Keep the existing required-if-present handling aligned with
the shared config by ensuring these checks remain under the
`required-if-present-contexts` mechanism instead of being hard-required in the
`required_status_checks` list.
In
`@core-services/prow/02_config/openshift/custom-domains-operator/_prowconfig.yaml`:
- Around line 9-17: The new branch-protection configuration is hard-requiring
Prow checks that are intended to be required-if-present. Update the
custom-domains-operator branch-protection block by removing the
skip_if_only_changed-based contexts from required_status_checks.contexts and
keep only the always-required checks such as the Konflux context,
ci/prow/images, and ci/prow/e2e-binary-build-success. Use the branch-protection
stanza in the _prowconfig.yaml config to ensure Tide can enforce the conditional
checks without conflicting with checkconfig --strict.
In
`@core-services/prow/02_config/openshift/deadmanssnitch-operator/_prowconfig.yaml`:
- Around line 12-16: The required status checks for deadmanssnitch-operator
currently include checks that the PR rationale says are registered as
required-if-present via skip_if_only_changed, which conflicts with checkconfig
--strict. Update the _prowconfig.yaml required_status_checks list to remove
ci/prow/coverage, ci/prow/lint, ci/prow/test, and ci/prow/validate, keeping only
the checks that should be strictly required; use the existing
required_status_checks block to make this adjustment.
In
`@core-services/prow/02_config/openshift/gcp-project-operator/_prowconfig.yaml`:
- Around line 12-16: Remove the newly added ci/prow/coverage, ci/prow/lint,
ci/prow/test, and ci/prow/validate entries from required_status_checks in the
_prowconfig.yaml configuration. Keep the required checks aligned with the PR’s
intended skip_if_only_changed behavior and the existing pattern used by
deadmanssnitch-operator, so only the non-gated checks remain required.
In
`@core-services/prow/02_config/openshift/must-gather-operator/_prowconfig.yaml`:
- Around line 1-16: Add only the branch-protection required_status_checks that
match the PR scope in the must-gather-operator config. In the branch-protection
entry for master, update the contexts list to keep just the intended Prow jobs
and remove the extra ci/prow/coverage, ci/prow/lint, and ci/prow/test
requirements; use the existing branch-protection block and its
required_status_checks contexts to locate the change.
In `@core-services/prow/02_config/openshift/osd-cluster-ready/_prowconfig.yaml`:
- Around line 13-14: Remove the newly added ci/prow/lint and ci/prow/test
entries from the required_status_checks list in the osd-cluster-ready Prow
config, since these jobs are skip_if_only_changed-gated and should not be
treated as required. Update the _prowconfig.yaml section that defines
required_status_checks so it only contains the checks that are meant to be
mandatory, keeping it consistent with the PR’s stated checkconfig --strict
intent.
In `@core-services/prow/02_config/openshift/pagerduty-operator/_prowconfig.yaml`:
- Around line 12-16: The required_status_checks.contexts list in the
pagerduty-operator Prow config is duplicating checks that are already covered by
required-if-present-contexts. Remove the skip_if_only_changed-gated entries from
the required_status_checks block and leave them only in the shared
required-if-present-contexts list, using the existing pagerduty-operator config
sections to locate the duplicate contexts.
In
`@core-services/prow/02_config/openshift/splunk-forwarder-operator/_prowconfig.yaml`:
- Around line 9-17: The new branch-protection entry in the _prowconfig.yaml
config is incorrectly hard-requiring checks that should remain
required-if-present. Update the required_status_checks.contexts for the
splunk-forwarder-operator branch protection block to remove the
skip_if_only_changed checks (ci/prow/coverage, ci/prow/lint, ci/prow/test,
ci/prow/validate) while keeping only the truly required contexts, using the same
pattern as the custom-domains-operator config to avoid checkconfig --strict
conflicts.
🪄 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: 1c5bac4e-3b6f-4ddb-94f1-caead2748f2f
⛔ Files ignored due to path filters (10)
ci-operator/jobs/openshift/aws-account-operator/openshift-aws-account-operator-master-presubmits.yamlis excluded by!ci-operator/jobs/**ci-operator/jobs/openshift/cloud-ingress-operator/openshift-cloud-ingress-operator-master-presubmits.yamlis excluded by!ci-operator/jobs/**ci-operator/jobs/openshift/configure-alertmanager-operator/openshift-configure-alertmanager-operator-master-presubmits.yamlis excluded by!ci-operator/jobs/**ci-operator/jobs/openshift/custom-domains-operator/openshift-custom-domains-operator-master-presubmits.yamlis excluded by!ci-operator/jobs/**ci-operator/jobs/openshift/deadmanssnitch-operator/openshift-deadmanssnitch-operator-master-presubmits.yamlis excluded by!ci-operator/jobs/**ci-operator/jobs/openshift/gcp-project-operator/openshift-gcp-project-operator-master-presubmits.yamlis excluded by!ci-operator/jobs/**ci-operator/jobs/openshift/managed-velero-operator/openshift-managed-velero-operator-master-presubmits.yamlis excluded by!ci-operator/jobs/**ci-operator/jobs/openshift/must-gather-operator/openshift-must-gather-operator-master-presubmits.yamlis excluded by!ci-operator/jobs/**ci-operator/jobs/openshift/pagerduty-operator/openshift-pagerduty-operator-master-presubmits.yamlis excluded by!ci-operator/jobs/**ci-operator/jobs/openshift/splunk-forwarder-operator/openshift-splunk-forwarder-operator-master-presubmits.yamlis excluded by!ci-operator/jobs/**
📒 Files selected for processing (22)
ci-operator/config/openshift/aws-account-operator/openshift-aws-account-operator-master.yamlci-operator/config/openshift/cloud-ingress-operator/openshift-cloud-ingress-operator-master.yamlci-operator/config/openshift/configure-alertmanager-operator/openshift-configure-alertmanager-operator-master.yamlci-operator/config/openshift/custom-domains-operator/openshift-custom-domains-operator-master.yamlci-operator/config/openshift/deadmanssnitch-operator/openshift-deadmanssnitch-operator-master.yamlci-operator/config/openshift/gcp-project-operator/openshift-gcp-project-operator-master.yamlci-operator/config/openshift/managed-velero-operator/openshift-managed-velero-operator-master.yamlci-operator/config/openshift/must-gather-operator/openshift-must-gather-operator-master.yamlci-operator/config/openshift/pagerduty-operator/openshift-pagerduty-operator-master.yamlci-operator/config/openshift/splunk-forwarder-operator/openshift-splunk-forwarder-operator-master.yamlcore-services/prow/02_config/openshift/aws-account-operator/_prowconfig.yamlcore-services/prow/02_config/openshift/cloud-ingress-operator/_prowconfig.yamlcore-services/prow/02_config/openshift/configure-alertmanager-operator/_prowconfig.yamlcore-services/prow/02_config/openshift/custom-domains-operator/_prowconfig.yamlcore-services/prow/02_config/openshift/deadmanssnitch-operator/_prowconfig.yamlcore-services/prow/02_config/openshift/gcp-project-operator/_prowconfig.yamlcore-services/prow/02_config/openshift/managed-cluster-validating-webhooks/_prowconfig.yamlcore-services/prow/02_config/openshift/managed-velero-operator/_prowconfig.yamlcore-services/prow/02_config/openshift/must-gather-operator/_prowconfig.yamlcore-services/prow/02_config/openshift/osd-cluster-ready/_prowconfig.yamlcore-services/prow/02_config/openshift/pagerduty-operator/_prowconfig.yamlcore-services/prow/02_config/openshift/splunk-forwarder-operator/_prowconfig.yaml
💤 Files with no reviewable changes (10)
- ci-operator/config/openshift/must-gather-operator/openshift-must-gather-operator-master.yaml
- ci-operator/config/openshift/managed-velero-operator/openshift-managed-velero-operator-master.yaml
- ci-operator/config/openshift/splunk-forwarder-operator/openshift-splunk-forwarder-operator-master.yaml
- ci-operator/config/openshift/pagerduty-operator/openshift-pagerduty-operator-master.yaml
- ci-operator/config/openshift/deadmanssnitch-operator/openshift-deadmanssnitch-operator-master.yaml
- ci-operator/config/openshift/aws-account-operator/openshift-aws-account-operator-master.yaml
- ci-operator/config/openshift/cloud-ingress-operator/openshift-cloud-ingress-operator-master.yaml
- ci-operator/config/openshift/custom-domains-operator/openshift-custom-domains-operator-master.yaml
- ci-operator/config/openshift/gcp-project-operator/openshift-gcp-project-operator-master.yaml
- ci-operator/config/openshift/configure-alertmanager-operator/openshift-configure-alertmanager-operator-master.yaml
✅ Files skipped from review due to trivial changes (1)
- core-services/prow/02_config/openshift/managed-velero-operator/_prowconfig.yaml
🚧 Files skipped from review as they are similar to previous changes (1)
- core-services/prow/02_config/openshift/cloud-ingress-operator/_prowconfig.yaml
7f1384c to
db5860d
Compare
|
/retest config |
|
@dustman9000: The The following commands are available to trigger optional jobs: Use 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 kubernetes-sigs/prow repository. |
Several SRE operator repos only required ci/prow/images in branch protection, allowing PRs to merge with failing lint, test, coverage, and validate checks. Discovered when Dependabot auto-merge merged cloud-ingress-operator PRs openshift#505-openshift#511 with 3 failing checks. Three changes per repo: 1. Remove skip_if_only_changed/run_if_changed from CI configs so all tests become always_run 2. Add all non-optional checks to required_status_checks in branch protection prowconfigs 3. Replace required-if-present-contexts in _config.yaml with skip-unknown-contexts (matching rbac-permissions-operator pattern) to avoid checkconfig conflict Repos updated (12): - cloud-ingress-operator - configure-alertmanager-operator - custom-domains-operator (new branch-protection) - deadmanssnitch-operator - splunk-forwarder-operator (new branch-protection) - must-gather-operator (new branch-protection) - aws-account-operator - gcp-project-operator - managed-velero-operator - managed-cluster-validating-webhooks - osd-cluster-ready - pagerduty-operator
db5860d to
dee7a28
Compare
|
[REHEARSALNOTIFIER]
A total of 49 jobs have been affected by this change. The above listing is non-exhaustive and limited to 25 jobs. A full list of affected jobs can be found here Interacting with pj-rehearseComment: Once you are satisfied with the results of the rehearsals, comment: |
|
/hold cancel |
|
/pj-rehearse ack |
|
@dustman9000: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
/lgtm |
|
/lgtm |
|
/approve |
|
@dustman9000: Updated the following 2 configmaps:
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 kubernetes-sigs/prow repository. |
Summary
Several SRE operator repos only required
ci/prow/imagesin branch protection, allowing PRs to merge with failing lint, test, coverage, and validate checks. Discovered when Dependabot auto-merge merged cloud-ingress-operator PRs #505-#511 with 3 failing checks.Three changes per repo:
Remove
skip_if_only_changedandrun_if_changedfrom CI configs so all tests becomealways_run. Trade-off: tests now run on docs-only PRs (sub-2-minute cost).Add all non-optional Prow checks to
required_status_checksin branch protection prowconfigs. Branchprotector syncs these to GitHub every 6 hours.Replace
required-if-present-contextswithskip-unknown-contextsin_config.yamlTide context options (matching the rbac-permissions-operator pattern). This resolves thecheckconfig --strictconflict where contexts can't be bothrequiredandrequired-if-present.Repos updated (12):
Test plan