feat: give the remaining unprobed workloads a health check #704

Merged
k.renner merged 1 commits from add-missing-probes into master 2026-08-28 11:26:21 +00:00
Member

Fortsetzung von #703. Ein Container ganz ohne Probe ist das Loch in der Arbeitsteilung: er bleibt Ready, während er tot ist, und niemand weiter unten kann es merken.

Nicht alle haben eine Probe bekommen — und die Ausnahmen sind die interessantere Hälfte.

Hinzugefügt

Workload Probe Warum genau die
whisper, openwakeword, speech-to-phrase tcpSocket auf den Wyoming-Port Wyoming ist rohes TCP ohne HTTP-Oberfläche und ohne Health-Verb. Eine Verbindung anzunehmen ist alles, was von außen behauptbar ist — schwach, aber vorher war es nichts, und ein toter Server bekam weiter Sprache geroutet.
ftp-scanner tcpSocket auf den FTP-Kontrollport Der Ausfall, der sich am besten versteckt: stirbt vsftpd, kann der Scanner nichts ablegen, und Paperless sieht ein leeres consume-Verzeichnis — ununterscheidbar davon, dass niemand gescannt hat.
code-server httpGet /healthz Antwortet 200, solange der Server läuft. Der Body meldet Session-Heartbeat, was nicht Gesundheit ist — ein unbenutzter Editor meldet expired und ist völlig in Ordnung.
external-secrets Chart-Werte livenessProbe/readinessProbe Das ist zugleich, was den Health-Server überhaupt startet — ohne die Werte exponiert der Controller nur seinen Metrics-Port. Ein festgefahrener Reconciler blieb Ready, während jedes ExternalSecret still aufhörte zu aktualisieren.

speech-to-phrase bekommt eine startupProbe statt einer langen Liveness-Verzögerung: es läuft mit --retrain-on-start, und das Nachtrainieren dauert, so lange es dauert. Das Tor erlaubt 10 Minuten und übergibt erst dann an Liveness — ein langsames Retraining wird nicht als Hänger missverstanden, ein echter Hänger danach aber binnen einer Minute erwischt.

Bewusst nicht angefasst

protonmail-bridge trägt bereits eine Begründung: die Bridge darf offline sein, während sich der Nutzer neu authentifiziert, und Paperless wiederholt den Mail-Abruf selbst. Eine Liveness-Probe würde sie mitten in der Re-Authentifizierung neu starten. Das ist eine durchdachte Entscheidung, kein Versäumnis — ich hätte sie beinahe überschrieben.

bridge-tls-proxy bindet socat auf 127.0.0.1:11143. Probes wählt das kubelet gegen die Pod-IP — dort ist nichts zu erreichen. Keine Probe möglich, nicht nur unbequem.

mariadb-operator — sein Chart bietet keine Probe-Werte und keine Health-Bind-Adresse (upstream values.yaml geprüft). Das über einen postRenderer zu erzwingen bräche beim nächsten Chart-Bump. Bleibt ungeprobed, und der Commit hält fest warum.

Die elf ak-outpost-* — Pod-Specs gehören dem goauthentik.io-Operator, von hier aus nicht ergänzbar.

Geprüft

Alle sechs Dateien mit Flux' Field-Manager gegen den Cluster dry-run-appliziert: serverside-applied, sauber.

Nach dem Merge

Die Rollouts ersetzen Pods in home-assistant, paperless, den drei Sprachdiensten und external-secrets. Ich prüfe für jeden, dass er Ready wird und 0 Restarts hat — ein Restart-Zähler über null hieße, dass eine Liveness-Probe zu streng ist, und das wäre schlimmer als gar keine Probe. Bei external-secrets prüfe ich zusätzlich, dass der Health-Server wirklich hochkommt, statt dem Chart zu glauben.

