From 98739044a56138d8dc7b28023dd857de6c59710b Mon Sep 17 00:00:00 2001 From: Minseong Choi Date: Tue, 30 Jun 2026 20:04:46 +0900 Subject: [PATCH] fix(operator): populate Status.Players from an RCON list probe A Running, ready server always reported 0/0 players: markRunningReady never wrote Status.Players, and markStopped only cleared it. The panel therefore showed an empty tally for live servers. Extend the readiness probe to also sample the player count. Prober.Probe now returns a PlayerCount{Online, Max}: RconProber still gates readiness on Dial+auth, then runs a best-effort `list` and parses the vanilla reply ("There are N of a max of M players online"). A failed or unparseable tally is swallowed (0/0) so it never blocks readiness. The reconciler threads the count into markRunningReady, which writes Status.Players; markStopped still resets it to zero. --- internal/operator/prober.go | 68 ++++++++++++++++++++--- internal/operator/prober_internal_test.go | 54 ++++++++++++++++++ internal/operator/reconciler.go | 15 +++-- internal/operator/reconciler_test.go | 26 ++++++++- 4 files changed, 149 insertions(+), 14 deletions(-) create mode 100644 internal/operator/prober_internal_test.go diff --git a/internal/operator/prober.go b/internal/operator/prober.go index 9240dcd..ea9d552 100644 --- a/internal/operator/prober.go +++ b/internal/operator/prober.go @@ -2,28 +2,44 @@ package operator import ( "context" + "regexp" + "strconv" "time" "felis.lolicon.best/internal/rcon" ) +// PlayerCount is a server's online/max player tally as read from RCON `list`. +// Both fields are zero when the count could not be read; that is not an error, +// only the absence of a fresh sample (see Prober). +type PlayerCount struct { + Online int32 + Max int32 +} + // Prober reports whether a server's RCON endpoint is reachable and accepts the -// password. A nil error is the loader-agnostic readiness gate (spec §5). It is -// an interface so the reconciler can be tested without a live server. +// 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 is +// zero (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. type Prober interface { - Probe(ctx context.Context, addr, password string) error + Probe(ctx context.Context, addr, password string) (PlayerCount, error) } // RconProber is the production Prober: a successful Dial (TCP connect + auth) -// is sufficient; the connection is closed immediately. +// 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 time.Duration } // Probe dials addr and authenticates with password, honoring the smaller of the -// configured timeout and any deadline already on ctx. -func (p RconProber) Probe(ctx context.Context, addr, password string) error { +// configured timeout and any deadline already on ctx. Auth success gates +// readiness; the player tally is then read with `list` on a best-effort basis — +// a failed or unparseable `list` yields a zero 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 @@ -35,7 +51,43 @@ func (p RconProber) Probe(ctx context.Context, addr, password string) error { } conn, err := rcon.Dial(addr, password, timeout) if err != nil { - return err + return PlayerCount{}, err } - return conn.Close() + defer conn.Close() + + // Readiness is already established by the successful Dial+auth above. Reading + // the tally must not jeopardize that verdict, so its errors are swallowed. + if err := conn.SetDeadline(time.Now().Add(timeout)); err != nil { + return PlayerCount{}, nil + } + reply, err := conn.Execute("list") + if err != nil { + return PlayerCount{}, nil + } + pc, _ := parseListReply(reply) + return pc, nil +} + +// listReplyPattern matches the vanilla/Paper `list` response, e.g. +// "There are 3 of a max of 20 players online: alice, bob, carol". The search is +// unanchored so leading color codes or trailing player names do not defeat it. +var listReplyPattern = regexp.MustCompile(`There are (\d+) of a max of (\d+) players online`) + +// parseListReply extracts the online/max tally from a `list` reply. ok is false +// (and the PlayerCount zero) when the reply does not match the known format, so +// callers can distinguish "no sample" from a genuine "0 of N". +func parseListReply(reply string) (PlayerCount, bool) { + m := listReplyPattern.FindStringSubmatch(reply) + if m == nil { + return PlayerCount{}, false + } + online, err := strconv.ParseInt(m[1], 10, 32) + if err != nil { + return PlayerCount{}, false + } + max, err := strconv.ParseInt(m[2], 10, 32) + if err != nil { + return PlayerCount{}, false + } + return PlayerCount{Online: int32(online), Max: int32(max)}, true } diff --git a/internal/operator/prober_internal_test.go b/internal/operator/prober_internal_test.go new file mode 100644 index 0000000..5bace20 --- /dev/null +++ b/internal/operator/prober_internal_test.go @@ -0,0 +1,54 @@ +package operator + +import "testing" + +func TestParseListReply(t *testing.T) { + cases := []struct { + name string + reply string + want PlayerCount + wantOK bool + }{ + { + name: "empty server", + reply: "There are 0 of a max of 20 players online:", + want: PlayerCount{Online: 0, Max: 20}, + wantOK: true, + }, + { + name: "with player names", + reply: "There are 3 of a max of 20 players online: alice, bob, carol", + want: PlayerCount{Online: 3, Max: 20}, + wantOK: true, + }, + { + name: "full server", + reply: "There are 20 of a max of 20 players online: ...", + want: PlayerCount{Online: 20, Max: 20}, + wantOK: true, + }, + { + name: "unrecognized reply yields no sample", + reply: "Unknown command. Try /help for a list of commands.", + want: PlayerCount{}, + wantOK: false, + }, + { + name: "empty reply yields no sample", + reply: "", + want: PlayerCount{}, + wantOK: false, + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + got, ok := parseListReply(tc.reply) + if ok != tc.wantOK { + t.Fatalf("ok = %v, want %v", ok, tc.wantOK) + } + if got != tc.want { + t.Errorf("count = %+v, want %+v", got, tc.want) + } + }) + } +} diff --git a/internal/operator/reconciler.go b/internal/operator/reconciler.go index 2ec3647..25ed4b6 100644 --- a/internal/operator/reconciler.go +++ b/internal/operator/reconciler.go @@ -108,7 +108,9 @@ func (r *Reconciler) reconcileRunning(ctx context.Context, server *v1alpha1.Mine return ctrl.Result{RequeueAfter: requeueStarting}, nil } - // Then the operator gates true readiness on an RCON probe (spec §5). + // Then the operator gates true readiness on an RCON probe (spec §5), which + // also samples the current player tally for Status.Players. + var players PlayerCount if server.Spec.Rcon.Enabled { password, err := r.rconPassword(ctx, server) if err != nil { @@ -118,16 +120,18 @@ func (r *Reconciler) reconcileRunning(ctx context.Context, server *v1alpha1.Mine } return ctrl.Result{RequeueAfter: requeueSecret}, nil } - if err := r.Prober.Probe(ctx, rconAddress(server), password); err != nil { + pc, err := r.Prober.Probe(ctx, rconAddress(server), password) + if err != nil { r.markStarting(server, "RconNotReachable", err.Error()) if perr := r.patchStatus(ctx, server); perr != nil { return ctrl.Result{}, perr } return ctrl.Result{RequeueAfter: requeueStarting}, nil } + players = pc } - r.markRunningReady(server) + r.markRunningReady(server, players) return ctrl.Result{}, r.patchStatus(ctx, server) } @@ -252,10 +256,13 @@ func (r *Reconciler) markStarting(server *v1alpha1.MinecraftServer, reason, msg r.setCondition(server, v1alpha1.ConditionRconReached, metav1.ConditionFalse, reason, msg) } -func (r *Reconciler) markRunningReady(server *v1alpha1.MinecraftServer) { +func (r *Reconciler) markRunningReady(server *v1alpha1.MinecraftServer, players PlayerCount) { server.Status.Phase = v1alpha1.PhaseRunning server.Status.Ready = true server.Status.ObservedGeneration = server.Generation + // Refresh the player tally sampled by this reconcile's RCON probe so the panel + // reports live occupancy instead of the 0/0 markStopped leaves behind. + server.Status.Players = v1alpha1.PlayersStatus{Online: players.Online, Max: players.Max} if server.Status.ReadySignalAt == nil { t := r.now() server.Status.ReadySignalAt = &t diff --git a/internal/operator/reconciler_test.go b/internal/operator/reconciler_test.go index 6ec1260..07e6e94 100644 --- a/internal/operator/reconciler_test.go +++ b/internal/operator/reconciler_test.go @@ -23,9 +23,14 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client/fake" ) -type fakeProber struct{ err error } +type fakeProber struct { + err error + players operator.PlayerCount +} -func (f fakeProber) Probe(context.Context, string, string) error { return f.err } +func (f fakeProber) Probe(context.Context, string, string) (operator.PlayerCount, error) { + return f.players, f.err +} func newScheme(t *testing.T) *runtime.Scheme { t.Helper() @@ -236,6 +241,23 @@ func TestReconcileRunning_RconProbeGatesReadiness(t *testing.T) { } } +func TestReconcileRunning_PopulatesPlayerTally(t *testing.T) { + r, c := newReconciler(t, fakeProber{players: operator.PlayerCount{Online: 3, Max: 20}}, runningServer(), rconSecret()) + + reconcile(t, r, "survival") // creates workload, Starting + markPodReady(t, c, "survival") + reconcile(t, r, "survival") // pod ready + probe OK -> Running + + server := getServer(t, c, "survival") + if server.Status.Phase != v1alpha1.PhaseRunning { + t.Fatalf("phase = %s, want Running", server.Status.Phase) + } + // The probe's tally must land on Status.Players so the panel stops reporting 0/0. + if server.Status.Players.Online != 3 || server.Status.Players.Max != 20 { + t.Errorf("status players = %d/%d, want 3/20", server.Status.Players.Online, server.Status.Players.Max) + } +} + func TestReconcileRunning_RconProbeFailureStaysStarting(t *testing.T) { r, c := newReconciler(t, fakeProber{err: errors.New("connection refused")}, runningServer(), rconSecret())