Add the Kubernetes pod provider (replaces the removed mock)
Create/Get/Delete/ListByTag against real corev1.Pod objects in the same cluster the operator runs in, running an ubuntu/squid container -- picked by actually checking Docker Hub metadata (Canonical-published, rebuilt the same day this was decided, 50M+ pulls) rather than guessing an image reference. It's a public image, so kind nodes pull it directly with no build/load step. providerID is "<namespace>/<podName>", parsed via cache.SplitMetaNamespaceKey -- the same self-contained-providerID reasoning the plan already calls for on the GCP provider's zone-qualified IDs. Pod state maps to InstanceState with Succeeded/Failed/Unknown all collapsing to Terminated, since the reconciler already treats Stopped and Terminated identically; Running-without-PodIP maps to Provisioning so an empty IP is never published. The client is built internally via ctrl.GetConfig() (in-cluster or local kubeconfig, whichever applies), not threaded through the registry Constructor signature -- this is what lets `make run` against a local kind cluster and running in-cluster share the exact same code path with no provider-specific wiring in cmd/main.go. New() is deliberately untested (0% coverage): it's the one function that must never run under `go test`, since it would happily connect to whatever cluster the developer's kubeconfig points at. Tests construct Provider via an unexported newWithClient(client, cfg) instead. ListByTag lists Pods across every namespace (orphan GC needs to find every tagged Pod regardless of where it landed), which means this provider's RBAC has to be a ClusterRole rather than namespace-scoped -- flagged now, wired in Step 10. provider.Config gains KubernetesConfig (replacing MockConfig) and drops the FailWith*/fault-injection surface entirely, since that need is now served by a small in-test stub Provider for reconciler tests (Step 4), not a config-driven mechanism on a real provider package. Tests use sigs.k8s.io/controller-runtime/pkg/client/fake -- real Pod objects, the real client.Client interface -- at 77.6% coverage. make test green across the whole repo. Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
@@ -303,3 +303,110 @@ end-to-end healthcheck, not a simulated one.
|
||||
|
||||
`internal/provider/mock` at 91.1% coverage. `make test` green across the
|
||||
whole repo.
|
||||
|
||||
## Step 3 (revised) — Kubernetes pod provider replaces the mock
|
||||
|
||||
After the mock provider above was built and working, the user pushed back:
|
||||
it felt too far from the real system to build confidence in, and they'd
|
||||
rather have simpler, more "real" code than a fast-but-simulated test
|
||||
double — a `kind`-based verification pass "once in a while" is an
|
||||
acceptable trade. They proposed replacing it outright with a provider that
|
||||
creates real Pods in the operator's own cluster, rather than keeping mock
|
||||
around as a fallback.
|
||||
|
||||
**Talked through the trade-off before agreeing to it, since it wasn't free
|
||||
of downsides.** Pods sharing a cluster's egress IPs don't solve what this
|
||||
operator actually exists for (routing around IP-based rate limiting needs
|
||||
genuinely distinct egress paths — that's still only `gcp`), and `envtest`
|
||||
has no kubelet, so a Pod-based provider can never be exercised by the fast
|
||||
test suite either. Net effect: this is a replacement for the mock's role
|
||||
in local dev/CI confidence-building, not a new alternative to GCP, and the
|
||||
reconciler's own state-machine tests (Step 4) still need a minimal
|
||||
in-test stub `Provider` — a handful of lines in the test file, not a
|
||||
package with its own config format or fault-injection surface, which is
|
||||
the specific kind of complexity the user was pushing back on.
|
||||
|
||||
**Picked the proxy software by actually checking what's out there,** not
|
||||
by guessing a Docker Hub path:
|
||||
|
||||
```bash
|
||||
curl -s "https://hub.docker.com/v2/repositories/vimagick/tinyproxy/" | head -c 400
|
||||
# last_updated 2021-07-22 — stale
|
||||
curl -s "https://hub.docker.com/v2/repositories/ubuntu/squid/" | head -c 400
|
||||
# last_updated 2026-08-07T04:36:14Z — updated the same day, Canonical-published, 50M+ pulls
|
||||
```
|
||||
|
||||
`ubuntu/squid` won clearly: actively maintained (rebuilt the same day this
|
||||
check ran), official publisher, and — since it's a public image — `kind`
|
||||
nodes pull it directly, no build/load step needed for the quickstart.
|
||||
Pinned to `6.6-24.04_edge` (Ubuntu 24.04 LTS base) rather than floating
|
||||
`latest`, for reproducibility.
|
||||
|
||||
**Design, in `internal/provider/kubernetes/`:**
|
||||
|
||||
- **`pod.go`** — `buildPod` is a pure function (mirrors the GCP provider's
|
||||
planned `buildInsertRequest`): builds a `corev1.Pod` with one Squid
|
||||
container. Config is generated in Go and passed via a `SQUID_CONF` env
|
||||
var that the container's command writes to `/etc/squid/squid.conf`
|
||||
before `exec squid` — deliberately not a separate `ConfigMap`, so
|
||||
there's still exactly one Kubernetes object per proxy instance to
|
||||
create, track, and clean up. The config itself is intentionally
|
||||
permissive (`http_access allow all`, `via off`, `forwarded_for off`) —
|
||||
documented in-code as a prototype-for-a-private-cluster choice, the same
|
||||
posture the spec already takes toward cloud-init on the GCP provider
|
||||
(installing/configuring proxy software is explicitly out of scope
|
||||
there; this provider doesn't try to do more).
|
||||
- **`kubernetes.go`** — `Create`/`Get`/`Delete`/`ListByTag` against a
|
||||
`client.Client`. **`providerID` is `<namespace>/<podName>`**, parsed
|
||||
with `k8s.io/client-go/tools/cache.SplitMetaNamespaceKey` — the exact
|
||||
same reasoning as the GCP provider's planned zone-qualified providerID
|
||||
(Step 8): `Get`/`Delete` need to be self-contained without re-deriving
|
||||
where the resource lives. State mapping collapses `Succeeded`/`Failed`/
|
||||
`Unknown` all into `Terminated`, since the reconciler already treats
|
||||
Stopped and Terminated identically (delete + recreate) — no finer
|
||||
distinction would change any behavior. `Running` with no `PodIP` yet
|
||||
maps to `Provisioning`, not `Running`, so an empty IP is never
|
||||
published — the same rule the plan already called out for GCP's
|
||||
`RUNNING`-without-`NatIP` case.
|
||||
- **Client construction is the one genuinely new pattern this provider
|
||||
needed** that GCP/mock didn't: it builds its own `client.Client` via
|
||||
`ctrl.GetConfig()`, which auto-detects in-cluster config when running as
|
||||
a Pod and falls back to the local kubeconfig otherwise. That's what
|
||||
makes `make run` against a local `kind` cluster and running in-cluster
|
||||
use the exact same code path with zero provider-specific wiring in
|
||||
`cmd/main.go`. The corresponding risk: `New()` must never run under
|
||||
`go test` — it would happily connect to whatever real cluster the
|
||||
developer's kubeconfig points at. Solved the same way `mock.Provider`
|
||||
solved clock injection: an unexported `newWithClient(c, cfg)` constructor
|
||||
that tests call directly, bypassing `ctrl.GetConfig()` entirely.
|
||||
`New()` sits at 0% test coverage, deliberately — it's the one function
|
||||
that must stay untested by design.
|
||||
- **RBAC implication worth flagging now** (implemented in Step 10):
|
||||
`ListByTag` lists Pods across every namespace, not just the namespace(s)
|
||||
Proxies live in, because orphan GC needs to find every Pod this operator
|
||||
tagged regardless of where it landed. That means the operator's Pod
|
||||
permissions have to be a `ClusterRole`, not scoped to a single
|
||||
namespace — a real trade-off against the "fleet lives in one namespace,
|
||||
keeps RBAC simple" principle the spec states for the CRD itself.
|
||||
Documented rather than special-cased away.
|
||||
- **One known simplification, documented in a code comment rather than
|
||||
solved:** Kubernetes surfaces both RBAC-denied and quota-exceeded as the
|
||||
same 403 Forbidden, and `apierrors` has no helper to tell them apart.
|
||||
Both classify as `ErrPermanent` — the safer of the two defaults (stop
|
||||
retrying rather than hammering an API server that will never allow the
|
||||
request), but a real ResourceQuota failure that would clear once other
|
||||
proxies are deleted won't get the 5-minute-backoff-and-retry treatment
|
||||
`ErrQuotaExceeded` gives on the GCP path.
|
||||
|
||||
**Testing:** unlike the mock, this package's tests use
|
||||
`sigs.k8s.io/controller-runtime/pkg/client/fake` — real `corev1.Pod`
|
||||
objects, the real `client.Client` interface, not a hand-rolled in-memory
|
||||
map. That's a strictly more realistic test double than what mock.Provider
|
||||
was, while still being fast (no cluster, no kubelet) — it just can't prove
|
||||
a container actually starts and serves traffic, which is exactly the gap
|
||||
the `kind`-based verification pass is for. 77.6% coverage; the only
|
||||
meaningfully uncovered function is `New()` itself, deliberately.
|
||||
|
||||
`make test` green across the whole repo (`go build`/`go vet` clean,
|
||||
`internal/provider` 96.0%, `internal/provider/kubernetes` 77.6%,
|
||||
`internal/provider/registry` 100%, unchanged).
|
||||
|
||||
Reference in New Issue
Block a user