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.

Step 4 — Provider methods

kvm.go now holds Provider, New (loads the SSH key and known_hosts up front via newSSHAuth, so a bad path fails at startup), the newWithDialer seam, and Create/Get/Delete/ListByTag. Each method resolves its host from config, then runs the whole libvirt conversation inside one hostPool.call, so a cancelled reconcile drops the connection and the blocked RPC returns. Errors pass through classify in one wrapErr, as the Step 3b note intended.

Behaviour that goes beyond the plan text, or that a future reader should know before touching this file:

• Create looks up the domain first. If it exists, only the running check runs and the volumes are never touched. If not, nothing can be using the volumes yet: the overlay is reused when it exists (its creation is atomic; ErrStorageVolExist → reuse) but the seed is always deleted and rebuilt. The plan said "exists → skip upload"; the reviewer pointed out that a reconcile cancelled mid-upload leaves a truncated seed that the compensating delete cannot remove, because hostPool.call has already closed the connection the delete would go over. Skipping such a seed would boot the VM without cloud-init and no retry would ever repair it. Rebuilding costs one ~40 KiB upload per attempt before the domain exists, nothing afterwards. The table test breaks Create after each step and re-runs it, and a separate test starts from a leftover truncated seed.
• Get swallows address-source errors. DomainInterfaceAddresses from ARP returns nothing until the guest has sent traffic and the agent source errors with ErrAgentUnresponsive until qemu-guest-agent is up, so both are logged at V(2) and mean "no IP yet" → Provisioning. Loopback, link-local and unspecified addresses are skipped when choosing the first IPv4 (the agent reports lo too).
• State mapping: running/blocked are active; nostate → Provisioning (the reconciler answers Stopped with delete-and-recreate, too drastic for a state libvirt could not determine); everything else (paused, pmsuspended, shutdown, shutoff, crashed) → Stopped.
• Delete treats a missing pool as "nothing to clean" rather than Permanent: failing there would pin the Proxy's finalizer on a config edit that renamed the pool. Domain removal is destroy (ErrOperationInvalid on a stopped domain is fine) → undefine with managed-save, snapshot metadata and nvram flags → overlay and seed deleted by name.
• ListByTag reads each domain's metadata once (passed into the same inspect that Get uses, so no second DomainGetMetadata round-trip) and skips domains without the operator's element. A domain that cannot be read at all — vanished mid-sweep, corrupt metadata, state call failing — is skipped, not fatal: GC never deletes what it does not see, so leaving one out is safe, whereas one bad domain blocking GC for every other one (the first version's behaviour, caught in review) is not. An unreachable host still fails the whole sweep. ConnectListAllDomains is called with flags 0 (active and inactive).
• A host named in a providerID but no longer in kvm.hosts is Permanent for Get and Delete: the operator cannot reach it, and retrying forever would hide the config problem.

Tests run the provider over a hostDialer that maps host names to fakeAPI instances (an unmapped host is "unreachable"). The image is pre-seeded in the pool for most tests; one test runs the full first-use download through httptest and confirms a second Proxy on the same host does not download again.

Found while running the package: TestBuildSeedISO_deterministic from Step 2 was wrong. iso9660.ImageWriter stamps time.Now() into the volume descriptor, so two builds in different seconds differ by six timestamp bytes; the test had only passed because both builds landed in the same second. Byte identity is nothing the provider depends on (an existing seed is never re-uploaded), so the test now compares label, files and size, and the comment on buildSeedISO no longer claims identical output.

The go-operator-reviewer agent ran on the first version of this step and found the two bugs above (truncated seed, wedged GC sweep) plus the double metadata read and the nostate mapping; all were fixed before committing. One finding it raised is deliberately not fixed here because it lives in Step 3b's image.go and changes the volume naming the plan chose: re-fetching a base image (checksum edited in config, or upstream content changed) deletes base-<key>.<fmt> while existing overlays still back onto it, breaking those VMs' disks. Content-addressed base names would avoid it. Flagged for the user; until decided, Step 5's docs must warn that an image's sha256/url must not change while Proxies using it exist.

Worth noting: go test -cover reports 88.2 % for the package; the uncovered lines are the real SSH/libvirt dial path and log-only branches. go vet and -race (three runs) are clean. Wiring into cmd/main.go is Step 5, so New is not yet reachable from a running operator.