a73x

internal/agent/reconcile/reconcile_test.go

Ref:   Size: 38.2 KiB   History

package reconcile

import (
	"context"
	"errors"
	"os"
	"path/filepath"
	"sync"
	"testing"
	"time"

	"github.com/a73x/eitri/internal/agent/ipalloc"
	"github.com/a73x/eitri/internal/agent/seed"
	"github.com/a73x/eitri/internal/agent/state"
	"github.com/a73x/eitri/internal/pb"
	"github.com/stretchr/testify/assert"
	"github.com/stretchr/testify/require"
)

// ---- fakes ----

// fakeProv records provisioner calls. Per-VM workers call it from several
// goroutines at once, so every method takes mu. Test-goroutine reads of the
// recorded slices are deliberately unguarded: they happen after f.step()'s
// waitIdle, which establishes happens-before against every worker's last write.
//
// It is also a real addressing backend — Boot allocates a sticky address the
// way netenv/dhcp does, Address reports it, Destroy releases it. Reconcile's
// addressing is only observable through the seam, so the fake has to own it for
// these tests to mean anything.
type fakeProv struct {
	// st is the same state store the engine writes through, so the fake can
	// see the host's disk exactly as a real backend does at Boot.
	st *state.Store

	mu        sync.Mutex
	running   map[string]bool
	prepCalls int // total PrepareRootDisk invocations, including failed ones
	prepared  []string
	booted    []string
	// bootedSpec is the spec each VM was last booted with. Attachment is part
	// of Boot on both real backends, so what a guest ends up holding is only
	// observable here.
	bootedSpec map[string]state.VMSpec
	// volumesAtBoot records, per VM, whether every volume its spec named was
	// already a file when Boot was called. The ordering that makes a volume
	// usable is not visible from the spec alone.
	volumesAtBoot map[string]bool
	shutdown      []string
	destroyed     []string
	prepErr       error
	bootErr       error // one-shot: consumed and cleared on first Boot call
	destroyErr    error // sticky: every Destroy fails until it is cleared
	preflightErr  error // sticky: the backend refuses this host outright

	cidr  string
	addrs map[string]string // vmID -> ip (sticky, mirrors the DHCP table)
	// netAddrs is what a snoop on the named NIC heard, per VM. Never populated
	// by Boot: this address comes from someone else's DHCP server, so a test
	// that wants one plays the guest with answerNetworkAddress.
	netAddrs map[string]string

	// failReason is what the fake's "hypervisor" left behind; empty is a
	// backend with nothing to add, which is the common case.
	failReason string

	// lateAddress switches the fake to the shape a host-run DHCP server has:
	// Boot attaches the NIC but assigns nothing, and the address exists only
	// once the guest has booted and asked for one (answerAddress).
	lateAddress bool
}

func newFakeProv(st *state.Store) *fakeProv {
	return &fakeProv{
		st:            st,
		running:       map[string]bool{},
		addrs:         map[string]string{},
		netAddrs:      map[string]string{},
		bootedSpec:    map[string]state.VMSpec{},
		volumesAtBoot: map[string]bool{},
		cidr:          "10.77.1.0/24",
	}
}

func (f *fakeProv) Preflight(_ context.Context) error {
	f.mu.Lock()
	defer f.mu.Unlock()
	return f.preflightErr
}

func (f *fakeProv) PrepareRootDisk(_ context.Context, s state.VMSpec, _ string) error {
	f.mu.Lock()
	defer f.mu.Unlock()
	f.prepCalls++ // counted BEFORE the error short-circuit: total invocations
	if f.prepErr != nil {
		return f.prepErr
	}
	f.prepared = append(f.prepared, s.VMID)
	return nil
}

func (f *fakeProv) Boot(_ context.Context, id string, spec state.VMSpec) error {
	f.mu.Lock()
	defer f.mu.Unlock()
	f.bootedSpec[id] = spec
	f.volumesAtBoot[id] = f.volumesOnDisk(spec)
	if f.bootErr != nil {
		err := f.bootErr
		f.bootErr = nil // one-shot: clear after first use
		return err
	}
	if !f.lateAddress {
		if err := f.attachLocked(id); err != nil {
			return err
		}
	}
	f.booted = append(f.booted, id)
	f.running[id] = true
	return nil
}

// volumesOnDisk reports whether every volume this spec names is a file already.
// A real backend hands each path to the hypervisor as a block device, so a
// missing one is a guest booting without the disk its owner is about to write
// to — which is what makes "materialise before dispatch" a testable claim.
func (f *fakeProv) volumesOnDisk(spec state.VMSpec) bool {
	for _, id := range spec.VolumeIDs {
		if _, err := os.Stat(f.st.VolumePath(id)); err != nil {
			return false
		}
	}
	return true
}

// attachLocked gives id a sticky address, mirroring netenv: a VM that already
// holds one keeps it, otherwise one is allocated over the set already handed
// out. Serialized by the fake's own lock, exactly as the real backend's DHCP
// table is — which is what makes concurrent creates collision-free.
func (f *fakeProv) attachLocked(id string) error {
	if _, ok := f.addrs[id]; ok {
		return nil
	}
	used := make([]string, 0, len(f.addrs))
	for _, ip := range f.addrs {
		used = append(used, ip)
	}
	ip, err := ipalloc.Alloc(f.cidr, used)
	if err != nil {
		return err
	}
	f.addrs[id] = ip
	return nil
}

func (f *fakeProv) Address(id string) string {
	f.mu.Lock()
	defer f.mu.Unlock()
	return f.addrs[id]
}

func (f *fakeProv) NetworkAddress(id string) string {
	f.mu.Lock()
	defer f.mu.Unlock()
	return f.netAddrs[id]
}

