feat: make the probe timings configurable - #5712
Merged
Matthew Christopher (matthchr) merged 2 commits intoSep 21, 2026
Merged
Matthew Christopher (matthchr) merged 2 commits into
Matthew Christopher (matthchr) merged 2 commits into
Conversation
The probes have been retuned in place three times, each time to suit one cluster: - Azure#2254 added the liveness probe. - Azure#2844 raised initialDelaySeconds because the operator "was running into timeouts on my arm64 mac machine ... so operator gets more time to load and apply CRDs". - Azure#4791 found that delay now penalised fast startups - "ASO finish starting up after about 10s, then idle for an extra ~45s" - and replaced it with a startup probe at 10s x 12. That budget is now too tight at the other end of the range. The operator applies its CRDs and starts a controller per installed CRD before it serves /healthz, so on a cluster with a large number of CRDs the kubelet kills it mid-apply and it never finishes starting. Rather than move the constant a fourth time, this exposes the timings as Helm values. The defaults are what the chart applies today, so the only change to the rendered output is that the Kubernetes defaults for the liveness and readiness probes are now written out explicitly. The probe paths and ports stay fixed; they have never been the problem.
andreidorin-oprea
requested review from
Matthew Christopher (matthchr),
Alex Leites (tallaxes) and
Bevan Arps (theunrepentantgeek)
as code owners
September 18, 2026 07:51
Contributor
There was a problem hiding this comment.
Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.
Note
This error may be related to your runner configuration. You can now configure runners for Copilot code review separately from Copilot cloud agent by creating a copilot-code-review.yml file with your setup steps. Read the docs for details.
Matthew Christopher (matthchr)
approved these changes
Sep 18, 2026
Matthew Christopher (matthchr)
left a comment
Member
There was a problem hiding this comment.
LGTM, small doc nit.
Bevan Arps (theunrepentantgeek)
left a comment
Member
There was a problem hiding this comment.
Looks good; will be happy to merge once the docs are tweaked.
Applies the review suggestion on the probes block: drops the sentence about the defaults matching what the chart already applied, which belongs in the PR description rather than the values file. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
Author
|
Bevan Arps (@theunrepentantgeek) docs tweaked in ea7e840 - Matthew Christopher (@matthchr)'s suggestion applied verbatim, comment-only change. Ready for merge. |
Bevan Arps (theunrepentantgeek)
approved these changes
Sep 21, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this PR does
Exposes the liveness, readiness and startup probe timings as Helm values. The defaults are exactly what the chart applies today, so this is a no-op unless you set them.
Paths and ports stay fixed - they have never been the thing people needed to change.
Why
The probes have been retuned in place three times, each time to suit whichever cluster the reporter had:
initialDelaySeconds10s x 12#2844 made startup too slow for small clusters, and #4791 made it too tight for large ones. We are at the far end of that range: the operator applies its CRDs and starts a controller per installed CRD before it serves
/healthz, and on our production cluster - roughly 800 CRDs - that outlasts the 120s budget, so the kubelet kills it mid-apply and it never finishes starting.Our only workaround today is a Helm post-renderer that rewrites the probe after the chart has rendered, which breaks silently whenever the chart's deployment changes shape.
Rather than move the constant a fourth time and push the problem back to the small-cluster end, this makes it a knob. Raising
failureThresholdcosts nothing on a fast startup, because a startup probe stops as soon as it first succeeds.Special notes
v2/config/manager/manager.yamlgets the same three fields on each probe socontroller:validate-helmkeeps matching the chart against the kustomize output.periodSeconds: 10,failureThreshold: 3,timeoutSeconds: 1) are now written out explicitly instead of being implied. Behaviour is unchanged.successThresholdout. Kubernetes requires it to be 1 for liveness and startup probes, so exposing it would mostly be a way to write an invalid config.diagnosing-problems/_index.md; both add a section under Common mistakes. Whichever lands second is a trivial rebase.How does this PR make you feel?
Checklist