From 4bd9aae03bca6b4e136cc4a8236da0de36d3f2d5 Mon Sep 17 00:00:00 2001 From: kacerr Date: Wed, 16 Sep 2026 13:38:29 +0000 Subject: [PATCH] Write the kvm2 provider spec from the template Co-Authored-By: Claude Fable 5.1 --- docs/kvm2-provider-spec.md | 625 ++++++++++++++++++ .../2026-09-16-1333-kvm2-provider.md | 55 ++ 2 files changed, 680 insertions(+) create mode 100644 docs/kvm2-provider-spec.md create mode 100644 docs/plans-executions/2026-09-16-1333-kvm2-provider.md diff --git a/docs/kvm2-provider-spec.md b/docs/kvm2-provider-spec.md new file mode 100644 index 0000000..1b5f311 --- /dev/null +++ b/docs/kvm2-provider-spec.md @@ -0,0 +1,625 @@ +# Provider spec: kvm2 + +Filled from [provider-spec-template.md](provider-spec-template.md) against +[provider-contract.md](provider-contract.md). This is the design the `kvm2` +package is built from. Where an entry names an axis in backticks, the answer is +one of that axis's profiles in contract §4. Nothing in this spec was taken from +the code of the existing `kvm` provider; backend facts come from +[kvm-provider.md](kvm-provider.md), the execution log of the first build, the +go-libvirt module in the module cache, and the beelink host. + +## Header + +- **Type**: `kvm2` +- **Example configured name**: `kvm2-home` +- **Backend and API version this spec was written against**: libvirt 11.6.0 and + QEMU 10.1.0 on Ubuntu (host `beelink`), reached with go-libvirt + `github.com/digitalocean/go-libvirt v0.0.0-20260814190004-1a83157e1858` over an + SSH-forwarded unix socket. +- **Client**: `rpc-over-ssh`. go-libvirt's RPC client (`libvirt.New(conn)` + then `Connect()`) over a `direct-streamlocal` channel opened with + `golang.org/x/crypto/ssh` to the host's libvirt socket. Both modules are + already direct requires in `go.mod`; no new dependency. +- **Author and date**: Claude (orchestrator session), 2026-09-16. + +## 1. Identity and providerID + +**Answer:** `hosts//domains/`, where `` is `hosts[].name` +from the config block and `` is `req.Name`. Parsing is strict: exactly +four segments, literal `hosts` and `domains`, non-empty host and name, no +further slashes; anything else is `ErrPermanent` from `Get` and `Delete`. Host +names are validated at config load by the shared `kvmIdentifier` regexp in +`internal/provider/config_kvm.go` (letters, digits, dots, dashes, underscores), +so a host name can never contain the separator. Instances do not migrate +between hosts (libvirt domains are per host), so the location belongs in the +ID. + +**Evidence:** contract §6; `kvmIdentifier` in `internal/provider/config_kvm.go`. + +## 2. Primary key (`primary-key`) + +**Answer:** `name-is-key`. libvirt addresses domains by name +(`DomainLookupByName`); `req.Name` is used verbatim as the domain name and as +the base of the per-VM volume names. Create looks the name up first and adopts +an existing domain only if its metadata `uid` is empty or equals `req.UID`; a +different non-empty `uid` returns `ErrPermanent` naming both UIDs. This closes +the contract's "adoption is not verified" limit for this provider. + +**Evidence:** `DomainLookupByName(Name string) (Domain, error)` in the +go-libvirt module; contract §2.1 idempotency and §13. + +## 3. Placement mapping + +**Answer:** three fields are catalogue keys checked before any backend call, one is ignored: + +| Field | Meaning | Required | Unknown value | +|---|---|---|---| +| `Zone` | a `hosts[].name` | only when more than one host is configured; with exactly one host an empty zone selects it | `ErrPermanent`: `placement.zone "x" is not in kvm2.hosts` | +| `MachineType` | a key of `machineTypes` (`vcpus`, `memoryMiB`) | yes | `ErrPermanent`: `placement.machineType "x" is not in kvm2.machineTypes` | +| `Image` | a key of `images`, or an absolute http(s) URL used directly | yes | `ErrPermanent`: `placement.image "x" is neither a key in kvm2.images nor an http(s) URL` | +| `Region` | ignored | | still part of the spec hash: changing it replaces the VM | + +All three checks run before any backend call. The guide documents the ignored +field and that catalogue edits affect only new VMs. + +**Evidence:** contract §2.1 (reject early, MAY ignore with documentation); +`IsHTTPURL` in `internal/provider/config_kvm.go`. + +## 4. Image source + +**Answer:** a catalogue in the config block (`images..url`, optional +`sha256`, `format` qcow2 or raw, `firmware` uefi or bios) or a URL given +directly in `placement.image`. The provider downloads the image into the +host's storage pool on first use and caches it there as a base volume named +`base-.` (for a URL: `base-.`), +with a sidecar marker volume `base-..sha256` holding the digest of +the verified upload, written last. A base volume without a marker is +incomplete and is deleted and redone. When `sha256` is configured and differs +from the marker's content, the image is re-downloaded. The download streams +from the URL through the operator into `StorageVolUpload` with a running +sha256; nothing is buffered. `Content-Length` is required because the volume's +capacity must be declared before the upload; a response without it is +`ErrPermanent` naming the URL. Per `(host, volume)` a keyed mutex makes +concurrent Creates download once. The download runs on a dedicated +connection to the host, not the pooled one, so a cancelled poll on another +Proxy cannot cut it off. After a successful upload `StoragePoolRefresh` makes +the file usable as a backing image. + +Catalogue edit warning for the guide: overlays back onto the base file, so +replacing or evicting a base volume while Proxies use it corrupts those VMs; +add a new key instead. + +**Evidence:** kvm-provider.md "The image cache" and "Known gaps"; execution log +of 2026-09-04 (raw qcow2 upload plus pool refresh accepted by libvirt 11.6; +435 MB trixie image); contract §1.4 (stream, no buffering) and §2.1 +(catalogue changes). + +## 5. Cloud-init delivery + +**Answer:** a NoCloud seed ISO built in-process with +`github.com/kdomanski/iso9660`, volume label `cidata`, files `user-data` (the +Proxy's `CloudInit`, byte for byte, never parsed), `meta-data` +(`instance-id: ` and `local-hostname: `, so a reboot is never taken +for a new instance) and `vendor-data`. The ISO is written into a temp +directory created under the default temp dir (`/tmp`, a 64Mi emptyDir in the +manager), uploaded as a raw volume `-seed.iso` and attached as a CD-ROM. +The seed is rebuilt on every Create attempt: a previous attempt may have been +cut off mid-upload, and a truncated seed would boot a VM without cloud-init. + +`vendor-data` carries a shell script that installs and starts +`qemu-guest-agent` with whichever of apt-get, dnf, yum, zypper or apk the image +has. It is a script and not a cloud-config on purpose: cloud-init merges a +vendor-data cloud-config under the user-data with mergers that keep the +user-data's value for every top-level key both define and never append lists, +so a Proxy with its own `packages:` would silently lose the agent. Scripts +bypass the merge. When the config's `vendorData` is set, the two travel as +separate parts of a MIME multipart (`text/plain` for the configured document, +`text/x-shellscript` for the agent script), headed by `MIME-Version` within +the first bytes. With `installGuestAgent: false` and no `vendorData` the file +is empty. Size: cloud-init has no practical limit for NoCloud; the ISO is a +few hundred KB. + +**Evidence:** kvm-provider.md "What lands on the host"; execution log run 7 +(vendor-data as YAML silently dropped) and run 8 (script survives); contract +§2.1 (byte for byte, never logged) and §11 (sentinel test). + +## 6. Machine type mapping + +**Answer:** a catalogue: `machineTypes.` is `{vcpus, memoryMiB}`, +validated at load (both greater than zero, at least one profile). Disk size is +provider-wide: `diskSizeGb` (default 10) is the capacity of every VM's overlay +volume, thin-provisioned; cloud-init's growpart expands the root filesystem +into it. + +**Evidence:** `KVMMachineType` and `validateKVM` in +`internal/provider/config_kvm.go`. + +## 7. Network and IP discovery (`readiness`, `ip-source`) + +**Answer:** `readiness`: `provisioning-window` (boot plus cloud-init, plus the +image download on a cold pool). `ip-source`: `discovered`. The VM gets one +virtio NIC on `hosts[].bridge`, which must lead to a LAN with DHCP that the +cluster can route to. The address comes from `DomainInterfaceAddresses`, guest +agent source first, then the ARP table (on beelink the agent answered about +20 s after start where ARP never did, because the host itself does not talk to +the guest). The first IPv4 that is not loopback, link-local or unspecified is +reported; errors from either source mean "no address yet" and `Get` returns +`StateProvisioning`. The guest needs the agent: it is installed by the +vendor-data script (entry 5) unless `installGuestAgent: false`, which needs an +image that ships it or a network where ARP sees guest traffic. `Port` is not +used by the provider; nothing on the host filters it. Reachability is a +prerequisite in the guide: the operator's pod network and the clients must +route to the bridge's LAN. + +**Evidence:** kvm-provider.md "Hypervisor prerequisites" and "Known gaps" +(IP discovery); execution log run 6 (agent 20 s, ARP never); contract §1.3 +and §2.2 (Running only with IP). + +## 8. GC marks and CreatedAt (`gc-mark`, `created-at`) + +**Answer:** `gc-mark`: `written-metadata`. `created-at`: `written-at-create`. +The domain XML passed to `DomainDefineXML` carries, under ``, an +element `` +with children `true`, `req.UID` and +`RFC3339 UTC`. The mark lands in the same call that +defines the domain, so there is no mark window. Read back with +`DomainGetMetadata(dom, DomainMetadataElement, uri, DomainAffectConfig)`; +libvirt 11.6 returns the element without its namespace declaration, so the +parser matches on the local name `instance` only (the URI was already used to +select it). A domain without the element is not managed and reports an empty +UID. A `createdAt` that does not parse gives the zero time, which GC treats as +old; that is accepted. The volumes carry no mark (documented GC blind spot, +entry 24). The namespace is kvm2's own, so kvm2 never lists or adopts domains +made by the `kvm` provider; the two types may share a host. + +**Evidence:** execution log run 3 (namespace asymmetry); +`DomainGetMetadata(Dom Domain, Type int32, Uri OptString, Flags DomainModificationImpact)` +in the go-libvirt module; contract §2.1 (mark in the create call) and §4.5. + +## 9. State mapping (`provisioning-state`) + +**Answer:** `provisioning-state`: `create-blocks`. Create defines and starts the +domain in one call and returns only once `DomainGetState` reports running, so +`Get` never sees a domain kvm2 defined but did not start. A domain that is +shut off later stopped on its own or was stopped by someone, and Stopped is +the right answer. + +| Backend state (`DomainGetState`) | Lock or task | Provider state | +|---|---|---| +| nostate (0) | none | Provisioning (libvirt could not determine it; next poll decides) | +| running (1), blocked (2) | no IP found | Provisioning | +| running (1), blocked (2) | IP found | Running | +| paused (3), shutdown (4), shutoff (5), crashed (6), pmsuspended (7) | none | Stopped | +| any other code | none | Stopped | + +libvirt has no locks or tasks; every RPC is synchronous. + +**Evidence:** go-libvirt `DomainState` constants; contract §2.2 and §4.11. + +## 10. Create: blocking and task tracking (`create-return`, `async-ops`) + +**Answer:** `create-return`: `blocking`. `async-ops`: `none`. Create returns +when the domain is running, holding one reconcile worker for the duration, +including the first image download on a host (minutes on a cold pool; the +guide says so). The wait after `DomainCreate` is capped at 2 minutes; every +RPC has its own deadline (entry 18). + +Steps, each idempotent, in order, all under the per-name lock (entry 19): + +1. Validate placement (entry 3). No backend call yet. +2. `DomainLookupByName`. Found: adoption guard (entry 2), then start it if it + is not running (`DomainCreate`; `ErrOperationInvalid` means already + running), return the ID. +3. Pool space precheck (entry 13, quota). +4. Ensure the base image (entry 4): marker present and matching means done. +5. Overlay volume `.qcow2`, qcow2 backed by the base file with + `diskSizeGb` capacity; `ErrStorageVolExist` means reuse (creation is + atomic, so an existing overlay is complete). +6. Seed volume `-seed.iso`: delete if it exists, create, upload + (entry 5). Always rebuilt. +7. `StorageVolGetPath` for the overlay and the seed. +8. `DomainDefineXML` with file-type disks (entry 15's AppArmor note), the + metadata element, firmware per image. +9. `DomainCreate`, then poll `DomainGetState` until running. +10. Return `hosts//domains/`. + +A retry after a crash finds whatever the previous attempt got to: an existing +overlay is reused, a seed is rebuilt, a defined domain is started, a running +domain is returned. `ErrNoDomain` and `ErrNoStorageVol` inside Create (a +volume vanishing between steps) are `ErrTransient`, never `ErrNotFound`. The +log line for a Create records `cloudInitBytes`, never the content. + +**Evidence:** contract §2.1, §4.2, §5 ("do not return NotFound from Create"); +execution log run 1 (AppArmor). + +## 11. Delete and secondary resources (`delete-return`, `secondary-resources`) + +**Answer:** `delete-return`: `waits`. `secondary-resources`: `explicit`. Under +the per-name lock: + +1. Parse the ID (`ErrPermanent` if malformed). A host no longer in the config + is a logged no-op success: the provider cannot reach it and refusing would + pin the finalizer; the guide tells admins to delete Proxies before retiring + a host. +2. `DomainLookupByName`; `ErrNoDomain` means the domain is already gone, skip + to step 5. +3. `DomainDestroy`; `ErrOperationInvalid` (not running) and `ErrNoDomain` are + success. +4. `DomainUndefineFlags` with `DomainUndefineNvram`, + `DomainUndefineManagedSave` and `DomainUndefineSnapshotsMetadata` + (UEFI domains keep NVRAM; undefine refuses without the flag); `ErrNoDomain` + is success. +5. Delete `.qcow2` and `-seed.iso` by name from the host's pool; + `ErrNoStorageVol` is success, and `ErrNoStoragePool` means nothing is left + to clean. +6. Return nil. The base image and its marker stay (shared cache). + +Everything is gone when Delete returns nil, so `Get` afterwards is NotFound. +A second Delete during the first waits on the lock and then finds nothing. +Volumes that exist without a domain (a crash between step 5 and 8 of Create) +are removed by the same Proxy's next Delete but are invisible to GC (entry 24). + +**Evidence:** kvm-provider.md "Known gaps"; execution log run 4 (NVRAM); +contract §2.3. + +## 12. ListByTag scope and partial failure (`list-partial`) + +**Answer:** `fail-whole`, per host, with single unreadable domains skipped. +Scope: every host in `hosts`, in config order, `ConnectListAllDomains` with +active and inactive domains. For each domain, read the metadata element; keep +those with `managed` true, then read state and addresses for the `Instance`. +A domain whose metadata or state cannot be read (mid teardown, corrupt +element) is skipped with a V(1) line; a host that cannot be dialled or whose +listing fails fails the whole call, so GC skips this provider for the sweep +rather than act on a partial view. Filtering is client-side; libvirt has no +label index. Cost: one `DomainGetMetadata` per domain per sweep, about half a +millisecond each over the tunnel. + +**Evidence:** execution log run 6 (per-domain metadata cost); contract §2.4 +and §4.7. + +## 13. Error classification (`not-found`, `quota`) + +**Answer:** `not-found`: `error-code`. `quota`: `detected`, from a pool-space +precheck rather than a backend error: before creating any volume, Create reads +`StoragePoolGetInfo` and compares `available` with the bytes it is about to +commit: the image's `Content-Length` when the base is not cached, plus +`diskSizeGb` in bytes, plus 16 MiB for the seed. Below that it returns +`ErrQuotaExceeded` naming the pool, available and needed, and the reconciler +waits `QuotaRetry`. + +One `classify(op string, err error)`: + +| Backend response | Class | Why | +|---|---|---| +| `libvirt.Error` code `ErrNoDomain`, `ErrNoStorageVol` | NotFound (Transient when `op == "create"`) | the object is gone; inside Create it vanished mid-call | +| `ErrNoStoragePool` | Permanent | config names a pool that does not exist | +| `ErrXMLError`, `ErrNoSupport`, `ErrInvalidArg` | Permanent | the request itself is wrong | +| `ErrAuthFailed`, `ErrAccessDenied` | Permanent | the SSH user may not use the socket | +| `ErrOperationInvalid` | Transient (Delete and Create treat the specific cases as success before classify) | state race; the next poll decides | +| any other `libvirt.Error` | Transient | unknown means retry | +| SSH: `ssh: unable to authenticate` (from `x/crypto/ssh`) | Permanent | the key is not in `authorized_keys` | +| SSH: `knownhosts.KeyError` with a known key that differs, or with no known key | Permanent | host key changed or missing from known_hosts; the admin must act | +| SSH key or known_hosts file missing (`fs.ErrNotExist`) | Transient | the Secret has not landed yet | +| SSH key present but does not parse | Permanent | wrong file in the Secret | +| `context.Canceled`, `context.DeadlineExceeded`, `net.OpError`, `io.EOF`, `io.ErrUnexpectedEOF` | Transient | network or cancellation | +| HTTP 404, 403, 410 on the image URL; missing `Content-Length`; sha256 mismatch | Permanent | the catalogue entry is wrong | +| HTTP 5xx, 429, network errors on the image URL | Transient | retry | +| an error already wrapped with a class (`errors.Is` any sentinel) | that class | validation and builders classify at the source | +| anything else | Transient | contract default | + +The unit test table uses a `libvirt.Error` per code, the real SSH error +strings produced by `x/crypto/ssh` against the in-process server, and a real +`knownhosts.KeyError`. + +**Evidence:** go-libvirt `ErrorNumber` constants and `Error{Code uint32, Message}` +(value receiver, so `errors.As` uses a `libvirt.Error` value); contract §5; +kvm-provider.md "Known gaps" (SSH auth retried forever in v1: closed here). + +## 14. Credentials and TLS (`credentials`) + +**Answer:** `mounted-files`. The config block names `sshKeyFile` (private key, +OpenSSH or PEM, parsed with `ssh.ParsePrivateKey`) and `knownHostsFile` +(OpenSSH `known_hosts`, parsed with `knownhosts.New`), both expected under the +existing optional `kvm-ssh` Secret mount at `/etc/proxy-operator/kvm` +(`config/manager/manager.yaml`; the mount is shared with the `kvm` provider and +its comment says so). `insecureIgnoreHostKey: true` replaces known_hosts with +`ssh.InsecureIgnoreHostKey()` and is logged as a warning by `New`. Both files +are read on every dial, so rotating the Secret takes effect on the next +connection without a restart. `New` tries to read them once to log a warning +when they are unusable, but never fails on them and never dials. + +Minimum privilege: the SSH user must be able to open the libvirt socket +(member of the `libvirt` group); libvirt itself is used as `qemu:///system` +through the socket, so no further host permissions. + +TLS: none (SSH provides transport security); `system-roots` for the image +download's HTTPS, which uses the distroless image's CA bundle. + +Hygiene: the private key bytes and the known_hosts content never appear in +logs, spans or error strings; errors from parsing say "parsing ssh key file +" plus the parser's message. Tested with a sentinel placed in a bad key +file. + +**Evidence:** kvm-provider.md "Cluster-side setup"; execution log run 8 +(lazy load: a missing Secret must not abort the provider set); contract §7. + +## 15. Config block + +**Answer:** the block is `kvm2:` and reuses the Go type `provider.KVMConfig` +from `internal/provider/config_kvm.go` unchanged, so an existing `kvm-home` +entry ports by changing `type: kvm` to `type: kvm2` and `kvm:` to `kvm2:`. +Validation calls the shared validator with the block name `kvm2` so messages +read `kvm2.hosts` and not `kvm.hosts`. + +| Field | Required | Default | Validation | Ends up in | +|---|---|---|---|---| +| `sshKeyFile` | yes | | non-empty | dial | +| `knownHostsFile` | unless `insecureIgnoreHostKey` | | non-empty | dial | +| `insecureIgnoreHostKey` | | false | | dial, warning at `New` | +| `diskSizeGb` | | 10 | not negative | overlay capacity, quota precheck | +| `vendorData` | | | | seed `vendor-data` | +| `installGuestAgent` | | true | | seed `vendor-data` | +| `images.` | | | key matches `kvmIdentifier` | volume name `base-.` | +| `images..url` | yes per entry | | http(s) URL | download | +| `images..sha256` | | | 64 hex | marker comparison | +| `images..format` | | qcow2 | qcow2 or raw | base volume XML, overlay backing format | +| `images..firmware` | | uefi | uefi or bios | domain XML `` | +| `machineTypes.` | at least one | | vcpus and memoryMiB greater than zero | domain XML | +| `hosts[].name` | yes | | unique, matches `kvmIdentifier` | providerID segment | +| `hosts[].address` | yes | | non-empty `host:port` | TCP dial | +| `hosts[].user` | yes | | non-empty | SSH | +| `hosts[].socket` | | `/var/run/libvirt/libvirt-sock` | | SSH channel target | +| `hosts[].pool` | | `default` | | every storage RPC | +| `hosts[].bridge` | yes | | non-empty | domain XML NIC | + +Wiring: `ProviderConfig.KVM2 *KVMConfig` with tag `kvm2,omitempty`; +`setBlocks` reports `kvm2`; the allow-list in `Config.validate` and the +constructor map in `cmd/main.go` both gain `kvm2` in the same commit. + +**Evidence:** `KVMConfig`, `validateKVM`, `kvmIdentifier` in +`internal/provider/config_kvm.go`; contract §10. + +## 16. Checker + +**Answer:** one target per `hosts[]` entry, `Name` the host name, `Address` +the SSH address, checked concurrently in config order on a fresh dedicated +connection each time (dial, `ConnectGetLibVersion`, `ConnectGetHostname`, +`StoragePoolLookupByName`, `StoragePoolGetInfo`, close). It never touches the +pooled connection, so a check that runs out of time closes only its own +connection and cannot disrupt a reconcile. Cost: one SSH handshake and four +cheap RPCs per host per round. + +Reasons: `Reachable`; `Timeout` (ctx ended before the check finished); +`Unreachable` (TCP connect or the SSH transport failed, or the link died +mid-check); `Unauthorized` (SSH authentication refused, host key unknown or +changed); `Forbidden` (`ErrAccessDenied` or `ErrAuthFailed` from libvirt: the +socket refused the user); `APIError` (any other libvirt error); +`CredentialsUnavailable` (the key or known_hosts file is missing or +unparsable, i.e. the Secret is absent or wrong: fixed in the cluster, not on +the host); `PoolNotFound`; `PoolNotActive`. + +Details on success: `user`, `hostname`, `libvirtVersion` (decoded +major.minor.micro), `pool`, `poolState`, `poolCapacityGiB`, `poolAvailableGiB` +(the closest thing to a capacity signal, and the same number the quota +precheck reads). The `status.type` comment in +`api/v1alpha1/providerhealth_types.go` gains `kvm2` and the CRD is +regenerated. + +**Evidence:** contract §8; `StoragePoolGetInfo` returning state, capacity, +allocation, available; `ConnectGetLibVersion` returning major*1e6 + minor*1e3 ++ micro. + +## 17. Client and child spans (`client-kind`, `child-spans`) + +**Answer:** `client-kind`: `rpc-over-ssh`. `child-spans`: `span-decorator`. + +The seam is an interface over exactly the go-libvirt methods the provider +uses, with `ctx` as the first parameter of every method (go-libvirt's own +methods take none and block): `DomainLookupByName`, `DomainDefineXML`, +`DomainCreate`, `DomainDestroy`, `DomainUndefineFlags`, `DomainGetState`, +`DomainGetMetadata`, `DomainInterfaceAddresses`, `ConnectListAllDomains`, +`StoragePoolLookupByName`, `StoragePoolRefresh`, `StoragePoolGetInfo`, +`StorageVolLookupByName`, `StorageVolGetPath`, `StorageVolCreateXML`, +`StorageVolUpload`, `StorageVolDownload`, `StorageVolDelete`, +`ConnectGetLibVersion`, `ConnectGetHostname`, plus `Close`. One production +adapter wraps `*libvirt.Libvirt` and applies the per-call deadline +(entry 18); one in-memory fake implements the seam for unit tests. A dialer +seam (`dial(ctx, host) (backend, error)`) separates the SSH transport so tests +inject fakes and an in-process SSH server test covers the real dialer. + +The decorator is applied once, inside the real dialer, at the point where a +connection is created, so provider code never sees an undecorated backend. Per +call it starts a span `kvm2.` (for example `kvm2.DomainDefineXML`) with +attributes `kvm2.host` and the identifiers passed (`kvm2.domain`, `kvm2.pool`, +`kvm2.volume`), records the libvirt error code on failure, and inside the span +writes the V(1) log line, so the line carries that span's ID. Two more spans: +`kvm2.dial` around the SSH handshake and libvirt connect (attributes address +and user) and `kvm2.download` around an image fetch (attributes URL host, +bytes, whether a sha256 was configured). All spans are children of the +decorator's `provider.create`, `provider.get`, `provider.delete`, +`provider.list` or the monitor's `providerhealth.check`. No CloudInit, key +material or upload bodies in attributes. A reflection test walks the seam +interface and asserts every method produces exactly one span and one log +line. + +**Evidence:** contract §4.15, §4.18, §9; `tracing.Start` in +`internal/tracing/logger.go`. + +## 18. Timeouts (`timeouts`) + +**Answer:** `per-call`. The adapter runs each blocking go-libvirt call in a +goroutine and waits on the result, `ctx.Done()` and a timer: + +| Call | Budget | +|---|---| +| TCP connect plus SSH handshake plus libvirt connect | 10 s | +| ordinary RPC | 30 s | +| `DomainCreate` | 90 s | +| `StorageVolUpload` (seed) | 60 s | +| `StorageVolUpload` (image), HTTP download | ctx, capped at 20 m | +| wait for running after `DomainCreate` | 2 m | +| HTTP client for images | no client timeout; the per-request ctx carries the cap | + +When a call is abandoned (ctx cancelled or budget exceeded), the adapter +closes the underlying connection, because that is the only way to unblock +go-libvirt, and marks it dead so the next call redials; the abandoned call +returns `ErrTransient` wrapping the ctx or deadline error. Consequence, +documented in the guide: other RPCs in flight on that host's pooled connection +fail once with Transient and their callers retry. The image download uses a +dedicated connection so the expensive case (a cancelled poll killing a +435 MB upload) cannot happen. The keepalive (`keepalive@openssh.com` request +every 15 s) marks the connection dead on failure. + +**Evidence:** contract §1.5 (no caller deadline; a hung ListByTag stalls GC), +§4.16; go-libvirt methods take no ctx. + +## 19. Concurrency + +**Answer:** shared state is the host connection table (one pooled connection +per host, dialled lazily, under a mutex with a per-host dial mutex so +concurrent callers dial once and share the result), the per-`(host, volume)` +download locks, and a keyed mutex per instance name. Create and Delete for the +same name run under that name's lock, so two Deletes (GC and the reconciler) +never race on destroy and undefine, and a Delete cannot interleave with a +Create's steps. Get and ListByTag take no name lock. go-libvirt multiplexes +RPCs on one connection, so calls for different names proceed concurrently +on the shared connection. Three reconciles, a GC sweep and a check round at +once are safe; the check uses its own connections. + +**Evidence:** contract §1.2 (concurrency), §4.15. + +## 20. Logging (`logging`) + +**Answer:** `tiered`. Logger from `logf.FromContext(ctx)` on every call, +named `kvm2`, with `provider` on every line and `host` and `name` on every +line about an instance. + +- V(1): one line per operation outcome (`kvm2 domain ensured running`, + `kvm2 domain fetched` with state, ip and uid, `kvm2 domain deleted`, + `kvm2 domains listed` with counts), one `libvirt call` line per RPC from the + decorator (`rpc`, identifiers, `duration`, on failure `error`, `libvirtCode` + and `libvirtError` name), connection events (dialled, dropped with reason), + and a line per failed backend call with the raw error. +- V(2): the same lines also carry domain and volume XML, the metadata element, + interface addresses, listed domain names, and the resolved Create parameters + (`cloudInitBytes`, never the content). +- No V(5) wire tier; go-libvirt exposes no hook for it. +- Never logged at any level: `CloudInit`, key material, known_hosts content, + upload or download bodies. + +**Evidence:** contract §4.17, §9. + +## 21. Tests (`backend-test`) + +**Answer:** `backend-test`: `integration-tag`. Unit tests run over the +in-memory fake behind the seam (and an in-process `x/crypto/ssh` server for +the dialer); the integration test is `kvm2_integration_test.go` behind +`//go:build integration`, skipped unless `KVM2_TEST_HOST`, `KVM2_TEST_USER`, +`KVM2_TEST_SSH_KEY`, `KVM2_TEST_BRIDGE` and `KVM2_TEST_IMAGE_URL` are set, +optional `KVM2_TEST_KNOWN_HOSTS` (without it host keys are not verified and the +log says so), `KVM2_TEST_IMAGE_SHA256`, `KVM2_TEST_POOL`, `KVM2_TEST_SOCKET`, +`KVM2_TEST_TIMEOUT` (default 20m), `KVM2_TEST_KEEP`, `KVM2_TEST_LOG_LEVEL`; +run with `make test-integration-kvm2`. It asserts, in order: `New` returns +without dialling; `Check` reports the host Ready with details; `Create` +returns `hosts//domains/`; a second `Create` returns the same ID +and the host's domain count is unchanged; `Get` reaches Running with an IPv4 +within the budget and its `UID` and `CreatedAt` equal what Create wrote; +`ListByTag` contains it; `Delete` returns nil; `Get` is `ErrNotFound`; +`Delete` again returns nil; the pool holds the base image and its marker and +no `.qcow2` or `-seed.iso`; with an in-memory span recorder, +`kvm2.DomainDefineXML` and `kvm2.dial` spans are children of +`provider.create`. + +Contract §11 list, with the test that covers each item: + +- [x] repeat `Create`: `TestCreate_repeatReturnsSameIDAndCreatesNothing` +- [x] `Get` after `Delete`: `TestGet_afterDeleteReturnsNotFound` +- [x] `Delete` twice: `TestDelete_twiceSucceeds` +- [x] malformed ID: `TestGet_malformedIDIsPermanent`, `TestDelete_malformedIDIsPermanent`, `TestParseProviderID_malformed` +- [x] unknown state maps to `StateStopped`: `TestMapState_table` +- [x] `StateRunning` requires an IP: `TestGet_runningWithoutIPIsProvisioning`, `TestGet_runningWithIPIsRunning` +- [x] `UID` and `CreatedAt` read back: `TestGet_readsUIDAndCreatedAtFromBackend` +- [x] `ListByTag` excludes unmarked instances: `TestListByTag_excludesUnmarkedDomains`, `TestListByTag_emptyUIDStillListed` +- [x] `Get` during an in-progress delete: `TestGet_duringDeleteStillReturnsInstance` +- [x] `Get` during an asynchronous operation or lock: `TestGet_backendBusyIsTransientNeverNotFound` +- [x] never started is never `StateStopped`: `TestCreate_returnsOnlyWhenRunning` +- [x] cancelled `ctx` returns promptly: `TestProvider_cancelledContextReturnsPromptly`, `TestCall_cancelUnblocksRPC` +- [x] `classify` table with captured responses: `TestClassify_table` +- [x] Checker reasons, expired `ctx`, stable order: `TestCheck_reasonsTable`, `TestCheck_expiredContextReturnsPromptly`, `TestCheck_oneEntryPerHostInConfigOrder` +- [x] sentinel tests for `CloudInit` and the credential: `TestCreate_cloudInitSentinelNeverLoggedOrInErrors`, `TestSSH_keyMaterialNeverInErrorsOrLogs` +- [x] child spans under the operation span: `TestObserve_childSpansUnderParent`, `TestObserve_everySeamMethodInstrumented` +- [x] real-backend lifecycle test: `TestKVM2_lifecycle` and `TestKVM2Check_realHost` in `kvm2_integration_test.go` + +Further tests per file are listed in the plan +(`docs/plans/2026-09-16-1333-kvm2-provider.md`). + +**Evidence:** contract §11; docs/testing.md conventions. + +## 22. Deployment + +**Answer:** the existing `kvm-ssh` mount is reused; the rest is samples and a Makefile target: + +- `config/manager/manager.yaml`: no new mount; the existing optional `kvm-ssh` + Secret at `/etc/proxy-operator/kvm` serves both `kvm` and `kvm2`, and its + comment says so. `/tmp` is already an emptyDir. +- `config/manager/providers_config.yaml` and + `config/samples/providers-config.yaml`: a commented `kvm2-home` entry, the + same block as `kvm-home` under `kvm2:`. +- `config/samples/proxy_kvm2.yaml`, listed in `config/samples/kustomization.yaml`: + provider `kvm2-home`, zone `beelink` (matching the shipped config, unlike the + `kvm` sample's `nas1`), machine type `small`, image `debian-13`, a cloud-init + that installs Squid. +- Makefile: `test-integration-kvm2` (`go test -race -tags=integration -count=1 + -v -timeout 40m -run TestKVM2 ./internal/provider/kvm2/`). The Secret helper + `make kvm-ssh-secret` already exists and is reused. +- RBAC: none; the provider does not talk to the cluster's API. +- `hack/providers-dev.yaml`: no entry (needs credentials). + +**Evidence:** contract §12; `config/manager/manager.yaml`. + +## 23. Docs + +**Answer:** `docs/kvm2-provider.md` (hypervisor prerequisites including OVMF +and the `libvirt` group and a bridge with DHCP; cluster-side setup reusing the +`kvm-ssh` Secret; config reference; placement mapping; what lands on the host +including the metadata namespace; the image cache and the quota precheck; the +health check reason table; logging tiers and span names; migrating an entry +from `kvm`, which requires deleting its Proxies first because kvm2 does not +adopt v1 domains; known gaps). A fourth column in the three tables of +`docs/providers-existing.md` and a kvm2 section. `docs/architecture.md`: +components row, a `kvm2` block in §5, the span topology in §10, a §11 row, and +Decisions bullets (dedicated download connection, checks on fresh connections, +own metadata namespace, quota precheck, SSH auth failures Permanent). A README +row. `docs/testing.md` rows and a kvm2 integration section. A `CHANGELOG.md` +entry after the user confirms. + +**Evidence:** contract §12. + +## 24. Known limits accepted at design time + +**Answer:** eight limits, each with its workaround: + +- **Create blocks**, including the first image download per host, holding one + of three reconcile workers. Workaround: warm the cache with one Proxy per + image per host before creating many. +- **Volumes without a domain are invisible to GC** (crash between volume + creation and define). The same Proxy's next reconcile removes them; a Proxy + deleted for good leaves them for `virsh vol-list`. +- **A host removed from the config orphans its VMs**: Delete is a no-op + success for it and GC does not sweep it. Delete the Proxies first. +- **No adoption of `kvm` (v1) domains**: switching an entry's type requires + deleting its Proxies first, or the old domains stay behind. +- **Cancelling one call drops the host's pooled connection**, so other RPCs + in flight on that host fail once with Transient and retry. The download is + isolated on its own connection. +- **`Content-Length` is required** for image downloads; a chunked mirror is + rejected as Permanent. +- **No v1 log-format compatibility**: the `libvirt call` lines have the same + keys where the same facts exist, but dashboards keyed on `kvm` logger names + need `kvm2`. +- Contract §13 limits that apply unchanged: config read once per process + (adding a host needs a restart; credentials do not), no installation identity + in GC, an unconfigured provider latches the Proxy. + +**Evidence:** contract §13; entries 4, 8, 11, 18 above. diff --git a/docs/plans-executions/2026-09-16-1333-kvm2-provider.md b/docs/plans-executions/2026-09-16-1333-kvm2-provider.md new file mode 100644 index 0000000..ef9010e --- /dev/null +++ b/docs/plans-executions/2026-09-16-1333-kvm2-provider.md @@ -0,0 +1,55 @@ +# Execution: kvm2, a clean-room libvirt/KVM provider built from the provider contract + +Plan: [docs/plans/2026-09-16-1333-kvm2-provider.md](../plans/2026-09-16-1333-kvm2-provider.md) + +- [x] Step 0 — Plan file and branch +- [x] Step 1 — The spec, `docs/kvm2-provider-spec.md` +- [ ] Step 2 — Config wiring, IDs, state mapping, classify +- [ ] Step 3 — Pure builders: domain and volume XML, metadata, seed +- [ ] Step 4 — Transport, seam, decorator, fakes +- [ ] Step 5 — Image cache and pool-space precheck +- [ ] Step 6 — The four contract methods +- [ ] Step 7 — Checker and the CRD comment +- [ ] Step 8 — Deployment files, Makefile, integration test +- [ ] Step 9 — Documentation +- [ ] Step 10 — Milestone review +- [ ] Step 11 — Real-host validation, changelog, MR + +## Step 0 — Plan file and branch + +`main` was fast-forwarded to the merge of PR #7 (`0dc25a1`), which carries the +three provider documents, and `feat/kvm2-provider` was cut from it. The plan +was committed alone as `854856c`. The pre-existing local modification of +`.claude/settings.json` stays uncommitted throughout. + +## Step 1 — The spec, `docs/kvm2-provider-spec.md` + +2026-09-16 13:37. Written by the orchestrator, not a subagent, since the spec +is the "specifics" a user hands to the implementer. All 24 entries answered; +every Evidence line cites the v1 user guide, the 2026-09-04 execution log, a +go-libvirt symbol, or the contract. Nothing was taken from +`internal/provider/kvm/`. + +Design choices that go beyond the user's profile answers, all recorded in the +spec: an adoption guard on `Create` (foreign UID is Permanent), a pool-space +precheck mapped to `ErrQuotaExceeded`, a dedicated connection for image +downloads so a cancelled poll cannot cut one off, the Checker on fresh +connections rather than the pool, SSH authentication and host-key failures +classified Permanent (v1 retried them forever), SSH material re-read on every +dial so rotation needs no restart, kvm2's own metadata namespace (no adoption +of v1 domains), and the shared `validateKVM` refactored to take the block name +so messages say `kvm2.`. + +Checks run on the spec: + +```sh +grep -c '^\*\*Answer:\*\*$' docs/kvm2-provider-spec.md # 0 blank answers +grep -n 'internal/provider/kvm/' docs/kvm2-provider-spec.md # empty +``` + +plus the axis-code check against contract §4 headings and the path-existence +loop from the docs-split log. All clean. + +Worth noting: the spec's entry 21 lists the concrete test name for every +contract §11 item, so the later steps' briefs can quote it rather than +re-derive the mapping.