fix(operator): 删除依赖 rcon-cli 的 preStop 死代码，缩容前由 operator 经 RCON 执行 save-all flush

internal/operator/builders.go

@@ -108,18 +108,6 @@ func rconAddress(server *v1alpha1.MinecraftServer) string {
 	return fmt.Sprintf("%s.%s.svc.cluster.local:%d", server.Name, server.Namespace, rconPort(server))
 }
 
-// preStopScript is the operator-injected graceful-shutdown sequence (spec §7):
-// flush the world, then stop the server, both over RCON. It relies on rcon-cli
-// being present in the Felis base image and reading the RCON_* env injected
-// alongside it.
-func preStopScript(server *v1alpha1.MinecraftServer) string {
-	port := rconPort(server)
-	return fmt.Sprintf(
-		`rcon-cli --port %d --password "$RCON_PASSWORD" save-all flush; rcon-cli --port %d --password "$RCON_PASSWORD" stop`,
-		port, port,
-	)
-}
-
 // buildHeadlessService backs the StatefulSet's stable network identity.
 func buildHeadlessService(server *v1alpha1.MinecraftServer) *corev1.Service {
 	svc := &corev1.Service{
@@ -198,9 +186,10 @@ func readinessProbe(server *v1alpha1.MinecraftServer) *corev1.Probe {
 	return probe
 }
 
-// buildStatefulSet renders the workload for replicas in {0,1}. It is where
-// graceful shutdown is injected: the pod gets terminationGracePeriodSeconds and
-// (when enabled) a preStop RCON save+stop hook.
+// buildStatefulSet renders the workload for replicas in {0,1}. Its half of
+// graceful shutdown is terminationGracePeriodSeconds, the time the server gets to
+// save on SIGTERM; the reconciler flushes the world over RCON before it scales to
+// zero (saveBeforeStop).
 func buildStatefulSet(server *v1alpha1.MinecraftServer, replicas int32, felisImage string) (*appsv1.StatefulSet, error) {
 	storageSize := server.Spec.Storage.Size
 	if storageSize == "" {
@@ -245,15 +234,6 @@ func buildStatefulSet(server *v1alpha1.MinecraftServer, replicas int32, felisIma
 		container.Ports = append(container.Ports, corev1.ContainerPort{
 			Name: "rcon", ContainerPort: rconPort(server), Protocol: corev1.ProtocolTCP,
 		})
-		if server.Spec.Lifecycle.PreStopSaveAndStop {
-			container.Lifecycle = &corev1.Lifecycle{
-				PreStop: &corev1.LifecycleHandler{
-					Exec: &corev1.ExecAction{
-						Command: []string{"/bin/sh", "-c", preStopScript(server)},
-					},
-				},
-			}
-		}
 	}
 
 	// Every server first hands its world volume to the game uid (prepareDataInitContainer),

internal/operator/prober.go

@@ -20,21 +20,52 @@ type PlayerCount struct {
 	Known  bool
 }
 
-// Prober reports whether a server's RCON endpoint is reachable and accepts the
-// password, and best-effort returns its current player tally. A nil error is the
-// loader-agnostic readiness gate (spec §5); the PlayerCount is advisory and has
-// Known=false (with a nil error) whenever the tally could not be sampled. It is an
-// interface so the reconciler can be tested without a live server.
+// Prober is the operator's RCON channel into a running server. It is an interface
+// so the reconciler can be tested without a live server.
+//
+// Probe reports whether the endpoint is reachable and accepts the password, and
+// best-effort returns the current player tally. A nil error is the loader-agnostic
+// readiness gate (spec §5); the PlayerCount is advisory and has Known=false (with
+// a nil error) whenever the tally could not be sampled.
+//
+// Save flushes the world to disk (`save-all flush`) and returns once the server
+// has answered, i.e. once the save is done. The reconciler runs it right before
+// scaling a server to zero (spec §7).
 type Prober interface {
 	Probe(ctx context.Context, addr, password string) (PlayerCount, error)
+	Save(ctx context.Context, addr, password string) error
 }
 
 // RconProber is the production Prober: a successful Dial (TCP connect + auth)
 // is the readiness gate; on that same connection it then runs `list` to sample
 // the player tally before closing.
 type RconProber struct {
-	// Timeout bounds a single probe. Defaults to 5s.
+	// Timeout bounds a single probe, and the connect+auth step of a save.
+	// Defaults to 5s.
 	Timeout time.Duration
+	// SaveTimeout bounds the wait for `save-all flush` to answer. Defaults to
+	// defaultSaveTimeout.
+	SaveTimeout time.Duration
+}
+
+// defaultSaveTimeout is how long a stop waits for the pre-stop save. The server
+// answers `save-all flush` only after every loaded chunk is written, which takes
+// seconds on a large world. Past this the stop goes ahead anyway: SIGTERM runs the
+// server's own shutdown save within the pod's grace period, so the explicit save
+// only moves most of that work ahead of the kill deadline.
+const defaultSaveTimeout = 30 * time.Second
+
+// boundTimeout returns d (or def when d is unset), shortened to ctx's deadline.
+func boundTimeout(ctx context.Context, d, def time.Duration) time.Duration {
+	if d <= 0 {
+		d = def
+	}
+	if dl, ok := ctx.Deadline(); ok {
+		if remaining := time.Until(dl); remaining > 0 && remaining < d {
+			d = remaining
+		}
+	}
+	return d
 }
 
 // Probe dials addr and authenticates with password, honoring the smaller of the
@@ -43,15 +74,7 @@ type RconProber struct {
 // a failed or unparseable `list` yields an unknown PlayerCount, never a probe error,
 // so a transient count-read hiccup can never flap a healthy server out of Ready.
 func (p RconProber) Probe(ctx context.Context, addr, password string) (PlayerCount, error) {
-	timeout := p.Timeout
-	if timeout <= 0 {
-		timeout = 5 * time.Second
-	}
-	if dl, ok := ctx.Deadline(); ok {
-		if remaining := time.Until(dl); remaining > 0 && remaining < timeout {
-			timeout = remaining
-		}
-	}
+	timeout := boundTimeout(ctx, p.Timeout, 5*time.Second)
 	conn, err := rcon.Dial(addr, password, timeout)
 	if err != nil {
 		return PlayerCount{}, err
@@ -71,6 +94,22 @@ func (p RconProber) Probe(ctx context.Context, addr, password string) (PlayerCou
 	return pc, nil
 }
 
+// Save runs `save-all flush` and waits for its reply. The reply text is not
+// checked: vanilla, Paper and the modded loaders word it differently, and any
+// reply at all means the command ran to completion on the server thread.
+func (p RconProber) Save(ctx context.Context, addr, password string) error {
+	conn, err := rcon.Dial(addr, password, boundTimeout(ctx, p.Timeout, 5*time.Second))
+	if err != nil {
+		return err
+	}
+	defer conn.Close()
+	if err := conn.SetDeadline(time.Now().Add(boundTimeout(ctx, p.SaveTimeout, defaultSaveTimeout))); err != nil {
+		return err
+	}
+	_, err = conn.Execute("save-all flush")
+	return err
+}
+
 // listReplyPatterns match the `list` replies of the loaders Felis runs, tried in
 // order against the reply with § color codes stripped:
 //

internal/operator/prober_internal_test.go

@@ -1,6 +1,13 @@
 package operator
 
-import "testing"
+import (
+	"context"
+	"encoding/binary"
+	"io"
+	"net"
+	"testing"
+	"time"
+)
 
 func TestParseListReply(t *testing.T) {
 	cases := []struct {
@@ -82,3 +89,73 @@ func TestParseListReply(t *testing.T) {
 		})
 	}
 }
+
+// serveFakeRcon accepts one RCON connection, accepts any password, and answers
+// each command after delay. Every command body is sent on the returned channel.
+func serveFakeRcon(t *testing.T, delay time.Duration) (string, <-chan string) {
+	t.Helper()
+	ln, err := net.Listen("tcp", "127.0.0.1:0")
+	if err != nil {
+		t.Fatalf("listen: %v", err)
+	}
+	t.Cleanup(func() { ln.Close() })
+	cmds := make(chan string, 4)
+	go func() {
+		conn, err := ln.Accept()
+		if err != nil {
+			return
+		}
+		defer conn.Close()
+		for {
+			var hdr [12]byte
+			if _, err := io.ReadFull(conn, hdr[:]); err != nil {
+				return
+			}
+			size := int32(binary.LittleEndian.Uint32(hdr[0:]))
+			id := int32(binary.LittleEndian.Uint32(hdr[4:]))
+			typ := int32(binary.LittleEndian.Uint32(hdr[8:]))
+			rest := make([]byte, size-8)
+			if _, err := io.ReadFull(conn, rest); err != nil {
+				return
+			}
+			reply := ""
+			if typ == 3 { // auth: answer with an auth response carrying the same id
+				typ = 2
+			} else {
+				cmds <- string(rest[:len(rest)-2])
+				time.Sleep(delay)
+				typ, reply = 0, "Saved the game"
+			}
+			out := binary.LittleEndian.AppendUint32(nil, uint32(4+4+len(reply)+2))
+			out = binary.LittleEndian.AppendUint32(out, uint32(id))
+			out = binary.LittleEndian.AppendUint32(out, uint32(typ))
+			out = append(append(out, reply...), 0, 0)
+			if _, err := conn.Write(out); err != nil {
+				return
+			}
+		}
+	}()
+	return ln.Addr().String(), cmds
+}
+
+func TestRconProberSaveFlushes(t *testing.T) {
+	addr, cmds := serveFakeRcon(t, 0)
+	if err := (RconProber{}).Save(context.Background(), addr, "pw"); err != nil {
+		t.Fatalf("Save: %v", err)
+	}
+	if got := <-cmds; got != "save-all flush" {
+		t.Errorf("command = %q, want save-all flush", got)
+	}
+}
+
+func TestRconProberSaveTimesOut(t *testing.T) {
+	addr, _ := serveFakeRcon(t, time.Second)
+	start := time.Now()
+	err := (RconProber{SaveTimeout: 100 * time.Millisecond}).Save(context.Background(), addr, "pw")
+	if err == nil {
+		t.Fatal("Save returned nil for a reply slower than SaveTimeout")
+	}
+	if elapsed := time.Since(start); elapsed > 900*time.Millisecond {
+		t.Errorf("Save took %v, want it bounded by SaveTimeout", elapsed)
+	}
+}

internal/operator/reconciler.go

@@ -1,7 +1,8 @@
 // Package operator reconciles MinecraftServer objects (spec §4, §5, §7). The
 // CRD is the lifecycle source-of-truth; this controller renders the
-// StatefulSet/Service/PVC from it, gates readiness on an RCON probe, and injects
-// graceful shutdown. It never reads or writes business-layer (Postgres) fields.
+// StatefulSet/Service/PVC from it, gates readiness on an RCON probe, and flushes
+// the world over RCON before scaling a server down. It never reads or writes
+// business-layer (Postgres) fields.
 package operator
 
 import (
@@ -25,6 +26,7 @@ import (
 	"k8s.io/apimachinery/pkg/types"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/controller-runtime/pkg/controller"
 	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
 )
 
@@ -96,9 +98,17 @@ func (r *Reconciler) SetupWithManager(mgr ctrl.Manager) error {
 		// and a watch is what lets a deleted Secret be noticed at all (a quiet
 		// Running server otherwise produces no events).
 		Owns(&corev1.Secret{}).
+		WithOptions(controller.Options{MaxConcurrentReconciles: maxConcurrentReconciles}).
 		Complete(r)
 }
 
+// maxConcurrentReconciles lets that many servers reconcile at once. A reconcile
+// blocks on RCON (up to 5s for a probe, and up to defaultSaveTimeout for the
+// save ahead of a stop), so with controller-runtime's default of one, a single
+// large world saving would stall every other server's start, stop and readiness.
+// The same server is never reconciled twice at once regardless.
+const maxConcurrentReconciles = 4
+
 // Reconcile drives a single MinecraftServer toward spec.desiredState.
 func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
 	var server v1alpha1.MinecraftServer
@@ -302,8 +312,11 @@ func (r *Reconciler) reconcileStopped(ctx context.Context, server *v1alpha1.Mine
 		return ctrl.Result{}, err
 	}
 
-	// Scaling to zero triggers each pod's preStop RCON save+stop (spec §7).
+	// Graceful shutdown (spec §7): flush the world over RCON, then scale to zero,
+	// which sends the server SIGTERM and so its own shutdown save within the
+	// grace period.
 	if sts.Spec.Replicas == nil || *sts.Spec.Replicas != 0 {
+		r.saveBeforeStop(ctx, server, &sts)
 		zero := int32(0)
 		sts.Spec.Replicas = &zero
 		if err := r.Update(ctx, &sts); err != nil {
@@ -323,6 +336,37 @@ func (r *Reconciler) reconcileStopped(ctx context.Context, server *v1alpha1.Mine
 	return ctrl.Result{}, r.patchStatus(ctx, server)
 }
 
+// saveBeforeStop runs `save-all flush` on a server that is about to be scaled to
+// zero. The SIGTERM that follows makes the server save again on its way out, but
+// that save races the grace period: a large world killed mid-save rolls back to
+// whatever was last flushed. Flushing first leaves the shutdown save with almost
+// nothing to write.
+//
+// It is best-effort and never holds up the stop. A server without RCON, or with no
+// ready pod (still booting, or already terminating), has nothing to flush it with,
+// and a failed save still leaves the shutdown save; holding a stop the user asked
+// for over it would only keep the server up. A conflict on the Update that follows
+// re-runs it on the next reconcile, which is harmless: a second flush right after
+// the first writes nothing.
+func (r *Reconciler) saveBeforeStop(ctx context.Context, server *v1alpha1.MinecraftServer, sts *appsv1.StatefulSet) {
+	if !server.Spec.Rcon.Enabled || sts.Status.ReadyReplicas == 0 {
+		return
+	}
+	logger := ctrl.LoggerFrom(ctx)
+	password, err := r.rconPassword(ctx, server)
+	if err != nil {
+		logger.Info("skipping pre-stop world save: RCON password unavailable", "error", err.Error())
+		return
+	}
+	start := time.Now()
+	if err := r.Prober.Save(ctx, rconAddress(server), password); err != nil {
+		logger.Info("pre-stop world save failed; stopping anyway, the server saves again on SIGTERM",
+			"error", err.Error(), "elapsed", time.Since(start).Round(time.Millisecond).String())
+		return
+	}
+	logger.Info("pre-stop world save done", "elapsed", time.Since(start).Round(time.Millisecond).String())
+}
+
 // maintenanceHold reports whether a restore, backup or file write holds the
 // server's world volume (internal/maintenance) while its pod is about to be
 // created. felis-api already refuses a wake in that state; this is the same rule

internal/operator/reconciler_test.go

@@ -3,6 +3,7 @@ package operator_test
 import (
 	"context"
 	"errors"
+	"slices"
 	"strings"
 	"testing"
 	"time"
@@ -27,12 +28,22 @@ import (
 type fakeProber struct {
 	err     error
 	players operator.PlayerCount
+	saveErr error
+	// saves, when set, records each Save's address.
+	saves *[]string
 }
 
 func (f fakeProber) Probe(context.Context, string, string) (operator.PlayerCount, error) {
 	return f.players, f.err
 }
 
+func (f fakeProber) Save(_ context.Context, addr, _ string) error {
+	if f.saves != nil {
+		*f.saves = append(*f.saves, addr)
+	}
+	return f.saveErr
+}
+
 func newScheme(t *testing.T) *runtime.Scheme {
 	t.Helper()
 	scheme := runtime.NewScheme()
@@ -61,7 +72,6 @@ func runningServer() *v1alpha1.MinecraftServer {
 			Storage:        v1alpha1.StorageSpec{Size: "10Gi"},
 			FallbackServer: "lobby",
 			Motd:           v1alpha1.MotdSpec{Running: "up", Stopped: "down", Starting: "booting"},
-			Lifecycle:      v1alpha1.LifecycleSpec{PreStopSaveAndStop: true},
 			Rcon: v1alpha1.RconSpec{
 				Enabled:   true,
 				Port:      25575,
@@ -171,18 +181,16 @@ func TestReconcileRunning_CreatesWorkloadAndInjectsGracefulShutdown(t *testing.T
 		t.Errorf("headless service ClusterIP = %q, want None", hl.Spec.ClusterIP)
 	}
 
-	// StatefulSet exists with graceful-shutdown injection.
+	// StatefulSet exists with the shutdown grace period. The pre-stop save runs
+	// from the operator over RCON, so the pod carries no preStop hook (none of the
+	// game images ship an RCON client to run one with).
 	sts := getSTS(t, c, "survival")
 	if got := sts.Spec.Template.Spec.TerminationGracePeriodSeconds; got == nil || *got != 300 {
 		t.Errorf("terminationGracePeriodSeconds = %v, want 300", got)
 	}
 	container := sts.Spec.Template.Spec.Containers[0]
-	if container.Lifecycle == nil || container.Lifecycle.PreStop == nil || container.Lifecycle.PreStop.Exec == nil {
-		t.Fatal("expected preStop exec hook to be injected")
-	}
-	preStop := strings.Join(container.Lifecycle.PreStop.Exec.Command, " ")
-	if !strings.Contains(preStop, "save-all flush") || !strings.Contains(preStop, "stop") {
-		t.Errorf("preStop hook missing save/stop sequence: %q", preStop)
+	if container.Lifecycle != nil {
+		t.Errorf("container lifecycle = %+v, want none", container.Lifecycle)
 	}
 	// RCON password is sourced from the Secret, never inlined.
 	var sawRconPassword bool
@@ -327,6 +335,93 @@ func TestReconcileStopped_ScalesRunningWorkloadDown(t *testing.T) {
 	}
 }
 
+// stopRunningServer takes survival to Running with a ready pod, then flips it to
+// Stopped and reconciles once.
+func stopRunningServer(t *testing.T, r *operator.Reconciler, c client.Client) {
+	t.Helper()
+	reconcile(t, r, "survival")
+	markPodReady(t, c, "survival")
+	reconcile(t, r, "survival")
+	server := getServer(t, c, "survival")
+	server.Spec.DesiredState = v1alpha1.DesiredStopped
+	if err := c.Update(context.Background(), server); err != nil {
+		t.Fatalf("flip desiredState: %v", err)
+	}
+	reconcile(t, r, "survival")
+}
+
+func TestReconcileStopped_SavesBeforeScalingDown(t *testing.T) {
+	var saves []string
+	r, c := newReconciler(t, fakeProber{saves: &saves}, runningServer(), rconSecret())
+
+	stopRunningServer(t, r, c)
+
+	if want := []string{"survival.minecraft.svc.cluster.local:25575"}; !slices.Equal(saves, want) {
+		t.Errorf("saves = %v, want %v", saves, want)
+	}
+	if sts := getSTS(t, c, "survival"); sts.Spec.Replicas == nil || *sts.Spec.Replicas != 0 {
+		t.Errorf("replicas = %v, want 0 after stop", sts.Spec.Replicas)
+	}
+
+	// Once scaled to zero the next reconcile only waits for the pod to go; it
+	// does not flush again.
+	reconcile(t, r, "survival")
+	if len(saves) != 1 {
+		t.Errorf("saves after second reconcile = %d, want 1", len(saves))
+	}
+}
+
+func TestReconcileStopped_FailedSaveStillStops(t *testing.T) {
+	var saves []string
+	prober := fakeProber{saves: &saves, saveErr: errors.New("i/o timeout")}
+	r, c := newReconciler(t, prober, runningServer(), rconSecret())
+
+	stopRunningServer(t, r, c)
+
+	if len(saves) != 1 {
+		t.Fatalf("saves = %d, want 1 attempt", len(saves))
+	}
+	if sts := getSTS(t, c, "survival"); sts.Spec.Replicas == nil || *sts.Spec.Replicas != 0 {
+		t.Errorf("replicas = %v, want 0: a failed save must not hold the stop", sts.Spec.Replicas)
+	}
+	if server := getServer(t, c, "survival"); server.Status.Phase != v1alpha1.PhaseStopping {
+		t.Errorf("phase = %s, want Stopping", server.Status.Phase)
+	}
+}
+
+func TestReconcileStopped_SkipsSaveWithoutReadyPodOrRcon(t *testing.T) {
+	t.Run("pod not ready", func(t *testing.T) {
+		var saves []string
+		r, c := newReconciler(t, fakeProber{saves: &saves}, runningServer(), rconSecret())
+		reconcile(t, r, "survival") // StatefulSet at replicas=1, pod never ready
+		server := getServer(t, c, "survival")
+		server.Spec.DesiredState = v1alpha1.DesiredStopped
+		if err := c.Update(context.Background(), server); err != nil {
+			t.Fatalf("flip desiredState: %v", err)
+		}
+		reconcile(t, r, "survival")
+		if len(saves) != 0 {
+			t.Errorf("saves = %v, want none for a pod that never became ready", saves)
+		}
+		if sts := getSTS(t, c, "survival"); sts.Spec.Replicas == nil || *sts.Spec.Replicas != 0 {
+			t.Errorf("replicas = %v, want 0", sts.Spec.Replicas)
+		}
+	})
+	t.Run("rcon disabled", func(t *testing.T) {
+		var saves []string
+		s := runningServer()
+		s.Spec.Rcon = v1alpha1.RconSpec{}
+		r, c := newReconciler(t, fakeProber{saves: &saves}, s)
+		stopRunningServer(t, r, c)
+		if len(saves) != 0 {
+			t.Errorf("saves = %v, want none without RCON", saves)
+		}
+		if sts := getSTS(t, c, "survival"); sts.Spec.Replicas == nil || *sts.Spec.Replicas != 0 {
+			t.Errorf("replicas = %v, want 0", sts.Spec.Replicas)
+		}
+	})
+}
+
 // --- idle auto-stop tests (spec §8) ---------------------------------------
 
 // TestIdleAutoStop_EmptyServerGetsTimestamp verifies that the first Running