egress-proxies-operator/docs/plans-executions/2026-09-16-1333-kvm2-provider.md

Execution: kvm2, a clean-room libvirt/KVM provider built from the provider contract

Plan: docs/plans/2026-09-16-1333-kvm2-provider.md

• Step 0 — Plan file and branch
• 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:

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.

Step 2 — Config wiring, IDs, state mapping, classify

2026-09-16 16:38. First subagent step. The brief named the allowed files, the spec entries, the test names and the prohibition on internal/provider/kvm/; the agent reported nothing opened there, with one honest caveat: a golangci-lint run printed seven single source lines from v1 files among its findings, which it did not act on. The review greps (grep -rn 'provider/kvm"' internal/provider/kvm2/, the v1 identifier smell list) are clean.

What landed: ProviderConfig.KVM2 reusing KVMConfig, setBlocks, the allow-list and dispatch in Config.validate, validateKVM refactored into validateKVMBlock(prefix, block, c) with validateKVM kept as a wrapper so v1 messages and tests are untouched, "kvm2": kvm2.New in cmd/main.go (both type lists in one commit), and the package skeleton: kvm2.go (New never dials, warns on unreadable files, withDefaults, host, wrapErr, stubbed methods), id.go, state.go, errors.go with 34 classify rows.

Decisions the agent made that the spec left open, all accepted: New checks file existence only (parsing belongs to the dialer); withDefaults copies hosts and images; firstIPv4 also drops an address whose type tag and literal disagree; there is no explicit fs.ErrNotExist branch in classify because it falls to the Transient default anyway.

Commands as run by the agent and re-run by the orchestrator:

go vet ./... && go build ./... && go test -race -count=1 ./internal/provider/... ./cmd/...
go test -short ./...

All packages ok.

Worth noting: bin/golangci-lint panics inside staticcheck's IR builder on this machine (unexpected expr: *ast.KeyValueExpr on stdlib internal/poll) because the local toolchain is Go 1.27.0 while go.mod says 1.26 and the bundled staticcheck predates it; it reproduces on untouched packages, so lint is deferred to Step 10 with a toolchain note. The .claude/settings.json modification is still local and uncommitted.

Step 3 — Pure builders: domain and volume XML, metadata, seed

2026-09-16 16:55. Second subagent step, new files only: xml.go, metadata.go, volume.go, seed.go and their tests (26 tests). The domain XML is built from encoding/xml structs, both disks are type="file" with resolved paths (the AppArmor lesson), UEFI is the default with the secure-boot feature disabled, the metadata element parses with or without its namespace declaration, and the seed ISO is staged under os.TempDir() (the library uses os.MkdirTemp("", ""), so TMPDIR is honoured; the test points it at a t.TempDir() and asserts nothing is left behind).

Decisions the agent made, all accepted: buildDomainXML defaults an empty disk format to qcow2 and rejects anything but qcow2 or raw; buildVolumeXML rejects a non-positive capacity; buildSeedISO rejects an empty name; the guest-agent script still runs the enable-and-start step when the agent is already installed; the multipart boundary is random, so vendor-data is not byte-deterministic across calls (the seed is rebuilt every Create anyway).

Commands:

gofmt -l ./internal/provider/kvm2 ; go vet ./internal/provider/kvm2/ && go test -race -count=1 ./internal/provider/kvm2/
go build ./... && go test -short ./...

All ok. Review greps for a v1 import or v1 identifiers are empty; the only type="volume" in the package is the comment explaining why it is not used. The agent reported "files read outside the allowed set: none".

Step 4 — Transport, seam, decorator, fakes

