diff --git a/docs/PLAN.md b/docs/PLAN.md index 342ea2e..d5c89e2 100644 --- a/docs/PLAN.md +++ b/docs/PLAN.md @@ -236,11 +236,14 @@ it is feature-complete for IPv4 TCP/UDP.)* - **Retry/backoff:** today a failed reconcile sets `status=error` and just relies on the 2s tick; replace with `attempts` + `next_attempt_at` (exponential backoff) and have `ListReconcilable` respect it. -- **Host-failure reschedule:** the host reaper marks a host `down` but **nothing - reschedules its servers** — `host_id` is only cleared on delete. Add: on - sustained host-down, clear `host_id`/`vm_id` and re-place, **with fencing** - (generation token / ensure the old VM is gone) to avoid split-brain. P5's - store-mediated reschedule is the data half; this is the control half. +- **Host-failure reschedule:** a *stop* now releases a server's host (clears + `host_id`, returns the reservation) so its next *start* re-places it on any + host — voluntary reschedule works. But the host reaper marks a dead host + `down` while **nothing reschedules the servers still desired-running on it**: + that involuntary path is the gap. Add: on sustained host-down, clear + `host_id`/`vm_id` and re-place, **with fencing** (generation token / ensure the + old VM is gone) to avoid split-brain. P5's store-mediated reschedule is the + data half; this is the control half. - **Draining:** `model.HostDraining` exists but is never entered or honored — wire a drain that blocks new placement and migrates servers off. - Optional leader election (advisory lock/lease) for multiple control-plane diff --git a/internal/agent/agent_test.go b/internal/agent/agent_test.go index 46a7098..adb0514 100644 --- a/internal/agent/agent_test.go +++ b/internal/agent/agent_test.go @@ -47,9 +47,39 @@ func TestFakeRuntimeLifecycle(t *testing.T) { assertState(t, rt, vm.ID, StateMissing) } +// TestFakeRuntimeEvict verifies Evict tears a VM down like Deprovision for the +// fake (which holds no durable world), freeing its capacity and host port. +func TestFakeRuntimeEvict(t *testing.T) { + ctx := context.Background() + rt := NewFakeRuntime("10.0.0.7", WithCapacity(4, 8192)) + + vm, err := rt.Provision(ctx, VMSpec{ServerID: "a", CPUs: 4, MemoryMB: 8192}) + if err != nil { + t.Fatalf("provision: %v", err) + } + if err := rt.Evict(ctx, vm.ID); err != nil { + t.Fatalf("evict: %v", err) + } + assertState(t, rt, vm.ID, StateMissing) + + // Capacity and the host port are freed: a fresh VM fits and reuses the port. + next, err := rt.Provision(ctx, VMSpec{ServerID: "b", CPUs: 4, MemoryMB: 8192}) + if err != nil { + t.Fatalf("provision after evict should fit: %v", err) + } + if next.Port != vm.Port { + t.Errorf("port after evict = %d, want freed port %d reused", next.Port, vm.Port) + } + // Evicting an unknown VM is a no-op. + if err := rt.Evict(ctx, "ghost"); err != nil { + t.Errorf("evict unknown = %v, want nil (idempotent)", err) + } +} + // TestFakeRuntimeRefusesOvercommit verifies a capacity-bounded host agent will // not boot a VM that would exceed its total: two 4/8192 servers cannot both run -// on a single 4/8192 host. Deprovisioning the first frees the slot for another. +// on a single 4/8192 host. Stopping the first frees its slot for another, and +// restarting it back over that taken slot is itself refused. func TestFakeRuntimeRefusesOvercommit(t *testing.T) { ctx := context.Background() rt := NewFakeRuntime("10.0.0.7", WithCapacity(4, 8192)) @@ -61,19 +91,24 @@ func TestFakeRuntimeRefusesOvercommit(t *testing.T) { if _, err := rt.Provision(ctx, VMSpec{ServerID: "b", CPUs: 4, MemoryMB: 8192}); !errors.Is(err, ErrInsufficientCapacity) { t.Fatalf("second provision err = %v, want ErrInsufficientCapacity", err) } - // A stopped VM keeps its slot, so it still cannot fit a second. + // A stopped VM frees its slot, so a second now fits. if err := rt.Stop(ctx, first.ID); err != nil { t.Fatalf("stop: %v", err) } - if _, err := rt.Provision(ctx, VMSpec{ServerID: "b", CPUs: 4, MemoryMB: 8192}); !errors.Is(err, ErrInsufficientCapacity) { - t.Fatalf("provision over a stopped VM err = %v, want ErrInsufficientCapacity", err) + second, err := rt.Provision(ctx, VMSpec{ServerID: "b", CPUs: 4, MemoryMB: 8192}) + if err != nil { + t.Fatalf("provision over a stopped VM should fit: %v", err) + } + // Restarting the first back over the now-taken slot is refused. + if _, err := rt.Start(ctx, first.ID); !errors.Is(err, ErrInsufficientCapacity) { + t.Fatalf("restart over a taken slot err = %v, want ErrInsufficientCapacity", err) } - // Deprovisioning frees the capacity. - if err := rt.Deprovision(ctx, first.ID); err != nil { + // Freeing the second's slot lets the first restart. + if err := rt.Deprovision(ctx, second.ID); err != nil { t.Fatalf("deprovision: %v", err) } - if _, err := rt.Provision(ctx, VMSpec{ServerID: "b", CPUs: 4, MemoryMB: 8192}); err != nil { - t.Fatalf("provision after deprovision should fit: %v", err) + if _, err := rt.Start(ctx, first.ID); err != nil { + t.Fatalf("restart after freeing capacity should fit: %v", err) } } diff --git a/internal/agent/firecracker/runtime.go b/internal/agent/firecracker/runtime.go index ac63752..91970f7 100644 --- a/internal/agent/firecracker/runtime.go +++ b/internal/agent/firecracker/runtime.go @@ -298,15 +298,19 @@ func (r *Runtime) Provision(ctx context.Context, spec agent.VMSpec) (*agent.VM, } // checkCapacity reports whether a VM needing cpus/memMB fits the host's -// remaining capacity, returning agent.ErrInsufficientCapacity if not. A live VM -// holds its slot whether running or stopped, so every tracked VM counts. A zero -// configured total leaves that dimension unconstrained. +// remaining capacity, returning agent.ErrInsufficientCapacity if not. Only a +// running VM holds its slot — a stopped VM's process is gone, freeing its +// cpu/memory for other work, at the cost that restarting it may then fail. A +// zero configured total leaves that dimension unconstrained. func (r *Runtime) checkCapacity(cpus, memMB int) error { r.mu.Lock() defer r.mu.Unlock() var usedCPUs, usedMem int for _, m := range r.vms { + if !m.running() { + continue + } usedCPUs += m.vcpus usedMem += m.memoryMB } @@ -331,6 +335,12 @@ func (r *Runtime) Start(ctx context.Context, vmID string) (*agent.VM, error) { if m.running() { return r.vmView(m), nil } + // A stopped VM gave up its capacity slot, so restarting it must clear the + // host backstop again — the cpu/memory it needs may have been handed to + // another VM while it was down. + if err := r.checkCapacity(m.vcpus, m.memoryMB); err != nil { + return nil, err + } if err := m.boot(ctx); err != nil { return nil, fmt.Errorf("firecracker: restart vm: %w", err) } @@ -360,8 +370,25 @@ func (r *Runtime) Stop(ctx context.Context, vmID string) error { return nil } -// Deprovision force-stops a VM and removes its working directory (idempotent). +// Evict force-stops a VM and removes its host-local footprint — working dir and +// local world disk — but keeps the durable world snapshot, so the server can be +// rescheduled onto another host and restore its world from the store there. It +// backs releasing a stopped server from its host (idempotent). +func (r *Runtime) Evict(_ context.Context, vmID string) error { + return r.teardown(vmID, false) +} + +// Deprovision force-stops a VM and removes it entirely, including its durable +// world snapshot — it is a server delete (idempotent). func (r *Runtime) Deprovision(_ context.Context, vmID string) error { + return r.teardown(vmID, true) +} + +// teardown force-stops a VM and removes its host-local footprint. When +// deleteWorld is set it also deletes the durable world snapshot (a server +// delete); otherwise the durable copy is preserved so the world can be restored +// after a reschedule. An unknown VM is a no-op. +func (r *Runtime) teardown(vmID string, deleteWorld bool) error { r.mu.Lock() m, ok := r.vms[vmID] if ok { @@ -383,18 +410,17 @@ func (r *Runtime) Deprovision(_ context.Context, vmID string) error { } _ = deleteTAP(m.tapName) } - // Destroy the world disk along with the VM. This is the point P5b will - // snapshot-then-upload before removing, so a deprovision becomes a safe - // teardown rather than data loss; for now destroy means destroy. Removing - // the keyed parent dir (DataDir/) takes the disk with it. + // Remove the host-local world disk. Removing the keyed parent dir + // (DataDir/) takes the disk with it. The durable copy in the store is + // the system of record across hosts; we delete it only on a true server + // delete, keeping it for a reschedule. if m.worldDisk != "" { if err := os.RemoveAll(filepath.Dir(m.worldDisk)); err != nil { return fmt.Errorf("firecracker: remove world disk: %w", err) } - // Delete the durable copy too: deprovision is a server delete, so the - // world is meant to be gone. Best-effort — an orphaned blob is harmless - // (a later GC can sweep it) and must not block teardown. - if r.store != nil { + if deleteWorld && r.store != nil { + // Best-effort — an orphaned blob is harmless (a later GC can sweep + // it) and must not block teardown. _ = r.store.Delete(context.Background(), m.worldKey) } } diff --git a/internal/agent/firecracker/worldsnapshot_test.go b/internal/agent/firecracker/worldsnapshot_test.go index 0ac5189..3c3e2ad 100644 --- a/internal/agent/firecracker/worldsnapshot_test.go +++ b/internal/agent/firecracker/worldsnapshot_test.go @@ -137,3 +137,69 @@ func TestPrepareWorldDiskFreshWhenStoreEmpty(t *testing.T) { t.Fatal("expected fresh-format fallback to invoke mkfs and fail") } } + +// TestEvictPreservesDurableWorld verifies the teardown split: Evict removes a +// VM's host-local footprint but keeps the durable world snapshot (so the server +// can restore it after a reschedule), while Deprovision deletes the durable copy +// too (a true server delete). +func TestEvictPreservesDurableWorld(t *testing.T) { + ctx := context.Background() + store, err := storage.NewDirStore(t.TempDir()) + if err != nil { + t.Fatal(err) + } + rt := newTestRuntime(t) + rt.store = store + + // seed builds a durable snapshot for key and a tracked machine with a local + // world disk, returning the machine. + seed := func(id, key string) *machine { + if err := store.Put(ctx, key, bytes.NewReader([]byte("world-"+key))); err != nil { + t.Fatalf("seed durable %s: %v", key, err) + } + base := t.TempDir() + vmDir := filepath.Join(base, id) + worldDir := filepath.Join(base, "worlds", key) + for _, d := range []string{vmDir, worldDir} { + if err := os.MkdirAll(d, 0o750); err != nil { + t.Fatal(err) + } + } + disk := filepath.Join(worldDir, "world.ext4") + if err := os.WriteFile(disk, []byte("local-"+key), 0o640); err != nil { + t.Fatal(err) + } + m := &machine{id: id, serverID: key, dir: vmDir, worldDisk: disk, worldKey: key} + rt.vms[id] = m + return m + } + + gone := func(path string) bool { + _, err := os.Stat(path) + return os.IsNotExist(err) + } + + // Evict: durable world survives, host-local footprint is gone. + mE := seed("vm-evict", "srv-evict") + if err := rt.Evict(ctx, mE.id); err != nil { + t.Fatalf("evict: %v", err) + } + if ok, _ := store.Exists(ctx, "srv-evict"); !ok { + t.Error("evict deleted the durable world; want preserved") + } + if !gone(mE.dir) || !gone(filepath.Dir(mE.worldDisk)) { + t.Error("evict left host-local footprint behind") + } + if _, ok := rt.vms[mE.id]; ok { + t.Error("evict left the machine tracked") + } + + // Deprovision: durable world is deleted too. + mD := seed("vm-deprov", "srv-deprov") + if err := rt.Deprovision(ctx, mD.id); err != nil { + t.Fatalf("deprovision: %v", err) + } + if ok, _ := store.Exists(ctx, "srv-deprov"); ok { + t.Error("deprovision kept the durable world; want deleted") + } +} diff --git a/internal/agent/link.go b/internal/agent/link.go index fe00df3..4e0abf8 100644 --- a/internal/agent/link.go +++ b/internal/agent/link.go @@ -21,6 +21,7 @@ const ( OpStart = "start" OpStop = "stop" OpSnapshot = "snapshot" + OpEvict = "evict" OpDeprovision = "deprovision" OpStatus = "status" ) @@ -168,6 +169,8 @@ func execOp(ctx context.Context, rt Runtime, cmd *pb.Command) (payload []byte, e return nil, errString(rt.Stop(ctx, vmRef(cmd))) case OpSnapshot: return nil, errString(rt.Snapshot(ctx, vmRef(cmd))) + case OpEvict: + return nil, errString(rt.Evict(ctx, vmRef(cmd))) case OpDeprovision: return nil, errString(rt.Deprovision(ctx, vmRef(cmd))) case OpStatus: diff --git a/internal/agent/runtime.go b/internal/agent/runtime.go index 595534a..f72590a 100644 --- a/internal/agent/runtime.go +++ b/internal/agent/runtime.go @@ -87,7 +87,15 @@ type Runtime interface { Start(ctx context.Context, vmID string) (*VM, error) // Stop halts a VM without destroying it. Idempotent (missing VM is success). Stop(ctx context.Context, vmID string) error - // Deprovision destroys a VM. Idempotent (missing VM is success). + // Evict destroys a VM and its host-local footprint (working dir, local world + // disk) while preserving the durable world snapshot, so the server can be + // rescheduled onto another host and restore its world there. It is the + // teardown half of releasing a stopped server from its host; Deprovision is + // the stronger form that also deletes the durable world. Idempotent (missing + // VM is success). + Evict(ctx context.Context, vmID string) error + // Deprovision destroys a VM, including its durable world. Idempotent (missing + // VM is success). Deprovision(ctx context.Context, vmID string) error // Status reports a VM's observed state, returning StateMissing for an // unknown id rather than an error. @@ -117,9 +125,10 @@ type FakeRuntime struct { usedPorts map[int]struct{} // host ports currently bound by a live VM } -// fakeVM is a simulated VM plus the host resources it holds. A provisioned VM -// occupies its cpu/memory until it is deprovisioned, even while stopped — -// mirroring a real host, where a halted VM keeps its slot. +// fakeVM is a simulated VM plus the host resources it holds. A running VM +// occupies its cpu/memory; stopping it frees that slot back to the host — +// mirroring a real host that reclaims a halted VM's resources — so only running +// VMs count against capacity. type fakeVM struct { vm *VM cpus int @@ -187,11 +196,16 @@ func (r *FakeRuntime) allocatePortLocked() int { } // fitsLocked reports whether a VM needing cpus/memMB fits the host's remaining -// capacity, returning ErrInsufficientCapacity if not. A zero total leaves that +// capacity, returning ErrInsufficientCapacity if not. Only running VMs hold a +// slot — a stopped VM frees its cpu/memory so the host can pack other work into +// it, at the cost that restarting it may then fail. A zero total leaves that // dimension unconstrained. Caller holds r.mu. func (r *FakeRuntime) fitsLocked(cpus, memMB int) error { var usedCPUs, usedMem int for _, fv := range r.vms { + if fv.vm.State != StateRunning { + continue + } usedCPUs += fv.cpus usedMem += fv.memMB } @@ -204,7 +218,10 @@ func (r *FakeRuntime) fitsLocked(cpus, memMB int) error { return nil } -// Start boots an existing VM back to running. +// Start boots an existing VM back to running, refusing to overcommit. Because a +// stopped VM gave up its slot, the cpu/memory it needs may have been handed to +// another VM while it was down, so a restart can fail with +// ErrInsufficientCapacity. Restarting an already-running VM is a no-op. func (r *FakeRuntime) Start(_ context.Context, vmID string) (*VM, error) { r.mu.Lock() defer r.mu.Unlock() @@ -213,7 +230,12 @@ func (r *FakeRuntime) Start(_ context.Context, vmID string) (*VM, error) { if !ok { return nil, ErrVMNotFound } - fv.vm.State = StateRunning + if fv.vm.State != StateRunning { + if err := r.fitsLocked(fv.cpus, fv.memMB); err != nil { + return nil, err + } + fv.vm.State = StateRunning + } return clone(fv.vm), nil } @@ -228,17 +250,35 @@ func (r *FakeRuntime) Stop(_ context.Context, vmID string) error { return nil } +// Evict destroys a VM, freeing its capacity and host port. The fake keeps no +// durable world, so it is indistinguishable from Deprovision here; the +// distinction (preserving the durable snapshot) only matters for the real +// runtime. Unknown VM is a no-op. +func (r *FakeRuntime) Evict(_ context.Context, vmID string) error { + r.mu.Lock() + defer r.mu.Unlock() + + r.removeLocked(vmID) + return nil +} + // Deprovision destroys a VM, freeing its capacity and host port. Unknown VM is // a no-op. func (r *FakeRuntime) Deprovision(_ context.Context, vmID string) error { r.mu.Lock() defer r.mu.Unlock() + r.removeLocked(vmID) + return nil +} + +// removeLocked drops a VM from the inventory and releases its host port. Caller +// holds r.mu; an unknown VM is a no-op. +func (r *FakeRuntime) removeLocked(vmID string) { if fv, ok := r.vms[vmID]; ok { delete(r.usedPorts, fv.vm.Port) delete(r.vms, vmID) } - return nil } // Status reports a VM's state, or a missing VM for an unknown id. diff --git a/internal/agentlink/hub.go b/internal/agentlink/hub.go index 3b72b84..d42a693 100644 --- a/internal/agentlink/hub.go +++ b/internal/agentlink/hub.go @@ -222,7 +222,14 @@ func (h *Hub) Snapshot(ctx context.Context, hostID, vmID string) error { return err } -// Deprovision asks the host's agent to destroy a VM. +// Evict asks the host's agent to tear down a VM while preserving its durable +// world, releasing the host so the server can be rescheduled elsewhere. +func (h *Hub) Evict(ctx context.Context, hostID, vmID string) error { + _, err := h.call(ctx, hostID, agent.OpEvict, agent.VMRef{VMID: vmID}) + return err +} + +// Deprovision asks the host's agent to destroy a VM, including its durable world. func (h *Hub) Deprovision(ctx context.Context, hostID, vmID string) error { _, err := h.call(ctx, hostID, agent.OpDeprovision, agent.VMRef{VMID: vmID}) return err diff --git a/internal/model/game_server.go b/internal/model/game_server.go index 81db154..2d52d8a 100644 --- a/internal/model/game_server.go +++ b/internal/model/game_server.go @@ -54,8 +54,9 @@ type GameServer struct { Env map[string]string `json:"env,omitempty"` // HostID is the fleet host the scheduler placed this server on (P2). It is - // set before provisioning and persists across stop/start (the VM stays put); - // it is cleared only on delete. Nil until placed. + // set before provisioning and cleared on stop and on delete — a stopped + // server holds no host, so its next start re-runs placement and may land on a + // different host. Nil until placed. HostID *string `json:"host_id,omitempty"` // Runtime details, populated once provisioned. diff --git a/internal/provisioner/provisioner.go b/internal/provisioner/provisioner.go index 8d54d9c..ecdc418 100644 --- a/internal/provisioner/provisioner.go +++ b/internal/provisioner/provisioner.go @@ -43,7 +43,12 @@ type Provisioner interface { Start(ctx context.Context, s *model.GameServer) (*Instance, error) // Stop halts the backing VM without destroying it. It must be idempotent. Stop(ctx context.Context, s *model.GameServer) error - // Deprovision tears down the backing VM. It must be idempotent. + // Evict releases the backing VM from its host while preserving the durable + // world, so the server can be rescheduled onto another host on its next + // start. It must be idempotent. + Evict(ctx context.Context, s *model.GameServer) error + // Deprovision tears down the backing VM, including its durable world. It must + // be idempotent. Deprovision(ctx context.Context, s *model.GameServer) error // Status reports the observed state of the backing VM. Status(ctx context.Context, s *model.GameServer) (State, error) @@ -91,6 +96,10 @@ func (f Fake) Start(ctx context.Context, s *model.GameServer) (*Instance, error) // Stop is a no-op for the fake backend; the VM is considered halted but kept. func (Fake) Stop(_ context.Context, _ *model.GameServer) error { return nil } +// Evict is a no-op for the fake backend; there is no host-local footprint to +// release. +func (Fake) Evict(_ context.Context, _ *model.GameServer) error { return nil } + // Deprovision is a no-op for the fake backend. func (Fake) Deprovision(_ context.Context, _ *model.GameServer) error { return nil } diff --git a/internal/provisioner/remote.go b/internal/provisioner/remote.go index 82f35fb..ce2f4ad 100644 --- a/internal/provisioner/remote.go +++ b/internal/provisioner/remote.go @@ -24,6 +24,7 @@ type Commander interface { Start(ctx context.Context, hostID, vmID string) (*agent.VM, error) Stop(ctx context.Context, hostID, vmID string) error Snapshot(ctx context.Context, hostID, vmID string) error + Evict(ctx context.Context, hostID, vmID string) error Deprovision(ctx context.Context, hostID, vmID string) error Status(ctx context.Context, hostID, vmID string) (*agent.VM, error) } @@ -101,6 +102,16 @@ func (p *RemoteProvisioner) Stop(ctx context.Context, s *model.GameServer) error return p.cmd.Stop(ctx, hostID, *s.VMID) } +// Evict releases the VM from its host while preserving the durable world, so the +// server can be rescheduled elsewhere on its next start (idempotent). A server +// with no backing VM has nothing to evict. +func (p *RemoteProvisioner) Evict(ctx context.Context, s *model.GameServer) error { + if s.HostID == nil || *s.HostID == "" || s.VMID == nil || *s.VMID == "" { + return nil + } + return p.cmd.Evict(ctx, *s.HostID, *s.VMID) +} + // Deprovision tears down the VM on its host (idempotent). A server that was // never placed or provisioned has nothing to tear down. func (p *RemoteProvisioner) Deprovision(ctx context.Context, s *model.GameServer) error { diff --git a/internal/provisioner/remote_test.go b/internal/provisioner/remote_test.go index 2d8c97b..4998232 100644 --- a/internal/provisioner/remote_test.go +++ b/internal/provisioner/remote_test.go @@ -26,6 +26,9 @@ func (c fakeCommander) Stop(ctx context.Context, _, vmID string) error { func (c fakeCommander) Snapshot(ctx context.Context, _, vmID string) error { return c.rt.Snapshot(ctx, vmID) } +func (c fakeCommander) Evict(ctx context.Context, _, vmID string) error { + return c.rt.Evict(ctx, vmID) +} func (c fakeCommander) Deprovision(ctx context.Context, _, vmID string) error { return c.rt.Deprovision(ctx, vmID) } @@ -57,6 +60,11 @@ func (c errCommander) Snapshot(context.Context, string, string) error { c.t.Fatal("Snapshot called, want no-op") return nil } +func (c errCommander) Evict(context.Context, string, string) error { + c.t.Helper() + c.t.Fatal("Evict called, want no-op") + return nil +} func (c errCommander) Deprovision(context.Context, string, string) error { c.t.Helper() c.t.Fatal("Deprovision called, want no-op") @@ -106,6 +114,12 @@ func TestRemoteProvisionerLifecycle(t *testing.T) { } assertRemoteState(t, p, s, StateRunning) + // Evict releases the VM from its host; the agent reports it gone. + if err := p.Evict(ctx, s); err != nil { + t.Fatalf("evict: %v", err) + } + assertRemoteState(t, p, s, StateMissing) + if err := p.Deprovision(ctx, s); err != nil { t.Fatalf("deprovision: %v", err) } @@ -126,6 +140,9 @@ func TestRemoteProvisionerUnplaced(t *testing.T) { if err := p.Deprovision(ctx, &model.GameServer{ID: "x"}); err != nil { t.Errorf("deprovision unplaced = %v, want nil", err) } + if err := p.Evict(ctx, &model.GameServer{ID: "x"}); err != nil { + t.Errorf("evict unplaced = %v, want nil", err) + } if err := p.Stop(ctx, &model.GameServer{ID: "x", HostID: ptr("h")}); err != nil { t.Errorf("stop with no vm = %v, want nil", err) } diff --git a/internal/reconciler/reconciler.go b/internal/reconciler/reconciler.go index 0c4d16f..1d79f85 100644 --- a/internal/reconciler/reconciler.go +++ b/internal/reconciler/reconciler.go @@ -115,12 +115,13 @@ func (r *Reconciler) start(ctx context.Context, s *model.GameServer) error { if s.Status == model.StatusRunning { return nil } - // A server that already has a VM was provisioned before and merely stopped; - // resume it rather than creating a fresh one. + // A server that still has a VM (e.g. an interrupted provision, retried before + // it was torn down) is resumed in place rather than rebuilt from scratch. provisioned := s.VMID != nil && *s.VMID != "" - // Place a not-yet-provisioned, unassigned server on a host before booting a - // new VM. A resumed VM already lives on its host; an already-assigned server - // keeps its reservation across a failed attempt, so neither re-schedules. + // Place an unassigned server on a host before booting a VM. A stopped server + // was unplaced on stop, so it re-schedules here and may land on a new host + // (its world rides the durable store). A mid-flight server that still has a + // VM or a host assignment keeps it across a retry, so neither re-schedules. if !provisioned && s.HostID == nil { if err := r.place(ctx, s); err != nil { return err @@ -181,11 +182,23 @@ func (r *Reconciler) stop(ctx context.Context, s *model.GameServer) error { if err := r.servers.MarkStatus(ctx, s.ID, model.StatusStopping, ""); err != nil { return err } - // Halt the VM but keep it; the world and runtime details survive for a later - // start. Destruction only happens on delete. + // Halt the VM, snapshotting its world to the durable store. if err := r.prov.Stop(ctx, s); err != nil { return err } + // Release the host: evict the VM's host-local footprint (keeping the durable + // world) and return its reservation, so a stopped server ties up no capacity + // and is free to reschedule onto any host when it next starts. The world + // rides the durable store, keyed by server id, so the restart restores it + // wherever it lands. NOTE: on a host with world persistence but no durable + // store, the world is host-local only, so a reschedule to a different host + // starts from an empty world — an accepted tradeoff for freeing capacity. + if err := r.prov.Evict(ctx, s); err != nil { + return err + } + if s.HostID != nil { + _ = r.sched.Release(ctx, *s.HostID, s.CPUs, s.MemoryMB) + } r.log.Info("server stopped", zap.String("id", s.ID)) return r.servers.MarkStopped(ctx, s.ID) } diff --git a/internal/repository/game_server.go b/internal/repository/game_server.go index 2e516e8..7f5372e 100644 --- a/internal/repository/game_server.go +++ b/internal/repository/game_server.go @@ -173,11 +173,11 @@ func (r *GameServerRepository) SetDesiredState(ctx context.Context, id, desired } // UsedCapacity returns the total cpu and memory currently committed to a host: -// the sum over every live (non-deleted) server assigned to it. A server keeps -// its reservation while stopped (the VM stays put), so this counts all assigned -// servers regardless of status. It lets the control plane rebuild a host's -// allocatable capacity from the durable record after a restart, instead of -// resetting it to total and forgetting in-flight placements. +// the sum over every live (non-deleted) server assigned to it. A stopped server +// has its host_id cleared (see MarkStopped), so it drops out of this sum and +// frees its reservation — only placed servers count. It lets the control plane +// rebuild a host's allocatable capacity from the durable record after a restart, +// instead of resetting it to total and forgetting in-flight placements. func (r *GameServerRepository) UsedCapacity(ctx context.Context, hostID string) (cpus, memoryMB int, err error) { if hostID == "" { return 0, 0, nil @@ -219,12 +219,15 @@ func (r *GameServerRepository) MarkRunning(ctx context.Context, id, vmID, host s return err } -// MarkStopped records a stopped server with its runtime details cleared. +// MarkStopped records a stopped server with its runtime details and host +// assignment cleared. Clearing host_id releases the server's place in the fleet: +// it no longer counts against its old host's capacity (see UsedCapacity), and +// its next start re-runs placement, so a stopped server can resume on any host. func (r *GameServerRepository) MarkStopped(ctx context.Context, id string) error { _, err := r.pool.Exec(ctx, ` UPDATE game_servers SET status = 'stopped', vm_id = NULL, host = NULL, port = NULL, - status_message = NULL, updated_at = now() + host_id = NULL, status_message = NULL, updated_at = now() WHERE id = $1`, id) return err diff --git a/test/e2e/servers_test.go b/test/e2e/servers_test.go index 51b4fa6..f0d8efd 100644 --- a/test/e2e/servers_test.go +++ b/test/e2e/servers_test.go @@ -174,8 +174,12 @@ func TestGameServerLifecycle(t *testing.T) { if running["host"] == nil || running["port"] == nil || running["vm_id"] == nil { t.Errorf("running server missing runtime details: %v", running) } - if running["port"].(float64) != 25565 { - t.Errorf("port = %v, want 25565", running["port"]) + // The fake runtime hands each VM the lowest free host port at or above the + // standard Minecraft port, so a server sharing the suite's host gets 25565 or + // the next free port up — assert it is a valid pool port, not an exact value + // that depends on how many other VMs are live when this test runs. + if p, ok := running["port"].(float64); !ok || p < 25565 { + t.Errorf("port = %v, want a host port >= 25565", running["port"]) } // Stop.