Provider docs: generic contract, spec template and appendix on the existing providers #7

Merged
kacerr merged 11 commits from feat/provider-blueprint into main 2026-09-16 13:48:40 +02:00
4 changed files with 595 additions and 4 deletions
Showing only changes of commit d96e56f1a0 - Show all commits
+4 -3
View File
@@ -27,9 +27,10 @@ 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. 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 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`.
## Conventions
+1 -1
View File
@@ -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 |
| 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) |
| 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 |
@@ -0,0 +1,70 @@
# Execution: provider blueprint
Plan: [docs/plans/2026-09-16-0841-provider-blueprint.md](../plans/2026-09-16-0841-provider-blueprint.md)
- [x] Step 0 — Save the plan; wait for the merge
- [x] Step 1 — Collect evidence (read-only, in parallel)
- [x] Step 2 — Write `docs/providers.md`
- [ ] Step 3 — Verify, then open the MR
## Step 0 — Save the plan; wait for the merge
The plan was committed on `feat/provider-health` (`80b2e9c`) and reached `main`
through PR #6. After the user confirmed the merge, the branch was cut from the
updated `main`:
```bash
git switch main && git pull --ff-only
git switch -c feat/provider-blueprint
```
Worth noting: `.claude/settings.json` has local, uncommitted permission edits
that carried over through the switch. They are deliberately left out of this
branch.
## Step 1 — Collect evidence (read-only, in parallel)
Three Explore agents answered the plan's 12-item questionnaire, one each for
kubernetes, gcp and kvm. Meanwhile I read the shared layer:
`internal/provider/*.go`, `registry.go`, the provider wiring in `cmd/main.go`,
`reconcileManaged`/`replaceInstance`/`reconcileDelete` in the controller,
`internal/gc/gc.go`, and how the monitor uses `Checker`.
Before relying on the agents' claims, I spot-checked them against the code. One
was wrong and did not go into the doc: the kvm agent said `Delete` skips volume
removal when `DomainDestroy` returns `ErrNoDomain`. In fact `removeDomain`
returns nil there, and `Delete` still calls `removeVolumes` afterwards
(`kvm.go:477-482`).
Rules that exist only in the callers, and not in any comment, were found by
reading the controller and GC. They went into the doc as MUST rules:
- A zero `CreatedAt` makes an instance immediately collectable (`gc.go:120`).
- `Get` returning `ErrPermanent` on the deletion path makes the reconciler call
`Delete` anyway.
- Replacement waits for `Get` to return NotFound before creating an instance
with the same name.
- `Stopped` and `Terminated` both lead to delete-and-recreate.
Worth noting, found outside this plan's scope and not changed here:
- `docs/gcp-vm-validation.md` filters on `labels.proxy-operator-managed=yes`,
but the value is `true`.
- According to the gcp agent, `docs/gcp-in-specific-project.md` holds a real
project ID, IAM output and an egress IP. The user was told.
## Step 2 — Write `docs/providers.md`
Wrote the eight sections the plan lists. Linked the new doc from the Provider
contract row in `docs/architecture.md` and from the CLAUDE.md "Architecture"
section.
Judgment calls:
- The file:line evidence sits in per-provider notes under the matrix, as
planned. The contract rules in §2 cite the reconciler and GC lines each rule
protects.
- The gap list includes documentation drift (kvm-provider.md's ARP order and
"from startup" dial, gcp-vm-validation.md's label value and its claim that
async errors reach status), as well as code gaps. Both would mislead the next
provider author in the same way.
- "Proposed features" stays a flat list with no design, as the user decided.
Worth noting: the line numbers in the doc are a snapshot. The doc says so and
asks readers to look for the named function when a line has moved.
+520
View File
@@ -0,0 +1,520 @@
# 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.
## 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=<req.UID>` or the backend's equivalent. kvm
uses domain `<metadata>` 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: `<namespace>/<name>`
- gcp: `zones/<zone>/instances/<name>`
- kvm: `hosts/<host>/domains/<name>`
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 §8.
- **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`).
- **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:* the reconciler calls `Delete` again on every poll until `Get` returns
NotFound (`:305-311`). GC and the reconciler can also race.
- **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 | condition `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**: condition `PermanentError`, no more provider calls until `metadata.generation` changes (`:130-135`, `: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 (§8).
- **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
`<location>/<name>`-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`).
## 5. Optional interfaces: `Checker`
`provider.Checker` is `Check(ctx) []TargetStatus` (`check.go:37-46`). The
monitor type-asserts it on the **unwrapped** provider, before `cmd/main.go` adds
the decorators (`cmd/main.go:211-222`). A provider without it shows up as
unsupported, not as failed.
- **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. 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 `<metadata>`; 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 |
| 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` |
### 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`. Dials lazily, one connection per host with a 15s
keepalive. On cancel, `call` closes the shared connection; `probe` abandons
the RPC and leaves the connection open.
- **Full guide:** [kvm-provider.md](kvm-provider.md).
## 7. Adding a provider: checklist
1. **Package:** create `internal/provider/<type>/` with the provider type and a
compile-time `var _ provider.Provider = (*Provider)(nil)` (and
`provider.Checker`).
- 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("<type>").WithValues("provider", name)`.
2. **Config:** in `internal/provider/config.go`:
- Add a `<Type>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_<type>.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. **Logging:**
- V(1): one line per operation outcome, plus a `logAPIError`-style line with
the raw backend error.
- V(2): request and response detail.
- Never log CloudInit contents; 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 package-level
`+kubebuilder:rbac` markers, then run `make manifests` and check
`git diff config/rbac/role.yaml`.
11. **Docs:**
- A `docs/<type>-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.
- A column in the §6 matrix above.
- A sample in `config/samples/`.
12. **Optional:** a demo script under `docs/demo/`.
## 8. 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.
**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`**.
**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`.
**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.
- **Doc drift in kvm-provider.md:**
- It says ARP is the fallback; the code tries ARP first.
- It describes a connection kept open "from startup"; the dial is lazy.
- It says `host` and `name` are on every log line; the check and list paths
lack them.
### Proposed features (not implemented)
- **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.