Files
egress-proxies-operator/internal/provider/kvm2/kvm2_test.go
T
kacerrandClaude Opus 5 c10f98a352 kvm2: name the cached base image after the provider entry (H4)
baseVolumeName was base-<key>.<format>, byte-identical to what the kvm
provider builds in the same pool -- for URL images too, since kvm takes
hex(sum[:8]) and kvm2 takes hex(sum[:])[:16], the same eight bytes -- with
a completely separate lock table. The shipped config has a live kvm-home
entry and a commented-out kvm2-home entry on the same host, the same key
debian-13 and the same default pool, and the comment invites the operator
to enable it. Both integration tests use key test-image. On beelink both
live overlays back onto kvm's base-debian-13.qcow2.

Since cachedImage accepts any 64-hex marker when no sha256 is configured
and never re-hashes the base, that meant two entries pointing one key at
different URLs silently shared a file; a concurrent cold fetch let one
entry delete the other's half-uploaded base and boot a guest from the
remains; and a sha256 edit deleted the base in place under live overlays,
which libvirt permits without complaint.

The name now carries the provider entry: base-<entry>-<key>.<format>.
imageSource gained an Entry field and resolveImage takes the provider
name, so every production call site follows from imageSource.volumeName().
That closes the cross-entry sharing outright, kvm<->kvm2 and kvm2<->kvm2
alike, and turns the per-Provider imageLocks from a latent bug into a
correct one: with names unique per entry, two entries cannot contend for a
file, so no process-wide lock table is needed.

The entry name now reaches a file name, so New checks it against the
existing kvmIdentifier rule, exported as provider.IsKVMIdentifier rather
than duplicated. Provider names are otherwise validated for all types at
once and only for being non-empty and unique, so a name with a slash would
have produced a volume nothing could address.

One companion fix, because the naming alone does not stop it:
cachedImage's digest-mismatch branch deleted the marker and the base and
refetched. It now returns ErrPermanent naming both digests and telling the
operator what to do. That branch was the remaining path that unlinks a
good file under running domains, and the contract already said "add a new
key instead" -- the code was contradicting it. The no-marker and
non-hex-marker branches keep deleting, since there the base is debris.

Not fixed, and said so in the docs rather than left to look closed:
editing images.<key>.url is still a silent no-op, since nothing records
the URL; and cross-process fencing still needs the deferred atomic publish
(upload to a temp name, verify, publish), which lands where H2 put the
discard.

Migration is a non-event. The base-debian-13.qcow2 on beelink is kvm's,
kvm keeps using it untouched, and kvm2 downloads its own copy once per
host. Nothing is replaced under a running guest; the pool needs room for
both, which the prerequisites now say.

The eviction recipe in the guide named base-debian-13.qcow2, which after
this change is kvm's file -- following it would have deleted the wrong
provider's base. It now greps for the prefix and deletes the marker first.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-17 17:57:02 +00:00

362 lines
11 KiB
Go