// answerNetworkAddress plays the site's own DHCP server: the guest's second NIC
// has now been granted an address, and the host's snoop heard it. Always late —
// there is no path by which this is known when Boot returns.
func (f *fakeProv) answerNetworkAddress(id, ip string) {
	f.mu.Lock()
	defer f.mu.Unlock()
	f.netAddrs[id] = ip
}

// answerAddress starts answering for id: the guest has now booted and asked the
// host's DHCP server, seconds after Boot returned. Only meaningful under
// lateAddress.
func (f *fakeProv) answerAddress(id string) {
	f.mu.Lock()
	defer f.mu.Unlock()
	_ = f.attachLocked(id)
}

// forgetAddresses drops the backend's in-memory reservations the way an agent
// restart does, leaving it unable to answer Address for a VM that is still
// running.
func (f *fakeProv) forgetAddresses() {
	f.mu.Lock()
	defer f.mu.Unlock()
	f.addrs = map[string]string{}
}

func (f *fakeProv) Shutdown(_ context.Context, id string) error {
	f.mu.Lock()
	defer f.mu.Unlock()
	f.shutdown = append(f.shutdown, id)
	f.running[id] = false
	return nil
}

func (f *fakeProv) Destroy(_ context.Context, id string) error {
	f.mu.Lock()
	defer f.mu.Unlock()
	f.destroyed = append(f.destroyed, id) // every attempt, failed ones included
	if f.destroyErr != nil {
		return f.destroyErr // released nothing: the backend still holds it all
	}
	f.running[id] = false
	delete(f.addrs, id)
	return nil
}

func (f *fakeProv) Running(id string) bool {
	f.mu.Lock()
	defer f.mu.Unlock()
	return f.running[id]
}

func (f *fakeProv) FailureReason(string) string {
	f.mu.Lock()
	defer f.mu.Unlock()
	return f.failReason
}

type fixture struct {
	eng  *Engine
	prov *fakeProv
	st   *state.Store
	now  time.Time
	boot string
}

func setup(t *testing.T) *fixture {
	t.Helper()
	st, err := state.Open(t.TempDir())
	require.NoError(t, err)
	f := &fixture{st: st, prov: newFakeProv(st), now: time.Unix(1_700_000_000, 0), boot: "boot-1"}
	f.eng = f.newEngine()
	t.Cleanup(f.eng.Stop)
	return f
}

// newEngine builds an Engine over this fixture's state dir and backend. setup
// calls it once; restart calls it again, which is the only way to model an
// agent coming back — an Engine is terminal once stopped, exactly like the
// process it lives in.
func (f *fixture) newEngine() *Engine {
	return &Engine{
		St:   f.st,
		Prov: f.prov,
		Images: func(ctx context.Context, url, sha string, _ func(done, total int64)) (string, error) {
			return "/cache/" + sha + ".raw", nil
		},
		Seed:              func(out string, p seed.Params) error { return nil },
		HostKey:           state.LoadOrCreateHostKey,
		BootID:            func() string { return f.boot },
		Now:               func() time.Time { return f.now },
		TombstoneGrace:    5 * time.Minute,
		VanishGrace:       time.Hour,
		MaxCreateAttempts: 3,
	}
}

// restart drops the running engine and builds a fresh one over the same state
// directory, the way an agent restart does: nothing survives but what is on
// disk.
func (f *fixture) restart(t *testing.T) {
	t.Helper()
	f.eng.Stop()
	f.eng = f.newEngine()
	t.Cleanup(f.eng.Stop)
}

// step drives ONE reconcile tick to completion: dispatch, wait for every
// worker's pass to finish, then aggregate. Production never waits — not
// blocking on workers is the point — but a test has to observe a tick before it
// can assert on it. A fenced snapshot dispatches nothing, so its report is
// returned as-is.
func (f *fixture) step(s *pb.Snapshot) *pb.Report {
	rep := f.eng.Step(context.Background(), s)
	if rep.FenceViolation {
		return rep
	}
	f.eng.manager().waitIdle()
	settled := f.aggregateNow(s.Epoch)
	// Volumes converge once per tick, inside Step and before dispatch. Carry
	// that tick's rows onto the settled report rather than reconciling them a
	// second time, which is not what a tick does.
	settled.Volumes = rep.Volumes
	return settled
}

// aggregateNow re-reads records and builds the report for epoch, the way Step
// does after its dispatch. Tests call it after waitIdle so the ack sees records
// the just-finished passes have already changed.
func (f *fixture) aggregateNow(epoch uint64) *pb.Report {
	recs, _ := f.st.LoadVMs()
	return f.eng.aggregate(epoch, recs, nil)
}

func snap(epoch uint64, vms ...*pb.VMSpec) *pb.Snapshot {
	return &pb.Snapshot{Epoch: epoch, Vms: vms}
}

func vm(id string, opts ...func(*pb.VMSpec)) *pb.VMSpec {
	v := &pb.VMSpec{VmId: id, Name: "vm-" + id, ImageUrl: "http://x/i.img",
		ImageSha256: "abc", Vcpus: 1, MemMb: 512, DiskGb: 5, PowerState: "running"}
	for _, o := range opts {
		o(v)
	}
	return v
}

func tombstoned(v *pb.VMSpec) *pb.VMSpec { v.Tombstoned = true; return v }
func stopped(v *pb.VMSpec)               { v.PowerState = "stopped" }

// ownImage gives a VM an image no other VM uses. The Images seam is told a URL
// and a sha and never a vm_id, so this is what makes an observable hung on the
// image cache a PER-VM observable — see occupancy.
func ownImage(v *pb.VMSpec) {
	v.ImageUrl = "http://x/" + v.VmId + ".img"
	v.ImageSha256 = v.VmId
}

func findVM(rep *pb.Report, id string) *pb.VMStatus {
	for _, v := range rep.Vms {
		if v.VmId == id {
			return v
		}
	}
	return nil
}

