Per-host connection pool over SSH with the libvirt unix socket forwarded through the session, lazy dial and redial after disconnect, context cancellation for go-libvirt's context-less RPCs, handshake deadline and SSH keepalives. Error classification onto the provider taxonomy. Tested with a fake dialer and an in-process SSH server that forwards unix sockets. Step 3 of docs/plans/2026-09-04-1404-kvm-provider.md. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
7.9 KiB
Execution: KVM/libvirt provider
Plan: 2026-09-04-1404-kvm-provider.md
- Step 0 — Save plan + branch
feat/kvm-provider - Step 1 — Config (
KVMConfig& friends,validate()case, tests) - Step 2 — Pure builders (
domainxml.go,volume.go,seed.go,id.go,metadata.go) - Step 3 — Transport + errors (
conn.go,errors.go) - Step 3b — Image cache (
image.go) - Step 4 — Provider methods (
kvm.go) over a fake API - Step 5 — Wiring, manifests, samples, docs, CHANGELOG
- Step 6 — Integration test against a real hypervisor
- Step 7 — Manual validation + MR
Step 0 — Save plan + branch
Plan committed alone on feat/proxy-operator, then branched:
git checkout -b feat/kvm-provider
Step 1 — Config
The new structs (KVMConfig, KVMHost, KVMMachineType, KVMImage) and
validateKVM went into their own file, internal/provider/config_kvm.go,
rather than being appended to config.go as the plan said: the KVM block has
far more fields and rules than the other two providers, and keeping it
separate leaves config.go readable. Tests likewise live in
config_kvm_test.go instead of extending the table in config_test.go; the
invalid-config table covers every required field, duplicate host names, image
URL scheme, sha256 shape, and the disk format enum.
While adding the kvm case to validate(), the pairwise "type is X but a Y
block is set" checks (which would have needed six branches with three types)
were replaced by a setBlocks() helper that lists the non-nil blocks and
rejects any that don't match type. The error strings are unchanged, so the
existing GCP/kubernetes tests still pass.
Dependencies were fetched up front so the API signatures in the plan could be
checked with go doc; they stay // indirect in go.mod until Step 2 imports
them:
go get github.com/digitalocean/go-libvirt@latest github.com/kdomanski/iso9660@v0.4.0
# go-libvirt resolved to v0.0.0-20260814190004-1a83157e1858 (no tagged release)
# side effect: golang.org/x/crypto 0.54.0 → 0.56.0, x/mod, x/text, x/tools bumped
Worth noting: go doc showed that go-libvirt's New(net.Conn) is deprecated
in favour of NewWithDialer(socket.Dialer), where Dialer is a one-method
Dial() (net.Conn, error) interface. Step 3 should wrap the SSH client's unix
forward in that interface instead of using New; the plan's transport design
is otherwise unaffected.
Step 2 — Pure builders
internal/provider/kvm/ now has the five builders from the plan plus
kvm.go holding the package doc and withDefaults (disk size 10 GiB, the
default vendor-data, /var/run/libvirt/libvirt-sock, pool default, image
format qcow2). Everything is a pure function over encoding/xml structs or
strings, unit-tested by unmarshalling the output back and checking fields.
Two design choices differ from the plan text:
- Domain disks reference pool volumes, not host paths. The plan had
buildDomainXMLtake the overlay and seed paths (needing twoStorageVolGetPathround-trips per Create). The domain XML now uses<disk type="volume"><source pool=... volume=.../>, so only the base image path is ever looked up (the overlay's<backingStore>needs a real path). Fewer RPCs and the builder's inputs are all names. - Capacities are always
unit="bytes". libvirt'sGmeans GiB butGBmeans 10^9; spelling the byte count out removes the ambiguity from both the overlay and the seed/base volumes.
The metadata element is written with a default namespace
(<instance xmlns="…/kvm">) because encoding/xml cannot emit prefixes on
marshal. libvirt may hand it back prefixed; parseInstanceMetadata is
tested against both spellings since encoding/xml matches on the namespace
URI. Whether libvirt accepts the default-namespace form on DomainDefineXML
is confirmed in Step 6 against a real host, not here.
Checked before relying on github.com/kdomanski/iso9660 for the seed: its
filename mangling lowercases and keeps -, so user-data/meta-data/
vendor-data come out as user-data;1 etc. in the directory record, which
the Linux isofs driver (and therefore cloud-init's NoCloud mount) presents as
user-data. The seed test opens the ISO with the same library's reader and
asserts label cidata, all three files, and byte-identical output for
identical inputs. NewWriter() stages files in a temp directory, so
Cleanup() is deferred.
go mod tidy ran as part of this step; it promoted kdomanski/iso9660 to a
direct dependency and dropped go-libvirt again because nothing imports it
yet — Step 3 brings it back.
Worth noting: withDefaults copies the Images map and Hosts slice
instead of mutating the config in place, so the registry's shared
ProviderConfig stays untouched. The default vendor-data installs
qemu-guest-agent via packages: + runcmd, which means the first boot
needs network access to the distro mirrors; a Proxy on an isolated bridge
would still work (ARP discovery), just without the agent.
Step 3 — Transport + errors
conn.go holds the libvirtAPI seam (17 methods, signatures as in the
plan), a per-host connection pool, and the SSH dialer; errors.go holds
classify, isLibvirtError, hasCode and logAPIError. Both are
unit-tested; the whole package still runs in about a second under -race.
The one thing the plan did not anticipate: go-libvirt RPCs take no
context and never time out. A hung hypervisor would pin a reconcile worker
forever. So the pool exposes call(ctx, host, fn) rather than handing out
raw clients: fn runs in a goroutine, and if ctx ends first the pool
closes the SSH connection out from under it (which makes the blocked RPC
return) and reports ctx.Err(). The same helper (awaitOrCancel) bounds
lv.Connect() during dial. Two more liveness measures for the same reason:
the SSH handshake gets a deadline (ctx's, or 30 s), and each connection runs
an OpenSSH keepalive@openssh.com loop every 15 s that closes the client
when a request errors or goes unanswered for one interval.
Connection-drop policy in call: any error that is not a libvirt.Error
(so io.EOF, "connection closed", SSH failures) drops the cached connection
and the next call redials; a daemon-reported libvirt.Error keeps it. Dials
are serialised per host (concurrent Creates on one host share a dial) but
hosts don't block each other.
As Step 1 noted, the dialer uses libvirt.NewWithDialer with a
socket.Dialer that wraps ssh.Client.Dial("unix", socket) rather than the
deprecated New(net.Conn).
Tests: the pool is exercised through a fake dialer (reuse, redial after
Disconnected, drop on transport error, keep on libvirt error, cancel
releases a blocked call, concurrent callers share one dial, hosts
independent). The SSH layer is tested against an in-process
x/crypto/ssh server that serves direct-streamlocal@openssh.com channels
by dialing the requested unix path (an echo socket standing in for
libvirt-sock), with keys generated per test and a real known_hosts file
written via knownhosts.Line. Covered: forward round-trip and the socket
path the server saw, unknown host key rejected with knownhosts.KeyError,
insecureIgnoreHostKey, wrong client key, cancelled ctx, missing key or
known_hosts file, and keepalive both closing a stalled client and leaving a
healthy one alone. The libvirt handshake (Connect()) itself is not
covered here — faking libvirt's XDR RPC just for that is not worth it; the
Step 6 integration test covers it against a real host.
go mod tidy promoted go-libvirt and golang.org/x/crypto to direct
dependencies.
Worth noting: classify maps ErrOperationInvalid (e.g. destroying a
domain that is already off) to Transient; Delete in Step 4 must check that
code itself via hasCode and treat it as success rather than relying on
the classifier. Nothing maps to ErrQuotaExceeded — libvirt has no quota
concept; an out-of-space pool surfaces as a Transient ErrInternalError.