Skip to content

fix: add operator liveness/readiness probes and gate readiness on the webhook server - #23

Open
xcompass wants to merge 1 commit into
mainfrom
fix/operator-probes
Open

xcompass wants to merge 1 commit into
mainfrom
fix/operator-probes

Conversation

@xcompass

Copy link
Copy Markdown
Member

Problem

The Helm chart renders the manager Deployment with no probes. The binary already serves /healthz and /readyz on :8081, and config/manager/manager.yaml (kustomize) has the kubebuilder probes, but the chart never carried them over (upstream's chart doesn't have them either). Consequences:

  • a hung manager is never restarted;
  • the pod is marked Ready as soon as the container starts.

Changes

Chart (templates/deployment.yaml, values.yaml)

  • livenessProbe → GET /healthz, readinessProbe → GET /readyz, on a named health port.
  • Timings from the kubebuilder scaffold (liveness 15s/20s, readiness 5s/10s); each probe configurable and switchable under probes.*.
  • --health-probe-bind-address=:<probes.port> passed explicitly, so the flag and the probe port can't drift.

Manager (cmd/main.go)

  • /readyz now also runs mgr.GetWebhookServer().StartedChecker(). The webhooks are registered unconditionally with failurePolicy: Fail, and readiness is what adds the pod to the webhook Service's endpoints. With only healthz.Ping, the pod could go Ready before the webhook server listened, rejecting every HermesInstance create/update in that window, including GitOps syncs during an operator rollout.

Verification

  • go build ./..., go vet ./cmd/...
  • helm lint
  • helm template: defaults render both probes + health port + the explicit bind flag; probes.*.enabled=false omits them
  • hack/check-chart-image-tags.sh passes
  • Scratch program on this module's controller-runtime: StartedChecker returns webhook server has not been started yet before Start, nil ~100ms after
  • CI green
  • Merge → release-please opens release 0.2.1 → merge that to publish
  • configuration: bump the operator appset to 0.2.1, confirm probes pass live

Worth upstreaming to stubbi/hermes-operator afterwards, since the upstream chart has the same gap.

https://claude.ai/code/session_019rd9pDQddWJ2HQMjNvT1CA

… webhook server

The Helm chart rendered the manager Deployment with no probes at all, even though the binary
serves /healthz and /readyz on :8081 and config/manager/manager.yaml (kustomize) has the
kubebuilder probes. A hung manager was therefore never restarted, and the pod was marked ready
the moment the container started.

Chart:
- livenessProbe GET /healthz and readinessProbe GET /readyz on a named `health` port, timings
  from the kubebuilder scaffold (15s/20s and 5s/10s), each configurable and switchable under
  `probes.*` in values.yaml.
- pass --health-probe-bind-address=:<probes.port> explicitly so the flag and the probe port
  cannot drift.

Manager:
- /readyz now also runs mgr.GetWebhookServer().StartedChecker(). The webhooks are registered
  unconditionally and use failurePolicy: Fail, and readiness is what puts the pod into the
  webhook Service's endpoints. With only healthz.Ping, the pod could go ready before the
  webhook server listened, rejecting every HermesInstance create/update in that window
  (including GitOps syncs during an operator rollout).

Verified: go build/vet; helm lint; helm template renders both probes by default and omits
them when disabled; hack/check-chart-image-tags.sh passes; a scratch program on this module's
controller-runtime confirmed StartedChecker returns "webhook server has not been started yet"
before Start and nil ~100ms after.

Claude-Session: https://claude.ai/code/session_019rd9pDQddWJ2HQMjNvT1CA
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant