Trace every libvirt call in the KVM provider at V(1), visible in the integration test
A tracingAPI decorator wraps the 17-method libvirt seam and logs one "libvirt call" line per RPC after it returns: method, the identifiers it was given, curated results, duration, and on failure the error with its libvirt code, name and message. At V(2) the same line carries the domain and volume XML, metadata and addresses; upload bodies are never logged. The pool applies it in call() when V(1) is enabled and logs connect, dial-failure and drop events, taking the caller's logger explicitly like every other helper in the package. The integration test installs a testr logger at KVM_TEST_LOG_LEVEL (default 1) so the trace interleaves with t.Log output, which also ends the "log.SetLogger(...) was never called" warning. Docs gained a Logging section and the test env var; the KVM provider execution log records run 4 (guest hang at GRUB) that prompted this. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
This commit is contained in:
+24
-1
@@ -186,6 +186,27 @@ replaces the VM).
|
||||
- `virsh domifaddr proxy-abc… --source arp` / `--source agent` shows what
|
||||
the operator sees when it looks for the IP.
|
||||
|
||||
## Logging
|
||||
|
||||
The provider logs through controller-runtime's logger, named `kvm`, with
|
||||
`provider`, `host` and `name` (the domain) on every line.
|
||||
|
||||
- `--zap-log-level=debug` (logr V(1)): one line per provider operation
|
||||
(`domain defined`, `KVM domain ensured running`, ...) **and one
|
||||
`libvirt call` line per libvirt RPC**, named `kvm.libvirt`, with
|
||||
`method`, the identifiers it was given (`domain`, `pool`, `volume`),
|
||||
curated results (`state`, `path`, `interfaces`), `duration`, and on
|
||||
failure `error` plus `libvirtCode`, `libvirtCodeName` and
|
||||
`libvirtMessage`. Connection events (`libvirt host connected`, dial
|
||||
failures, `libvirt connection dropped` with a reason) log at the same
|
||||
level. A Create is about ten RPCs; a Get is four.
|
||||
- `--zap-log-level=2`: the same lines also carry the domain and volume
|
||||
`xml`, the `metadata` element, the interface `addresses` and the
|
||||
domain `names` from a listing.
|
||||
- Cloud-init is never logged at any level: the seed ISO upload reports
|
||||
only `length` and `duration`. There is no raw wire tier like the GCP
|
||||
provider's V(5); go-libvirt exposes no hook for it.
|
||||
|
||||
## Known gaps
|
||||
|
||||
- **No quota or capacity detection.** libvirt reports an out-of-space
|
||||
@@ -211,4 +232,6 @@ replaces the VM).
|
||||
|
||||
Unit tests run over an in-memory fake of the libvirt API. The optional
|
||||
integration test against a real host is described in
|
||||
[testing.md](testing.md#the-kvm-integration-test-real-hypervisor).
|
||||
[testing.md](testing.md#the-kvm-integration-test-real-hypervisor); its
|
||||
`-v` output includes the `libvirt call` trace above, at the verbosity
|
||||
`KVM_TEST_LOG_LEVEL` selects (default 1).
|
||||
|
||||
@@ -462,3 +462,18 @@ lookup already selected the element by URI. The fake now strips the
|
||||
declaration when storing metadata, so the provider's Get and ListByTag
|
||||
tests parse the shape libvirt really returns, and a new
|
||||
`TestParseInstanceMetadata_libvirtBareForm` pins it.
|
||||
|
||||
### Run 4: guest hangs at GRUB's "Booting" line
|
||||
|
||||
Create and start passed; Get polled `Provisioning` with no IP while the
|
||||
VM sat at 100 % CPU after GRUB printed "Booting `Debian GNU/Linux`" and
|
||||
before the kernel wrote anything to the serial console. The domain XML
|
||||
matches a working VM on the same host (q35, BIOS, host-passthrough,
|
||||
virtio disk, SATA cdrom, br0), so the difference is the disk content:
|
||||
the cached image was downloaded without a checksum and never verified.
|
||||
Diagnosis was handed to the user (sha512 against Debian's published
|
||||
sums, `qemu-img check`, and a `virt-install --import` of the same image
|
||||
outside the operator). Because the provider's own logs were invisible in
|
||||
the test output, this run also produced a side plan:
|
||||
[2026-09-10-0805-kvm-libvirt-tracing](2026-09-10-0805-kvm-libvirt-tracing.md),
|
||||
which traces every libvirt RPC at V(1) and routes it into `-v` output.
|
||||
|
||||
@@ -0,0 +1,57 @@
|
||||
# Execution: Trace every libvirt call in the KVM provider
|
||||
|
||||
Plan: [2026-09-10-0805-kvm-libvirt-tracing.md](../plans/2026-09-10-0805-kvm-libvirt-tracing.md)
|
||||
|
||||
- [x] Step 0 — Save plan
|
||||
- [x] Step 1 — `errors.go`: shared error fields
|
||||
- [x] Step 2 — `trace.go`: the `tracingAPI` decorator
|
||||
- [x] Step 3 — `conn.go`: pool logging and wrapping
|
||||
- [x] Step 4 — Call sites
|
||||
- [x] Step 5 — Integration test output
|
||||
- [x] Step 6 — Unit tests
|
||||
- [ ] Step 7 — Docs, execution log, CHANGELOG (docs and this log done; CHANGELOG waits for the live run)
|
||||
|
||||
## Steps 1–6 — Tracer, pool logging, tests, integration wiring
|
||||
|
||||
Done as one commit on `feat/kvm-provider`; the plan's assumption that
|
||||
this belongs on the KVM branch rather than its own feature branch was
|
||||
accepted at plan approval.
|
||||
|
||||
The code follows the plan closely. Things that came out differently
|
||||
or are worth knowing:
|
||||
|
||||
- **Logger names join with `/` in funcr, `.` in zap.** The trace lines
|
||||
are `kvm/libvirt` in the unit-test capture and in `testr` output, and
|
||||
`kvm.libvirt` under the operator's zap logger. The docs use the zap
|
||||
spelling; the unit test asserts the funcr one. First run of the tier
|
||||
test failed on exactly this.
|
||||
- **Cancel is not reported as a transport error.** The original `call`
|
||||
dropped the connection twice on cancellation (once from the abort
|
||||
closure, once from the "not a libvirt error" branch, harmlessly, since
|
||||
drop is idempotent). With a log line on each drop that would have
|
||||
printed both `context cancelled` and `transport error` for one event,
|
||||
so the transport branch now skips `ctx.Err()`. Covered by
|
||||
`TestHostPool_call_logsDropReason`.
|
||||
- **`ConnectListAllDomains` in the fake includes every defined domain**,
|
||||
so the trace test's fixture (one running managed domain plus one
|
||||
stopped domain for `DomainCreate` to start) lists two; the V(2)
|
||||
assertion checks for the `names` key rather than an exact list.
|
||||
- **`libvirtErrorFields` also feeds `logAPIError`**, which therefore
|
||||
gains `libvirtCodeName` (e.g. `ErrNoDomain`) next to the numeric code.
|
||||
- The seam-coverage guard (`reflect` count of `libvirtAPI` methods vs
|
||||
the test table) is the only thing standing between a future 18th RPC
|
||||
and a silently untraced call; keep it.
|
||||
|
||||
Verification run before committing:
|
||||
|
||||
```bash
|
||||
go build ./... && go vet ./... && go vet -tags=integration ./internal/provider/kvm/
|
||||
go test -short ./...
|
||||
go test -race -count=1 -cover ./internal/provider/kvm/ # 90.2 %, up from 88.2
|
||||
go test -tags=integration -count=1 -run TestKVM ./internal/provider/kvm/ # self-skips without env
|
||||
```
|
||||
|
||||
Worth noting: the live run (`make test-integration-kvm` against beelink)
|
||||
had not happened when this was written; it is what Step 7 of the KVM
|
||||
provider plan is waiting on, and its output is the acceptance test for
|
||||
this feature. Expected shape is in the plan's Verification section.
|
||||
+10
-3
@@ -56,7 +56,8 @@ manual runs too.
|
||||
| `internal/provider/gcp` | `insert_test.go`, `errors_test.go`, `gcp_test.go` | field-by-field `buildInsertRequest`; HTTP-code classification table; zone-qualified IDs, 409-is-success, 9-row state mapping, ListByTag filter + `ReturnPartialSuccess` | pure builder needs **no fake**; the rest uses the flattened `instancesAPI` seam |
|
||||
| `internal/provider/kvm` | `domainxml_test.go`, `volume_test.go`, `seed_test.go`, `id_test.go`, `metadata_test.go` | pure builders: domain/volume XML unmarshalled back and checked field by field, seed ISO reopened with the same library, providerID round-trip, metadata in both namespace spellings | pure |
|
||||
| | `errors_test.go`, `conn_test.go` | libvirt-code classification table; connection pool (reuse, redial, drop on transport error, cancel releases a blocked RPC, one dial per host under concurrency); SSH dial, host-key policy, keepalive | fake dialer; a **real in-process `x/crypto/ssh` server** forwarding a unix socket |
|
||||
| | `image_test.go`, `kvm_test.go` | image cache (fresh fetch, ready, marker missing, checksum change/mismatch, 4xx/5xx, concurrent callers); Create resume after every partial step, leftover truncated seed, state/IP mapping, every Delete already-gone case, ListByTag across hosts / one down / unreadable domain | `httptest` image server + in-memory `fakeAPI` behind the `libvirtAPI` seam |
|
||||
| | `image_test.go`, `kvm_test.go` | image cache (fresh fetch, ready, marker missing, checksum change/mismatch, 4xx/5xx, concurrent callers); Create resume after every partial step, leftover truncated seed, state/IP mapping, every Delete already-gone case, ListByTag across hosts / one down / unreadable domain; logging tiers (V0 silent, V1 one line per RPC, V2 XML) with cloud-init never logged | `httptest` image server + in-memory `fakeAPI` behind the `libvirtAPI` seam |
|
||||
| | `trace_test.go`, `conn_test.go`, `errors_test.go` | every `libvirtAPI` method traced exactly once (guarded by reflection on the interface), results and errors pass through, libvirt error fields, detail keys only at V2, upload content never logged; pool connect/drop lines, no wrapping below V1 | `fakeAPI` + `funcr` capture |
|
||||
| | `kvm_integration_test.go` (`-tags=integration`) | one real VM: image fetch, Create, idempotent re-Create, Running with IP, ListByTag, Delete, pool state afterwards | a **real hypervisor** — see below |
|
||||
| `internal/health` | `probe_test.go` | probes through a **real CONNECT-capable proxy stub** to a real TLS server: tunnel success, refused CONNECT, wrong status, dead proxy, plain-http forward | `httptest` + hijacked bidirectional tunnel |
|
||||
| | `engine_test.go` | thresholds, first-verdict, latency suppression matrix, dropped-event retry, stale-probe UID guard, tick scheduling/seeding/pruning, `Start` end-to-end | fake `client.Reader` + injected `probeFn` |
|
||||
@@ -116,8 +117,14 @@ make test-integration-kvm
|
||||
Optional: `KVM_TEST_KNOWN_HOSTS` (without it host keys are **not**
|
||||
verified — the log says so), `KVM_TEST_IMAGE_SHA256`, `KVM_TEST_POOL`
|
||||
(default `default`), `KVM_TEST_SOCKET`, `KVM_TEST_TIMEOUT` (default
|
||||
`20m`, must cover the first-time image download), and `KVM_TEST_KEEP`
|
||||
to leave the VM behind for `virsh console` when something goes wrong.
|
||||
`20m`, must cover the first-time image download), `KVM_TEST_KEEP`
|
||||
to leave the VM behind for `virsh console` when something goes wrong,
|
||||
and `KVM_TEST_LOG_LEVEL` (default `1`) for how much of the provider's
|
||||
logging the `-v` output shows: at `1` every libvirt RPC is one
|
||||
`libvirt call` line with method, identifiers, duration and error code,
|
||||
interleaved with the test's own `t.Log` lines; `2` adds the domain and
|
||||
volume XML, metadata and addresses; `0` leaves only `t.Log`. See the
|
||||
Logging section of [kvm-provider.md](kvm-provider.md#logging).
|
||||
|
||||
What one run asserts, in order: `New` dials the host; `Create` returns a
|
||||
`hosts/<host>/domains/<name>` ID (downloading the image on a cold host —
|
||||
|
||||
Reference in New Issue
Block a user