Add the Proxy reconciler state machine with action-table, phase, and envtest suites
Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
@@ -8,7 +8,7 @@ Pairs with [docs/plans/2026-08-07-1747-proxy-operator.md](../plans/2026-08-07-17
|
||||
- [x] Step 1 — API types (`api/v1alpha1/proxy_types.go`)
|
||||
- [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)
|
||||
- [ ] Step 4 — Reconciler (`internal/controller/`)
|
||||
- [x] Step 4 — Reconciler (`internal/controller/`)
|
||||
- [ ] Step 5 — Health engine (`internal/health/`)
|
||||
- [ ] Step 6 — Lease store (`internal/lease/`)
|
||||
- [ ] Step 7 — Discovery API (`internal/discovery/`)
|
||||
@@ -560,3 +560,69 @@ Verified: `bin/kustomize build config/default` and `... config/crd` both
|
||||
render cleanly (no dangling references), `go build`/`go vet` clean with and
|
||||
without `-tags=e2e`, `make test` green with coverage numbers identical to
|
||||
pre-cleanup.
|
||||
|
||||
## Step 4 — Reconciler (`internal/controller/`)
|
||||
|
||||
Implemented the state machine per the plan's action table:
|
||||
`proxy_controller.go` (dispatch + managed/external/delete paths, cloud-init
|
||||
resolution, spec-hash annotation persistence, Secret→Proxy watch mapping),
|
||||
`status.go` (condition reasons, `computePhase`, the single deferred
|
||||
`patchStatusIfChanged`), and `spechash.go` (explicit
|
||||
`{placement, resolved cloud-init, port}` hash input, SHA-256 hex). Tests:
|
||||
the action-table suite against a fake client with an in-test `stubProvider`
|
||||
(`reconcile_test.go`), `computePhase` truth table, spec-hash
|
||||
stability/normalization/sensitivity tables, and a rewritten envtest suite
|
||||
(`proxy_controller_test.go`) driving full lifecycles — provision→Running,
|
||||
spec-change replacement, finalizer deletion, External tracking — against
|
||||
the real apiserver with real CRD defaulting.
|
||||
|
||||
**Deviation from the plan's `Requeue: true` rows:** `ctrl.Result{Requeue}`
|
||||
is deprecated in controller-runtime v0.24 (verified in the vendored source,
|
||||
`pkg/reconcile/reconcile.go`: "Deprecated: Use `RequeueAfter` instead"), and
|
||||
golangci's staticcheck would flag it. Those rows use a fifth configurable
|
||||
interval instead, `RequeueNow` (default 1s) — same "process the next state
|
||||
promptly" semantics, still shrinkable in tests like the other four.
|
||||
|
||||
**A real bug the new tests caught on their first run** (both the fake-client
|
||||
and envtest suites, independently): in the replacement path's
|
||||
instance-is-gone branch, the status clear (`ProviderID = ""`) was staged
|
||||
*before* `setSpecHash`'s metadata `Update` — and `client.Update` refreshes
|
||||
the whole object from the server's response, *including status*, so the
|
||||
staged clear was silently overwritten and the proxy wedged with a stale
|
||||
providerID. Fix: stage status changes only after any metadata Update
|
||||
(the create branch already did it in that order). Worth remembering for
|
||||
every future reconciler: **`r.Update` clobbers in-memory status staged
|
||||
before it.**
|
||||
|
||||
Two judgment calls the plan left open, now documented in code:
|
||||
|
||||
- `computePhase` maps Provisioned=True with no Healthy verdict yet to
|
||||
`Provisioning`, not `Ready` — a proxy nobody has probed shouldn't be
|
||||
advertised as Ready. Health (Step 5) flips it.
|
||||
- `deletionFailure` (the finalizer path's error handler) never latches
|
||||
`ErrPermanent` the way `providerFailure` does — latching there would
|
||||
wedge the object forever with no retry; it keeps retrying visibly
|
||||
instead.
|
||||
|
||||
The permanent-failure latch compares the condition's `observedGeneration`
|
||||
against the CR generation, so a spec edit automatically clears Failed and
|
||||
retries — no manual annotation-poking needed to recover.
|
||||
|
||||
Verification:
|
||||
|
||||
```bash
|
||||
make test # regenerates manifests (role.yaml gains secrets get;list;watch), envtest green
|
||||
KUBEBUILDER_ASSETS="$PWD/bin/k8s/1.36.2-darwin-arm64" go test -race ./internal/controller/
|
||||
go test -short ./internal/controller/ # 0.6s — envtest suite correctly skipped
|
||||
go build -tags=e2e ./... && go vet -tags=e2e ./...
|
||||
```
|
||||
|
||||
`internal/controller` at 75.9% coverage; the envtest suite runs in ~6s and
|
||||
is now guarded by `testing.Short()` per the testing conventions.
|
||||
|
||||
Worth noting: the envtest specs simulate instance state by mutating the
|
||||
stub between direct `Reconcile` calls rather than running the manager —
|
||||
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.
|
||||
|
||||
Reference in New Issue
Block a user