OCPBUGS-78095: Restore registry-server startup probe FailureThreshold to 15 - #3876
Conversation
openshift-ci
Bot
added
the
needs-ok-to-test
|
Hi @emmahone. Thanks for your PR. I'm waiting for a operator-framework member to verify that this patch is reasonable to test. If it is, they should reply with Regular contributors should join the org to skip this step. Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. 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. |
There was a problem hiding this comment.
Pull request overview
This pull request adjusts the generated CatalogSource registry-server Pod spec to restore the original startup probe time budget, improving tolerance for slower starts (e.g., large catalogs or constrained environments) in the registry reconciler.
Changes:
- Restore
StartupProbe.FailureThresholdfor the registry-server container from10to15(150s budget withPeriodSeconds: 10). - Update unit test fixtures to reflect the resulting
olm.pod-spec-hashlabel changes and the restored probe threshold.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| pkg/controller/registry/reconciler/reconciler.go | Restores StartupProbe.FailureThreshold to 15 for the registry-server Pod. |
| pkg/controller/registry/reconciler/reconciler_test.go | Updates expected startup probe threshold and recomputed olm.pod-spec-hash values in fixtures. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
/ok-to-test |
openshift-ci
Bot
added
ok-to-test
|
/test e2e-flakes |
|
@tmshort: No presubmit jobs available for operator-framework/operator-lifecycle-manager@master 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. |
|
/approve |
openshift-ci
Bot
added
the
approved
|
@emmahone is there a Red Hat Jira ticket associated with this change? |
|
/approve cancel |
openshift-ci
Bot
removed
the
approved
|
@tmshort This PR is associate with https://redhat.atlassian.net/browse/OCPBUGS-78095. Its already added in jira as a a web link. Do I need to do anything differently? |
tmshort
changed the title
|
@emmahone I put it as part of the title. |
|
@CodeRabbit review |
✅ Action performedReview finished.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThe registry pod startup probe failure threshold increases from 10 to 15. Reconciliation tests update probe assertions and expected pod-spec hashes across registry pod fixtures. ChangesRegistry startup probe threshold
Estimated code review effort: 1 (Trivial) | ~5 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.12.2)Error: can't load config: unsupported version of the configuration: "" See https://golangci-lint.run/docs/product/migration-guide for migration instructions Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: tmshort The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
Bot
added
the
approved
|
And... now this needs a rebase @emmahone |
Restores StartupProbe.FailureThreshold from 10 back to 15, returning the startup budget from 100s to 150s. This reverts the threshold portion of 401bfff. The 100s budget can be too tight when there are performance constraints or large custom CatalogSources (OCPBUGS-78095). The commit message in 401bfff states the intent was to maintain the ~150s startup time as originally designed, suggesting the reduction was unintentional. Test fixtures updated to reflect recomputed olm.pod-spec-hash values. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Evan Mahoney <emahoney@emahoney-thinkpadp1gen4i.rht.csb>
5efafc2 to
2c0c9d0
Compare
|
To be clear, this PR doesn't resolve the bug, but it does correct what looks like a mistake on the math of the original intent. In the cases where the probe failures are pathological, the probe duration is immaterial. |
Description of the change:
Restores `StartupProbe.FailureThreshold` for the registry-server container from `10` back to `15`, returning the startup budget from 100s to 150s. This reverts the threshold portion of 401bfff, which changed FailureThreshold from 15 to 10 while adding TimeoutSeconds.
Test fixtures updated to reflect the recomputed `olm.pod-spec-hash` values.
Motivation for the change:
The 100s budget (`FailureThreshold: 10 × PeriodSeconds: 10`) can be too tight when there are performance constraints or large custom CatalogSources (OCPBUGS-78095). The commit message in 401bfff states the intent was to "maintain the ~150s startup time failure as originally designed," suggesting the reduction from 15 to 10 was unintentional. This change restores `FailureThreshold` to `15` (150s budget), which provides sufficient margin for these scenarios without being excessive.
Architectural changes:
None.
Summary by CodeRabbit