Fortsetzung von #703. Ein Container ganz ohne Probe ist das Loch in der Arbeitsteilung: er bleibt `Ready`, während er tot ist, und niemand weiter unten kann es merken. **Nicht alle haben eine Probe bekommen — und die Ausnahmen sind die interessantere Hälfte.** ## Hinzugefügt | Workload | Probe | Warum genau die | |---|---|---| | `whisper`, `openwakeword`, `speech-to-phrase` | `tcpSocket` auf den Wyoming-Port | Wyoming ist rohes TCP ohne HTTP-Oberfläche und ohne Health-Verb. Eine Verbindung anzunehmen ist alles, was von außen behauptbar ist — schwach, aber vorher war es *nichts*, und ein toter Server bekam weiter Sprache geroutet. | | `ftp-scanner` | `tcpSocket` auf den FTP-Kontrollport | Der Ausfall, der sich am besten versteckt: stirbt vsftpd, kann der Scanner nichts ablegen, und Paperless sieht ein leeres `consume`-Verzeichnis — ununterscheidbar davon, dass niemand gescannt hat. | | `code-server` | `httpGet /healthz` | Antwortet 200, solange der Server läuft. Der Body meldet Session-Heartbeat, was **nicht** Gesundheit ist — ein unbenutzter Editor meldet `expired` und ist völlig in Ordnung. | | `external-secrets` | Chart-Werte `livenessProbe`/`readinessProbe` | Das ist zugleich, was den Health-Server überhaupt startet — ohne die Werte exponiert der Controller nur seinen Metrics-Port. Ein festgefahrener Reconciler blieb `Ready`, während jedes ExternalSecret still aufhörte zu aktualisieren. | `speech-to-phrase` bekommt eine **`startupProbe`** statt einer langen Liveness-Verzögerung: es läuft mit `--retrain-on-start`, und das Nachtrainieren dauert, so lange es dauert. Das Tor erlaubt 10 Minuten und übergibt erst dann an Liveness — ein langsames Retraining wird nicht als Hänger missverstanden, ein echter Hänger danach aber binnen einer Minute erwischt. ## Bewusst nicht angefasst **`protonmail-bridge`** trägt bereits eine Begründung: die Bridge darf offline sein, während sich der Nutzer neu authentifiziert, und Paperless wiederholt den Mail-Abruf selbst. Eine Liveness-Probe würde sie **mitten in der Re-Authentifizierung neu starten**. Das ist eine durchdachte Entscheidung, kein Versäumnis — ich hätte sie beinahe überschrieben. **`bridge-tls-proxy`** bindet socat auf `127.0.0.1:11143`. Probes wählt das kubelet gegen die **Pod-IP** — dort ist nichts zu erreichen. Keine Probe möglich, nicht nur unbequem. **`mariadb-operator`** — sein Chart bietet **keine** Probe-Werte und keine Health-Bind-Adresse (upstream `values.yaml` geprüft). Das über einen postRenderer zu erzwingen bräche beim nächsten Chart-Bump. Bleibt ungeprobed, und der Commit hält fest warum. **Die elf `ak-outpost-*`** — Pod-Specs gehören dem goauthentik.io-Operator, von hier aus nicht ergänzbar. ## Geprüft Alle sechs Dateien mit **Flux' Field-Manager** gegen den Cluster dry-run-appliziert: `serverside-applied`, sauber. ## Nach dem Merge Die Rollouts ersetzen Pods in `home-assistant`, `paperless`, den drei Sprachdiensten und `external-secrets`. Ich prüfe für jeden, dass er `Ready` wird **und 0 Restarts** hat — ein Restart-Zähler über null hieße, dass eine Liveness-Probe zu streng ist, und das wäre schlimmer als gar keine Probe. Bei `external-secrets` prüfe ich zusätzlich, dass der Health-Server wirklich hochkommt, statt dem Chart zu glauben.
claude-bot added 1 commit 2026-08-28 11:24:37 +00:00
feat: give the remaining unprobed workloads a health check
ci/woodpecker/pr/ci-manifests-test Pipeline was successful
ci/woodpecker/pr/ci-scripts-test Pipeline was successful
ci/woodpecker/pr/pr-review Pipeline was successful
469cdbc2ee
Continues the paperless change. Under the split where gatus watches from
outside and Kubernetes watches the pods, a container with no probe at all is
the hole: it stays Ready while dead, and nothing downstream can tell.

Not every one of them got a probe, and the ones that did not are the more
interesting half.

Added:

  whisper, openwakeword       tcpSocket on the wyoming port. These speak a raw
  speech-to-phrase            TCP protocol with no HTTP surface and no health
                              verb, so accepting a connection is all that can
                              be asserted from outside the process. Weak, but
                              the previous state was nothing at all, and a dead
                              server kept being routed speech.

                              speech-to-phrase gets a startupProbe rather than
                              a long liveness delay: it runs --retrain-on-start
                              and retraining takes as long as it takes. The
                              gate allows 10 minutes and only then hands over,
                              so a slow retrain is not mistaken for a hang
                              while a real hang is still caught in a minute.

  ftp-scanner                 tcpSocket on the FTP control port. This is the
                              failure that hides best: if vsftpd dies, the
                              scanner cannot deposit anything and paperless
                              just sees an empty consume directory, which looks
                              exactly like nobody having scanned.

  code-server                 httpGet /healthz. Answers 200 whenever the server
                              is up; its body reports session heartbeat, which
                              is not health — an idle editor reports "expired"
                              and is fine.

  external-secrets            livenessProbe/readinessProbe enabled through the
                              chart's own values, which is also what starts its
                              health server: without them the controller
                              exposes nothing but a metrics port. A wedged
                              reconciler stayed Ready while every ExternalSecret
                              silently stopped refreshing.

