diff --git a/README.md b/README.md index 106552f..4ac1071 100644 --- a/README.md +++ b/README.md @@ -181,6 +181,10 @@ laptop. Health checks against in-cluster pods do **not** (see the quickstart note) — use the in-cluster deploy to see a kubernetes-provider proxy go `Ready`. +The full test inventory — what each suite covers, the deliberate gaps, +and the manual kind verification procedure — is in +[docs/testing.md](docs/testing.md). + Project layout, reconcile-loop diagrams, and the decision log are in [docs/architecture.md](docs/architecture.md); the build history is in [docs/plans-executions/](docs/plans-executions/). diff --git a/docs/testing.md b/docs/testing.md new file mode 100644 index 0000000..61a2d8a --- /dev/null +++ b/docs/testing.md @@ -0,0 +1,123 @@ +# Testing + +What tests exist, how to run them, and where the boundaries of the +automated suites are. The philosophy throughout: **test against the most +real double available** — a real envtest API server over a fake client, a +real CONNECT-capable proxy stub over a mocked HTTP client, the real lease +store under the discovery handlers — and leave the gaps that only real +infrastructure can close to the kind verification run, documented at the +bottom. + +## Running + +```sh +make test # THE canonical run: codegen + fmt + vet + envtest setup, + # then `go test -race` on every package, with coverage +go test -short ./... # skip the envtest suite (runs in <1s per package) +go tool cover -html=cover.out # browse coverage from the last make test +``` + +Targeted runs: + +```sh +go test -race ./internal/lease/ # one package +go test -race -run TestAcquire ./internal/lease/ # one test (or a prefix) +go test -race -count=2 ./internal/health/ # flake-shaking: run twice +``` + +**Gotcha — plain `go test ./...` fails the controller package** with an +error like `/usr/local/kubebuilder/bin/etcd: no such file`. That is not a +code problem: the envtest suite needs `KUBEBUILDER_ASSETS` pointing at the +API-server/etcd binaries, which only `make test` sets up (via +`setup-envtest`). Either use `make test`, or export it once: + +```sh +KUBEBUILDER_ASSETS="$PWD/bin/k8s/--" go test -race ./... +``` + +Everything runs with `-race` — `make test` enforces it; keep the flag on +manual runs too. + +## The suites, package by package + +| Package | Files | What is covered | Test double / technique | +|---|---|---|---| +| `api/v1alpha1` | `helpers_test.go` | `EffectivePort/EffectiveHost/HealthCheckOrDefault/MaxLeasesOrDefault` | pure table-driven; CEL rules are **not** unit-testable — see envtest below | +| `internal/controller` | `reconcile_test.go` | every row of the reconciler's action table: create, poll, publish IP, replace, adopt, NotFound recovery, delete/finalizer, quota/permanent/transient errors, cloud-init resolution | `controller-runtime/pkg/client/fake` + an in-test `stubProvider` | +| | `status_test.go` | `computePhase` truth table (10 rows) | pure | +| | `spechash_test.go` | hash stability, nil/empty normalization, sensitivity to every replacement-triggering field | pure | +| | `health_test.go` | Healthy-condition representation: Ready/Unhealthy phases, no-verdict, stale-verdict cleanup on replacement, nil snapshotter | fake client + `fakeSnapshotter` | +| | `proxy_controller_test.go` (envtest, ginkgo) | full lifecycles against a **real API server**: provision→Running, spec-change replacement, finalizer deletion, External tracking, Ready-through-health, quota-vs-permanent, adoption; **all CEL rules** (six invalid creates, mode/provider immutability incl. removal) and the `default={}` materialization | envtest apiserver + stub provider; the only place CEL and structural defaulting actually execute | +| `internal/provider` | `name_test.go` | deterministic naming: idempotency, `^proxy-[a-z2-7]{16}$`, 10k-UID distinctness | pure | +| | `errors_test.go` | taxonomy: `Class()` mapping, `errors.Is` **and** `errors.As` through the multi-unwrap | pure | +| | `config_test.go` | providers-config parsing + fail-fast validation | pure | +| | `metrics_test.go` | `WithMetrics` decorator: classified result labels, error passthrough | fake recorder + static provider | +| `internal/provider/kubernetes` | `kubernetes_test.go`, `pod_test.go` | Create/Get/Delete/ListByTag over real Pod objects; pure `buildPod`/`squidConf` incl. the `max_filedescriptors` OOM-regression assertion | `client/fake` (real `corev1.Pod`s, no kubelet) | +| `internal/provider/gcp` | `insert_test.go`, `errors_test.go`, `gcp_test.go` | field-by-field `buildInsertRequest`; HTTP-code classification table; zone-qualified IDs, 409-is-success, 9-row state mapping, ListByTag filter + `ReturnPartialSuccess` | pure builder needs **no fake**; the rest uses the flattened `instancesAPI` seam | +| `internal/health` | `probe_test.go` | probes through a **real CONNECT-capable proxy stub** to a real TLS server: tunnel success, refused CONNECT, wrong status, dead proxy, plain-http forward | `httptest` + hijacked bidirectional tunnel | +| | `engine_test.go` | thresholds, first-verdict, latency suppression matrix, dropped-event retry, stale-probe UID guard, tick scheduling/seeding/pruning, `Start` end-to-end | fake `client.Reader` + injected `probeFn` | +| `internal/lease` | `store_test.go` | capacity, `MaxLeases=0`, all three selection tie-breaks, target-scoped vs global cooldowns, expiry via fake clock, report-on-retained-lease, idempotent release, **40 concurrent acquires never exceeding capacity** | injectable clock; the concurrency case is why `-race` matters | +| `internal/discovery` | `server_test.go` | auth matrix (incl. `/healthz` exemption), list filtering, grant shape, full 409 arithmetic, invalid TTL/body/result, idempotent release, report→cooldown→409 round trip, `Start`/shutdown | `httptest` over the real handler chain, fake cache reader, **real** `lease.Store` | +| `internal/gc` | `gc_test.go` | the true-orphan matrix (live/deleting/young/unlabelled all kept), broken-provider isolation, list-failure skips sweep, namespace guard, sweep loop | fake reader + canned-list provider | +| `internal/metrics` | `metrics_test.go` | scrape-time collectors (`GatherAndCompare`), per-proxy series cleanup via `ForgetProxy`, label counts, fresh-registry-per-test property | `prometheus/client_golang/testutil` | + +Conventions (from `CLAUDE.md`): stdlib `testing`, table-driven by default, +`t.Parallel()` where safe, tests next to source, ginkgo only in the +envtest suite the scaffold generated. + +## What the automated suites deliberately do NOT cover + +- **`New()` constructors that dial real infrastructure**: the kubernetes + provider's `New` (connects to whatever your kubeconfig points at), the + GCP provider's `New` and its `realInstances` SDK adapter (ADC + real + Google endpoints). Both are thin; both are exercised by the kind run / + real deployments. Their 0% coverage is by design — do not "fix" it. +- **CEL and structural defaulting under the fake client**: the fake + client runs neither. Every unit test that relies on defaults calls the + `*OrDefault` helpers; every CEL rule is asserted in the envtest suite + instead. +- **A container actually starting and serving**: envtest has no kubelet, + so a Pod created there sits Pending forever. Whether Squid really comes + up and tunnels CONNECT is provable only on a real cluster — that is the + kind verification's job, and it is exactly what caught the Squid OOM + bug (below). +- **Live GCP**: no test talks to Google. The seam boundary + (`buildInsertRequest` + classification) is tested exhaustively instead. + +## The scaffolded `make test-e2e` suite + +`test/e2e/` is the kubebuilder-generated smoke suite (build image → kind +cluster → deploy → assert the manager pod runs and serves metrics). It +compiles only under `-tags=e2e`, manages its own kind cluster +(`make test-e2e` / `make cleanup-test-e2e`), and has been kept compiling +(`go vet -tags=e2e ./...` is part of the routine) but is **not part of +`make test` and was not used for the release verification** — the manual +kind run below covers strictly more. Treat it as scaffold to grow into if +CI wants an automated cluster smoke test. + +## The kind verification run (the real end-to-end) + +The README quickstart **is** the e2e test, run by hand before the MR: +deploy the operator in-cluster on a throwaway kind cluster, watch a real +Squid pod reach `Ready` via a real CONNECT probe, then exercise the whole +lease lifecycle through the discovery API (list → lease → report +`rate_limited` → same-target lease answered 409 `inCooldown:1` → release +204 twice) and delete the CR to watch the finalizer remove the pod. + +Worth knowing about that run (full detail in +[plans-executions/2026-08-07-1747-proxy-operator.md](plans-executions/2026-08-07-1747-proxy-operator.md), +"Verification" section): it was not a clean pass-through. It caught two +real bugs the automated suites structurally could not — + +1. `make run-dev` on a laptop can never produce a `Ready` + kubernetes-provider proxy: health probes originate on the host, which + cannot route to kind's pod network. The quickstart was rewritten to + deploy in-cluster. +2. Squid was OOM-killed at startup: it sizes file-descriptor tables from + `RLIMIT_NOFILE`, which containerd under kind sets effectively + unlimited. Fixed with `max_filedescriptors 1024` in the generated + config, plus a regression assertion in `pod_test.go`. + +That is the pattern to keep: when the kind run finds something, the fix +lands **with a unit-level regression test**, so the manual run stays a +discovery tool rather than a recurring gate.