From f8acdfdfe19bbca49382493feac99933190f31b9 Mon Sep 17 00:00:00 2001 From: MeepMr <67143880+MeepMr@users.noreply.github.com> Date: Wed, 2 Sep 2026 13:01:54 +0200 Subject: [PATCH 1/2] urlaubsverwaltung: readiness must see the database, health must not lie (9.0.0+up6.7.0) MANAGEMENT_ENDPOINT_HEALTH_GROUP_READINESS_INCLUDE=readinessState,db puts the datasource into Spring Boot's readiness group, which shipped with readinessState alone: a total database outage left the pod 1/1 Ready while every request failed. Naming the members replaces the default, so readinessState is repeated. The liveness group stays untouched on purpose, because an outage has to keep traffic away rather than add a crash loop to it. MANAGEMENT_HEALTH_MAIL_ENABLED=false removes the mail indicator, which pinned the aggregate /actuator/health at 503 for the whole life of a healthy release, because the chart's mail wiring points at localhost:1025 where nothing listens. values.yaml and NOTES.txt document what that costs and when it has to be switched back on. --- README.md | 2 + urlaubsverwaltung/Chart.yaml | 2 +- urlaubsverwaltung/templates/NOTES.txt | 120 ++++++++++++++---- .../urlaubsverwaltung.deployment.yaml | 26 ++++ urlaubsverwaltung/values.yaml | 33 +++++ 5 files changed, 155 insertions(+), 28 deletions(-) diff --git a/README.md b/README.md index 2b82990..f658db2 100644 --- a/README.md +++ b/README.md @@ -270,6 +270,8 @@ Reference implementation: `template-chart/values.schema.json` and `template-char **3. Do not hang liveness on an external dependency where a dependency-free path exists.** `zipline` deliberately probes `/` for liveness and `/api/healthcheck` only for readiness and startup, so a database outage cannot restart the pod. Where no such path exists — `outline` returns 500 on `/` when the database is down — all three probes may share the endpoint, but the reason belongs in a comment on the helper, or the next reader will "fix" the inconsistency. + The other half of the same rule: readiness **must** hang on the dependency, and a separate path is not proof that it does. `urlaubsverwaltung` probed `/actuator/health/liveness` and `/actuator/health/readiness` for two years and still stayed 1/1 Ready through a total database outage (#218), because Spring Boot's `readiness` group contains `readinessState` **alone** — the split paths were right, the group content was empty of anything external. Whatever aggregates behind a probe path (Spring health groups, a `/healthz` a framework assembles for you) has to be read off the **running** endpoint, not off the path name; the chart names the members explicitly (`MANAGEMENT_ENDPOINT_HEALTH_GROUP_READINESS_INCLUDE=readinessState,db`) so the composition is in the template rather than in an upstream default. + **4. A fallback reproduces *this chart's* defaults, not the repo standard.** Five charts deviate from the hardening baseline on purpose. A "cleaned up" standard default is not tidying, it is an outage: `calibre-web-automated` must keep `readOnlyRootFilesystem: false`, because in read-only mode the image disables its PUID/PGID remapping and the runtime uid silently moves from 1000 to 911 — **making existing volume data inaccessible**. The same applies to non-standard values that look like mistakes: `satisfactory` keeps `limitFactor: 2` (that is what holds the memory limit at the published 8Gi) and `fsGroupChangePolicy: OnRootMismatch` (which avoids a recursive chown of multi-GB game files on every start). **5. A probe default carries the probe's context, not just its timings.** Two different symptoms, both from real charts: `teamspeak-server` addresses its TCP port by the name `file-transfer` because the voice service is UDP and cannot be probed — a default without that name leaves the workload **with no working probe at all**. `patchmon` sends a `Host` header because the server answers **403** to any request whose Host is neither loopback nor a CORS_ORIGIN host — a default without it means the pod never becomes Ready. Port names, `Host` headers and `exec` commands belong in the fallback; where the context is dynamic, pass the root context in and rebuild it (patchmon derives the header from `ingress.domain`). diff --git a/urlaubsverwaltung/Chart.yaml b/urlaubsverwaltung/Chart.yaml index 0beab97..c05627d 100644 --- a/urlaubsverwaltung/Chart.yaml +++ b/urlaubsverwaltung/Chart.yaml @@ -4,5 +4,5 @@ description: A Helm chart to deploy urlaubsverwaltung on Kubernetes icon: https://raw.githubusercontent.com/urlaubsverwaltung/urlaubsverwaltung/main/src/main/resources/static/favicons/apple-touch-icon.png type: application -version: 8.1.0+up6.7.0 +version: 9.0.0+up6.7.0 appVersion: 6.7.0 diff --git a/urlaubsverwaltung/templates/NOTES.txt b/urlaubsverwaltung/templates/NOTES.txt index f88020c..8ec6286 100644 --- a/urlaubsverwaltung/templates/NOTES.txt +++ b/urlaubsverwaltung/templates/NOTES.txt @@ -199,33 +199,75 @@ THE MEMORY LIMIT SIZES THE JAVA HEAP — IT IS NOT JUST A CEILING kubectl -n {{ .Release.Namespace }} logs deployment/{{ .Release.Name }}-{{ .Chart.Name }}-deployment | head -1 -DO NOT HEALTH-CHECK THIS RELEASE ON /actuator/health - That endpoint aggregates every health indicator, and one of them is the mail - sender pointed at localhost:1025 by this chart (see below). Nothing listens - there, so the aggregate answers 503 DOWN on a completely healthy release, - for the lifetime of that release: +THE THREE HEALTH PATHS AND WHAT EACH ONE ANSWERS FOR + This chart sets two health settings in the deployment template, and they are + what makes the three paths mean different things: - {"components":{ ... "mail":{"status":"DOWN"} ... },"status":"DOWN"} + MANAGEMENT_ENDPOINT_HEALTH_GROUP_READINESS_INCLUDE=readinessState,db + MANAGEMENT_HEALTH_MAIL_ENABLED=false - Everything else in it — db, ping, diskSpace, ssl — is UP. The probes this - chart configures deliberately use the /actuator/health/liveness and - /actuator/health/readiness GROUPS instead, which contain only the - application's lifecycle state and answer 200. Any external monitor you point - at this release has to use the same two paths, or it will page you forever - about a mail server you never configured. + /actuator/health everything the release actually runs: db, + diskSpace, ping, ssl and the two lifecycle + states. 200 UP on a healthy release. The mail + indicator is NOT in it — see the mail section. + → point external monitoring here. This is the only path that also fails on + diskSpace or ssl. -WHAT A READY POD DOES NOT PROVE - Those two groups report lifecycle state and nothing else — the datasource is - not in them. Measured here: with the database scaled away underneath a - running pod, both groups kept answering 200, the pod stayed 1/1 Ready and was - never restarted. The database is verified exactly once, at boot, by the - migration. + /actuator/health/readiness readinessState AND db. Goes DOWN (503) on a + database outage, which takes the pod out of Ready + and stops traffic reaching it. + → the readinessProbe. - Know the resulting picture in advance: during a database outage the pod stays - Ready, its endpoint keeps receiving traffic{{ if .Values.ingress.enabled }}, the Ingress keeps routing to - it{{ end }}, and users get application errors while Kubernetes reports everything - healthy. Nothing in the pod's status or restart count points at the database. - Look there first anyway. + /actuator/health/liveness livenessState alone, deliberately WITHOUT db. + Stays 200 through a database outage. + → the livenessProbe and the startupProbe. + + The split is the point: a database outage has to keep traffic away, but + restarting the pod does not bring the database back. Adding db to the + liveness group would turn every outage into a crash loop on top of the + outage. Measured on this chart, with the database scaled to zero underneath a + running pod: readiness 503, liveness 200, pod 0/1 Ready, restart count 0. + + How readiness actually fails is worth knowing before you tune it. The db + indicator does not answer 503 quickly — it waits out the JDBC connection + timeout first, measured at 30 seconds against a database whose Service has no + endpoints at all. readinessProbe.timeoutSeconds is 5, so the kubelet gives up + long before the answer arrives and logs + + Readiness probe failed: ... context deadline exceeded + + rather than a 503. The pod leaves Ready either way, and faster; the 503 is + what a client with patience sees. Raising timeoutSeconds past the JDBC + timeout would swap the timeout for the 503 and delay the reaction — there is + no reason to. + +WHAT A READY POD DOES AND DOES NOT PROVE + Ready now means the pod can reach the database — that is checked on every + readiness period, not once at boot. It still does not mean the identity + provider is reachable: OIDC is resolved once at startup (see below) and no + health indicator watches it afterwards, so a provider that goes down after + the rollout leaves a Ready pod on which nobody can log in. + + A database outage now shows up in the pod's status: READY drops to 0/1 while + STATUS stays Running and RESTARTS stays put. That combination — running, not + restarting, not Ready — is the signature of a dependency outage rather than + of a broken release. The reason is in the endpoint, not in the log: + + kubectl -n {{ .Release.Namespace }} get pod -l app={{ .Release.Name }}-{{ .Chart.Name }}-deployment + kubectl -n {{ .Release.Namespace }} port-forward svc/{{ .Release.Name }}-{{ .Chart.Name }}-service 8080:80 + curl -s localhost:8080/actuator/health + + The body names components only when show-details is on; the chart does not + set it, so the aggregate answers {"status":"DOWN","groups":[...]} and the + status code carries the message. To see WHICH component is down, add + + urlaubsverwaltung.env: + - name: MANAGEMENT_ENDPOINT_HEALTH_SHOW_DETAILS + value: always + + and know what that publishes to anyone who can reach the endpoint: the + database product, the container's filesystem path and free bytes, the + certificate chains, and the stack-trace class of whatever failed. Note also that there is no shell in this image: "kubectl exec" cannot open one, and neither can a debug sidecar sharing the process namespace run the @@ -241,10 +283,34 @@ MAIL AND THE APPLICATION URL ARE PLACEHOLDERS UNTIL YOU SET THEM UV_MAIL_APPLICATIONURL = http://localhost:8080 Nothing in this pod listens on localhost:1025, so no notification mail leaves - the release and the mail health indicator stays DOWN. UV_MAIL_APPLICATIONURL - is what the app puts into the links inside those mails — it is NOT derived - from ingress.domain. Left as it is, every link points at the reader's own - machine. + the release. UV_MAIL_APPLICATIONURL is what the app puts into the links + inside those mails — it is NOT derived from ingress.domain. Left as it is, + every link points at the reader's own machine. + + BECAUSE that wiring is a placeholder, the chart sets + + MANAGEMENT_HEALTH_MAIL_ENABLED=false + + and the mail indicator is gone from /actuator/health. Otherwise the aggregate + would answer 503 DOWN on a perfectly healthy release for its whole lifetime, + and any monitor pointed at it would page forever about a mail server nobody + configured. + + THE PART THAT BITES LATER: the day you configure real mail delivery, switch + the indicator back on in the SAME change — + + urlaubsverwaltung.env: + - name: MANAGEMENT_HEALTH_MAIL_ENABLED + value: "true" + + Left at false it does not report a false alarm, it reports nothing: the + endpoint answers 200 while the mail server is unreachable and notifications + are silently not delivered. The setting is only honest while there is no mail + server to be honest about. + + Note that this variable repeats a name the chart already sets, so per the + rule below it can only be added at INSTALL time — putting it into an already + installed release makes the next upgrade fail in the API server. urlaubsverwaltung.env IS ADDITIVE ONLY, AND A COLLISION BREAKS THE UPGRADE The entries are appended AFTER the chart's own list{{ if $envNames }} (this release appends: diff --git a/urlaubsverwaltung/templates/urlaubsverwaltung.deployment.yaml b/urlaubsverwaltung/templates/urlaubsverwaltung.deployment.yaml index 0b7755b..f7c1bad 100644 --- a/urlaubsverwaltung/templates/urlaubsverwaltung.deployment.yaml +++ b/urlaubsverwaltung/templates/urlaubsverwaltung.deployment.yaml @@ -50,6 +50,32 @@ spec: secretKeyRef: name: {{ .Release.Name }}-{{ .Chart.Name }}-db-secret key: password + # Health indicators and probe groups + # + # Spring Boot creates the "liveness" and "readiness" groups by + # itself here (measured: /actuator/health answers + # {"groups":["liveness","readiness"]} with nothing configured), but + # the readiness group ships with readinessState ALONE. Without the + # line below a total database outage leaves /actuator/health/readiness + # at 200 and the pod 1/1 Ready while every request fails (#218). + # Naming the group's members REPLACES the default, so readinessState + # has to be repeated. "db" is the datasource indicator's real name, + # read off the running endpoint. + - name: MANAGEMENT_ENDPOINT_HEALTH_GROUP_READINESS_INCLUDE + value: "readinessState,db" + # The liveness group is deliberately NOT extended: a database + # outage has to keep traffic away, but restarting the pod does not + # bring the database back — it would only add a crash loop to the + # outage. + # + # The mail indicator is switched off because the mail wiring below + # points at localhost:1025, where nothing listens. Left on, it holds + # the aggregate /actuator/health at 503 DOWN for the whole life of a + # healthy release (#217). Anyone who later configures real mail + # delivery has to switch it back on via urlaubsverwaltung.env, or + # the endpoint stays blind to a broken mail server. + - name: MANAGEMENT_HEALTH_MAIL_ENABLED + value: "false" # Mail - name: UV_MAIL_FROM value: urlaubsverwaltung@example.org diff --git a/urlaubsverwaltung/values.yaml b/urlaubsverwaltung/values.yaml index a83750f..5933670 100644 --- a/urlaubsverwaltung/values.yaml +++ b/urlaubsverwaltung/values.yaml @@ -45,6 +45,39 @@ urlaubsverwaltung: capabilities: drop: - ALL + # HEALTH INDICATORS — TWO SETTINGS THE CHART FIXES IN THE TEMPLATE + # + # The chart sets these two as plain environment variables, not as values, + # because urlaubsverwaltung.env below already overrides anything: + # + # MANAGEMENT_ENDPOINT_HEALTH_GROUP_READINESS_INCLUDE=readinessState,db + # puts the datasource into the readiness group, so a database outage + # takes the pod out of Ready instead of leaving it serving errors (#218). + # The liveness group stays untouched on purpose — an outage must keep + # traffic away, not restart the pod in a loop. + # MANAGEMENT_HEALTH_MAIL_ENABLED=false + # removes the mail indicator, which otherwise pins the aggregate + # /actuator/health at 503 DOWN forever, because the chart's mail wiring + # points at localhost:1025 where nothing listens (#217). + # + # THE CONSEQUENCE TO KNOW BEFORE IT BITES: whoever later configures real mail + # delivery (SPRING_MAIL_HOST/SPRING_MAIL_PORT via urlaubsverwaltung.env) MUST + # switch the indicator back on in the same change: + # + # env: + # - name: MANAGEMENT_HEALTH_MAIL_ENABLED + # value: "true" + # + # Left at false, /actuator/health keeps answering 200 while the configured + # mail server is unreachable and no notification mail is delivered. The + # setting trades a permanently false alarm for silence, and silence is only + # correct as long as there is nothing to report about. + # + # That override repeats a name the chart already sets, and env is additive: + # it renders the variable twice (the later entry wins in the container), and + # adding it to an ALREADY INSTALLED release makes the next upgrade fail in + # the API server. See the "env IS ADDITIVE ONLY" section of NOTES.txt — the + # switch has to go in at install time, not at upgrade time. livenessProbe: httpGet: port: 8080 From f761767630a5c23983d630f165e42d12d6529bf5 Mon Sep 17 00:00:00 2001 From: MeepMr <67143880+MeepMr@users.noreply.github.com> Date: Wed, 2 Sep 2026 13:22:58 +0200 Subject: [PATCH 2/2] urlaubsverwaltung: correct the env-collision claim, answer the recovery question The chart told operators that adding a duplicate env name to an installed release makes the next upgrade fail in the API server, quoting "The order in patch list ... doesn't match $setElementOrder list". Measured against helm 3.19.0 and Kubernetes 1.35, it does not fail: the upgrade succeeds, helm prints "Release has been upgraded", and Kubernetes merges the env list by name, so the duplicate pair collapses to the chart's own entry and the operator's override is discarded without a word. At install time the same values behave differently - both entries reach the pod, the later one wins, and the API server says so with "hides previous definition of ..., which may be dropped when using apply". The advice that followed from the old claim survives - such a switch only takes at install time - but the stated reason was wrong in the dangerous direction. A reader who expects a loud failure treats a green upgrade as proof the override took effect, and green is exactly what a silently dropped override looks like. The text now names the real failure mode and tells the reader to read the value back off the Deployment. README carries the same correction, because every chart in this repo offers the same additive env block. Also records what the earlier measurement left open. With the database scaled back up the pod returns to Ready on its own after about five seconds, restart count still 0, so leaving the liveness group untouched costs no manual recovery. And the limit of that signal belongs next to it: the db indicator validates a connection, not the schema, so a database that came back empty still put the pod back to Ready with /actuator/health at 200. --- README.md | 1 + urlaubsverwaltung/templates/NOTES.txt | 52 ++++++++++++++++++++------- urlaubsverwaltung/values.yaml | 13 ++++--- 3 files changed, 48 insertions(+), 18 deletions(-) diff --git a/README.md b/README.md index f658db2..7f8e4f3 100644 --- a/README.md +++ b/README.md @@ -245,6 +245,7 @@ These guidelines are **binding for every current and future chart** in this repo - Every workload container offers an additive `env` value block, appended to the env entries the chart always sets. - Entries are rendered through `tpl` (so values may contain Helm templating) and support the `value`, `secretKeyRef` and `configMapKeyRef` forms (pattern: `template-chart/templates/xxx.deployment.yaml`). +- **Additive means additive: an entry repeating a name the chart already sets does not replace it, and what the pair does depends on when it arrives.** At install both entries reach the pod and the later one — the user's — wins. On an *existing* release the same entry is lost: Kubernetes merges `env` by name, the pair collapses to the chart's own value, and `helm upgrade` still reports success. Measured on `urlaubsverwaltung` (helm 3.19.0, Kubernetes 1.35): `helm get manifest` shows the variable twice while the live Deployment shows it once, carrying the chart's value. The failure mode is silence, so a chart whose `NOTES.txt` tells operators to override one of its own variables has to say that the override only takes at install time — and that a green upgrade proves nothing, the value belongs read back off the Deployment. ### 11. Scaling diff --git a/urlaubsverwaltung/templates/NOTES.txt b/urlaubsverwaltung/templates/NOTES.txt index 8ec6286..dab9013 100644 --- a/urlaubsverwaltung/templates/NOTES.txt +++ b/urlaubsverwaltung/templates/NOTES.txt @@ -226,7 +226,14 @@ THE THREE HEALTH PATHS AND WHAT EACH ONE ANSWERS FOR restarting the pod does not bring the database back. Adding db to the liveness group would turn every outage into a crash loop on top of the outage. Measured on this chart, with the database scaled to zero underneath a - running pod: readiness 503, liveness 200, pod 0/1 Ready, restart count 0. + running pod: readiness 503, liveness 200, pod 0/1 Ready, restart count 0 — + the pod left Ready about 20 seconds after the database went away. + + The other half of that measurement matters just as much: when the database + came back, the pod returned to Ready by itself within about five seconds, + still with restart count 0. Nothing had to be rolled, deleted or scaled by + hand. That is what leaving the liveness group alone buys — the outage is + ridden out and then simply over. How readiness actually fails is worth knowing before you tune it. The db indicator does not answer 503 quickly — it waits out the JDBC connection @@ -248,6 +255,13 @@ WHAT A READY POD DOES AND DOES NOT PROVE health indicator watches it afterwards, so a provider that goes down after the rollout leaves a Ready pod on which nobody can log in. + Ready also proves reachability, not correctness: the db indicator validates a + connection, it never looks at the schema. Measured here — a database that came + back EMPTY put the pod straight back to Ready with /actuator/health at 200, + while not one table existed in it. A restored-but-wrong database reads as + healthy, so a probe going green after a database incident is not on its own + evidence that the data came back with it. + A database outage now shows up in the pod's status: READY drops to 0/1 while STATUS stays Running and RESTARTS stays put. That combination — running, not restarting, not Ready — is the signature of a dependency outage rather than @@ -308,24 +322,36 @@ MAIL AND THE APPLICATION URL ARE PLACEHOLDERS UNTIL YOU SET THEM are silently not delivered. The setting is only honest while there is no mail server to be honest about. - Note that this variable repeats a name the chart already sets, so per the - rule below it can only be added at INSTALL time — putting it into an already - installed release makes the next upgrade fail in the API server. + Note that this variable repeats a name the chart already sets, so per the rule + below it only takes effect when it goes in at INSTALL time. Put into an + already installed release it does nothing at all, and says so nowhere: helm + reports a successful upgrade while the container keeps "false". -urlaubsverwaltung.env IS ADDITIVE ONLY, AND A COLLISION BREAKS THE UPGRADE +urlaubsverwaltung.env IS ADDITIVE ONLY, AND A COLLISION IS LOST ON UPGRADE The entries are appended AFTER the chart's own list{{ if $envNames }} (this release appends: {{ join ", " $envNames }}){{ end }}. An entry that repeats a name the chart already sets does not replace it — it renders the variable twice, and - that has a sharp edge: adding such a name to an ALREADY INSTALLED release - makes the next upgrade fail in the API server, before anything changes, with + what happens to that pair depends on WHEN it first appears: - UPGRADE FAILED: cannot patch "{{ .Release.Name }}-{{ .Chart.Name }}-deployment" ... - The order in patch list ... doesn't match $setElementOrder list + AT INSTALL both entries reach the pod and the later one — yours — wins in the + container. The API server accepts it with a warning: - because env entries are merged by name and a duplicate name breaks that - ordering. A fresh install with the same values goes through, so the failure - looks arbitrary until you know it. Treat the variables above as fixed for the - life of a release: to really change one, reinstall. + Warning: spec.template.spec.containers[0].env[N]: hides previous definition + of "NAME", which may be dropped when using apply + + AT UPGRADE that warning comes true. Kubernetes merges env by name, so the pair + collapses back to ONE entry and the surviving value is the CHART's, not yours. + Nothing fails: helm prints "Release has been upgraded", the revision counts up, + and the override is simply gone. Measured on this chart (helm 3.19.0, + Kubernetes 1.35): "helm get manifest" shows the variable twice while the live + Deployment shows it once, with the chart's value. + + So the trap is silence, not an error. Treat the variables above as fixed for + the life of a release: to really change one, reinstall. And never read a green + upgrade as proof that an override took effect — read it off the object: + + kubectl -n {{ .Release.Namespace }} get deployment {{ .Release.Name }}-{{ .Chart.Name }}-deployment \ + -o jsonpath='{.spec.template.spec.containers[0].env}' Anything sensitive belongs in valueFrom.secretKeyRef rather than a plain value: a value is stored in the pod spec and readable by anyone who can read diff --git a/urlaubsverwaltung/values.yaml b/urlaubsverwaltung/values.yaml index 5933670..0d22d95 100644 --- a/urlaubsverwaltung/values.yaml +++ b/urlaubsverwaltung/values.yaml @@ -73,11 +73,14 @@ urlaubsverwaltung: # setting trades a permanently false alarm for silence, and silence is only # correct as long as there is nothing to report about. # - # That override repeats a name the chart already sets, and env is additive: - # it renders the variable twice (the later entry wins in the container), and - # adding it to an ALREADY INSTALLED release makes the next upgrade fail in - # the API server. See the "env IS ADDITIVE ONLY" section of NOTES.txt — the - # switch has to go in at install time, not at upgrade time. + # That override repeats a name the chart already sets, and env is additive, so + # it renders the variable twice. At INSTALL time both entries reach the pod and + # the later one — yours — wins in the container. Added to an ALREADY INSTALLED + # release it is lost instead: Kubernetes merges env by name, the pair collapses + # to the chart's own entry, and helm still reports a successful upgrade. The + # override does not fail loudly, it fails silently. See the "env IS ADDITIVE + # ONLY" section of NOTES.txt — the switch has to go in at install time, and + # after any such change the result belongs verified on the live Deployment. livenessProbe: httpGet: port: 8080