// ---- tests ----

func TestCreateAllocatesIPPreparesAndBoots(t *testing.T) {
	f := setup(t)
	rep := f.step(snap(1, vm("vm1")))
	assert.Equal(t, []string{"vm1"}, f.prov.prepared)
	assert.Equal(t, []string{"vm1"}, f.prov.booted)
	av := findVM(rep, "vm1")
	require.NotNil(t, av)
	assert.Equal(t, "10.77.1.2", av.Ip, ".1 is the gateway")
	assert.Equal(t, "running", av.PowerState)
	assert.Equal(t, "ready", av.Phase)
	assert.Equal(t, uint64(1), rep.LastSeenEpoch)
}

// TestIncompleteCreateWithDiskPresentIsRetried pins the completion-witness fix:
// a create that failed after PrepareRootDisk wrote disk.raw but before boot leaves a
// record with an empty BootID AND a disk file on disk. The witness is BootID, not
// disk presence — so the next tick must re-run create() (rebuilding disk + seed),
// NOT divert to converge() (which rebuilds neither and would strand the VM).
func TestIncompleteCreateWithDiskPresentIsRetried(t *testing.T) {
	f := setup(t)
	const id = "vm1"

	// Simulate the interrupted create: record saved pre-boot (BootID == ""),
	// with the disk already materialised so the disk file exists on disk.
	require.NoError(t, f.st.SaveVM(state.Record{
		Spec:      state.VMSpec{VMID: id, Name: "vm-" + id, VCPUs: 1, MemMB: 512, DiskGB: 5},
		IP:        "10.77.1.2",
		BootID:    "", // create never completed
		CreatedAt: f.now,
	}))
	require.NoError(t, os.MkdirAll(f.st.VMDir(id), 0o700))
	require.NoError(t, os.WriteFile(f.st.DiskPath(id), []byte("partial"), 0o600))
	_, statErr := os.Stat(f.st.DiskPath(id))
	require.NoError(t, statErr, "precondition: disk present")

	rep := f.step(snap(1, vm(id)))

	assert.Equal(t, []string{id}, f.prov.prepared, "must re-run create (PrepareRootDisk), not converge")
	assert.Equal(t, []string{id}, f.prov.booted)
	av := findVM(rep, id)
	require.NotNil(t, av)
	assert.Equal(t, "ready", av.Phase)
}

// TestCreateSurvivesRecordLoadFailure pins that a create still succeeds when
// the store cannot list records: a transient ReadDir failure makes LoadVMs
// return a nil map, and the tick must degrade to "no local records" rather than
// panic or strand the VM. We force the failure by removing the vms/ dir; SaveVM
// re-creates vms/<id> via MkdirAll, so the create commits regardless.
func TestCreateSurvivesRecordLoadFailure(t *testing.T) {
	f := setup(t)
	vmsDir := filepath.Dir(f.st.VMDir("placeholder"))
	require.NoError(t, os.RemoveAll(vmsDir))

	require.NotPanics(t, func() {
		rep := f.step(snap(1, vm("vm1")))
		av := findVM(rep, "vm1")
		require.NotNil(t, av)
		assert.Equal(t, "ready", av.Phase)
	})
}

// TestUnreadableRecordDoesNotRecreateTheVM pins the consequence of Store.Get
// telling an unreadable record apart from an absent one. A live VM whose record
// cannot be read must NOT be re-created: create() runs PrepareRootDisk over the disk
// its guest is running from and boots a second cloud-hypervisor over the first,
// and it is the only path here that corrupts state instead of retrying. The pass
// is skipped instead, and the VM keeps its last-known row until the store
// recovers.
func TestUnreadableRecordDoesNotRecreateTheVM(t *testing.T) {
	f := setup(t)
	const id = "vm1"

	rep := f.step(snap(1, vm(id)))
	require.Equal(t, []string{id}, f.prov.prepared, "precondition: created once")
	require.Equal(t, "ready", findVM(rep, id).GetPhase())

	require.NoError(t, os.WriteFile(filepath.Join(f.st.VMDir(id), "record.json"), []byte("{not json"), 0o600))
	f.prov.prepared = nil

	rep = f.step(snap(2, vm(id)))

	assert.Empty(t, f.prov.prepared, "a VM whose record cannot be read must not be re-created")
	assert.Equal(t, "ready", findVM(rep, id).GetPhase(), "the VM keeps its last-known row")
}

// TestSameTickSiblingCountsFailedCreateAgainstQuota pins that a VM whose create
// fails (here: at boot) still has its Spec committed to the
// admission ledger, so a SAME-TICK sibling's quota check counts it. Both VMs are
// identical (2 vCPU) under a 3-vCPU cap, so whichever is processed first fails to
// boot and the other must be quota-blocked — NEITHER boots. Before the fix
// the failed VM was invisible to the sibling, which wrongly booted.
func TestSameTickSiblingCountsFailedCreateAgainstQuota(t *testing.T) {
	f := setup(t)
	f.eng.MaxVCPUs = 3
	f.prov.bootErr = assert.AnError
	twoVCPU := func(v *pb.VMSpec) { v.Vcpus = 2 }

	rep := f.step(snap(1, vm("vm1", twoVCPU), vm("vm2", twoVCPU)))

	ready := 0
	for _, v := range rep.Vms {
		if v.Phase == "ready" {
			ready++
		}
	}
	assert.Equal(t, 0, ready, "a failed-create sibling must count against the other's cap")
	assert.Empty(t, f.prov.booted, "neither VM should boot")
}

