🐛 OCPBUGS-81452: prevent webhook rollout stalls by issuing certificates earlier#2616
🐛 OCPBUGS-81452: prevent webhook rollout stalls by issuing certificates earlier#2616
Conversation
openshift-ci
bot
added
do-not-merge/work-in-progress
✅ Deploy Preview for olmv1 ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
There was a problem hiding this comment.
Pull request overview
This PR is intended to reduce webhook rollout stalls by issuing cert-manager Certificate objects earlier, but the provided diff primarily changes build/CI and repository metadata rather than applier phase ordering.
Changes:
- Adjust
go buildquoting for-ldflagsin theMakefile - Add downstream repo metadata (
commitchecker.yaml,DOWNSTREAM_OWNERS*,.ci-operator.yaml) and a nestedgo.modunderhack/ - Remove multiple GitHub Actions workflows and GitHub metadata files (templates, issue templates, dependabot)
Reviewed changes
Copilot reviewed 27 out of 25191 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| hack/kind-config/containerd/certs.d/go.mod | Introduces a nested module marker; adds a go directive. |
| commitchecker.yaml | Adds commitchecker configuration for an upstream merge base. |
| Makefile | Updates go build invocation to use double quotes for -ldflags; adds explanatory comments. |
| DOWNSTREAM_OWNERS_ALIASES | Adds downstream reviewer/approver alias definitions. |
| DOWNSTREAM_OWNERS | Adds downstream reviewers/approvers configuration referencing aliases. |
| .github/workflows/update-demos.yaml | Removes workflow. |
| .github/workflows/unit-test.yaml | Removes workflow. |
| .github/workflows/tilt.yaml | Removes workflow. |
| .github/workflows/test-regression.yaml | Removes workflow. |
| .github/workflows/stale.yml | Removes workflow. |
| .github/workflows/sanity.yaml | Removes workflow. |
| .github/workflows/release.yaml | Removes workflow. |
| .github/workflows/pr-title.yaml | Removes workflow. |
| .github/workflows/pages.yaml | Removes workflow. |
| .github/workflows/go-verdiff.yaml | Removes workflow. |
| .github/workflows/go-apidiff.yaml | Removes workflow. |
| .github/workflows/files-diff.yaml | Removes workflow. |
| .github/workflows/e2e.yaml | Removes workflow. |
| .github/workflows/crd-diff.yaml | Removes workflow. |
| .github/workflows/api-diff-lint.yaml | Removes workflow. |
| .github/workflows/add-to-project.yaml | Removes workflow. |
| .github/pull_request_template.md | Removes PR template. |
| .github/dependabot.yml | Removes Dependabot config. |
| .github/OWNERS | Removes GitHub OWNERS file. |
| .github/ISSUE_TEMPLATE/docs-issue.yml | Removes issue template. |
| .github/ISSUE_TEMPLATE/config.yml | Removes issue template config. |
| .ci-operator.yaml | Adds OpenShift CI build root image reference (golang 1.25). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
commitchecker.yaml
Outdated
| expectedMergeBase: 253feaef5babbdabfb4b63d1cb3a60455947be3a | ||
| upstreamBranch: main | ||
| upstreamOrg: operator-framework | ||
| upstreamRepo: operator-controller |
There was a problem hiding this comment.
The PR description/title focus on changing applier phase ordering for cert-manager Certificate objects and updating phase sorting tests, but the provided diff shows only build/CI/repo metadata changes (Makefile quoting, downstream ownership files, CI config, and deletion of GitHub workflows). Please either (a) update the PR description/title to match the actual changes, or (b) split these infra/CI changes into a separate PR and include the missing phase-ordering/test diffs here.
DOWNSTREAM_OWNERS_ALIASES
Outdated
| - grokspawn | ||
| - joelanford | ||
| - oceanc80 | ||
| - perdasilva | ||
| - tmshort | ||
| - jianzhangbjz | ||
|
|
||
| # contributors who can review/lgtm any PRs in the repo | ||
| operator-framework-reviewers: | ||
| - anik120 | ||
| - ankitathomas | ||
| - bentito | ||
| - camilamacedo86 | ||
| - dtfranz | ||
| - grokspawn | ||
| - joelanford | ||
| - oceanc80 | ||
| - pedjak | ||
| - perdasilva | ||
| - rashmigottipati | ||
| - thetechnick | ||
| - tmshort | ||
| - trgeiger |
There was a problem hiding this comment.
This YAML indentation appears invalid: the sequence items under operator-framework-approvers: are aligned with the key instead of being indented beneath it. YAML parsers will typically reject this. Indent list items (and the corresponding reviewer list below) at least one level deeper than the alias key.
| - grokspawn | |
| - joelanford | |
| - oceanc80 | |
| - perdasilva | |
| - tmshort | |
| - jianzhangbjz | |
| # contributors who can review/lgtm any PRs in the repo | |
| operator-framework-reviewers: | |
| - anik120 | |
| - ankitathomas | |
| - bentito | |
| - camilamacedo86 | |
| - dtfranz | |
| - grokspawn | |
| - joelanford | |
| - oceanc80 | |
| - pedjak | |
| - perdasilva | |
| - rashmigottipati | |
| - thetechnick | |
| - tmshort | |
| - trgeiger | |
| - grokspawn | |
| - joelanford | |
| - oceanc80 | |
| - perdasilva | |
| - tmshort | |
| - jianzhangbjz | |
| # contributors who can review/lgtm any PRs in the repo | |
| operator-framework-reviewers: | |
| - anik120 | |
| - ankitathomas | |
| - bentito | |
| - camilamacedo86 | |
| - dtfranz | |
| - grokspawn | |
| - joelanford | |
| - oceanc80 | |
| - pedjak | |
| - perdasilva | |
| - rashmigottipati | |
| - thetechnick | |
| - tmshort | |
| - trgeiger |
Makefile
Outdated
| # Use double quotes (not single quotes) for build flags like -ldflags, -tags, etc. | ||
| # Single quotes are passed literally and not stripped by `go build`, which can cause errors | ||
| # or inject unintended characters into the binary (e.g., version metadata). | ||
| go build $(GO_BUILD_FLAGS) $(GO_BUILD_EXTRA_FLAGS) -tags '$(GO_BUILD_TAGS)' -ldflags "$(GO_BUILD_LDFLAGS)" -gcflags '$(GO_BUILD_GCFLAGS)' -asmflags '$(GO_BUILD_ASMFLAGS)' -o ./testdata/push/bin/push ./testdata/push/push.go |
There was a problem hiding this comment.
The comment claims single quotes are passed literally to go build, but in typical POSIX shells quotes are removed by the shell; the real issue here appears to be nested quoting because $(GO_BUILD_LDFLAGS) itself contains single quotes. Also, the command still uses single quotes for -tags, -gcflags, and -asmflags, which contradicts the comment. Please update the comment to accurately describe the nested-quote problem (and/or align the quoting usage with what the comment recommends).
| # Use double quotes (not single quotes) for build flags like -ldflags, -tags, etc. | |
| # Single quotes are passed literally and not stripped by `go build`, which can cause errors | |
| # or inject unintended characters into the binary (e.g., version metadata). | |
| go build $(GO_BUILD_FLAGS) $(GO_BUILD_EXTRA_FLAGS) -tags '$(GO_BUILD_TAGS)' -ldflags "$(GO_BUILD_LDFLAGS)" -gcflags '$(GO_BUILD_GCFLAGS)' -asmflags '$(GO_BUILD_ASMFLAGS)' -o ./testdata/push/bin/push ./testdata/push/push.go | |
| # Be careful with nested quoting in build flags (e.g., -ldflags, -tags, etc.). | |
| # $(GO_BUILD_LDFLAGS) may contain single-quoted segments, so we wrap it in double quotes | |
| # here to avoid malformed arguments being passed to `go build`. | |
| go build $(GO_BUILD_FLAGS) $(GO_BUILD_EXTRA_FLAGS) -tags "$(GO_BUILD_TAGS)" -ldflags "$(GO_BUILD_LDFLAGS)" -gcflags "$(GO_BUILD_GCFLAGS)" -asmflags "$(GO_BUILD_ASMFLAGS)" -o ./testdata/push/bin/push ./testdata/push/push.go |
.ci-operator.yaml
Outdated
| build_root_image: | ||
| name: release | ||
| namespace: openshift | ||
| tag: rhel-9-release-golang-1.25-openshift-4.22 |
There was a problem hiding this comment.
This PR deletes a large number of GitHub Actions workflows (unit tests, e2e, sanity/lint, release, docs/pages, stale bot, diff checks). If this repo is expected to rely on OpenShift CI instead, it would be helpful to (a) confirm the replacement CI signal covers the removed checks, and (b) document that migration so contributors don’t lose required automation unexpectedly.
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: 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 |
bandrade
changed the title
bandrade
force-pushed
the
feature/OCPBUGS-81452-rollout-stall
branch
from
57d188a to
3a7e987
Compare
openshift-ci
bot
removed
the
needs-rebase
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2616 +/- ##
==========================================
- Coverage 68.85% 68.81% -0.05%
==========================================
Files 139 139
Lines 9930 9930
==========================================
- Hits 6837 6833 -4
- Misses 2578 2580 +2
- Partials 515 517 +2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
openshift-ci
bot
removed
the
do-not-merge/work-in-progress
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
What changed
Certificateobjects into theinfrastructurephaseIssuerininfrastructureand leave workloadDeploymentobjects indeployWhy
Webhook installs can stall in
RollingOutwhen certificate issuance and deployment availability are gated within the same phase. Starting certificate issuance earlier shortens the rollout critical path for webhook-backed operators.Impact
This reduces the chance that
ClusterExtensioninstallations for webhook operators remain stuck waiting for the generated deployment to become available.Root cause
The rollout path allowed cert-manager
Certificateobjects to be applied in the same phase as the webhook deployment. For operators that mount the serving cert secret, that can delay deployment availability long enough to hit external rollout timeouts.Validation
go test -tags containers_image_openpgp ./internal/operator-controller/applier -run Test_PhaseSort