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".

Step 8 — Deployment files, Makefile, integration test

2026-09-16 19:39. Seventh subagent step. kvm2_integration_test.go (//go:build integration, KVM2_TEST_* variables, the eleven ordered assertions of spec entry 21 plus TestKVM2Check_realHost), config/samples/proxy_kvm2.yaml (zone beelink, so the sample matches the shipped config), a commented kvm2-home entry in both providers configs with the porting note, comment-only edits on the shared kvm-ssh mount, the test-integration-kvm2 Makefile target and the kvm2 rows and section in docs/testing.md. The integration test was not run; that is Step 11 with the user's go-ahead.

The agent found that New builds the production dialer without tracing options, so in production the dialer and decorator use the global tracer provider (correct: cmd/main.go sets it up), while a test that wants every span family in one recorder sets the dialer's option itself. The test does so and asserts that kvm2.DomainDefineXML and kvm2.dial are children of provider.create.

Decisions the agent made, accepted: the UID is generated from crypto/rand rather than promoting google/uuid to a direct dependency; the sentinel check looks for the full cloud-init string because the provider name also appears in attributes; -run TestKVM2 matches both tests, as the kvm target's pattern does. Forward links to docs/kvm2-provider.md dangle until Step 9.

Commands:

go vet ./... && go vet -tags=integration ./... && go vet -tags=e2e ./...
bin/kustomize build config/default > /dev/null && bin/kustomize build config/samples > /dev/null
make -n test-integration-kvm2
# go test -race -tags=integration -count=1 -v -timeout 40m -run TestKVM2 ./internal/provider/kvm2/

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

Step 9 — Documentation

2026-09-16 19:54. Eighth subagent step: docs/kvm2-provider.md (sixteen sections modelled on the kvm guide), a kvm2 column in the three tables of docs/providers-existing.md plus a kvm2 section and renumbering, the components row, §5 block, §10 span bullet, §11 row and eight Decisions bullets in docs/architecture.md, a README row and paragraph, and the guide in the docs/providers.md index. The agent verified each claim against the package rather than the spec and dropped a draft sentence claiming kvm and kvm2 share the base-image cache, because checking it would have meant reading v1's volume names.

Checks re-run by the orchestrator: no em-dash in the guide, no line references, each of the 19 axis codes appears once in the appendix table with seven cells, every link and anchor across the seven touched documents resolves. The one added em-dash is the Leader-elected cell of the new components-table row, where the table's own convention wins.

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

Step 10 — Milestone review

2026-09-16 21:19. The go-operator-reviewer agent reviewed the package against the contract and the spec (v1 code off limits to it as well) and returned seven findings, all accepted and applied by a fresh fix agent in one commit with regression tests:

1. waitRunning's two-minute cap only ever elapsed in the bad cases and pinned a reconcile worker; now 15 s, a domain that dies right after start is Transient at once, and a paused or pmsuspended domain is Permanent before DomainCreate (an admin's doing).
2. ListByTag read state and addresses that GC never uses, one agent RPC per active domain per sweep; it now builds instances from the name and the metadata only (TestListByTag_doesNotReadStateOrAddresses).
3. A health round that ran out of time was blamed on the host (Unreachable); checkHost now reports Timeout whenever the round's ctx ended, whichever dial stage was running, and the promptness test uses the production error shape.
4. Create adopted an unmarked domain and never wrote the mark, so GC could not see it; an unmarked domain under the name is now Permanent (TestCreate_adoptsOnlyMarkedDomains).
5. hostTable.closeAll had no production caller and withBudgets could not work (the dialer captured the defaults first); both removed.
6. The per-name and per-image lock maps grew without bound; replaced by a reference-counted keyedLocks whose entries vanish when uncontended.
7. imageSizeIfUncached falls back to GET on any non-2xx HEAD, and two Delete no-op lines moved from V(0) to V(1).

The spec (entries 2, 10, 12, 16, 18, 19, 24), the guide, the appendix and the architecture flow were updated in the same commit. The reviewer also listed what it checked and found correct: the error boundary, the state rules, Delete's idempotency and the replacement window, ListByTag's partial-failure policy, the abandon policy and double-close guards, resource handling on every path, and secret hygiene in the format strings.

Lint: bin/golangci-lint still panics on this toolchain (Go 1.27 against a staticcheck built for 1.26), reproducible on untouched packages, so make lint could not gate this step; go vet with every build tag is clean. Tests: go test -race -count=3 ./internal/provider/kvm2/ (168 top-level tests) and go test -short ./... pass; make test with envtest runs next.