func TestFenceRefusesLowerEpochWithoutActing(t *testing.T) {
	f := setup(t)
	f.step(snap(5, vm("vm1")))
	rep := f.step(snap(3)) // restore signature: vm1 missing, lower epoch
	assert.True(t, rep.FenceViolation)
	assert.Empty(t, f.prov.destroyed, "fenced snapshot must trigger no destroys")
	assert.Empty(t, f.prov.shutdown)
	assert.Equal(t, uint64(5), rep.LastSeenEpoch)
}

func TestCreateRetryIsBoundedThenTerminalFailed(t *testing.T) {
	f := setup(t)
	f.prov.prepErr = assert.AnError
	for range 3 {
		f.step(snap(1, vm("vm1")))
	}
	f.prov.prepErr = nil // even if the cause clears...
	rep := f.step(snap(1, vm("vm1")))
	av := findVM(rep, "vm1")
	require.NotNil(t, av)
	assert.Equal(t, "failed", av.Phase)
	assert.Empty(t, f.prov.prepared, "...no 4th attempt after MaxCreateAttempts")
}

func TestUserStopIsStoppedNotLost(t *testing.T) {
	f := setup(t)
	f.step(snap(1, vm("vm1")))
	f.step(snap(2, vm("vm1", stopped)))
	assert.Equal(t, []string{"vm1"}, f.prov.shutdown)

	rep := f.step(snap(2, vm("vm1", stopped)))
	av := findVM(rep, "vm1")
	assert.Equal(t, "stopped", av.PowerState)
	assert.NotEqual(t, "failed", av.Phase, "recorded stop request: stopped != lost")
}

// TestFailedRestartCarriesTheHypervisorsReason pins that the report a guest
// produces when it cannot be got running says WHY. The error reconcile has in
// hand describes the attempt, not the guest: a VM handed an image its host
// cannot execute dies exactly here, and the hypervisor's own complaint is the
// only thing that names the cause.
func TestFailedRestartCarriesTheHypervisorsReason(t *testing.T) {
	f := setup(t)
	f.step(snap(1, vm("vm1")))
	f.prov.running["vm1"] = false
	f.prov.bootErr = errors.New("start vm1")
	f.prov.failReason = "Error: VmBoot(NoBootableDevice)"

	av := findVM(f.step(snap(1, vm("vm1"))), "vm1")
	assert.Equal(t, "failed", av.Phase)
	assert.Equal(t, "start vm1: Error: VmBoot(NoBootableDevice)", av.LastError)
}

// TestFailedRestartWithoutAReasonStaysBare pins the empty case: a backend with
// nothing to add must not produce a dangling separator. Empty means "nothing to
// add", not "nothing went wrong".
func TestFailedRestartWithoutAReasonStaysBare(t *testing.T) {
	f := setup(t)
	f.step(snap(1, vm("vm1")))
	f.prov.running["vm1"] = false
	f.prov.bootErr = errors.New("start vm1")
	f.prov.failReason = ""

	av := findVM(f.step(snap(1, vm("vm1"))), "vm1")
	assert.Equal(t, "start vm1", av.LastError)
}

// TestLostVMRestartsAfterHostReboot pins the restart policy, which is now the
// only one there is: a host comes back, every guest on it is detected lost, and
// every guest on it is booted again. There is no field to consult first.
func TestLostVMRestartsAfterHostReboot(t *testing.T) {
	f := setup(t)
	f.step(snap(1, vm("vm1")))
	f.boot = "boot-2"
	f.prov.running["vm1"] = false
	f.prov.booted = nil
	rep := f.step(snap(1, vm("vm1")))
	assert.Equal(t, []string{"vm1"}, f.prov.booted, "desired running: restart")
	assert.Equal(t, "ready", findVM(rep, "vm1").Phase)
}

// TestProcessDiedWithoutStopIsRestarted is the same policy at a smaller scale:
// the host stayed up and the hypervisor died under it. Nobody asked for the
// guest to stop, so it is lost, so it comes back.
func TestProcessDiedWithoutStopIsRestarted(t *testing.T) {
	f := setup(t)
	f.step(snap(1, vm("vm1")))
	f.prov.running["vm1"] = false // crashed; no stop request, same boot ID
	f.prov.booted = nil
	rep := f.step(snap(1, vm("vm1")))
	assert.Equal(t, []string{"vm1"}, f.prov.booted)
	assert.Equal(t, "ready", findVM(rep, "vm1").Phase)
}

func TestTombstoneQuarantinesThenDestroysAfterGrace(t *testing.T) {
	f := setup(t)
	f.step(snap(1, vm("vm1")))
	rep := f.step(snap(2, tombstoned(vm("vm1"))))
	require.Len(t, rep.Quarantined, 1)
	assert.Equal(t, "vm1", rep.Quarantined[0].VmId)
	assert.NotEmpty(t, rep.Quarantined[0].VmspecJson, "spec travels with quarantine (un-delete after restore)")
	assert.Equal(t, []string{"vm1"}, f.prov.shutdown)
	assert.Empty(t, rep.Destroyed, "still in grace")

	f.now = f.now.Add(6 * time.Minute) // past TombstoneGrace
	rep = f.step(snap(2, tombstoned(vm("vm1"))))
	assert.Equal(t, []string{"vm1"}, f.prov.destroyed)
	assert.Contains(t, rep.Destroyed, "vm1", "destroy ack after grace")
	recs, _ := f.st.LoadVMs()
	assert.NotContains(t, recs, "vm1")
	// Destroy is the whole teardown: the guest AND its network resources.
	assert.Empty(t, f.prov.Address("vm1"), "destroy releases the VM's address")
}

func TestVanishedWithoutTombstoneGetsLongGrace(t *testing.T) {
	f := setup(t)
	f.step(snap(1, vm("vm1")))
	// vm1 absent AND not tombstoned at a HIGHER epoch: the bug signature, long grace.
	f.step(snap(2))
	f.now = f.now.Add(30 * time.Minute)
	rep := f.step(snap(2))
	assert.Empty(t, f.prov.destroyed, "vanished VMs get the full VanishGrace (1h)")
	require.Len(t, rep.Quarantined, 1)

	f.now = f.now.Add(31 * time.Minute)
	f.step(snap(2))
	assert.Equal(t, []string{"vm1"}, f.prov.destroyed)
}

