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())