Unverified Commit bd909d58 authored by Lemon-miaow's avatar Lemon-miaow
Browse files

fix(operator): 读不出玩家数时暂停空闲自动停服,支持 1.12/Bukkit/EssentialsX 与颜色码

parent 857c73a6
Loading
Loading
Loading
Loading
+28 −15
Changes for docs/troubleshooting.md: 28 added lines, 15 removed lines.
Original line number Diff line number Diff line
@@ -716,24 +716,37 @@ Check it first.
kubectl get minecraftserver <name> -o jsonpath='{.spec.rcon.enabled}'
```

The player tally is a by-product of the RCON readiness probe — `prober.go:63`
runs `list` on the same connection that just authenticated, and `parseListReply`
extracts the tally from `There are (\d+) of a max of (\d+) players online`. With
RCON disabled the probe never runs, `players` keeps its zero value, and
`markRunningReady` (`reconciler.go:413`) writes that zero into
`status.players.online`. So a permanent 0 means "never sampled", not "nobody
online".

Idle auto-stop (`reconciler.go:175`) reads that same tally, which is why it
carries the RCON condition explicitly:
The player tally is a by-product of the RCON readiness probe: `prober.go` runs
`list` on the same connection that just authenticated, and `parseListReply`
reads the tally from the vanilla/Paper/Fabric/Forge reply (`There are 3 of a
max of 20 players online`), the 1.12/Bukkit reply (`There are 3/20 players
online`) or the EssentialsX reply (`There are 3 out of maximum 20 players
online`, vanished players included), with `§` color codes stripped. With RCON
disabled the probe never runs and no tally is ever read, so a permanent 0 means
"never sampled", not "nobody online".

A tally that could not be read counts as **unknown**, never as zero. Idle
auto-stop only acts on a count it actually read:

```go
if server.Spec.Rcon.Enabled && server.Spec.Idle.AutoStopEnabled && server.Spec.Idle.EmptySecondsBeforeStop > 0 {
if players.Known && server.Spec.Idle.AutoStopEnabled && server.Spec.Idle.EmptySecondsBeforeStop > 0 {
```

An unknown tally leaves `status.players` at the last real count, neither starts
nor clears the empty countdown, and sets the `PlayersCounted` condition to
`False` with reason `ListUnreadable`. So enabling `spec.idle.*` without RCON is
a no-op by design, and a server whose `list` reply is in a format Felis does
not know (a plugin that rewrites `/list`, a translated reply) never idles out:

```sh
kubectl get minecraftserver <name> -o jsonpath='{.status.conditions[?(@.type=="PlayersCounted")]}'
# Reason ListUnreadable: run `list` in the server's panel console to see
# what the server actually answers.
```

The comment above it says why: with RCON off the zero tally "would read as
'empty' and use to stop a server full of people". So the guard is deliberate —
enabling `spec.idle.*` without RCON is a no-op by design, not a missing feature.
Fix it by restoring a supported `/list` (for example, drop the plugin's
override or its translation of that one message). The server keeps running
either way; only the idle stop waits.

Both fields set and still nothing happens? Then the probe is failing rather than
disabled: the server would be stuck in `Starting` with `RconNotReachable`
@@ -1286,7 +1299,7 @@ for 10 seconds (the Free plan's limits).
| Build push 400 / SA denied / egress hang / Failed / executor ImagePullBackOff | §8, §8e |
| Registry push/pull unreachable | §9 |
| World deleted unexpectedly / backup skipped | §10 |
| Idle auto-stop not firing; player count 0 | §11 |
| Idle auto-stop not firing; player count 0; `PlayersCounted=False` | §11 |
| A config field seems ignored | §12 |
| PVC left behind after delete | §13 |
| Node out of disk; pods evicted / ImagePullBackOff | §13b |
+3 −0
Changes for internal/apis/felis/v1alpha1/minecraftserver_types.go: 3 added lines, 0 removed lines.
Original line number Diff line number Diff line
@@ -72,6 +72,9 @@ const (
	ConditionReady       = "Ready"
	ConditionRconReached = "RconReached"
	ConditionProvisioned = "Provisioned"
	// ConditionPlayersCounted is False while the RCON `list` reply cannot be
	// read; idle auto-stop waits for a real count (spec §8).
	ConditionPlayersCounted = "PlayersCounted"
)

// +kubebuilder:object:root=true
+47 −15
Changes for internal/operator/prober.go: 47 added lines, 15 removed lines.
Original line number Diff line number Diff line
@@ -10,17 +10,20 @@ import (
)

// 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).
// Known is false when the count could not be read (the command failed, or its
// reply matched no format below); that is not a probe error, only the absence
// of a fresh sample. The zero value is "unknown", so a caller that forgets to
// check can never mistake a failed read for an empty server.
type PlayerCount struct {
	Online int32
	Max    int32
	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 is
// zero (with a nil error) whenever the tally could not be sampled. It is an
// 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.
type Prober interface {
	Probe(ctx context.Context, addr, password string) (PlayerCount, error)
@@ -37,7 +40,7 @@ type RconProber struct {
// Probe dials addr and authenticates with password, honoring the smaller of the
// 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,
// 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
@@ -68,26 +71,55 @@ func (p RconProber) Probe(ctx context.Context, addr, password string) (PlayerCou
	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`)
// listReplyPatterns match the `list` replies of the loaders Felis runs, tried in
// order against the reply with § color codes stripped:
//
//   - vanilla 1.13+ / Paper / Fabric / Forge:
//     "There are 3 of a max of 20 players online: alice, bob, carol"
//   - vanilla 1.12 and older, Bukkit's own list:
//     "There are 3/20 players online:"
//   - EssentialsX (its /list replaces the vanilla one, RCON included); with
//     vanished players it prints visible/hidden, and both count as online:
//     "There are 3 out of maximum 20 players online."
//     "There are 3/1 out of maximum 20 players online."
//
// Each has groups (online, hidden, max); hidden is empty where the format has
// none. The search is unanchored so trailing player names do not defeat it.
var listReplyPatterns = []*regexp.Regexp{
	regexp.MustCompile(`There are (\d+)() of a max(?:imum)? of (\d+) players online`),
	regexp.MustCompile(`There are (\d+)(?:/(\d+))? out of (?:a )?maximum (?:of )?(\d+) players online`),
	regexp.MustCompile(`There are (\d+)()/(\d+) players online`),
}

// colorCode matches a legacy § formatting code (color, bold, reset, ...).
var colorCode = regexp.MustCompile(`(?i)§[0-9a-fk-orx]`)

// 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".
// (and the PlayerCount unknown) when the reply matches none of the known
// formats, so callers can distinguish "no sample" from a genuine "0 of N".
func parseListReply(reply string) (PlayerCount, bool) {
	m := listReplyPattern.FindStringSubmatch(reply)
	plain := colorCode.ReplaceAllString(reply, "")
	for _, re := range listReplyPatterns {
		m := re.FindStringSubmatch(plain)
		if m == nil {
		return PlayerCount{}, false
			continue
		}
		online, err := strconv.ParseInt(m[1], 10, 32)
		if err != nil {
			return PlayerCount{}, false
		}
	max, err := strconv.ParseInt(m[2], 10, 32)
		if m[2] != "" {
			hidden, err := strconv.ParseInt(m[2], 10, 32)
			if err != nil {
				return PlayerCount{}, false
			}
	return PlayerCount{Online: int32(online), Max: int32(max)}, true
			online += hidden
		}
		max, err := strconv.ParseInt(m[3], 10, 32)
		if err != nil {
			return PlayerCount{}, false
		}
		return PlayerCount{Online: int32(online), Max: int32(max), Known: true}, true
	}
	return PlayerCount{}, false
}
+33 −3
Changes for internal/operator/prober_internal_test.go: 33 added lines, 3 removed lines.
Original line number Diff line number Diff line
@@ -12,21 +12,51 @@ func TestParseListReply(t *testing.T) {
		{
			name:   "empty server",
			reply:  "There are 0 of a max of 20 players online:",
			want:   PlayerCount{Online: 0, Max: 20},
			want:   PlayerCount{Online: 0, Max: 20, Known: true},
			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},
			want:   PlayerCount{Online: 3, Max: 20, Known: true},
			wantOK: true,
		},
		{
			name:   "full server",
			reply:  "There are 20 of a max of 20 players online: ...",
			want:   PlayerCount{Online: 20, Max: 20},
			want:   PlayerCount{Online: 20, Max: 20, Known: true},
			wantOK: true,
		},
		{
			name:   "color codes around the numbers",
			reply:  "§6There are §c2§6 of a max of §c50§6 players online:§r alice, bob",
			want:   PlayerCount{Online: 2, Max: 50, Known: true},
			wantOK: true,
		},
		{
			name:   "vanilla 1.12 and Bukkit slash form",
			reply:  "There are 4/32 players online:\nalice, bob, carol, dave",
			want:   PlayerCount{Online: 4, Max: 32, Known: true},
			wantOK: true,
		},
		{
			name:   "EssentialsX",
			reply:  "§6There are §c0§6 out of maximum §c20§6 players online.",
			want:   PlayerCount{Online: 0, Max: 20, Known: true},
			wantOK: true,
		},
		{
			name:   "EssentialsX with vanished players counts them online",
			reply:  "§6There are §c1§6/§c2§6 out of maximum §c20§6 players online.",
			want:   PlayerCount{Online: 3, Max: 20, Known: true},
			wantOK: true,
		},
		{
			name:   "translated reply yields no sample",
			reply:  "当前有 0 个玩家在线,最大在线人数为 20 个玩家。",
			want:   PlayerCount{},
			wantOK: false,
		},
		{
			name:   "unrecognized reply yields no sample",
			reply:  "Unknown command. Try /help for a list of commands.",
+19 −5
Changes for internal/operator/reconciler.go: 19 added lines, 5 removed lines.
Original line number Diff line number Diff line
@@ -217,16 +217,27 @@ func (r *Reconciler) reconcileRunning(ctx context.Context, server *v1alpha1.Mine
			return ctrl.Result{RequeueAfter: requeueStarting}, nil
		}
		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
		// never stops idle shows why.
		if pc.Known {
			r.setCondition(server, v1alpha1.ConditionPlayersCounted, metav1.ConditionTrue, "Counted", "RCON list reply read")
		} else {
			r.setCondition(server, v1alpha1.ConditionPlayersCounted, metav1.ConditionFalse, "ListUnreadable",
				"RCON list failed or its reply matched no known format; idle auto-stop is paused until the player count can be read")
		}
	}

	// Idle auto-stop (spec §8): when enabled, the server is Running, and the
	// player tally is zero, track the empty duration and auto-stop when the
	// configured timeout expires. The existing RCON probe already supplies
	// the player count — no extra network cost.
	// Rcon.Enabled is part of the condition because `players` is only a real tally
	// when the probe above ran: with RCON off it keeps its zero value, which this
	// branch would read as "empty" and use to stop a server full of people.
	if server.Spec.Rcon.Enabled && server.Spec.Idle.AutoStopEnabled && server.Spec.Idle.EmptySecondsBeforeStop > 0 {
	// players.Known gates the whole branch: an unread tally (RCON off, `list`
	// failed, or a reply format the parser does not know) must never read as
	// "empty" and stop a server full of people. It neither stamps nor clears
	// EmptySince, so a flaky read does not restart the countdown either; the
	// stop itself only ever follows a sample that really said zero.
	if players.Known && server.Spec.Idle.AutoStopEnabled && server.Spec.Idle.EmptySecondsBeforeStop > 0 {
		if players.Online == 0 {
			if server.Status.EmptySince == nil {
				t := r.now()
@@ -525,8 +536,11 @@ func (r *Reconciler) markRunningReady(server *v1alpha1.MinecraftServer, players
	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.
	// reports live occupancy instead of the 0/0 markStopped leaves behind. An
	// unread tally keeps the last one shown.
	if players.Known {
		server.Status.Players = v1alpha1.PlayersStatus{Online: players.Online, Max: players.Max}
	}
	if server.Status.ReadySignalAt == nil {
		t := r.now()
		server.Status.ReadySignalAt = &t
Loading