Deliberately left alone:

  protonmail-bridge           Already carries a rationale: the bridge can be
                              offline while the user re-authenticates, and
                              paperless retries mail fetches itself. A liveness
                              probe would restart it mid-re-auth. That is a
                              considered decision, not an omission.

  bridge-tls-proxy            socat binds 127.0.0.1:11143. Probes are dialled
                              by the kubelet against the pod IP, so there is
                              nothing for it to reach.

  mariadb-operator            Its chart exposes no probe values and no health
                              bind address. Forcing them through a postRenderer
                              would break on the next chart bump, so it stays
                              unprobed and this records why.

  ak-outpost-* (11)           Pod specs are owned by the goauthentik.io
                              operator; nothing here can add to them.

Validated with Flux's field manager against the live cluster.
k.renner merged commit 7a278da1df into master 2026-08-28 11:26:21 +00:00
review-bot left a comment
Member

🤖 Automated review — model claude-sonnet-5 (complexity: normal), commit 469cdbc2eea28261f9fc17c5b2d1e048068e7be9.

Advisory review by the review-bot pipeline. It never approves; a human merges. Push new commits to get a fresh review.


Summary

Adds livenessProbe/readinessProbe (or startupProbe where warranted) to five previously-unprobed workloads (whisper, openwakeword, speech-to-phrase, ftp-scanner, code-server) plus enables the external-secrets chart's built-in probes. The reasoning in the PR body is careful and each choice (tcpSocket for Wyoming/FTP, httpGet for code-server, startupProbe for the retrain-on-start case) is sound in isolation. One change has a real blast-radius bug that the PR's own framing (avoid making things less available) argues against.

🔴 Blocking

  • kubernetes/apps/home-assistant/deployment.yaml:228-234 — the new readinessProbe on the code-server sidecar couples that container's health to the whole Pod's Ready condition. This Deployment is replicas: 1 and three Services select the same Pod purely by app.kubernetes.io/name: home-assistant label: service.yaml (the proxy path, used by the ingress), bgp-service.yaml (home-assistant-bgp, the direct LAN/BGP entry at ha.renner.uno), and vscode-service.yaml. Kubernetes computes Service endpoints from the Pod's aggregate Ready condition — the AND of every container's readiness — not per-container. If /healthz on code-server hangs, errors, or the container is simply slow/OOMing, the whole Pod flips NotReady and is pulled from all three Services, taking down Home Assistant itself (both the proxied and BGP-direct paths) because of a failure in an unrelated browser editor sidecar. That is a strictly worse outcome than the status quo this PR is trying to fix elsewhere (a dead workload silently staying Ready) — here a live HA becomes silently unready due to code-server. The livenessProbe addition is fine (it only restarts the code-server container, no cross-container effect); it's specifically the readinessProbe that should either be dropped, or paired with readinessGates/a separate Service story if per-container isolation is wanted. Contrast with the pre-existing proxy container's readinessProbe (line 181-184 pre-change), which is correctly coupled to Pod readiness because proxy is the traffic path to HA — code-server is not.

🟡 Suggestions

  • kubernetes/infrastructure/external-secrets/helmrelease.yaml:179-182 — I could not verify against the actual external-secrets chart source (no network access from this review sandbox) that livenessProbe.enabled / readinessProbe.enabled are the correct value paths for the pinned chart range (>=2.7.0 <3.0.0), as opposed to the probes being unconditionally rendered (with the health server just not started by default) or gated by a differently-named key. The PR body's "dry-run against the cluster, serverside-applied, sauber" check doesn't actually settle this: a dry-run of the HelmRelease object only validates the CR against the Flux CRD schema (values is a free-form object there), not that the referenced Helm chart accepts or acts on these keys — that only happens on actual reconciliation/template render by helm-controller. Worth confirming with helm show values external-secrets/external-secrets --version <resolved-version> (or checking the post-merge rollout, which the PR already plans to do) before treating this as settled.
  • kubernetes/apps/paperless/ftp-scanner.yaml:78-89 — minor: readinessProbe here removes the Pod from the ftp-scanner LoadBalancer's endpoints on any transient TCP hiccup (the Service uses externalTrafficPolicy: Local). Given vsftpd's passive-port dance already depends on control-port continuity, a flapping readiness probe mid-transfer would drop in-flight FTP sessions the same way a real outage would. Probably an acceptable trade given the periodSeconds/failureThreshold defaults (3 failures needed), just worth being aware of it as a slightly sharper failure mode than "silently stale".

🟢 Nits

None beyond the above.

Tests

This is a GitOps manifest repo; there's no unit-test harness for probe correctness, and the PR body describes an appropriate manual verification plan post-merge (confirm Ready + 0 Restarts per rollout, and specifically check the external-secrets health server actually binds rather than trusting the chart). That's the right kind of check for this change and I have no additional test to ask for beyond fixing the blocking issue above and re-verifying home-assistant's Pod stays Ready under a simulated code-server hang (or dropping the readinessProbe entirely).

