Add the health engine: through-proxy probes, thresholds, channel-fed transitions
Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
@@ -9,7 +9,7 @@ Pairs with [docs/plans/2026-08-07-1747-proxy-operator.md](../plans/2026-08-07-17
|
||||
- [x] Step 2 — Provider contract (`internal/provider/`)
|
||||
- [x] Step 3 — Kubernetes pod provider (`internal/provider/kubernetes/`; first built as an in-memory mock, then replaced — see the two Step 3 sections below)
|
||||
- [x] Step 4 — Reconciler (`internal/controller/`)
|
||||
- [ ] Step 5 — Health engine (`internal/health/`)
|
||||
- [x] Step 5 — Health engine (`internal/health/`)
|
||||
- [ ] Step 6 — Lease store (`internal/lease/`)
|
||||
- [ ] Step 7 — Discovery API (`internal/discovery/`)
|
||||
- [ ] Step 8 — GCP provider (`internal/provider/gcp/`)
|
||||
@@ -626,3 +626,71 @@ deterministic and fast, at the cost of not exercising watch-driven
|
||||
requeues; Step 11's manager-driven cases cover that. The Secret watch is
|
||||
wired in `SetupWithManager` but the label-restricted Secret cache it
|
||||
assumes arrives with `cmd/main.go` in Step 10.
|
||||
|
||||
## Step 5 — Health engine (`internal/health/`)
|
||||
|
||||
Implemented the engine per the plan's design: `probe.go` (through-the-proxy
|
||||
probe with the plan's exact transport — fresh per probe,
|
||||
`DisableKeepAlives: true` so every probe re-exercises CONNECT) and
|
||||
`engine.go` (leader-elected manager Runnable: 1 s scheduler tick + a pool
|
||||
of 8 workers, per-proxy threshold state under one mutex, transition-only
|
||||
emission over a buffered `chan event.GenericEvent`). The reconciler side
|
||||
landed in the same step: a `HealthSnapshotter` interface + `applyHealth`
|
||||
staging the Healthy condition/latency/lastHealthCheckTime from
|
||||
`Engine.Snapshot`, and a conditional
|
||||
`WatchesRawSource(source.Channel(...))` in `SetupWithManager`. Everything
|
||||
tolerates nil (engine unwired) until `cmd/main.go` connects the two in
|
||||
Step 10.
|
||||
|
||||
Deviations and judgment calls beyond the plan text:
|
||||
|
||||
- **First-probe scheduling is split by whether a verdict was seeded.** The
|
||||
plan's startup jitter (`nextDue = now + rand(0, interval)`) applies only
|
||||
to proxies whose state was seeded from an existing Healthy condition —
|
||||
the restart case it exists for. A never-probed proxy is probed on the
|
||||
next tick instead; making a brand-new proxy wait up to a full interval
|
||||
for its first verdict would be pure lag with no thundering-herd benefit.
|
||||
- **The latency-change emission rule only applies while the verdict is
|
||||
healthy.** Caught by the first test run, not foreseen: a success streak
|
||||
still below `successThreshold` (verdict unhealthy, reported unhealthy)
|
||||
satisfied the plan's rule (c) — latency delta vs a stale reported value,
|
||||
rate window open — and emitted a pointless latency-only update for a
|
||||
proxy still reported as unhealthy. Guarded with `res.ok && *st.healthy`.
|
||||
- **`ProbeTLSConfig` field added to the engine** (nil = system roots). The
|
||||
probe function needs a CA override to be testable against
|
||||
`httptest.NewTLSServer`, and the same knob is genuinely useful for
|
||||
probing targets signed by a private CA. Not a test-only backdoor.
|
||||
- **State pruning doubles as replacement hygiene:** any proxy with no
|
||||
probeable host (provisioning, mid-replacement, deleting) has its state
|
||||
dropped each tick, so a replacement instance always starts with fresh
|
||||
counters. Complementarily, the reconciler's create branch removes the
|
||||
stale Healthy condition and latency fields — a new VM shouldn't wear its
|
||||
predecessor's verdict.
|
||||
|
||||
Tests: a real CONNECT-capable proxy stub (hijack + bidirectional
|
||||
`io.Copy`) probing a real `httptest.NewTLSServer` — CONNECT success,
|
||||
refused CONNECT, unexpected status, dead proxy, plain-http forwarding;
|
||||
table-driven threshold/suppression/seeding/pruning tests driving
|
||||
`record`/`tick` directly; an end-to-end `Start` test (fake reader, fake
|
||||
probeFn, 5 ms tick) asserting event delivery, snapshot content, and clean
|
||||
shutdown on context cancel; and controller-side tests with a
|
||||
`fakeSnapshotter` proving Running+healthy ⇒ `Ready`, Running+unhealthy ⇒
|
||||
`Unhealthy`, no-verdict ⇒ no condition, and stale-verdict cleanup on
|
||||
replacement.
|
||||
|
||||
Verification (all green):
|
||||
|
||||
```bash
|
||||
make test # envtest + units; health 93.4%, controller 77.4%
|
||||
KUBEBUILDER_ASSETS="$PWD/bin/k8s/1.36.2-darwin-arm64" go test -race ./...
|
||||
go test -race -count=2 ./internal/health/ # shook out the emission-rule bug above
|
||||
```
|
||||
|
||||
Worth noting: `docs/architecture.md` (created between Steps 4 and 5 on
|
||||
user request) gained a §6 for the engine and now shows the third workqueue
|
||||
feed (`source.Channel`). The `Healthy` condition reasons live in the
|
||||
controller package (`ReasonProbeSucceeded`/`ReasonProbeFailed`) — the
|
||||
engine deliberately knows nothing about conditions except reading one at
|
||||
seed time, keeping the state/representation split honest. The
|
||||
`hint`-driven `wg.Go` idiom (Go 1.25+) replaced the classic
|
||||
`wg.Add/defer wg.Done` in the worker pool.
|
||||
|
||||
Reference in New Issue
Block a user