From 4cad0f0cceac6d414b7f363bf27c99f2b9b7c154 Mon Sep 17 00:00:00 2001 From: Josh Creek <8179928+jcreek@users.noreply.github.com> Date: Tue, 1 Sep 2026 13:44:16 +0100 Subject: [PATCH] feat(multiplayer): report assignment-ready from the supervisor Closes the second blocker named last commit. Re-traced the actual code path rather than trusting the earlier assumption: server_boot.gd verifies its mounted roster file synchronously in _ready(), before NetworkManager.host() runs and before ServerControl.set_process_ready is ever called -- so by the time the loopback /ready probe (and thus Agones Ready, and thus process-ready registration) succeeds, Godot has already verified its own roster. And the API's ASSIGNMENT_READY gate (AdvanceServerRegistrationSQL) checks only durable `assignments` rows server-side, nothing Godot reports. No new Godot-side state was needed -- the earlier 'needs Godot's own roster-verification state exposed' claim was overcautious and is corrected here. The supervisor now calls registerControlPlane(ctx, true) right after process-ready succeeds, with a bounded retry (default 5 attempts, 2s apart, both configurable) rather than a single attempt: the durable `assignments` rows the server-side gate checks may not have propagated by the first attempt, and that is expected, not fatal. Unlike a process-ready registration failure, a persistent assignment-ready failure does NOT kill the child -- the process is already legitimately listening and usable, and killing a healthy process over a lagging control-plane read would be actively harmful; it's logged to stderr instead. Covered by two tests: the full process-ready-then-assignment-ready sequence and body shapes, and a retry test that fails the assignment- ready call twice with 409 (simulating the real gate not yet satisfied) before succeeding on the third attempt, asserting Start() still succeeds and the child is never killed. --- server/cmd/game-server-supervisor/main.go | 5 ++ server/supervisor/supervisor.go | 44 ++++++++++++ server/supervisor/supervisor_test.go | 83 +++++++++++++++++++++-- 3 files changed, 127 insertions(+), 5 deletions(-) diff --git a/server/cmd/game-server-supervisor/main.go b/server/cmd/game-server-supervisor/main.go index b273306b..6a335a82 100644 --- a/server/cmd/game-server-supervisor/main.go +++ b/server/cmd/game-server-supervisor/main.go @@ -47,6 +47,8 @@ func main() { matchIDEnv := options.String("match-id-env", "COSMIC_CLASH_MATCH_ID", "environment variable containing the allocated match ID; if unset/empty, falls back to the cosmic-clash.io/match-id annotation on the allocated GameServer") protocolVersion := options.Int("protocol-version", 0, "protocol version reported at registration") imageDigestEnv := options.String("image-digest-env", "COSMIC_CLASH_IMAGE_DIGEST", "environment variable containing this build's sha256 image digest") + assignmentReadyAttempts := options.Int("assignment-ready-attempts", 5, "retry attempts for assignment-ready registration after process-ready succeeds (a slow-to-propagate signed roster is not fatal)") + assignmentReadyBackoff := options.Duration("assignment-ready-backoff", 2*time.Second, "delay between assignment-ready retry attempts") if err := options.Parse(args[:separator]); err != nil { os.Exit(2) } @@ -70,6 +72,9 @@ func main() { MatchID: os.Getenv(*matchIDEnv), ProtocolVersion: *protocolVersion, ImageDigest: os.Getenv(*imageDigestEnv), + + AssignmentReadyAttempts: *assignmentReadyAttempts, + AssignmentReadyBackoff: *assignmentReadyBackoff, }) if err != nil { fmt.Fprintf(os.Stderr, "game-server-supervisor: %v\n", err) diff --git a/server/supervisor/supervisor.go b/server/supervisor/supervisor.go index 5e4c00e6..4b156915 100644 --- a/server/supervisor/supervisor.go +++ b/server/supervisor/supervisor.go @@ -70,6 +70,18 @@ type Config struct { MatchID string ProtocolVersion int ImageDigest string + + // AssignmentReadyAttempts/AssignmentReadyBackoff bound the retry loop for + // reporting assignment-ready once process-ready has already succeeded. + // The control-plane's own durable gate (every participant already + // holding a live, unexpired assignment -- see + // AdvanceServerRegistrationSQL) may not be satisfied on the very first + // attempt if the signed roster is still propagating, and that is + // expected, not fatal: unlike a process-ready registration failure, this + // does not kill the child, since the process is already legitimately + // listening and usable either way. Default 5 attempts, 2s apart. + AssignmentReadyAttempts int + AssignmentReadyBackoff time.Duration } type Supervisor struct { @@ -91,6 +103,12 @@ func New(config Config) (*Supervisor, error) { if config.PollInterval <= 0 { config.PollInterval = 100 * time.Millisecond } + if config.AssignmentReadyAttempts <= 0 { + config.AssignmentReadyAttempts = 5 + } + if config.AssignmentReadyBackoff <= 0 { + config.AssignmentReadyBackoff = 2 * time.Second + } if config.Transport == "" { config.Transport = "enet" } @@ -176,9 +194,35 @@ func (s *Supervisor) Start(ctx context.Context) error { _ = s.cmd.Process.Kill() return err } + s.reportAssignmentReady(ctx) return nil } +// reportAssignmentReady is best-effort: process-ready has already succeeded, +// so the process is legitimately usable either way. A persistent failure is +// written to stderr rather than returned, since treating it as fatal would +// kill a perfectly healthy process over what is usually just the signed +// roster's durable rows not having propagated yet. +func (s *Supervisor) reportAssignmentReady(ctx context.Context) { + if s.config.ControlPlaneURL == "" { + return + } + var lastErr error + for attempt := 0; attempt < s.config.AssignmentReadyAttempts; attempt++ { + if attempt > 0 { + select { + case <-ctx.Done(): + return + case <-time.After(s.config.AssignmentReadyBackoff): + } + } + if lastErr = s.registerControlPlane(ctx, true); lastErr == nil { + return + } + } + fmt.Fprintf(os.Stderr, "game-server-supervisor: assignment-ready registration did not succeed after %d attempts: %v\n", s.config.AssignmentReadyAttempts, lastErr) +} + // registerControlPlane reports the allocated process's readiness to the // matchmaking control plane (POST /v1/servers/{id}/register). It is a no-op // whenever ControlPlaneURL is unset, which is the default and preserves diff --git a/server/supervisor/supervisor_test.go b/server/supervisor/supervisor_test.go index f7b5e3d2..dafb58b4 100644 --- a/server/supervisor/supervisor_test.go +++ b/server/supervisor/supervisor_test.go @@ -8,6 +8,7 @@ import ( "os" "path/filepath" "strings" + "sync" "testing" "time" ) @@ -79,8 +80,10 @@ func TestControlPlaneRegistrationRejectsIncompleteConfig(t *testing.T) { } } -func TestControlPlaneRegistrationReportsProcessReadyWithWorkloadToken(t *testing.T) { - var gotAuth, gotIdempotency, gotBody string +func TestControlPlaneRegistrationReportsProcessReadyThenAssignmentReady(t *testing.T) { + var mu sync.Mutex + var gotAuth, gotIdempotency string + var bodies []string server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { switch { case r.URL.Path == "/gameserver": @@ -90,10 +93,12 @@ func TestControlPlaneRegistrationReportsProcessReadyWithWorkloadToken(t *testing case r.URL.Path == "/ready": w.WriteHeader(http.StatusOK) case r.URL.Path == "/v1/servers/server-1/register": + mu.Lock() gotAuth = r.Header.Get("Authorization") gotIdempotency = r.Header.Get("Idempotency-Key") body, _ := io.ReadAll(r.Body) - gotBody = string(body) + bodies = append(bodies, string(body)) + mu.Unlock() w.WriteHeader(http.StatusNoContent) default: w.WriteHeader(http.StatusNotFound) @@ -108,6 +113,7 @@ func TestControlPlaneRegistrationReportsProcessReadyWithWorkloadToken(t *testing s, err := New(Config{ Command: []string{"/bin/sh", "-c", "true"}, SDKBaseURL: server.URL, ReadyURL: server.URL + "/ready-probe", ReadyTimeout: time.Second, PollInterval: time.Millisecond, ControlPlaneURL: server.URL, WorkloadTokenPath: tokenPath, ServerID: "server-1", MatchID: "match-1", ProtocolVersion: 1, ImageDigest: "sha256:aa", + AssignmentReadyAttempts: 3, AssignmentReadyBackoff: time.Millisecond, }) if err != nil { t.Fatal(err) @@ -122,8 +128,75 @@ func TestControlPlaneRegistrationReportsProcessReadyWithWorkloadToken(t *testing if len(gotIdempotency) < 16 { t.Fatalf("Idempotency-Key = %q, too short", gotIdempotency) } - if !strings.Contains(gotBody, `"match_id":"match-1"`) || !strings.Contains(gotBody, `"assignment_ready":false`) || !strings.Contains(gotBody, `"image_digest":"sha256:aa"`) { - t.Fatalf("register body = %s", gotBody) + mu.Lock() + defer mu.Unlock() + if len(bodies) != 2 { + t.Fatalf("expected exactly 2 register calls (process-ready, assignment-ready), got %d: %v", len(bodies), bodies) + } + if !strings.Contains(bodies[0], `"match_id":"match-1"`) || !strings.Contains(bodies[0], `"assignment_ready":false`) || !strings.Contains(bodies[0], `"image_digest":"sha256:aa"`) { + t.Fatalf("process-ready register body = %s", bodies[0]) + } + if !strings.Contains(bodies[1], `"assignment_ready":true`) { + t.Fatalf("assignment-ready register body = %s", bodies[1]) + } +} + +func TestAssignmentReadyRegistrationRetriesUntilTheControlPlaneCatchesUp(t *testing.T) { + var mu sync.Mutex + assignmentReadyAttempts := 0 + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch { + case r.URL.Path == "/gameserver": + _, _ = w.Write([]byte(`{"status":{"address":"203.0.113.9","ports":[{"name":"game","port":31001}]}}`)) + case r.URL.Path == "/ready-probe", r.URL.Path == "/ready": + w.WriteHeader(http.StatusOK) + case r.URL.Path == "/v1/servers/server-1/register": + body, _ := io.ReadAll(r.Body) + if !strings.Contains(string(body), `"assignment_ready":true`) { + w.WriteHeader(http.StatusNoContent) + return + } + mu.Lock() + assignmentReadyAttempts++ + attempt := assignmentReadyAttempts + mu.Unlock() + if attempt < 3 { + // Simulates the durable `assignments` rows not having + // propagated yet -- the API's own real gate for this. + w.WriteHeader(http.StatusConflict) + return + } + w.WriteHeader(http.StatusNoContent) + default: + w.WriteHeader(http.StatusNotFound) + } + })) + defer server.Close() + + tokenPath := filepath.Join(t.TempDir(), "token") + if err := os.WriteFile(tokenPath, []byte("workload-jwt"), 0o600); err != nil { + t.Fatal(err) + } + s, err := New(Config{ + Command: []string{"/bin/sh", "-c", "true"}, SDKBaseURL: server.URL, ReadyURL: server.URL + "/ready-probe", ReadyTimeout: time.Second, PollInterval: time.Millisecond, + ControlPlaneURL: server.URL, WorkloadTokenPath: tokenPath, ServerID: "server-1", MatchID: "match-1", ProtocolVersion: 1, ImageDigest: "sha256:aa", + AssignmentReadyAttempts: 5, AssignmentReadyBackoff: time.Millisecond, + }) + if err != nil { + t.Fatal(err) + } + // Start must still succeed -- a slow-to-propagate assignment-ready must + // never be treated as a Start() failure (which would kill the child). + if err := s.Start(context.Background()); err != nil { + t.Fatalf("Start failed despite assignment-ready eventually succeeding: %v", err) + } + if err := s.Wait(); err != nil { + t.Fatalf("child was killed despite Start succeeding: %v", err) + } + mu.Lock() + defer mu.Unlock() + if assignmentReadyAttempts != 3 { + t.Fatalf("assignment-ready attempts = %d, want exactly 3 (2 conflicts then success)", assignmentReadyAttempts) } }