From 86b8d21be397c57b4443b90b6d70e08ecc126c53 Mon Sep 17 00:00:00 2001 From: kacerr Date: Wed, 16 Sep 2026 11:36:49 +0000 Subject: [PATCH] Turn docs/providers.md into an index and repoint its inbound links Co-Authored-By: Claude Fable 5.1 --- CLAUDE.md | 12 +- README.md | 9 +- docs/architecture.md | 2 +- .../2026-09-16-1108-provider-docs-split.md | 21 +- docs/providers.md | 769 +----------------- 5 files changed, 74 insertions(+), 739 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 62b750b..1026f73 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -27,10 +27,14 @@ entrypoint yet, so there's nothing to run locally. Documented in [docs/architecture.md](docs/architecture.md). It holds the component table, the reconcile flow, and one section per runnable: health engine, discovery and -lease API, orphan GC, metrics, tracing, and provider health checks. The provider -contract (MUST/SHOULD rules, feature matrix, checklist for adding a provider, known -gaps) is in [docs/providers.md](docs/providers.md). Provider-specific detail is in -`docs/kvm-provider.md` and the GCP docs; the test layout is in `docs/testing.md`. +lease API, orphan GC, metrics, tracing, and provider health checks. The generic +provider contract (MUST/SHOULD rules, behaviour profiles, checklist for adding a +provider, known limits) is in [docs/provider-contract.md](docs/provider-contract.md); +a new provider starts from [docs/provider-spec-template.md](docs/provider-spec-template.md); +how the existing providers meet the contract, with their gaps, is in +[docs/providers-existing.md](docs/providers-existing.md); [docs/providers.md](docs/providers.md) +indexes them. Provider-specific detail is in `docs/kvm-provider.md` and the GCP +docs; the test layout is in `docs/testing.md`. ## Conventions diff --git a/README.md b/README.md index 271f9fa..832b2f1 100644 --- a/README.md +++ b/README.md @@ -190,9 +190,12 @@ can be two differently-configured instances of the same `gcp` type (see ### Adding a provider -The contract, the rules each caller depends on, a feature matrix of the -existing providers and a step-by-step checklist are in -[docs/providers.md](docs/providers.md). Start there: several rules (a +The generic contract is [docs/provider-contract.md](docs/provider-contract.md), +the questionnaire a new provider's design fills in is +[docs/provider-spec-template.md](docs/provider-spec-template.md), and how the +existing providers meet the contract is +[docs/providers-existing.md](docs/providers-existing.md); +[docs/providers.md](docs/providers.md) indexes them. Start there: several rules (a self-contained `providerID`, `Get` returning `ErrNotFound` only when the instance is really gone, `CreatedAt` read back for orphan GC) are enforced by callers, not by the interface. diff --git a/docs/architecture.md b/docs/architecture.md index 983d69c..f1a9703 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -6,7 +6,7 @@ |---|---|---|---|---| | Proxy + ProviderHealth CRDs | `api/v1alpha1` | types | — | `Proxy` spec/status, CEL validation, defaulting, pure helpers; the status-only, cluster-scoped `ProviderHealth` | | Reconciler | `internal/controller` | controller | yes (with the manager) | the state machine: provision, replace, delete, represent health | -| Provider contract | `internal/provider` | library | — | `Provider` interface, optional `Checker`, error taxonomy, deterministic naming, config, metrics decorator; rules and per-provider matrix in [providers.md](providers.md) | +| Provider contract | `internal/provider` | library | — | `Provider` interface, optional `Checker`, error taxonomy, deterministic naming, config, metrics decorator; rules in [provider-contract.md](provider-contract.md), per-provider profile table in [providers-existing.md](providers-existing.md) | | 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 | | kvm provider | `internal/provider/kvm` | library | — | VMs on libvirt hosts over SSH-forwarded sockets; image cache, overlay disks, NoCloud seeds | diff --git a/docs/plans-executions/2026-09-16-1108-provider-docs-split.md b/docs/plans-executions/2026-09-16-1108-provider-docs-split.md index 3966a62..bbc56ab 100644 --- a/docs/plans-executions/2026-09-16-1108-provider-docs-split.md +++ b/docs/plans-executions/2026-09-16-1108-provider-docs-split.md @@ -6,7 +6,7 @@ Plan: [docs/plans/2026-09-16-1108-provider-docs-split.md](../plans/2026-09-16-11 - [x] Step 1 — Write `docs/provider-contract.md` - [x] Step 2 — Write `docs/provider-spec-template.md` - [x] Step 3 — Write `docs/providers-existing.md` -- [ ] Step 4 — Rewrite `docs/providers.md` as the index; link edits +- [x] Step 4 — Rewrite `docs/providers.md` as the index; link edits - [ ] Step 5 — Verification sweep - [ ] Step 6 — Final summary, CHANGELOG after confirmation @@ -121,3 +121,22 @@ Worth noting: matrix rows that the profile table now carries (Create non-blocking, GC marks, IP source, quota detection, Checker presence, credentials, logging, tracing, tests) were dropped from "Other differences" rather than kept twice. + +## Step 4 — Rewrite `docs/providers.md` as the index; link edits + +2026-09-16 11:40. `docs/providers.md` was rewritten in place (same path, so +the inbound links from `README.md`, `CLAUDE.md`, `docs/architecture.md` and +the historical plan files still resolve, and the pre-split blueprint stays +reachable in the path's git history). The three inbound sentences were +updated to name the new files: + +- `README.md` "### Adding a provider": the first sentence now names the + contract, the template and the appendix, with `docs/providers.md` as the + index; the "Start there" sentence is unchanged. +- `CLAUDE.md` "## Architecture": the providers.md sentence now names all + three files and the index. +- `docs/architecture.md` components row "Provider contract": "rules in + provider-contract.md, per-provider profile table in providers-existing.md". + +The historical files under `docs/plans/` and `docs/plans-executions/` keep +their `providers.md` links on purpose. diff --git a/docs/providers.md b/docs/providers.md index c94edaf..5fbb976 100644 --- a/docs/providers.md +++ b/docs/providers.md @@ -1,730 +1,39 @@ -# Providers: the blueprint - -This page is for whoever writes the next provider. It explains the contract a -provider must meet, which caller depends on each rule, how the three existing -providers (kubernetes, gcp, kvm) meet it, and what is still missing. - -Where the rest lives: - -- **Data flow:** the calls each provider makes are in - [architecture.md §5](architecture.md#5-what-provider-calls-do-in-the-outside-world). -- **Health checks:** the monitor that runs them is in - [architecture.md §11](architecture.md#11-provider-health-checks-internalproviderhealth). -- **Per-provider guides:** [kvm-provider.md](kvm-provider.md) and - [gcp-vm-validation.md](gcp-vm-validation.md). -- **Code:** the contract itself is - [`internal/provider/provider.go`](../internal/provider/provider.go), - [`errors.go`](../internal/provider/errors.go) and - [`check.go`](../internal/provider/check.go). - -Rule keywords: **MUST** means breaking the rule breaks a caller. **SHOULD** -means every current provider does it and a new one needs a reason not to. -**MAY** means it's allowed. - -Code references are `path:line` at the time of writing. When a line has moved, -look for the function named next to it. - -## 1. What a provider is - -A provider is a Go type implementing `provider.Provider`: four methods over one -VM-like instance, identified by an opaque `providerID` string. Nothing else in -the operator knows how a backend works: - -- **The reconciler** (`internal/controller`) calls `Create`, `Get` and `Delete` - and branches only on the error class and `Instance.State`. -- **Orphan GC** (`internal/gc`) calls `ListByTag` and `Delete`. -- **The provider health monitor** (`internal/providerhealth`) calls the optional - `Checker`. - -The reconciler never watches backend resources. Everything it knows comes from -polling `Get` (`ProvisioningPoll` and `DeletionPoll` 10s, `RequeueNow` 1s, -`QuotaRetry` 5m, `internal/controller/proxy_controller.go:452-467`). Up to 3 -Proxies reconcile at once (`MaxConcurrentReconciles: 3`, `:442`). - -Providers are named instances of a type. `gcp-eu` and `gcp-us` can both have -type `gcp`. The name is what `Proxy.spec.provider` refers to. `spec.provider` is -immutable, so a Proxy never moves between providers. - -**What the instance is for.** It is a forward proxy with its own egress path. -Two parties connect to `http://:`: - -- The health engine, from the operator pod, sends a real `CONNECT` through it - (`internal/health/engine.go:226`). -- Crawler clients, which get `ip` and `port` from the discovery API - (`internal/discovery/handlers.go:37`). - -So the IP a provider reports MUST be reachable on that port from the operator's -pod network and from the clients. That may require firewalls, security groups -or routing: gcp needs a manual firewall rule whose source range covers the -cluster's egress IP ([gcp-vm-validation.md](gcp-vm-validation.md)). - -Providers don't know which proxy software runs. The proxy is installed and -configured by the Proxy's `cloudInit`, and building install logic into the -operator is an explicit non-goal (`docs/prompts/__initial-prompt.md` §12). -kubernetes is the exception: a Pod has no cloud-init, so it generates a Squid -config from `Port` itself. - -**Who calls, and from where.** - -- **Leader only:** the reconciler, orphan GC, the health engine and the provider - health monitor. -- **Every replica, at startup:** the constructors run in `cmd/main.go` before - the manager starts. -- **Concurrently:** up to 3 reconciles, a GC sweep and a health-check round can - call the same provider at the same time. A provider **MUST be safe for - concurrent use**, including two calls for the same instance name. Nothing - above the provider serialises them. - -**Where it runs.** The manager container runs: - -- as non-root, with a **read-only root filesystem** - (`config/manager/manager.yaml:57`, `:122`); -- with a 64Mi `emptyDir` at `/tmp` (`:176`); -- with a **128Mi memory limit** (`:144`). - -So a provider: - -- MUST stream large payloads rather than buffering them (kvm streams image - downloads, `image.go:182`); -- MUST write scratch files only under `/tmp`; -- gets credentials either as mounted files (the providers ConfigMap at - `/etc/proxy-operator`, optional Secret mounts such as `kvm-ssh`) or from the - environment (ADC, the in-cluster service account). - -## 2. The contract, method by method - -### Create(ctx, CreateRequest) (providerID, error) - -`CreateRequest` carries: - -- `Name`: from `NameFromUID`, 22 characters, `proxy-` plus base32. -- `UID`: the owning Proxy's UID. -- `Namespace` and `ProxyName`. -- `Placement`: `Region`, `Zone`, `MachineType`, `Image`. -- `CloudInit`: already resolved from inline text or a Secret. -- `Port`. - -Rules: - -- **MUST be idempotent by `req.Name`.** A repeat call for a name that already - exists returns that instance's providerID with a nil error. It must not return - an error and must not create a duplicate. - *Why:* the reconciler records the ID in status only after `Create` returns - (`proxy_controller.go:155-172`). A crash between the two, or a status write - that is lost, means `Create` runs again for the same Proxy. `replaceInstance` - also depends on this to recover from a crash between its two writes - (`:240-244`). -- **MUST mark the instance as managed when it is created**, with - `LabelManaged=true` and `LabelUID=` or the backend's equivalent. kvm - uses domain `` because libvirt has no labels. - *Why:* orphan GC finds instances only through that mark (`internal/gc/gc.go:105`), - and never deletes one with an empty UID (`gc.go:113-117`). An instance that - exists for a while without the mark leaks if the operator crashes. -- **MUST return a providerID that is self-contained.** `Get` and `Delete` - receive only the ID, and the config or spec may have changed since `Create`, - for example a zone edit during a replacement. So put the location in the ID: - - kubernetes: `/` - - gcp: `zones//instances/` - - kvm: `hosts//domains/` - - Validate any config values that end up in the ID so they can't contain the - separator (`config_kvm.go:108-114`). -- **MUST honour `ctx`.** The reconcile context is cancelled on shutdown and when - leadership is lost. -- **SHOULD return once the request is submitted, not once the instance is - running.** Readiness is discovered by polling `Get`. A blocking `Create` holds - one of the 3 reconcile workers for its whole duration. kvm breaks this rule on - purpose, see §9. -- **SHOULD reject a request it can never satisfy** with `ErrPermanent` before - touching the backend: a missing zone, machine type or image, or a name that is - not in its catalogue (gcp `gcp.go:137-141`, kvm `kvm.go:126-144`, `:160-164`). - The CRD can't validate `placement`, because it is provider-agnostic, so this - error is the user's only feedback. The message SHOULD name the offending field - and value. -- **Catalogue changes don't replace instances.** The spec hash covers - `placement`, the resolved `cloudInit` and `port` (`spechash.go:15-19`), but not - the provider config. When a placement value names a config entry (kvm's - `images` and `machineTypes`), changing what that entry means affects only new - instances. For kvm it can even corrupt running VMs - ([kvm-provider.md § The image cache](kvm-provider.md#the-image-cache)). Document - this, and tell users to add a new key rather than edit one in use. -- **MAY ignore `Placement` fields that don't apply** (`provider.go:56-57`). If a - field is ignored, the provider's doc page MUST say so, because the fields are - still part of the spec hash. Changing an ignored field replaces the instance - and has no other effect. -- **SHOULD deliver `CloudInit` as user-data.** A provider that can't (kubernetes) - documents that it ignores the field. -- **MUST NOT log `CloudInit` contents.** It can hold secrets. Log its byte count - instead. gcp and kvm test this with a sentinel string (`kvm_test.go:975-995`, - `gcp_test.go:330-407`). - -### Get(ctx, providerID) (*Instance, error) - -- **MUST return `ErrNotFound` when, and only when, the instance is gone.** - *Why:* `ErrNotFound` triggers three actions: - - In normal reconciling, the reconciler clears the ID and creates again - (`proxy_controller.go:197-202`). - - During replacement, it is the signal to create the new instance - (`:237-251`). - - During deletion, it releases the finalizer (`:287-291`). - - A false NotFound during an outage creates a duplicate or loses the instance. - "Unreachable", "wrong credentials" and "can't tell" are Transient or - Permanent, never NotFound. -- **MUST keep returning the instance, not NotFound, while a delete is in - progress** and the name is still taken. - *Why:* the replacement instance gets the same name. The reconciler waits for - NotFound before creating it (`:228-233` comment, `:237-251`). A NotFound too - early would make `Create` adopt the dying instance. -- **MUST report `Running` only with a non-empty `IP`.** Running without an - address yet is `Provisioning`. - *Why:* the IP from a `Running` answer goes straight into status and the health - engine (`:212-215`). All three providers do this. -- **MUST report `Stopped` or `Terminated` only when the instance will not - recover by itself.** SHOULD map states it doesn't recognise to `Stopped`. - *Why:* the reconciler deletes and recreates on either state (`:216-226`); the - instances are treated as disposable. The two states cause exactly the same - reaction. -- **MUST fill `UID` and `CreatedAt` by reading them back from the backend**, - never from memory. - *Why:* GC compares `UID` with the live Proxies. It deletes anything whose - `CreatedAt` is older than `MinAge` (10m), and **a zero `CreatedAt` counts as - very old** (`gc.go:120`). A provider that forgets `CreatedAt` makes instances - still being created collectable. An unparsable timestamp gives the zero time - in both gcp and kvm. That is accepted because it never shields an orphan - forever (`gcp.go:253-256`). -- **SHOULD return `ErrPermanent` for an ID it can never resolve**, such as a - malformed ID or a host no longer in the config. - *Why:* the deletion path then tries `Delete` anyway and releases the - finalizer if that works (`:292-302`). If the provider returned Transient, - deletion would poll forever. - -### Delete(ctx, providerID) error - -- **MUST be idempotent.** Deleting something that is already gone returns nil. - *Why:* GC and the reconciler can race, and a crash can repeat a call. -- **MUST accept being called again while an earlier delete is still in - progress.** Treat "already deleting" as success, and SHOULD keep that cheap. - *Why:* while `Get` still returns the instance, the reconciler calls `Delete` - again on every `DeletionPoll` (10s) until `Get` returns NotFound (`:305-311`). - gcp submits a new delete operation each time. -- **MUST remove everything `Create` made**: disks, volumes, seeds, addresses. - Either delete them explicitly or rely on the backend doing it (gcp - `AutoDelete` on the boot disk). - *Why:* GC sees only what `ListByTag` returns, so secondary resources have no - other cleanup path. -- **MAY return before the instance is gone** (gcp, kubernetes), as long as `Get` - keeps showing it until it is (see Get). -- **SHOULD succeed for an ID whose `Get` fails permanently** when there is - nothing left it could reach. Example: kvm on a host that was removed from the - config returns nil (`kvm.go:466-476`). - *Why:* on the deletion path a permanent error isn't latched. It keeps - retrying (`deletionFailure`, `:349-359`), so a Proxy is only released through - the `crawl.example.com/skip-cleanup` annotation. - -### ListByTag(ctx) ([]Instance, error) - -- **MUST return only instances that carry the managed mark**, each with an `ID` - that `Delete` accepts, plus `UID` and `CreatedAt`. - *Why:* GC deletes every listed instance whose UID isn't live and whose age is - past `MinAge` (`gc.go:111-131`). Listing something without the mark is how an - operator deletes a VM it doesn't own. -- **MUST return an error rather than a list it can't vouch for.** An error makes - GC skip that provider for the sweep (`gc.go:105-110`). -- **MAY leave out instances it couldn't read.** Leaving something out is always - safe, because GC never deletes what isn't listed. The current providers make - different choices here: - - gcp silently leaves out unreachable zones (`ReturnPartialSuccess`, - `gcp.go:229`). - - kvm fails the whole list when one host is down but skips a single unreadable - domain (`kvm.go:540-565`, `:580-596`). - - kubernetes makes a single call. -- **SHOULD cover the whole scope the provider can create in**: every namespace, - zone or host. Anything outside that scope is never collected. -- **Scope caveat:** GC protects instances by UID, not by operator install. Two - installs, or two clusters, sharing one GCP project or hypervisor will delete - each other's instances ([gcp-vm-validation.md](gcp-vm-validation.md#gotchas)). - Two provider entries of one install that share a scope are safe, because they - share the live-UID set. - -## 3. Error taxonomy - -Every error a provider method returns MUST be built with -`provider.Wrap(class, op, providerName, id, err)`. `op` is one of `create`, -`get`, `delete` or `list`, and `providerName` is the **configured name** -(`cfg.Name`), not the type. - -`Wrap` keeps both `errors.Is(err, class)` and `errors.As` to the SDK error -working (`errors.go:53-62`). The shared metrics and tracing decorators label -results with the class (`metrics.go:61-74`, `tracing.go:67-80`). - -| Class | Use it for | During normal reconciling | While deleting | -|---|---|---|---| -| `ErrNotFound` | the instance doesn't exist | Get: recreate or advance (§2); other ops: returned as the error, so exponential backoff | Get: release the finalizer | -| `ErrQuotaExceeded` | quota or rate limit | `Provisioned=False` with reason `QuotaExceeded`, requeue after `QuotaRetry` (5m), not counted as an error (`:337-340`) | requeue after `QuotaRetry` | -| `ErrTransient` | network, 5xx, timeouts, cancellation, anything unknown | error returned, so workqueue exponential backoff | same | -| `ErrPermanent` | bad config, bad argument, auth or permission denied, malformed ID | **latched**: `Provisioned=False` with reason `PermanentError`, no more provider calls until `metadata.generation` changes (`:133-139`, `:341-343`) | error returned and retried forever; Get Permanent also triggers `Delete` (`:292-302`) | - -Pitfalls: - -- **Unclassified means Transient.** `provider.Class` returns `ErrTransient` for - any error not built with `Wrap` (`errors.go:71-91`). That is the safe default, - but a permanent failure that was never classified retries forever, and a quota - error backs off far too fast. -- **Permanent is sticky.** Use it only for errors that editing the Proxy spec or - fixing the config can resolve. A 403 that could be a quota problem is the hard - case: gcp checks the error reason (`gcp/errors.go:31-34`), while kubernetes - can't tell quota from RBAC and picks Permanent (`kubernetes.go:201-206`). -- **Don't return NotFound from `Create`.** The reconciler treats it as Transient - there, but it misleads the metrics and anyone reading logs. kvm can currently - do this (§9). -- **Classify in one `classify` function** with a table-driven test. All three - providers do. - -## 4. Identity, names and GC marks - -- **Names:** `provider.NameFromUID(uid)` gives `proxy-` plus 16 lowercase base32 - characters (`name.go:29-33`). It is valid as a GCP name, a DNS label and a - libvirt domain name. A backend with stricter rules SHOULD map it - deterministically rather than invent a new scheme, because idempotency depends - on the name being the same every time. -- **providerID:** opaque to everyone except the provider. Keep it - `/`-shaped, parse it strictly, and return `ErrPermanent` for - a malformed one (§2). -- **Marks:** `LabelManaged = "proxy-operator-managed"` with value `"true"`, and - `LabelUID = "proxy-operator-uid"` (`provider.go:28-32`). Use native labels or - tags where the backend has them. Otherwise store them in metadata that is - written in the same call that creates the instance (kvm: `metadata.go`). -- **`CreatedAt`:** read it from the backend's own creation timestamp. If the - backend has none, write it into the metadata at creation (kvm: - `kvm.go:277-298`). -- **Provider names:** keep a configured `name` a lowercase DNS-1123 label. - `spec.provider` allows at most 63 characters (`proxy_types.go:236`). The - `ProviderHealth` object is named after the provider, and the monitor skips - writing it for a name that isn't a valid object name (`monitor.go:141`, - `:248`). The metrics still work. Config loading doesn't check this yet (§9). - -## 5. Optional interfaces: `Checker` - -`provider.Checker` is `Check(ctx) []TargetStatus` (`check.go:37-46`). -`cmd/main.go` type-asserts it on the **unwrapped** provider (`cmd/main.go:216`) -before adding the decorators (`:219-222`). The monitor treats a nil Checker as -unsupported, not as failed (`monitor.go:212`). - -- **MUST never fail as a whole.** Every failure is a `TargetStatus` with - `Ready=false`. -- **MUST return promptly once `ctx` ends.** The monitor gives the whole round - one timeout (`monitor.go:180`). -- **MUST return one entry per target, in a stable order.** Targets are what can - fail independently: the apiserver, a project, each hypervisor. -- **MUST NOT disrupt reconciles.** A check that shares a connection with - provisioning must not tear that connection down when it times out. kvm uses - `hostPool.probe` for this, not `call` (`kvm/check.go:65`, `kvm/conn.go:213-242`). -- **SHOULD use the shared reasons** (`Reachable`, `Timeout`, `Unreachable`, - `Unauthorized`, `Forbidden`, `APIError`). Add provider-specific reasons only - for failures an admin would fix differently: kvm has `SSHHandshakeFailed`, - `PoolNotFound` and others. -- **SHOULD check with the same credentials, verb and scope that `ListByTag` and - `Create` use**, and be cheap: one page, one object. -- **MAY fill `Details`** with facts useful when triaging, such as versions or - capacity. Only kvm does today. - -## 6. Observability: tracing, logs and metrics - -Tracing is OpenTelemetry, set up in `internal/tracing` from the standard -`OTEL_*` environment variables. With no exporter configured, spans are no-ops -and logs are unchanged ([architecture.md §10](architecture.md#10-tracing-internaltracing)). -A provider has three layers to think about; the first comes for free. - -**1. Operation spans and metrics: the decorators, nothing to write.** -`cmd/main.go:219-222` wraps every provider in -`WithTracing(WithMetrics(...))`: - -- `provider.WithTracing` opens one client span per call, `provider.create`, - `provider.get`, `provider.delete` or `provider.list`, with `provider.name`, - `provider.id` and `provider.result` (the error class) as attributes - (`internal/provider/tracing.go:27-80`). `ErrNotFound` keeps the span status - Ok, because polling to NotFound is expected. -- `provider.WithMetrics` counts each call as - `proxy_operator_provider_requests_total{provider,op,result}` - (`metrics.go:15-74`, `internal/metrics/metrics.go:40-43`). A `Checker` feeds - `proxy_operator_provider_target_up`, `_check_duration_seconds` and - `_check_timestamp_seconds`, labelled `{provider,target}`. -- The spans sit under `Reconcile Proxy → reconcile.managed | - reconcile.replaceInstance | reconcile.delete`, or under `gc.sweep`. - -Rules: - -- **MUST NOT** open its own per-operation span or count its own requests. They - would duplicate the decorators. -- **MUST pass the caller's `ctx`** to every backend call and log through - `logf.FromContext(ctx)` **at call time**. The decorator starts the span with - `tracing.StartSpan`, which puts a logger carrying `traceID` and `spanID` into - `ctx` (`internal/tracing/logger.go:32-44`). A logger cached at construction, - or a backend call made with `context.Background()`, loses the trace. -- **Health checks bypass the decorators** (`cmd/main.go:211-218`). A - `Checker` runs under the monitor's `providerhealth.check` span - (`monitor.go:176`), and the same `ctx` rules apply to it. - -**2. Child spans for backend calls: SHOULD.** -One operation span says that `Create` took 40 seconds. Child spans say which -backend call took them. Every provider SHOULD produce one child span per backend -request, under the operation span: - -- **HTTP backends:** wrap the SDK's transport with `otelhttp`. Gate it on an - existing parent span, as `tracing.RestConfigWrapper` does - (`internal/tracing/transport.go:12-51`). Otherwise background traffic such as - watches, token refreshes or keepalives becomes root spans. Check first whether - the SDK already does this: Google's does. -- **Non-HTTP backends** (RPC over SSH, gRPC without interceptors, CLIs): start - child spans yourself with `tracing.Start(ctx, ".")`, around each - RPC and around long steps such as a dial or an image download. Use a - decorator over the SDK seam, not call-site code. -- **MUST NOT** put secrets or CloudInit contents in span attributes. Put - identifiers, sizes and error codes there. - -**3. Logs.** V(1): one line per operation outcome and per failed backend call. -V(2): curated detail. V(5): raw wire, if the backend has one. Log lines are -matched to traces by the `traceID` key from layer 1, which is how kvm's -per-RPC log lines join a trace today. - -What each provider does now: - -| Layer | kubernetes | gcp | kvm | -|---|---|---|---| -| 1. Operation spans and metrics | yes: decorators | yes: decorators | yes: decorators | -| 2. Child spans per backend call | yes: `tracing.RestConfigWrapper()` on its `rest.Config` (`cmd/main.go:196-200`, `kubernetes.go:58-60`), parent-gated | yes: the Google SDK adds `otelhttp` itself unless telemetry is disabled (`google.golang.org/api/transport/http/dial.go:308-312`); not parent-gated, but every call site has a parent | **no**: `kvm/trace.go` is a log decorator, not spans | -| 3. Logs carry `traceID`/`spanID` | n/a: the package doesn't log | yes, including the V(5) wire log (`gcp/wirelog.go:54-59`) | yes: every `libvirt call` line at V(1) | -| Tested against a real collector | yes: kind e2e checks the `provider.create` and `provider.delete` traces in Tempo (`test/e2e/tracing_test.go:200-238`), operation spans only | no | no | - -## 7. Feature matrix - -Legend: **yes** / **partial** / **no**, with a short note. The evidence is in -the per-provider notes below the table. - -| Feature | kubernetes | gcp | kvm | -|---|---|---|---| -| providerID self-contained | yes: `ns/name` | yes: `zones/z/instances/n`; project from config | yes: `hosts/h/domains/n` | -| Create idempotent | yes: AlreadyExists = success | yes: 409 = success | yes: lookup by name; overlay reused, seed rebuilt | -| Create verifies adopted instance's UID | no | no | no | -| Create non-blocking | yes | yes: operation not awaited | **no**: waits for `DomainCreate`; first use of an image downloads it | -| Placement used | none | Zone, MachineType, Image | Zone (host), MachineType, Image | -| CloudInit delivered | **no**: ignored | yes: `user-data` metadata | yes: NoCloud seed ISO | -| Port used | yes: Squid config and containerPort | no: firewall is manual | no | -| GC marks on create | yes: Pod labels | yes: instance labels | yes: domain ``; volumes unmarked | -| States produced | Provisioning, Running, Terminated | all four | Provisioning, Running, Stopped | -| IP source | PodIP | external NAT IP only | ARP, then guest agent; first IPv4 | -| UID / CreatedAt readback | yes | yes | yes | -| Delete idempotent | yes | yes | yes, at each step | -| Delete waits until gone | no | no | yes | -| Delete cleans secondary resources | n/a | yes: boot disk auto-deleted | yes: overlay and seed; base image kept | -| ListByTag scope | cluster-wide | one project, all zones | configured hosts | -| ListByTag on partial failure | n/a: single call | leaves out failed zones | whole list fails if a host is down; unreadable domain skipped | -| Quota detection | no | yes: 429 and 403 quota reasons | no | -| Error.Provider = configured name | **no**: hardcoded `"kubernetes"` | yes | yes | -| Checker | yes: `apiserver` | yes: the project | yes: each host | -| Checker Details | no | no | yes | -| Credentials | in-cluster config / kubeconfig | ADC | SSH key + known_hosts files, usually a Secret | -| Credential rotation without restart | n/a | via ADC | no: cached after first load | -| Connection handling | client-go | SDK client; SDK retries on Get and List | one pooled SSH+libvirt connection per host, keepalive | -| Logging | none in the package | V(1) ops, V(2) detail, wire log | V(1) ops and RPCs, V(2) detail | -| Tracing: operation spans (§6) | yes | yes | yes | -| Tracing: child spans per backend call | yes: parent-gated transport | yes: SDK's otelhttp | **no** | -| Unit tests with a fake | yes: controller-runtime fake client | yes: `fakeAPI` + httptest | yes: `fakeAPI` + in-process SSH server | -| Real-backend test | kind e2e | manual runbook | `//go:build integration` | -| Proxy software | generated Squid config (`pod.go:60-75`) | from `cloudInit`; image must run cloud-init (debian-cloud doesn't) | from `cloudInit`, plus the guest-agent vendor-data | -| IP reachable from the operator | Pod network, so in-cluster only (not from `make run-dev` on a laptop) | external IP, once a firewall rule allows the cluster's egress IP | LAN address on the host's bridge; the operator needs a route to that LAN | -| Distinct egress IP per proxy | no: the cluster's egress | yes: ephemeral external IP | whatever the host's LAN routes through; not per VM by itself | - -### kubernetes notes (`internal/provider/kubernetes`) - -- **ID:** `kubernetes.go:152-165`. An empty name part is not rejected. -- **Create:** `kubernetes.go:90-100` and `pod.go:23-50`. Only `Name`, - `Namespace`, `UID` and `Port` are used. Squid config is generated from the - port and passed through `SQUID_CONF` (`pod.go:38-75`). -- **State:** `stateFromPod`, `kubernetes.go:185-199`. Pending, or Running with - no PodIP, is Provisioning. Succeeded, Failed and Unknown are Terminated. A Pod - being deleted still reports its phase. -- **Delete:** `kubernetes.go:118-127`. ListByTag: `kubernetes.go:134-146`. -- **Errors:** `classify`, `kubernetes.go:201-218`. 429 and timeouts are - Transient. Forbidden, Invalid, BadRequest and Unauthorized are Permanent. - Nothing is Quota. -- **Check:** `check.go:21-53`. It lists at most one managed Pod cluster-wide, so - it proves read and list access, not create. -- **Config:** only `kubernetes.image` (`config.go:45-48`, default at - `kubernetes.go:28`). - -### gcp notes (`internal/provider/gcp`) - -- **ID:** `gcp.go:289-299`. -- **Create:** `gcp.go:135-167` and `insert.go:34-74`. `Operation.Wait` is never - called. There is one NIC with an ephemeral external IP, and the boot disk has - `AutoDelete`. -- **State:** `mapState`, `gcp.go:271-287`. RUNNING without a NAT IP is - Provisioning. Unknown statuses are Stopped. -- **Get:** `gcp.go:172-192`. Delete: `gcp.go:196-219`. ListByTag: - `gcp.go:224-244`. -- **Errors:** `errors.go:20-39`. Non-googleapi errors are Transient. 404 is - NotFound. 429 and 403 with reason `quotaExceeded` or `rateLimitExceeded` are - Quota. 400, 401 and other 403s are Permanent. -- **Check:** `check.go:30-78`. One `AggregatedList` page with `maxResults=1`. -- **Config:** `project` is required. `network`, `networkTag` and `diskSizeGb` - have defaults (`config.go:51-61`, `insert.go:12-30`). -- **Logging:** V(1) and V(2) tiers in `gcp.go`. The wire log in `wirelog.go` - runs at V(5), with payloads elided unless `--gcp-wire-log-full-payloads` is - set. - -### kvm notes (`internal/provider/kvm`) - -- **ID:** `id.go:11-21`. Host names are restricted by `config_kvm.go:108-114`. -- **Create:** `kvm.go:154-221`: - - Looks up the domain by name and starts it if needed (`ensureRunning`, - `:300-318`). - - Otherwise: base image (`image.go`), then overlay (reused if it exists, - `:225-245`), then seed (always rebuilt, `:249-275`), then define with - metadata (`:277-298`), then start. -- **Get:** `kvm.go:323-356`. States come from `mapState` (`:442-453`): nostate - is Provisioning, running or blocked without an IP is Provisioning, everything - else is Stopped. -- **Addresses:** `address` (`:397-428`) tries ARP first, then the guest agent. -- **Delete:** `kvm.go:461-538`. Destroy, undefine (with NVRAM, managed save and - snapshot metadata), then remove the overlay and seed by name. A host that is no - longer configured is a success. -- **ListByTag:** `kvm.go:545-596`. -- **Errors:** `errors.go:21-42`. No domain or no volume is NotFound. XML, - unsupported, invalid-argument, auth and access errors, and a missing pool, are - Permanent. Everything else is Transient, including SSH handshake and auth - failures. Nothing is Quota. -- **Check:** `check.go:41-139`. Runs concurrently per host through - `hostPool.probe`. Reasons are tagged by the dial stage. -- **Connections:** `conn.go`. One connection per host with a 15s keepalive. - The pool dials on first use; with provider checks on (the default), the - leader's first health check opens it at startup. On cancel, `call` closes the shared connection; `probe` abandons - the RPC and leaves the connection open. -- **Full guide:** [kvm-provider.md](kvm-provider.md). - -## 8. Adding a provider: checklist - -1. **Package:** create `internal/provider//` with the provider type and a - compile-time interface assertions: `var _ provider.Checker = (*Provider)(nil)` - as in `kubernetes/check.go:15`, and the same for `provider.Provider`, which - the existing providers don't assert yet. - - Put the backend SDK behind a small interface (gcp `instancesAPI`, kvm - `libvirtAPI`) so a fake can stand in for it in tests. - - Build the logger with - `logf.FromContext(ctx).WithName("").WithValues("provider", name)`. -2. **Config:** in `internal/provider/config.go`: - - Add a `Config` block and a `ProviderConfig` field. - - Add the block to `setBlocks`. - - Add the type to the `switch p.Type` allow-list (`config.go:102-108`). - - Add required-field validation, in its own `config_.go` if it is - large. - - Add rows to `config_test.go`. - - The file is parsed with `UnmarshalStrict`, so unknown keys already fail. -3. **Constructor:** `New(ctx, provider.ProviderConfig) (provider.Provider, - error)`, registered in the `registry.Build` map in `cmd/main.go:195-205`. - - A constructor error stops the manager. - - So fail only on config errors, and **don't dial the backend here**: one - unreachable hypervisor or API must not take down the other providers (kvm - logs a warning and connects lazily, `kvm.go:89-101`). -4. **Decorators:** nothing to do. `cmd/main.go:219-222` wraps every provider in - `WithTracing(WithMetrics(...))`. Don't count requests or open per-operation - spans yourself. -5. **Contract:** implement each rule in §2, and test each one against the fake: - - repeat `Create` - - `Get` after `Delete` - - `Delete` twice - - malformed ID - - unknown state mapped to `Stopped` - - `Running` requires an IP - - `UID` and `CreatedAt` read back - - `ListByTag` excludes unmarked instances -6. **Errors:** one `classify` plus a table test covering every class, including - "not an SDK error → Transient". Use `cfg.Name` in `provider.Wrap`. -7. **Checker:** shared reasons first, a table test for the reasons, and a test - that `Check` returns promptly with an expired `ctx`. -8. **Tracing and logging** (§6): - - Child spans for backend calls: an existing SDK instrumentation, a - parent-gated transport wrapper, or a span decorator over the SDK seam. - Add a test with an in-memory span recorder - (`sdktrace.NewTracerProvider(sdktrace.WithSpanProcessor(tracetest.NewSpanRecorder()))`, - as in `internal/provider/tracing_test.go`) asserting the spans are - children of the operation span. - - Take the logger from `ctx` on every call so log lines carry `traceID`. - - V(1): one line per operation outcome, plus a `logAPIError`-style line with - the raw backend error. - - V(2): request and response detail. - - Never put CloudInit contents in logs or span attributes; add a sentinel - test for it. -9. **Real-backend test:** behind `//go:build integration`, skipped when its - environment variables are unset. It runs the whole lifecycle: create, create - again, poll until Running, list, delete, delete again, and confirm it is gone - and cleaned up. See `kvm/kvm_integration_test.go` and - [testing.md](testing.md). -10. **RBAC:** if the provider talks to the cluster, add `+kubebuilder:rbac` - markers in their own comment block, not inside a doc comment. The kubernetes - provider's Pod markers live next to the reconciler's - (`internal/controller/proxy_controller.go:86-88`); `controller-gen` scans - `./...`, so either place works. Then run `make manifests` and check - `git diff config/rbac/role.yaml`. -11. **Deployment:** - - A credentials mount in `config/manager/manager.yaml`, marked - `optional: true` so the manager still starts without it (see `kvm-ssh`). - - A commented entry in `config/manager/providers_config.yaml` and - `config/samples/providers-config.yaml`. - - A `config/samples/proxy_.yaml` with a cloud-init that actually - installs a proxy on the sample image. - - Optionally, a Makefile helper for the Secret (`make kvm-ssh-secret`). -12. **Docs:** - - A `docs/-provider.md` covering prerequisites, config reference, - placement mapping, what lands on the backend, the health check, and known - gaps. - - A row in the architecture.md components table, and a block in §5 and - §11. If the provider adds spans, update the span topology in §10. - - A column in the §7 matrix above. - - A row in README.md "Implemented providers". -13. **Optional:** a demo script under `docs/demo/`. - -## 9. Gaps and proposed features - -These are listed only. Nothing below is designed or implemented. - -### Current gaps - -**Across providers and the contract** - -- **Adoption isn't verified.** `Create` adopts any existing instance with the - right name without checking its `LabelUID`. None of the three providers check - it. -- **The Create comment doesn't match kvm.** The contract says `Create` returns - as soon as the request is submitted (`provider.go:90-94`). kvm blocks until - the domain has started, and on first use of an image, for the whole download. -- **`Stopped` and `Terminated` are indistinguishable to the reconciler.** - kubernetes never produces Stopped, and kvm never produces Terminated. -- **Different ListByTag partial-failure policies.** gcp leaves failed parts out, - kvm fails the whole list. Both are safe for GC; the contract doesn't say which - to prefer. -- **Config is read once per process.** Adding a provider or host, or rotating - credentials, needs a manager restart. -- **Stale comments.** `provider.go:3-4` and `registry/registry.go:5-12` still - describe a `mock` subpackage, which was replaced by the kubernetes provider. - `spec.provider`'s doc (`proxy_types.go:232`, and so the generated CRD) still - gives `"mock"` as an example. -- **GC doesn't know about installations.** Two clusters sharing one GCP project - or hypervisor delete each other's instances, because GC only knows its own - live UIDs. This was a merge-blocker in the 2026-08-10 PR review - ([docs/reviews](reviews/2026-08-10-1134-proxy-operator-pr-review.md)) and is - still open. -- **An unconfigured provider latches the Proxy.** A Proxy naming a provider - missing from the config is latched `PermanentError`. Adding the provider and - restarting doesn't change `metadata.generation`, and `spec.provider` is - immutable, so the Proxy stays latched until it is recreated (same review). -- **Provider names aren't validated** as DNS-1123 at config load (§4). -- **The type list exists twice:** the config allow-list (`config.go:102-108`) - and the constructor map in `cmd/main.go:195-205` must be kept in step by hand. -- **Delete is resubmitted on every deletion poll** (§2). For gcp that is a new - API operation every 10s until the VM is gone. - -**kubernetes** - -- **Wrong provider name in errors.** `Error.Provider` is hardcoded to - `"kubernetes"` (`kubernetes.go:105-216`), so errors and logs show the wrong - name when the configured name differs. -- **Spec fields ignored.** `CloudInit` and all of `Placement` are ignored. -- **No quota class.** A quota 403 is Permanent. -- **Deleting Pods look healthy.** A Pod being deleted still reports its phase, - so it can read as `Running`. -- **Unguarded Pods.** No securityContext, no resources, and - `http_access allow all` (`pod.go`). -- **No logging** in the package, and **no Checker `Details`**. -- **No `OwnerReference` on Pods.** A bypassed finalizer leaves the Pod behind. -- **Pod RBAC is always installed:** cluster-wide `pods` create/delete ships even - when no kubernetes provider is configured (`proxy_controller.go:86-88`). - -**Tracing** - -- **kvm has no child spans.** A kvm trace shows `provider.create` and nothing - under it: no libvirt RPCs, no SSH dial, no image download, although that is - where the time goes. Correlation works only through the `traceID` on its V(1) - log lines, which needs debug logging on. -- **Only kubernetes is tested against a real collector**, and that test checks - only the operation spans. Nothing checks that gcp's SDK spans or the - kubernetes API spans actually nest under `provider.*`. - -**gcp** - -- **Asynchronous insert failures are never read.** An insert that the API - accepts but that fails later (quota, bad image) is never seen. The next `Get` - returns NotFound and the reconciler creates it again, so the reason never - reaches status. [gcp-vm-validation.md](gcp-vm-validation.md#gotchas) says - such errors do reach status. -- **External IP only.** A VM without an external NAT IP stays `Provisioning` - forever. -- **No firewall management.** The rule for the proxy port is a manual - prerequisite, and `req.Port` is unused. -- **The SDK client is never closed**, and `Placement.Region` is unused. -- **Wrong label value in the docs.** `gcp-vm-validation.md` filters on - `labels.proxy-operator-managed=yes`; the value is `true`. -- **The sample can't work.** `config/samples/proxy_gcp.yaml` uses a - `debian-cloud` image, which has no cloud-init, so its Squid install never runs - ([gcp-in-specific-project.md](gcp-in-specific-project.md) uses Ubuntu for that - reason). - -**kvm** (see also [kvm-provider.md § Known gaps](kvm-provider.md#known-gaps)) - -- **No quota or capacity detection.** -- **Create blocks**, including the first image download on each host. -- **GC blind spots.** Volumes without a domain are invisible to GC, and so are - VMs on a host removed from the config. `Delete` treats the latter as done. -- **SSH material is cached after the first successful load**, so rotating keys - needs a restart. -- **Cancelling one operation cuts off the others on that host.** A cancelled - `call` closes the shared per-host connection, aborting other operations in - flight on that host (`conn.go:199-211`). -- **SSH auth failures retry forever.** SSH authentication and host-key failures - are Transient, while libvirt auth errors are Permanent. -- **`Create` can return `ErrNotFound`** when a volume or domain disappears - during the call. -- **No reused-overlay check.** An existing overlay is reused without checking - its backing image or size. -- **Sample mismatch.** `config/samples/proxy_kvm.yaml` places on host `nas1`, - while both providers configs define only `beelink`. -- **Doc drift in kvm-provider.md:** - - It says ARP is the fallback; the code tries ARP first. - - It says `host` and `name` are on every log line; the check and list paths - lack them. - -### Proposed features (not implemented) - -- **Child spans in kvm:** extend the `tracingAPI` decorator in `kvm/trace.go`, - which already wraps every libvirt call, to also start a span per RPC, and add - spans for the SSH dial and the image download. The seam's methods take no - `ctx` today (`hostPool.call` hands `fn` only the API), so the span context has - to be plumbed in. -- **Tempo e2e checks for child spans**, and for gcp and kvm through their real - backend runbooks. -- **A shared conformance suite:** for example `internal/provider/providertest`, - which runs the §2 rules against any provider plus a fake backend, so a new - provider gets the contract tests for free. -- **Verify the UID when `Create` adopts an instance.** Return `ErrPermanent` on - a mismatch instead of adopting an instance that belongs to someone else. -- **Report asynchronous create failures:** gcp keeps the operation name, and - `Get` reports the failure instead of NotFound. -- **Capacity or quota signals** for kvm, such as pool free space and host - memory, mapped to `ErrQuotaExceeded`. -- **GC for secondary resources:** mark volumes and similar resources, and let - `ListByTag` or a second listing method cover them. -- **Reload credentials and config without a restart.** -- **Checker `Details`** for kubernetes (server version) and gcp (quota - headroom). -- **Internal-IP mode** for gcp, and a firewall rule managed per provider. -- **An installation-identity mark**, set on create and filtered on in - `ListByTag`, so installations sharing a backend don't collect each other's - instances. -- **A permanent-failure latch that sees provider availability**, so adding a - missing provider releases the Proxies waiting on it. -- **Validate provider names at load**, and derive the config type allow-list - from the constructor map. -- **Per-provider RBAC** as an optional kustomize component, and an - `OwnerReference` on kubernetes Pods. +# Providers + +The provider documentation is split into three files so that a new provider +can be specified and built from the generic rules alone, without reading how +the existing ones happen to work. + +- **[provider-contract.md](provider-contract.md)**: what every provider MUST, + SHOULD and MAY do, which caller depends on each rule, the behaviour profiles + where providers are allowed to differ, the error taxonomy, credential + hygiene, testing obligations, the checklist for adding a provider, and the + contract's known limits. It never names an existing provider. +- **[provider-spec-template.md](provider-spec-template.md)**: the questionnaire + a new provider's design answers, one entry per decision the contract leaves + open. Copy it to `docs/-provider-spec.md`, fill it in, and hand it to + the implementer with the other two files. +- **[providers-existing.md](providers-existing.md)**: how kubernetes, gcp and + kvm meet the contract, a table mapping each of them to every profile, their + per-provider reference notes, shared gaps and proposed features. + +Per-provider user guides: [kvm-provider.md](kvm-provider.md), +[gcp-vm-validation.md](gcp-vm-validation.md) and +[gcp-in-specific-project.md](gcp-in-specific-project.md). + +Related sections of [architecture.md](architecture.md): the components table, +[§5 what provider calls do](architecture.md#5-what-provider-calls-do-in-the-outside-world), +[§10 tracing](architecture.md#10-tracing-internaltracing), +[§11 provider health checks](architecture.md#11-provider-health-checks-internalproviderhealth) +and the "Decisions" list. The test layout is in [testing.md](testing.md). + +## Reading order for a new provider + +1. The contract, end to end. +2. The template, filled in for the new backend; every unanswered entry is a + design decision still to make. +3. The appendix, for a worked example of each profile the spec chose. +4. The user guide of the existing provider closest to the new one. + +The single-file blueprint that preceded this split is in the git history of +this path.