diff --git a/deploy/crd/felis.lolicon.best_minecraftservers.yaml b/deploy/crd/felis.lolicon.best_minecraftservers.yaml index de66cd3..c4672e4 100644 --- a/deploy/crd/felis.lolicon.best_minecraftservers.yaml +++ b/deploy/crd/felis.lolicon.best_minecraftservers.yaml @@ -370,8 +370,10 @@ spec: description: |- EmptySince is when the operator first observed 0 online players during a Running phase (spec §8 idle auto-stop). It is reset when a player joins - or the server stops, so the empty-duration counter starts fresh each time - the server becomes unoccupied. + or the server restarts or stops, so the empty-duration counter starts fresh + each time the server becomes unoccupied. A zero sampled within three minutes + of a run's first ready probe stamps nothing: the players it came up for may + not be in yet. format: date-time type: string endpoint: @@ -422,7 +424,7 @@ spec: readySignalAt: description: |- ReadySignalAt is when the first RCON probe of the current run succeeded. - Every Starting pass clears it, so each start is measured once. + Every Starting or Stopping pass clears it, so each start is measured once. format: date-time type: string startRequestedAt: diff --git a/internal/apis/felis/v1alpha1/minecraftserver_types.go b/internal/apis/felis/v1alpha1/minecraftserver_types.go index 6928916..749562a 100644 --- a/internal/apis/felis/v1alpha1/minecraftserver_types.go +++ b/internal/apis/felis/v1alpha1/minecraftserver_types.go @@ -329,7 +329,7 @@ type MinecraftServerStatus struct { // LiveMotd is the MOTD currently advertised for the active phase. LiveMotd string `json:"liveMotd,omitempty"` // ReadySignalAt is when the first RCON probe of the current run succeeded. - // Every Starting pass clears it, so each start is measured once. + // Every Starting or Stopping pass clears it, so each start is measured once. ReadySignalAt *metav1.Time `json:"readySignalAt,omitempty"` // StartRequestedAt is when the current start attempt was first observed // (the first Starting reconcile after desiredState=Running). It anchors the @@ -340,8 +340,10 @@ type MinecraftServerStatus struct { StartRequestedAt *metav1.Time `json:"startRequestedAt,omitempty"` // EmptySince is when the operator first observed 0 online players during a // Running phase (spec §8 idle auto-stop). It is reset when a player joins - // or the server stops, so the empty-duration counter starts fresh each time - // the server becomes unoccupied. + // or the server restarts or stops, so the empty-duration counter starts fresh + // each time the server becomes unoccupied. A zero sampled within three minutes + // of a run's first ready probe stamps nothing: the players it came up for may + // not be in yet. EmptySince *metav1.Time `json:"emptySince,omitempty"` // StopNoticeAt is when the operator told the players on a server that it is // about to stop (desiredState flipped to Stopped with players online). The stop diff --git a/internal/operator/events_test.go b/internal/operator/events_test.go index e78c3a5..ae106f7 100644 --- a/internal/operator/events_test.go +++ b/internal/operator/events_test.go @@ -101,8 +101,10 @@ func TestIdleStopRecordsAnEvent(t *testing.T) { reconcile(t, r, "survival") markPodReady(t, c, "survival") reconcile(t, r, "survival") + clock = base.Add(operator.ArrivalWindow) + reconcile(t, r, "survival") drain(rec) - clock = base.Add(61 * time.Second) + clock = base.Add(operator.ArrivalWindow + 61*time.Second) reconcile(t, r, "survival") expectEvents(t, rec, "idle", "Normal IdleStop no players online for 60s; set desiredState to Stopped") } diff --git a/internal/operator/export_test.go b/internal/operator/export_test.go index 415fbaf..fa7eb71 100644 --- a/internal/operator/export_test.go +++ b/internal/operator/export_test.go @@ -7,3 +7,7 @@ func (r *Reconciler) ProbeMissesTracked() int { defer r.probeMu.Unlock() return len(r.probeFailures) } + +// ArrivalWindow is how long after a run's first ready probe a zero tally starts no +// idle countdown. +const ArrivalWindow = arrivalWindow diff --git a/internal/operator/reconciler.go b/internal/operator/reconciler.go index 0766c35..cafa21e 100644 --- a/internal/operator/reconciler.go +++ b/internal/operator/reconciler.go @@ -61,6 +61,13 @@ const ( maxAutoRestarts = v1alpha1.MaxAutoRestarts autoRestartBaseBackoff = time.Minute defaultReadinessTimeoutSec = 300 + // arrivalWindow is how long after a run's first ready probe a zero tally starts + // no idle countdown. A server usually comes up because someone asked for it: the + // player who woke it is still being moved in from the lobby, players a restart + // dropped are reconnecting, and a modpack client sits in its configuration phase, + // missing from `list`, for a minute or more. Sampled before they arrived, the + // zero stopped a server with a short idle timeout just as they got in. + arrivalWindow = 3 * time.Minute ) // RconSecretAnnotation stamps the pod template with a fingerprint of the @@ -437,15 +444,16 @@ func (r *Reconciler) reconcileRunning(ctx context.Context, server *v1alpha1.Mine if players.Known && idleStopApplies(server) { if players.Online == 0 { if server.Status.EmptySince == nil { - t := r.now() - server.Status.EmptySince = &t + if now := r.now(); !arriving(server, now) { + server.Status.EmptySince = &now + } } else if r.now().Time.Sub(server.Status.EmptySince.Time).Seconds() >= float64(server.Spec.Idle.EmptySecondsBeforeStop) { // Merge patch, not Update: an unrelated reconcile writes status // concurrently, and shipping the whole object back risks // clobbering it (the reaper's Stop uses the same pattern for // the same reason). EmptySince is deliberately left for - // markStopped to clear once the scale-down completes. + // markStopping to clear once the scale-down starts. patch := client.MergeFrom(server.DeepCopy()) server.Spec.DesiredState = v1alpha1.DesiredStopped if err := r.Patch(ctx, server, patch); err != nil { @@ -494,6 +502,14 @@ func idleStopApplies(server *v1alpha1.MinecraftServer) bool { server.Spec.Idle.AutoStopEnabled && server.Spec.Idle.EmptySecondsBeforeStop > 0 } +// arriving reports whether a zero tally at now may only mean the players this run +// came up for are not in yet: it is the run's first ready probe (ReadySignalAt is +// stamped after it) or within arrivalWindow of that probe. +func arriving(server *v1alpha1.MinecraftServer, now metav1.Time) bool { + ready := server.Status.ReadySignalAt + return ready == nil || now.Time.Before(ready.Add(arrivalWindow)) +} + func (r *Reconciler) reconcileStopped(ctx context.Context, server *v1alpha1.MinecraftServer) (ctrl.Result, error) { var sts appsv1.StatefulSet err := r.Get(ctx, types.NamespacedName{Namespace: server.Namespace, Name: server.Name}, &sts) @@ -937,6 +953,9 @@ func (r *Reconciler) markStarting(server *v1alpha1.MinecraftServer, reason, msg // the last run's ready time would make markRunningReady treat the start as // already observed, and every start after the first would go unmeasured. server.Status.ReadySignalAt = nil + // The empty clock belongs to a run too: the last run's would stop the new one at + // its first zero, before anyone it came up for is in. + server.Status.EmptySince = nil server.Status.Endpoint = v1alpha1.EndpointStatus{Mode: v1alpha1.EndpointFallback, Address: server.Spec.FallbackServer} server.Status.LiveMotd = server.Spec.Motd.Starting r.setCondition(server, v1alpha1.ConditionReady, metav1.ConditionFalse, reason, msg) @@ -982,6 +1001,11 @@ func (r *Reconciler) markRunningReady(server *v1alpha1.MinecraftServer, players func (r *Reconciler) markStopping(server *v1alpha1.MinecraftServer) { server.Status.Phase = v1alpha1.PhaseStopping + // The run ends here. A wake can land before the scale-down completes and bring the + // pod straight back: the idle stop's stale clock would then stop it again at its + // first zero, and the players the wake is for get their arrival window. + server.Status.EmptySince = nil + server.Status.ReadySignalAt = nil server.Status.Ready = false server.Status.ObservedGeneration = server.Generation server.Status.Endpoint = v1alpha1.EndpointStatus{Mode: v1alpha1.EndpointFallback, Address: server.Spec.FallbackServer} diff --git a/internal/operator/reconciler_test.go b/internal/operator/reconciler_test.go index 5ec21bf..09a7eac 100644 --- a/internal/operator/reconciler_test.go +++ b/internal/operator/reconciler_test.go @@ -435,10 +435,14 @@ func TestReconcileStopped_SkipsSaveWithoutReadyPodOrRcon(t *testing.T) { // --- idle auto-stop tests (spec §8) --------------------------------------- -// TestIdleAutoStop_EmptyServerGetsTimestamp verifies that the first Running -// reconcile with zero players stamps EmptySince and keeps the server Running. +// TestIdleAutoStop_EmptyServerGetsTimestamp verifies that the first zero +// sampled once the arrival window is over stamps EmptySince and keeps the server +// Running. func TestIdleAutoStop_EmptyServerGetsTimestamp(t *testing.T) { r, c := newReconciler(t, fakeProber{players: operator.PlayerCount{Online: 0, Max: 20, Known: true}}, runningServer(), rconSecret()) + base := time.Date(2026, 7, 1, 10, 0, 0, 0, time.UTC) + clock := base + r.Now = func() metav1.Time { return metav1.NewTime(clock) } // Enable idle auto-stop with a generous timeout so we don't trigger the // actual stop in this test. s := getServer(t, c, "survival") @@ -450,13 +454,15 @@ func TestIdleAutoStop_EmptyServerGetsTimestamp(t *testing.T) { reconcile(t, r, "survival") markPodReady(t, c, "survival") reconcile(t, r, "survival") + clock = base.Add(operator.ArrivalWindow) + reconcile(t, r, "survival") server := getServer(t, c, "survival") if server.Status.Phase != v1alpha1.PhaseRunning || !server.Status.Ready { t.Fatalf("phase = %s ready=%v, want Running ready", server.Status.Phase, server.Status.Ready) } - if server.Status.EmptySince == nil { - t.Fatal("EmptySince should be set for an empty server with idle autostop enabled") + if server.Status.EmptySince == nil || !server.Status.EmptySince.Equal(ptrTime(metav1.NewTime(clock))) { + t.Fatalf("EmptySince = %v, want %v: the first zero after the arrival window", server.Status.EmptySince, clock) } } @@ -478,21 +484,24 @@ func TestIdleAutoStop_StopsAfterTimeout(t *testing.T) { t.Fatalf("enable idle: %v", err) } - // First reconcile: Running, 0 players → stamp EmptySince = base. + // Running, 0 players: the first sample past the arrival window stamps EmptySince. reconcile(t, r, "survival") markPodReady(t, c, "survival") reconcile(t, r, "survival") + stamp := base.Add(operator.ArrivalWindow) + clock = stamp + reconcile(t, r, "survival") server := getServer(t, c, "survival") if server.Status.Phase != v1alpha1.PhaseRunning { t.Fatalf("phase = %s, want Running", server.Status.Phase) } - if server.Status.EmptySince == nil || !server.Status.EmptySince.Equal(ptrTime(metav1.NewTime(base))) { - t.Fatalf("EmptySince = %v, want %v", server.Status.EmptySince, base) + if server.Status.EmptySince == nil || !server.Status.EmptySince.Equal(ptrTime(metav1.NewTime(stamp))) { + t.Fatalf("EmptySince = %v, want %v", server.Status.EmptySince, stamp) } // Advance past timeout. - clock = base.Add(61 * time.Second) + clock = stamp.Add(61 * time.Second) reconcile(t, r, "survival") server = getServer(t, c, "survival") @@ -506,6 +515,9 @@ func TestIdleAutoStop_StopsAfterTimeout(t *testing.T) { func TestIdleAutoStop_ResetsWhenPlayerJoins(t *testing.T) { emptyProber := fakeProber{players: operator.PlayerCount{Online: 0, Max: 20, Known: true}} r, c := newReconciler(t, emptyProber, runningServer(), rconSecret()) + base := time.Date(2026, 7, 1, 10, 0, 0, 0, time.UTC) + clock := base + r.Now = func() metav1.Time { return metav1.NewTime(clock) } s := getServer(t, c, "survival") s.Spec.Idle = v1alpha1.IdleSpec{AutoStopEnabled: true, EmptySecondsBeforeStop: 900} @@ -513,10 +525,12 @@ func TestIdleAutoStop_ResetsWhenPlayerJoins(t *testing.T) { t.Fatalf("enable idle: %v", err) } - // First reconcile: Running, 0 players → stamp EmptySince. + // Running, 0 players past the arrival window → stamp EmptySince. reconcile(t, r, "survival") markPodReady(t, c, "survival") reconcile(t, r, "survival") + clock = base.Add(operator.ArrivalWindow) + reconcile(t, r, "survival") server := getServer(t, c, "survival") if server.Status.EmptySince == nil { @@ -553,6 +567,7 @@ func TestIdleAutoStop_UnreadTallyNeverStops(t *testing.T) { reconcile(t, r, "survival") markPodReady(t, c, "survival") reconcile(t, r, "survival") + clock = base.Add(operator.ArrivalWindow) // The tally becomes unreadable: nothing is stamped, the last count stays. r.Prober = fakeProber{} @@ -578,7 +593,7 @@ func TestIdleAutoStop_UnreadTallyNeverStops(t *testing.T) { t.Fatalf("PlayersCounted = %+v, want True after a readable reply", cond) } r.Prober = fakeProber{} - clock = base.Add(10 * time.Minute) + clock = base.Add(operator.ArrivalWindow + 10*time.Minute) reconcile(t, r, "survival") server = getServer(t, c, "survival") if server.Spec.DesiredState != v1alpha1.DesiredRunning { @@ -633,23 +648,137 @@ func TestIdleAutoStop_SystemServerNeverIdles(t *testing.T) { func TestIdleAutoStop_RequeuesUntilDeadline(t *testing.T) { r, c := newReconciler(t, fakeProber{players: operator.PlayerCount{Online: 0, Max: 20, Known: true}}, runningServer(), rconSecret()) base := time.Date(2026, 7, 1, 10, 0, 0, 0, time.UTC) - r.Now = func() metav1.Time { return metav1.NewTime(base) } + clock := base + r.Now = func() metav1.Time { return metav1.NewTime(clock) } s := getServer(t, c, "survival") - s.Spec.Idle = v1alpha1.IdleSpec{AutoStopEnabled: true, EmptySecondsBeforeStop: 30} + s.Spec.Idle = v1alpha1.IdleSpec{AutoStopEnabled: true, EmptySecondsBeforeStop: 90} if err := c.Update(context.Background(), s); err != nil { t.Fatalf("enable idle: %v", err) } reconcile(t, r, "survival") markPodReady(t, c, "survival") + // Inside the arrival window there is no deadline yet; the idle probe cadence + // notices the window closing. + if res := reconcile(t, r, "survival"); res.RequeueAfter != 30*time.Second { + t.Fatalf("RequeueAfter = %v, want the 30s idle probe while players may still be arriving", res.RequeueAfter) + } + clock = base.Add(operator.ArrivalWindow) res := reconcile(t, r, "survival") - if res.RequeueAfter != 30*time.Second { - t.Fatalf("RequeueAfter = %v, want exactly 30s (wake at the auto-stop deadline)", res.RequeueAfter) + if res.RequeueAfter != 90*time.Second { + t.Fatalf("RequeueAfter = %v, want exactly 90s (wake at the auto-stop deadline)", res.RequeueAfter) } } +// idleServer is a Running server with a 60s idle auto-stop, probed as empty on a +// clock the test moves; ready is when its first ready probe ran. +func idleServer(t *testing.T) (r *operator.Reconciler, c client.Client, clock *time.Time, ready time.Time) { + t.Helper() + srv := runningServer() + srv.Spec.Idle = v1alpha1.IdleSpec{AutoStopEnabled: true, EmptySecondsBeforeStop: 60} + r, c = newReconciler(t, fakeProber{players: operator.PlayerCount{Online: 0, Max: 20, Known: true}}, srv, rconSecret()) + ready = time.Date(2026, 7, 1, 10, 0, 0, 0, time.UTC) + now := ready + clock = &now + r.Now = func() metav1.Time { return metav1.NewTime(*clock) } + reconcile(t, r, "survival") + markPodReady(t, c, "survival") + reconcile(t, r, "survival") + return r, c, clock, ready +} + +// expectIdle checks the idle countdown after one more reconcile at at. +func expectIdle(t *testing.T, r *operator.Reconciler, c client.Client, clock *time.Time, at time.Time, desired v1alpha1.DesiredState, emptySince *time.Time) { + t.Helper() + *clock = at + reconcile(t, r, "survival") + s := getServer(t, c, "survival") + gotSince, wantSince := "none", "none" + if s.Status.EmptySince != nil { + gotSince = s.Status.EmptySince.UTC().String() + } + if emptySince != nil { + wantSince = emptySince.UTC().String() + } + if s.Spec.DesiredState != desired || gotSince != wantSince { + t.Fatalf("at %v: desiredState=%s emptySince=%s, want %s and %s", at, s.Spec.DesiredState, gotSince, desired, wantSince) + } +} + +// TestIdleAutoStop_ArrivalWindowHoldsTheCountdown: a server comes up because +// someone asked for it, and that player is still being moved in when the first +// ready probe reads zero. With a short timeout, the countdown stamped then stopped +// the server just as they got in; it starts once the arrival window is over. +func TestIdleAutoStop_ArrivalWindowHoldsTheCountdown(t *testing.T) { + r, c, clock, ready := idleServer(t) + if s := getServer(t, c, "survival"); s.Status.Phase != v1alpha1.PhaseRunning || s.Status.EmptySince != nil { + t.Fatalf("first ready probe: phase=%s emptySince=%v, want Running and no countdown", s.Status.Phase, s.Status.EmptySince) + } + expectIdle(t, r, c, clock, ready.Add(2*time.Minute), v1alpha1.DesiredRunning, nil) + expectIdle(t, r, c, clock, ready.Add(operator.ArrivalWindow-time.Second), v1alpha1.DesiredRunning, nil) + stamp := ready.Add(operator.ArrivalWindow) + expectIdle(t, r, c, clock, stamp, v1alpha1.DesiredRunning, &stamp) + expectIdle(t, r, c, clock, stamp.Add(59*time.Second), v1alpha1.DesiredRunning, &stamp) + expectIdle(t, r, c, clock, stamp.Add(60*time.Second), v1alpha1.DesiredStopped, &stamp) +} + +// TestIdleAutoStop_CalledBackStopStartsAFreshRun: a wake can land after the idle +// stop but before the scale-down completes, and the same pod comes straight back. +// The stop's EmptySince used to survive into that run and stop it again at its +// first zero, with the player who woke it still on the way in. +func TestIdleAutoStop_CalledBackStopStartsAFreshRun(t *testing.T) { + r, c, clock, ready := idleServer(t) + stamp := ready.Add(operator.ArrivalWindow) + expectIdle(t, r, c, clock, stamp, v1alpha1.DesiredRunning, &stamp) + expectIdle(t, r, c, clock, stamp.Add(61*time.Second), v1alpha1.DesiredStopped, &stamp) + + reconcile(t, r, "survival") // scaled down; the fake pod still counts as ready + s := getServer(t, c, "survival") + if s.Status.Phase != v1alpha1.PhaseStopping { + t.Fatalf("phase = %s, want Stopping while the pod is still up", s.Status.Phase) + } + s.Spec.DesiredState = v1alpha1.DesiredRunning + if err := c.Update(context.Background(), s); err != nil { + t.Fatalf("wake: %v", err) + } + + woke := stamp.Add(70 * time.Second) + expectIdle(t, r, c, clock, woke, v1alpha1.DesiredRunning, nil) + if s := getServer(t, c, "survival"); s.Status.Phase != v1alpha1.PhaseRunning { + t.Fatalf("phase = %s, want Running on the pod that came back", s.Status.Phase) + } + // The run that came back gets its own arrival window. + expectIdle(t, r, c, clock, woke.Add(2*time.Minute), v1alpha1.DesiredRunning, nil) +} + +// TestIdleAutoStop_RestartStartsAFreshRun: a run that drops back to Starting (the +// pod restarted) comes back to players reconnecting. The old run's EmptySince +// stopped it at its first zero. +func TestIdleAutoStop_RestartStartsAFreshRun(t *testing.T) { + r, c, clock, ready := idleServer(t) + stamp := ready.Add(operator.ArrivalWindow) + expectIdle(t, r, c, clock, stamp, v1alpha1.DesiredRunning, &stamp) + + *clock = stamp.Add(30 * time.Second) + sts := getSTS(t, c, "survival") + sts.Status.ReadyReplicas = 0 + if err := c.Status().Update(context.Background(), sts); err != nil { + t.Fatalf("update sts status: %v", err) + } + reconcile(t, r, "survival") + if s := getServer(t, c, "survival"); s.Status.Phase != v1alpha1.PhaseStarting || s.Status.EmptySince != nil { + t.Fatalf("after the restart: phase=%s emptySince=%v, want Starting and no countdown", s.Status.Phase, s.Status.EmptySince) + } + + back := stamp.Add(45 * time.Second) + *clock = back + markPodReady(t, c, "survival") + expectIdle(t, r, c, clock, back, v1alpha1.DesiredRunning, nil) + expectIdle(t, r, c, clock, back.Add(2*time.Minute), v1alpha1.DesiredRunning, nil) +} + // TestIdleAutoStop_RequeuesWhileOccupied verifies the slow probe cadence that // notices the last player leaving: with players online there is no deadline to // aim at, but the tally must still be re-sampled.