feat(chart): add startupProbe support to the pms container - #183
Open
alphayax wants to merge 2 commits into
Open
feat(chart): add startupProbe support to the pms container#183alphayax wants to merge 2 commits into
alphayax wants to merge 2 commits into
Conversation
The chart supports livenessProbe and readinessProbe (plexinc#117) but not startupProbe, which forces a bad trade-off for anyone using probes. PMS answers 503 on /identity in two very different situations: 1. while it runs database migrations at startup — transient, can take many minutes on a large library, and restarting makes it worse because the migration starts over; 2. when the server is wedged and will never recover on its own. Only a liveness probe catches (2), because the process stays alive. But with no startupProbe available, the liveness budget has to be stretched to cover the worst case of (1), or it turns a slow migration into a restart loop — the exact risk raised in the review of plexinc#117: "if it's a particularly long db migration, the restart will just result in it being run again from the beginning and will indefinitely until the migration is allowed enough time to complete." startupProbe is the primitive that separates the two: it holds the liveness probe off until the container answers once, then hands over. With it, the same deployment can tolerate a long migration and still keep a tight liveness probe afterwards. Changes, with no behaviour change unless the new value is set: - templates/statefulset.yaml: one {{- with }} block mirroring the existing two, placed before livenessProbe. - values.yaml: startupProbe: {} with a commented-out example, in the same style as its neighbours. - README.md: helm-docs row. - Chart.yaml: 1.7.1 -> 1.8.0, matching how plexinc#168, plexinc#170 and plexinc#175 each bumped the minor version for an additive feature. Verified: `helm template` with no values renders byte-identically to before apart from the helm.sh/chart label; with startupProbe and livenessProbe both set, both render correctly nested and readinessProbe stays absent; the rendered manifest passes kubectl apply --dry-run=server against a live cluster. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
alphayax
requested review from
MarshallAsch and
cookandy
and removed request for
a team
September 2, 2026 02:50
cilindrox
approved these changes
Sep 2, 2026
Co-authored-by: Gaston Festari <cilindrox@gmail.com>
Member
|
@alphayax linter's failing - seems to be missing a |
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.
Why
The chart supports
livenessProbeandreadinessProbe(added in #117) but notstartupProbe. That gap forces a bad trade-off, and the reason is already written down in #117's own review discussion:That is exactly right, and it is the problem. PMS answers 503 on
/identityin two completely different situations:A liveness probe is the only thing that catches (2), because the process never exits. But with only a liveness probe available, its budget has to be stretched to cover the worst case of (1) — otherwise it turns a slow migration into a restart loop. So users end up either with no probe at all, or with a probe deliberately detuned to the point of being slow to help.
startupProbeis the Kubernetes primitive that separates the two: it suspends the liveness probe until the container answers once, then hands over. With it, the same deployment can tolerate a long migration and keep a tight liveness probe afterwards.I hit this in production: PMS deadlocked on its nightly database backup and served 503 to every request for over 13 hours while the container stayed
Running. Adding a liveness probe fixes the outage, but without astartupProbeI had to widen it to ~32 minutes to stay safe across version upgrades, which gives up most of the benefit.What this changes
Four files, and no behaviour change for anyone who does not set the new value:
templates/statefulset.yaml— one{{- with }}block, mirroring the existing two exactly, placed beforelivenessProbe.values.yaml—startupProbe: {}with a commented-out working example, in the same style as the neighbouring two.README.md— helm-docs row, alphabetical position.Chart.yaml—1.7.1→1.8.0, matching how feat: add support for Service.spec.trafficDistribution #168, feat: add HTTPRoute label support #170 and feat(helm): add podSecurityContext #175 each bumped the minor version for an additive feature.Verification
helm templatewith no values: output is byte-identical to the unpatched chart apart from thehelm.sh/chartversion label. No probe key is injected by default.helm templatewithpms.startupProbeandpms.livenessProbeboth set: both render, correctly nested under the container,readinessProbecorrectly absent.kubectl apply --dry-run=serveragainst a live cluster.Note on the example values
The commented example uses
periodSeconds: 10/failureThreshold: 180, i.e. a 30-minute startup budget. That is deliberately generous: the cost of an over-long startup probe is a slow first start, while the cost of one that is too short is a migration restarted from scratch, repeatedly. Happy to change the numbers if you would rather the example be less conservative.