func TestDestroyedIsLevelTriggeredForUnknownTombstones(t *testing.T) {
	f := setup(t)
	// Tombstoned VM the agent has no record of (created+deleted while offline,
	// or state dir wiped): ack it EVERY report until the server hard-deletes.
	rep := f.step(snap(1, tombstoned(vm("ghost"))))
	assert.Contains(t, rep.Destroyed, "ghost")
	rep = f.step(snap(1, tombstoned(vm("ghost"))))
	assert.Contains(t, rep.Destroyed, "ghost", "repeated until it leaves desired state")
}

// Fix 1: un-quarantine on un-delete so next delete gets a fresh grace window.
func TestUndeleteClearsQuarantineSoNextDeleteGetsFullGrace(t *testing.T) {
	f := setup(t)
	f.step(snap(1, vm("vm1")))
	// delete -> quarantined
	f.step(snap(2, tombstoned(vm("vm1"))))
	// un-delete: vm1 back in desired, not tombstoned
	f.now = f.now.Add(2 * time.Minute)
	f.step(snap(3, vm("vm1")))
	// much later, delete again: must get a FRESH grace window, not instant kill
	f.now = f.now.Add(24 * time.Hour)
	rep := f.step(snap(4, tombstoned(vm("vm1"))))
	assert.Empty(t, f.prov.destroyed, "fresh quarantine window required after un-delete")
	require.Len(t, rep.Quarantined, 1)
	recs, _ := f.st.LoadVMs()
	require.Contains(t, recs, "vm1")
	assert.NotNil(t, recs["vm1"].QuarantinedAt)
}

// Fix 2: editing a VM's spec resets CreateAttempts so the new spec gets a fresh retry budget.
func TestEditedSpecResetsCreateAttempts(t *testing.T) {
	f := setup(t)
	f.prov.prepErr = assert.AnError
	for range 3 {
		f.step(snap(1, vm("vm1")))
	}
	f.prov.prepErr = nil
	// user edits the VM (more memory): retry must happen
	edited := vm("vm1")
	edited.MemMb = 1024
	rep := f.step(snap(2, edited))
	assert.Equal(t, []string{"vm1"}, f.prov.prepared, "edited spec must reset the attempt budget")
	assert.Equal(t, "ready", findVM(rep, "vm1").Phase)
}

// Fix 3: a successful converge-path boot clears LastError.
// Scenario: VM created successfully, then crashes (lost), converge restarts
// it but boot fails (one-shot bootErr). Record now has LastError set. Next step:
// bootErr cleared, converge retries and succeeds → LastError must be cleared.
func TestConvergeBootSuccessClearsLastError(t *testing.T) {
	f := setup(t)
	// Step 1: create succeeds — VM is running, BootID set.
	f.step(snap(1, vm("vm1")))
	require.Equal(t, []string{"vm1"}, f.prov.booted)

	// Step 2: simulate host reboot (boot-2), VM process gone. Converge will try to
	// restart but boot fails (one-shot bootErr). This sets LastError on the record.
	f.boot = "boot-2"
	f.prov.running["vm1"] = false
	f.prov.bootErr = assert.AnError
	rep := f.step(snap(1, vm("vm1")))
	av := findVM(rep, "vm1")
	require.NotNil(t, av)
	require.Equal(t, "failed", av.Phase, "boot failure must report failed")
	require.NotEmpty(t, av.LastError)

	// Step 3: bootErr is cleared (one-shot). Converge retries and succeeds.
	// LastError must be cleared from both report and persisted record.
	rep = f.step(snap(1, vm("vm1")))
	av = findVM(rep, "vm1")
	require.NotNil(t, av)
	assert.Equal(t, "ready", av.Phase)
	assert.Empty(t, av.LastError, "successful converge boot must clear LastError")
	recs, _ := f.st.LoadVMs()
	assert.Empty(t, recs["vm1"].LastError, "persisted LastError must be cleared after successful boot")
}

// Fix 4: fence-path report must include quarantined VMs.
func TestFenceReportIncludesQuarantinedVMs(t *testing.T) {
	f := setup(t)
	// Create a VM, then quarantine it.
	f.step(snap(5, vm("vm1")))
	f.step(snap(6, tombstoned(vm("vm1"))))
	// Confirm it is quarantined.
	recs, _ := f.st.LoadVMs()
	require.Contains(t, recs, "vm1")
	require.NotNil(t, recs["vm1"].QuarantinedAt)

	// Send a lower-epoch snapshot: fence path must fire and include quarantined entry.
	rep := f.step(snap(3))
	assert.True(t, rep.FenceViolation)
	require.Len(t, rep.Quarantined, 1, "fence report must include quarantined VMs")
	assert.Equal(t, "vm1", rep.Quarantined[0].VmId)
	// The quarantined VM must NOT appear in rep.Vms (it is quarantined, not active).
	assert.Nil(t, findVM(rep, "vm1"), "quarantined VM must not appear in Vms on fence path")
}

