Files
egress-proxies-operator/docs/plans-executions/2026-09-16-1333-kvm2-provider.md
T
kacerrandClaude Fable 5.1 b45f83327a kvm2: SSH transport, ctx-first libvirt seam, host table and span decorator
Every go-libvirt call runs under its own budget and a cancelled or overlong
call kills the connection; credentials are read on every dial; one span and
one log line per RPC come from a decorator applied where connections are
made.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
2026-09-16 17:43:27 +00:00

174 lines
8.4 KiB
Markdown

# Execution: kvm2, a clean-room libvirt/KVM provider built from the provider contract
Plan: [docs/plans/2026-09-16-1333-kvm2-provider.md](../plans/2026-09-16-1333-kvm2-provider.md)
- [x] Step 0 — Plan file and branch
- [x] Step 1 — The spec, `docs/kvm2-provider-spec.md`
- [x] Step 2 — Config wiring, IDs, state mapping, classify
- [x] Step 3 — Pure builders: domain and volume XML, metadata, seed
- [x] 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:
```sh
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:
```sh
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:
```sh
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):
```sh
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`.