kacerrandClaude Opus 5
04dd2c0284
Each configured host is checked concurrently over the pooled SSH+libvirt connection: libvirt version and hostname prove the daemon answers, then the configured pool must exist and be running, with its free space reported. Dial failures are now tagged with their stage (SSH material, TCP, SSH handshake, libvirt connect) by a dialError that keeps the message and unwraps to the cause, so classification is unchanged and the check can say why a host is unreachable. A pool verdict is decided after the call returns so it never drops a working connection. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
141 lines
6.3 KiB
Markdown
141 lines
6.3 KiB
Markdown
# 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
|
|
- [ ] 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.
|