Files
egress-proxies-operator/docs/plans-executions/2026-09-04-1404-kvm-provider.md
T
kacerrandClaude Fable 5.1 6322184d50 Add KVM provider transport: SSH-forwarded libvirt connections and error classification
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>
2026-09-10 19:13:51 +00:00

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 buildDomainXML take the overlay and seed paths (needing two StorageVolGetPath round-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's G means GiB but GB means 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.