From 6ec1b2726cbfaa01948df6379e07927136bce636 Mon Sep 17 00:00:00 2001 From: Lemon-miaow Date: Fri, 25 Sep 2026 19:19:57 +0800 Subject: [PATCH] =?UTF-8?q?feat(operator):=20=E6=B8=B8=E6=88=8F=E5=AE=B9?= =?UTF-8?q?=E5=99=A8=E5=8A=A0=20startup/liveness=20=E6=8E=A2=E9=92=88?= =?UTF-8?q?=EF=BC=8C=E8=B6=85=E6=97=B6=E5=90=AF=E5=8A=A8=E6=8C=89=201/2/4?= =?UTF-8?q?=20=E5=88=86=E9=92=9F=E9=80=80=E9=81=BF=E9=87=8D=E5=BB=BA=20Pod?= =?UTF-8?q?=20=E8=87=B3=E5=A4=9A=203=20=E6=AC=A1=EF=BC=8CRunning=20?= =?UTF-8?q?=E6=AF=8F=2060s=20=E9=87=8D=E6=8E=A2=E4=B8=94=E8=BF=9E=E7=BB=AD?= =?UTF-8?q?=203=20=E6=AC=A1=E5=A4=B1=E8=B4=A5=E6=89=8D=E9=99=8D=E7=BA=A7?= =?UTF-8?q?=EF=BC=8CFailed=20=E6=94=BE=E7=BC=93=E9=87=8D=E6=8E=92?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../felis.lolicon.best_minecraftservers.yaml | 12 ++ .../felis/v1alpha1/minecraftserver_types.go | 6 + .../felis/v1alpha1/zz_generated.deepcopy.go | 3 + internal/operator/autorestart_test.go | 184 ++++++++++++++++++ internal/operator/builders.go | 47 ++++- internal/operator/probes_internal_test.go | 56 ++++++ internal/operator/reconciler.go | 134 +++++++++++-- internal/operator/reconciler_test.go | 13 +- internal/platform/rbac.go | 9 +- internal/platform/rbac_test.go | 11 +- 10 files changed, 440 insertions(+), 35 deletions(-) create mode 100644 internal/operator/autorestart_test.go create mode 100644 internal/operator/probes_internal_test.go diff --git a/deploy/crd/felis.lolicon.best_minecraftservers.yaml b/deploy/crd/felis.lolicon.best_minecraftservers.yaml index 1afe293..4f4f926 100644 --- a/deploy/crd/felis.lolicon.best_minecraftservers.yaml +++ b/deploy/crd/felis.lolicon.best_minecraftservers.yaml @@ -285,6 +285,13 @@ spec: status: description: MinecraftServerStatus is the observed state (spec ยง4 status.*). properties: + autoRestarts: + description: |- + AutoRestarts counts how often the operator recreated the pod of a start + that timed out (at most 3, with a doubling backoff); reaching Ready or + stopping resets it. + format: int32 + type: integer conditions: description: Conditions are the standard metav1 conditions (Ready, RconReached, ...). @@ -361,6 +368,11 @@ spec: description: Mode is "direct" or "fallback". type: string type: object + lastAutoRestartAt: + description: LastAutoRestartAt is when the operator last recreated + the pod. + format: date-time + type: string liveMotd: description: LiveMotd is the MOTD currently advertised for the active phase. diff --git a/internal/apis/felis/v1alpha1/minecraftserver_types.go b/internal/apis/felis/v1alpha1/minecraftserver_types.go index 2094169..31b85cb 100644 --- a/internal/apis/felis/v1alpha1/minecraftserver_types.go +++ b/internal/apis/felis/v1alpha1/minecraftserver_types.go @@ -277,6 +277,12 @@ type MinecraftServerStatus struct { // or the server stops, so the empty-duration counter starts fresh each time // the server becomes unoccupied. EmptySince *metav1.Time `json:"emptySince,omitempty"` + // AutoRestarts counts how often the operator recreated the pod of a start + // that timed out (at most 3, with a doubling backoff); reaching Ready or + // stopping resets it. + AutoRestarts int32 `json:"autoRestarts,omitempty"` + // LastAutoRestartAt is when the operator last recreated the pod. + LastAutoRestartAt *metav1.Time `json:"lastAutoRestartAt,omitempty"` // ObservedGeneration is the spec generation this status reflects. ObservedGeneration int64 `json:"observedGeneration,omitempty"` // Conditions are the standard metav1 conditions (Ready, RconReached, ...). diff --git a/internal/apis/felis/v1alpha1/zz_generated.deepcopy.go b/internal/apis/felis/v1alpha1/zz_generated.deepcopy.go index de5276f..a125a81 100644 --- a/internal/apis/felis/v1alpha1/zz_generated.deepcopy.go +++ b/internal/apis/felis/v1alpha1/zz_generated.deepcopy.go @@ -121,6 +121,9 @@ func (in *MinecraftServerStatus) DeepCopyInto(out *MinecraftServerStatus) { if in.EmptySince != nil { out.EmptySince = in.EmptySince.DeepCopy() } + if in.LastAutoRestartAt != nil { + out.LastAutoRestartAt = in.LastAutoRestartAt.DeepCopy() + } if in.Conditions != nil { l := make([]metav1.Condition, len(in.Conditions)) for i := range in.Conditions { diff --git a/internal/operator/autorestart_test.go b/internal/operator/autorestart_test.go new file mode 100644 index 0000000..fe91c3f --- /dev/null +++ b/internal/operator/autorestart_test.go @@ -0,0 +1,184 @@ +package operator_test + +import ( + "context" + "errors" + "testing" + "time" + + corev1 "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + + "felis.lolicon.best/internal/apis/felis/v1alpha1" + "felis.lolicon.best/internal/operator" +) + +func gamePod() *corev1.Pod { + return &corev1.Pod{ObjectMeta: metav1.ObjectMeta{Name: "survival-0", Namespace: "minecraft"}} +} + +// A start that timed out is retried by recreating its pod after a doubling +// backoff (1m, 2m, 4m past the timeout), three times, then left Failed. +func TestTimedOutStartRecreatesThePodWithBackoff(t *testing.T) { + srv := runningServer() + srv.Spec.Startup.TimeoutSeconds = 30 + r, c := newReconciler(t, fakeProber{}, srv, rconSecret(), gamePod()) + base := time.Date(2026, 7, 1, 10, 0, 0, 0, time.UTC) + clock := base + r.Now = func() metav1.Time { return metav1.NewTime(clock) } + podExists := func() bool { + err := c.Get(context.Background(), types.NamespacedName{Namespace: "minecraft", Name: "survival-0"}, &corev1.Pod{}) + if err != nil && !apierrors.IsNotFound(err) { + t.Fatalf("get pod: %v", err) + } + return err == nil + } + at := func(offset time.Duration) *v1alpha1.MinecraftServer { + clock = base.Add(offset) + reconcile(t, r, "survival") + return getServer(t, c, "survival") + } + + at(0) // anchors the start at base + // Each attempt: [anchor, the instant just before it is due, the due instant]. + attempts := [][3]time.Duration{ + {0, 89 * time.Second, 90 * time.Second}, // 30s timeout + 1m + {90 * time.Second, 239 * time.Second, 240 * time.Second}, // + 30s + 2m + {240 * time.Second, 509 * time.Second, 510 * time.Second}, // + 30s + 4m + } + for i, a := range attempts { + if s := at(a[1]); s.Status.Phase != v1alpha1.PhaseFailed || !podExists() || s.Status.AutoRestarts != int32(i) { + t.Fatalf("attempt %d before due: phase=%s pod=%v restarts=%d", i+1, s.Status.Phase, podExists(), s.Status.AutoRestarts) + } + s := at(a[2]) + if podExists() { + t.Fatalf("attempt %d: pod survived the due instant", i+1) + } + if s.Status.AutoRestarts != int32(i+1) || s.Status.Phase != v1alpha1.PhaseStarting { + t.Fatalf("attempt %d: restarts=%d phase=%s", i+1, s.Status.AutoRestarts, s.Status.Phase) + } + if s.Status.LastAutoRestartAt == nil || !s.Status.LastAutoRestartAt.Time.Equal(base.Add(a[2])) { + t.Fatalf("attempt %d: lastAutoRestartAt=%v", i+1, s.Status.LastAutoRestartAt) + } + if s.Status.StartRequestedAt == nil || !s.Status.StartRequestedAt.Time.Equal(base.Add(a[2])) { + t.Fatalf("attempt %d: start not re-anchored: %v", i+1, s.Status.StartRequestedAt) + } + if err := c.Create(context.Background(), gamePod()); err != nil { // the StatefulSet's replacement + t.Fatal(err) + } + } + + // Attempts spent: long after the next timeout the pod stays and so does Failed. + s := at(time.Hour) + if s.Status.Phase != v1alpha1.PhaseFailed || s.Status.AutoRestarts != 3 || !podExists() { + t.Fatalf("after three attempts: phase=%s restarts=%d pod=%v", s.Status.Phase, s.Status.AutoRestarts, podExists()) + } +} + +// An RCON channel that never answers on a TCP-ready pod gets the same retry. +func TestReadinessTimeoutRecreatesThePod(t *testing.T) { + srv := runningServer() + srv.Spec.Startup.ReadinessTimeoutSeconds = 30 + r, c := newReconciler(t, fakeProber{err: errors.New("connection refused")}, srv, rconSecret(), gamePod()) + base := time.Date(2026, 7, 1, 10, 0, 0, 0, time.UTC) + clock := base + r.Now = func() metav1.Time { return metav1.NewTime(clock) } + + reconcile(t, r, "survival") + markPodReady(t, c, "survival") + clock = base.Add(89 * time.Second) + reconcile(t, r, "survival") + if s := getServer(t, c, "survival"); s.Status.Phase != v1alpha1.PhaseFailed || s.Status.AutoRestarts != 0 { + t.Fatalf("before due: phase=%s restarts=%d", s.Status.Phase, s.Status.AutoRestarts) + } + clock = base.Add(90 * time.Second) + reconcile(t, r, "survival") + s := getServer(t, c, "survival") + err := c.Get(context.Background(), types.NamespacedName{Namespace: "minecraft", Name: "survival-0"}, &corev1.Pod{}) + if !apierrors.IsNotFound(err) || s.Status.AutoRestarts != 1 || s.Status.Phase != v1alpha1.PhaseStarting { + t.Fatalf("due: pod err=%v restarts=%d phase=%s", err, s.Status.AutoRestarts, s.Status.Phase) + } +} + +// switchProber fails while *down is true, so one server can miss and recover. +type switchProber struct{ down *bool } + +func (p switchProber) Probe(context.Context, string, string) (operator.PlayerCount, error) { + if *p.down { + return operator.PlayerCount{}, errors.New("i/o timeout") + } + return operator.PlayerCount{Online: 1, Max: 20}, nil +} + +func (switchProber) Save(context.Context, string, string) error { return nil } + +// A Running server keeps its phase and endpoint through two missed probes, +// re-probing every 10s; the third consecutive miss degrades it to Starting, +// and any success in between starts the count over. +func TestRunningServerDegradesOnlyAfterThreeMisses(t *testing.T) { + down := false + r, c := newReconciler(t, switchProber{down: &down}, runningServer(), rconSecret()) + reconcile(t, r, "survival") + markPodReady(t, c, "survival") + if res := reconcile(t, r, "survival"); res.RequeueAfter != time.Minute { + t.Fatalf("healthy Running requeue = %v, want 1m", res.RequeueAfter) + } + stillRunning := func(label string) { + t.Helper() + s := getServer(t, c, "survival") + if s.Status.Phase != v1alpha1.PhaseRunning || !s.Status.Ready || s.Status.Endpoint.Address != "10.43.0.42:25565" { + t.Fatalf("%s: phase=%s ready=%v endpoint=%q", label, s.Status.Phase, s.Status.Ready, s.Status.Endpoint.Address) + } + } + stillRunning("ready") + + down = true + for i := 1; i <= 2; i++ { + if res := reconcile(t, r, "survival"); res.RequeueAfter != 10*time.Second { + t.Fatalf("miss %d requeue = %v, want 10s", i, res.RequeueAfter) + } + stillRunning("after a miss") + } + down = false + reconcile(t, r, "survival") // a success clears the two misses + down = true + reconcile(t, r, "survival") + reconcile(t, r, "survival") + stillRunning("two misses after a recovery") + + reconcile(t, r, "survival") // third in a row + if s := getServer(t, c, "survival"); s.Status.Phase != v1alpha1.PhaseStarting || s.Status.Ready { + t.Fatalf("third miss: phase=%s ready=%v, want Starting not-ready", s.Status.Phase, s.Status.Ready) + } +} + +// A Failed server wakes when its next auto-restart falls due, and every 5m +// once the attempts are spent, instead of every 2s. +func TestFailedServerRequeuesWhenTheRetryIsDue(t *testing.T) { + srv := runningServer() + srv.Spec.Startup.TimeoutSeconds = 30 + r, c := newReconciler(t, fakeProber{}, srv, rconSecret(), gamePod()) + base := time.Date(2026, 7, 1, 10, 0, 0, 0, time.UTC) + clock := base + r.Now = func() metav1.Time { return metav1.NewTime(clock) } + + if res := reconcile(t, r, "survival"); res.RequeueAfter != 2*time.Second { + t.Fatalf("Starting requeue = %v, want 2s", res.RequeueAfter) + } + clock = base.Add(40 * time.Second) // timed out at 30s, first retry due at 90s + if res := reconcile(t, r, "survival"); res.RequeueAfter != 50*time.Second { + t.Fatalf("Failed requeue = %v, want 50s", res.RequeueAfter) + } + + s := getServer(t, c, "survival") + s.Status.AutoRestarts = 3 + if err := c.Status().Update(context.Background(), s); err != nil { + t.Fatal(err) + } + clock = base.Add(41 * time.Second) + if res := reconcile(t, r, "survival"); res.RequeueAfter != 5*time.Minute { + t.Fatalf("spent requeue = %v, want 5m", res.RequeueAfter) + } +} diff --git a/internal/operator/builders.go b/internal/operator/builders.go index 3e00488..5deb790 100644 --- a/internal/operator/builders.go +++ b/internal/operator/builders.go @@ -1,6 +1,8 @@ package operator import ( + "time" + "fmt" "strconv" @@ -165,25 +167,50 @@ func servicePorts(server *v1alpha1.MinecraftServer) []corev1.ServicePort { // RCON-less loader's own "started" signal โ€” not the mere fact that the game // socket is bound โ€” gates readiness. Timings are identical across both modes. func readinessProbe(server *v1alpha1.MinecraftServer) *corev1.Probe { - probe := &corev1.Probe{ + return &corev1.Probe{ + ProbeHandler: healthHandler(server), InitialDelaySeconds: 20, PeriodSeconds: 10, FailureThreshold: 6, } +} + +// startupProbe holds liveness off until the server first answers. Its budget +// outlasts the operator's startup timeout plus the first auto-restart backoff, +// so a slow first world generation is the operator's to judge (a counted, +// bounded pod restart) and never a kubelet restart loop. +func startupProbe(server *v1alpha1.MinecraftServer) *corev1.Probe { + const period = 10 + budget := startupTimeout(server) + autoRestartBaseBackoff + return &corev1.Probe{ + ProbeHandler: healthHandler(server), + PeriodSeconds: period, + FailureThreshold: int32((budget+period*time.Second-1)/(period*time.Second)) + 1, + } +} + +// livenessProbe restarts a server that stopped answering for two minutes: long +// enough to ride out a world save or a lag spike. +func livenessProbe(server *v1alpha1.MinecraftServer) *corev1.Probe { + return &corev1.Probe{ + ProbeHandler: healthHandler(server), + PeriodSeconds: 20, + TimeoutSeconds: 5, + FailureThreshold: 6, + } +} + +// healthHandler is a plain TCP check on the game port, or an HTTP GET on the +// loader's health endpoint when StartupSpec.HealthHTTPPort is set. +func healthHandler(server *v1alpha1.MinecraftServer) corev1.ProbeHandler { if hp := server.Spec.Startup.HealthHTTPPort; hp > 0 { path := server.Spec.Startup.HealthHTTPPath if path == "" { path = "/healthz" } - probe.ProbeHandler = corev1.ProbeHandler{ - HTTPGet: &corev1.HTTPGetAction{Path: path, Port: intstr.FromInt32(hp)}, - } - return probe + return corev1.ProbeHandler{HTTPGet: &corev1.HTTPGetAction{Path: path, Port: intstr.FromInt32(hp)}} } - probe.ProbeHandler = corev1.ProbeHandler{ - TCPSocket: &corev1.TCPSocketAction{Port: intstr.FromInt32(GamePort)}, - } - return probe + return corev1.ProbeHandler{TCPSocket: &corev1.TCPSocketAction{Port: intstr.FromInt32(GamePort)}} } // buildStatefulSet renders the workload for replicas in {0,1}. Its half of @@ -217,6 +244,8 @@ func buildStatefulSet(server *v1alpha1.MinecraftServer, replicas int32, felisIma // StartupSpec.HealthHTTPPort) that reports true readiness โ€” used below when // set. ReadinessProbe: readinessProbe(server), + StartupProbe: startupProbe(server), + LivenessProbe: livenessProbe(server), // The server runs untrusted plugins, so it keeps no capability and can never // regain one. The root filesystem stays writable: an arbitrary Paper image // may unpack its runtime or write temp files outside /data. diff --git a/internal/operator/probes_internal_test.go b/internal/operator/probes_internal_test.go new file mode 100644 index 0000000..5dcd0ba --- /dev/null +++ b/internal/operator/probes_internal_test.go @@ -0,0 +1,56 @@ +package operator + +import ( + "testing" + "time" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + "felis.lolicon.best/internal/apis/felis/v1alpha1" +) + +func TestReadyAndStopResetAutoRestarts(t *testing.T) { + r := &Reconciler{Now: func() metav1.Time { return metav1.NewTime(time.Date(2026, 7, 1, 10, 0, 0, 0, time.UTC)) }} + s := &v1alpha1.MinecraftServer{ObjectMeta: metav1.ObjectMeta{Name: "survival", Namespace: "minecraft"}} + s.Status.AutoRestarts = 2 + r.markRunningReady(s, PlayerCount{}, "10.43.0.9") + if s.Status.AutoRestarts != 0 { + t.Fatalf("ready: restarts = %d", s.Status.AutoRestarts) + } + s.Status.AutoRestarts = 3 + r.markStopped(s) + if s.Status.AutoRestarts != 0 { + t.Fatalf("stopped: restarts = %d", s.Status.AutoRestarts) + } +} + +func TestGameContainerProbes(t *testing.T) { + srv := &v1alpha1.MinecraftServer{ObjectMeta: metav1.ObjectMeta{Name: "survival", Namespace: "minecraft"}} + srv.Spec.Image = "itzg/minecraft-server:java21" + srv.Spec.Startup.TimeoutSeconds = 300 + sts, err := buildStatefulSet(srv, 1, "felis:test") + if err != nil { + t.Fatal(err) + } + c := sts.Spec.Template.Spec.Containers[0] + if p := c.StartupProbe; p == nil || p.TCPSocket == nil || p.TCPSocket.Port.IntValue() != 25565 || p.PeriodSeconds != 10 || p.FailureThreshold != 37 { + t.Fatalf("startup probe = %+v", p) + } + if p := c.LivenessProbe; p == nil || p.TCPSocket == nil || p.TCPSocket.Port.IntValue() != 25565 || p.PeriodSeconds != 20 || p.TimeoutSeconds != 5 || p.FailureThreshold != 6 { + t.Fatalf("liveness probe = %+v", p) + } + + srv.Spec.Startup.HealthHTTPPort = 8080 + srv.Spec.Startup.TimeoutSeconds = 0 // default 300s + sts, err = buildStatefulSet(srv, 1, "felis:test") + if err != nil { + t.Fatal(err) + } + c = sts.Spec.Template.Spec.Containers[0] + if p := c.LivenessProbe; p == nil || p.HTTPGet == nil || p.HTTPGet.Port.IntValue() != 8080 || p.HTTPGet.Path != "/healthz" { + t.Fatalf("login-gate liveness = %+v", p) + } + if p := c.StartupProbe; p == nil || p.HTTPGet == nil || p.FailureThreshold != 37 { + t.Fatalf("login-gate startup = %+v", p) + } +} diff --git a/internal/operator/reconciler.go b/internal/operator/reconciler.go index e6ce7d9..7f9b4ef 100644 --- a/internal/operator/reconciler.go +++ b/internal/operator/reconciler.go @@ -12,6 +12,7 @@ import ( "encoding/hex" "encoding/json" "fmt" + "sync" "time" "felis.lolicon.best/internal/apis/felis/v1alpha1" @@ -38,12 +39,25 @@ const ( // still answers 409 not_running: at 5s the observed lag after container-ready // was 6~10s; 2s keeps wake-to-usable snappy without hammering a booting Java // process (the probe only runs on this cadence while the server is unreachable). - requeueStarting = 2 * time.Second - requeueStopping = 5 * time.Second - requeueSecret = 10 * time.Second - requeueIdleProbe = 30 * time.Second - requeueMaintenance = 5 * time.Second - defaultTimeoutSeconds = 300 + requeueStarting = 2 * time.Second + requeueStopping = 5 * time.Second + requeueSecret = 10 * time.Second + requeueIdleProbe = 30 * time.Second + requeueRunningProbe = 60 * time.Second + // requeueProbeRetry re-probes a Running server soon after a miss; + // runningProbeMissesBeforeDegrade consecutive misses demote it to Starting. + requeueProbeRetry = 10 * time.Second + runningProbeMissesBeforeDegrade = 3 + // requeueFailed paces a Failed server whose auto-restarts are spent; a + // StatefulSet or CR event still wakes it at once. + requeueFailed = 5 * time.Minute + requeueMaintenance = 5 * time.Second + defaultTimeoutSeconds = 300 + // maxAutoRestarts bounds how often a timed-out start is retried by + // recreating its pod; autoRestartBaseBackoff is the first wait, doubling + // per attempt. + maxAutoRestarts = 3 + autoRestartBaseBackoff = time.Minute defaultReadinessTimeoutSec = 300 ) @@ -110,6 +124,12 @@ type Reconciler struct { Jobs client.Reader // Watch records the passes in flight for the liveness probe. Nil skips it. Watch *ReconcileWatch + + // probeFailures counts consecutive failed RCON probes of a Running server, + // by UID. In memory: an operator restart forgets them, which only delays a + // degrade by a few probes. + probeMu sync.Mutex + probeFailures map[types.UID]int } func (r *Reconciler) now() metav1.Time { @@ -231,11 +251,14 @@ func (r *Reconciler) reconcileRunning(ctx context.Context, server *v1alpha1.Mine r.markStarting(server, "PodNotReady", "waiting for pod TCP readiness") if r.startupTimedOut(server) { r.markFailed(server, "StartupTimeout", "pod did not become ready within startup timeout") + if err := r.recoverFailedStart(ctx, server, startupTimeout(server)); err != nil { + return ctrl.Result{}, err + } } if err := r.patchStatus(ctx, server); err != nil { return ctrl.Result{}, err } - return ctrl.Result{RequeueAfter: requeueStarting}, nil + return ctrl.Result{RequeueAfter: r.startingRequeue(server, startupTimeout(server))}, nil } if endpointAddress == "" { r.markStarting(server, "ServiceAddressPending", "waiting for the client Service ClusterIP") @@ -259,15 +282,24 @@ func (r *Reconciler) reconcileRunning(ctx context.Context, server *v1alpha1.Mine } pc, err := r.Prober.Probe(ctx, rconAddress(server), password) if err != nil { + // One missed probe of a Running server is noise (a lag spike, a save); + // it keeps its status and endpoint until the misses run consecutive. + if server.Status.Phase == v1alpha1.PhaseRunning && r.noteProbeFailure(server) < runningProbeMissesBeforeDegrade { + return ctrl.Result{RequeueAfter: requeueProbeRetry}, nil + } r.markStarting(server, "RconNotReachable", err.Error()) if r.readinessTimedOut(server) { r.markFailed(server, "ReadinessTimeout", "RCON probe did not succeed within readiness timeout") + if err := r.recoverFailedStart(ctx, server, readinessTimeout(server)); err != nil { + return ctrl.Result{}, err + } } if perr := r.patchStatus(ctx, server); perr != nil { return ctrl.Result{}, perr } - return ctrl.Result{RequeueAfter: requeueStarting}, nil + return ctrl.Result{RequeueAfter: r.startingRequeue(server, readinessTimeout(server))}, nil } + r.clearProbeFailures(server) players = pc // A tally that could not be read pauses idle auto-stop (below) instead of // counting as an empty server; the condition says so, so a server that @@ -332,7 +364,10 @@ func (r *Reconciler) reconcileRunning(ctx context.Context, server *v1alpha1.Mine } return ctrl.Result{RequeueAfter: requeueIdleProbe}, nil } - return ctrl.Result{}, nil + // Without idle auto-stop nothing else wakes a Running server either: the + // player tally and the RCON check would only refresh on a StatefulSet or CR + // event. A slow cadence keeps Status.Players and readiness current. + return ctrl.Result{RequeueAfter: requeueRunningProbe}, nil } // idleStopApplies reports whether idle auto-stop is configured for server. A @@ -644,6 +679,7 @@ func (r *Reconciler) markStarting(server *v1alpha1.MinecraftServer, reason, msg } func (r *Reconciler) markRunningReady(server *v1alpha1.MinecraftServer, players PlayerCount, endpointAddress string) { + server.Status.AutoRestarts = 0 server.Status.Phase = v1alpha1.PhaseRunning server.Status.Ready = true server.Status.ObservedGeneration = server.Generation @@ -689,6 +725,7 @@ func (r *Reconciler) markStopping(server *v1alpha1.MinecraftServer) { } func (r *Reconciler) markStopped(server *v1alpha1.MinecraftServer) { + server.Status.AutoRestarts = 0 server.Status.Phase = v1alpha1.PhaseStopped server.Status.Ready = false server.Status.ObservedGeneration = server.Generation @@ -727,20 +764,83 @@ func (r *Reconciler) startupTimedOut(server *v1alpha1.MinecraftServer) bool { if server.Status.StartRequestedAt == nil { return false } - timeout := time.Duration(server.Spec.Startup.TimeoutSeconds) * time.Second - if timeout <= 0 { - timeout = defaultTimeoutSeconds * time.Second + return r.now().Time.Sub(server.Status.StartRequestedAt.Time) >= startupTimeout(server) +} + +func startupTimeout(server *v1alpha1.MinecraftServer) time.Duration { + if t := time.Duration(server.Spec.Startup.TimeoutSeconds) * time.Second; t > 0 { + return t } - return r.now().Time.Sub(server.Status.StartRequestedAt.Time) >= timeout + return defaultTimeoutSeconds * time.Second } func (r *Reconciler) readinessTimedOut(server *v1alpha1.MinecraftServer) bool { if server.Status.StartRequestedAt == nil { return false } - timeout := time.Duration(server.Spec.Startup.ReadinessTimeoutSeconds) * time.Second - if timeout <= 0 { - timeout = defaultReadinessTimeoutSec * time.Second + return r.now().Time.Sub(server.Status.StartRequestedAt.Time) >= readinessTimeout(server) +} + +func readinessTimeout(server *v1alpha1.MinecraftServer) time.Duration { + if t := time.Duration(server.Spec.Startup.ReadinessTimeoutSeconds) * time.Second; t > 0 { + return t } - return r.now().Time.Sub(server.Status.StartRequestedAt.Time) >= timeout + return defaultReadinessTimeoutSec * time.Second +} + +// startingRequeue paces a start: every 2s while Starting; once Failed, only +// when the next auto-restart falls due, or every requeueFailed when the +// attempts are spent. +func (r *Reconciler) startingRequeue(server *v1alpha1.MinecraftServer, timeout time.Duration) time.Duration { + if server.Status.Phase != v1alpha1.PhaseFailed { + return requeueStarting + } + n := server.Status.AutoRestarts + if n >= maxAutoRestarts || server.Status.StartRequestedAt == nil { + return requeueFailed + } + wait := server.Status.StartRequestedAt.Add(timeout + autoRestartBaseBackoff<= maxAutoRestarts || server.Status.StartRequestedAt == nil { + return nil + } + due := server.Status.StartRequestedAt.Add(timeout + autoRestartBaseBackoff< Running - if res.RequeueAfter != 0 { - t.Errorf("expected no requeue once Running, got %+v", res) + // Running re-probes on a slow cadence to keep players and readiness current. + if res.RequeueAfter != time.Minute { + t.Errorf("expected the 1m Running re-probe, got %+v", res) } server := getServer(t, c, "survival") @@ -609,8 +610,8 @@ func TestIdleAutoStop_SystemServerNeverIdles(t *testing.T) { t.Fatalf("system server: desiredState=%s emptySince=%v, want Running and no countdown", server.Spec.DesiredState, server.Status.EmptySince) } - if res.RequeueAfter != 0 { - t.Fatalf("RequeueAfter = %v, want none for a server that never idles", res.RequeueAfter) + if res.RequeueAfter != time.Minute { + t.Fatalf("RequeueAfter = %v, want only the 1m Running re-probe for a server that never idles", res.RequeueAfter) } } @@ -669,8 +670,8 @@ func TestNoIdleRequeueWhenDisabled(t *testing.T) { markPodReady(t, c, "survival") res := reconcile(t, r, "survival") - if res.RequeueAfter != 0 { - t.Fatalf("RequeueAfter = %v, want 0 when idle auto-stop is disabled", res.RequeueAfter) + if res.RequeueAfter != time.Minute { + t.Fatalf("RequeueAfter = %v, want only the 1m Running re-probe when idle auto-stop is disabled", res.RequeueAfter) } } diff --git a/internal/platform/rbac.go b/internal/platform/rbac.go index bc9bb8d..4ac3573 100644 --- a/internal/platform/rbac.go +++ b/internal/platform/rbac.go @@ -153,8 +153,10 @@ func APIBuildRole(p Params) *rbacv1.Role { // call fails closed with a 403), and reads RCON Secrets. Jobs are list-only, // through the manager's uncached API reader: before scaling a server up from zero // the operator checks that no restore/backup/file-write Job holds its world -// (internal/maintenance). It never touches pods, PVCs, Events, or finalizers, so -// none appear here. +// (internal/maintenance). Pods are delete-only: a start that timed out is retried +// by deleting its pod for the StatefulSet to recreate (bounded, three attempts; +// internal/operator.recoverFailedStart). It never touches PVCs, Events, or +// finalizers, so none appear here. func OperatorRole(p Params) *rbacv1.Role { p = p.withDefaults() return role(p.MinecraftNamespace, "felis-operator", ComponentOperator, []rbacv1.PolicyRule{ @@ -172,6 +174,9 @@ func OperatorRole(p Params) *rbacv1.Role { // list only: an uncached List (no informer, so no watch) of the world-volume // maintenance Jobs; the operator never creates or deletes a Job. rule([]string{groupBatch}, []string{"jobs"}, []string{"list"}), + // delete only, through the direct client: no read of pods is needed to + // remove the one named -0. + rule([]string{groupCore}, []string{"pods"}, []string{"delete"}), }) } diff --git a/internal/platform/rbac_test.go b/internal/platform/rbac_test.go index 4a0c6c4..189baf6 100644 --- a/internal/platform/rbac_test.go +++ b/internal/platform/rbac_test.go @@ -189,11 +189,20 @@ func TestOperatorRole_ScopeExact(t *testing.T) { } } // Hard exclusions. - for _, res := range []string{"persistentvolumeclaims", "pods", "events"} { + for _, res := range []string{"persistentvolumeclaims", "events"} { if grantsResource(op, groupCore, res) { t.Errorf("operator must NOT touch core/%s", res) } } + // Pods are delete-only: the bounded retry of a timed-out start. + if !hasRule(op, groupCore, "pods", "delete") { + t.Error("operator must have pods:delete (auto-restart of a timed-out start)") + } + for _, v := range []string{"get", "list", "watch", "create", "update", "patch", "deletecollection", "*"} { + if hasRule(op, groupCore, "pods", v) { + t.Errorf("operator pods rule must be delete-only, found %s", v) + } + } // The world-volume lock check lists Jobs uncached; it never writes one. if !hasRule(op, groupBatch, "jobs", "list") { t.Error("operator must have jobs:list (maintenance hold before scale-up)")