orca-chart: Add container probes - #120
Open
AlveElde wants to merge 4 commits into
Open
Conversation
The container port name was only "http" when exactly one HTTP listener was configured, otherwise every listener got an "http-<port>" name. Both Services hardcode "targetPort: http" and "targetPort: https", so any multi-listener config rendered a Service aimed at a port name no container declared. Name the first listener of each scheme "http" and "https", and suffix only the ones after it. Single-listener output is unchanged. This also makes "port: http" safe as a probe default.
_pod.tpl had no probe block, so the livenessProbe and readinessProbe keys in values.yaml were dead: settable, documented, and rendered nowhere. Add the three `with` blocks, and a startup probe to own the slow part of boot. Kubernetes suspends liveness and readiness for as long as a startup probe is failing, so a cold start cannot trip a restart and the pod stays out of the Service until it can serve.
The readiness probe asked /healthz, inherited from the chart scaffold rather than chosen. Today the two endpoints are the same static synth(200) in main.vcl, so this changes no behavior. It matters for what comes next: /healthz is to report that the process is up, and /readyz that the pod can actually serve traffic. Pairing each probe with the endpoint it means now avoids a silent behavior change in the chart when the two are pulled apart. Startup stays on /readyz, which also means a pod that never finishes booting is eventually restarted rather than sitting live but useless.
There was a problem hiding this comment.
Pull request overview
This PR wires container health probes into the Orca Helm chart’s pod template, updates listener port naming so http/https always exist as stable named ports, and documents the new probe behavior.
Changes:
- Render
startupProbe,livenessProbe, andreadinessProbefromvalues.yamlinto the container spec. - Adjust multi-listener port naming so the first port of each scheme is named
http/httpsand only additional ports are suffixed. - Update unit tests and README docs to reflect probe defaults and the new port naming behavior.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| orca-chart/values.yaml | Adds default startup probe and changes readiness endpoint to /readyz, plus extended probe documentation comments. |
| orca-chart/templates/_pod.tpl | Implements stable port naming for multi-listener configs and renders probes into the pod spec via with blocks. |
| orca-chart/test/unit_common/common.bats | Updates/extends unit tests for port naming and for probe rendering/overrides. |
| orca-chart/README.md | Updates values table entries for probes and adds a new “Startup time and probes” section. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Member
Author
|
Bump |
Collaborator
|
looks good to me, the big question is: do you need so much configurability? Would a |
Member
Author
|
Orca can be a bit slow to come up, VCL compilation, MSE4 init, and Firewall ruleset fetching can all take a good chunk of time depending on your config and environment. So I'd prefer having some configurables that let you extend the thresholds if you start bumping into them |
Collaborator
|
That makes sense |
gquintard
approved these changes
Aug 20, 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.
_pod.tplhad no probe block, so thelivenessProbeandreadinessProbekeys invalues.yamlwere unused. This wires them up and adds a startup probe.Port names
The container port was only named
httpwhen exactly one HTTP listener was configured, otherwise every listener got anhttp-<port>name. The first listener of each scheme now keeps the bare name and only the ones after it are suffixed.Startup probe
Kubernetes suspends the liveness and readiness probes for as long as a startup probe is failing, so a slow first boot cannot trip a restart and the pod stays out of the Service until it can serve.
Endpoints
Liveness asks
/healthz, readiness and startup ask/readyz. Today these are the same staticsynth(200)inmain.vcl, but the plan is to split them so/healthzanswers from process start and/readyzonly when the pod can actually serve.Upgrade notes
livenessProbeorreadinessProbein their own values has had it silently ignored until now. Those settings start applying, including any that were wrong.http-<port>tohttp. That is a pod spec change, so it triggers a rollout on upgrade, and their Service starts resolving.