Files

T

kacerrandClaude Fable 5.1 6b3c985b4a kvm2: base image cache with a streamed upload and the pool-space precheck

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

2026-09-16 18:19:05 +00:00

10 KiB

Raw Blame History

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