Recommendation

Request changes — 1 blocking issue.

🤖 **Automated review** — model `claude-sonnet-5` (complexity: normal), commit `469cdbc2eea28261f9fc17c5b2d1e048068e7be9`. > Advisory review by the `review-bot` pipeline. It never approves; a human merges. Push new commits to get a fresh review. --- ## Summary Adds `livenessProbe`/`readinessProbe` (or `startupProbe` where warranted) to five previously-unprobed workloads (`whisper`, `openwakeword`, `speech-to-phrase`, `ftp-scanner`, `code-server`) plus enables the external-secrets chart's built-in probes. The reasoning in the PR body is careful and each choice (tcpSocket for Wyoming/FTP, httpGet for code-server, startupProbe for the retrain-on-start case) is sound in isolation. One change has a real blast-radius bug that the PR's own framing (avoid making things *less* available) argues against. ## 🔴 Blocking - `kubernetes/apps/home-assistant/deployment.yaml:228-234` — the new `readinessProbe` on the `code-server` sidecar couples that container's health to the whole Pod's `Ready` condition. This Deployment is `replicas: 1` and three Services select the same Pod purely by `app.kubernetes.io/name: home-assistant` label: `service.yaml` (the `proxy` path, used by the ingress), `bgp-service.yaml` (`home-assistant-bgp`, the direct LAN/BGP entry at `ha.renner.uno`), and `vscode-service.yaml`. Kubernetes computes Service endpoints from the Pod's aggregate `Ready` condition — the AND of every container's readiness — not per-container. If `/healthz` on code-server hangs, errors, or the container is simply slow/OOMing, the whole Pod flips NotReady and is pulled from **all three** Services, taking down Home Assistant itself (both the proxied and BGP-direct paths) because of a failure in an unrelated browser editor sidecar. That is a strictly worse outcome than the status quo this PR is trying to fix elsewhere (a dead workload silently staying `Ready`) — here a live HA becomes silently *unready* due to code-server. The `livenessProbe` addition is fine (it only restarts the `code-server` container, no cross-container effect); it's specifically the `readinessProbe` that should either be dropped, or paired with `readinessGates`/a separate Service story if per-container isolation is wanted. Contrast with the pre-existing `proxy` container's `readinessProbe` (line 181-184 pre-change), which is correctly coupled to Pod readiness because `proxy` *is* the traffic path to HA — `code-server` is not. ## 🟡 Suggestions - `kubernetes/infrastructure/external-secrets/helmrelease.yaml:179-182` — I could not verify against the actual `external-secrets` chart source (no network access from this review sandbox) that `livenessProbe.enabled` / `readinessProbe.enabled` are the correct value paths for the pinned chart range (`>=2.7.0 <3.0.0`), as opposed to the probes being unconditionally rendered (with the health server just not started by default) or gated by a differently-named key. The PR body's "dry-run against the cluster, serverside-applied, sauber" check doesn't actually settle this: a dry-run of the `HelmRelease` object only validates the CR against the Flux CRD schema (`values` is a free-form object there), not that the referenced Helm chart accepts or acts on these keys — that only happens on actual reconciliation/template render by helm-controller. Worth confirming with `helm show values external-secrets/external-secrets --version <resolved-version>` (or checking the post-merge rollout, which the PR already plans to do) before treating this as settled. - `kubernetes/apps/paperless/ftp-scanner.yaml:78-89` — minor: `readinessProbe` here removes the Pod from the `ftp-scanner` LoadBalancer's endpoints on any transient TCP hiccup (the Service uses `externalTrafficPolicy: Local`). Given vsftpd's passive-port dance already depends on control-port continuity, a flapping readiness probe mid-transfer would drop in-flight FTP sessions the same way a real outage would. Probably an acceptable trade given the periodSeconds/failureThreshold defaults (3 failures needed), just worth being aware of it as a slightly sharper failure mode than "silently stale". ## 🟢 Nits None beyond the above. ## Tests This is a GitOps manifest repo; there's no unit-test harness for probe correctness, and the PR body describes an appropriate manual verification plan post-merge (confirm `Ready` + `0 Restarts` per rollout, and specifically check the external-secrets health server actually binds rather than trusting the chart). That's the right kind of check for this change and I have no additional test to ask for beyond fixing the blocking issue above and re-verifying `home-assistant`'s Pod stays `Ready` under a simulated code-server hang (or dropping the readinessProbe entirely). ## Recommendation Request changes — 1 blocking issue. <!-- pr-review:sha=469cdbc2eea28261f9fc17c5b2d1e048068e7be9 -->
Sign in to join this conversation.