128 lines
6.0 KiB
Markdown
128 lines
6.0 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
|
|
- [ ] 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".
|