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](../plans/2026-09-11-1526-provider-health-checks.md)

- [x] Step 0 — Save the plan
- [x] Step 1 — ProviderHealth API and CRD
- [x] Step 2 — Contract plus the kubernetes and GCP checks
- [x] Step 3 — KVM check
- [x] 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:

```bash
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:

```bash
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:

```bash
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:

```bash
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
```

```bash
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:

```bash
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:

```bash
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):

```bash
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.