// TestVMTimeoutBoundsSlowOperations pins the per-VM watchdog: a wedged
// operation (here: an image fetch that never returns until its context is
// cancelled) must not block that VM's reconcile forever. With VMTimeout set,
// the pass's context expires, the create fails with the ctx error, and the
// tick completes so the next one can retry.
func TestVMTimeoutBoundsSlowOperations(t *testing.T) {
	f := setup(t)
	f.eng.VMTimeout = 50 * time.Millisecond
	f.eng.Images = func(ctx context.Context, url, sha string, _ func(done, total int64)) (string, error) {
		<-ctx.Done() // wedged until the watchdog fires
		return "", ctx.Err()
	}

	done := make(chan *pb.Report, 1)
	go func() { done <- f.step(snap(1, vm("vm1"))) }()

	select {
	case rep := <-done:
		row := findVM(rep, "vm1")
		require.NotNil(t, row)
		assert.Equal(t, "creating", row.Phase, "first failed attempt stays in creating (retry budget)")
		assert.Contains(t, row.LastError, "context deadline exceeded")
	case <-time.After(2 * time.Second):
		t.Fatal("the VM's pass never finished: a wedged operation was not bounded by VMTimeout")
	}
}

// TestVMTimeoutZeroDisablesWatchdog: the zero value must not impose any
// deadline (all pre-existing behavior and tests rely on an unbounded pass).
func TestVMTimeoutZeroDisablesWatchdog(t *testing.T) {
	f := setup(t)
	sawDeadline := false
	f.eng.Images = func(ctx context.Context, url, sha string, _ func(done, total int64)) (string, error) {
		_, sawDeadline = ctx.Deadline()
		return "/cache/x.raw", nil
	}
	rep := f.step(snap(1, vm("vm1")))
	require.NotNil(t, findVM(rep, "vm1"))
	assert.False(t, sawDeadline, "VMTimeout==0 must not set a deadline")
}

// TestWatchdogExpiryDoesNotBurnCreateAttempts pins that a fired watchdog is the
// WATCHDOG's failure, not the VM's: a record must not lose retry budget when
// its pass's ctx expired. Each VM is bounded independently, so a wedged VM can
// no longer starve a sibling of its turn either.
func TestWatchdogExpiryDoesNotBurnCreateAttempts(t *testing.T) {
	f := setup(t)
	f.eng.VMTimeout = 50 * time.Millisecond
	f.eng.Images = func(ctx context.Context, url, sha string, _ func(done, total int64)) (string, error) {
		<-ctx.Done() // every fetch wedges until the watchdog fires
		return "", ctx.Err()
	}

	_ = f.step(snap(1, vm("vm1"), vm("vm2")))

	recs, err := f.st.LoadVMs()
	require.NoError(t, err)
	require.Len(t, recs, 2, "both VMs got their own bounded turn")
	for id, rec := range recs {
		assert.Zero(t, rec.CreateAttempts,
			"vm %s: watchdog expiry must not burn the retry budget", id)
	}
}

// permErr is a test error carrying the consumer-owned permanence marker.
type permErr struct{ msg string }

func (e permErr) Error() string   { return e.msg }
func (e permErr) Permanent() bool { return true }

// TestPermanentCreateErrorFailsTerminallyInOneAttempt pins fast-fail: an
// error marked Permanent() must not burn the retry budget across ticks —
// the first attempt goes straight to terminal failed, and subsequent steps
// do not retry the provisioner.
func TestPermanentCreateErrorFailsTerminallyInOneAttempt(t *testing.T) {
	f := setup(t)
	f.prov.prepErr = permErr{"disk_gb 1 is smaller than base image"}

	rep := f.step(snap(1, vm("vm1")))
	row := findVM(rep, "vm1")
	require.NotNil(t, row)
	assert.Equal(t, "failed", row.Phase, "permanent error must be terminal on attempt 1")
	assert.Contains(t, row.LastError, "smaller than base image")

	require.Equal(t, 1, f.prov.prepCalls, "exactly one PrepareRootDisk invocation")
	rep = f.step(snap(1, vm("vm1")))
	row = findVM(rep, "vm1")
	require.NotNil(t, row)
	assert.Equal(t, "failed", row.Phase)
	assert.Equal(t, 1, f.prov.prepCalls, "no further provisioner attempts after a permanent failure")
}

// TestPreflightRefusalFailsBeforeTheImageFetch is the ordering this seam exists
// for. A host that cannot run guests must say so before create spends anything:
// the image fetch is a multi-gigabyte download, and letting it run first means
// the operator reads whatever it tripped over — an image error, say — instead
// of the host's own verdict.
func TestPreflightRefusalFailsBeforeTheImageFetch(t *testing.T) {
	f := setup(t)
	var fetches int
	f.eng.Images = func(context.Context, string, string, func(int64, int64)) (string, error) {
		fetches++
		return "/cache/x.raw", nil
	}
	f.prov.preflightErr = permErr{"no VM runtime on this host (darwin/arm64)"}

	rep := f.step(snap(1, vm("vm1")))
	row := findVM(rep, "vm1")
	require.NotNil(t, row)
	assert.Equal(t, "failed", row.Phase, "a permanent refusal is terminal on attempt 1")
	assert.Contains(t, row.LastError, "no VM runtime on this host",
		"the reported error must be the host's verdict, not a later step's symptom")
	assert.Zero(t, fetches, "must not download an image for a VM this host can never boot")
	assert.Zero(t, f.prov.prepCalls, "must not reach the disk either")
}

// TestPreflightPassingLeavesCreateUnchanged guards the other direction: the new
// gate must be invisible on a host that can run guests.
func TestPreflightPassingLeavesCreateUnchanged(t *testing.T) {
	f := setup(t)
	rep := f.step(snap(1, vm("vm1")))
	row := findVM(rep, "vm1")
	require.NotNil(t, row)
	assert.Equal(t, "ready", row.Phase)
	assert.Equal(t, []string{"vm1"}, f.prov.booted)
}