package kvm2
import (
"context"
"errors"
"strings"
"sync"
"sync/atomic"
"testing"
"time"
"github.com/go-logr/logr/funcr"
logf "sigs.k8s.io/controller-runtime/pkg/log"
"gitea.home.hrajfrisbee.cz/kacerr/egress-proxies-operator/internal/provider"
)
// captureContext collects funcr output at V(2). The sink is locked because
// Check logs from one goroutine per host.
func captureContext() (context.Context, *[]string) {
lines := &[]string{}
var mu sync.Mutex
log := funcr.New(func(prefix, args string) {
mu.Lock()
defer mu.Unlock()
*lines = append(*lines, prefix+" "+args)
}, funcr.Options{Verbosity: 2})
return logf.IntoContext(context.Background(), log), lines
}
func twoHosts() provider.KVMConfig {
return provider.KVMConfig{
SSHKeyFile: "/nonexistent/kvm2/id_ed25519",
KnownHostsFile: "/nonexistent/kvm2/known_hosts",
MachineTypes: map[string]provider.KVMMachineType{"small": {VCPUs: 1, MemoryMiB: 1024}},
Hosts: []provider.KVMHost{
{Name: "beelink", Address: "beelink:22", User: "virt", Bridge: "br0"},
{Name: "lab2", Address: "lab2:22", User: "virt", Bridge: "br1", Socket: "/run/s", Pool: "fast"},
},
}
}
func TestNew_doesNotDialAndWarnsOnMissingFiles(t *testing.T) {
t.Parallel()
ctx, lines := captureContext()
cfg := twoHosts()
cfg.Hosts[0].Address = "192.0.2.1:22"
start := time.Now()
p, err := New(ctx, provider.ProviderConfig{Name: "kvm2-home", Type: "kvm2", KVM2: &cfg})
elapsed := time.Since(start)
if err != nil {
t.Fatalf("New() error = %v, want nil", err)
}
if p == nil {
t.Fatal("New() = nil provider")
}
if elapsed > 100*time.Millisecond {
t.Errorf("New() took %v, want under 100ms (it must not dial)", elapsed)
}
var warnings int
for _, l := range *lines {
if strings.Contains(l, "WARNING:") {
warnings++
}
if !strings.Contains(l, `"provider"="kvm2-home"`) {
t.Errorf("log line lacks the provider value: %s", l)
}
}
if warnings != 2 {
t.Errorf("got %d WARNING: lines, want 2 (key and known_hosts):\n%s", warnings, strings.Join(*lines, "\n"))
}
}
func TestNew_insecureIgnoreHostKeyWarns(t *testing.T) {
t.Parallel()
ctx, lines := captureContext()
cfg := twoHosts()
cfg.KnownHostsFile = ""
cfg.InsecureIgnoreHostKey = true
if _, err := New(ctx, provider.ProviderConfig{Name: "kvm2-home", Type: "kvm2", KVM2: &cfg}); err != nil {
t.Fatalf("New() error = %v, want nil", err)
}
found := false
for _, l := range *lines {
if strings.Contains(l, "WARNING:") && strings.Contains(l, "host keys are not verified") {
found = true
}
if strings.Contains(l, "known_hosts") {
t.Errorf("known_hosts warning emitted although host keys are ignored: %s", l)
}
}
if !found {
t.Errorf("no insecureIgnoreHostKey warning in:\n%s", strings.Join(*lines, "\n"))
}
}
func TestNew_nilBlockDoesNotPanic(t *testing.T) {
t.Parallel()
ctx, _ := captureContext()
p, err := New(ctx, provider.ProviderConfig{Name: "kvm2-empty", Type: "kvm2"})
if err != nil {
t.Fatalf("New() error = %v, want nil", err)
}
kp, ok := p.(*Provider)
if !ok {
t.Fatalf("New() = %T, want *Provider", p)
}
if kp.name != "kvm2-empty" || kp.cfg.DiskSizeGB != defaultDiskSizeGB || kp.now == nil {
t.Errorf("provider = %+v, want name kvm2-empty, default disk size and a clock", kp)
}
}
// TestNew_rejectsUnsafeEntryName guards the one config value that reaches a
// volume name without passing the kvm validator: the provider entry's own
// name, which is checked only for being non-empty and unique.
func TestNew_rejectsUnsafeEntryName(t *testing.T) {
t.Parallel()
ctx, _ := captureContext()
_, err := New(ctx, provider.ProviderConfig{Name: "kvm2/home", Type: "kvm2"})
if !errors.Is(err, provider.ErrPermanent) || !strings.Contains(err.Error(), "kvm2/home") {
t.Fatalf("New() error = %v, want ErrPermanent naming the entry", err)
}
}
func TestHost_table(t *testing.T) {
t.Parallel()
tests := []struct {
name string
hosts []provider.KVMHost
zone string
wantName string
wantErr string
}{
{
name: "one host and empty zone selects it",
hosts: twoHosts().Hosts[:1],
zone: "",
wantName: "beelink",
},
{
name: "several hosts and empty zone is permanent",
hosts: twoHosts().Hosts,
zone: "",
wantErr: "placement.zone is required when kvm2.hosts lists more than one host",
},
{
name: "known name",
hosts: twoHosts().Hosts,
zone: "lab2",
wantName: "lab2",
},
{
name: "unknown name is permanent",
hosts: twoHosts().Hosts,
zone: "nas9",
wantErr: `host "nas9" is not in kvm2.hosts`,
},
{
name: "no hosts at all is permanent",
hosts: nil,
zone: "",
wantErr: "placement.zone is required",
},
}
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
t.Parallel()
p := newProvider("kvm2-home", provider.KVMConfig{Hosts: tc.hosts}, &stubDialer{})
h, err := p.host(tc.zone)
if tc.wantErr != "" {
if err == nil {
t.Fatalf("host(%q) = %+v, want error containing %q", tc.zone, h, tc.wantErr)
}
if !errors.Is(err, provider.ErrPermanent) {
t.Errorf("host(%q) error = %v, want ErrPermanent", tc.zone, err)
}
if !strings.Contains(err.Error(), tc.wantErr) {
t.Errorf("host(%q) error = %q, want substring %q", tc.zone, err, tc.wantErr)
}
return
}
if err != nil {
t.Fatalf("host(%q) error = %v, want nil", tc.zone, err)
}
if h.Name != tc.wantName {
t.Errorf("host(%q).Name = %q, want %q", tc.zone, h.Name, tc.wantName)
}
})
}
}
func TestWithDefaults_table(t *testing.T) {
t.Parallel()
tests := []struct {
name string
in provider.KVMConfig
check func(t *testing.T, got provider.KVMConfig)
}{
{
name: "empty config gets disk size and empty maps",
in: provider.KVMConfig{},
check: func(t *testing.T, got provider.KVMConfig) {
if got.DiskSizeGB != 10 {
t.Errorf("DiskSizeGB = %d, want 10", got.DiskSizeGB)
}
if got.Hosts == nil || got.Images == nil {
t.Errorf("Hosts = %v, Images = %v, want non-nil", got.Hosts, got.Images)
}
},
},
{
name: "explicit disk size kept",
in: provider.KVMConfig{DiskSizeGB: 40},
check: func(t *testing.T, got provider.KVMConfig) {
if got.DiskSizeGB != 40 {
t.Errorf("DiskSizeGB = %d, want 40", got.DiskSizeGB)
}
},
},
{
name: "host socket and pool default",
in: provider.KVMConfig{Hosts: []provider.KVMHost{{Name: "a"}, {Name: "b", Socket: "/run/s", Pool: "fast"}}},
check: func(t *testing.T, got provider.KVMConfig) {
if got.Hosts[0].Socket != "/var/run/libvirt/libvirt-sock" || got.Hosts[0].Pool != "default" {
t.Errorf("hosts[0] = %+v, want default socket and pool", got.Hosts[0])
}
if got.Hosts[1].Socket != "/run/s" || got.Hosts[1].Pool != "fast" {
t.Errorf("hosts[1] = %+v, want explicit socket and pool kept", got.Hosts[1])
}
},
},
{
name: "image format and firmware default",
in: provider.KVMConfig{Images: map[string]provider.KVMImage{
"d": {URL: "https://x/d.qcow2"},
"u": {URL: "https://x/u.img", Format: provider.KVMFormatRaw, Firmware: provider.KVMFirmwareBIOS},
}},
check: func(t *testing.T, got provider.KVMConfig) {
if got.Images["d"].Format != provider.KVMFormatQcow2 || got.Images["d"].Firmware != provider.KVMFirmwareUEFI {
t.Errorf("images[d] = %+v, want qcow2 and uefi", got.Images["d"])
}
if got.Images["u"].Format != provider.KVMFormatRaw || got.Images["u"].Firmware != provider.KVMFirmwareBIOS {
t.Errorf("images[u] = %+v, want raw and bios kept", got.Images["u"])
}
},
},
}
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
t.Parallel()
tc.check(t, withDefaults(tc.in))
})
}
}
func TestWithDefaults_doesNotAliasInput(t *testing.T) {
t.Parallel()
in := provider.KVMConfig{
Hosts: []provider.KVMHost{{Name: "a"}},
Images: map[string]provider.KVMImage{"d": {URL: "https://x/d.qcow2"}},
}
got := withDefaults(in)
got.Hosts[0].Name = "changed"
got.Images["d"] = provider.KVMImage{URL: "changed"}
if in.Hosts[0].Name != "a" || in.Images["d"].URL != "https://x/d.qcow2" {
t.Errorf("input mutated through the defaulted copy: %+v", in)
}
}
func lockCount(k *keyedLocks) int {
k.mu.Lock()
defer k.mu.Unlock()
return len(k.locks)
}
func TestKeyedLocks_entryRemovedWhenUncontended(t *testing.T) {
t.Parallel()
k := newKeyedLocks()
release, err := k.acquire(context.Background(), "a")
if err != nil {
t.Fatalf("acquire error = %v", err)
}
if lockCount(k) != 1 {
t.Errorf("entries while held = %d, want 1", lockCount(k))
}
release()
if lockCount(k) != 0 {
t.Errorf("entries after release = %d, want 0", lockCount(k))
}
if _, err := k.acquire(context.Background(), "a"); err != nil {
t.Fatalf("acquire after release error = %v", err)
}
}
func TestKeyedLocks_waitersShareOneLock(t *testing.T) {
t.Parallel()
k := newKeyedLocks()
const n = 20
var inside, overlaps atomic.Int32
var wg sync.WaitGroup
for range n {
wg.Go(func() {
release, err := k.acquire(context.Background(), "shared")
if err != nil {
t.Errorf("acquire error = %v", err)
return
}
if inside.Add(1) > 1 {
overlaps.Add(1)
}
time.Sleep(time.Millisecond)
inside.Add(-1)
release()
})
}
wg.Wait()
if overlaps.Load() != 0 {
t.Errorf("%d holders overlapped on one key", overlaps.Load())
}
if lockCount(k) != 0 {
t.Errorf("entries after every holder released = %d, want 0", lockCount(k))
}
}
func TestKeyedLocks_waiterGivesUpWithContext(t *testing.T) {
t.Parallel()
k := newKeyedLocks()
release, err := k.acquire(context.Background(), "a")
if err != nil {
t.Fatalf("acquire error = %v", err)
}
ctx, cancel := context.WithTimeout(context.Background(), 20*time.Millisecond)
defer cancel()
start := time.Now()
if _, err := k.acquire(ctx, "a"); !errors.Is(err, context.DeadlineExceeded) {
t.Fatalf("waiter error = %v, want DeadlineExceeded", err)
}
if time.Since(start) > time.Second {
t.Errorf("waiter took %v, want to give up with its ctx", time.Since(start))
}
if lockCount(k) != 1 {
t.Errorf("entries with the holder still in = %d, want 1: the waiter's exit must not drop the holder's entry", lockCount(k))
}
release()
if lockCount(k) != 0 {
t.Errorf("entries after the holder released = %d, want 0", lockCount(k))
}
}
func TestWrapErr_carriesProviderNameOpAndID(t *testing.T) {
t.Parallel()
p := newProvider("kvm2-home", twoHosts(), &stubDialer{})
err := p.wrapErr("get", "hosts/beelink/domains/proxy-x", permanentf("boom"))
var pe *provider.Error
if !errors.As(err, &pe) {
t.Fatalf("wrapErr() = %T, want *provider.Error", err)
}
if pe.Provider != "kvm2-home" || pe.Op != "get" || pe.ID != "hosts/beelink/domains/proxy-x" || pe.Class != provider.ErrPermanent {
t.Errorf("wrapErr() = %+v, want provider kvm2-home, op get, the id and ErrPermanent", pe)
}
}