diff --git a/agent/cmd/main.go b/agent/cmd/main.go index f1ca077f9..3900c18f2 100644 --- a/agent/cmd/main.go +++ b/agent/cmd/main.go @@ -138,36 +138,39 @@ func main() { modsSpec = capSpec.Mods } - var rconClient interface { - Exec(cmd string) (string, error) - } + var transport rcon.Executor switch { case !rconEnabled: // The game declares no RCON (e.g. consoleMode pty/none). Dialing // 127.0.0.1:25575 would just fail on every request; the Disabled // client makes that explicit so handlers degrade cleanly. - rconClient = rcon.Disabled{} + transport = rcon.Disabled{} case strings.EqualFold(rconProtocol, "telnet"): - rconClient = rcon.NewTelnet(rconHost, rconPort, rcon.PasswordFromFile(rconPassFile)) + transport = rcon.NewTelnet(rconHost, rconPort, rcon.PasswordFromFile(rconPassFile)) case strings.EqualFold(rconProtocol, "websocket"): - rconClient = rcon.NewWebSocket(rconHost, rconPort, rcon.PasswordFromFile(rconPassFile)) + transport = rcon.NewWebSocket(rconHost, rconPort, rcon.PasswordFromFile(rconPassFile)) case strings.EqualFold(rconProtocol, "battleye"): - rconClient = rcon.NewBattlEye(rconHost, rconPort, rcon.PasswordFromFile(rconPassFile)) + transport = rcon.NewBattlEye(rconHost, rconPort, rcon.PasswordFromFile(rconPassFile)) case strings.EqualFold(rconProtocol, "satisfactory"): - rconClient = rcon.NewSatisfactory(rconHost, rconPort, rcon.PasswordFromFile(rconPassFile)) + transport = rcon.NewSatisfactory(rconHost, rconPort, rcon.PasswordFromFile(rconPassFile)) case strings.EqualFold(rconProtocol, "palworld"): - rconClient = rcon.NewPalworld(rconHost, rconPort, rcon.PasswordFromFile(rconPassFile)) + transport = rcon.NewPalworld(rconHost, rconPort, rcon.PasswordFromFile(rconPassFile)) case strings.EqualFold(rconProtocol, "nuclearoption"): - rconClient = rcon.NewNuclearOption(rconHost, rconPort, rcon.PasswordFromFile(rconPassFile)) + transport = rcon.NewNuclearOption(rconHost, rconPort, rcon.PasswordFromFile(rconPassFile)) case strings.EqualFold(rconProtocol, "rest"): - rconClient = rcon.NewREST(rconHost, rconPort, rcon.PasswordFromFile(rconPassFile), rcon.WithGame(gameName)) + transport = rcon.NewREST(rconHost, rconPort, rcon.PasswordFromFile(rconPassFile), rcon.WithGame(gameName)) case strings.EqualFold(rconProtocol, "cli"): - rconClient = rcon.NewCLI(rconHost, rconPort, rcon.PasswordFromFile(rconPassFile), rcon.WithPipePath(cliPipe)) + transport = rcon.NewCLI(rconHost, rconPort, rcon.PasswordFromFile(rconPassFile), rcon.WithPipePath(cliPipe)) default: // "source", empty, or anything unrecognized: back-compat default. - rconClient = rcon.New(rconHost, rconPort, rcon.PasswordFromFile(rconPassFile)) + transport = rcon.New(rconHost, rconPort, rcon.PasswordFromFile(rconPassFile)) } + commands := rcon.NewCommands(transport) + rconClient := transport + if quiesceSpec != nil { + rconClient = commands + } r := chi.NewRouter() r.Use(middleware.RequestID, middleware.Recoverer) @@ -197,7 +200,7 @@ func main() { protected.Group(func(bounded chi.Router) { bounded.Use(middleware.Timeout(30 * time.Second)) players.Mount(bounded, rconClient, gameName, playerActions) - quiesce.Mount(bounded, rconClient, gameName, quiesceSpec) + quiesce.Mount(bounded, commands, gameName, quiesceSpec) lifecycle.Mount(bounded, rconClient, gameName, lifecycleSpec) actions.Mount(bounded, rconClient, gameName, actionSpecs) statusSource.Mount(bounded) @@ -213,11 +216,17 @@ func main() { }) // Backups bind save-off/save-on to the process they prepared. The service // may already route to a replacement pod, which must reject the old UID. - r.Route("/v1/pods/{uid}", func(protected chi.Router) { - protected.Use(authCheck.Middleware, target.Guard(envOr("GAMEPLANE_POD_UID", "")), middleware.Timeout(30*time.Second)) - protected.Get("/identity", func(w http.ResponseWriter, _ *http.Request) { w.WriteHeader(http.StatusNoContent) }) - quiesce.Mount(protected, rconClient, gameName, quiesceSpec) - }) + for _, version := range []string{"v1", "v2"} { + r.Route("/"+version+"/pods/{uid}", func(protected chi.Router) { + protected.Use(authCheck.Middleware, target.Guard(envOr("GAMEPLANE_POD_UID", "")), middleware.Timeout(30*time.Second)) + protected.Get("/identity", func(w http.ResponseWriter, _ *http.Request) { w.WriteHeader(http.StatusNoContent) }) + if version == "v2" { + quiesce.MountVerified(protected, commands, gameName, quiesceSpec) + } else { + quiesce.Mount(protected, commands, gameName, quiesceSpec) + } + }) + } srv := &http.Server{ Addr: addr, diff --git a/agent/internal/quiesce/quiesce.go b/agent/internal/quiesce/quiesce.go index 7d77d2589..c258e5cc3 100644 --- a/agent/internal/quiesce/quiesce.go +++ b/agent/internal/quiesce/quiesce.go @@ -5,9 +5,9 @@ // The command sequences are per-game and declared by the module's // template (spec.capabilities.quiesce), e.g. Minecraft toggles // auto-save with "save-off" + "save-all flush" / "save-on" over RCON. -// Games that declare nothing get a best-effort no-op so the backup -// pipeline can proceed unconditionally — backups should never fail -// because a game can't be paused. +// A verified pause reports unsupported before issuing any RCON command when +// the agent cannot check the declared sequence. The operator decides whether +// that permits a backup to proceed based on any earlier pause attempt. package quiesce import ( @@ -15,18 +15,19 @@ import ( "log/slog" "net/http" "regexp" + "slices" + "strings" "github.com/go-chi/chi/v5" "github.com/GameplanePanel/gameplane/agent/internal/caps" "github.com/GameplanePanel/gameplane/agent/internal/httpjson" + "github.com/GameplanePanel/gameplane/agent/internal/rcon" ) // Rcon is the slice of *rcon.Client we actually use. Defined here as // an interface so tests can swap in a recording fake. -type Rcon interface { - Exec(cmd string) (string, error) -} +type Rcon = rcon.Executor // Quiescer encapsulates the game-specific RCON sequence for pausing // writes (Quiesce) and resuming them (Unquiesce). Implementations @@ -39,11 +40,8 @@ type Quiescer interface { Unquiesce(rc Rcon) error } -// Pick builds the quiescer from the module's declared sequences -// (spec.capabilities.quiesce). A template that declares no sequence (or -// only half of one) gets a no-op quiescer: backups proceed -// unconditionally and we never pause a game we can't resume. Quiesce is -// module-driven, with no per-game special-casing in the agent. +// Pick builds the legacy quiescer from spec.capabilities.quiesce. It reports +// unsupported unless the template supplies both a pause and a resume sequence. func Pick(spec *caps.Quiesce) Quiescer { if spec != nil && len(spec.Quiesce) > 0 && len(spec.Unquiesce) > 0 { return newDeclaredQuiescer(spec) @@ -53,54 +51,99 @@ func Pick(spec *caps.Quiesce) Quiescer { type response struct { Quiesced bool `json:"quiesced"` + Outcome string `json:"outcome,omitempty"` Reason string `json:"reason,omitempty"` } -// Mount registers POST /quiesce and POST /unquiesce on r. -func Mount(r chi.Router, rc Rcon, game string, spec *caps.Quiesce) { - q := Pick(spec) - r.Post("/quiesce", func(w http.ResponseWriter, _ *http.Request) { - if !q.Supported() { - httpjson.Write(w, http.StatusOK, response{Quiesced: false, Reason: "game does not support quiesce"}) - return - } - if err := q.Quiesce(rc); err != nil { - slog.Warn("quiesce failed", "game", game, "err", err) - httpjson.Write(w, http.StatusBadGateway, response{Quiesced: false, Reason: "rcon error"}) - return - } - httpjson.Write(w, http.StatusOK, response{Quiesced: true}) - }) - r.Post("/unquiesce", func(w http.ResponseWriter, _ *http.Request) { - if !q.Supported() { - httpjson.Write(w, http.StatusOK, response{Quiesced: false, Reason: "game does not support quiesce"}) - return - } - if err := q.Unquiesce(rc); err != nil { - // Unquiesce failure is more dangerous than quiesce failure - // (the game is now stuck with auto-save off), but we still - // must not block the controller — surface 502 so the - // operator can record an event. - slog.Warn("unquiesce failed", "game", game, "err", err) - httpjson.Write(w, http.StatusBadGateway, response{Quiesced: true, Reason: "unquiesce rcon error"}) - return - } - httpjson.Write(w, http.StatusOK, response{Quiesced: false}) - }) +// Mount preserves the legacy declaration-driven endpoints for older clients. +func Mount(r chi.Router, rc *rcon.Commands, game string, spec *caps.Quiesce) { + mount(r, rc, game, Pick(spec), false) +} + +// MountVerified serves the verified Minecraft Java save-off, save-all flush and +// save-on sequence. An unsupported response means this request sent no command. +// A previous request may still have paused the game before losing its reply. +func MountVerified(r chi.Router, rc *rcon.Commands, game string, spec *caps.Quiesce) { + var q Quiescer = unsupportedQuiescer{} + if game == "minecraft-java" && spec != nil && + slices.Equal(spec.Quiesce, []string{"save-off", "save-all flush"}) && + slices.Equal(spec.Unquiesce, []string{"save-on"}) { + declared := newDeclaredQuiescer(spec) + declared.rollbackFirst = true + q = declared + } + mount(r, rc, game, q, true) +} + +func mount(r chi.Router, rc *rcon.Commands, game string, q Quiescer, verified bool) { + handle := func(path, outcome string, paused bool, run func(Rcon) error) { + r.Post(path, func(w http.ResponseWriter, req *http.Request) { + if !q.Supported() { + httpjson.Write(w, http.StatusOK, response{Outcome: "unsupported", Reason: "game does not support quiesce"}) + return + } + err := rc.Sequence(req.Context(), func(transport rcon.Executor) error { + if verified { + transport = minecraftReplies{transport} + } + return run(transport) + }) + if err != nil { + slog.Warn("backup command failed", "path", path, "game", game, "err", err) + httpjson.Write(w, http.StatusBadGateway, response{Quiesced: !paused, Reason: "backup command failed"}) + return + } + ack := response{Quiesced: paused} + if verified { + ack.Outcome = outcome + } + httpjson.Write(w, http.StatusOK, ack) + }) + } + handle("/quiesce", "paused", true, q.Quiesce) + handle("/unquiesce", "resumed", false, q.Unquiesce) +} + +type minecraftReplies struct{ Rcon } + +// Vanilla Java 1.21.4 en_us commands.save.* messages. RCON can concatenate +// the two flush messages with no separator. Never accept a success substring +// embedded in an error, or a reply to the opposite command. +func (m minecraftReplies) Exec(cmd string) (string, error) { + reply, err := m.Rcon.Exec(cmd) + if err != nil { + return "", err + } + out := strings.TrimSpace(reply) + var ok bool + switch cmd { + case "save-off": + ok = out == "Automatic saving is now disabled" || out == "Saving is already turned off" + case "save-on": + ok = out == "Automatic saving is now enabled" || out == "Saving is already turned on" + case "save-all flush": + out = strings.TrimSpace(strings.TrimPrefix(out, "Saving the game (this may take a moment!)")) + ok = out == "Saved the game" + } + if !ok { + return "", fmt.Errorf("game did not acknowledge %q", cmd) + } + return reply, nil } // --- Declared (module-driven) ------------------------------------------ type declaredQuiescer struct { - quiesce []string - unquiesce []string - failRE *regexp.Regexp + quiesce []string + unquiesce []string + failRE *regexp.Regexp + rollbackFirst bool } // newDeclaredQuiescer compiles the declared sequences. An invalid // FailurePattern is logged and ignored — the commands still run, the // output check just can't. -func newDeclaredQuiescer(spec *caps.Quiesce) Quiescer { +func newDeclaredQuiescer(spec *caps.Quiesce) declaredQuiescer { q := declaredQuiescer{quiesce: spec.Quiesce, unquiesce: spec.Unquiesce} if spec.FailurePattern != "" { re, err := regexp.Compile("(?i)" + spec.FailurePattern) @@ -122,10 +165,9 @@ func (q declaredQuiescer) Quiesce(rc Rcon) error { err = fmt.Errorf("quiesce command %q reported failure", cmd) } if err != nil { - // Roll auto-save (or whatever the game paused) back on so we - // don't leave the world frozen — but only if any pausing - // command already ran. - if i > 0 { + // A timed-out save-off may already have reached the game. + // The verified save-on contract accepts "already on" on retry. + if i > 0 || q.rollbackFirst { _ = q.Unquiesce(rc) } return err @@ -137,7 +179,11 @@ func (q declaredQuiescer) Quiesce(rc Rcon) error { func (q declaredQuiescer) Unquiesce(rc Rcon) error { var firstErr error for _, cmd := range q.unquiesce { - if _, err := rc.Exec(cmd); err != nil && firstErr == nil { + out, err := rc.Exec(cmd) + if err == nil && q.failRE != nil && q.failRE.MatchString(out) { + err = fmt.Errorf("unquiesce command %q reported failure", cmd) + } + if err != nil && firstErr == nil { firstErr = err } } diff --git a/agent/internal/quiesce/quiesce_test.go b/agent/internal/quiesce/quiesce_test.go index 62aca43a4..64671914c 100644 --- a/agent/internal/quiesce/quiesce_test.go +++ b/agent/internal/quiesce/quiesce_test.go @@ -12,6 +12,7 @@ import ( "github.com/go-chi/chi/v5" "github.com/GameplanePanel/gameplane/agent/internal/caps" + "github.com/GameplanePanel/gameplane/agent/internal/rcon" "github.com/GameplanePanel/gameplane/agent/internal/target" ) @@ -83,7 +84,7 @@ func TestUnsupportedQuiescerNoOp(t *testing.T) { func newTestRouter(rc Rcon, spec *caps.Quiesce) *chi.Mux { r := chi.NewRouter() - Mount(r, rc, "testgame", spec) + Mount(r, rcon.NewCommands(rc), "testgame", spec) return r } @@ -94,7 +95,7 @@ func TestPodBoundRoutesRejectReplacementBeforeRCON(t *testing.T) { r := chi.NewRouter() r.Route("/v1/pods/{uid}", func(bound chi.Router) { bound.Use(target.Guard(actual)) - Mount(bound, rc, "test", minecraftQuiesceSpec()) + Mount(bound, rcon.NewCommands(rc), "test", minecraftQuiesceSpec()) }) for _, path := range []string{"quiesce", "unquiesce"} { response := httptest.NewRecorder() diff --git a/agent/internal/quiesce/verified_test.go b/agent/internal/quiesce/verified_test.go new file mode 100644 index 000000000..66ae039cf --- /dev/null +++ b/agent/internal/quiesce/verified_test.go @@ -0,0 +1,137 @@ +package quiesce + +import ( + "context" + "errors" + "net/http" + "net/http/httptest" + "reflect" + "sync" + "testing" + + "github.com/go-chi/chi/v5" + + "github.com/GameplanePanel/gameplane/agent/internal/rcon" +) + +func TestMinecraftPositiveCommandResponses(t *testing.T) { + for _, tc := range []struct { + cmd, reply string + ok bool + }{ + {"save-off", "Automatic saving is now disabled", true}, + {"save-off", "Saving is already turned off", true}, + {"save-on", "Automatic saving is now enabled", true}, + {"save-on", "Saving is already turned on", true}, + {"save-all flush", "Saving the game (this may take a moment!)Saved the game", true}, + {"save-all flush", "Saving the game (this may take a moment!)\nSaved the game", true}, + {"save-all flush", "Saved the game", true}, + {"save-all flush", "Saving the game (this may take a moment!)", false}, + {"save-all flush", "Unable to save the game (is there enough disk space?)", false}, + {"save-on", "Automatic saving is now disabled", false}, + {"save-on", "", false}, + {"save-on", "Unknown or incomplete command", false}, + {"save-on", "Error: Automatic saving is now enabled", false}, + } { + t.Run(tc.cmd+"/"+tc.reply, func(t *testing.T) { + m := minecraftReplies{&fakeRcon{respond: func(string) (string, error) { return tc.reply, nil }}} + if _, err := m.Exec(tc.cmd); (err == nil) != tc.ok { + t.Fatalf("reply %q: %v", tc.reply, err) + } + }) + } +} + +func TestVerifiedSequenceBlocksCleanupUntilRollbackFinishes(t *testing.T) { + reached, release := make(chan struct{}), make(chan struct{}) + var once sync.Once + rc := &fakeRcon{respond: func(cmd string) (string, error) { + switch cmd { + case "save-off": + return "Automatic saving is now disabled", nil + case "save-all flush": + once.Do(func() { close(reached) }) + <-release + return "", context.DeadlineExceeded + case "save-on": + return "Saving is already turned on", nil + } + return "", errors.New("unexpected command") + }} + commands := rcon.NewCommands(rc) + router := chi.NewRouter() + MountVerified(router, commands, "minecraft-java", minecraftQuiesceSpec()) + pause := make(chan int, 1) + go func() { + w := httptest.NewRecorder() + router.ServeHTTP(w, httptest.NewRequestWithContext(t.Context(), http.MethodPost, "/quiesce", nil)) + pause <- w.Code + }() + <-reached + ctx, cancel := context.WithCancel(t.Context()) + cancel() + w := httptest.NewRecorder() + router.ServeHTTP(w, httptest.NewRequestWithContext(ctx, http.MethodPost, "/quiesce", nil)) + if w.Code != http.StatusBadGateway { + t.Errorf("canceled pause: %d", w.Code) + } + resumed := make(chan *httptest.ResponseRecorder, 1) + go func() { + w := httptest.NewRecorder() + router.ServeHTTP(w, httptest.NewRequestWithContext(t.Context(), http.MethodPost, "/unquiesce", nil)) + resumed <- w + }() + close(release) + if status := <-pause; status != http.StatusBadGateway { + t.Errorf("timed-out pause: %d", status) + } + w = <-resumed + if w.Code != http.StatusOK || decodeResponse(t, w.Body.Bytes()).Outcome != "resumed" { + t.Fatalf("cleanup response: %d %s", w.Code, w.Body) + } + want := []string{"save-off", "save-all flush", "save-on", "save-on"} + if got := rc.calls(); !reflect.DeepEqual(got, want) { + t.Fatalf("commands = %v, want %v", got, want) + } +} + +func TestVerifiedRejectsUnqualifiedDeclarations(t *testing.T) { + rc := &fakeRcon{} + r := chi.NewRouter() + MountVerified(r, rcon.NewCommands(rc), "other-game", minecraftQuiesceSpec()) + for _, path := range []string{"/quiesce", "/unquiesce"} { + w := httptest.NewRecorder() + r.ServeHTTP(w, httptest.NewRequestWithContext(t.Context(), http.MethodPost, path, nil)) + if got := decodeResponse(t, w.Body.Bytes()); got.Outcome != "unsupported" || got.Quiesced || len(rc.calls()) != 0 { + t.Fatalf("%s = %+v, calls %v", path, got, rc.calls()) + } + } +} + +func TestVerifiedRejectedResumeCannotAcknowledgeAutosave(t *testing.T) { + rc := &fakeRcon{respond: func(cmd string) (string, error) { + switch cmd { + case "save-off": + return "Saving is already turned off", nil + case "save-all flush": + return "Saved the game", nil + } + return "Unknown or incomplete command", nil + }} + router := chi.NewRouter() + MountVerified(router, rcon.NewCommands(rc), "minecraft-java", minecraftQuiesceSpec()) + for _, tc := range []struct { + path string + status int + outcome string + }{ + {"/quiesce", http.StatusOK, "paused"}, {"/quiesce", http.StatusOK, "paused"}, + {"/unquiesce", http.StatusBadGateway, ""}, + } { + w := httptest.NewRecorder() + router.ServeHTTP(w, httptest.NewRequestWithContext(t.Context(), http.MethodPost, tc.path, nil)) + if got := decodeResponse(t, w.Body.Bytes()); w.Code != tc.status || got.Outcome != tc.outcome || !got.Quiesced { + t.Fatalf("%s: %d %+v", tc.path, w.Code, got) + } + } +} diff --git a/agent/internal/rcon/commands.go b/agent/internal/rcon/commands.go new file mode 100644 index 000000000..14d1be824 --- /dev/null +++ b/agent/internal/rcon/commands.go @@ -0,0 +1,41 @@ +package rcon + +import "context" + +// Executor is the command transport shared by all handlers for one game. +type Executor interface { + Exec(string) (string, error) +} + +// Commands admits either one command or one complete backup sequence at a time. +type Commands struct { + transport Executor + busy chan struct{} +} + +// NewCommands shares one admission queue across clients of a transport. +func NewCommands(transport Executor) *Commands { + return &Commands{transport: transport, busy: make(chan struct{}, 1)} +} + +// Exec admits one command when no sequence is in progress. +func (c *Commands) Exec(command string) (string, error) { + c.busy <- struct{}{} + defer func() { <-c.busy }() + return c.transport.Exec(command) +} + +// Sequence gives run exclusive use of the underlying transport, including +// any compensating commands. A canceled waiter must never start a late pause. +func (c *Commands) Sequence(ctx context.Context, run func(Executor) error) error { + select { + case <-ctx.Done(): + return ctx.Err() + case c.busy <- struct{}{}: + } + defer func() { <-c.busy }() + if err := ctx.Err(); err != nil { + return err + } + return run(c.transport) +} diff --git a/agent/internal/rcon/commands_test.go b/agent/internal/rcon/commands_test.go new file mode 100644 index 000000000..6f8c9c540 --- /dev/null +++ b/agent/internal/rcon/commands_test.go @@ -0,0 +1,55 @@ +package rcon + +import ( + "context" + "errors" + "slices" + "testing" + "testing/synctest" +) + +type recordCommand func(string) (string, error) + +func (f recordCommand) Exec(command string) (string, error) { return f(command) } + +func TestCommandsExcludeOtherCallersFromSequence(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + var calls []string + c := NewCommands(recordCommand(func(s string) (string, error) { calls = append(calls, s); return s, nil })) + release := make(chan struct{}) + go func() { + err := c.Sequence(t.Context(), func(rc Executor) error { + _, _ = rc.Exec("first") + <-release + _, _ = rc.Exec("second") + return nil + }) + if err != nil { + t.Error(err) + } + }() + synctest.Wait() + go func() { + if out, err := c.Exec("console"); out != "console" || err != nil { + t.Errorf("console: %q, %v", out, err) + } + }() + ctx, cancel := context.WithCancel(t.Context()) + go func() { + if err := c.Sequence(ctx, func(Executor) error { t.Error("ran canceled sequence"); return nil }); !errors.Is(err, context.Canceled) { + t.Errorf("canceled sequence: %v", err) + } + }() + synctest.Wait() + cancel() + synctest.Wait() + if !slices.Equal(calls, []string{"first"}) { + t.Fatalf("sequence interleaved: %v", calls) + } + close(release) + synctest.Wait() + if !slices.Equal(calls, []string{"first", "second", "console"}) { + t.Fatalf("calls=%v", calls) + } + }) +} diff --git a/agent/openapi.yaml b/agent/openapi.yaml index 481eee593..49b94061e 100644 --- a/agent/openapi.yaml +++ b/agent/openapi.yaml @@ -376,3 +376,52 @@ paths: quiesced: { type: boolean, description: "false on success" } reason: { type: string } "502": { description: RCON unavailable; the game may still be quiesced — retry } + + /v2/pods/{uid}/identity: + parameters: + - &podUID + name: uid + in: path + required: true + schema: { type: string } + get: + summary: Verify the pod UID and availability of the verified backup protocol + responses: + "204": { description: Matching pod UID } + "404": { description: Missing or mismatched pod UID } + /v2/pods/{uid}/quiesce: + parameters: [*podUID] + post: + summary: Pause with verified game command replies + responses: + "200": + description: Paused or unsupported + content: + application/json: + schema: + type: object + required: [outcome, quiesced] + properties: + outcome: { type: string, enum: [paused, unsupported] } + quiesced: { type: boolean, description: "true only when paused" } + reason: { type: string } + "502": { description: Transport failure or unacknowledged command } + "404": { description: Missing or mismatched pod UID } + /v2/pods/{uid}/unquiesce: + parameters: [*podUID] + post: + summary: Resume with verified game command replies + responses: + "200": + description: Only resumed confirms completion of cleanup + content: + application/json: + schema: + type: object + required: [outcome, quiesced] + properties: + outcome: { type: string, enum: [resumed, unsupported] } + quiesced: { type: boolean, enum: [false] } + reason: { type: string } + "502": { description: Transport failure or unacknowledged command } + "404": { description: Missing or mismatched pod UID } diff --git a/agent/specs.md b/agent/specs.md index 2ee465f71..40935f565 100644 --- a/agent/specs.md +++ b/agent/specs.md @@ -187,8 +187,9 @@ Resource usage env vars (set by the operator): | `/players/banned` | GET | Currently banned players; response: `[ { name, reason?, source? } ]`; 502 if RCON unavailable | | `/quiesce` | POST | Pause auto-saves before snapshot; response: `{ quiesced, reason? }` (boolean, reason optional); unsupported games return `quiesced: false` with a reason | | `/unquiesce` | POST | Resume auto-saves after snapshot; response: `{ quiesced, reason? }` | -| `/v1/pods/{uid}/identity` | GET | Operator preflight: 204 only when `{uid}` matches `GAMEPLANE_POD_UID`; never mutates game state | +| `/v2/pods/{uid}/identity`, `/v1/pods/{uid}/identity` | GET | Operator preflight: 204 only when `{uid}` matches `GAMEPLANE_POD_UID`; never mutates game state | | `/v1/pods/{uid}/quiesce`, `/v1/pods/{uid}/unquiesce` | POST | Pod-bound backup RPCs with the same response contract as the legacy routes; reject missing/mismatched Pod UID with 404 before RCON | +| `/v2/pods/{uid}/quiesce`, `/v2/pods/{uid}/unquiesce` | POST | Verified backup RPCs. HTTP 200 requires `outcome: paused` with `quiesced: true`, or `outcome: resumed` with `quiesced: false`. The shipped Minecraft Java sequence checks each vanilla English RCON reply, including already-on/off retries. Unsupported sequences return `outcome: unsupported`, `quiesced: false` and a reason without issuing commands. The whole sequence, including rollback, excludes other agent RCON calls. | | `/lifecycle/stop` | POST | Run stop sequence before scale-to-zero; RCON connection drop is the expected outcome | | `/actions/run` | POST | Execute module-declared operator action; request: `{ id, params }`; response: `{ ok, raw? }` | | `/status` | GET | Live game metrics (module-declared); response: bare JSON array `[ { id, displayName?, value, unit? } ]` | diff --git a/changelog/unreleased/899-backup-resume.md b/changelog/unreleased/899-backup-resume.md new file mode 100644 index 000000000..0980b9ae5 --- /dev/null +++ b/changelog/unreleased/899-backup-resume.md @@ -0,0 +1 @@ +Live Minecraft Java backups now verify the pause, flush and resume commands against the original game process. The operator retains an unconfirmed resume across restarts and checks for container replacement before sending `save-on`. Before upgrading agents that only support the v1 backup endpoint, let active live backups finish and check their `Unquiesced` condition. See the [operator upgrade guidance](../../docs/operator-operations.md#shared-operation-ownership). diff --git a/docs/module-authoring.md b/docs/module-authoring.md index a6fb80600..08a2886ec 100644 --- a/docs/module-authoring.md +++ b/docs/module-authoring.md @@ -1214,13 +1214,13 @@ capabilities: From `modules/palworld/template.yaml` (note: Palworld uses the REST admin API `protocol: palworld`, not RCON, so the players list is fetched via the REST endpoint, not a console command). -- The quiesce sequence runs in order; any command error — or output - matching `failurePattern` (case-insensitive) — aborts the backup. If - at least one earlier command in the sequence already succeeded, - `unquiesce` then runs best-effort so the game is never left paused; - a failure on the *first* command means nothing was paused yet, so - `unquiesce` is skipped. Games that can't quiesce simply omit the - block; backups proceed without pausing. +- The operator uses the verified agent endpoint for live backups. For + `minecraft-java`, declare the sequence above. The agent checks the English + vanilla response to each command, including the already-on and already-off + replies on retries. A rejected flush triggers a save-on attempt. Unrecognized + command sequences report unsupported before issuing a command. The legacy + agent endpoints still execute module-declared sequences and check their + optional failure pattern on both pause and resume. - `lifecycle.stop` runs before the pod is scaled down (stop button, restarts): the operator issues the sequence — over RCON when the template has it, otherwise over a stdin pod-attach for `consoleMode: diff --git a/docs/operator-operations.md b/docs/operator-operations.md index 9a123bdc1..9cb3f17cf 100644 --- a/docs/operator-operations.md +++ b/docs/operator-operations.md @@ -129,6 +129,12 @@ or agent connectivity before retrying. Removing owner annotations or force-remov finalizers cannot establish that a worker has stopped. A failed or canceled restore leaves its target suspended for inspection. +Before upgrading from agents with only the v1 backup endpoint, let active live +backups finish and check their `Unquiesced` condition. The new operator checks +for v2 support before starting a new pause. If an upgrade interrupts an existing +pause, the operator retains the owner until it receives a verified resume or +observes the original process exit. An older agent cannot supply that acknowledgment. + Upgrades recognize existing owner annotations and import backup recovery lineage. If that legacy record lacks the original Pod UID, cleanup cannot safely address save-on to a running Pod. Its owner remains held until an uncached read proves diff --git a/operator/go.mod b/operator/go.mod index f9636b483..ddfd4972f 100644 --- a/operator/go.mod +++ b/operator/go.mod @@ -22,6 +22,7 @@ require ( k8s.io/api v0.37.2 k8s.io/apimachinery v0.37.2 k8s.io/client-go v0.37.2 + k8s.io/utils v0.0.0-20260707023825-cf1189d6abe3 oras.land/oras-go/v2 v2.7.0 sigs.k8s.io/controller-runtime v0.25.2 sigs.k8s.io/yaml v1.6.0 @@ -178,7 +179,6 @@ require ( k8s.io/klog/v2 v2.140.0 // indirect k8s.io/kube-openapi v0.0.0-20260721132016-d427ff9ee9ad // indirect k8s.io/streaming v0.37.2 // indirect - k8s.io/utils v0.0.0-20260707023825-cf1189d6abe3 // indirect sigs.k8s.io/controller-tools v0.20.1 // indirect sigs.k8s.io/json v0.0.0-20250730193827-2d320260d730 // indirect sigs.k8s.io/randfill v1.0.0 // indirect diff --git a/operator/internal/agent/client.go b/operator/internal/agent/client.go index 2b1c06923..90694bba8 100644 --- a/operator/internal/agent/client.go +++ b/operator/internal/agent/client.go @@ -16,23 +16,21 @@ import ( "net/http" "net/url" "os" + "strings" "time" ) -// ErrUnsupported is returned when the agent acknowledges the request -// but reports the underlying game has no quiesce equivalent. Callers -// should treat this as success-with-degradation, not as a hard error -// — backups must still proceed when the game can't be paused. +// ErrUnsupported means the agent declined the pause because it cannot quiesce +// this game. A first verified refusal allows the backup to proceed without a +// pause. It cannot clear the resume obligation from an earlier ambiguous call. var ErrUnsupported = errors.New("agent: game does not support quiesce") // Client speaks to the agent's HTTPS endpoints. Zero value is unusable; // build via New. type Client struct { http *http.Client - // Disabled is true when no mTLS material was configured. All - // methods on a disabled client return nil without dialing — this - // lets the operator boot in dev clusters without a chart-managed - // Secret. Production deployments are expected to set the flags. + // Disabled is true when no mTLS material was configured. Legacy requests + // may return success without dialing. Pod-bound operations return an error. Disabled bool } @@ -46,18 +44,13 @@ type Config struct { Timeout time.Duration } -// Enabled reports whether c can actually reach an agent. A disabled -// client (no mTLS material configured) silently no-ops every method, so -// callers that need to pick a transport based on availability — rather -// than just calling a method and accepting a no-op — must check this -// instead of a nil-client check alone: agent.New never returns a nil -// *Client, only a non-nil one with Disabled set. +// Enabled reports whether the client has mTLS configured. It does not probe an agent. func (c *Client) Enabled() bool { return !c.Disabled } // New builds an mTLS-configured Client. Empty cert paths produce a -// disabled client (no-op methods). +// disabled client. Partial configuration returns an error. func New(cfg Config) (*Client, error) { if cfg.CABundle == "" && cfg.ClientCert == "" && cfg.ClientKey == "" { return &Client{Disabled: true}, nil @@ -103,7 +96,8 @@ func agentURL(namespace, server, path string) string { } type quiesceResponse struct { - Quiesced bool `json:"quiesced"` + Quiesced *bool `json:"quiesced"` + Outcome string `json:"outcome"` Reason string `json:"reason,omitempty"` } @@ -129,7 +123,7 @@ func (c *Client) CheckPod(ctx context.Context, namespace, server, podUID string) if c.Disabled || podUID == "" { return fmt.Errorf("agent: pod identity check requires an enabled client and pod UID") } - req, err := http.NewRequestWithContext(ctx, http.MethodGet, agentURL(namespace, server, "/v1/pods/"+url.PathEscape(podUID)+"/identity"), nil) + req, err := http.NewRequestWithContext(ctx, http.MethodGet, agentURL(namespace, server, "/v2/pods/"+url.PathEscape(podUID)+"/identity"), nil) if err != nil { return err } @@ -145,11 +139,15 @@ func (c *Client) CheckPod(ctx context.Context, namespace, server, podUID string) } // QuiescePod disables saving only on the selected Pod's agent. +// A lost acknowledgment can leave saving disabled. Record the resume obligation +// before sending the request. func (c *Client) QuiescePod(ctx context.Context, namespace, server, podUID string) error { return c.callPodQuiesce(ctx, namespace, server, podUID, "/quiesce", true) } // UnquiescePod restores saving only on the selected Pod's agent. +// A Pod UID survives a container restart. The caller must check game container +// identity before sending save-on to the process that owes a resume. func (c *Client) UnquiescePod(ctx context.Context, namespace, server, podUID string) error { return c.callPodQuiesce(ctx, namespace, server, podUID, "/unquiesce", false) } @@ -158,7 +156,7 @@ func (c *Client) callPodQuiesce(ctx context.Context, namespace, server, podUID, if c.Disabled || podUID == "" { return fmt.Errorf("agent: pod-bound quiesce requires an enabled client and pod UID") } - return c.callQuiesce(ctx, namespace, server, "/v1/pods/"+url.PathEscape(podUID)+path, expectQuiesced) + return c.callQuiesce(ctx, namespace, server, "/v2/pods/"+url.PathEscape(podUID)+path, expectQuiesced) } // expectQuiesced is what the response should report on a successful @@ -177,25 +175,40 @@ func (c *Client) callQuiesce(ctx context.Context, namespace, server, path string return fmt.Errorf("agent: %s: %w", path, err) } defer func() { _ = resp.Body.Close() }() - body, _ := io.ReadAll(io.LimitReader(resp.Body, 16<<10)) + const maxReply = 16 << 10 + body, err := io.ReadAll(io.LimitReader(resp.Body, maxReply+1)) + if err != nil { + return fmt.Errorf("agent: %s: read response: %w", path, err) + } + if len(body) > maxReply { + return fmt.Errorf("agent: %s: oversized response", path) + } if resp.StatusCode != http.StatusOK { return fmt.Errorf("agent: %s: status %d", path, resp.StatusCode) } var parsed quiesceResponse - if len(body) > 0 { - if err := json.Unmarshal(body, &parsed); err != nil { - return fmt.Errorf("agent: %s: decode response: %w", path, err) - } - } - // Detect the unsupported-game case via the canonical reason string - // so we can report it distinctly without leaking it as a hard error. - if !parsed.Quiesced && expectQuiesced { - if parsed.Reason == "game does not support quiesce" { - return ErrUnsupported - } - return fmt.Errorf("agent: %s: refused (%s)", path, parsed.Reason) + if err := json.Unmarshal(body, &parsed); err != nil { + return fmt.Errorf("agent: %s: decode response: %w", path, err) + } + if parsed.Quiesced == nil { + return fmt.Errorf("agent: %s: missing quiesced field", path) + } + verified := strings.HasPrefix(path, "/v2/pods/") + if expectQuiesced && !*parsed.Quiesced && parsed.Reason == "game does not support quiesce" && + (!verified || parsed.Outcome == "unsupported") { + return ErrUnsupported + } + if *parsed.Quiesced != expectQuiesced || parsed.Reason != "" { + return fmt.Errorf("agent: %s: refused or contradictory acknowledgment", path) + } + want := "resumed" + if expectQuiesced { + want = "paused" + } + if verified && parsed.Outcome != want { + return fmt.Errorf("agent: %s: missing or contradictory outcome", path) } return nil } diff --git a/operator/internal/agent/client_test.go b/operator/internal/agent/client_test.go index 7d04dc9d1..987912bf7 100644 --- a/operator/internal/agent/client_test.go +++ b/operator/internal/agent/client_test.go @@ -129,14 +129,14 @@ func TestPodBoundQuiesceNeverFallsBackToLegacyRoute(t *testing.T) { c, closeServer := newMockClient(t, func(w http.ResponseWriter, r *http.Request) { paths = append(paths, r.URL.Path) w.WriteHeader(status) - _, _ = w.Write([]byte(`{"quiesced":true}`)) + _, _ = w.Write([]byte(`{"quiesced":true,"outcome":"paused"}`)) }) defer closeServer() err := c.QuiescePod(t.Context(), "ns", "server", "original-pod") if (err == nil) != (status == http.StatusOK) { t.Fatalf("status=%d error=%v", status, err) } - if len(paths) != 1 || paths[0] != "/v1/pods/original-pod/quiesce" { + if len(paths) != 1 || paths[0] != "/v2/pods/original-pod/quiesce" { t.Fatalf("unbound mutation/fallback: %v", paths) } }) @@ -145,10 +145,10 @@ func TestPodBoundQuiesceNeverFallsBackToLegacyRoute(t *testing.T) { func TestPodBoundUnquiesceAndDisabledClient(t *testing.T) { c, closeServer := newMockClient(t, func(w http.ResponseWriter, r *http.Request) { - if r.URL.Path != "/v1/pods/original-pod/unquiesce" { + if r.URL.Path != "/v2/pods/original-pod/unquiesce" { t.Errorf("wrong cleanup target: %s", r.URL.Path) } - _, _ = w.Write([]byte(`{"quiesced":false}`)) + _, _ = w.Write([]byte(`{"quiesced":false,"outcome":"resumed"}`)) }) defer closeServer() if err := c.UnquiescePod(t.Context(), "ns", "server", "original-pod"); err != nil { @@ -303,7 +303,7 @@ func TestCheckPodRequiresIdentityEndpoint(t *testing.T) { calls := 0 c, cleanup := newMockClient(t, func(w http.ResponseWriter, req *http.Request) { calls++ - if req.Method != http.MethodGet || req.URL.Path != "/v1/pods/original/identity" { + if req.Method != http.MethodGet || req.URL.Path != "/v2/pods/original/identity" { t.Errorf("unexpected preflight request: %s %s", req.Method, req.URL.Path) } w.WriteHeader(status) @@ -339,3 +339,15 @@ type errTransport struct{} func (errTransport) RoundTrip(*http.Request) (*http.Response, error) { return nil, errors.New("dial fail") } + +func TestPodBoundQuiesceRequiresDirectionalAcknowledgment(t *testing.T) { + for _, body := range []string{"", "not json", "null", "{}", `{"outcome":"resumed"}`, `{"quiesced":null,"outcome":"resumed"}`, `{"quiesced":true,"outcome":"resumed"}`, `{"quiesced":false,"outcome":"paused"}`, `{"quiesced":false}`, `{"quiesced":false,"outcome":"unsupported","reason":"game does not support quiesce"}`, `{"quiesced":false,"outcome":"resumed","reason":"rcon error"}`, `{"quiesced":false,"outcome":"resumed"}{}`, strings.Repeat(" ", 16<<10+1)} { + t.Run(body[:min(64, len(body))], func(t *testing.T) { + c, closeServer := newMockClient(t, func(w http.ResponseWriter, _ *http.Request) { _, _ = w.Write([]byte(body)) }) + defer closeServer() + if err := c.UnquiescePod(t.Context(), "ns", "game", "pod"); err == nil || errors.Is(err, ErrUnsupported) { + t.Fatalf("invalid response discharged cleanup: %v", err) + } + }) + } +} diff --git a/operator/internal/controller/backup_container_restart_test.go b/operator/internal/controller/backup_container_restart_test.go index 5043340b2..81334b8db 100644 --- a/operator/internal/controller/backup_container_restart_test.go +++ b/operator/internal/controller/backup_container_restart_test.go @@ -44,6 +44,24 @@ func backupGameChanges() map[string]func(*corev1.Pod) { "waiting": func(p *corev1.Pod) { p.Status.ContainerStatuses[0].State = corev1.ContainerState{Waiting: &corev1.ContainerStateWaiting{Reason: "CrashLoopBackOff"}} }, + "waiting-unrelated-exit": func(p *corev1.Pod) { + game := &p.Status.ContainerStatuses[0] + game.ContainerID = "" + game.LastTerminationState.Terminated = &corev1.ContainerStateTerminated{ContainerID: "containerd://earlier-game", ExitCode: 1} + game.State = corev1.ContainerState{Waiting: &corev1.ContainerStateWaiting{Reason: "CrashLoopBackOff"}} + }, + "waiting-after-exit": func(p *corev1.Pod) { + game := &p.Status.ContainerStatuses[0] + game.LastTerminationState.Terminated = &corev1.ContainerStateTerminated{ContainerID: game.ContainerID, ExitCode: 1} + game.ContainerID = "" + game.State = corev1.ContainerState{Waiting: &corev1.ContainerStateWaiting{Reason: "CrashLoopBackOff"}} + }, + "waiting-after-restart": func(p *corev1.Pod) { + game := &p.Status.ContainerStatuses[0] + game.RestartCount++ + game.ContainerID = "" + game.State = corev1.ContainerState{Waiting: &corev1.ContainerStateWaiting{Reason: "ContainerCreating"}} + }, "terminated": func(p *corev1.Pod) { p.Status.ContainerStatuses[0].State = corev1.ContainerState{Terminated: &corev1.ContainerStateTerminated{ExitCode: 1}} }, @@ -109,8 +127,16 @@ func TestBackupGameContainerChangesAbortAndDrain(t *testing.T) { if err := r.Get(t.Context(), client.ObjectKeyFromObject(&gs), &gs); err != nil { t.Fatal(err) } - if gs.Annotations[lifecycleOperationAnnotation] != "" || (stage != "before-quiesce" && q.unquiesced != 1) { - t.Fatalf("drained copy did not finish cleanup: owner=%q, agent=%+v", gs.Annotations[lifecycleOperationAnnotation], q) + unidentified := stage != "before-quiesce" && (name == "waiting" || name == "waiting-unrelated-exit" || name == "missing-id" || name == "missing-status") + if (gs.Annotations[lifecycleOperationAnnotation] != "") != unidentified || q.unquiesced != 0 { + t.Fatalf("wrong cleanup after worker drain: owner=%q, agent=%+v", gs.Annotations[lifecycleOperationAnnotation], q) + } + if stage != "before-quiesce" { + b = guardGetBackup(t, r, "first") + condition := meta.FindStatusCondition(b.Status.Conditions, "Unquiesced") + if condition == nil || (unidentified && condition.Status != metav1.ConditionFalse) || (!unidentified && condition.Reason != "SourceProcessGone") { + t.Fatalf("wrong original-process cleanup condition: %+v", condition) + } } }) } diff --git a/operator/internal/controller/backup_controller.go b/operator/internal/controller/backup_controller.go index 694429e53..9f61106a8 100644 --- a/operator/internal/controller/backup_controller.go +++ b/operator/internal/controller/backup_controller.go @@ -404,7 +404,8 @@ func (r *BackupReconciler) maybeQuiesce(ctx context.Context, b *gameplanev1alpha } return nil } - if state := b.Annotations[annoQuiesceAttempted]; state != "" && state != "pending" { + state := b.Annotations[annoQuiesceAttempted] + if state != "" && state != "pending" { return nil } // Verify the versioned endpoint before creating the first cleanup debt. @@ -447,9 +448,9 @@ func (r *BackupReconciler) maybeQuiesce(ctx context.Context, b *gameplanev1alpha annoQuiesceAttempted: "true", annoQuiescedAt: now, }) - case errors.Is(err, agent.ErrUnsupported): - // Game has no quiesce equivalent. Mark attempted and proceed — - // games like Valheim write to disk consistently anyway. + case errors.Is(err, agent.ErrUnsupported) && state != "pending": + // Only a first, explicitly unsupported operation proves that no + // command was issued. An ambiguous earlier attempt still owes resume. patchBackupAnnotations(b, map[string]string{ annoQuiesceAttempted: "unsupported", }) @@ -689,7 +690,7 @@ func (r *BackupReconciler) setUnquiescedCondition( cond.Reason = "Unquiesced" if b.Annotations[backupCleanupOutcomeAnnotation] == "source-gone" { cond.Reason = "SourceProcessGone" - cond.Message = "original process no longer exists; no unquiesce RPC was sent" + cond.Message = "original game process no longer exists" } } else { cond.Status = metav1.ConditionFalse @@ -805,11 +806,11 @@ func (r *BackupReconciler) unquiesceTargetGone(ctx context.Context, b *gameplane if err != nil { return false, err } - var podUID types.UID - if preparation != nil { - podUID = preparation.PodUID + if preparation == nil { + preparation = &backupPreparation{} } - return backupQuiesceProcessGone(ctx, r.backupAPIReader(), b.Namespace, targetName, podUID) + state, err := backupGameState(ctx, r.backupAPIReader(), b.Namespace, targetName, preparation.PodUID, preparation.GameContainerID, preparation.GameRestartCount) + return state == gameGone, err } // unquiesceRetryExpired records (on first call) when finalizeDelete's @@ -897,30 +898,20 @@ func (r *BackupReconciler) maybeUnquiesce(ctx context.Context, b *gameplanev1alp return fmt.Errorf("legacy quiesce cleanup is blocked by a protected backup") } } - if targetName != "" { - gone, err := r.unquiesceTargetGone(ctx, b) - if err != nil { - return err - } - if gone { - patchBackupAnnotations(b, map[string]string{backupCleanupOutcomeAnnotation: "source-gone"}) - return r.recordBackupUnquiesced(ctx, b) - } - } - if !agentQuiesceAvailable(r.AgentClient) { - return fmt.Errorf("cannot release quiesced world without an agent client") - } preparation, err := parseBackupPreparation(b) if err != nil { return err } - var podUID types.UID - if preparation != nil { - podUID = preparation.PodUID + if preparation == nil { + preparation = &backupPreparation{} } - if err := callBackupQuiesce(ctx, r.AgentClient, b.Namespace, targetName, podUID, true); err != nil { + gone, err := resumeBackupProcess(ctx, r.backupAPIReader(), r.AgentClient, b.Namespace, targetName, preparation.PodUID, preparation.GameContainerID, preparation.GameRestartCount) + if err != nil { return err } + if gone { + patchBackupAnnotations(b, map[string]string{backupCleanupOutcomeAnnotation: "source-gone"}) + } return r.recordBackupUnquiesced(ctx, b) } diff --git a/operator/internal/controller/backup_guard_test.go b/operator/internal/controller/backup_guard_test.go index c995b8b38..4aadc7634 100644 --- a/operator/internal/controller/backup_guard_test.go +++ b/operator/internal/controller/backup_guard_test.go @@ -316,34 +316,45 @@ func TestBackupGuardRolloutWaitsForLegacyWorkersAndCleanup(t *testing.T) { } func TestBackupGuardOrphanRecoveryWaitsForWorkers(t *testing.T) { - r, q := guardFixture(t, guardBackup("first"), guardBackup("second")) - guardPass(t, r, "first") - b := guardGetBackup(t, r, "first") - b.Finalizers = nil - if err := r.Update(context.Background(), b); err != nil { - t.Fatal(err) - } - if err := r.Delete(context.Background(), b); err != nil { - t.Fatal(err) - } - var job batchv1.Job - if err := r.Get(context.Background(), types.NamespacedName{Namespace: "ns", Name: "first"}, &job); err != nil { - t.Fatal(err) - } - pod := &corev1.Pod{ObjectMeta: metav1.ObjectMeta{Name: "orphan", Namespace: "ns", Labels: job.Spec.Template.Labels}, Status: corev1.PodStatus{Phase: corev1.PodRunning}} - if err := r.Create(context.Background(), pod); err != nil { - t.Fatal(err) - } - guardPass(t, r, "second") - if q.quiesced != 1 || q.unquiesced != 0 { - t.Fatalf("reclaimed before orphan drain: %+v", q) - } - if err := r.Delete(context.Background(), pod); err != nil { - t.Fatal(err) - } - guardPass(t, r, "second") - if q.quiesced != 2 || q.unquiesced != 1 { - t.Fatalf("orphan owner not recovered: %+v", q) + for _, replaced := range []bool{false, true} { + t.Run(fmt.Sprintf("game-replaced-%t", replaced), func(t *testing.T) { + r, q := guardFixture(t, guardBackup("first"), guardBackup("second")) + guardPass(t, r, "first") + b := guardGetBackup(t, r, "first") + b.Finalizers = nil + if err := r.Update(context.Background(), b); err != nil { + t.Fatal(err) + } + if err := r.Delete(context.Background(), b); err != nil { + t.Fatal(err) + } + var job batchv1.Job + if err := r.Get(context.Background(), types.NamespacedName{Namespace: "ns", Name: "first"}, &job); err != nil { + t.Fatal(err) + } + pod := &corev1.Pod{ObjectMeta: metav1.ObjectMeta{Name: "orphan", Namespace: "ns", Labels: job.Spec.Template.Labels}, Status: corev1.PodStatus{Phase: corev1.PodRunning}} + if err := r.Create(context.Background(), pod); err != nil { + t.Fatal(err) + } + guardPass(t, r, "second") + if q.quiesced != 1 || q.unquiesced != 0 { + t.Fatalf("reclaimed before orphan drain: %+v", q) + } + if err := r.Delete(context.Background(), pod); err != nil { + t.Fatal(err) + } + wantResume := 1 + if replaced { + changeBackupGamePod(t, r, backupGameChanges()["container-id"]) + wantResume = 0 + } + // Recovery must use the persisted predecessor identity after restart. + r = &BackupReconciler{Client: r.Client, Scheme: r.Scheme, AgentClient: q, LogReader: r.LogReader} + guardPass(t, r, "second") + if q.quiesced != 2 || q.unquiesced != wantResume { + t.Fatalf("orphan owner not recovered: %+v", q) + } + }) } } @@ -485,6 +496,17 @@ func TestBackupQuiesceIntentSurvivesPostAgentWriteFailure(t *testing.T) { if b.Annotations[annoQuiesceAttempted] != "pending" { t.Fatal("cleanup intent was not durable before save-off") } + // A fresh reconciler reads the pending intent. An unsupported retry + // describes that retry only; the previous save-off may have succeeded. + r = &BackupReconciler{Client: base, Scheme: r.Scheme, AgentClient: q, LogReader: r.LogReader} + q.quiesceErr = agent.ErrUnsupported + if err := r.maybeQuiesce(t.Context(), b); err == nil { + t.Fatal("unsupported retry erased an ambiguous pause") + } + b = guardGetBackup(t, r, "first") + if b.Annotations[annoQuiesceAttempted] != "pending" || b.Status.Phase != gameplanev1alpha1.BackupPhaseFailed { + t.Fatalf("lost pending cleanup across restart: %+v", b) + } if err := r.Delete(context.Background(), b); err != nil { t.Fatal(err) } @@ -744,7 +766,7 @@ func TestBackupResticCleanupWithoutSnapshotScheme(t *testing.T) { b.Status.Phase = gameplanev1alpha1.BackupPhaseSucceeded b.Status.SnapshotID = "abc123" q := &scrapeQuiescer{} - r := newScrapeReconciler(t, b, nil, time.Hour, q) + r := newLiveScrapeReconciler(t, b, nil, time.Hour, q) r.Scheme = nil res, err := r.Reconcile(context.Background(), ctrl.Request{NamespacedName: client.ObjectKeyFromObject(b)}) if err != nil || res.RequeueAfter > 0 || q.unquiesced != 1 { diff --git a/operator/internal/controller/backup_preparation.go b/operator/internal/controller/backup_preparation.go index 1993d0500..15a8c257f 100644 --- a/operator/internal/controller/backup_preparation.go +++ b/operator/internal/controller/backup_preparation.go @@ -289,16 +289,86 @@ func (r *BackupReconciler) backupSourceDrained(ctx context.Context, b *gameplane // unready pod, or terminating pod with the same UID is never proof of exit. // Empty UID is legacy state: only absence or an observed terminal pod suffices. func backupQuiesceProcessGone(ctx context.Context, reader client.Reader, namespace, server string, uid types.UID) (bool, error) { + state, err := backupGameState(ctx, reader, namespace, server, uid, "", nil) + return state == gameGone, err +} + +type gameState int + +const ( + gameUnknown gameState = iota + gameOriginal + gameGone +) + +// A bound game container may restart without replacing the pod. Missing +// kubelet status is ambiguous. Never send save-on to an unidentified game. +func backupGameState(ctx context.Context, reader client.Reader, namespace, server string, uid types.UID, containerID string, restarts *int32) (gameState, error) { var pod corev1.Pod err := reader.Get(ctx, client.ObjectKey{Namespace: namespace, Name: server + "-0"}, &pod) if apierrors.IsNotFound(err) { - return true, nil + return gameGone, nil } if err != nil { - return false, err + return gameUnknown, err } if uid != "" && pod.UID != uid { - return true, nil + return gameGone, nil + } + if pod.DeletionTimestamp.IsZero() && (pod.Status.Phase == corev1.PodSucceeded || pod.Status.Phase == corev1.PodFailed) { + return gameGone, nil + } + // Pre-upgrade owners can lack a container fingerprint. Preserve their + // pod-bound cleanup rules and never invent a fingerprint from current state. + if uid == "" || (containerID == "" && restarts == nil) { + return gameOriginal, nil + } + for _, game := range pod.Status.ContainerStatuses { + if game.Name != "game" { + continue + } + if restarts != nil && game.RestartCount > *restarts { + return gameGone, nil + } + if last := game.LastTerminationState.Terminated; last != nil && containerID != "" && last.ContainerID == containerID { + return gameGone, nil + } + if game.ContainerID == "" { + continue + } + if game.State.Terminated != nil { + return gameGone, nil + } + if game.State.Running != nil { + if containerID != "" && game.ContainerID != containerID { + return gameGone, nil + } + if restarts == nil || game.RestartCount == *restarts { + return gameOriginal, nil + } + } + } + return gameUnknown, nil +} + +// Confirm the same process both sides of the verified RPC. Cleanup callers +// record a replacement as process exit, independently of a successful reply. +func resumeBackupProcess(ctx context.Context, reader client.Reader, q AgentQuiescer, ns, server string, uid types.UID, containerID string, restarts *int32) (bool, error) { + if server == "" { + return false, fmt.Errorf("cleanup target has no server name") + } + check := func() (bool, error) { + state, err := backupGameState(ctx, reader, ns, server, uid, containerID, restarts) + if err == nil && state == gameUnknown { + err = fmt.Errorf("original game container state is unavailable") + } + return state == gameGone, err + } + if gone, err := check(); gone || err != nil { + return gone, err + } + if err := callBackupQuiesce(ctx, q, ns, server, uid, true); err != nil { + return false, err } - return pod.DeletionTimestamp.IsZero() && (pod.Status.Phase == corev1.PodSucceeded || pod.Status.Phase == corev1.PodFailed), nil + return check() } diff --git a/operator/internal/controller/backup_quiesce_lifecycle_test.go b/operator/internal/controller/backup_quiesce_lifecycle_test.go index c1c7d90e1..120469567 100644 --- a/operator/internal/controller/backup_quiesce_lifecycle_test.go +++ b/operator/internal/controller/backup_quiesce_lifecycle_test.go @@ -46,7 +46,7 @@ func TestReconcile_RetriesUnquiesceOnTerminalBackup(t *testing.T) { b.Status.SnapshotID = "snap-x" q := &scrapeQuiescer{unquiesceErr: errors.New("agent unreachable")} - r := newScrapeReconciler(t, b, nil, time.Hour, q) + r := newLiveScrapeReconciler(t, b, nil, time.Hour, q) req := ctrl.Request{NamespacedName: types.NamespacedName{Namespace: b.Namespace, Name: b.Name}} res, err := r.Reconcile(context.Background(), req) diff --git a/operator/internal/controller/backup_snapshot_scrape_test.go b/operator/internal/controller/backup_snapshot_scrape_test.go index 4c0d4e01c..bfe302b17 100644 --- a/operator/internal/controller/backup_snapshot_scrape_test.go +++ b/operator/internal/controller/backup_snapshot_scrape_test.go @@ -12,7 +12,7 @@ import ( corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" - "sigs.k8s.io/controller-runtime/pkg/client/fake" + "sigs.k8s.io/controller-runtime/pkg/client" gameplanev1alpha1 "github.com/GameplanePanel/gameplane/operator/api/v1alpha1" ) @@ -100,20 +100,30 @@ func succeededJob(completedAgo time.Duration) *batchv1.Job { } func newScrapeReconciler( + t *testing.T, b *gameplanev1alpha1.Backup, lr BackupLogReader, + grace time.Duration, agent AgentQuiescer, extra ...client.Object, +) *BackupReconciler { + t.Helper() + r := newScrapeReconcilerWithObjects(t, b, agent, extra...) + r.LogReader = lr + r.SnapshotScrapeGracePeriod = grace + return r +} + +// A live cleanup test must observe the same pod that the Backup recorded. +// Otherwise the reconciler must withhold save-on or retire the original process. +func newLiveScrapeReconciler( t *testing.T, b *gameplanev1alpha1.Backup, lr BackupLogReader, grace time.Duration, agent AgentQuiescer, ) *BackupReconciler { t.Helper() - s := scrapeScheme(t) - cl := fake.NewClientBuilder().WithScheme(s). - WithObjects(b). - WithStatusSubresource(&gameplanev1alpha1.Backup{}). - Build() - r := &BackupReconciler{Client: cl, Scheme: s, LogReader: lr, SnapshotScrapeGracePeriod: grace} - if agent != nil { - r.AgentClient = agent + b.Spec.ServerRef.Name = "server" + gs := &gameplanev1alpha1.GameServer{ObjectMeta: metav1.ObjectMeta{Name: "server", Namespace: b.Namespace, UID: "server-uid"}} + pod := &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{Name: "server-0", Namespace: b.Namespace, UID: "game-pod-uid"}, + Status: corev1.PodStatus{Phase: corev1.PodRunning, ContainerStatuses: []corev1.ContainerStatus{backupRunningGameStatus()}}, } - return r + return newScrapeReconciler(t, b, lr, grace, agent, gs, pod) } // A successful Job whose logs carry a summary line resolves to Succeeded with @@ -188,7 +198,7 @@ func TestMirrorJobStatus_FailsWhenSnapshotUnavailablePastGrace(t *testing.T) { func TestMirrorJobStatus_UnquiescesBeforeFailingPastGrace(t *testing.T) { b := quiescedBackup() q := &scrapeQuiescer{} - r := newScrapeReconciler(t, b, scrapeLogReader{body: ""}, time.Millisecond, q) + r := newLiveScrapeReconciler(t, b, scrapeLogReader{body: ""}, time.Millisecond, q) if _, err := r.mirrorJobStatus(context.Background(), b, succeededJob(time.Minute)); err != nil { t.Fatalf("mirrorJobStatus: %v", err) @@ -206,7 +216,7 @@ func TestMirrorJobStatus_UnquiescesBeforeFailingPastGrace(t *testing.T) { func TestMirrorJobStatus_DoesNotFailWhileUnquiesceIsPending(t *testing.T) { b := quiescedBackup() q := &scrapeQuiescer{unquiesceErr: errors.New("agent unreachable")} - r := newScrapeReconciler(t, b, scrapeLogReader{body: ""}, time.Millisecond, q) + r := newLiveScrapeReconciler(t, b, scrapeLogReader{body: ""}, time.Millisecond, q) res, err := r.mirrorJobStatus(context.Background(), b, succeededJob(time.Minute)) if err != nil { @@ -227,7 +237,7 @@ func TestRetrySnapshotScrape_FailsWhenJobIsGone(t *testing.T) { b := quiescedBackup() b.Status.Phase = gameplanev1alpha1.BackupPhaseSucceeded q := &scrapeQuiescer{} - r := newScrapeReconciler(t, b, scrapeLogReader{body: ""}, time.Hour, q) + r := newLiveScrapeReconciler(t, b, scrapeLogReader{body: ""}, time.Hour, q) if _, err := r.retrySnapshotScrape(context.Background(), b); err != nil { t.Fatalf("retrySnapshotScrape: %v", err) diff --git a/operator/internal/controller/backup_stopped_test.go b/operator/internal/controller/backup_stopped_test.go index a61c43712..ab3d5a522 100644 --- a/operator/internal/controller/backup_stopped_test.go +++ b/operator/internal/controller/backup_stopped_test.go @@ -212,7 +212,7 @@ func TestBackupLiveTimeoutRetainsOwnerUntilOriginalPodGone(t *testing.T) { t.Fatal(err) } op, _, err := lifecycleOwner(&gs) - if err != nil || op.QuiescePodUID != "game-pod-uid" || !op.NeedsUnquiesce { + if err != nil || op.QuiescePodUID != "game-pod-uid" || op.QuiesceContainerID != backupRunningGameStatus().ContainerID || op.QuiesceRestartCount == nil || *op.QuiesceRestartCount != 0 || !op.NeedsUnquiesce { t.Fatalf("cleanup identity not durable: %+v %v", op, err) } guardPass(t, r, "first") diff --git a/operator/internal/controller/lifecycle_operation.go b/operator/internal/controller/lifecycle_operation.go index 20aff9bbb..261c06cee 100644 --- a/operator/internal/controller/lifecycle_operation.go +++ b/operator/internal/controller/lifecycle_operation.go @@ -11,6 +11,7 @@ import ( apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" + "k8s.io/utils/ptr" "sigs.k8s.io/controller-runtime/pkg/client" gameplanev1alpha1 "github.com/GameplanePanel/gameplane/operator/api/v1alpha1" @@ -27,15 +28,17 @@ type lifecycleIdentity struct { // PreservePower allows v2 restores to stop with the guard, without altering user suspend intent. PreservePower bool `json:"preservePower,omitempty"` - StoppedBackup *stoppedBackup `json:"stoppedBackup,omitempty"` - Kind string `json:"kind"` - Name string `json:"name"` - UID types.UID `json:"uid"` - TargetUID types.UID `json:"targetUID"` - Generation int64 `json:"generation"` - Token string `json:"token,omitempty"` - NeedsUnquiesce bool `json:"needsUnquiesce,omitempty"` - QuiescePodUID types.UID `json:"quiescePodUID,omitempty"` + StoppedBackup *stoppedBackup `json:"stoppedBackup,omitempty"` + Kind string `json:"kind"` + Name string `json:"name"` + UID types.UID `json:"uid"` + TargetUID types.UID `json:"targetUID"` + Generation int64 `json:"generation"` + Token string `json:"token,omitempty"` + NeedsUnquiesce bool `json:"needsUnquiesce,omitempty"` + QuiescePodUID types.UID `json:"quiescePodUID,omitempty"` + QuiesceContainerID string `json:"quiesceContainerID,omitempty"` + QuiesceRestartCount *int32 `json:"quiesceRestartCount,omitempty"` // InitialStartup permits only the first workload of a CSI restore's new // target (including WaitForFirstConsumer PVCs), while other operations wait. InitialStartup bool `json:"initialStartup,omitempty"` @@ -323,17 +326,7 @@ func recoverLifecycleOperation(ctx context.Context, c client.Client, reader clie if !predecessor.NeedsUnquiesce { continue } - gone, err := backupQuiesceProcessGone(ctx, reader, gs.Namespace, gs.Name, predecessor.QuiescePodUID) - if err != nil { - return false, err - } - if gone { - continue - } - if !agentQuiesceAvailable(quiescer) { - return false, fmt.Errorf("cannot recover orphaned quiesce without an agent client") - } - if err := callBackupQuiesce(ctx, quiescer, gs.Namespace, gs.Name, predecessor.QuiescePodUID, true); err != nil { + if _, err := resumeBackupProcess(ctx, reader, quiescer, gs.Namespace, gs.Name, predecessor.QuiescePodUID, predecessor.QuiesceContainerID, predecessor.QuiesceRestartCount); err != nil { return false, fmt.Errorf("recover orphaned lifecycle quiesce: %w", err) } } @@ -385,23 +378,30 @@ func (r *BackupReconciler) setLifecycleUnquiesceIntent(ctx context.Context, b *g if err != nil { return err } - var podUID types.UID if needed { - preparation, err := parseBackupPreparation(b) + prep, err := parseBackupPreparation(b) if err != nil { return err } - if preparation != nil { - podUID = preparation.PodUID + if prep != nil { + if op.NeedsUnquiesce && op.QuiescePodUID != "" && op.QuiescePodUID != prep.PodUID { + return fmt.Errorf("cleanup pod identity changed") + } + if op.NeedsUnquiesce && op.QuiesceContainerID != "" && + (op.QuiesceContainerID != prep.GameContainerID || !ptr.Equal(op.QuiesceRestartCount, prep.GameRestartCount)) { + return fmt.Errorf("cleanup game incarnation changed") + } + if op.NeedsUnquiesce && op.QuiescePodUID == prep.PodUID && op.QuiesceContainerID == prep.GameContainerID && ptr.Equal(op.QuiesceRestartCount, prep.GameRestartCount) { + return nil + } + op.QuiescePodUID = prep.PodUID + op.QuiesceContainerID = prep.GameContainerID + op.QuiesceRestartCount = prep.GameRestartCount } - } - if op.NeedsUnquiesce == needed && (!needed || op.QuiescePodUID == podUID) { + } else if !op.NeedsUnquiesce { return nil } op.NeedsUnquiesce = needed - if needed { - op.QuiescePodUID = podUID - } gs.Annotations[lifecycleOperationAnnotation] = lifecycleOperationJSON(op) return r.Update(ctx, gs) } diff --git a/operator/specs.md b/operator/specs.md index ad0254e01..6dcea63c7 100644 --- a/operator/specs.md +++ b/operator/specs.md @@ -430,7 +430,7 @@ Primary reconcilers register with the manager in `cmd/main.go` and handle CRD li - Preparation defaults to 10m and live quiesce attempts to 5m (`--backup-preparation-timeout`, `--backup-quiesce-timeout`; matching Helm `operator.backupPreparationTimeout`, `operator.backupQuiesceTimeout`). Deadlines are annotations written once and survive restart/configuration changes, including when an attempted quiesce is unsupported. Copies requested raw, without an agent, or from an already-stopped source retain the full Job deadline. The waiting shell validates token expiry and bounds Restic with `timeout -s TERM -k 5`. On expiry/deletion, invalidation and token revocation precede foreground cancellation; actual live/terminating workers still block save-on and owner release. A timeout is not evidence of drainage. Existing pre-upgrade Jobs retain their observation protocol. - Initial source/fence waits share an existing preparation deadline. CSI captures and older Jobs without one persist a separate two-minute source-startup deadline after acquiring lifecycle ownership; queue age and restarts do not spend or reset that budget. Expired unavailable sources fail visibly and enter normal cleanup. A foreign same-name worker is awaited before acquiring ownership or quiescing. Existing owned workers continue through their original gate/completion checks without resolving the template again; template resolution failure when constructing a new worker aborts and releases only after drainage and save-on. - With quiesce and an agent client enabled, preparation is recorded once in `backup.gameplane.local/preparation`, bound to the target UID and StatefulSet UID/generation. `already-stopped` requires uncached proof of desired and observed zero replicas, caught-up observed generation, and no live game/PVC user pods or pending writer Jobs. Current copy workers are excluded by Job ownership. This path sends no agent calls and preserves suspend/idle/wake intent under the shared workload fence. It applies equally to restic and CSI copies. - - Live preparation also pins the game Pod UID. The operator checks the agent's read-only `/v1/pods/{uid}/identity` endpoint before recording the first cleanup intent, then uses pod-bound quiesce/unquiesce routes; the agent rejects a different UID before RCON. A missing endpoint or identity mismatch fails closed without a legacy-route fallback. Ambiguous quiesce errors retain cleanup intent and the original Pod UID on both the Backup and shared owner. Only confirmed save-on or uncached evidence that the original pod is absent/replaced/terminal permits cleanup; an HTTP timeout, unready pod, or terminating pod is insufficient. Legacy attempts without a UID retire only on absence/terminal proof and never adopt a new UID. `Unquiesced=True, Reason=SourceProcessGone` distinguishes process exit from an actual save-on RPC. + - Live preparation also pins the game Pod UID. The operator checks the agent's read-only `/v2/pods/{uid}/identity` endpoint before recording the first cleanup intent, then uses pod-bound quiesce/unquiesce routes; the agent rejects a different UID before RCON. A missing endpoint or identity mismatch fails closed without a legacy-route fallback. Ambiguous quiesce errors retain cleanup intent and the original Pod UID on both the Backup and shared owner. The v2 response must include the matching pause/resume outcome and boolean. A resume checks the pinned container before and after the RPC. The GameServer owner carries the original container ID and restart count through ownership transfer. Only a confirmed save-on or uncached evidence of original process exit (including replacement within the same Pod) permits cleanup; an HTTP timeout, unready pod, or terminating pod is insufficient. Legacy attempts without a UID retire only on absence/terminal proof and never adopt a new UID. `Unquiesced=True, Reason=SourceProcessGone` distinguishes process exit from an actual save-on RPC. - Live source proof also records the running `game` container's ContainerID and RestartCount, independently of the agent container. It is checked before and after quiesce and on each copy reconcile, including completion. A changed incarnation or missing/non-running game status after binding invalidates the capture even when the Pod UID and phase remain unchanged. Initial unavailable container evidence waits within the existing persisted startup/preparation deadline. In-progress legacy live preparations without this proof fail conservatively without backfilling an incarnation after quiesce; they remain parseable for cleanup, and completed historical manifests are unchanged. Observed workload/process replacement fails the Backup before it can report a restorable success. Owner recovery retains the pod UID even if the Backup is force-deleted, drains predecessor workers, and never sends its cleanup to a successor pod. - Imported legacy recovery records and backups without a recorded Pod UID cannot call unbound save-on, even if an agent is available. Their cleanup intent remains durable until an uncached read proves the game Pod absent or terminal and not terminating; a live or unready replacement is not exit proof. - Upgrade the operator and agent image together and allow existing GameServer pods to roll out the new agent and `GAMEPLANE_POD_UID` DownwardAPI env. Older agents fail the identity preflight before cleanup debt, so the workload can continue its upgrade after terminal cleanup releases the owner. Retry failed copies after rollout; preflight never bypasses protection. This does not automatically stop a running game. diff --git a/test/e2e/gamebot_helpers_e2e_test.go b/test/e2e/gamebot_helpers_e2e_test.go index 31f28c4a3..58aee501b 100644 --- a/test/e2e/gamebot_helpers_e2e_test.go +++ b/test/e2e/gamebot_helpers_e2e_test.go @@ -158,13 +158,14 @@ type gameBotSpec struct { RCON map[string]any // spec.rcon ConsoleMode string // spec.consoleMode Actions []any // spec.capabilities.actions; nil means no actions + Quiesce map[string]any // optional backup declaration Control pathAControl // Path A: drive the server through Gameplane (optional) } // runGameBotTest creates the GameTemplate + GameServer, waits for // status.phase == Running, then runs the in-cluster probe. It expects // the probe to reach ExpectDepth or the test fails. -func runGameBotTest(t *testing.T, s gameBotSpec) { +func runGameBotTest(t *testing.T, s gameBotSpec) (string, string) { t.Helper() // Enforcement: verify the test name suffix matches the asserted depth. @@ -253,6 +254,7 @@ func runGameBotTest(t *testing.T, s gameBotSpec) { if s.Control.Mode != "" { runGameBotPathA(t, gsName, ns, s.Control) } + return gsName, ns } // createGameBotServer creates the GameTemplate and the GameServer described by s @@ -323,10 +325,15 @@ func createGameBotServer(t *testing.T, env *Env, s gameBotSpec) (gsName, namespa if s.ConsoleMode != "" { spec["consoleMode"] = s.ConsoleMode } + capabilities := map[string]any{} if s.Actions != nil { - spec["capabilities"] = map[string]any{ - "actions": s.Actions, - } + capabilities["actions"] = s.Actions + } + if s.Quiesce != nil { + capabilities["quiesce"] = s.Quiesce + } + if len(capabilities) > 0 { + spec["capabilities"] = capabilities } tmpl := &unstructured.Unstructured{Object: map[string]any{ diff --git a/test/e2e/minecraft_bot_e2e_test.go b/test/e2e/minecraft_bot_e2e_test.go index 5d7253ec4..ecd04f105 100644 --- a/test/e2e/minecraft_bot_e2e_test.go +++ b/test/e2e/minecraft_bot_e2e_test.go @@ -3,9 +3,14 @@ package e2e import ( + "fmt" + "strings" "testing" "time" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "github.com/GameplanePanel/gameplane/test/e2e/internal/protocol/joindepth" ) @@ -45,7 +50,15 @@ import ( func TestGameServer_MinecraftJavaBot_Joined(t *testing.T) { skipUnlessGameInScope(t, "minecraft-java") - runGameBotTest(t, minecraftBotSpec()) + spec := minecraftBotSpec() + spec.Env["PAUSE_WHEN_EMPTY_SECONDS"] = "-1" + // The image creates private server files under /data. Use the same + // unprivileged identity as the Restic reader for this local-path PVC. + spec.Env["UID"] = "65532" + spec.Env["GID"] = "65532" + spec.Quiesce = map[string]any{"quiesce": []any{"save-off", "save-all flush"}, "unquiesce": []any{"save-on"}} + gs, ns := runGameBotTest(t, spec) + verifyMinecraftBackupResumesAutosave(t, ns, gs) } // minecraftBotSpec is the trimmed vanilla Minecraft Java 1.21.4 GameServer used by @@ -139,3 +152,52 @@ func minecraftBotSpec() gameBotSpec { }, } } + +// Write a new command-storage marker after recovery. Only an automated save +// can put it on disk: this probe issues neither save-all nor a server stop. +func verifyMinecraftBackupResumesAutosave(t *testing.T, ns, gs string) { + t.Helper() + ensureResticRepo(t) + name := gs + "-autosave" + b := &unstructured.Unstructured{Object: map[string]any{ + "apiVersion": "gameplane.local/v1alpha1", "kind": "Backup", + "metadata": map[string]any{"namespace": ns, "name": name}, + "spec": map[string]any{"serverRef": map[string]any{"name": gs}, "repoRef": map[string]any{"name": "e2e-restic-creds", "key": "repo"}, "quiesce": true}, + }} + backups := envInstance.Dyn.Resource(backupGVR).Namespace(ns) + if _, err := backups.Create(t.Context(), b, metav1.CreateOptions{}); err != nil { + t.Fatal(err) + } + t.Cleanup(func() { deleteBackupCR(t, ns, name) }) + defer func() { + if t.Failed() { + logs, err := envInstance.Kubectl(t.Context(), "logs", "-n", ns, "-l", "job-name="+name, "--all-containers", "--prefix", "--tail=25") + t.Logf("backup worker logs before cleanup: %s (%v)", logs, err) + } + }() + waitBackupSucceeded(t, ns, name, 8*time.Minute) + envInstance.Eventually(t, time.Minute, func() (bool, string) { + got, err := backups.Get(t.Context(), name, metav1.GetOptions{}) + if err != nil { + return false, err.Error() + } + a := got.GetAnnotations() + return a["backup.gameplane.local/quiesce-attempted"] == "true" && + a["backup.gameplane.local/unquiesced-at"] != "" && a["backup.gameplane.local/cleanup-outcome"] != "source-gone", "waiting for pause and resume of the live game" + }) + exec := func(args ...string) (string, error) { + return envInstance.Kubectl(t.Context(), append([]string{"exec", "-n", ns, "pod/" + gs + "-0", "-c", "game", "--"}, args...)...) + } + // An old command storage file cannot satisfy a new marker. + marker := fmt.Sprintf("gameplane_autosave_%d", time.Now().UnixNano()) + if out, err := exec("rcon-cli", "data modify storage gpaudit:backup marker set value "+marker); err != nil || !strings.Contains(out, "Modified storage") { + t.Fatalf("set post-backup marker: %s (%v)", out, err) + } + if out, err := exec("rcon-cli", "tick sprint 7000t"); err != nil { + t.Fatalf("advance vanilla autosave ticks: %s (%v)", out, err) + } + envInstance.Eventually(t, 7*time.Minute, func() (bool, string) { + out, err := exec("gzip", "-dc", "/data/world/data/command_storage_gpaudit.dat") + return err == nil && strings.Contains(out, marker), "waiting for automatic persistence of the post-backup marker" + }) +}