// TestTwoVMsCreatedInOneStepGetDistinctIPs pins that when a single Step creates
// two VMs, each gets a distinct address. The backend owns the used-address set
// under its own lock (netenv's DHCP table does; the fake mirrors it), so two
// concurrent creates cannot collide. The reconcile loop's converge order is
// randomized, so this must hold regardless of which VM is created first.
func TestTwoVMsCreatedInOneStepGetDistinctIPs(t *testing.T) {
	f := setup(t)
	f.step(snap(1, vm("vm1"), vm("vm2")))
	recs, err := f.st.LoadVMs()
	require.NoError(t, err)
	require.Contains(t, recs, "vm1")
	require.Contains(t, recs, "vm2")
	assert.NotEmpty(t, recs["vm1"].IP)
	assert.NotEmpty(t, recs["vm2"].IP)
	assert.NotEqual(t, recs["vm1"].IP, recs["vm2"].IP,
		"two VMs created in one Step must not share an IP (serialized admission)")
}

// TestSecondVMInOneStepBustingCapIsBlocked pins intra-tick quota consistency:
// two 2-vcpu VMs sum to 4 > the 3-vcpu cap, so exactly one may boot per tick.
// Admission is serialized, so the second create's quota check sees the first's
// just-committed ledger entry and the running total (2 + 2 = 4) trips the cap.
// Converge order is randomized, so we assert on the count and identify the
// blocked VM dynamically.
func TestSecondVMInOneStepBustingCapIsBlocked(t *testing.T) {
	f := setup(t)
	f.eng.MaxVCPUs = 3
	rep := f.step(snap(1,
		vm("vm1", withRes(2, 512, 5)),
		vm("vm2", withRes(2, 512, 5))))
	require.Len(t, f.prov.booted, 1, "exactly one of two cap-busting VMs may boot in one tick")
	other := "vm1"
	if f.prov.booted[0] == "vm1" {
		other = "vm2"
	}
	av := findVM(rep, other)
	require.NotNil(t, av)
	assert.Equal(t, "failed", av.Phase, "the second cap-busting VM must be quota-blocked")
	assert.Contains(t, av.GetLastError(), "capacity limit")
	assert.Contains(t, av.GetLastError(), "vcpus")
}

// TestTransientCreateErrorStillRetries pins the counterpart: unmarked errors
// keep the existing bounded-retry behavior (phase creating until the budget
// is spent).
func TestTransientCreateErrorStillRetries(t *testing.T) {
	f := setup(t)
	f.prov.prepErr = errors.New("cp: reflink failed") // unmarked -> transient

	for i := 1; i < f.eng.MaxCreateAttempts; i++ {
		rep := f.step(snap(1, vm("vm1")))
		row := findVM(rep, "vm1")
		require.NotNil(t, row)
		assert.Equal(t, "creating", row.Phase, "attempt %d stays in the retry budget", i)
	}
	rep := f.step(snap(1, vm("vm1")))
	row := findVM(rep, "vm1")
	require.NotNil(t, row)
	assert.Equal(t, "failed", row.Phase, "budget spent -> terminal failed")
	assert.Equal(t, f.eng.MaxCreateAttempts, f.prov.prepCalls, "one invocation per budgeted attempt")
}

// TestAddressDiscoveredAfterBootIsPersisted pins the reason per-VM networking
// lives behind the Provisioner seam at all: on a backend whose host OS hands out
// the address, it does not exist when Boot returns. The VM is healthy without
// one, and the address arrives on a later converge poll — no boot, no state
// change, nothing else for that tick to do but notice it.
func TestAddressDiscoveredAfterBootIsPersisted(t *testing.T) {
	f := setup(t)
	f.prov.lateAddress = true

	rep := f.step(snap(1, vm("vm1")))
	av := findVM(rep, "vm1")
	require.NotNil(t, av)
	require.Equal(t, []string{"vm1"}, f.prov.booted)
	assert.Empty(t, av.Ip, "the guest has not asked for an address yet")
	assert.Equal(t, "running", av.PowerState, "no address is not a failure")
	assert.Equal(t, "ready", av.Phase)
	recs, _ := f.st.LoadVMs()
	require.Contains(t, recs, "vm1")
	assert.Empty(t, recs["vm1"].IP)

	f.prov.answerAddress("vm1") // guest booted and DHCP'd

	rep = f.step(snap(2, vm("vm1")))
	av = findVM(rep, "vm1")
	require.NotNil(t, av)
	assert.Equal(t, "10.77.1.2", av.Ip, "a converge poll must pick the address up")
	recs, _ = f.st.LoadVMs()
	assert.Equal(t, "10.77.1.2", recs["vm1"].IP, "and persist it, not just report it")
}

// TestNetworkedVMReportsBothAddresses pins the additive shape at the layer that
// reports it: the NAT address is there from the first tick, as it is for every
// guest, and the named NIC's address joins it — separately, later, without ever
// displacing it. The gate path uses the first, so it has no discovery window at
// all; the LAN address arrives when the site's DHCP server says so.
func TestNetworkedVMReportsBothAddresses(t *testing.T) {
	f := setup(t)
	lan := func(v *pb.VMSpec) { v.Network = "lan" }

	rep := f.step(snap(1, vm("vm1", lan)))
	av := findVM(rep, "vm1")
	require.NotNil(t, av)
	assert.Equal(t, "10.77.1.2", av.Ip, "the NAT NIC is unconditional and known at boot")
	assert.Empty(t, av.NetworkIp, "the LAN address is not known until the guest asks for it")

	f.prov.answerNetworkAddress("vm1", "192.168.0.42") // the site's server answered

	rep = f.step(snap(2, vm("vm1", lan)))
	av = findVM(rep, "vm1")
	require.NotNil(t, av)
	assert.Equal(t, "10.77.1.2", av.Ip, "which never moves")
	assert.Equal(t, "192.168.0.42", av.NetworkIp)

	recs, _ := f.st.LoadVMs()
	require.Contains(t, recs, "vm1")
	assert.Equal(t, "10.77.1.2", recs["vm1"].IP)
	assert.Equal(t, "192.168.0.42", recs["vm1"].NetworkIP, "persisted, not just reported")
	assert.Equal(t, "lan", recs["vm1"].Spec.Network, "and the spec that asked survives the restart")

	// The empty guard covers this address too: a snoop that has heard nothing
	// since the agent restarted must not erase the fleet's record of where the
	// guest is on the LAN.
	f.prov.answerNetworkAddress("vm1", "")
	rep = f.step(snap(3, vm("vm1", lan)))
	av = findVM(rep, "vm1")
	require.NotNil(t, av)
	assert.Equal(t, "192.168.0.42", av.NetworkIp, "an empty answer is 'not known', not 'gone'")
}

