egress-proxies-operator/docs/plans-executions/2026-09-04-1404-kvm-provider.md

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.

Step 3b — Image cache

image.go follows the plan closely: resolveImage maps placement.image to a configured key or a direct http(s) URL (else Permanent); imageCache.ensure checks base volume + marker (+ marker content when a checksum is configured), tears down and re-fetches when anything is off, streams the HTTP body through StorageVolUpload with a sha256 tee, refreshes the pool, then writes the marker last. A per-(host, volume) mutex serialises concurrent callers; the pool itself is the only cache state.

Errors from this file are pre-tagged with the taxonomy sentinel via fmt.Errorf("%w: …", provider.ErrPermanent, …) and classify now honours such tags before looking at libvirt codes. That keeps one wrapErr path in the provider instead of special-casing image errors.

fake_test.go landed with this step rather than Step 4: an in-memory libvirtAPI with pools, volumes (content, format, capacity, backing path), domains (state, metadata, per-source interface addresses), per-method error injection (fail always / failOnce) and an uploadHook to break a stream mid-way. It models the libvirt error codes the provider branches on (ErrStorageVolExist, ErrNoStorageVol, ErrNoDomain, ErrOperationInvalid, ErrNoDomainMetadata, ErrAgentUnresponsive).

Tests cover every case in the plan plus: no checksum configured (marker is still written with the computed digest; second call needs no marker read), 403/500 alongside 404/503, missing pool → Permanent, cancelled ctx → Transient with nothing created, and different hosts fetching independently.

One test-harness lesson worth keeping: httptest's server drops Content-Length and switches to chunked encoding for bodies larger than its 2 KiB write buffer, so the fake mirror sets the header explicitly. Real distro mirrors send it; a server that doesn't is reported as Permanent with a message naming the header.

Worth noting: the base volume is created with capacity = Content-Length and the image's format (qcow2), and the raw bytes are uploaded over it. That relies on libvirt's upload writing the file verbatim (which is how virsh vol-upload populates volumes); the following StoragePoolRefresh makes libvirt re-read the real virtual size from the qcow2 header. Step 6 confirms this on a real host. Also, the whole download runs inside one pool.call, so cancellation aborts the HTTP body (ctx-bound) and the upload stream fails; the half-written base volume is deleted on the spot and, failing that, on the next attempt via the missing marker.