fix(operator): populate Status.Players from an RCON list probe (98739044) · Commits · FelisMC / Felis · GitLab

internal/operator/prober.go

+60 −8

Changes for internal/operator/prober.go: 60 added lines, 8 removed lines.
Original line number	Diff line number	Diff line
		@@ -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
		}
		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 conn.Close()
		return PlayerCount{Online: int32(online), Max: int32(max)}, true
		}

internal/operator/prober_internal_test.go

0 → 100644

+54 −0

Changes for internal/operator/prober_internal_test.go: 54 added lines, 0 removed lines.
Original line number	Diff line number	Diff line
		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)
		}
		})
		}
		}

internal/operator/reconciler.go

+11 −4

Changes for internal/operator/reconciler.go: 11 added lines, 4 removed lines.
Original line number	Diff line number	Diff line
		@@ -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

internal/operator/reconciler_test.go

+24 −2

Changes for internal/operator/reconciler_test.go: 24 added lines, 2 removed lines.
Original line number	Diff line number	Diff line
		@@ -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())