// TestNATOnlyVMReportsNoNetworkAddress is the guard for every guest running
// today: nothing about the second NIC leaks into a VM that never asked for one,
// even when the backend is asked about it every tick.
func TestNATOnlyVMReportsNoNetworkAddress(t *testing.T) {
	f := setup(t)

	rep := f.step(snap(1, vm("vm1")))
	av := findVM(rep, "vm1")
	require.NotNil(t, av)
	assert.Equal(t, "10.77.1.2", av.Ip)
	assert.Empty(t, av.NetworkIp)

	recs, _ := f.st.LoadVMs()
	assert.Empty(t, recs["vm1"].NetworkIP)
}

// TestSeedCarriesBothNICsOnlyForANetworkedGuest pins what the guest is told
// about its own hardware: a networked guest's seed names both MACs so netplan
// can match each NIC on its own, and a NAT-only guest's names no second one at
// all — which is what keeps its network-config the single-stanza document every
// running guest already booted with.
func TestSeedCarriesBothNICsOnlyForANetworkedGuest(t *testing.T) {
	f := setup(t)
	var got seed.Params
	f.eng.Seed = func(_ string, p seed.Params) error { got = p; return nil }

	f.step(snap(1, vm("vm1", func(v *pb.VMSpec) { v.Network = "lan" })))
	assert.Equal(t, state.MAC("vm1"), got.MAC)
	assert.Equal(t, state.NetMAC("vm1"), got.NetworkMAC)

	f.step(snap(2, vm("vm2")))
	assert.Equal(t, state.MAC("vm2"), got.MAC)
	assert.Empty(t, got.NetworkMAC, "a guest that asked for no network is told of no second NIC")
}

// TestEmptyAddressAnswerKeepsTheKnownOne pins noteAddress's empty guard: "I have
// no answer" is not "it has no address". Blanking rec.IP would cut the guest SSH
// tunnel to a VM that is reachable and running.
func TestEmptyAddressAnswerKeepsTheKnownOne(t *testing.T) {
	f := setup(t)
	f.step(snap(1, vm("vm1")))
	recs, _ := f.st.LoadVMs()
	require.Equal(t, "10.77.1.2", recs["vm1"].IP, "precondition: address is known")

	f.prov.forgetAddresses() // backend can no longer answer for a live VM

	rep := f.step(snap(2, vm("vm1")))
	av := findVM(rep, "vm1")
	require.NotNil(t, av)
	assert.Equal(t, "10.77.1.2", av.Ip, "an empty answer must not clear a known address")
	recs, _ = f.st.LoadVMs()
	assert.Equal(t, "10.77.1.2", recs["vm1"].IP)
}

// TestRestartRebootsAVMAndRepublishesItsAddress pins that a host reboot puts a
// guest back on its feet: reconcile boots it again, and whatever address the
// backend then reports is the one that reaches both the report and the durable
// record.
func TestRestartRebootsAVMAndRepublishesItsAddress(t *testing.T) {
	f := setup(t)
	f.step(snap(1, vm("vm1")))
	recs, _ := f.st.LoadVMs()
	require.Equal(t, "10.77.1.2", recs["vm1"].IP)

	f.boot = "boot-2" // host rebooted; the agent and its backend restarted with it
	f.prov.running["vm1"] = false
	f.prov.booted = nil

	rep := f.step(snap(2, vm("vm1")))
	require.Equal(t, []string{"vm1"}, f.prov.booted)
	assert.Equal(t, "10.77.1.2", f.prov.Address("vm1"))
	assert.Equal(t, "10.77.1.2", findVM(rep, "vm1").GetIp())
	recs, _ = f.st.LoadVMs()
	assert.Equal(t, "10.77.1.2", recs["vm1"].IP)
}

// TestFailedDestroyKeepsTheRecordForRetry pins the anti-orphan invariant: only
// Destroy returning nil promises the backend has released the VM's process and
// its address, and only then may the record go. Deleting it on a failure would
// leave host resources with nothing left to reap them by, so the record stays
// and the level-triggered loop retries.
func TestFailedDestroyKeepsTheRecordForRetry(t *testing.T) {
	f := setup(t)
	f.step(snap(1, vm("vm1")))
	f.step(snap(2, tombstoned(vm("vm1"))))
	f.prov.destroyErr = assert.AnError

	f.now = f.now.Add(6 * time.Minute) // past TombstoneGrace
	rep := f.step(snap(2, tombstoned(vm("vm1"))))
	require.Equal(t, []string{"vm1"}, f.prov.destroyed, "precondition: destroy was attempted")
	recs, _ := f.st.LoadVMs()
	assert.Contains(t, recs, "vm1", "a failed destroy must keep the record")
	assert.NotContains(t, rep.Destroyed, "vm1", "nothing to ack while the backend still holds it")

	f.prov.destroyErr = nil
	rep = f.step(snap(2, tombstoned(vm("vm1"))))
	assert.Equal(t, []string{"vm1", "vm1"}, f.prov.destroyed, "the next tick retries the destroy")
	recs, _ = f.st.LoadVMs()
	assert.NotContains(t, recs, "vm1")
	assert.Contains(t, rep.Destroyed, "vm1")
}