kacerrandClaude Fable 5.1
95768281be
The kvm2 block reuses KVMConfig; validateKVM gains a block-name variant so messages say kvm2. The package skeleton has a non-dialing New, strict providerID parsing, the state table and a 34-row classify test. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
97 lines
4.5 KiB
Markdown
97 lines
4.5 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
|
|
- [ ] 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:40. 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.
|