Wire the composition root: flags, providers, runnables, manifests, samples, docs
Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
@@ -1,10 +1,20 @@
|
||||
# Architecture
|
||||
|
||||
> **Status:** the operator is built through Step 9 (orphan GC + metrics) of
|
||||
> [docs/plans/2026-08-07-1747-proxy-operator.md](plans/2026-08-07-1747-proxy-operator.md).
|
||||
> This document covers the event/reconcile flow, the HTTP-driven
|
||||
> lease/discovery path, and the GC sweep; the components table and the
|
||||
> Decisions section arrive with Step 10.
|
||||
## Components
|
||||
|
||||
| Component | Package | Runs as | Leader-elected | Role |
|
||||
|---|---|---|---|---|
|
||||
| Proxy CRD + helpers | `api/v1alpha1` | types | — | `Proxy` spec/status, CEL validation, defaulting, pure helpers |
|
||||
| Reconciler | `internal/controller` | controller | yes (with the manager) | the state machine: provision, replace, delete, represent health |
|
||||
| Provider contract | `internal/provider` | library | — | `Provider` interface, error taxonomy, deterministic naming, config, metrics decorator |
|
||||
| kubernetes provider | `internal/provider/kubernetes` | library | — | real Squid pods in this cluster (local dev/CI) |
|
||||
| gcp provider | `internal/provider/gcp` | library | — | Compute Engine VMs, four API calls, fire-and-forget ops |
|
||||
| Health engine | `internal/health` | Runnable | yes | through-the-proxy probes, thresholds, transition events |
|
||||
| Lease store | `internal/lease` | Runnable (expiry sweep) | no | in-memory leases + cooldowns, single mutex |
|
||||
| Discovery API | `internal/discovery` | Runnable | no | HTTP list/lease/release/report on `:8090` |
|
||||
| Orphan GC | `internal/gc` | Runnable | yes | deletes tagged instances whose CR is gone |
|
||||
| Metrics | `internal/metrics` | library | — | explicit registration, scrape-time collectors |
|
||||
| Composition root | `cmd/main.go` | binary | — | flags, provider registry, wires everything onto one manager |
|
||||
|
||||
## Event flow: cluster events → reconciler functions
|
||||
|
||||
@@ -287,3 +297,108 @@ Each consuming package defines its own small recorder interface
|
||||
(`health.ProbeMetrics`, `discovery.LeaseMetrics`, `provider.RequestRecorder`);
|
||||
`metrics.Metrics` satisfies all of them structurally, so no package other
|
||||
than `cmd/main.go` imports the metrics package.
|
||||
|
||||
## Decisions
|
||||
|
||||
Judgment calls the spec left open, and deliberate deviations — recorded so
|
||||
they read as choices, not accidents. Chronological by build step.
|
||||
|
||||
- **Registry takes its constructor map as a parameter** instead of holding
|
||||
a package-level map: avoids the provider⇄registry import cycle and puts
|
||||
the wiring at the composition root, where it is visible.
|
||||
- **Mock provider replaced by the kubernetes-pod provider** (user
|
||||
decision, mid-build): a simulated in-memory provider was too far from
|
||||
the real system to build confidence in. Local dev/CI now runs real
|
||||
`ubuntu/squid` pods (Canonical's actively maintained image, verified
|
||||
50M+ pulls, pinned tag) in the operator's own cluster. Trade-off
|
||||
accepted: envtest has no kubelet, so end-to-end proof lives in the kind
|
||||
quickstart, and cluster pods share one egress IP — distinct egress
|
||||
paths remain the GCP provider's job.
|
||||
- **`RequeueAfter: RequeueNow` instead of the plan's `Requeue: true`:**
|
||||
`ctrl.Result.Requeue` is deprecated in controller-runtime v0.24; a fifth
|
||||
configurable interval (default 1s) keeps identical semantics and stays
|
||||
shrinkable in tests.
|
||||
- **Quota exhaustion is a wait, not a failure:** `ErrQuotaExceeded` sets a
|
||||
condition and requeues slowly (5m) with a nil error — off the backoff
|
||||
curve, out of the error log, and never `phase: Failed`. Only
|
||||
`ErrPermanent` latches Failed, keyed to the generation so a spec edit
|
||||
auto-recovers.
|
||||
- **The finalizer path never latches permanent failures:** a permanent
|
||||
error during deletion keeps retrying visibly instead — latching there
|
||||
would wedge the object forever with no path out but manual finalizer
|
||||
surgery.
|
||||
- **Health transitions travel reconciler-ward over a channel**
|
||||
(`source.Channel`), not direct status patches: `phase` derives from both
|
||||
provisioning and health, so two status writers would race and flap. One
|
||||
writer of status; the engine owns health *state*, the reconciler its
|
||||
*representation*; write-only-on-transition falls out for free.
|
||||
- **Health state seeds from the existing Healthy condition on leader
|
||||
handover** (verdict kept, counters zeroed, first probe jittered), so a
|
||||
healthy fleet doesn't flap to Unknown on restart — but a real
|
||||
transition still needs a full threshold run. A never-probed proxy skips
|
||||
the jitter and probes on the next tick: startup spread matters for
|
||||
restarts, not for a single new proxy.
|
||||
- **Latency suppression is `max(20ms, 50%)` + a 60s rate limit, and only
|
||||
while the verdict is healthy.** The spec's bare ">50% change" is
|
||||
undefined at 0 and lets a proxy jittering 40↔61ms write status forever;
|
||||
the healthy-only guard (found by test) stops a below-threshold success
|
||||
streak from emitting latency updates for a proxy still reported
|
||||
unhealthy. Consequence: `status.lastHealthCheckTime` means "last
|
||||
status-affecting probe" — true probe recency is in the metrics.
|
||||
- **Deterministic instance names** are `proxy-` + 16 chars of
|
||||
base32(SHA-256(CR UID)): legal for both GCP (`[a-z2-7]` ⊂ `[-a-z0-9]`,
|
||||
22 ≤ 63 chars) and Pod names, 80 bits against birthday collisions at a
|
||||
fleet of tens. The replacement VM therefore has the *same name* as the
|
||||
one being deleted — which is why replacement polls to NotFound before
|
||||
recreating instead of racing a 409.
|
||||
- **`banned` and `rate_limited` share one cooldown window:** a second
|
||||
duration knob the spec doesn't ask for; the report's semantic
|
||||
difference is preserved in the API but not the store.
|
||||
- **Report targets fall back report → lease → global**, so a client that
|
||||
leased with a target can't accidentally poison the proxy's global pool
|
||||
by omitting the target in its report.
|
||||
- **The 409 body's `considered` counts unhealthy matches too** (the store
|
||||
only ever sees healthy candidates): `considered = atCapacity +
|
||||
inCooldown + unhealthy + eligible-but-outranked`, keeping the numbers
|
||||
additive for a human debugging "why no proxy?".
|
||||
- **TTLs above `--max-lease-ttl` are a 400, not a silent clamp** — a
|
||||
client asking for a week should find out.
|
||||
- **Discovery is not leader-elected and ships `replicas: 1`:** caches
|
||||
start before non-leader-election runnables (verified in
|
||||
controller-runtime's ordering), and a leader-elected server would leave
|
||||
non-leader replicas as broken Service endpoints. One replica because
|
||||
lease state is per-process.
|
||||
- **GCP `Create` requires zone, machineType, and image** and fails
|
||||
`ErrPermanent` naming the missing field — inventing machine-type
|
||||
defaults would silently create billable VMs of arbitrary shape.
|
||||
- **Unknown GCP instance statuses map to `Stopped`:** the reconciler's
|
||||
answer to Stopped is delete-and-recreate, the always-safe move for
|
||||
cattle when the API grows a new state.
|
||||
- **Kubernetes 403s classify as `ErrPermanent`** even though quota
|
||||
exhaustion also surfaces as 403 (indistinguishable from RBAC denial in
|
||||
`apierrors`): not hammering an API server that may never allow the
|
||||
request is the safer default; a real ResourceQuota 403 forgoes the
|
||||
gentler quota backoff. Documented at the classification site.
|
||||
- **GC kills log at Info with a `WARNING:` prefix** — logr has no Warn
|
||||
level; the plan's "log at Warn" is met in spirit with provider,
|
||||
providerID, and UID always attached. Same convention as the
|
||||
discovery server's empty-token warning.
|
||||
- **GC trusts only provable orphans:** instances without the UID label
|
||||
are never deleted, a CR with a deletionTimestamp still counts as live
|
||||
(its finalizer owns that deletion), and an unreadable Proxy list skips
|
||||
the whole sweep. The namespace guard refuses to sweep a
|
||||
namespace-restricted cache without `--gc-allow-namespaced`.
|
||||
- **Cloud-init Secrets must carry `crawl.example.com/cloud-init: "true"`:**
|
||||
the manager caches only labelled Secrets (the operator holds
|
||||
cluster-wide Secret read RBAC — an unrestricted cache would hold every
|
||||
Secret in scope). Unlabelled referenced Secrets are invisible by
|
||||
construction, surfacing as `CloudInitError`.
|
||||
- **Events RBAC from the plan is omitted:** nothing wires an
|
||||
EventRecorder in the prototype, and granting verbs nothing uses would
|
||||
be RBAC lint noise. Add the marker together with the recorder if events
|
||||
land later.
|
||||
- **logr, not slog, inside controller paths:** the repo convention says
|
||||
`slog`, but `log.FromContext(ctx)` hands controller-runtime's logr
|
||||
logger to everything running under the manager — fighting that would
|
||||
mean two logging systems in one process. Noted as a deviation rather
|
||||
than silently ignored.
|
||||
|
||||
@@ -14,7 +14,7 @@ Pairs with [docs/plans/2026-08-07-1747-proxy-operator.md](../plans/2026-08-07-17
|
||||
- [x] Step 7 — Discovery API (`internal/discovery/`)
|
||||
- [x] Step 8 — GCP provider (`internal/provider/gcp/`)
|
||||
- [x] Step 9 — Orphan GC + metrics
|
||||
- [ ] Step 10 — Wiring, config, docs
|
||||
- [x] Step 10 — Wiring, config, docs
|
||||
- [ ] Step 11 — Tests
|
||||
- [ ] Verification (vet/test/kind e2e) + commit, push, open MR
|
||||
|
||||
@@ -939,3 +939,73 @@ Worth noting: `prometheus/client_golang` was already in the module via
|
||||
controller-runtime's metrics server, so no new dependency — `go mod tidy`
|
||||
just promoted it to direct. `docs/architecture.md` gained §8 (GC sweep)
|
||||
and §9 (metrics shape).
|
||||
|
||||
## Step 10 — Wiring, config, docs
|
||||
|
||||
The composition root and everything around it. `cmd/main.go` now: parses
|
||||
the plan's flag set (plus `--gc-allow-namespaced` from Step 9), loads the
|
||||
provider config first and fails fast, builds the registry with
|
||||
`{"kubernetes": kubernetes.New, "gcp": gcp.New}`, wraps every provider in
|
||||
`provider.WithMetrics`, then adds the lease store, health engine,
|
||||
discovery server, and GC sweeper to one manager and hands the reconciler
|
||||
its providers + health snapshotter + events channel. Metrics register on
|
||||
controller-runtime's global registry with scrape-time closures (phase
|
||||
counts from the cache, active leases summed from `store.Counts()`).
|
||||
|
||||
Two cache decisions became concrete here:
|
||||
|
||||
- The Secret cache is restricted to Secrets labelled
|
||||
`crawl.example.com/cloud-init=true` (new constant
|
||||
`v1alpha1.LabelCloudInit`) — the operator holds cluster-wide Secret
|
||||
read RBAC, and an unrestricted cache would hold every Secret in scope.
|
||||
Consequence documented in the README: an unlabelled referenced Secret
|
||||
is invisible → `CloudInitError`.
|
||||
- `--proxy-namespace` restricts the whole cache via `DefaultNamespaces`
|
||||
and flips the GC sweeper's `NamespaceRestricted` guard.
|
||||
|
||||
Manifests: `config/manager/manager.yaml` gained the
|
||||
`--providers-config` arg, the optional `DISCOVERY_TOKEN` secretKeyRef
|
||||
(`optional: true` — without the Secret the API runs unauthenticated with
|
||||
its loud warning), the ConfigMap volume mount, and containerPort 8090;
|
||||
new `config/manager/providers_config.yaml` (kubernetes-only default) and
|
||||
`config/default/discovery_service.yaml`. RBAC: the pods marker landed in
|
||||
the controller RBAC block (cluster-scoped role — the kubernetes
|
||||
provider's ListByTag spans namespaces). The plan's events RBAC was
|
||||
deliberately omitted: nothing wires an EventRecorder, and unused verbs
|
||||
are lint noise — recorded in Decisions.
|
||||
|
||||
Samples: `proxy_kubernetes.yaml` / `proxy_gcp.yaml` (with a working
|
||||
Squid-installing cloud-init) / `proxy_external.yaml` replace the scaffold
|
||||
placeholder; `providers-config.yaml` documents both provider blocks;
|
||||
`hack/providers-dev.yaml` + a new `run-dev` Makefile target run locally
|
||||
with plain-HTTP metrics:
|
||||
|
||||
```bash
|
||||
make run-dev
|
||||
# go run ./cmd/main.go --providers-config hack/providers-dev.yaml \
|
||||
# --metrics-bind-address :8080 --metrics-secure=false
|
||||
```
|
||||
|
||||
Docs: README rewritten per the plan (60-second architecture, kind
|
||||
quickstart, in-cluster deploy incl. token Secret creation, GCP setup with
|
||||
the IAM roles, the two prominent caveats, the no-substitutions version
|
||||
pins note). `docs/architecture.md` gained the components table and the
|
||||
full Decisions section — the plan's listed decisions plus everything
|
||||
accumulated in this log (RequeueNow, quota≠Failed, 409 arithmetic,
|
||||
TTL-400-not-clamp, GCP required placement, unknown-status→Stopped,
|
||||
banned==rate_limited window, GC logging convention, the Secret label
|
||||
contract, events-RBAC omission, logr-not-slog). `CHANGELOG.md` got its
|
||||
first entry with a real timestamp.
|
||||
|
||||
Verified: `make test` green across the repo (coverage unchanged),
|
||||
`bin/kustomize build config/default` and `config/samples` render clean,
|
||||
`make build` produces the binary, e2e-tagged build + vet clean. The kind
|
||||
end-to-end run is deliberately still ahead — it is the Verification
|
||||
step's job, after Step 11 closes the remaining test gaps.
|
||||
|
||||
Worth noting: `make run-dev` passes `--metrics-secure=false` because the
|
||||
scaffold's secure-serving default requires authn/authz reachability that
|
||||
a local process doesn't have; in-cluster deployments keep the secure
|
||||
default from the kustomize patch. The `providers` map wrapping happens
|
||||
*before* any consumer sees it, so the reconciler and GC only ever hold
|
||||
instrumented providers.
|
||||
|
||||
Reference in New Issue
Block a user