Remove the in-memory mock provider
The mock provider (state simulated via an injectable clock, a hand-rolled shared/refcounted CONNECT-proxy listener per port to work around macOS's loopback restrictions) worked, but the user felt it was too far removed from the real system to build confidence in, and doesn't need the automated test suite to stay fast enough to justify that complexity — a kind-based verification pass "once in a while" is an acceptable trade for tests that actually look like the final product. Replacing it with a provider that creates real Pods in the same cluster, running an actual Squid container. internal/provider/registry was already designed to have zero dependency on any concrete provider package, so removing this one required no changes anywhere else in the tree — go build is clean with nothing implementing provider.Provider yet. docs/plans/2026-08-07-1747-proxy-operator.md's Step 3 (and every other reference to the mock provider throughout the plan) is updated in this same commit to describe the replacement. Narrative on why and the replacement's design lands in docs/plans-executions once it's built. Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
@@ -22,7 +22,7 @@ spec change deletes and recreates the VM. No in-place update logic.
|
||||
| API group | `crawl.example.com`, `v1alpha1`, kind `Proxy`, **namespaced** |
|
||||
| Git flow | Scaffold commit, then work on `feat/proxy-operator`, MR via `tea`, no merge/delete from CLI |
|
||||
| kubebuilder | `go install sigs.k8s.io/kubebuilder/v4/cmd/kubebuilder@v4.15.0` |
|
||||
| Mock health | Mock provider runs a **real in-process HTTP CONNECT proxy** per instance, so healthchecks genuinely pass and the kind demo is truly end-to-end |
|
||||
| Local/CI provider | **Kubernetes pod provider**, not an in-memory mock: creates real `ubuntu/squid` pods in-cluster. Revised mid-build — see Step 3 |
|
||||
| Verification | vet + unit + envtest, then a throwaway kind cluster running the README quickstart, then delete it |
|
||||
|
||||
### Verified environment — no version substitutions needed
|
||||
@@ -38,7 +38,7 @@ omitting the substitutions section.
|
||||
|
||||
### Milestone order — each ends green on `go build ./... && go vet ./...`
|
||||
|
||||
1. Scaffold + pins → 2. `api/v1alpha1` + CEL → 3. `internal/provider` + mock →
|
||||
1. Scaffold + pins → 2. `api/v1alpha1` + CEL → 3. `internal/provider` + kubernetes-pod →
|
||||
4. reconciler + envtest → 5. health engine → 6. lease + discovery → 7. GCP provider →
|
||||
8. orphan GC + metrics → 9. `cmd/main.go` wiring + `config/` → 10. docs + kind run.
|
||||
|
||||
@@ -64,8 +64,9 @@ plan to `docs/plans/2026-08-07-1747-proxy-operator.md` per CLAUDE.md. **Commit t
|
||||
untouched scaffold on its own** so every later diff is reviewable.
|
||||
|
||||
Post-scaffold hand-edits: `CONTROLLER_TOOLS_VERSION ?= v0.21.0` in the Makefile (CEL
|
||||
emission at the 1.36 API level); add a `run-mock` target; delete the scaffolded
|
||||
`.github/workflows/` (the remote is Gitea).
|
||||
emission at the 1.36 API level); add a `run-dev` target wired to a sample
|
||||
`--providers-config` (Step 10); delete the scaffolded `.github/workflows/` (the
|
||||
remote is Gitea).
|
||||
|
||||
---
|
||||
|
||||
@@ -120,12 +121,15 @@ health, discovery, and the hash.
|
||||
```
|
||||
internal/provider/{provider,errors,name,config,metrics}.go
|
||||
internal/provider/registry/registry.go # type→constructor — SEPARATE package
|
||||
internal/provider/{mock,gcp}/
|
||||
internal/provider/{kubernetes,gcp}/
|
||||
```
|
||||
|
||||
**Import-cycle trap:** a registry inside `internal/provider` would have to import
|
||||
`internal/provider/mock`, which imports `internal/provider`. `init()` self-registration
|
||||
is banned by CLAUDE.md, so the registry goes in its own leaf-importing package.
|
||||
`internal/provider/kubernetes` (or `.../gcp`), both of which import `internal/provider`
|
||||
for the interface. `init()` self-registration is banned by CLAUDE.md, so the registry
|
||||
goes in its own leaf-importing package, and its `Build` function takes the
|
||||
type→constructor map as a parameter instead — see Step 2's execution log entry for why
|
||||
this ended up better than a package-level map even beyond avoiding the cycle.
|
||||
|
||||
`Instance` needs **two fields the spec omits**, or orphan GC is unimplementable:
|
||||
`UID string` (from the label, for the liveness match) and `CreatedAt time.Time` (for
|
||||
@@ -157,23 +161,67 @@ same 80 bits. 80 bits → birthday collision at ~2^40 objects against a fleet of
|
||||
|
||||
---
|
||||
|
||||
## Step 3 — Mock provider (`internal/provider/mock/`)
|
||||
## Step 3 — Kubernetes pod provider (`internal/provider/kubernetes/`)
|
||||
|
||||
**State is a pure function of an injectable clock — no background timers.** `Get`/
|
||||
`ListByTag` derive state from `createdAt`/`deletedAt` vs `now()`:
|
||||
`< provisionDelay` → Provisioning; else Running; `deletedAt` set and `< deleteDelay` →
|
||||
Terminated; beyond that → purged, `ErrNotFound`. Deterministic under a fake clock,
|
||||
correct under the real one, and no goroutine lifecycle to leak.
|
||||
**Revised after Step 3 was first built as an in-memory mock provider** (state-machine
|
||||
simulation + a hand-rolled CONNECT proxy on a shared, refcounted local listener). The
|
||||
user found that too far from the real system to build confidence in, and didn't need
|
||||
tests to be fast enough to justify the complexity it cost — a real `kind`-cluster
|
||||
verification pass "once in a while" is an acceptable trade for tests that actually
|
||||
look like the final product. Full narrative of the reversal is in
|
||||
`docs/plans-executions/2026-08-07-1747-proxy-operator.md`; this section describes the
|
||||
replacement, which is what actually gets built for local dev/CI going forward. No
|
||||
in-memory provider remains in the tree — GCP is now the only other provider, per the
|
||||
spec's original two-provider scope.
|
||||
|
||||
**Real proxy listener (the user's decision):** when a record first reports `Running`,
|
||||
lazily start an `http.Server` on `127.0.0.1:0` implementing HTTP `CONNECT` tunnelling
|
||||
(hijack + bidirectional `io.Copy`) plus plain-HTTP forwarding, and report `127.0.0.1`
|
||||
plus the real listener port. `Delete` shuts it down. This is what makes the health
|
||||
engine's probe a genuine CONNECT through a real proxy, so the kind quickstart actually
|
||||
reaches Ready and leasable. One `http.Server` per instance, bounded by fleet size.
|
||||
**What it does:** `Create` creates a `corev1.Pod` running a proxy container in the
|
||||
same cluster (and same namespace as the owning Proxy CR — `req.Namespace`); `Get`
|
||||
reads the Pod's phase/IP; `Delete` deletes it (tolerating NotFound); `ListByTag` lists
|
||||
Pods by the standard `LabelManaged`/`LabelUID` labels, unscoped by namespace (the
|
||||
operator's RBAC needs cluster-scoped Pod permissions — see RBAC note below).
|
||||
|
||||
Fault injection: config-driven `failNextCreates`/`failWith` for the demo, plus
|
||||
`InjectCreateFailures(n int, class error)` for tests. `Create` is idempotent by name.
|
||||
**Proxy software: `ubuntu/squid`** (Canonical's actively maintained LTS image on
|
||||
Docker Hub, verified before picking it — 50M+ pulls, updated the same day this
|
||||
decision was made), not a hand-rolled proxy. It's a public image, so `kind` nodes
|
||||
pull it directly; no build/load step needed for the quickstart. Squid's config
|
||||
(`http_port <req.Port>`, permissive ACL) is generated in Go and injected via an env
|
||||
var the container's command writes to `/etc/squid/squid.conf` before exec'ing squid —
|
||||
no separate ConfigMap object, so there's still only one Kubernetes object per proxy
|
||||
instance to create, track, and clean up.
|
||||
|
||||
**providerID format: `<namespace>/<podName>`** (parseable with
|
||||
`k8s.io/client-go/tools/cache.SplitMetaNamespaceKey`), so `Get`/`Delete` are
|
||||
self-contained without needing to re-derive the namespace — the same reasoning as the
|
||||
GCP provider's zone-qualified providerID in Step 8.
|
||||
|
||||
**Pod naming:** reuses `provider.NameFromUID` unchanged — the same deterministic name
|
||||
satisfies Kubernetes Pod naming rules (`^[a-z0-9]([-a-z0-9]*[a-z0-9])?$`, ≤253 chars)
|
||||
with room to spare.
|
||||
|
||||
**State mapping:** Pod phase `Pending`, or `Running` with no PodIP yet → Provisioning
|
||||
(never publish an empty IP); `Running` with a PodIP → Running; `Succeeded`/`Failed`/
|
||||
`Unknown` → Terminated (the reconciler treats Stopped and Terminated identically —
|
||||
delete and recreate, cattle not pets — so collapsing three failure-ish phases into one
|
||||
is enough).
|
||||
|
||||
**Client:** built internally via `ctrl.GetConfig()` (auto-detects in-cluster config,
|
||||
falls back to the local kubeconfig otherwise), not threaded through the registry
|
||||
`Constructor` signature — this is what makes `make run` against a local `kind`
|
||||
cluster and running in-cluster use the exact same code path with no provider-specific
|
||||
wiring in `cmd/main.go`.
|
||||
|
||||
**Testing, given envtest can't schedule real Pods** (no kubelet — a Pod created
|
||||
against `envtest`'s API server just sits `Pending` forever): `internal/provider/kubernetes`
|
||||
itself is unit-tested against `sigs.k8s.io/controller-runtime/pkg/client/fake` — real
|
||||
Pod objects, real client interface, fully exercises Create/Get/Delete/ListByTag
|
||||
logic and the Pod-construction function in isolation, just without a kubelet actually
|
||||
starting a container. Real end-to-end proof (does a probe actually tunnel through a
|
||||
real Squid pod) only happens against a real `kind` cluster, in the Verification
|
||||
section — which is exactly what the user asked for. Step 4's reconciler tests use a
|
||||
small `Provider`-interface stub defined directly in the controller test file (a
|
||||
handful of lines, not a package) for exercising the state-machine's branching logic —
|
||||
categorically simpler than what mock.Provider was, since it has no config format, no
|
||||
fault-injection surface, and exists only inside test code.
|
||||
|
||||
---
|
||||
|
||||
@@ -425,14 +473,17 @@ registry → manager → `mgr.Add` health engine, GC, lease expiry loop, discove
|
||||
`SetupWithManager`. Contexts from `ctrl.SetupSignalHandler()` throughout.
|
||||
|
||||
RBAC markers: proxies CRUD + status + finalizers, secrets get/list/watch, events
|
||||
create/patch. `config/`: providers ConfigMap mount, `DISCOVERY_TOKEN` from a Secret,
|
||||
containerPort 8090 + Service. Samples: `proxy_mock.yaml`, `proxy_gcp.yaml`,
|
||||
`proxy_external.yaml`, `providers-config.yaml`.
|
||||
create/patch, and (for the kubernetes-pod provider) pods get/list/watch/create/delete
|
||||
— cluster-scoped, since `ListByTag` enumerates across namespaces. `config/`: providers
|
||||
ConfigMap mount, `DISCOVERY_TOKEN` from a Secret, containerPort 8090 + Service.
|
||||
Samples: `proxy_kubernetes.yaml`, `proxy_gcp.yaml`, `proxy_external.yaml`,
|
||||
`providers-config.yaml`.
|
||||
|
||||
`docs/architecture.md`: components table, ASCII data-flow diagram, and a **Decisions**
|
||||
section covering the channel-vs-patch choice, replacement-polls-to-NotFound, base32
|
||||
naming, discovery without leader election, single-mutex lease store, mock-runs-a-real-
|
||||
proxy, leader-handover health seeding, the latency-suppression refinement, the
|
||||
naming, discovery without leader election, single-mutex lease store, the mock→
|
||||
kubernetes-pod-provider revision (why, and why `ubuntu/squid` over a hand-rolled
|
||||
proxy), leader-handover health seeding, the latency-suppression refinement, the
|
||||
`lastHealthCheckTime` semantics, and the logr-not-slog deviation (CLAUDE.md says
|
||||
`slog`, but `log.FromContext(ctx)` returns logr inside controller paths — noted, not
|
||||
silently ignored).
|
||||
@@ -448,16 +499,17 @@ note confirming no substitutions were needed. Then a `CHANGELOG.md` entry with a
|
||||
|
||||
## Step 11 — Tests
|
||||
|
||||
**envtest** (`internal/controller/`), mock provider + a fake `HealthSnapshotter`,
|
||||
intervals shrunk to 50–200 ms, whole suite behind `testing.Short()`:
|
||||
Managed→Ready; spec change → old mock instance gone and providerID changed; delete →
|
||||
finalizer runs and instance removed; External → Ready on first health pass, no
|
||||
finalizer; injected quota → `Provisioned=False/QuotaExceeded` and phase *not* Failed;
|
||||
permanent error → Failed and no further provider calls; adopt (strip annotation →
|
||||
restored, providerID unchanged); and the **CEL cases only a real API server can test** —
|
||||
mode/provider mutation rejected, Managed-without-provider, External-without-endpoint,
|
||||
cloudInit both/neither, and `healthCheck` omitted → nested defaults materialized (the
|
||||
`default={}` assertion).
|
||||
**envtest** (`internal/controller/`), a small in-test stub `Provider` (not the real
|
||||
kubernetes-pod provider — envtest has no kubelet, so a real Pod never leaves Pending)
|
||||
plus a fake `HealthSnapshotter`, intervals shrunk to 50–200 ms, whole suite behind
|
||||
`testing.Short()`: Managed→Ready; spec change → old stub instance gone and
|
||||
providerID changed; delete → finalizer runs and instance removed; External → Ready
|
||||
on first health pass, no finalizer; injected quota → `Provisioned=False/QuotaExceeded`
|
||||
and phase *not* Failed; permanent error → Failed and no further provider calls; adopt
|
||||
(strip annotation → restored, providerID unchanged); and the **CEL cases only a real
|
||||
API server can test** — mode/provider mutation rejected, Managed-without-provider,
|
||||
External-without-endpoint, cloudInit both/neither, and `healthCheck` omitted → nested
|
||||
defaults materialized (the `default={}` assertion).
|
||||
|
||||
**Action-table unit tests** — the highest-value tests in the repo: `fake` client with
|
||||
`WithStatusSubresource`, calling `Reconcile` directly, table-driven over every row of
|
||||
@@ -466,12 +518,15 @@ client runs neither CEL nor defaulting — that's what the envtest CEL cases cov
|
||||
|
||||
**Units:** name derivation (idempotency, `^proxy-[a-z2-7]{16}$`, 10k-UID distinctness);
|
||||
`Class()` mapping + `errors.Is`/`errors.As` through the multi-unwrap; config loading;
|
||||
mock state at the delay boundary with a fake clock; `buildInsertRequest` field-by-field
|
||||
+ GCP error classification + RUNNING-without-IP; `computePhase` truth table; `SpecHash`
|
||||
stability *and* sensitivity; lease store (capacity, `MaxLeases=0`, least-loaded with
|
||||
latency tie-break, cooldown with/without target, report on an expired-but-retained
|
||||
lease, concurrent acquire under `-race` never exceeding `MaxLeases`); discovery
|
||||
handlers over `httptest` + fake reader + real store; health thresholds against a real
|
||||
kubernetes-pod provider Create/Get/Delete/ListByTag against
|
||||
`sigs.k8s.io/controller-runtime/pkg/client/fake` (real Pod objects, real client
|
||||
interface, no kubelet needed for this level) plus pure tests of the generated Squid
|
||||
config and Pod spec; `buildInsertRequest` field-by-field + GCP error classification +
|
||||
RUNNING-without-IP; `computePhase` truth table; `SpecHash` stability *and*
|
||||
sensitivity; lease store (capacity, `MaxLeases=0`, least-loaded with latency
|
||||
tie-break, cooldown with/without target, report on an expired-but-retained lease,
|
||||
concurrent acquire under `-race` never exceeding `MaxLeases`); discovery handlers
|
||||
over `httptest` + fake reader + real store; health thresholds against a real
|
||||
CONNECT-capable `httptest` proxy stub.
|
||||
|
||||
Everything runs with `-race`.
|
||||
@@ -485,19 +540,20 @@ go vet ./... && make test && make build # unit + envtest, -race
|
||||
|
||||
kind create cluster --name proxy-operator-demo
|
||||
make install
|
||||
make run-mock & # --providers-config hack/providers-mock.yaml
|
||||
kubectl apply -f config/samples/proxy_mock.yaml
|
||||
kubectl get px -w # expect Ready with an IP
|
||||
make run-dev & # --providers-config hack/providers-dev.yaml
|
||||
kubectl apply -f config/samples/proxy_kubernetes.yaml
|
||||
kubectl get px -w # expect Ready with an IP (a real squid Pod)
|
||||
curl -s 'localhost:8090/v1/proxies?healthy=true' | jq
|
||||
curl -s -XPOST localhost:8090/v1/leases -d '{"selector":{"geo":"eu"},"ttlSeconds":300}' | jq
|
||||
curl -s -XPOST localhost:8090/v1/leases/<id>/report -d '{"result":"rate_limited","target":"example.com"}'
|
||||
curl -si -XDELETE localhost:8090/v1/leases/<id> # 204, and 204 again
|
||||
kubectl delete -f config/samples/proxy_mock.yaml # finalizer runs, object goes
|
||||
kubectl delete -f config/samples/proxy_kubernetes.yaml # finalizer runs, Pod is deleted
|
||||
kind delete cluster --name proxy-operator-demo
|
||||
```
|
||||
|
||||
Success bar: a competent SRE clones the repo, follows the README, and holds a lease on a
|
||||
healthy mock proxy in under 10 minutes.
|
||||
healthy proxy — a real Squid pod running in their own `kind` cluster — in under 10
|
||||
minutes.
|
||||
|
||||
Then commit on `feat/proxy-operator`, push with `-u`, open the MR with
|
||||
`tea pr create --base main --head feat/proxy-operator`, print the URL. No merging or
|
||||
|
||||
Reference in New Issue
Block a user