Repository navigation
feat: default healthcheck port and readiness probe - #163
Open
cherylaurelia wants to merge 1 commit into
Open
cherylaurelia wants to merge 1 commit into
cherylaurelia wants to merge 1 commit into
Conversation
levkk
reviewed
Nov 9, 2026
| # healthcheckPort on which PgDog will expose its HTTP healthcheck endpoint. | ||
| # It returns 502 when every connection pool is down. Unless readinessProbe is | ||
| # set, the chart uses it as the readiness probe. Set to null to disable. | ||
| healthcheckPort: 8080 |
Contributor
There was a problem hiding this comment.
We shouldn't be enabling this by default.
levkk
reviewed
Nov 9, 2026
| exit 1 | ||
| fi | ||
|
|
||
| echo "" |
Contributor
There was a problem hiding this comment.
Could you remove this and add a yaml test into the test folder instead? We automatically validate config generation there using kubeconform
The chart rendered no readiness probe, so Kubernetes treated a PgDog pod whose pools are all down as ready, even with healthcheckPort set. When healthcheckPort is set, declare it as a container port and, unless the user has set readinessProbe, add an HTTP readiness probe on it. The endpoint only fails when every pool is down, so one unreachable database keeps the pod ready. Pools report healthy until their first connection attempt fails (up to connect_timeout), hence the 10s initial delay. healthcheckPort stays unset by default, so default renders are unchanged, and a user-set readinessProbe is unchanged. No liveness probe: restarting PgDog during a database outage would drop every client. test/values-healthcheck-port.yaml renders the probe through the kubeconform loop. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
cherylaurelia
force-pushed
the
fix/default-readiness-probe
branch
from
November 9, 2026 20:12
3eb07fc to
1a598ac
Compare
Author
|
thanks! made healthcheckPort opt-in (commented out by default, so no probe unless it's set) and moved the test to test/values-healthcheck-port.yaml so kubeconform picks it up |
This branch has not been deployed
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.
Closes #121, using the approach suggested there. Without a readiness probe, a bad database setting plus
kubectl rollout restartreplaces every pod with one whose pools are all down. On kind this took 2 s, and every query then failed withconnection pool ... is down. With this change andhealthcheckPortset, the first new pod fails readiness, the rollout stops, and the old pods keep serving.Who changes on upgrade: only installs that set
healthcheckPortand notreadinessProbe. They get ahealthcheckcontainer port and an HTTP readiness probe on it.healthcheckPortstays unset by default, so default renders are unchanged, and a user-setreadinessProbeis kept. I left the chart version for you to bump.It's readiness only, since a liveness probe would restart PgDog during a database outage. If Postgres is fully down, every pod goes NotReady, so clients connecting through the Service get connection refused instead of
pool is down. The 10 s initial delay is a heuristic: pools report healthy until their first connect fails, which takes up toconnect_timeout(5 s). A slower failure would still pass, and each pod takes about 10 s longer to become ready. The cleaner fix is in pgdog itself: pools start healthy (TargetHealth::new()), and starting them unknown until the first check would let this delay go.Testing
./test/test.shpasses.test/values-healthcheck-port.yamlrenders the probe through the kubeconform loop; an invalid probe field fails it.healthcheckPortset: a host typo (3 runs), an unreachable IP, and one of two databases down. The rollout stopped every time while healthy pods kept serving. After a 45 s Postgres outage, pods were Ready again about 20 s after it came back.configcheckpasses except forpgdog-config-extras, which fails the same way on main.Written with Claude Code, which also ran the tests and the kind runs above.
🤖 Generated with Claude Code