diff --git a/Makefile b/Makefile index d348f5d..b621dac 100644 --- a/Makefile +++ b/Makefile @@ -66,6 +66,10 @@ vet: ## Run go vet against code. test: manifests generate fmt vet setup-envtest ## Run tests. KUBEBUILDER_ASSETS="$(shell "$(ENVTEST)" use $(ENVTEST_K8S_VERSION) --bin-dir "$(LOCALBIN)" -p path)" go test -race $$(go list ./... | grep -v /e2e) -coverprofile cover.out +.PHONY: test-integration-kvm +test-integration-kvm: ## Run the KVM provider test against a real hypervisor. Needs KVM_TEST_HOST/USER/SSH_KEY/BRIDGE/IMAGE_URL; see docs/testing.md. + go test -race -tags=integration -count=1 -v -timeout 40m -run TestKVM ./internal/provider/kvm/ + # TODO(user): To use a different vendor for e2e tests, modify the setup under 'tests/e2e'. # The default setup assumes Kind is pre-installed and builds/loads the Manager Docker image locally. # kubectl kuberc is disabled by default for test isolation; enable with: diff --git a/docs/plans-executions/2026-09-04-1404-kvm-provider.md b/docs/plans-executions/2026-09-04-1404-kvm-provider.md index 02da6f9..215080a 100644 --- a/docs/plans-executions/2026-09-04-1404-kvm-provider.md +++ b/docs/plans-executions/2026-09-04-1404-kvm-provider.md @@ -9,7 +9,7 @@ Plan: [2026-09-04-1404-kvm-provider.md](../plans/2026-09-04-1404-kvm-provider.md - [x] Step 3b — Image cache (`image.go`) - [x] Step 4 — Provider methods (`kvm.go`) over a fake API - [ ] Step 5 — Wiring, manifests, samples, docs, CHANGELOG -- [ ] Step 6 — Integration test against a real hypervisor +- [x] Step 6 — Integration test against a real hypervisor (written and compiled; first real run is part of Step 7) - [ ] Step 7 — Manual validation + MR ## Step 0 — Save plan + branch @@ -283,3 +283,58 @@ uncovered lines are the real SSH/libvirt dial path and log-only branches. `go vet` and `-race` (three runs) are clean. Wiring into `cmd/main.go` is Step 5, so `New` is not yet reachable from a running operator. + +## Step 6 — Integration test + +Done before Step 5, at the user's request, after a discussion of what the +unit tests can and cannot prove: they run over an in-memory fake of the +libvirt calls, so every assumption about libvirt's real behaviour (domain +XML accepted, raw qcow2 upload + pool refresh usable as a backing image, +guest boots and reports an address) needed a test against a real host. +The wiring in Step 5 is not a prerequisite — the test drives the provider +directly. + +`internal/provider/kvm/kvm_integration_test.go` is behind +`//go:build integration` and skips itself under `-short` or when any of +`KVM_TEST_HOST`, `KVM_TEST_USER`, `KVM_TEST_SSH_KEY`, `KVM_TEST_BRIDGE`, +`KVM_TEST_IMAGE_URL` is unset. Optional knobs beyond the plan's list: +`KVM_TEST_KNOWN_HOSTS` (without it the run disables host-key checking +and logs that), `KVM_TEST_IMAGE_SHA256`, `KVM_TEST_POOL`, +`KVM_TEST_SOCKET`, `KVM_TEST_TIMEOUT` (default 20 m, must cover a cold +image download) and `KVM_TEST_KEEP` to leave a failed run's VM behind for +`virsh console`. + +One sequential test, `TestKVM_lifecycle`, not subtests: each stage +depends on hypervisor state the previous one left. Create (timed, so the +log shows the image fetch), re-Create returns the same ID, poll Get to +Running with an IPv4 (Stopped fails fast with a hint to use the console), +UID and CreatedAt survive the metadata round-trip, ListByTag includes it, +Delete, Get → NotFound, second Delete is a no-op, ListByTag excludes it, +and the pool is inspected through the provider's own connection pool +(`p.hosts.call`, reachable because the test is in package `kvm`): overlay +and seed gone, base image and marker kept. A TCP connect to the guest's +port 22 is attempted but only logged, since the test machine may not +route to the bridge network. Cleanup deletes the VM on failure with its +own two-minute budget, because the test context may already be spent. + +Makefile target, so the invocation is not something to remember: + +```bash +make test-integration-kvm +# = go test -race -tags=integration -count=1 -v -timeout 40m -run TestKVM ./internal/provider/kvm/ +``` + +`docs/testing.md` gained the kvm package rows in the suite table, a +"live libvirt" bullet under the deliberately-not-covered list, and a +section on running this test, with the deviation from the testcontainers +convention called out as the plan asked. + +Worth noting: **the test has not been executed yet.** This session had +no hypervisor credentials; it was verified with +`go vet -tags=integration ./...` and by running it without the +environment (self-skip). Its first real run is the opening act of +Step 7's manual validation, and it is the point at which the Step 2 and +Step 3b "confirm on a real host" notes get answered. The test's config +is built directly rather than through the provider package's validation +(unexported), so a typo in an env var surfaces from `New` or from +libvirt, not from a validation message. diff --git a/docs/testing.md b/docs/testing.md index 03fee39..5ee1633 100644 --- a/docs/testing.md +++ b/docs/testing.md @@ -54,6 +54,10 @@ manual runs too. | | `metrics_test.go` | `WithMetrics` decorator: classified result labels, error passthrough | fake recorder + static provider | | `internal/provider/kubernetes` | `kubernetes_test.go`, `pod_test.go` | Create/Get/Delete/ListByTag over real Pod objects; pure `buildPod`/`squidConf` incl. the `max_filedescriptors` OOM-regression assertion | `client/fake` (real `corev1.Pod`s, no kubelet) | | `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 | +| | `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` | | `internal/lease` | `store_test.go` | capacity, `MaxLeases=0`, all three selection tie-breaks, target-scoped vs global cooldowns, expiry via fake clock, report-on-retained-lease, idempotent release, **40 concurrent acquires never exceeding capacity** | injectable clock; the concurrency case is why `-race` matters | @@ -83,6 +87,51 @@ envtest suite the scaffold generated. bug (below). - **Live GCP**: no test talks to Google. The seam boundary (`buildInsertRequest` + classification) is tested exhaustively instead. +- **Live libvirt, by default**: the KVM unit tests run over an in-memory + fake of the 17 libvirt calls the provider uses. Whether libvirt accepts + the domain XML, whether a raw qcow2 upload plus pool refresh yields a + usable backing image, and whether the guest actually boots and reports + an address are provable only on a real host — that is the opt-in + integration test below. + +## The KVM integration test (real hypervisor) + +`internal/provider/kvm/kvm_integration_test.go` compiles only under +`-tags=integration` and skips itself unless the environment names a +host. It deviates from the "testcontainers for real infra" convention on +purpose: libvirt needs `/dev/kvm` and a bridge to a DHCP network, and a +container has neither. So it targets whichever hypervisor you point it +at and leaves that host's pool with one more cached base image than it +had. + +```sh +export KVM_TEST_HOST=nas1.home.lan:22 # SSH endpoint +export KVM_TEST_USER=virt # must be in the libvirt group +export KVM_TEST_SSH_KEY=$HOME/.ssh/id_ed25519 # private key for that user +export KVM_TEST_BRIDGE=br0 # bridge to a network with DHCP +export KVM_TEST_IMAGE_URL=https://cloud.debian.org/images/cloud/trixie/latest/debian-13-generic-amd64.qcow2 +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. + +What one run asserts, in order: `New` dials the host; `Create` returns a +`hosts//domains/` ID (downloading the image on a cold host — +the log prints how long it took); a second `Create` returns the same ID; +`Get` reaches `Running` with an IPv4 within the budget, and the UID and +`CreatedAt` survive the metadata round-trip through libvirt; `ListByTag` +lists it; `Delete` removes it, `Get` says `ErrNotFound`, a second +`Delete` is a no-op, and the pool holds the base image and its marker +but no per-VM overlay or seed. It also tries a TCP connect to port 22 on +the guest, but only logs the outcome — the machine running the test may +not route to the bridge network. + +The test is not part of `make test` and not run in CI. Keep it compiling +with `go vet -tags=integration ./...` when the provider's internals move. ## The scaffolded `make test-e2e` suite diff --git a/internal/provider/kvm/kvm_integration_test.go b/internal/provider/kvm/kvm_integration_test.go new file mode 100644 index 0000000..1e1faf7 --- /dev/null +++ b/internal/provider/kvm/kvm_integration_test.go @@ -0,0 +1,291 @@ +//go:build integration + +package kvm + +// Integration test against a real libvirt/KVM hypervisor. It deviates from +// the repo's testcontainers convention on purpose: libvirt needs /dev/kvm +// and a bridge, neither of which a container provides, so this runs +// against whatever host the KVM_TEST_* variables name. It creates one VM +// from a stock cloud image, waits for it to report an address, deletes it, +// and checks the pool afterwards. The base image stays cached on the host. +// +// Required: +// +// KVM_TEST_HOST SSH endpoint, host:port +// KVM_TEST_USER SSH user (member of the libvirt group) +// KVM_TEST_SSH_KEY path to the private key +// KVM_TEST_BRIDGE host bridge for the VM's NIC +// KVM_TEST_IMAGE_URL cloud image to fetch (http/https) +// +// Optional: +// +// KVM_TEST_KNOWN_HOSTS known_hosts file; without it host keys are NOT verified +// KVM_TEST_IMAGE_SHA256 expected digest of the image +// KVM_TEST_POOL storage pool (default "default") +// KVM_TEST_SOCKET libvirt socket path on the host +// KVM_TEST_TIMEOUT overall budget incl. the image download (default 20m) +// KVM_TEST_KEEP set to anything to leave the VM behind for inspection + +import ( + "context" + "crypto/rand" + "encoding/hex" + "errors" + "fmt" + "net" + "os" + "testing" + "time" + + libvirt "github.com/digitalocean/go-libvirt" + "k8s.io/apimachinery/pkg/types" + + "gitea.home.hrajfrisbee.cz/kacerr/egress-proxies-operator/internal/provider" +) + +const ( + integrationHostName = "kvm-test" + integrationImageKey = "test-image" + integrationPoll = 5 * time.Second +) + +type integrationEnv struct { + cfg provider.ProviderConfig + timeout time.Duration + keep bool +} + +func loadIntegrationEnv(t *testing.T) integrationEnv { + t.Helper() + if testing.Short() { + t.Skip("integration test skipped under -short") + } + required := map[string]string{} + for _, key := range []string{"KVM_TEST_HOST", "KVM_TEST_USER", "KVM_TEST_SSH_KEY", "KVM_TEST_BRIDGE", "KVM_TEST_IMAGE_URL"} { + v := os.Getenv(key) + if v == "" { + t.Skipf("%s not set; skipping KVM integration test", key) + } + required[key] = v + } + cfg := &provider.KVMConfig{ + SSHKeyFile: required["KVM_TEST_SSH_KEY"], + KnownHostsFile: os.Getenv("KVM_TEST_KNOWN_HOSTS"), + Images: map[string]provider.KVMImage{ + integrationImageKey: {URL: required["KVM_TEST_IMAGE_URL"], SHA256: os.Getenv("KVM_TEST_IMAGE_SHA256")}, + }, + MachineTypes: map[string]provider.KVMMachineType{"small": {VCPUs: 1, MemoryMiB: 1024}}, + Hosts: []provider.KVMHost{{ + Name: integrationHostName, + Address: required["KVM_TEST_HOST"], + User: required["KVM_TEST_USER"], + Socket: os.Getenv("KVM_TEST_SOCKET"), + Pool: os.Getenv("KVM_TEST_POOL"), + Bridge: required["KVM_TEST_BRIDGE"], + }}, + } + if cfg.KnownHostsFile == "" { + cfg.InsecureIgnoreHostKey = true + t.Log("KVM_TEST_KNOWN_HOSTS not set: host key verification disabled for this run") + } + pc := provider.ProviderConfig{Name: "kvm-integration", Type: "kvm", KVM: cfg} + timeout := 20 * time.Minute + if v := os.Getenv("KVM_TEST_TIMEOUT"); v != "" { + d, err := time.ParseDuration(v) + if err != nil { + t.Fatalf("KVM_TEST_TIMEOUT: %v", err) + } + timeout = d + } + return integrationEnv{cfg: pc, timeout: timeout, keep: os.Getenv("KVM_TEST_KEEP") != ""} +} + +func randomUID(t *testing.T) string { + t.Helper() + var b [16]byte + if _, err := rand.Read(b[:]); err != nil { + t.Fatal(err) + } + h := hex.EncodeToString(b[:]) + return fmt.Sprintf("%s-%s-%s-%s-%s", h[0:8], h[8:12], h[12:16], h[16:20], h[20:32]) +} + +// TestKVM_lifecycle is one sequential story, not parallel subtests: each +// stage depends on the state the previous one left on the hypervisor. +func TestKVM_lifecycle(t *testing.T) { + env := loadIntegrationEnv(t) + ctx, cancel := context.WithTimeout(context.Background(), env.timeout) + defer cancel() + + prov, err := New(ctx, env.cfg) + if err != nil { + t.Fatalf("New: %v", err) + } + p := prov.(*Provider) + host := p.cfg.Hosts[0] + + uid := randomUID(t) + req := provider.CreateRequest{ + Name: provider.NameFromUID(types.UID(uid)), + UID: uid, + Namespace: "integration", + ProxyName: "kvm-integration", + Placement: provider.Placement{Zone: integrationHostName, MachineType: "small", Image: integrationImageKey}, + CloudInit: "#cloud-config\n", + Port: 3128, + } + t.Logf("host %s (%s), pool %q, bridge %s, domain %s", host.Name, host.Address, host.Pool, host.Bridge, req.Name) + + // Cleanup runs even when a stage fails, with its own budget: the test + // ctx may already be spent by then. + t.Cleanup(func() { + if env.keep { + t.Logf("KVM_TEST_KEEP set: leaving domain %s and its volumes on %s", req.Name, host.Address) + return + } + cctx, ccancel := context.WithTimeout(context.Background(), 2*time.Minute) + defer ccancel() + if err := p.Delete(cctx, formatProviderID(host.Name, req.Name)); err != nil { + t.Errorf("cleanup Delete: %v", err) + } + }) + + // --- create (includes the image fetch on a cold host) --- + start := time.Now() + id, err := p.Create(ctx, req) + if err != nil { + t.Fatalf("Create: %v", err) + } + t.Logf("Create returned %s after %s", id, time.Since(start).Round(time.Second)) + if want := formatProviderID(host.Name, req.Name); id != want { + t.Fatalf("providerID = %s, want %s", id, want) + } + + // --- create again: must find what it made, not duplicate it --- + id2, err := p.Create(ctx, req) + if err != nil { + t.Fatalf("second Create: %v", err) + } + if id2 != id { + t.Fatalf("second Create returned %s, want %s", id2, id) + } + + // --- wait for the guest to come up with an address --- + inst := waitForRunning(ctx, t, p, id) + t.Logf("domain Running with IP %s (state reported after %s)", inst.IP, time.Since(start).Round(time.Second)) + if inst.UID != uid { + t.Errorf("UID read back = %q, want %q (metadata round-trip through libvirt)", inst.UID, uid) + } + if age := time.Since(inst.CreatedAt); inst.CreatedAt.IsZero() || age < 0 || age > time.Hour { + t.Errorf("CreatedAt = %s (age %s), want a recent timestamp", inst.CreatedAt, age) + } + if conn, err := net.DialTimeout("tcp", net.JoinHostPort(inst.IP, "22"), 5*time.Second); err != nil { + t.Logf("note: tcp %s:22 not reachable from this machine (%v); fine if the bridge network is not routed here", inst.IP, err) + } else { + conn.Close() + t.Logf("tcp %s:22 reachable from this machine", inst.IP) + } + + // --- the GC view must include it --- + listed, err := p.ListByTag(ctx) + if err != nil { + t.Fatalf("ListByTag: %v", err) + } + if !containsID(listed, id) { + t.Errorf("ListByTag = %+v, want it to include %s", listed, id) + } + + // --- delete, then verify nothing of the VM remains but the base image does --- + if err := p.Delete(ctx, id); err != nil { + t.Fatalf("Delete: %v", err) + } + if _, err := p.Get(ctx, id); !errors.Is(err, provider.ErrNotFound) { + t.Errorf("Get after Delete = %v, want ErrNotFound", err) + } + if err := p.Delete(ctx, id); err != nil { + t.Errorf("second Delete: %v, want nil (idempotent)", err) + } + listed, err = p.ListByTag(ctx) + if err != nil { + t.Fatalf("ListByTag after Delete: %v", err) + } + if containsID(listed, id) { + t.Errorf("ListByTag still includes %s after Delete", id) + } + + src, err := resolveImage(p.cfg, integrationImageKey) + if err != nil { + t.Fatal(err) + } + err = p.hosts.call(ctx, host, func(api libvirtAPI) error { + pool, err := api.StoragePoolLookupByName(host.Pool) + if err != nil { + return err + } + for _, gone := range []string{overlayVolumeName(req.Name), seedVolumeName(req.Name)} { + if _, err := api.StorageVolLookupByName(pool, gone); !hasCode(err, libvirt.ErrNoStorageVol) { + t.Errorf("volume %s after Delete: lookup err = %v, want ErrNoStorageVol", gone, err) + } + } + for _, kept := range []string{src.volumeName(), src.markerName()} { + if _, err := api.StorageVolLookupByName(pool, kept); err != nil { + t.Errorf("cached image volume %s: %v, want it kept after Delete", kept, err) + } + } + return nil + }) + if err != nil { + t.Fatalf("inspecting pool after Delete: %v", err) + } + // KVM_TEST_KEEP is for inspecting a failed run; a passed run has + // nothing left to keep, so let cleanup's Delete confirm that. + env.keep = false +} + +// waitForRunning polls Get until the instance is Running with an IP or ctx +// ends. Transient errors are retried; anything else fails the test. +func waitForRunning(ctx context.Context, t *testing.T, p *Provider, id string) *provider.Instance { + t.Helper() + var last provider.InstanceState + ticker := time.NewTicker(integrationPoll) + defer ticker.Stop() + for { + inst, err := p.Get(ctx, id) + switch { + case err == nil: + if inst.State != last { + t.Logf("state %s ip %q", inst.State, inst.IP) + last = inst.State + } + if inst.State == provider.StateRunning { + return inst + } + if inst.State == provider.StateStopped { + t.Fatalf("domain reported %s while waiting for Running; check `virsh console %s` on the host", inst.State, idName(id)) + } + case errors.Is(err, provider.ErrTransient): + t.Logf("Get: transient error, retrying: %v", err) + default: + t.Fatalf("Get: %v", err) + } + select { + case <-ctx.Done(): + t.Fatalf("timed out waiting for Running (last state %s): %v", last, ctx.Err()) + case <-ticker.C: + } + } +} + +func idName(id string) string { + _, name, _ := parseProviderID(id) + return name +} + +func containsID(list []provider.Instance, id string) bool { + for _, inst := range list { + if inst.ID == id { + return true + } + } + return false +}