2026-09-16 17:42. Third subagent step, the largest: backend.go (the ctx-first seam backend, the production adapter libvirtBackend, a generic call[T] helper with per-call budgets and the abandon policy), ssh.go (credentials read on every dial, stage-tagged dialError, the forwarded socket, keepalive), hosts.go (hostTable with a per-host channel semaphore so waiting for a dial ends with the caller's ctx, dialFresh for dedicated connections, the production libvirtDialer starting the kvm2.dial span and applying the decorator once), observe.go (one span kvm2.<Method> and one libvirt call line per RPC), plus an in-memory fakeBackend, an in-process SSH server and 41 tests. The agent also drove the real adapter over go-libvirt's own libvirttest mock through a net.Pipe, which the brief did not ask for.

Abandon policy as built: a call that outlives its ctx or budget kills the link (closes the socket and the SSH client), returns Transient wrapping the ctx or deadline error, and other in-flight calls on that connection fail once with go-libvirt's ErrInterrupted; the next get redials. Results travel through a channel so a late-finishing abandoned call never writes into its caller's frame; the agent reported that the first -race run caught exactly that bug in an earlier version.

Decisions the agent made, accepted: liveness is a separate unexported interface so the reflection test over backend stays at the 20 RPCs plus Close; Close sends a bounded ConnectClose before killing the link; observe and the dialer take ...tracing.Option so tests can inject a recording tracer provider, as provider.WithTracing does.

Clean-room note: three type names coincide with names v1 uses (dialError, dialStage, hostEntry). The brief described each concept ("an error type that records which stage failed", "a per-host entry") so the names follow from the description; the greps for a v1 import and for the listed v1 identifiers are empty and the agent attested to reading nothing under internal/provider/kvm/.

Commands (agent and orchestrator):

gofmt -l ./internal/provider/kvm2 ; go vet ./internal/provider/kvm2/ && go test -race -count=2 ./internal/provider/kvm2/
go build ./... && go test -short ./...
grep -n 'context.Background()' internal/provider/kvm2/*.go | grep -v _test.go   # empty

All ok; the agent also ran the timing-sensitive tests at -count=8.

Step 5 — Image cache and pool-space precheck

2026-09-16 18:11. Fourth subagent step: image.go and image_test.go (27 tests), plus additive changes to fake_test.go (a twin() sharing state with its own call log, so a test can tell the pooled connection from the dedicated one) and kvm2.go (http client, budgets and tracing options behind functional options, so New is unchanged). The pipeline: marker lookup and bounded read on the pooled connection, then a GET bound to ctx plus the 20-minute cap under a kvm2.download span, StorageVolCreateXML and a streamed StorageVolUpload through sha256 on a fresh connection, StoragePoolRefresh, and the marker written last; every failure after the volume exists discards it with the cancellation stripped from ctx so the cleanup RPC still runs.

Streaming is asserted by counting: a hook on the dedicated fake's upload snapshots how many bytes the 64 MiB test server had written when the upload began, and asserts under 16 MiB. The agent chose that over a process-wide MemStats delta because the tests run in parallel.

Decisions the agent made, accepted: the base volume is created on the dedicated connection (nothing exists if the dial fails); Accept-Encoding: identity is sent so transparent gunzip cannot hide the Content-Length; a marker whose content is not 64 hex characters counts as debris; Content-Length: 0 is its own Permanent error; the image lock is a one-slot channel so a waiter gives up with its ctx instead of sitting out a transfer; the fetch logger is derived after the span starts so its lines carry the span ID (the agent's first test run caught that).

Commands (agent and orchestrator):

gofmt -l ./internal/provider/kvm2 ; go vet ./internal/provider/kvm2/ && go test -race -count=2 ./internal/provider/kvm2/
go build ./... && go test -short ./...
grep -n 'io.ReadAll\|bytes.Buffer' internal/provider/kvm2/image.go            # empty
grep -n 'context.Background()' internal/provider/kvm2/*.go | grep -v _test.go  # empty

All ok. Attestation: "files read outside the allowed set: none".

Step 6 — The four contract methods

2026-09-16 18:36. Fifth subagent step: create.go, get.go, delete.go, list.go and five test files (the package now has 150 top-level tests). Create follows spec entry 10 step for step under a per-name lock, with the adoption guard, the quota precheck, the image cache, an overlay that is reused and a seed that is always rebuilt, file-type disks from resolved paths, the metadata element in the define call, and a wait for running capped at two minutes. Get maps only ErrNoDomain to NotFound, prefers the guest agent over ARP, and reads UID and CreatedAt back every time. Delete is idempotent at every step and a no-op success for an unconfigured host. ListByTag fails whole per host and skips single unreadable domains.

Decisions the agent made, accepted: the adoption path also waits for running, so a repeat Create on a shut-off domain blocks like a first one; address and ListByTag propagate ctx cancellation instead of reading it as "no address" or "skip", so a cancelled Get is Transient rather than a false Provisioning; NotFound from Get gets its own V(1) line because polling to NotFound is expected; the domain XML rides on the domain defined line at V(2). Flagged, not solved: adopting a paused domain fails Transient on every retry (DomainCreate refuses and the wait expires), which the spec does not cover; recorded as a known gap for the docs step.

The agent found a one-word conflict between the brief and the spec (DomainAffectCurrent versus DomainAffectConfig for DomainGetMetadata); the code uses DomainAffectCurrent, and spec entry 8 was corrected to match in this commit.

Commands (agent and orchestrator):

gofmt -l ./internal/provider/kvm2 ; go vet ./internal/provider/kvm2/ && go test -race -count=2 ./internal/provider/kvm2/
go build ./... && go test -short ./...
grep -n 'CloudInit' internal/provider/kvm2/create.go          # only len(req.CloudInit) and the seed builder call
grep -n 'not implemented' internal/provider/kvm2/*.go          # empty
grep -n 'provider.Wrap(' internal/provider/kvm2/*.go | grep -v _test.go | grep -v 'p.name'   # empty

All ok. Attestation: "files read outside the allowed set: none".

Step 7 — Checker and the CRD comment

2026-09-16 18:52. Sixth subagent step: check.go and check_test.go (ten tests, the reasons table has 19 rows). Every host is checked concurrently on a fresh dedicated connection that is closed when the probe ends; the pooled table is never consulted, which the test proves with a counting dialer. Details fill in as facts arrive, so a PoolNotFound verdict still says which libvirt version answered. The status.type comment in api/v1alpha1/providerhealth_types.go gained kvm2 and make manifests changed only the CRD's description line:

make manifests && git diff --stat config/ api/
#  api/v1alpha1/providerhealth_types.go                    | 2 +-
#  config/crd/bases/crawl.example.com_providerhealths.yaml | 2 +-

Decisions the agent made, accepted: a dial that runs out of time is Unreachable (the host did not answer), and Timeout is reserved for ctx or budget expiry on an established connection; Close runs synchronously in the probe so the close-count assertion is deterministic, at the cost of up to five seconds past ctx if a disconnect hangs on a live link.

The agent also reported that the captureContext helper in kvm2_test.go appends to an unlocked slice, which the race detector flagged once Check logged from several goroutines. The helper was outside its allowed file set, so it built its own locked capture; the orchestrator fixed captureContext with a mutex in this commit and re-ran the package at -race -count=3.

Attestation: "files read outside the allowed set: none".