You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
None of our OpenShift/OADP forks of Velero (or its plugins) currently configure required_status_checks.contexts in their Prow branch-protection config. This means merges are gated only by tide's label requirements (approved, lgtm, no hold/work-in-progress/etc.), not by any specific CI job actually passing. A PR can theoretically merge with failing GitHub Actions checks as long as the label conditions are met.
Add a required_status_checks.contexts list to the openshift/velero prowconfig (and the sibling plugin-fork prowconfigs — see below), e.g.:
branch-protection:
orgs:
openshift:
repos:
velero:
allow_force_pushes: trueinclude:
- oadp-dev
- ^oadp-protect: truerequired_status_checks:
contexts:
- ci/prow/lint
- ci/prow/images
- Run CI
- Run Linter Check
Context names pulled from a recent openshift/velero PR (#494/#517) so the list reflects the actual status/check names GitHub reports today:
Prow-native contexts (from the commit status API): ci/prow/unit-test, ci/prow/images, ci/prow/lint
GitHub Actions checks currently running unguarded in the fork: Run CI, Run Linter Check, Check for invalid characters in file paths, Run Codespell, run-e2e-test (matrix), build
This lines up with the review discussion on velero-io#10001 (which added if: github.repository == 'velero-io/velero' guards to several workflows so they skip on forks). In that PR, @kaovilai specifically called out checks that should keep running — and by extension be good candidates for "required" — in the openshift/velero fork:
e2e-test-kind.yaml: "We wanted this in our fork, so we would like to keep running these tests in the forks if possible. There is nothing inherently specific to the upstream repo here, as evidenced by the many passing rates in our forks of these jobs."
pr-linter-check.yml: "I think we still want these so that if we develop features that will be later put back on upstream, the lint is up to date the config in upstream."
nightly-trivy-scan.yml: "We wanted this in our downstream fork."
pr-filepath-check.yml: "Though we probably also want this" — note this one was guarded by that PR (now skipped on forks) despite that comment, which is worth double-checking/reverting if we want it back as a required check here.
Reduce duplicate test effort: Makefile.prow vs. upstream GitHub Actions
Makefile.prow (in openshift/velero) is what backs the Prow-native ci/prow/unit-test and ci/prow/lint contexts:
.PHONY: ci
ci: verify-modules verify all test
.PHONY: test
test: setup-env
...
KUBEBUILDER_ASSETS=$(KUBEBUILDER_ASSETS) GOFLAGS=$(GOFLAGS) hack/test.sh
.PHONY: lint
lint: golangci-lint
PATH=$(PROWBIN):$(PATH) GOFLAGS=$(GOFLAGS) hack/lint.sh
This wraps the same hack/test.sh / hack/lint.sh that upstream velero-io/velero runs via its own GitHub Actions (Run CI → make ci, Run Linter Check). So today we run the equivalent test/lint twice on every PR: once through Prow (ci/prow/unit-test, ci/prow/lint, driven by Makefile.prow), and once through the GitHub Actions inherited from upstream (Run CI, Run Linter Check). That's duplicate CI effort, and it means test/lint fixes or config drift have to be reconciled in two separate places (Makefile.prow here vs. hack/*.sh and workflow files upstream) instead of one.
Proposal: retire the redundant Prow-native ci/prow/unit-test / ci/prow/lint jobs (or point Makefile.prow at the same invocation path, not a separate wrapper) and standardize on the upstream-aligned GitHub Actions checks (Run CI, Run Linter Check) as the source of truth for required status checks. That keeps us upstream-aligned on tests — we get upstream's test fixes for free, and any test fixes we make land in the same place upstream would look for them, instead of being siloed in Makefile.prow.
Plugin forks (aws / gcp / microsoft-azure)
Filing this against openshift/velero only, since the plugin fork repos (openshift/velero-plugin-for-aws, openshift/velero-plugin-for-gcp, openshift/velero-plugin-for-microsoft-azure) don't have an Issues tab enabled.
Each of these forks has an upstream in velero-io and has the same gap: their _prowconfig.yaml (e.g. core-services/prow/02_config/openshift/velero-plugin-for-aws/_prowconfig.yaml) also has no required_status_checks. Each currently runs a single unguarded GitHub Actions workflow (.github/workflows/pr.yaml, job named Run CI, doing make ci + Codecov upload) plus the shared Prow ci/prow/* contexts. The same fix — and the same Makefile.prow-vs-upstream-CI de-duplication question — applies there too:
required_status_checks:
contexts:
- Run CI
(openshift/velero-plugin-for-csi and openshift/velero-plugin-for-legacy-aws are excluded from this ask — they have no velero-io upstream.)
Ask
Add required_status_checks.contexts to the openshift/velero prowconfig in openshift/release, and the same to the three plugin-fork prowconfigs listed above.
Decide whether to retire the Makefile.prow-driven ci/prow/unit-test / ci/prow/lint Prow jobs in favor of the upstream-aligned GitHub Actions checks, to stop running duplicate test/lint effort and stay aligned with upstream fixes.
Summary
None of our OpenShift/OADP forks of Velero (or its plugins) currently configure
required_status_checks.contextsin their Prow branch-protection config. This means merges are gated only bytide's label requirements (approved,lgtm, nohold/work-in-progress/etc.), not by any specific CI job actually passing. A PR can theoretically merge with failing GitHub Actions checks as long as the label conditions are met.Prow's branch-protection
Policysupports arequired_status_checksfield (ContextPolicy.Contexts []string— "Contexts appends required contexts that must be green to merge"):https://github.com/kubernetes-sigs/prow/blob/0633879af8026d056e1a5dbe1e29f5a98f6acec3/pkg/config/branch_protection.go#L38
Our current config for
openshift/velerohas no such block:https://github.com/openshift/release/blob/main/core-services/prow/02_config/openshift/velero/_prowconfig.yaml#L1
Proposal
Add a
required_status_checks.contextslist to theopenshift/veleroprowconfig (and the sibling plugin-fork prowconfigs — see below), e.g.:Context names pulled from a recent
openshift/veleroPR (#494/#517) so the list reflects the actual status/check names GitHub reports today:ci/prow/unit-test,ci/prow/images,ci/prow/lintRun CI,Run Linter Check,Check for invalid characters in file paths,Run Codespell,run-e2e-test(matrix),buildThis lines up with the review discussion on velero-io#10001 (which added
if: github.repository == 'velero-io/velero'guards to several workflows so they skip on forks). In that PR, @kaovilai specifically called out checks that should keep running — and by extension be good candidates for "required" — in theopenshift/velerofork:e2e-test-kind.yaml: "We wanted this in our fork, so we would like to keep running these tests in the forks if possible. There is nothing inherently specific to the upstream repo here, as evidenced by the many passing rates in our forks of these jobs."pr-linter-check.yml: "I think we still want these so that if we develop features that will be later put back on upstream, the lint is up to date the config in upstream."nightly-trivy-scan.yml: "We wanted this in our downstream fork."pr-filepath-check.yml: "Though we probably also want this" — note this one was guarded by that PR (now skipped on forks) despite that comment, which is worth double-checking/reverting if we want it back as a required check here.Reduce duplicate test effort: Makefile.prow vs. upstream GitHub Actions
Makefile.prow(inopenshift/velero) is what backs the Prow-nativeci/prow/unit-testandci/prow/lintcontexts:https://github.com/openshift/velero/blob/main/Makefile.prow
This wraps the same
hack/test.sh/hack/lint.shthat upstreamvelero-io/veleroruns via its own GitHub Actions (Run CI→make ci,Run Linter Check). So today we run the equivalent test/lint twice on every PR: once through Prow (ci/prow/unit-test,ci/prow/lint, driven byMakefile.prow), and once through the GitHub Actions inherited from upstream (Run CI,Run Linter Check). That's duplicate CI effort, and it means test/lint fixes or config drift have to be reconciled in two separate places (Makefile.prowhere vs.hack/*.shand workflow files upstream) instead of one.Proposal: retire the redundant Prow-native
ci/prow/unit-test/ci/prow/lintjobs (or pointMakefile.prowat the same invocation path, not a separate wrapper) and standardize on the upstream-aligned GitHub Actions checks (Run CI,Run Linter Check) as the source of truth for required status checks. That keeps us upstream-aligned on tests — we get upstream's test fixes for free, and any test fixes we make land in the same place upstream would look for them, instead of being siloed inMakefile.prow.Plugin forks (aws / gcp / microsoft-azure)
Filing this against
openshift/veleroonly, since the plugin fork repos (openshift/velero-plugin-for-aws,openshift/velero-plugin-for-gcp,openshift/velero-plugin-for-microsoft-azure) don't have an Issues tab enabled.Each of these forks has an upstream in
velero-ioand has the same gap: their_prowconfig.yaml(e.g.core-services/prow/02_config/openshift/velero-plugin-for-aws/_prowconfig.yaml) also has norequired_status_checks. Each currently runs a single unguarded GitHub Actions workflow (.github/workflows/pr.yaml, job namedRun CI, doingmake ci+ Codecov upload) plus the shared Prowci/prow/*contexts. The same fix — and the same Makefile.prow-vs-upstream-CI de-duplication question — applies there too:(
openshift/velero-plugin-for-csiandopenshift/velero-plugin-for-legacy-awsare excluded from this ask — they have novelero-ioupstream.)Ask
required_status_checks.contextsto theopenshift/veleroprowconfig inopenshift/release, and the same to the three plugin-fork prowconfigs listed above.pr-filepath-check.ymlbeing skipped on forks (per Skip upstream-only workflows on forks velero-io/velero#10001) conflicts with wanting it as a required check.Makefile.prow-drivenci/prow/unit-test/ci/prow/lintProw jobs in favor of the upstream-aligned GitHub Actions checks, to stop running duplicate test/lint effort and stay aligned with upstream fixes.Note
Responses generated with Claude