Files
egress-proxies-operator/docs/plans-executions/2026-09-18-1502-proxmox-provider.md
kacerrandClaude Opus 5.5 3d72507894 proxmox: review fixes, real-host validation record, changelog
Phase 10. Two review passes (go-operator-reviewer, /code-review high)
over the branch, recorded in docs/reviews/2026-09-23-proxmox-provider-review/:
9 findings fixed, 3 accepted as documented limits, 7 rejected with a
reason.

- Delete removes the seed between the stop and the destroy, so a failed
  seed delete leaves the VM for the next Delete instead of leaking the
  ISO; a Permanent refusal on the every-node path fails the call.
- Create skips a probe VMID whose listed name is someone else's without
  waiting on its lock.
- The three answers Phase 0 never captured are fixtures now: a missing
  storage is a 400 (the Checker reports StorageNotFound), a hibernated
  VM is destroyable (Delete destroys ours), a paused VM stops.
- TestProxmox_lifecycle and TestProxmoxCheck_realHost passed on pv1-1
  (PVE 9.2.20); the docs record the run and drop "not yet run".

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
2026-09-24 10:28:28 +00:00

1442 lines
80 KiB
Markdown
Raw Permalink Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
# Execution: proxmox, a Proxmox VE provider built from the provider contract
Pairs with [docs/plans/2026-09-18-1502-proxmox-provider.md](../plans/2026-09-18-1502-proxmox-provider.md).
## Status
- [x] Phase 0 — Plan file, branch, host recon, fixtures
- [x] Phase 1 — The spec (`docs/proxmox-provider-spec.md`)
- [x] Phase 2 — Config block, registration, package skeleton
- [x] Phase 3 — Extract `nocloud` and `keyedlock` from kvm2
- [x] Phase 4 — Seam, HTTP client, decorator, fake
- [x] Phase 5 — Get, ListByTag, task helpers
- [x] Phase 6 — Create
- [x] Phase 7 — Delete, Checker, CRD comment
- [x] Phase 8 — Contract and observability sweep, integration test, testing.md
- [x] Phase 9 — Deployment and documentation
- [x] Phase 10 — Review, real-host validation, changelog, MR
## Phase 0 — Plan file, branch, host recon, fixtures
The plan was committed alone on `main` on 2026-09-18 (`708989c`, then
`2167819` retargeting it at PVE 9.2). `origin/main` had not moved in the
meantime, so the branch was cut straight from local `main`:
```bash
git fetch -q origin && git rev-list --left-right --count origin/main...main # 0 2
git switch -c feat/proxmox-provider
```
The host is **pv1-1**, a single node (no cluster peers), `pve-manager/9.2.20`,
kernel `7.0.14-17-pve`, QEMU 11.0. The user prepared the token
(`~/.config/proxmox-token`, privsep off) and the CA (`~/.config/proxmox-ca.pem`)
before the session.
**TLS: 192.168.2.8 is not in the node certificate.** The first probe failed
with curl exit 60. The CA is right (`openssl s_client -CAfile` verifies the
chain), but the leaf's SANs are `127.0.0.1, ::1, localhost, 192.168.0.8,
pv1-1, pv1-1.home.lab`, and neither name resolves on the workstation.
192.168.0.8 and 192.168.2.8 present the same certificate fingerprint: the
node sits on `vmbr0` (192.168.0.8/24, untagged) with VLAN interfaces
`vmbr0.2` (192.168.2.8/24) and `vmbr0.4` (192.168.4.8/24). All recon kept
full verification by connecting to 192.168.2.8 while verifying the
certificate as `pv1-1.home.lab`:
```bash
curl -s --resolve pv1-1.home.lab:8006:192.168.2.8 --cacert ~/.config/proxmox-ca.pem \
-H @<(printf 'Authorization: PVEAPIToken=%s\n' "$(tr -d '[:space:]' < ~/.config/proxmox-token)") \
https://pv1-1.home.lab:8006/api2/json/version
# {"data":{"release":"9.2","version":"9.2.20","repoid":"49318c671b82f31e"}}
```
Every capture went through a scratch helper wrapping exactly that call. It
passes the token through a file descriptor so it never reaches argv or
output, and writes `{"status","reason","body"}` to
`internal/provider/proxmox/testdata/<name>.json`, refusing to save a body
that contains `PVEAPIToken`.
A real NoCloud seed was used instead of the plan's 1 KiB placeholder, so the
agent could be installed and F21 captured from a live guest. No ISO tool is
installed locally, so a throwaway module in the scratchpad reused the
repository's `github.com/kdomanski/iso9660 v0.4.0` from the module cache and
copied kvm2's `guestAgentScript` verbatim:
```bash
GOFLAGS=-mod=mod GOPROXY=off GOSUMDB=off go run . proxy-reconaaaaaaaaaaa proxy-reconaaaaaaaaaaa-seed.iso
# 45056 bytes, ISO 9660 'cidata'
```
Write experiments ran against VMID 199999 (`proxy-reconaaaaaaaaaaa`), a
clone of template 9001. Sequence: validation error; clone; resources, config
write and start while the clone lock was held; status and agent while
running bare; stop; config read; clone onto the taken VMID; tag tests;
upload twice plus a bad name; attach `ide2`, set cores and memory; three
resizes; start and agent poll; suspend and resume; a clone onto a storage
without image support; stop; destroy with `purge=1&destroy-unreferenced-disks=1`;
destroy again; ISO delete.
### Facts F1 to F23 (cited as Evidence by the spec)
- **F1.** Errors carry the text twice: in the HTTP reason phrase and as the
body's `message`, which ends in `\n`. A 400 adds `errors`, a map from
parameter to message. A 401 has the reason phrase only and an empty body.
The client should use the trimmed body `message` and fall back to the
reason phrase. Headers are ordinary CRLF.
- **F2.** Missing VM, identical for `GET config`, `GET status/current` and
`DELETE`: `500 Configuration file 'nodes/pv1-1/qemu-server/999999.conf' does not exist`.
- **F3.** A start issued while the VM is clone-locked is **accepted** (200
with a UPID). The task log reads `trying to acquire lock... OK` and the VM
starts once the clone finishes. Setting a lock through the API was not
tried.
- **F4.** Clone onto a taken VMID, synchronous:
`500 unable to create VM 199999: config file already exists`.
- **F5.** Agent enabled but not running: `500 QEMU guest agent is not running`.
Agent not enabled in the config: `500 No QEMU guest agent configured`.
Stopped VM with `agent: 1`: `500 VM 199999 is not running`.
- **F6.** Not produced. `local-lvm` is thin and overprovisions, so
`quota: absent`.
- **F7.** Config write while clone-locked: `500 VM is locked (clone)`.
- **F8.** `/cluster/resources?type=vm` entries carry `vmid, name, node,
status, type, template, tags, maxcpu, maxmem, maxdisk, uptime` and more;
`lock` appears only when set. Status values seen: `running`, `stopped`,
and `unknown` during a clone. **Mid-clone the entry has no `name`**: only
`type, vmid, id, node, lock: clone, status: unknown`.
- **F9.** Hyphenated tags are **accepted** and stored as given. Uppercase is
folded (`Proxy_Operator` → `proxy_operator`), the separator is `;`, and
characters such as a space or `!` give
`400 … tags: invalid format - invalid characters in tag`. The datacenter
has no `tag-style` or `user-tag-access` set.
- **F10.** `description` on clone is accepted and survives verbatim,
newlines included. The 22-character `proxy-<base32>` name is accepted.
- **F11.** The clone's `meta` is copied from the template
(`creation-qemu=11.0.3,ctime=1790145676`, identical to 9001), so it says
nothing about when the clone was made. `written-at-create` is required.
- **F12.** Upload: multipart with field `content=iso` and the file part named
`filename`, carrying the target file name. Returns a UPID for an
`imgcopy` task. A name without `.iso` gives
`400 … filename: wrong file extension`. **Re-uploading an existing name
silently overwrites it** (the task log shows a plain `cp`).
`Datastore.AllocateTemplate` suffices.
- **F13.** Deleting a volume needs `Datastore.Allocate` on
`/storage/<id>`; without it the call gives
`403 Permission check failed (/storage/local, Datastore.Allocate)`. With it
(on `proxy-seeds`), the delete **runs as an `imgdel` task** and returns a
UPID. Deleting a volume that no longer exists is also accepted and its
task still ends `OK`; the log reads `disk image '…' does not exist`, then
`Removed volume`. Seed cleanup is idempotent with no message matching.
- **F14.** Stopped: `status: stopped, qmpstatus: stopped`. Paused:
`status: running, qmpstatus: paused` in `status/current`, while
`/cluster/resources` shows plain `running`.
- **F15.** A clone onto a storage without image support is rejected up
front (`400 … storage 'local' does not support vm images`) and leaves no
VM. A clone failing mid-task was not observed.
- **F16.** Task status fields: `status` (`running`, `stopped`), `exitstatus`
(`OK` or the error text), `type, id, node, user, tokenid, upid, starttime,
pid, pstart`. A full clone of the 3 GiB template onto `local-lvm` finished
within about two seconds.
- **F17.** The planned role, granted on `/`, covered clone, config, resize,
start, stop, suspend, agent read, upload and destroy. **Missing:
`Datastore.Allocate` for deleting the seed ISO.** `VM.Config.Cloudinit` is
not needed for the `ide2` write. `SDN.Use` was held but never exercised,
because no call rewrote `net0`.
- **F18.** HTTP/1.1 only: `curl --http2` negotiates `HTTP/1.1`.
- **F19.** Template 9001 (`debian-13-template`): `scsi0` on `local-lvm`,
`boot: order=scsi0`, `ide2` free, `agent: 1`, OVMF, q35, `net0` untagged on
`vmbr0`. **Attaching `ide2` makes Proxmox append it to the boot order**
(`order=scsi0;ide2`). Template 9000 (Ubuntu) is unsuitable: it has a
Proxmox cloud-init drive on `ide2` and VLAN tag 4 on `net0`.
- **F20.** Resize **runs as a task** (returns a UPID). The same size is an
`OK` no-op; shrinking fails inside the task with `exitstatus`
`shrinking disks is not supported`.
- **F21.** Agent response: `result[]` of `{name, ip-addresses[{ip-address,
ip-address-type}]}`. The live guest reported `lo` and `ens18` with
`192.168.0.150`, 50 seconds after start, via DHCP on untagged `vmbr0`.
This proves the NoCloud-seed-on-`ide2` route end to end on this host.
- **F22.** A wrong secret and an unknown token id both give
`401 Authentication failed!`; no credentials gives `401 No ticket`.
- **F23.** Uploads are staged in `/var/tmp/pveupload-*` and then copied to
`/var/lib/vz/template/iso/`, so the tmpfs `/tmp` is not involved.
### Corrections to the plan that Phase 1 must fold into the spec
1. **Check `lock` before the name.** A mid-clone VM has no name in
`/cluster/resources` (F8), so name-first logic turns a clone in progress
into NotFound. Get, Delete and Create's lookup must treat a locked or
nameless entry at our VMID as in-progress (Transient or wait), never
absent.
2. **Inspect a taken VMID before probing forward.** After a crash mid-clone,
the retry's name lookup misses the nameless VM, the clone onto the
deterministic VMID fails with F4, and forward probing would clone a
duplicate. On F4, read that VMID: `lock: clone` means wait for it, and our
UID in the description means adopt it. Probe forward only for a VM that
is demonstrably someone else's.
3. **A start queues behind a lock** instead of failing (F3).
4. **Resize is asynchronous** (F20); wait on its task.
5. **Drop delete-before-upload** for the seed: upload overwrites (F12).
**Seed deletion is a task** (F13): wait on it. A repeat delete succeeds,
so Delete needs no absent-volume matcher.
6. **Get reads `status/current`**, not only `/cluster/resources`, to tell
paused from running (F14).
7. **Tag rationale**: hyphens are accepted on 9.2.20 (F9). `proxy_operator`
stays, since it is valid everywhere, but the claim that hyphens are
rejected is removed.
8. **The boot-disk logic must skip `ide2`**, which Proxmox adds to the boot
order (F19). The plan's rule already excludes it; add a test with
`order=scsi0;ide2`.
9. **Three agent messages, not one** (F5): `QEMU guest agent is not running`
(booting, or the install failed), `No QEMU guest agent configured` (our
`agent=1` was removed), and `VM <vmid> is not running` (stopped between
the status read and the agent call). The spec decides each class; none
may become NotFound.
10. **Internet on the proxy network is a hard prerequisite**, twice over: the
vendor-data agent install needs a package mirror, and the proxy itself
needs egress. The guide says so, and recommends baking
`qemu-guest-agent` into the template with `installGuestAgent: false` to
drop the mirror dependency from IP discovery.
### Decisions made with the user in Phase 0
- **Seeds go to a dedicated directory storage**, `proxy-seeds`, with ISO
content only. The token gets `Datastore.Allocate` there and nowhere else,
so it can delete seeds but never backups or other ISOs on `local`. The
sample config's `isoStorage` becomes `proxy-seeds`. The guide must say that
an ACL on a more specific path replaces the inherited one for that user,
so the role granted on `/storage/proxy-seeds` must repeat
`Datastore.AllocateTemplate`, `AllocateSpace` and `Audit` alongside
`Allocate`.
- **Proxies land on a VLAN** set in config. `ProxmoxConfig` gains an
optional `vlanTag` (1 to 4094), applied when Create rewrites `net0`
together with the optional bridge override, keeping the MAC. Unset leaves
`net0` as cloned. This is the first call that exercises `SDN.Use` (F17).
- **The API URL is `https://192.168.0.8:8006`**, the address in the node
certificate, so `caFile` verifies with no provider change. No
`tlsServerName` option. The operator's cluster must be able to reach
192.168.0.x.
### Seed storage and VLAN 4
The user created `proxy-seeds` (`dir`, ISO content only), granted the token
`Datastore.Allocate`, `AllocateSpace`, `AllocateTemplate` and `Audit` on
`/storage/proxy-seeds` through a role `ProxyOperatorSeeds`, and freed the
leftover recon ISO on `local`. An upload to `proxy-seeds` and two deletes of
a throwaway ISO there produced `content_delete_response.json`,
`content_delete_again_response.json` and `task_status_volume_missing.json`.
The user gave the VLAN as "vlan 4, bridge: vmbr0.4@vmbr0". That is the
host's own VLAN interface name as `ip link` prints it; a VM NIC cannot
attach to it because it is not a bridge. On the VLAN-aware `vmbr0` the
Proxmox form is `bridge=vmbr0,tag=4`, which is what template 9000 uses, so
the provider's `vlanTag: 4` appends `tag=4` to the cloned `net0`.
A second clone of 9001 at VMID 199999 got its seed from `proxy-seeds` and
`net0` rewritten from `virtio=BC:24:11:EE:48:83,bridge=vmbr0` to
`virtio=BC:24:11:EE:48:83,bridge=vmbr0,tag=4`. **The write succeeded, so the
token's `SDN.Use` on `/` covers a VLAN tag on `vmbr0`**
(`set_config_vlan_response.json`).
**The first VLAN 4 attempt never produced an agent** in seven minutes of
polling (every 13 seconds, 33 polls). The only signal was the agent call, so
the cause could not be seen from the API. A debug seed added a `recon` user
(user-data only, with a new instance id so cloud-init re-ran), and the user
logged in on the console: DHCP worked, but VLAN 4 had no route to the
internet because of a routing misconfiguration on the Proxmox host, so the
vendor-data `apt-get install qemu-guest-agent` could not run. The user fixed
the routing.
**Rerun from scratch after the fix**, same sequence the provider will use
(destroy and seed delete first, then a fresh clone and seed, `instance-id`
equal to the name):
| Step | Elapsed |
|---|---|
| clone task done | 4 s |
| seed upload task done | 4 s |
| config (`ide2`, `net0` with `tag=4`, tag) | 4 s |
| start task done | 5 s |
| agent reports `192.168.4.147` | 47 s after start |
The guest was reachable from the workstation (ping, TCP 22), as the contract
requires of a proxy address. Stopping it gave the F5 stopped-VM fixture;
destroy and seed delete then left the host with only its original VMs 100,
9000 and 9001, no ISOs on `local` or `proxy-seeds`, and no `local-lvm`
volumes of 199999. The capture is `agent_network_get_interfaces_vlan.json`.
### Worth noting
- The token file is mode 664; 600 would be tighter.
- Fixtures: 48 files, all valid JSON, none containing `PVEAPIToken`.
Several names differ from the plan's list because what they captured
differed from what was expected: `start_while_clone_locked_response.json`
(the start succeeded), `task_status_stopped_error.json` (from the shrink,
not a start), `upload_overwrite_response.json` (success, not an error),
`resize_same_size_response.json` and `resize_shrink_response.json`
(UPIDs), `err_forbidden_volume_delete.json` (the 403 from F13).
`err_no_space.json` was not produced (F6).
- The host is back to its original state; see the table above.
- The scratch seed builder (`seedgen` in the session scratchpad) took the
user-data file and instance id as arguments by the end, for the debug
login. It is not part of the repository; Phase 3's `nocloud` package
replaces it.
- Time to IP was about 50 s on both networks, with the agent installed at
first boot. Budgets in the plan (running wait 60 s for the VM state, the
agent discovered by `Get` polling) are consistent with this.
## Phase 1 — The spec
Wrote [docs/proxmox-provider-spec.md](../proxmox-provider-spec.md), all 24
entries of the template, from the plan, the contract and the Phase 0 facts.
The ten Phase 0 corrections are folded in, and the plan now opens with a
note that the spec wins where the two disagree.
Judgment calls made here, each recorded in its entry:
- **Create's allocation loop** (entry 2) runs an ownership check on any
taken VMID: wait out a lock or a nameless entry, then adopt when the name
and UID are ours, skip when the name differs, refuse as Permanent when the
name is ours but the mark is not. This is the fix for the duplicate-VM bug.
- **Get's check order** (entry 9): lock, then name and mark from the config,
then `status/current`, then the agent. `lock: suspended` alone maps to
Stopped, since a hibernated VM never resumes by itself.
- **Agent messages** (entry 7): `QEMU guest agent is not running` is
Provisioning, `VM <vmid> is not running` is Transient, and
`No QEMU guest agent configured` is **Stopped**, so the reconciler replaces
a VM that can never report an address instead of latching the Proxy.
- **Templates are checked before cloning** (entry 4): not a template, or a
Proxmox cloud-init drive in any slot, is Permanent. This makes the "no
cloud-init drive" prerequisite enforceable instead of a silent failure.
- **`isoStorage` is required**, with no default (entry 15), because the token
must hold `Datastore.Allocate` on it.
- **TLS verification failures are Permanent**, with a Checker reason
`CertificateInvalid` (entries 13, 16), since the fix is the URL or CA file.
- **The CA is loaded lazily and cached**; the token is read per request
(entry 14).
Checks, as the plan's Verify clause asks:
```bash
grep -c '^## [0-9]*\. ' docs/proxmox-provider-spec.md # 24
grep -n '^\*\*Answer:\*\*$' docs/proxmox-provider-spec.md # nothing
grep -c '—' docs/proxmox-provider-spec.md # 0
```
plus a script comparing every `` `<axis>`: `<profile>` `` pair against the
contract's §4.20 table (no mismatches, every axis answered) and a list of
the F numbers cited (all 23).
Worth noting: the first pass stated two things as fact that recon could not
show and was corrected before commit. That an unreachable node's guests stay
listed as `unknown` is Proxmox's documented design, unverifiable on a single
node; entry 13 now says so. And the matcher for `VM <vmid> is not running`
is a whole-message regexp, because ` is not running` alone also matches
`QEMU guest agent is not running`.
## Phase 2 — Config block, registration, package skeleton
Added the `proxmox:` block (`internal/provider/config_proxmox.go`), wired it
into `config.go` (the field, `setBlocks`, the allow-list and the dispatch:
three `"proxmox"` sites) and registered `"proxmox": proxmox.New` in
`cmd/main.go`, so the two type lists changed in one commit. The package
`internal/provider/proxmox` now holds the seam interface and its data types
(`api.go`), `id.go`, `mark.go`, `state.go`, `errors.go` and the constructor
(`proxmox.go`). The four contract methods are stubs that answer
`ErrTransient`, so a Proxy on a configured `proxmox` entry retries instead of
latching until Phases 5 to 7 land. `New` passes a nil `api` until Phase 4
builds the HTTP client.
Every test that depends on Proxmox's wording or JSON shape reads the Phase 0
captures through `fixtures_test.go` (`readCapture`, `loadAPIError`,
`decodeData`), so no message string is repeated in a test. The one exception
is `TestClassify_table`'s synthetic rows (a 404, 502, 595, 596, a storage
message), for statuses recon never produced.
Deviations from the plan's Phase 2 list, both making it smaller:
- **No temporary `locks.go`.** Only Create and Delete take the per-name lock,
and they arrive after Phase 3 extracts `internal/provider/keyedlock`, so
copying kvm2's lock now would only be deleted next phase.
- **No `provider.Checker` assertion yet.** `cmd/main.go` type-asserts the
Checker and would call a stub; the assertion comes with the real `Check`
in Phase 7.
Added beyond the plan: `api_test.go` decodes the captured responses into the
seam's types, so a misspelled JSON tag fails now rather than in Phase 5, and
`TestParseDescription_fromClone` reads the mark back from the description of
the VM cloned in Phase 0.
A mutation check confirmed the suite can fail. Matching `VM <vmid> is not
running` as a plain suffix (the first spec draft's mistake) failed
`TestAPIError_matchers` on the agent-not-running capture; treating a paused
VM as running failed `TestMapState_table` and `TestMapState_fromCaptures`.
Both mutations were reverted and the files compared identical to backups.
```bash
gofmt -w cmd/main.go # realigned the constructor map for the longer key
grep -c '"proxmox"' internal/provider/config.go # 3
go build ./... && go vet ./...
go test -race -count=1 ./internal/provider/... ./cmd/... # all ok
go test -short -count=1 ./... # all ok
go test -count=1 -v ./internal/provider/proxmox/ | grep -c '^=== RUN' # 152
```
Worth noting: the `validProxmoxYAML` constant is the shape Phase 9's commented
sample entry should take, with the user's values (`192.168.0.8`, `pv1-1`,
`proxy-seeds`, `vlanTag: 4`, template 9001).
## Phase 3 — Extract `nocloud` and `keyedlock` from kvm2
kvm2's `seed.go` and `seed_test.go` moved to `internal/provider/nocloud`
with `git mv`, so history follows them. The package exports `BuildSeedISO`,
`BuildVendorData`, `GuestAgentScript`, `SeedLabel` and a new `ErrEmptyName`
sentinel in place of kvm2's `permanentf`. kvm2's `keyedLocks` became
`internal/provider/keyedlock` (`Locks`, `New`, `Acquire`), and its three
`TestKeyedLocks_*` tests moved with it under their old names. kvm2 now
imports both packages, and its `create.go` still wraps every seed-build error
with `permanentf`, so kvm2's behaviour is unchanged. proxmox gained the
`nameLocks` field and `lockName`.
The test counts match the plan's Verify clause: kvm2 dropped from 465 to 447
`=== RUN` entries, which is the 12 moved tests plus the 6 subtests of
`TestSeedISO_userDataByteForByte`. `nocloud` (15) and `keyedlock` (3) carry
exactly those 18.
```bash
git mv internal/provider/kvm2/seed.go internal/provider/nocloud/seed.go
git mv internal/provider/kvm2/seed_test.go internal/provider/nocloud/seed_test.go
go test -count=1 -v ./internal/provider/kvm2/ | grep -c '^=== RUN' # 465 before, 447 after
grep -rn 'iso9660' internal/provider/kvm2/ # nothing
go build ./... && go vet ./... && go vet -tags=integration ./...
go test -race -count=1 ./internal/provider/... # all ok, run three times
go test -short -count=1 ./... # all ok
```
**Pulled forward from Phase 6 so proxmox imports `nocloud` now**, as the
"Done when" line requires: `seed.go` with `seedISOName`, `seedVolID` and
`buildSeed`, the pure part of the seed step. `uploadSeed` still belongs to
Phase 6, which calls `buildSeed`. The new tests are `TestSeedNames_table`,
`TestBuildSeed_table` (4 rows),
`TestBuildSeed_emptyNameIsPermanentWithoutPayload`,
`TestBuildSeed_stagingFailureIsTransient` and
`TestLockName_givesUpWithContextAsTransient`. proxmox went from 152 to 161
test entries.
**proxmox does not copy kvm2's open review finding S5.1**
(`docs/reviews/2026-09-17-kvm2-provider-review/sweep.md`). In kvm2, a
staging failure such as a missing `/tmp` or a full emptyDir becomes
Permanent, and that latches the Proxy. `buildSeed` makes only
`ErrEmptyName` Permanent and wraps any other error with `%w`, so
`classify` reads it as Transient. The sentinel makes the same three-line
fix possible in kvm2, but this phase leaves kvm2's behaviour unchanged.
Worth noting:
- The older `kvm` provider has its own `seed.go` with a different
signature (`buildSeedISO` returns `[]byte`). The plan scopes the
extraction to kvm2, so `kvm` is untouched.
- `GuestAgentScript` now says the operator reads the address "from the
hypervisor" instead of "through libvirt", in both the Go comment and the
script's comment line inside the guest. The script's commands are
unchanged.
- The docs now name the new packages: `kvm2-provider.md`,
`kvm2-provider-spec.md` entry 19, the kvm2 columns and section of
`providers-existing.md`, plus a "Shared provider helpers" row in the
`architecture.md` component table. `docs/reviews/` keeps the old names,
because it records the code as it was reviewed.
- `TestCheck_hostsCheckedConcurrently` in kvm2 failed once under the full
`-race` suite (latency 199.96 ms against a 200 ms floor). It passed 20
runs out of 20 in isolation, with and without this phase's changes. The
test starts its 200 ms release timers before `Check` starts its clock, so
under load the measured latency can come in just under 200 ms. The flake
predates this phase, and the test is not touched here.
## Phase 4 — Seam, HTTP client, decorator, fake
`client.go` holds `httpAPI`, the thin `net/http` adapter for all sixteen
seam methods. It reads the token file on every request, turns any non-2xx
answer into `*apiError` from the body's `message` or the reason phrase
(F1), and wraps its transport in `tracing.RestConfigWrapper`. `observe.go`
is the logging decorator: one V(1) `proxmox api call` line per call,
detail at V(2), and no span, because the transport already traces each
request. `New` now builds `observe(newHTTPAPI(...))` instead of passing a
nil `api`, still without touching the network or the token file.
`fake_test.go` holds `fakeAPI`, which Phases 5 to 7 build on.
Where this phase departs from the plan's Phase 4 text:
- **The `http.Client` is built on first use, not in `newHTTPAPI`.** Spec
entry 14 wins over the plan: the CA is loaded lazily, a missing CA file
is Transient and retried until the file lands, one without a certificate
is Permanent, and the pool is cached only once it loads. A transport's
`RootCAs` cannot change after first use, so the whole client waits for
the CA (`TestClient_caFileLoadedLazily`).
- **The upload is not an `io.Pipe` with a goroutine.** The multipart
framing (the `content=iso` field, the `filename` part header and the
closing boundary) is rendered into two small buffers, and the body is
`io.MultiReader(head, io.LimitReader(r, size), tail)` with an exact
`Content-Length`. The seed is still streamed and never held by the
client. There is also no goroutine to leak, and the request carries a
length, as Phase 0's `curl -F` upload did. A chunked upload to pveproxy
was never tried. `TestClient_uploadStreamsMultipart` proves the
streaming with a reader that stalls after its first 256 KiB until the
server has seen file bytes.
- **The fake's `DeleteVolume` tolerates a missing volume**, as spec
correction 5 and F13 say, instead of the plan's error.
- **Added:** secret redaction in `apiError` (contract §7), because
otherwise a server that echoes the header would put the secret into
`Proxy` status; an `errCredentials` sentinel on token-file errors, so
Phase 7's Checker can report `CredentialsUnavailable`; and the V(1)
identifiers `newid` and `name` on a clone, `filename` on an upload and
`volid` on a volume delete. Spec entry 20 now lists them.
**The volid's `/` travels as `%2F`.** Neither the plan nor the spec says
how `{volid}` is encoded in the content-delete path. The Phase 0 session's
transcript shows every delete went through a helper encoding the whole
volid:
```bash
enc() { python3 -c "import urllib.parse,sys; print(urllib.parse.quote(sys.argv[1], safe=''))" "$1"; }
curl ... -X DELETE ".../storage/proxy-seeds/content/$(enc 'proxy-seeds:iso/proxy-reconaaaaaaaaaaa-seed.iso')"
```
So `apiPath` runs `url.PathEscape` over each segment
(`TestClient_volIDSlashIsEscaped`).
`net/http` cannot send a custom reason phrase, so the client tests replay
every capture byte for byte through a hijacked connection (`serveCapture`).
`TestClient_apiErrorFromStatusReasonAndBody` globs `testdata/err_*.json`
and skips `err_tag_hyphen.json`, which is a 200 despite its name: hyphenated
tags were accepted (F9).
Tests beyond the plan's list: `TestClient_methodPathAndParams` (the method,
path, query and form of every seam method, with a reflection check that no
method lacks a row), `TestClient_cloneOmitsEmptyStorageAndTarget`,
`TestClient_volIDSlashIsEscaped`, `TestClient_caFileLoadedLazily`,
`TestClient_x509FailureIsPermanent` (from spec entry 21),
`TestDecodeConfig_scalarsAsStrings`, `TestReadToken_trimsTrailingNewline`,
`TestObserve_startsNoSpan`, `TestObserve_identifiersAtV1`,
`TestNew_wiresObservedHTTPClient`, and six `TestFakeAPI_*` tests.
`TestFakeAPI_errorsMatchCaptures` compares the fake's error texts with the
captures, so every test that goes through the fake exercises the real
wording. Texts that recon never produced start with `fake:`.
A mutation check confirmed the new tests can fail: dropping `redact`,
buffering the upload with `io.ReadAll`, and logging V(2) detail at V(1)
each failed their test. Removing only the empty `TLSNextProto` map did
**not** fail `TestClient_http1Only`. A transport with a custom
`TLSClientConfig` and `DialContext` never offers h2 unless
`ForceAttemptHTTP2` is set, so the map is a second guard. With both
guards removed the test failed with `client spoke HTTP/2`. Every mutation
was reverted, and the files were compared identical to backups.
```bash
go test -race -count=2 ./internal/provider/proxmox/ # ok, run three times
grep -n 'ReadAll' internal/provider/proxmox/client.go # only the 64 KiB error-body read
grep -n 'context.Background()' internal/provider/proxmox/*.go | grep -v _test.go # nothing
go build ./... && go vet ./... && go vet -tags=integration ./...
go test -short -count=1 ./... # all ok
go test -count=1 -v ./internal/provider/proxmox/ | grep -c '^=== RUN' # 161 before, 289 after
```
Worth noting:
- `bin/golangci-lint run ./internal/provider/proxmox/...` panics inside
staticcheck's IR builder (`unexpected expr: *ast.KeyValueExpr` in the
Go 1.27 standard library's `poll` package). This is a toolchain problem,
not this code. With `--disable staticcheck` the new files' findings are
goconst on form keys (kvm2 carries 50 of the same) and two unparam hits
on the fake's `addTemplate` and `resourceByID`, which Phases 5 and 6 will
call with other VMIDs. The mapsloop and gocyclo findings in `proxmox.go`
and `api_test.go` date from Phase 2.
- Nothing in this phase talked to the host. The client has only met
`httptest` servers replaying the captures. The first real request is
Phase 10's integration run, which is where the `Content-Length` upload
and the `%2F` volid get proven against pveproxy.
- The fake refuses to destroy a running VM with an uncaptured
`fake: VM <vmid> is running - destroy failed`, so Phase 7's
stop-before-destroy order is enforced by the fake even though the real
text is unknown.
## Phase 5 — Get, ListByTag, task helpers
`task.go` holds `parseUPID`, a shared `pollUntil` loop, `waitTask` and
`waitUnlocked`. `get.go` holds `Get`, the guest-agent read and `resolve`
(one resources call, QEMU entries only, which Delete will reuse). `list.go`
holds `ListByTag` and its candidate rule. The Get and ListByTag stubs are
gone, and `TestStubs_answerTransient` now covers only Create and Delete.
The spec overrides the plan in the places the Phase 4 notes listed. Get
checks in entry 9's order, so a lock or `status: unknown` in the resources
entry is Transient before the name is ever read, and a running Get makes
four calls (`TestGet_runningMakesFourCalls`). The three agent messages
follow entry 7. The poll interval is `budgets.poll`, and a failed task logs
a V(1) `proxmox task failed` line with `upid` and `exitstatus`.
Judgment calls, most of them spec gaps:
- **`WARNINGS: <n>` counts as a successful task.** Proxmox reports a task
that finished but logged warnings with that exitstatus and treats it as
finished, not failed. Under the spec's old "anything but `OK` is
Transient" rule, a start that logged a warning would make Create retry
forever. Recon never produced one, so this row relies on how Proxmox is
known to behave, not on a capture. The spec's entry 10 text and entry 13
table now say so.
- **A `suspended` lock still goes through the name and mark checks.** The
entry 9 table puts "lock suspended is Stopped" before the config rows.
Taken literally, a hibernated VM that reused our VMID would be reported
as ours and Stopped, and Delete would then skip it as foreign in a loop.
Get therefore reads the config, which it needs anyway for UID and
CreatedAt, applies the name and mark checks, and only then answers
Stopped. It skips `status/current` and the agent. Entry 9 now says this.
- **`TestGet_twoVMsWithOurNameIsPermanent` became
`TestGet_otherVMWithOurNameIsIgnored`.** Spec entries 2 and 9 address Get
by VMID, and only Create refuses a duplicate name. Get answering
Permanent would latch a working Proxy as Failed because of a VM someone
else created.
- **`TestGet_unmanagedDescriptionHasEmptyUID` tests a mark with no `uid`
line.** The plan's name read literally ("unmanaged") would contradict
"no mark is NotFound".
- **Agent errors outside the entry 7 table are forced to Transient**, even
a 403 or a `does not exist`. Otherwise `classify` would make them
Permanent or NotFound, and the spec says "anything else: Transient".
- **`waitUnlocked` returns the unlocked config**, which saves Create's
ownership check one read. When the VM disappears while it waits (a
failed clone), it returns the does-not-exist error and lets Create
decide.
Pulled forward and left behind:
- **`provider_test.go` now exists** with `newFixture`, `testBudgets`,
`addOurVM` and `assertProviderError`, because every Get and ListByTag
test needs them. The plan created this file in Phase 8. Phase 8 adds the
contract sweep to it.
- **`waitRunning` is left to Phase 6.** The plan's package layout puts it
in `task.go`, but Phase 5 lists no test for it and only Create uses it.
It should be built on `pollUntil`.
- **The fake gained two things.** `fakeVM.kind` makes an entry listed as
`lxc` invisible to the qemu endpoints. `hookOn` is a general per-method
hook, and `failWith` is now built on it. Tests use `hookOn` (through
`mutateOn` in `get_test.go`) to change a VM between two of Get's calls.
A mutation check confirmed the new tests can fail. Each of these failed a
test: classifying agent errors instead of forcing Transient; dropping the
config lock check; dropping the `suspended` special case; ignoring an ended
ctx in ListByTag; treating `WARNINGS` as failure; and not mapping
`No QEMU guest agent configured` to Stopped. Removing the resources-entry
lock and `unknown` checks together failed only
`TestGet_unknownStatusIsTransient`. The fake reports a lock in the config
as well, so the config check catches every lock row, including a mid-clone
VM. The resources check stays because the spec orders it first and
because it spares a config read. Every mutation was reverted, and the
files were compared identical to backups.
```bash
go test -race -count=2 ./internal/provider/proxmox/ # ok
grep -n 'not implemented' internal/provider/proxmox/*.go | grep -v _test # Create and Delete only
go build ./... && go vet ./... && go vet -tags=integration ./...
go test -race -count=1 ./internal/provider/... ./cmd/... # all ok
go test -short -count=1 ./... # all ok
go test -count=1 -v ./internal/provider/proxmox/ | grep -c '^=== RUN' # 289 before, 393 after
```
Lint ran with staticcheck disabled, because staticcheck panics on
Go 1.27:
```bash
bin/golangci-lint run --disable staticcheck ./internal/provider/proxmox/...
```
Worth noting:
- The lint findings in the new files are goconst on test strings and
logcheck on `get`, `agentAddress` and `waitTask` taking both a ctx and a
logger. That shape copies kvm2, which has five of the same findings. The
errcheck hits on `assertGetFails` were fixed by returning the error text.
The two unparam hits and the mapsloop hits predate this phase.
- `waitUnlocked` waits on any lock, `suspended` included. Phase 6's
ownership check should decide whether a hibernated VM at our VMID is
worth waiting out the clone budget for, or should be refused at once.
- Nothing in this phase talked to the host. Every Proxmox message a test
relies on comes from `testdata/` or from fake errors that
`TestFakeAPI_errorsMatchCaptures` ties to the captures.
## Phase 6 — Create
`create.go` holds `Create` and its steps: placement, the by-name lookup,
the template check, the VMID probe with its ownership check (`inspect`),
the clone, and `provision` (seed, config, resize, start). `configParams`,
`rewriteNet0`, `resizeTarget`, `bootDisk` and `diskBytes` are pure helpers
with their own table tests. `seed.go` gains `uploadSeed`, which uploads
over any existing file with no delete first (F12). `task.go` gains
`waitRunning`, built on `pollUntil` with `budgets.running`. It finishes
only at `running`/`running`, and a `stopped` status after the start is
Transient at once. The Create stub is gone, and `TestStubs_answerTransient`
now covers only Delete.
The spec won over the plan wherever the Phase 6 notes said it would. The
plan's allocation loop is replaced by entry 2: a taken VMID and a clone
answering F4 both go through the ownership check before any probing, and a
locked or nameless entry is waited on with `waitUnlocked` and the clone
budget. The seed is uploaded without a delete. The resize is a task. `net0`
changes only when `bridge` or `vlanTag` is set, and `tag=` is replaced or
appended in place.
Judgment calls, with spec entries 2, 4 and 10 updated to match:
- **The Phase 5 open question: a `suspended` lock is not waited on.**
`waitUnlocked` now returns at once when it sees `suspended`, because a
hibernated VM is not a task and would otherwise burn the 10-minute clone
budget on every attempt. The ownership check then judges the VM the way
`Get` does. A foreign hibernated VM is probed past. Ours is Permanent
("resume or destroy it"), following kvm2's precedent for a paused or
pmsuspended domain in Create. Transient was the alternative: it would
self-heal once an admin acts, but it would retry indefinitely on
something only an admin can fix. `TestWaitUnlocked_table` has a new row,
and `TestCreate_hibernatedVMIsNotWaitedOn` covers both cases.
- **The template is checked only when a clone is needed.** Spec entry 10
read the template's config in step 2, before the by-name lookup. A retry
for a VM that already exists would then fail Permanent if the template
had been deleted since, which entry 4 explicitly allows. The template
check now runs after the by-name lookup misses, so a repeat Create costs
three calls: resources, config and status
(`TestCreate_adoptionDoesNotNeedTheTemplate`).
- **The template's shape is checked before the clone.** No `net0` while
`bridge` or `vlanTag` is set, or no boot disk (or none with a readable
`size=`) while `diskGiB` is set, is Permanent. Found after the clone, the
VM would already carry the bad config, so fixing the template could not
help that Proxy's next attempt. `template()` runs the same
`configParams` and `resizeTarget` over the template's config that
`provision` later runs over the clone's.
- **An empty `req.Name` is Permanent at placement.** A nameless request
would match nameless mid-clone entries in the by-name lookup.
- **Adopted VMs get one `status/current` read** rather than trusting the
listing's status, which may be stale after a lock wait. A running VM,
whatever its `qmpstatus`, returns its ID at once, as entry 10 says. `Get`
then reports prelaunch as Provisioning and paused as Stopped.
- **After the clone task, Create reads the config through
`waitUnlocked`.** That read supplies the `net0` and boot disk the later
steps need, and it turns a clone that ended but kept its lock (F15) into
a Transient lock wait rather than a refused config write.
- **An F4 whose VMID then resolves to no QEMU VM probes on.** Either the
racer's clone failed and removed it, or a container took it.
- **Errors carry no ID** (`wrapErr("create", "", …)`), since the VMID is
known only after allocation. The messages name the VMID wherever one
exists.
Test names: every plan and entry 21 name exists except
`TestCreate_resizeOnlyWhenLarger`. It is covered by the spec's pair:
`TestCreate_resizeWaitsForTask` for the larger case and
`TestCreate_resizeSkippedWhenNotLarger` for equal, smaller and unset. Tests
beyond both lists: `TestCreate_vmThatStopsAfterStartIsTransientAtOnce`,
`TestCreate_hibernatedVMIsNotWaitedOn`,
`TestCreate_templateShapeRefusedBeforeClone`,
`TestCreate_adoptionDoesNotNeedTheTemplate`,
`TestCreate_cloneTargetAndStorage` (read from the decorator's V(2) clone
line), `TestCreate_adoptedVMStaysOnItsNode`, `TestCreate_outcomeLine`,
`TestCreate_quietAtV0`, `TestRewriteNet0_table`, `TestBootDisk_capturedClone`
and `TestDiskBytes_table`. The only new test helpers are the small ones in
`create_test.go` (`slot`, `newCreateFixture`, `req`/`create`/`createFails`,
`assertRunning`, `callsBefore`, `clearLockOnVMConfig`). Everything else
reuses the Phase 5 fixture and fake. Two tests check config-write contents
without a recording wrapper. `TestCreate_markWrittenInCloneCall` reads the
VM at the start of `UploadISO`, which is after the clone and before the
config write. `TestCreate_noBridgeOverrideLeavesNet0` changes `net0` at the
start of `SetVMConfig`, so a write that carried `net0` would put the old
value back.
A mutation check applied 18 mutations, and each failed at least one test.
They were: no lock wait in the ownership check; taken VMIDs never
inspected; the F4 race skipping the ownership check; no cloud-init drive
check; upload or resize task not waited on; resize at equal size; a
CD-ROM counted as a boot disk; prelaunch counted as running; the suspended
lock waited on; `net0` always written; `bridge=` appended instead of
replaced; duplicate names ignored; a running VM re-provisioned; `target`
always set; a foreign UID adopted; the tag dropped; probes not capped by
the range. The last one was first **missed**. A 4-VMID range with 32
probes wraps, re-inspects each VM eight times and still ends Transient, so
`TestCreate_vmidRangeExhaustedIsTransient` now pins five config reads.
Five mutations did not compile on the first try and were redone. Every
mutation was reverted, and the files were compared identical to backups.
```bash
go test -race -count=2 ./internal/provider/proxmox/ # ok
grep -n 'CloudInit' internal/provider/proxmox/create.go internal/provider/proxmox/seed.go
# seed.go: the pass-through into nocloud.BuildSeedISO; create.go: len(req.CloudInit)
grep -n 'not implemented' internal/provider/proxmox/*.go | grep -v _test # Delete only
grep -n 'context.Background()' internal/provider/proxmox/*.go | grep -v _test.go # nothing
go build ./... && go vet ./... && go vet -tags=integration ./...
go test -short -count=1 ./... # all ok but the parked kvm2 flake, below
go test -count=1 -v ./internal/provider/proxmox/ | grep -c '^=== RUN' # 393 before, 519 after
bin/golangci-lint run --disable staticcheck ./internal/provider/proxmox/...
```
Worth noting:
- `go test -short ./...` failed once on the parked kvm2 flake
`TestCheck_hostsCheckedConcurrently` (measured 199.98 ms against a
200 ms floor). Three reruns passed. This phase does not touch kvm2.
- The new lint findings follow the baseline's shape. There are logcheck
hits on `create`, `allocate` and `clone` (ctx plus logger, as in kvm2),
and goconst on test strings. One more unparam than the notes list:
`(*fakeAPI).iso`'s `storage` always receives `fakeISOStorage`, because
this phase is its first caller. `addTemplate`'s hit changed from a
constant VMID to an unused result.
- The config write sets `agent=1` and `tags=proxy_operator` as values. A
template's agent options (such as `fstrim_cloned_disks=1`) and its own
tags are replaced, not merged. That is what entry 10 specifies, and the
guide should say so.
- In the fake, a cross-node clone puts the config on the target at once.
Real Proxmox may move it at the end of the task, in which case a lock
wait on the target node would see `does not exist` and the next attempt
would find the VM. This stays untested, like every cross-node path
(entry 24).
- Nothing in this phase talked to the host.
## Phase 7 — Delete, Checker, CRD comment
`delete.go` holds `Delete`: parse, the per-name lock, `destroy` (resolve,
the lock and status checks, config, ownership, stop, destroy) and the seed
delete. `check.go` holds `Check`, which fans out one `checkNode` per node
with `wg.Go` and returns the verdicts in config order, plus `checkReason`
and the four provider reasons. The Delete stub and
`TestStubs_answerTransient` are gone, and `grep -rn 'not implemented'
internal/provider/proxmox/` is empty. All four contract methods and
`Check` are real. The `status.type` comment in
`api/v1alpha1/providerhealth_types.go` now ends `kvm2 or proxmox`, and
`make manifests` changed only that description line in the ProviderHealth
CRD.
The spec won over the plan as the Phase 7 notes said. An absent VM still
gets its seed deleted, on every configured node. A task waits on the seed
delete, and nothing matches F13's absent-volume message. A paused VM gets
the stop too. The Checker's reasons include `CertificateInvalid` and
`CredentialsUnavailable`, and pveproxy's 595 and 596 are `Unreachable`.
Open questions decided (spec entries 11 and 16 updated):
- **Our hibernated VM (`lock: suspended`) is Permanent in Delete:** "resume
or destroy it in Proxmox", the same answer Create's ownership check
gives. Proxmox's destroy refuses a locked VM (the fake answers with F7's
`VM is locked (…)` text; the suspended variant was never captured), and
that lock never clears on its own. Transient would retry forever. A
Permanent from Delete in `reconcileManaged`'s Stopped branch goes through
`providerFailure` and latches the Proxy with the admin action in its
status. On the finalizer path `deletionFailure` keeps retrying, and the
retries can be seen. Nothing captured shows what a hibernated VM does on
a stop or start through the API, so there was no evidence for a
self-healing path (for example, start to resume, then stop and destroy).
A hibernated VM that is not ours is left alone. The resources-entry lock
check skips `suspended`, so ownership is judged from the config, as in
Get.
- **The Checker reads `p.cfg.TokenFile` itself, through the client's
`readToken`**, before its first request, and keeps only the part before
`=`. A seam method would break the rule of one method per endpoint, and
both `TestClient_methodPathAndParams` and
`TestObserve_everySeamMethodLogsOneLine` enumerate the interface. An
unusable file answers `CredentialsUnavailable` without a request.
`readToken`'s errors never carry the content. Every row of
`TestCheck_reasonsTable` runs at V(2) over the decorator with a sentinel
secret and asserts that the secret is in no message, detail or log line.
Other judgment calls:
- **The seed is deleted on the VM's node only when Delete destroyed a VM
of ours; otherwise on every node.** Entry 11 said "skip to step 7" for
a VM that is foreign, unmarked, or gone by the time the config is read,
without naming a node. In each of those cases our VM's node is unknown:
the node in the listing belongs to someone else's VM, or to one that
just vanished. A container at the VMID takes the same path.
- **Stop's `does not exist` means the VM is gone**, the same as Destroy's.
The seed is then deleted on its node, and Destroy is not called.
- **`errCAFile` joins `errCredentials` in `client.go`.** A missing CA file
or one without a certificate is `CertificateInvalid` ("fix `caFile`").
Without the sentinel it would be `APIError`. Its class is unchanged
(Transient or Permanent, as entry 14 says).
- **Any 500 from the storage status call is `StorageNotFound`**, carrying
Proxmox's text. The plan matched a 500 "does not exist", but no response
for a missing storage was captured. Matching an invented string would go
against the fixture discipline, and every 500 there (missing, not
enabled on that node, offline) is fixed on the storage anyway. A 401,
403, 595 or 596 keeps its shared reason.
- **`checkReason` puts `*net.OpError` before the deadline check**, so a
dial that runs out of its own 10 s budget is `Unreachable`, not
`Timeout`. A request timing out on its own while the round still runs
is `Timeout`. The round's ctx ending is `Timeout` whatever the error
says, and that guard is in `checkNode`, as in kvm2.
- **The outcome line `proxmox vm deleted` carries `destroyed`**, which is
false when there was no VM of ours left to destroy. It has no `node` in
that case. Entry 20 lists the new V(1) no-op lines.
- `templates` counts catalogue entries, not unique VMIDs.
Test names. These plan names changed with the behaviour:
`TestDelete_absentIsNoOp` → `TestDelete_absentStillDeletesSeedOnEveryNode`,
`TestDelete_foreignNameIsNoOp` → `TestDelete_foreignNameLeavesVMAlone`,
`TestDelete_unmarkedIsNoOp` → `TestDelete_unmarkedLeavesVMAlone`, and
`TestDelete_toleratesMissingISO` merged into entry 21's
`TestDelete_seedDeleteIsTaskAndIdempotent`, which runs with the seed both
present and absent and ties the absent case to
`task_status_volume_missing.json`. Every other plan and entry 21 name
exists. `TestDelete_usesPurgeAndDestroyUnreferencedDisks` runs Delete over
the real `httpAPI` against the Phase 0 destroy captures
(`cluster_resources_during_destroy`, `config_during_destroy`,
`destroy_response`, `content_delete_response`,
`task_status_volume_missing`). `TestGet_duringDeleteStillReturnsInstance`
holds the destroy task in its first poll and first checks that the
mid-destroy captures show the VM listed, stopped and without a lock.
Tests beyond both lists: `TestDelete_containerAtVMIDIsLeftAlone`,
`TestDelete_hibernatedVMIsPermanentIfOurs`,
`TestDelete_seedDeleteFailureOnVMNodeFails`,
`TestDelete_seedDeleteNodeFailureSkippedWhenNodeUnknown`,
`TestDelete_goneMidwayIsSuccess`, `TestDelete_resourcesFailureIsTransient`,
`TestDelete_waitsOnTheNameLock` (a held name lock means no call at all,
which is what makes a Delete racing a Create wait),
`TestDelete_cancelledMidWaitReturnsPromptly`, `TestDelete_outcomeLine` and
`TestCheck_reasonsOverHTTPClient`, which takes the transport reasons from
real net/http and crypto/tls errors: unknown CA, hostname, missing CA file,
a replayed 401, 596, a closed server and a cut response. The fake gained
`destroyFailsWith`. New test helpers are only the small ones in
`delete_test.go` (`putSeed`, `seedPresent`, `delete`/`deleteFails`,
`seedDeleteNodes` read from the decorator's lines, `twoNodeFixture`) and
`check_test.go` (`newCheckFixture`, `assertNoSecret`, `setStorage`).
`TestCheck_nodesCheckedConcurrently` uses a barrier, not a timer: every
node's `Version` waits until all have arrived, so checking the nodes one
at a time fails the verdicts deterministically and does not measure
elapsed time. That avoids the kind of flake kvm2's 200 ms test has.
The mutation check applied 34 mutations: 17 in `delete.go`, 16 in
`check.go` and 1 in `client.go`. Every one compiled and every one failed at
least one test. Three did not compile on the first try (an unused `upid`
or `tokenID`) and were redone. The first run of "never stop" did fail
tests, but it also **hung** `TestDelete_concurrentSameNameSafe`, whose
wait for the first `Stop` call could not end. The wait is now bounded by
the first Delete returning, and the rerun fails fast. One reasons row
("dial cut short by the round's deadline") was added before the run,
because no existing row would have caught dropping `checkNode`'s ctx
guard. Every mutation was reverted by a script that compared each file's
hash with the original.
```bash
go test -race -count=2 ./internal/provider/proxmox/ ./internal/providerhealth/ # ok
make manifests && git diff --stat config/ # crawl.example.com_providerhealths.yaml | 2 +-
grep -rn 'not implemented' internal/provider/proxmox/ # nothing
go build ./... && go vet ./... && go vet -tags=integration ./...
go test -short -count=1 ./... # all ok
go test -count=1 -v ./internal/provider/proxmox/ | grep -c '^=== RUN' # 519 before, 649 after
bin/golangci-lint run --disable staticcheck --max-same-issues 0 --max-issues-per-linter 0 ./internal/provider/proxmox/...
```
Worth noting:
- Lint defaults hide repeats: with `--max-same-issues 0` the logcheck hits
are 4 in `create.go`, 4 in the new `delete.go`, and 2, 1 and 1 in
`get.go`, `seed.go` and `task.go`, all the ctx-plus-logger shape.
Nothing else is new except goconst on test strings. A prealloc hit in
`delete_test.go` was fixed.
- `TestCreate_concurrentSameNameSerialised` has the same unbounded
`for callCount("Clone") == 0` wait that hung the Delete test under a
mutation. It is left for the Phase 8 sweep, because this phase does not
touch Create.
- Never seen on the host, for Phase 10: what the storage status call
answers for a missing or disabled storage, what destroy says for a
hibernated VM, and a stop on a paused VM. The try-every-node seed delete
and the multi-node Checker have only met the fake (entry 24).
- Nothing in this phase talked to the host.
## Phase 8 — Contract and observability sweep, integration test, testing.md
The sweep went into the existing `provider_test.go` and reuses the earlier
phases' helpers. The new helpers are `cancelWhileBlocked`, `within`,
`newLoggingFixture` with `lifecycle`/`sentinelReq`, `assertNeverLogged`,
`assertEveryV1Line` and `msgLine`.
- `TestProvider_cancelledContextReturnsPromptly` blocks Create, Get,
Delete and ListByTag on `ClusterResources`, and Check on `Version`,
with `blockOn`. It asserts that the blocked call was the first in the
call log, then cancels. Each must return within 1 s, as a Transient
that still carries `context.Canceled` (Check: one target, `Timeout`).
Two more rows block mid-operation: Create on `UploadISO` and Delete on
`Stop`.
- `TestProvider_everyErrorIsWrappedWithConfiguredName` has 20 rows, one
or more behind every `wrapErr` call site: placement, the name lock with
an ended ctx, resources, a 403 on the clone, a failed start task, the
malformed, missing, locked and 401 Get, the hibernated VM, a failed
destroy task, a 403 on the seed delete, and the list with a ctx that
ends on a candidate. Each row also checks `Error.ID` (empty for
Create) and the `provider proxmox-home: <op>` prefix.
- `TestLogging_v0SilentV1OutcomeV2Detail` runs one whole lifecycle over
the observed fake with a token file: Create with a sentinel in
CloudInit, a Get that finds the captured address, ListByTag, Delete,
Delete of the gone VM, and Check. It runs at V(0), V(1) and V(2). V(0)
logs nothing. V(1) has each outcome line once, with its keys, and two
`proxmox vm deleted` lines: `destroyed=true` with node, then
`destroyed=false` without. Every line has the `proxmox` logger, the
provider and level 1, and no V(2) key. V(2) has the resolved line with
`cloudInitBytes`, agent addresses, the listed instance, and the
clone's description. The sentinel, `#cloud-config` and the token
secret appear at no level.
- `TestLogging_noOpLinesAtV1` (a name beyond the plan) covers entry 20's
no-op lines. Each carries provider and vmid: VM already gone, foreign,
unmarked, seed delete skipped on each of two nodes, unreadable
candidate.
- `TestTracing_httpSpansUnderOperationSpan` routes captures by path, as
`TestDelete_usesPurgeAndDestroyUnreferencedDisks` does. The captures
are `cluster_resources_during_destroy`, `config_during_destroy`,
`status_current_running` and `agent_network_get_interfaces`. A Get
runs over the real `httpAPI`, wrapped in `observe` and
`provider.WithTracing`, with the recorder's tracer passed through
`newHTTPAPI`'s opts. It asserts Running at 192.168.0.150, and exactly
four spans named `HTTP GET` (otelhttp v0.70's
`defaultTransportFormatter`), each a child of `provider.get` in its
trace. No attribute on any span carries the secret or `PVEAPIToken`.
Judgment calls:
- **"Instance lines carry vmid and name" is checked on the provider's
lines, not the decorator's.** Entry 20 gives `proxmox api call` the
fields `call`, `node`, `vmid`, `storage`, `upid` and `duration`, with
`name` only on a clone. So `assertEveryV1Line` exempts that line and
the two lines that are not about one instance (`vms listed`, `node
checked`), and requires vmid and name on all the others.
- **The integration test compares `CreatedAt` with the mark Create
wrote.** kvm2 only checks that it is non-zero. Entry 21 says "equal
what Create wrote", so the test reads the VM's config through the
bare client and parses its description.
- **Get's poll tolerates Transient.** kvm2's poll fails on any error.
Entry 7 makes every agent answer the provider cannot place Transient,
and a booting guest gives some. Those answers are logged and polled
through; any other class fails.
- **The integration machine type has `diskGiB: 4`.** The 3 GiB recon
template is then grown by the resize task on the real host. A larger
template skips the resize.
- **The seed check after Delete is one extra read-only GET in the test**:
`client.do` on `/nodes/{node}/storage/{storage}/content?content=iso`,
decoded the way `storage_content_iso.json` is shaped. No seam method
was added. The VM count and the by-name lookup use the existing
`ClusterResources`.
- **The cleanup finds the VM by name**, so a Create that failed halfway
is also removed. With `PROXMOX_TEST_KEEP` it prints `qm stop <vmid>`
before `qm destroy <vmid> --purge`, because destroy refuses a running
VM. It then prints `pvesm free <isoStorage>:iso/<name>-seed.iso`.
**Unbounded waits.** `TestCreate_concurrentSameNameSerialised` now uses
the `first` channel shape of `TestDelete_concurrentSameNameSafe`. The grep
found no other sleep loop; `client_test.go`'s `for {` reads multipart
parts until EOF. The mutation check found a different shape of the same
hang: tests that call a blocking operation directly on the test goroutine,
where the lock or `blockOn` is released only after the call returns. Under
"lockName ignores ctx", "Check ignores ctx" and "pollUntil ignores ctx"
the package ran into its 60 s timeout on
`TestDelete_waitsOnTheNameLock`, `TestLockName_givesUpWithContextAsTransient`,
`TestCheck_expiredContextReturnsPromptly`, two `TestCheck_reasonsTable`
rows, `TestCreate_cancelledMidWaitReturnsPromptly`,
`TestDelete_cancelledMidWaitReturnsPromptly`, and the ended-ctx rows of
`TestWaitTask_table` and `TestWaitUnlocked_table`. Each call now runs in
`within(t, 5s or 10s, …)`, which fails the test by name. After the fix
the same mutations fail those tests in seconds.
The mutation check ran 14 mutations on production code. The script copied
each file, applied the edit (asserting that it matched exactly once), and
ran `go test -c` before counting a mutation. It then ran
`go test -count=1 -timeout=60s`, restored the file from the copy, and
asserted the sha256 matched. Afterwards `sha256sum -c` over the eight
files hashed up front was OK. `task.go`, added for M14, was covered by the
script's own assertion. `git status` showed no production file changed.
Results:
- Caught by the new tests:
- Get drops the span context before its requests →
`TestTracing_httpSpansUnderOperationSpan`.
- Cloud-init logged at V(2) → `TestLogging_…` and
`TestCreate_cloudInitSentinel…`.
- Delete's destroy path skips `wrapErr` → the wrap test and 6 Delete
tests.
- ListByTag's ended-ctx path skips `wrapErr` → the wrap test's `list
ended on a candidate` row.
- Delete's resolve with `context.Background()` → the cancel test's
`delete` row.
- `lockName` with `context.Background()` → the wrap test's two lock
rows.
- Check's probe with `context.Background()` → the cancel test's
`check` row.
- Delete's logger without the name, `destroyed` always true, `vms
listed` at V(0), the clone description at V(1), and the list skip
line without vmid → `TestLogging_*`.
- Caught only by the older tests: `pollUntil` without its `ctx.Done()`
case → the four waits tests above. The first attempt deleted the case
and did not compile (the `provider` import became unused). The rerun
replaced it with a nil channel.
- Survived, and equivalent: `newHTTPAPI` without the tracer-provider
option. The transport starts a span only under a valid parent. With no
`WithTracerProvider`, otelhttp v0.70 takes the tracer from
`trace.SpanFromContext(r.Context()).TracerProvider()`
(`transport.go:99`), so the child always lands in the parent's
provider. No test can tell the two apart.
`Makefile`: `test-integration-proxmox` next to the kvm2 target, with a
`##` help line naming the five required variables. `docs/testing.md`
gained:
- rows for `nocloud`, `keyedlock` and five for `proxmox` (both packages
were missing);
- a "Live Proxmox, by default" bullet under "deliberately do NOT cover",
naming the cross-node paths and the three answers never captured;
- a "The proxmox integration test" section: variables, the eleven
assertions in order, the KEEP cleanup, the cost, and that it has not
run yet.
In spec entry 21 every §11 box is ticked. The list's intro says the two
integration tests compile but have not run against a host. The cancel and
sentinel items name the new coverage.
```bash
# the names in entry 21, each checked before ticking: 42 of 42 exist
awk '/^## 21\. Tests/,/^## 22\./' docs/proxmox-provider-spec.md | grep -o '`Test[A-Za-z0-9_]*`' | tr -d '`' | sort -u \
| while read n; do grep -rqE "^func $n\(" internal/provider/proxmox/ && echo "ok $n" || echo "MISSING $n"; done
go test -race -count=2 ./internal/provider/proxmox/ # ok
go vet -tags=integration ./... && go vet -tags=e2e ./... # ok
make -n test-integration-proxmox
# go test -race -tags=integration -count=1 -v -timeout 30m -run TestProxmox ./internal/provider/proxmox/
go test -tags=integration -count=1 -v -run TestProxmox ./internal/provider/proxmox/ # both SKIP: PROXMOX_TEST_URL is not set
go build ./... && go test -short -count=1 ./... # all ok
go test -count=1 -v ./internal/provider/proxmox/ | grep -c '^=== RUN' # 649 before, 688 after
bin/golangci-lint run --disable staticcheck --max-same-issues 0 --max-issues-per-linter 0 ./internal/provider/proxmox/...
bin/golangci-lint run --build-tags integration --disable staticcheck --max-same-issues 0 --max-issues-per-linter 0 ./internal/provider/proxmox/...
```
Worth noting:
- Lint is at the baseline:
- the logcheck hits (4/4/2/1/1), mapsloop ×2 in `proxmox.go`, unparam
×3 in `fake_test.go`, gocyclo in `api_test.go`, and goconst on test
strings.
- Two new gocyclo hits were removed, not accepted.
`TestLogging_v0SilentV1OutcomeV2Detail` was at 32; the per-line loop
became `assertEveryV1Line`. `TestProxmox_lifecycle` was at 40; steps
6 and 7 became `assertMarkReadBack` and `assertListed`. kvm2's
lifecycle test is still at 39.
- The integration tests only compile so far. The first real run is Phase
10's. That run meets the Datastore.Allocate point too: a token without
it on the ISO storage fails step 9 as Permanent. Phase 0 granted it on
`proxy-seeds`, and it is already a Phase 9 guide item.
- The parked items did not bite: kvm2's S5.1 and its 200 ms
`TestCheck_hostsCheckedConcurrently` passed in every
`go test -short ./...` run of this phase.
- `.claude/settings.json` stays out of the commit. Nothing in this phase
talked to the host.
## Phase 9 — Deployment and documentation
Deployment: the optional `proxmox-token` Secret (keys `token` and `ca.pem`)
is mounted read-only at `/etc/proxy-operator/proxmox` in
`config/manager/manager.yaml`, next to `kvm-ssh` and commented the same way.
The Makefile gains `proxmox-token-secret`. Both providers configs gain a
commented `proxmox-home` entry after kvm2's. `config/samples/proxy_proxmox.yaml`
is listed in the samples kustomization. There is no RBAC change and no
`hack/providers-dev.yaml` entry. Docs: `docs/proxmox-provider.md` follows
kvm2's guide heading for heading and has no `—`. The rows and sections
named below went into `providers-existing.md`, `architecture.md`, the
README, `providers.md` and `CLAUDE.md`. `docs/testing.md` and the
integration Makefile target were done in Phase 8 and were not touched; the
guide's Testing section links to them.
Where the spec and the plan's Phase 9 text disagree, the spec won. The URL
is `https://192.168.0.8:8006`, not 192.168.2.8, which is not in the node
certificate. The Secret has two keys, and the target takes
`PROXMOX_CA_FILE` as well as `PROXMOX_TOKEN_FILE`.
Judgment calls:
- **The target also checks both files' content.** The token file must be
exactly one non-blank line matching
`<user>@<realm>!<tokenid>=<secret>`. That is the shape `readToken`
accepts: `TrimSpace`, then an anchored regexp. The check uses `grep -c`
and `grep -qx`, so the secret is never printed. The CA check mirrors
`kvm-ssh-secret`'s `PRIVATE KEY` check: the file must contain
`BEGIN CERTIFICATE`. With `insecureSkipTLSVerify` the guide says to
create the Secret by hand with the `token` key only.
- **The sample-entry test covers both files.** `commentedEntry` finds the
`# - name: proxmox-home` line and takes its indent. It keeps the lines
that continue at `<indent># ` and strips `# ` at that indent. The same
helper then works for the ConfigMap, whose entry sits six spaces deep
inside a `|` block. So `TestLoadConfig_proxmoxSampleEntry` has a
`sample` subtest and a `configmap` subtest. It checks every field the
spec names, and that the optional fields stay commented out.
- **A second test goes beyond the plan:**
`TestLoadConfig_proxmoxSampleProxyMatchesSampleEntry`. The sample Proxy's
provider, zone, machine type and image must resolve in the sample
entry. kvm has exactly this bug as a known gap (`proxy_kvm.yaml` places
on `nas1`).
- **The sample entry also has `medium: {cores: 2, memoryMiB: 2048,
diskGiB: 10}`**, next to the `small` the spec asks for, so `diskGiB`
appears somewhere. The optional fields are commented out with a short
note each, because no live proxmox entry exists to explain them. The
kvm2 entry can rely on kvm's.
- **In the ConfigMap the proxmox entry is commented out**, although kvm2's
is live there. It needs a host and a token.
- **providers-existing.md needed more than the column and the section.**
The intro now says five providers and links the guide. In the shared
gaps, six sentences would otherwise be false, and each now names
proxmox where it applies: adoption verified in two of the five, Create
blocking in all three VM providers, no Terminated, the list policy, the
collector gap, and the "kvm2 does this" in proposed features. The new
section is §8, so the shared gaps and proposed features move to §9 and
§10. No doc links those sections by anchor (grep).
- **architecture.md §11 now says "All five providers do".** The sentence
counts Checkers, and proxmox has one. The four Decisions bullets are
entry 23's: the VMID from the UID with the ownership check, the mark in
the description, the seed uploaded through the API, and the node
resolved per call.
- **The guide lists the ten design limits first, then four more.** They
are the hibernated-VM latch, the one-request seed upload, the open-proxy
sample (kvm2's guide lists it too), and "not yet proven end to end" with
the three answers never captured. The Phase 6 to 8 guide items are
each in the section where a reader looks for them. The config write
replacing `agent` and `tags` is in *What lands on Proxmox*, and so are
the seed cleanup, `qm unlock` and the 403 that makes Delete Permanent.
The certificate rule is in *Cluster-side setup*. CA caching versus
token rotation is in both.
- **The guide resumes a hibernated VM with `qm start`, not
`qm resume`.** A VM suspended to disk resumes from its state file on
its next start; `qm resume` is for a VM paused in RAM.
- **The guide's `pvesm add dir proxy-seeds --path /var/lib/proxy-seeds
--content iso` is the standard form, not a recorded command.** Phase 0
says only that the user created `proxy-seeds` as a `dir` storage with
ISO content; the path is illustrative.
The token-file check, run without touching a cluster: `KUBECTL=echo`
prints only the apply stage, and `KUBECTL=true` runs nothing. Scratch
files covered one good line, two lines, a bare secret, blank lines around
a good line, a non-PEM CA and a missing file. The real files on this
workstation pass:
```bash
make -n proxmox-token-secret
make -s proxmox-token-secret PROXMOX_TOKEN_FILE=$D/<case> PROXMOX_CA_FILE=$D/ca KUBECTL=echo
# good, ws: "apply -f -"; two, bad: "... is not one line of user@realm!tokenid=secret"; noca: "... holds no PEM certificate"
make -s proxmox-token-secret KUBECTL=true && echo "real files pass" # real files pass
```
The mutation check for the sample tests edited one line per run,
asserting a single match. It ran the two tests, restored the file from a
copy, and at the end `sha256sum -c` confirmed the three files. All five
mutations failed the tests: `vlanTag: 5000` and `isoStorag:` in the
sample, `192.168.2.8` and `type: kvm2` in the ConfigMap, and
`image: debian-12` in the sample Proxy. `type: kvm2` was rerun after lint
removed the test's own type comparison. It still fails, because
`LoadConfig` refuses a proxmox block under another type.
The link sweep is a short script. It takes the docs as arguments, so it
can be rerun after any later edit. It skips fenced code and inline code
spans. For each relative link it checks that the file exists, and that
the `#anchor` matches a GitHub-style slug of a heading in the target file
(or the same file). Before the real run it was checked on a scratch copy
of the guide with five links added. It reported exactly the three bad
ones: a missing file, a missing anchor in `testing.md`, and a missing
same-file anchor. It checked 78 relative links in the six edited docs,
plus this log's, and found 0 broken:
```bash
cat > doclinks.py <<'PY'
import os, re, sys
def slug(h, seen):
s = re.sub(r"[^\w\- ]", "", h.strip().lower()).replace(" ", "-")
n = seen.get(s, 0); seen[s] = n + 1
return s if n == 0 else f"{s}-{n}"
def prose(path):
out, fence = [], False
for line in open(path, encoding="utf-8"):
if line.lstrip().startswith("```"): fence = not fence; continue
if not fence: out.append(line)
return out
def anchors(path):
seen = {}
return {slug(re.sub(r"`", "", l.lstrip("#").strip()), seen) for l in prose(path) if re.match(r"#{1,6} ", l)}
bad = 0
for doc in sys.argv[1:]:
for n, line in enumerate(prose(doc), 1):
for target in re.findall(r"\]\(([^)\s]+)\)", re.sub(r"`[^`]*`", "", line)):
if re.match(r"[a-z]+:", target): continue
f, _, a = target.partition("#")
p = os.path.normpath(os.path.join(os.path.dirname(doc), f)) if f else doc
if not os.path.exists(p): print(f"{doc}: missing file {target}"); bad += 1
elif a and p.endswith(".md") and a not in anchors(p): print(f"{doc}: missing anchor {target}"); bad += 1
print(f"{bad} broken"); sys.exit(1 if bad else 0)
PY
python3 doclinks.py docs/proxmox-provider.md docs/providers-existing.md docs/architecture.md \
README.md docs/providers.md CLAUDE.md docs/plans-executions/2026-09-18-1502-proxmox-provider.md # 0 broken
```
Verification:
```bash
bin/kustomize build config/default >/dev/null && bin/kustomize build config/samples >/dev/null # ok
go test -count=1 -run 'TestLoadConfig_proxmoxSample' -v ./internal/provider/ # 3 PASS
grep -c '—' docs/proxmox-provider.md # 0
go build ./... && go vet ./... && go vet -tags=integration ./... && go test -short -count=1 ./... # all ok
bin/golangci-lint run --disable staticcheck --max-same-issues 0 --max-issues-per-linter 0 ./internal/provider/
git status --short config/ # the four edited files named above, plus proxy_proxmox.yaml
```
Worth noting:
- Lint on `internal/provider` is at its baseline of three goconst hits.
The first version of the new test pushed it to five: its
`p.Type != "proxmox"` made `"proxmox"` a three-time literal, flagged in
both `config.go` and the test. That comparison added nothing, so it
went. `internal/provider/proxmox` was not touched.
- The `tmp` volume comment in `manager.yaml` still says only the kvm
provider stages seeds there. kvm2 and proxmox do too. It was already
stale for kvm2, so it was left out of this phase's diff.
- The architecture §5 block's right-hand bracket was aligned by
character, not byte (`→` is three bytes), so every line puts `│` in
column 83.
- Nothing in this phase talked to the Proxmox host or a cluster.
`.claude/settings.json` stays out of the commit. The parked kvm2 items
(S5.1, and the 200 ms `TestCheck_hostsCheckedConcurrently`) did not
bite: `go test -short ./...` passed.
- Next is Phase 10: the first real run of `TestProxmox_lifecycle` and
`TestProxmoxCheck_realHost` on pv1-1, with the env block in
`docs/testing.md`. The three answers never captured go with it, and the
guide's "Not yet proven end to end" gap and the README's matching
clause come out once they pass. Then the CHANGELOG entry after the user
confirms, and the MR.
## Phase 10 — Review, real-host validation, changelog, MR
Where the plan's Phase 10 text and the spec disagreed, the spec won, as in
every phase since Phase 1. The host is pv1-1 at `https://192.168.0.8:8006`,
not 192.168.2.8, the template is 9001 and the ISO storage `proxy-seeds`.
**Review.** Two read-only passes over `git diff main...HEAD`: the
`go-operator-reviewer` agent, briefed with the spec and the contract
(P1 to P6, S1 to S3), then `/code-review high` on the branch (CR1 to
CR10). Of the 19 findings (two found by both passes), 9 were fixed, 3
accepted as documented limits and 7 rejected with a reason, all recorded in
[docs/reviews/2026-09-23-proxmox-provider-review/REPORT.md](../reviews/2026-09-23-proxmox-provider-review/REPORT.md).
The one that mattered was CR1/P1. Delete destroyed the VM and then deleted
the seed, so a failed seed delete left a seed nothing would ever remove:
the next `Get` is NotFound, the finalizer goes, and GC lists only VMs.
Delete now removes the seed between the stop and the destroy, so the VM
`Get` looks at goes last. A 403 on the every-node path now fails instead
of being skipped. P2 (CR3) was the other code change: a probe VMID whose
listed name is someone else's is skipped at once, instead of waiting up to
the clone budget on its lock on every retry.
**Lint.** `make lint` still panics inside staticcheck on Go 1.27, so, as
in Phases 8 and 9:
```bash
bin/golangci-lint run --disable staticcheck --max-same-issues 0 --max-issues-per-linter 0 ./internal/provider/proxmox/... ./internal/provider/
bin/golangci-lint run --build-tags integration --disable staticcheck --max-same-issues 0 --max-issues-per-linter 0 ./internal/provider/proxmox/... ./internal/provider/
```
Each run was diffed against the pre-fix baseline by file and message,
with line numbers and occurrence counts stripped. Three new goconst hits
came in along the way, and all three were fixed rather than accepted. The
P2 test's lock value made `"backup"`, then `"migrate"`, a repeated
literal; it now uses `"snapshot-delete"`. The fake's missing-storage error
repeated `"Parameter verification failed."`; it is now built from the raw
captured body. Final counts: goconst 224 (231 with the tag), logcheck 12,
modernize 2, unparam 3, gocyclo 1.
**Real host.** The user ran the host checklist on pv1-1 and then:
```bash
export PROXMOX_TEST_URL=https://192.168.0.8:8006
export PROXMOX_TEST_TOKEN_FILE=$HOME/.config/proxmox-token
export PROXMOX_TEST_CA_FILE=$HOME/.config/proxmox-ca.pem
export PROXMOX_TEST_NODE=pv1-1
export PROXMOX_TEST_TEMPLATE_VMID=9001
export PROXMOX_TEST_ISO_STORAGE=proxy-seeds
export PROXMOX_TEST_VLAN_TAG=4
make test-integration-proxmox
# --- PASS: TestProxmox_lifecycle (59.68s)
# --- PASS: TestProxmoxCheck_realHost (0.13s)
```
It ran the review fixes, uncommitted, on the first try. Clone task about
3 s, Create 6.2 s, address 50.2 s after Create returned, Delete plus Get
plus a second Delete 3.1 s. The Delete deleted the seed while the stopped
VM's `ide2` still referenced it (imgdel `OK`), then destroyed the VM, and
step 10 found neither left. That settles the one assumption behind the
reorder that the fake could not prove. The run confirmed P6 as well: the
first `Get`, about 6 s after the clone, met `status: unknown` in the
listing and answered Transient, blaming the node, which was up. The
message now names both causes.
**The three answers Phase 0 never captured.** Captured with a scratch
helper that passes the token to curl through a process substitution and
writes the Phase 0 fixture shape. Its first offline test caught a bug:
Python's text mode folds `\r\n`, so the header split found nothing, and
the file is now opened with `newline=""`. The calls were:
```bash
pvecap storage_status_missing GET /nodes/pv1-1/storage/proxy-missing/status
pve POST /nodes/pv1-1/qemu/9001/clone -d newid=199999 -d name=proxy-reconaaaaaaaaaaa -d full=1
pve POST /nodes/pv1-1/qemu/199999/status/start
pve POST /nodes/pv1-1/qemu/199999/status/suspend # pause
pvecap stop_paused_response POST /nodes/pv1-1/qemu/199999/status/stop
pvecap task_status_stop_paused GET /nodes/pv1-1/tasks/<upid>/status
pvecap status_current_after_stop_paused GET /nodes/pv1-1/qemu/199999/status/current
pve POST /nodes/pv1-1/qemu/199999/status/start
pve POST /nodes/pv1-1/qemu/199999/status/suspend -d todisk=1 # hibernate: stopped, lock suspended
pvecap err_destroy_hibernated DELETE '/nodes/pv1-1/qemu/199999?purge=1&destroy-unreferenced-disks=1'
pvecap task_status_destroy_hibernated GET /nodes/pv1-1/tasks/<upid>/status
```
Two of the three contradicted the code:
- A missing storage is a **400** with the field error
`storage: No such storage.`, not a 500, so the Checker reported
`APIError`. `isNoSuchStorage` matches that exact field error, and a 500
is still taken as missing (not captured). The matcher matrix caught a
first version that fired on any `storage` field error, which a clone
onto a storage without image support also has.
- The destroy **accepts a hibernated VM**. Its task ended `OK`, removing
both disks and the `vm-199999-state-suspend-…` volume. Delete refused
such a VM of ours as Permanent, believing the destroy would, so it now
destroys it like a stopped one. Create's adoption still refuses one it
finds before it was ever recorded. The capture was renamed
`destroy_hibernated_response.json`, because the client test globs
`err_*.json` as errors.
- A stop on a paused VM ends `OK` with the VM stopped, as assumed.
The destroy also cleaned up the scratch VM, and a listing of local-lvm
content for VMID 199999 came back empty.
**Kind pass.** The user built and loaded the image, deployed, applied a
scratch providers config with only `kubernetes` and `proxmox-home`, ran
the sample Proxy to Ready, ran a `curl -x` through it, deleted it and
checked pv1-1 for leftovers. Everything passed. The first attempt at the
override went to a ConfigMap named `providers-config`. `config/default`'s
`namePrefix` makes the mounted one `egress-proxies-operator-providers-config`,
so the pod kept the shipped config: no `proxmox-home` ProviderHealth, and
`kvm-home` and `kvm2-home` failing on the missing SSH key. The corrected
command replaced the prefixed ConfigMap and deleted the stray one. The
Secret keeps its plain name (`proxmox-token`), since kustomize renames
only resources it builds. `make deploy`'s image edit to
`config/manager/kustomization.yaml` was reverted, not committed.
Worth noting:
- **Behaviour changes from Phase 9.** Delete's order is now stop, seed,
destroy. A hibernated VM of ours is destroyed rather than latched. The
Checker reports a missing ISO storage as `StorageNotFound`. A
foreign-named VM on a probe VMID is skipped without a config read.
- **Accepted, not fixed** (guide Known gaps, spec entry 24):
- P3: Create adopts a leftover stopped VM whatever the current
`placement` says.
- P4: a retry can clone a second VM in a narrow crash window.
- **Still unproven:**
- A storage that exists but cannot be used on a node.
- Every cross-node path. The host has one node.
- **Flakes.** kvm's `TestKeepalive_keepsHealthyClientOpen` failed once
under `-race` ("unexpected packet in response to channel open") and
passed 5 of 5 on rerun. This branch does not touch `internal/provider/kvm`.
The parked kvm2 items (S5.1, the 200 ms `TestCheck_hostsCheckedConcurrently`)
did not bite, though the reviewer agent saw the latter fail once.
- **Docs.** The "not yet run" wording is gone from the README, the
guide, `testing.md`, spec entry 21 and `providers-existing.md`.
`testing.md` gained a timings table and the `PROXMOX_TEST_VLAN_TAG`
export line. The link sweep script from the Phase 9 entry found 0
broken links over the seven edited docs, and the guide has no `—`.
- `.claude/settings.json` stays out of the commit.