egress-proxies-operator/docs/plans-executions/2026-09-11-1526-provider-health-checks.md

Execution: provider health checks

Plan: docs/plans/2026-09-11-1526-provider-health-checks.md

• Step 0 — Save the plan
• Step 1 — ProviderHealth API and CRD
• Step 2 — Contract plus the kubernetes and GCP checks
• Step 3 — KVM check
• Step 4 — Monitor
• Step 5 — Metrics and wiring
• Step 6 — Docs
• Step 7 — Validate, record, push, open the PR

Step 0 — Save the plan

Branched off main (at the PR #5 merge, eb8f075). The plan was copied out of ~/.claude/plans/ and committed on its own, before any code, as the repo's plan rules require:

git checkout -b feat/provider-health main
cp ~/.claude/plans/now-i-would-like-flickering-wall.md docs/plans/2026-09-11-1526-provider-health-checks.md

The uncommitted .claude/settings.json change was left unstaged and stays out of this branch.

Step 1 — ProviderHealth API and CRD

kubebuilder isn't installed here (only bin/controller-gen, bin/kustomize and bin/setup-envtest are), so api/v1alpha1/providerhealth_types.go and the PROJECT entry were written by hand instead of with kubebuilder create api. For a cluster-scoped resource PROJECT simply omits namespaced, which is what kubebuilder itself writes.

The new types are registered in the existing AddKnownTypes call in proxy_types.go, not in a second init().

The deepcopy code, the CRD and the RBAC come from the usual generators:

make generate manifests
go build ./... && go vet ./api/... && go test -short ./api/...

To confirm the new CRD and the viewer role actually reach the rendered install, I built the kustomization and grepped for them:

bin/kustomize build config/default | grep -n providerhealth
# providerhealths.crawl.example.com (scope: Cluster, shortName ph)
# egress-proxies-operator-providerhealth-viewer-role

Worth noting:

• The only role added is the viewer role. The operator is the object's only writer, so the admin and editor roles kubebuilder scaffolds for Proxy were not added.
• The type file has no unit test, since it has no logic. The schema, including the required ready/lastTransitionTime and the list-map keys, is proven by the envtest round-trip in Step 4.
• config/rbac/role.yaml is unchanged so far. The manager's own permissions arrive in Step 4 with the Monitor's RBAC markers.

Step 2 — Contract plus the kubernetes and GCP checks

provider.Checker is an optional interface in internal/provider/check.go, kept out of Provider. Each implementation has a compile-time assertion, because the monitor's type assertion would otherwise fail silently.

• kubernetes: lists at most one managed Pod cluster-wide, using the same client, verb and scope as ListByTag. The target is apiserver, at the restCfg.Host now stored in the new apiHost field.
• GCP: calls a new seam method, Probe. realInstances.Probe lives in check.go, next to its only caller. ListByTag's Sprintf filter became the shared managedFilter constant, so the check and the GC sweep can't drift apart.

Before relying on "MaxResults is only a page size", I checked the SDK's REST AggregatedList in the module cache:

grep -n "func (c \*instancesRESTClient) AggregatedList" -A80 \
  "$(go list -m -f '{{.Dir}}' cloud.google.com/go/compute)"/apiv1/instances_client.go
# InternalFetch: req.MaxResults = pageSize ... return elems, resp.GetNextPageToken(), nil

go build ./... && go vet ./internal/provider/... && go test -race -count=1 ./internal/provider/...

Worth noting:

• The planned TestRealInstances_Probe_fetchesOnePage became a three-case TestRealInstances_Probe table.
  • It keeps the first page's nextPageToken, and asserts exactly one request with maxResults=1 for each case.
  • "empty project" covers iterator.Done.
  • "api error keeps its status" proves the real client returns a *googleapi.Error, so a disabled Compute API is reported as Forbidden rather than Unreachable.
• Once a context has ended, both checks report Timeout whatever the error says. The monitor's timeout is the usual cause, and the underlying error only repeats it.

Step 3 — KVM check

conn.go now wraps each of the four dial stages in a dialError: ssh-material, tcp, ssh-handshake and libvirt-connect. Error() returns the inner message unchanged and Unwrap() returns the cause, so classify/isClassified and every existing message-matching test behave as before. TestDialError_keepsClassification pins this down.

The three new RPCs went into libvirtAPI, tracingAPI and fakeAPI. The method-count guard in TestTracingAPI_logsEveryMethod fails until a traced row exists for each new method, which is what forced the three new rows.

Check runs one goroutine per host, over the shared connection, through hostPool.call. The pool verdict is decided only after call returns, because returning it as an error from inside the callback would make call drop a perfectly good connection.

Build, vet (also with the integration tag, since that file changed) and race tests:

go build ./internal/provider/... && go vet ./internal/provider/kvm/ \
  && go vet -tags=integration ./internal/provider/kvm/ \
  && go test -race -count=1 ./internal/provider/kvm/

Worth noting:

• Pool state wording: poolState uses virsh's words (running, inactive, …) rather than go-libvirt's constant names, so it reads like virsh pool-list output.
• Details: each host also reports its SSH user. That is the first thing to check when the reason is SSHHandshakeFailed.
• Tests renamed or merged:
  • The planned TestSSHAuth_dial_stages is TestLazySSH_dial_tagsTheFailingStage. It dials through lazySSH, so the ssh-material stage is covered as well as the three stages the in-process SSH server can trigger.
  • The pool-inactive and pool-missing cases are merged into TestCheck_libvirtVerdicts, which also covers a daemon error. Each case runs two checks and asserts a single dial.
• Tests added beyond the plan:
  • TestCheck_timeout: a daemon that stops answering gives Timeout.
  • TestDialError_keepsClassification.
• Not run yet: TestKVMCheck_realHost in the integration file only compiles here. It needs beelink, so it goes on the Step 7 checklist for the user to run.

Step 4 — Monitor

internal/providerhealth has the Monitor runnable plus the pure buildStatus.

• Scheduling: the Monitor is leader-only. It cleans up once at start, checks immediately, then checks on a ticker.
• Each round: providers are checked concurrently under one timeout, then logged and published one after another. A failed write is logged, and the next round writes again.
• Reads: objects are read through the reader it is given. main.go passes the manager's API reader (Step 5).

The first make manifests left config/rbac/role.yaml unchanged, with no error. I had put the +kubebuilder:rbac markers inside Monitor's doc comment, and controller-gen only reads RBAC markers from package-level comment groups. So they were silently attached to the type and ignored. An empty git diff config/rbac/role.yaml was the only sign. Moving them into their own comment block fixed it:

make manifests && git diff config/rbac/role.yaml
# + providerhealths: create, delete, get, list
# + providerhealths/status: get, patch

Vet, race tests and the envtest round-trip (using the bin/k8s/1.36.2-linux-amd64 binaries make setup-envtest had already fetched):

go vet ./internal/providerhealth/ && go test -race -count=1 -v ./internal/providerhealth/
# --- PASS: TestMonitor_envtestRoundTrip (6.45s) ... ok

Worth noting:

• Fake client scope: the fake client needs a RESTMapper that registers ProviderHealth as RESTScopeRoot. Its default mapper treats every kind as namespaced.
• Why the envtest calls publish directly: checkOnce only logs write errors, so a schema rejection would look like a missing object. Calling publish makes the API server's answer the test failure.
  • Its first round writes a 2000-character message; the stored message is truncated to 1024.
  • Its second round proves that only the target that flipped gets a new lastTransitionTime.
• No guard against a hung checker: the Monitor trusts Checker's contract to return once the context ends, and doesn't abandon a checker that ignores it. All three checkers stop when the context ends: KVM through awaitOrCancel, and kubernetes and GCP through their clients.
• Test renamed: the planned TestMonitor_timeoutBoundsBlockingChecker is TestMonitor_checkOnce_timeoutBoundsEachRound. It also asserts that a fast provider still publishes next to a hung one.

Step 5 — Metrics and wiring

metrics.RegisterProviderTargets registers a scrape-time collector for three gauges, each labelled {provider,target}: up, check duration and check timestamp. Register's signature is unchanged.

cmd/main.go:

• gains --provider-check-interval (1m) and --provider-check-timeout (15s);
• validates them before even loading the providers config;
• collects the Checkers from the unwrapped providers in config order, just before the metrics and tracing wrap loop;
• adds the Monitor with the manager's API reader;
• registers the collector through a small providerTargetSamples closure next to proxyPhaseCounts.

Build, vet and metrics tests:

go build ./... && go vet ./cmd/... ./internal/metrics/ && go test -race -count=1 ./internal/metrics/

To confirm bad flags fail fast, before anything touches the cluster, I ran the binary with a 5s interval. zap's development mode prints a stack trace after the error, hence the grep:

go run ./cmd --providers-config=/dev/null --provider-check-interval=5s 2>&1 | grep -m1 "Invalid provider check flags"
# ERROR setup Invalid provider check flags {"error": "--provider-check-interval must be 0 (off) or at least 10s, got 5s"}
go vet -tags=e2e ./test/e2e/

The e2e suite gets a new ordered spec, "should publish provider health". It waits for providerhealth kubernetes to report Ready=True. It runs before the existing metrics spec, which now also asserts proxy_operator_provider_target_up{provider="kubernetes",target="apiserver"} 1. By then the first round is known to be done, so the series is certain to exist.

Worth noting:

• Main-only wiring: providerTargetSamples is an 8-line loop in package main, which has no test file. The e2e metrics assertion is its only coverage.
• The KVM entry isn't asserted in e2e: the default providers config also ships kvm-home (beelink), which would report SSHMaterialUnavailable in kind. That entry is the user's own host, and asserting it would break the suite as soon as the default config changes.
• Manifest: config/manager/manager.yaml only gained a comment about the two flags. The defaults need no args.

Step 6 — Docs

• docs/architecture.md:
  • component rows for the CRD, the Checker contract and the monitor;
  • a new §11 covering the provider table, the loop diagram, the write, read and decorator choices, the gauges, and sample kubectl get ph output;
  • cross-references from the metrics (§9) and tracing (§10) sections;
  • four Decisions entries.
• docs/kvm-provider.md:
  • a "Health check" section with the reason → meaning → where-to-look table and the two consequences of sharing the connection;
  • under Known gaps, the capacity bullet now points at poolAvailable, and a new bullet says a rotated kvm-ssh Secret needs a restart;
  • the Testing section names TestKVMCheck_realHost.
• docs/testing.md:
  • suite rows for the three check_test.go files, internal/providerhealth (unit and envtest) and the metrics gauges;
  • "17 libvirt calls" is now 20;
  • the integration and e2e sections describe the new tests.
• CLAUDE.md:
  • the Architecture TODO now points at docs/architecture.md;
  • the stale "only go.mod exists" gotcha is replaced by a note that the "What this is" and "Commands" sections predate the code, plus Step 4's RBAC-marker gotcha.

Worth noting:

• docs/api.md is untouched. It documents the discovery HTTP API, which this change doesn't alter.
• CLAUDE.md's "What this is" and "Commands" sections were left stale on purpose. Rewriting them is outside this plan, so the new gotcha flags them instead of leaving them silently wrong.

Step 7 — Validate, record, push, open the PR

The full suite runs codegen, go fmt and go vet, then go test -race on every package, with both envtest suites included:

make test
# every package ok; internal/providerhealth 92.5 %, internal/provider/kvm 92.0 % coverage

go fmt rewrote nothing, and git status showed only the Step 6 docs.

make lint could not run, for a reason unrelated to this branch. The Makefile rebuilt the custom golangci-lint, which reports "built with go1.27.0", and its bundled staticcheck (honnef.co/go/tools v0.7.0) panics on the standard library before it reaches any repo package:

make lint
# buildir: package "poll" (isInitialPkg: false, needAnalyzeSource: true): unexpected expr: *ast.KeyValueExpr
# make: *** [Makefile:107: lint] Error 3

The local toolchain is go1.27.0 while go.mod says 1.26.0; the otel-tracing plan's log shows make lint running normally before the toolchain moved. Retrying with GOTOOLCHAIN=go1.26.0 would have meant downloading a toolchain, and the user declined that. go vet is clean in all three configurations: plain inside make test, plus -tags=integration and -tags=e2e.

The kind e2e suite creates and deletes its own egress-proxies-operator-test-e2e cluster. I checked KIND_CLUSTER first, because the two clusters already on the machine (kind, proxy-operator-demo) must not be torn down by cleanup-test-e2e:

make test-e2e > <scratch>/e2e.log 2>&1
# Ran 3 of 5 Specs in 86.210 seconds
# SUCCESS! -- 3 Passed | 0 Failed | 0 Pending | 2 Skipped

In the deployed manager, the kubernetes ProviderHealth reported Ready=True about a second after the pod was up (20:30:05.6 → 20:30:06.7). The metrics scrape that followed contained proxy_operator_provider_target_up{provider="kubernetes",target="apiserver"} 1. That exercises the generated RBAC, the API-reader path and the main.go gauge wiring end to end. The two skipped specs are the Tempo-gated tracing specs; TEMPO_URL was not set.

Follow-up — the check must not cost the reconciles their connection

Reviewing the shared-connection decision after the automated validation turned up two ways the health check could hurt the work it shares the link with, both of which the plan's "a check against a hung host drops the shared connection" line accepted too easily:

• A check that timed out went through hostPool.call, whose cancel path drops the connection. A libvirtd slow to answer one ConnectGetLibVersion would therefore cut off an image upload or a Create running on the same link.
• hostEntry was guarded by a sync.Mutex, so waiting for a host's slot could not be interrupted. A reconcile redialing a host that is down holds the slot for as long as the SSH dial takes — minutes on a blackholed address — and the whole check round would wait behind it, past its own timeout.

So the check now calls hostPool.probe instead of call: on ctx ending it abandons the RPC and leaves the connection alone (a link that is really dead is closed by the keepalive anyway, which ends the abandoned fn too). One abandoned probe per host is allowed to stay waiting — hostEntry.probing is an atomic.Bool, not a counter, so a daemon that never answers cannot pile up goroutines; further probes get the new errProbeBusy, which checkReason maps to Timeout. hostEntry.mu became a one-slot channel with an acquire(ctx) that only bounds waiting: an uncontended caller takes a free slot even with an expired context, so call's behaviour is unchanged where nothing is contended.

The second change came out of the same pass: --provider-check-timeout defaulted to a flat 15s while the flag validation rejected a timeout longer than the interval, so --provider-check-interval=12s alone failed to start. The flag now defaults to 0 and Monitor.setDefaults derives min(15s, Interval); ValidateSchedule accepts 0 and rejects only negatives and timeouts past the interval.

Re-verified with the same commands as above:

go vet ./...
go test -race -count=1 ./internal/provider/kvm/ ./internal/providerhealth/
make test
# every package ok; internal/provider/kvm 92.2 %, internal/providerhealth 93.7 % coverage

Worth noting:

• make lint still cannot run, for the toolchain reason recorded above; unchanged by this follow-up.
• The two new kvm tests are timing-shaped by necessity — they assert that a caller with a 50ms deadline does not wait out a dial that is blocked on a channel. The assertions are one-sided (> 1s fails) so a slow machine does not make them flaky.
• docs/architecture.md, docs/kvm-provider.md and docs/testing.md were corrected in the same commit: the old text promised the drop-on-timeout behaviour that is now gone.
• The real-hypervisor commands from the plan's Verification section are still the user's to run; nothing here replaces them.