helm: configurable liveness, readiness and startup probes + named container ports - #2172
helm: configurable liveness, readiness and startup probes + named container ports#2172DrFaust92 wants to merge 2 commits into
Conversation
Runner and forwarder probe blocks are rendered only when set, so the default install is byte-identical to before. Liveness is documented on /metrics rather than /healthz. /healthz reflects sink health, so a transient sink outage on a liveness probe restarts the runner while nothing is wrong with it - the reason the 2023 probes were reverted in 2b2292c. /healthz is documented as a readiness option instead. Closes robusta-dev#1158
ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (1)
Included review availability: Your plan provides up to 8 included reviews per hour; 6 remain after this review. WalkthroughThe Helm chart adds optional startup, liveness, and readiness probes for the runner and forwarder. The templates expose named container ports. Setup documentation describes endpoints, configuration examples, and endpoint selection. ChangesHealth probe configuration
Priority: ⬇️ Low — Defer this Helm chart change because it narrowly adds optional health probes, named ports, and setup documentation without supplied evidence of elevated customer or external urgency. Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: 🔵 Low · up to This change adds optional health probes and named container ports without altering the default installation. The forwarder documentation may mislead ServiceMonitor users into selecting a container port name instead of the Service port, potentially interrupting metrics scraping until corrected. Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Declares http/5000 on the runner and metrics/2112 on the forwarder so probes can reference the port by name, and documents why that is preferable. Service targetPort stays numeric on purpose. A named targetPort resolves per pod, so upgrading from a chart version whose pods had no named port leaves the old pod Ready with its endpoint published with no port at all - verified on kind, the EndpointSlice comes back with ports: null - and the Service black-holes traffic until the rollout completes.
DrFaust92
changed the title
1 participant
What
Adds
livenessProbe,readinessProbeandstartupProbevalues for the runner and the forwarder, rendered only when set, and names the container ports so probes can reference them by name (http/5000 on the runner,metrics/2112 on the forwarder).New docs page at
docs/setup-robusta/health-probes.rstcovering which endpoints each component actually serves, which to point probes at, and why to reference ports by name.The only change to a default render is the two
ports:blocks, so existing installs get one pod restart on upgrade and no other behavior change.Why
Follows the endpoint/history writeup and proposal in #1158, which @adamantal 👍'd.
/healthzreflects sink health, so a liveness probe on it restarts the runner whenever a sink flaps — nothing is wrong with the runner. That is what the 2023 probes did (add liveness probe to the runner (check discovery health) #825, add startup probe for long boot time #845) before2b2292c5reverted them, and chore(helm): define pod probes #1960 stalled after re-adding pod probes. Liveness on/metricsrestarts only a genuinely wedged process;/healthzis documented as a readiness option for anyone who wants the sink-aware signal./healthz— only/metricson:2112— so that is what the docs point at.port: httpinstead of repeating5000, survives a listen-port change, and resolves in NetworkPolicy and Prometheus Operator configs too. A name that no container declares also fails with an explicitport "..." not foundrather than a generic connection refused.Service
targetPortdeliberately stays numeric. A namedtargetPortresolves per pod, so upgrading from a chart version whose pods had no named port leaves the old podReadywith its endpoint published with no port at all — I checked on kind and the EndpointSlice comes backports: null— and the Service black-holes traffic until the rollout finishes. Numeric resolves either way. It can move to the name in a later version once no supported upgrade path starts from unnamed pods.Note on the original report:
automountServiceAccountToken: falseno longer produces a running-but-unready pod. On 0.49.0 the runner exits on its first API call and the forwarder exitsfatalon the missing kubeconfig, so both land inCrashLoopBackOff— Kubernetes already reports that. Probes here are for wedged processes, slow starts, and rollout/Service gating; the docs say so rather than implying they detect misconfiguration.Testing
Installed on kind twice — once with numeric probe ports, once with all probes referencing the names (
port: http,port: metrics). Both pods reached1/1 Readywith 0 restarts, and EndpointSlices kept real ports. Confirmedhelm templateis byte-identical to master apart from the twoports:blocks, and rendered clean withrunner.hardenedFs=trueandgrafanaRenderer.enableContainer=true.