kvm2 provider: clean-room libvirt/KVM provider, code review, and the four high findings #8

Merged
kacerr merged 22 commits from feat/kvm2-provider into main 2026-09-17 23:12:46 +02:00
2 changed files with 680 additions and 0 deletions
Showing only changes of commit 4bd9aae03b - Show all commits
+625
View File
@@ -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/<host>/domains/<name>`, where `<host>` is `hosts[].name`
from the config block and `<name>` 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.<key>.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-<key>.<format>` (for a URL: `base-<first 16 hex of sha256(url)>.<format>`),
with a sidecar marker volume `base-<key>.<format>.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: <name>` and `local-hostname: <name>`, 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 `<name>-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.<key>` 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 `<metadata>`, an
element `<instance xmlns="https://gitea.home.hrajfrisbee.cz/kacerr/egress-proxies-operator/kvm2">`
with children `<managed>true</managed>`, `<uid>req.UID</uid>` and
`<createdAt>RFC3339 UTC</createdAt>`. 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 `<name>.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 `<name>-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/<host>/domains/<name>`.
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 `<name>.qcow2` and `<name>-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
<path>" 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>` | | | key matches `kvmIdentifier` | volume name `base-<key>.<format>` |
| `images.<key>.url` | yes per entry | | http(s) URL | download |
| `images.<key>.sha256` | | | 64 hex | marker comparison |
| `images.<key>.format` | | qcow2 | qcow2 or raw | base volume XML, overlay backing format |
| `images.<key>.firmware` | | uefi | uefi or bios | domain XML `<os>` |
| `machineTypes.<key>` | 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.<Method>` (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/<host>/domains/<name>`; 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 `<name>.qcow2` or `<name>-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.
@@ -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.