From bf396afcaf232802cc4d0eab686a979690baa885 Mon Sep 17 00:00:00 2001 From: Josh Creek <8179928+jcreek@users.noreply.github.com> Date: Tue, 1 Sep 2026 18:05:43 +0100 Subject: [PATCH] fix(multiplayer): recover ambiguous agones allocations --- multiplayer-next.md | 2 +- server/agones/allocation.go | 71 ++++++++++++++++++++++++++++++-- server/agones/allocation_test.go | 34 +++++++++++++++ server/allocator/service.go | 11 +++++ server/allocator/worker.go | 27 ++++++++++-- server/allocator/worker_test.go | 24 +++++++++++ 6 files changed, 161 insertions(+), 8 deletions(-) diff --git a/multiplayer-next.md b/multiplayer-next.md index 7ee989c3..fd475e0b 100644 --- a/multiplayer-next.md +++ b/multiplayer-next.md @@ -1219,7 +1219,7 @@ the local/CI/community transport, not a silent production fallback. | 8.27 `[D:8.26]` | **IN PROGRESS.** Go supervisor package provides local-safe Agones REST discovery, validates assigned address/port data, injects dynamic `SDR_LISTEN_PORT`/`SDR_IP`, performs explicit process-ready probing and Ready transition; direct mode bypasses Agones | `server/supervisor/` covers allocated/direct startup, invalid endpoint rejection, dynamic endpoint/Ready ordering and authenticated drain; allocated Godot now supplies a loopback readiness/drain control surface and `agones_sdk.gd` supplies sidecar Health/Ready/Shutdown/annotation REST operations; metadata watch, real Agones annotation/shutdown confirmation and emulator integration remain | | 8.28 `[D:8.6,8.27]` | **IN PROGRESS.** Supervisor separates explicit process-ready from Agones Ready and never scrapes stdout; allocated mode refuses to mark Ready without a configured readiness probe. The durable control-plane counterpart now exists: a workload-authenticated `POST /v1/servers/{id}/register` (and its `/api/v1` contract alias) advances a match's `ALLOCATING -> PROCESS_READY -> ASSIGNMENT_READY` state, and every participant's queue ticket with it, as one idempotent SERIALIZABLE transaction, gated on every participant already holding a live, unexpired assignment. The supervisor now calls it: once Agones Ready succeeds, it POSTs process-ready (`assignment_ready=false`) using a workload token read fresh from disk each call (matching kubelet's in-place rotation of a projected service-account token), and a registration failure kills the child rather than leaving an Agones-Ready-but-control-plane-unregistered process running; `ControlPlaneURL` unset (the default) is a total no-op. It then reports assignment-ready too: `server_boot.gd` already verifies its mounted roster synchronously before `/ready` is ever exposed (so process-ready implies the roster was valid), and the API's `ASSIGNMENT_READY` gate checks only durable `assignments` rows server-side — so no new Godot-side state was needed, correcting an earlier overcautious note here. The supervisor retries assignment-ready (default 5 attempts, 2s apart, configurable) since those durable rows may lag process-ready slightly; a persistent failure there does not kill the child, unlike process-ready. Per-allocation data (currently `match-id`) now has a real channel to an already-Ready pod: `server/agones.Client.Allocate` requests `cosmic-clash.io/match-id`/`cosmic-clash.io/allocation-id` as `GameServerAllocation.spec.metadata.annotations` (Agones applies these to the allocated GameServer's own `object_meta` — the only channel that exists post-allocation, since env vars are fixed at pod creation), and the supervisor reads them back from its existing `/gameserver` SDK call, falling back to them only when `MatchID` isn't explicitly configured. The image now exists: a new `Dockerfile` `game-server` target packages the supervisor as ENTRYPOINT alongside the same dedicated-server export `server` produces | `server/supervisor/` tests prove Ready follows the probe and direct mode remains functional; `server_control.gd`, `agones_sdk.gd` and process-level smokes prove loopback `/ready`, `/health`, bearer-protected `/drain`, sidecar-shaped Health/Ready calls and drain admission fencing; `server/api/service.go`, `server/store/allocation_match_sql.go` and adversarial tests cover the registration route, digest/protocol validation, idempotent replay/conflict and the assignment-count gate; `server/supervisor/supervisor_test.go` covers opt-in registration, the workload-token/body/idempotency-key shape, the kill-on-failure path, the match-ID annotation fallback (both that it's used and that its absence fails closed before any HTTP call), the full process-ready-then-assignment-ready sequence and body shapes, and a retry test that fails assignment-ready twice with 409 before succeeding, asserting `Start()` still succeeds and the child is never killed; `server/agones/allocation_test.go` covers the requested annotations. `docker build --target game-server` verified for real: both binaries present, correct permissions, supervisor prints its usage; `server/store/stalled_allocation_sql.go`/`_test.go` and a live `TestPostgreSQLStalledAllocationsAreReclaimedWithoutPenalisingPlayers` cover the deadline boundary (a recent match must survive untouched), the no-penalty requeue and refreshed expiry, participant release, and idempotence against a second pass. **Fixed in passing**: `Dockerfile`'s `server` stage's `ubuntu` base digest had gone dead on Docker Hub (`docker pull` returned "not found", verified independently) — `make verify-phase6` was silently broken for a clean build before the re-pin; confirmed fixed with a full `make verify-phase6` run (arenas rotated, both goals observed, clean teardown). **Still not wired into `deploy/k8s/base/fleet.yaml`**: the manifest doesn't reference the `game-server` image or invoke any supervisor flags yet — the concrete remaining step is deciding and adding the per-deployment values (`--control-plane-url`, `--workload-token-path` plus the projected token volume, `--server-id-env`/`--image-digest-env` Downward API wiring), deliberately not guessed at here since they're environment-specific. `deploy/cosmic-clash-server` now wraps its exec in `stdbuf -oL -eL` (falling back to unwrapped if unavailable), fixing a real, live-confirmed bug: a genuinely detached (`docker run -d`) container showed zero `docker logs` output — not even the startup line — for 20+ seconds while the process ran normally, and `docker stop`'s SIGTERM lost that buffered output permanently rather than delaying it; re-verified fixed against the real launcher script, then a full `make verify-phase6` re-run confirmed no regression. Health-reclaim now exists: `store.ExpireStalledAllocations` reclaims a match stuck in `ALLOCATING`/`PROCESS_READY`/`ASSIGNMENT_READY` past a deadline (server crashed, or was reclaimed by Agones as unhealthy, before ever registering) by failing the match and requeuing every participant to `QUEUED` with a fresh expiry rather than penalising them — task 8.50's own "infrastructure-caused cases cannot penalise affected players" criterion directly settles the requeue-vs-fail design question this had been blocked on. Wired into `cmd/maintenance` alongside the season-rollover sweep (`--stalled-allocation-deadline` default 2m, `--stalled-allocation-batch`). **Superseding the `fleet.yaml` framing above**: §8.10's `WorkloadVerify` blocker, and its delivery channel, are both now closed — a control-plane-self-issued signed token (not a Kubernetes JWT), minted by `cmd/allocator` into a `cosmic-clash.io/workload-token` annotation and read back by the supervisor, exactly the way `match-id` already worked — see §8.10. `/register` and `/result` no longer 503 unconditionally once every `--workload-secret` (control plane, allocator) is set consistently. What remains for `fleet.yaml` is now purely the manifest itself: it doesn't yet reference the `game-server` image or invoke any supervisor flags (`--control-plane-url`, `--server-id-env`/`--image-digest-env` Downward API wiring — `--workload-token-path` is no longer required, since the annotation fallback covers it) — deliberately not guessed at here since these are environment-specific values, and this whole path has only run against HTTP-level Agones fakes, never a real cluster (see §8.10's "what's still missing") | | 8.29 `[D:8.26,8.27]` | **IN PROGRESS.** Supervisor discovers and validates the Agones endpoint, propagates the actual dynamic `--port`, and exports `SDR_LISTEN_PORT`/`SDR_IP` only for Hosted-SDR while preserving an isolated ENet path | `server/supervisor/` tests cover invalid address/port rejection, dynamic port argument/env propagation and SDR-vs-ENet separation; real Agones dynamic/passthrough mapping, POP/cert/firewall/NAT and multi-match fixture remain | -| 8.30 `[D:8.18,8.26,8.28,8.29]` | **IN PROGRESS.** Pure Go allocator filters Ready servers by region/build/protocol/transport, atomically claims one with idempotent allocation replay, verifies unanimous accepted-proposal/playlist/participant invariants before provider invocation, and now owns the assignment-publication boundary; PostgreSQL adds durable GameServer registration and compatible `SKIP LOCKED` claims with request-digest fencing, plus a leased `ALLOCATING`-match claim that derives the immutable compatibility tuple and fences bind/release by deterministic allocation ID; bind atomically attaches only a recorded provider allocation and advances every accepted participant ticket to `ALLOCATING`; `server/agones` strictly projects Ready GameServers from Fleet compatibility labels, submits/validates namespaced `GameServerAllocation` responses and dynamic endpoints; `server/allocator` reconciles provider success into durable state before exposing the endpoint; `cmd/allocator` refreshes that Ready projection before driving the lease → provider → durable-record → match-bind sequence and rebinds a recovered durable provider allocation without a second provider call | `server/domain/allocator.go`, `server/store/allocator_sql.go`, `server/store/allocation_match_sql.go`, `server/store/allocation_match_adapter.go`, `server/agones/allocation.go`, `server/allocator/service.go`, `server/allocator/worker.go`, `server/cmd/allocator`, `server/migrations/0004_allocator_registry.sql`, `0006_match_allocation_claims.sql` and tests cover strict Ready-server projection, stale-projection protection for allocated rows, deterministic compatible selection, exhaustion, conflicting/identical allocation replay, allocation-match lease recovery/bind fencing, atomic participant lifecycle transition, recovered-allocation binding without provider recall, immutable Fleet selector labels, provider ambiguity lease retention, unknown allocations, SQL claim ordering, invalid input, provider error/malformed response/IPv6 endpoint handling, accepted-proposal gating, durable-reconciliation failure isolation and assignment replay/conflict; `TestPostgreSQLAllocatorClaimReplayAndCapacityFence` and `TestPostgreSQLAllocationMatchClaimLeaseAndBindFence` cover the live database paths when the disposable database gate is run; `TestPostgreSQLConcurrentAllocationClaimNeverDoubleBooksAReadyServer` now races real concurrent claims against fewer Ready servers than requesters and proves no double-booking and no stray errors, stable across repeated `-race` runs; `scripts/run_allocator_integration.sh` and `TestRealAllocatorWorkerReconcilesAgonesAllocationAndBindsMatch` now add a real PostgreSQL + Agones-shaped HTTP provider integration covering Ready projection → worker lease → provider request → durable reconciliation → match/ticket bind; unknown provider-outcome reconciliation, signed roster metadata and live Agones cluster integration remain | +| 8.30 `[D:8.18,8.26,8.28,8.29]` | **IN PROGRESS.** Pure Go allocator filters Ready servers by region/build/protocol/transport, atomically claims one with idempotent allocation replay, verifies unanimous accepted-proposal/playlist/participant invariants before provider invocation, and now owns the assignment-publication boundary; PostgreSQL adds durable GameServer registration and compatible `SKIP LOCKED` claims with request-digest fencing, plus a leased `ALLOCATING`-match claim that derives the immutable compatibility tuple and fences bind/release by deterministic allocation ID; bind atomically attaches only a recorded provider allocation and advances every accepted participant ticket to `ALLOCATING`; `server/agones` strictly projects Ready GameServers from Fleet compatibility labels, submits/validates namespaced `GameServerAllocation` responses and dynamic endpoints, and can recover an already-Allocated GameServer by allocation ID after an ambiguous write; `server/allocator` reconciles provider success into durable state before exposing the endpoint; `cmd/allocator` refreshes that Ready projection before driving the lease → provider → durable-record → match-bind sequence and rebinds a recovered durable provider allocation without a second provider call | `server/domain/allocator.go`, `server/store/allocator_sql.go`, `server/store/allocation_match_sql.go`, `server/store/allocation_match_adapter.go`, `server/agones/allocation.go`, `server/allocator/service.go`, `server/allocator/worker.go`, `server/cmd/allocator`, `server/migrations/0004_allocator_registry.sql`, `0006_match_allocation_claims.sql` and tests cover strict Ready-server projection, stale-projection protection for allocated rows, deterministic compatible selection, exhaustion, conflicting/identical allocation replay, allocation-match lease recovery/bind fencing, atomic participant lifecycle transition, recovered-allocation binding without provider recall, immutable Fleet selector labels, provider ambiguity lease retention, unknown allocations, SQL claim ordering, invalid input, provider error/malformed response/IPv6 endpoint handling, accepted-proposal gating, provider recovery metadata/duplicate detection, durable-reconciliation failure isolation and assignment replay/conflict; `TestPostgreSQLAllocatorClaimReplayAndCapacityFence` and `TestPostgreSQLAllocationMatchClaimLeaseAndBindFence` cover the live database paths when the disposable database gate is run; `TestPostgreSQLConcurrentAllocationClaimNeverDoubleBooksAReadyServer` now races real concurrent claims against fewer Ready servers than requesters and proves no double-booking and no stray errors, stable across repeated `-race` runs; `scripts/run_allocator_integration.sh` and `TestRealAllocatorWorkerReconcilesAgonesAllocationAndBindsMatch` now add a real PostgreSQL + Agones-shaped HTTP provider integration covering Ready projection → worker lease → provider request → durable reconciliation → match/ticket bind; full live unknown-provider-outcome recovery and signed roster metadata/cluster integration remain | | 8.31 `[D:8.9,8.30]` | **IN PROGRESS.** Pure Go assignment gate requires Allocated state, exact allocation ID/match/server/region/build/protocol/transport compatibility, non-empty hosted endpoint and verified manifest signature before exposure; allocator publication cannot expose Ready state; durable roster persistence now verifies each canonical join-authorisation signature before publishing player rows; allocator service gates roster publication on allocated state and endpoint presence | `server/domain/assignment.go`, `allocator.go`, `store/assignment_sql.go` plus adversarial fixtures cover early-connect, tampered signature/manifest, wrong compatibility, empty endpoint, unknown allocation, forged roster signature, valid signature, premature publication and post-publication mutation rejection; Agones metadata watch, hosted-address registration, production signer and client-ticket publication remain | | 8.32 `[D:8.2,8.26,8.30]` | **IN PROGRESS.** Provider-neutral FleetAutoscaler baseline preserves a two-process Ready buffer, caps warm capacity, and leaves Allocated scale-down independent of the Ready floor; Fleet image references remain digest-pinned for current/rollback pre-pull | `deploy/k8s/base/fleet-autoscaler.yaml` and manifest tests cover Fleet ownership, Buffer policy and floor/cap invariants; regional on-demand node pools/failure domains, pre-pull rollout, warm-allocation p95/p99 and N+1 certification remain | | 8.33 `[D:8.26,8.32]` | **IN PROGRESS.** Fleet scheduling now requires on-demand capacity and spreads Ready processes across zones with skew 1; the autoscaler preserves the two-process Ready floor | `deploy/k8s/base/fleet.yaml` and manifest tests reject interruptible placement and single-zone concentration structurally; regional node pools, forced node-loss testing and measured N+1 headroom remain | diff --git a/server/agones/allocation.go b/server/agones/allocation.go index 3dcab86a..8f24db35 100644 --- a/server/agones/allocation.go +++ b/server/agones/allocation.go @@ -85,15 +85,80 @@ type allocationResponse struct { type gameServerListResponse struct { Items []struct { Metadata struct { - Name string `json:"name"` - Labels map[string]string `json:"labels"` + Name string `json:"name"` + Labels map[string]string `json:"labels"` + Annotations map[string]string `json:"annotations"` } `json:"metadata"` Status struct { - State string `json:"state"` + State string `json:"state"` + Address string `json:"address"` + Ports []struct { + Name string `json:"name"` + Port int `json:"port"` + } `json:"ports"` } `json:"status"` } `json:"items"` } +// RecoverAllocation finds a provider-side allocation that may have completed +// before the durable allocation record was written. The allocation ID and +// compatibility tuple are checked together so a stale or forged provider +// object cannot be rebound to another match. +func (c Client) RecoverAllocation(ctx context.Context, request domain.AllocationRequest, now time.Time) (AllocatedServer, bool, error) { + if request.AllocationID == "" || request.MatchID == "" || now.IsZero() { + return AllocatedServer{}, false, domain.ErrAllocationInput + } + if c.HTTP == nil { + c.HTTP = http.DefaultClient + } + base, err := c.endpoint() + if err != nil { + return AllocatedServer{}, false, err + } + req, err := http.NewRequestWithContext(ctx, http.MethodGet, base+"/apis/agones.dev/v1/namespaces/"+url.PathEscape(c.Namespace)+"/gameservers", nil) + if err != nil { + return AllocatedServer{}, false, err + } + response, err := c.HTTP.Do(req) + if err != nil { + return AllocatedServer{}, false, err + } + defer response.Body.Close() + if response.StatusCode < 200 || response.StatusCode >= 300 { + return AllocatedServer{}, false, fmt.Errorf("Agones GameServer recovery returned %s", response.Status) + } + var decoded gameServerListResponse + if err := json.NewDecoder(io.LimitReader(response.Body, 1<<20)).Decode(&decoded); err != nil { + return AllocatedServer{}, false, fmt.Errorf("decode Agones recovery list: %w", err) + } + found := false + var recovered AllocatedServer + for _, item := range decoded.Items { + if item.Status.State != "Allocated" || item.Metadata.Annotations["cosmic-clash.io/allocation-id"] != request.AllocationID { + continue + } + if found { + return AllocatedServer{}, false, domain.ErrConflict + } + if item.Metadata.Name == "" || item.Status.Address == "" || strings.ContainsAny(item.Status.Address, " \t\r\n") { + return AllocatedServer{}, false, fmt.Errorf("Agones recovered GameServer has invalid identity or address") + } + if item.Metadata.Annotations["cosmic-clash.io/match-id"] != request.MatchID { + return AllocatedServer{}, false, domain.ErrConflict + } + if item.Metadata.Labels["cosmic-clash.io/region"] != request.Region || item.Metadata.Labels["cosmic-clash.io/build"] != request.Build || item.Metadata.Labels["cosmic-clash.io/protocol"] != strconv.Itoa(request.Protocol) || item.Metadata.Labels["cosmic-clash.io/transport"] != request.Transport { + return AllocatedServer{}, false, domain.ErrConflict + } + port, err := selectPort(item.Status.Ports) + if err != nil { + return AllocatedServer{}, false, err + } + recovered = AllocatedServer{Allocation: domain.Allocation{AllocationID: request.AllocationID, MatchID: request.MatchID, ServerID: item.Metadata.Name, Region: request.Region, Build: request.Build, Protocol: request.Protocol, Transport: request.Transport, State: domain.ServerAllocated, AllocatedAt: now}, Endpoint: net.JoinHostPort(item.Status.Address, strconv.Itoa(port)), GameServer: item.Metadata.Name} + found = true + } + return recovered, found, nil +} + // ListReadyServers projects only Agones Ready GameServers into the durable // allocator registry. Compatibility fields must be present as Fleet labels; // malformed Ready objects fail closed instead of creating selectable capacity. diff --git a/server/agones/allocation_test.go b/server/agones/allocation_test.go index f5584d28..7dc3e6ee 100644 --- a/server/agones/allocation_test.go +++ b/server/agones/allocation_test.go @@ -154,3 +154,37 @@ func TestListReadyServersFailsClosedOnInvalidReadyCompatibility(t *testing.T) { t.Fatal("invalid Ready GameServer accepted") } } + +func TestRecoverAllocationFindsMatchingAllocatedGameServer(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method != http.MethodGet || !strings.HasSuffix(r.URL.Path, "/gameservers") { + t.Fatalf("request=%s %s", r.Method, r.URL.Path) + } + _, _ = w.Write([]byte(`{"items":[{"metadata":{"name":"gs-recovered","labels":{"cosmic-clash.io/region":"EU","cosmic-clash.io/build":"build-1","cosmic-clash.io/protocol":"1","cosmic-clash.io/transport":"enet"},"annotations":{"cosmic-clash.io/allocation-id":"allocation-1","cosmic-clash.io/match-id":"match-1"}},"status":{"state":"Allocated","address":"127.0.0.1","ports":[{"name":"default","port":31001}]}},{"metadata":{"name":"gs-other","annotations":{"cosmic-clash.io/allocation-id":"other"},"status":{"state":"Allocated"}}}]}`)) + })) + defer server.Close() + recovered, found, err := (Client{BaseURL: server.URL, Namespace: "games", HTTP: server.Client()}).RecoverAllocation(context.Background(), request(), time.Unix(1000, 0)) + if err != nil || !found || recovered.GameServer != "gs-recovered" || recovered.Endpoint != "127.0.0.1:31001" || recovered.Allocation.ServerID != "gs-recovered" { + t.Fatalf("recovered=%+v found=%t err=%v", recovered, found, err) + } +} + +func TestRecoverAllocationRejectsMismatchedBinding(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + _, _ = w.Write([]byte(`{"items":[{"metadata":{"name":"gs-forged","labels":{"cosmic-clash.io/region":"NA","cosmic-clash.io/build":"build-1","cosmic-clash.io/protocol":"1","cosmic-clash.io/transport":"enet"},"annotations":{"cosmic-clash.io/allocation-id":"allocation-1","cosmic-clash.io/match-id":"other-match"}},"status":{"state":"Allocated","address":"127.0.0.1","ports":[{"name":"default","port":31001}]}}]}`)) + })) + defer server.Close() + if _, found, err := (Client{BaseURL: server.URL, Namespace: "games", HTTP: server.Client()}).RecoverAllocation(context.Background(), request(), time.Unix(1000, 0)); err == nil || found { + t.Fatalf("mismatched recovery accepted: found=%t err=%v", found, err) + } +} + +func TestRecoverAllocationRejectsDuplicateProviderMatches(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + _, _ = w.Write([]byte(`{"items":[{"metadata":{"name":"gs-one","labels":{"cosmic-clash.io/region":"EU","cosmic-clash.io/build":"build-1","cosmic-clash.io/protocol":"1","cosmic-clash.io/transport":"enet"},"annotations":{"cosmic-clash.io/allocation-id":"allocation-1","cosmic-clash.io/match-id":"match-1"}},"status":{"state":"Allocated","address":"127.0.0.1","ports":[{"name":"default","port":31001}]}},{"metadata":{"name":"gs-two","labels":{"cosmic-clash.io/region":"EU","cosmic-clash.io/build":"build-1","cosmic-clash.io/protocol":"1","cosmic-clash.io/transport":"enet"},"annotations":{"cosmic-clash.io/allocation-id":"allocation-1","cosmic-clash.io/match-id":"match-1"}},"status":{"state":"Allocated","address":"127.0.0.1","ports":[{"name":"default","port":31002}]}}]}`)) + })) + defer server.Close() + if _, found, err := (Client{BaseURL: server.URL, Namespace: "games", HTTP: server.Client()}).RecoverAllocation(context.Background(), request(), time.Unix(1000, 0)); err == nil || found { + t.Fatalf("duplicate recovery accepted: found=%t err=%v", found, err) + } +} diff --git a/server/allocator/service.go b/server/allocator/service.go index 6e89706c..a81f7936 100644 --- a/server/allocator/service.go +++ b/server/allocator/service.go @@ -14,6 +14,10 @@ type Provider interface { Allocate(context.Context, domain.AllocationRequest, map[string]string, time.Time) (agones.AllocatedServer, error) } +type ProviderRecoverer interface { + RecoverAllocation(context.Context, domain.AllocationRequest, time.Time) (agones.AllocatedServer, bool, error) +} + type Durable interface { RecordProviderAllocation(context.Context, domain.Allocation, time.Time) (domain.Allocation, error) } @@ -84,6 +88,13 @@ func (s Service) Allocate(ctx context.Context, request domain.AllocationRequest, return result, nil } +func (s Service) RecordProviderAllocation(ctx context.Context, result agones.AllocatedServer, now time.Time) (domain.Allocation, error) { + if s.Durable == nil || result.Allocation.State != domain.ServerAllocated || result.Endpoint == "" { + return domain.Allocation{}, domain.ErrAllocationInput + } + return s.Durable.RecordProviderAllocation(ctx, result.Allocation, now) +} + var errNotConfigured = &configurationError{} type configurationError struct{} diff --git a/server/allocator/worker.go b/server/allocator/worker.go index 3cca20da..291f4a01 100644 --- a/server/allocator/worker.go +++ b/server/allocator/worker.go @@ -43,11 +43,30 @@ func (w Worker) RunOnce(ctx context.Context) (bool, error) { return true, fmt.Errorf("recover allocation for match %s: %w", request.MatchID, err) } if !recorded { - result, err := w.Service.Allocate(ctx, request, AllocationLabels(request)) - if err != nil { - return true, fmt.Errorf("allocate claimed match %s: %w", request.MatchID, err) + if recoverer, ok := w.Service.Provider.(ProviderRecoverer); ok { + recovered, found, err := recoverer.RecoverAllocation(ctx, request, w.Now()) + if err != nil { + return true, fmt.Errorf("recover provider allocation for match %s: %w", request.MatchID, err) + } + if found { + if _, err := w.Service.RecordProviderAllocation(ctx, recovered, w.Now()); err != nil { + return true, fmt.Errorf("record recovered allocation for match %s: %w", request.MatchID, err) + } + allocation = recovered.Allocation + } else { + result, err := w.Service.Allocate(ctx, request, AllocationLabels(request)) + if err != nil { + return true, fmt.Errorf("allocate claimed match %s: %w", request.MatchID, err) + } + allocation = result.Allocation + } + } else { + result, err := w.Service.Allocate(ctx, request, AllocationLabels(request)) + if err != nil { + return true, fmt.Errorf("allocate claimed match %s: %w", request.MatchID, err) + } + allocation = result.Allocation } - allocation = result.Allocation } if err := w.Claims.BindAllocatedMatch(ctx, allocation); err != nil { return true, fmt.Errorf("bind allocated match %s: %w", request.MatchID, err) diff --git a/server/allocator/worker_test.go b/server/allocator/worker_test.go index 45c400ad..2b35e10b 100644 --- a/server/allocator/worker_test.go +++ b/server/allocator/worker_test.go @@ -21,6 +21,17 @@ type matchClaimSpy struct { bindErr error } +type recoverableProviderSpy struct { + providerSpy + recovered agones.AllocatedServer + found bool + recoverErr error +} + +func (p *recoverableProviderSpy) RecoverAllocation(_ context.Context, _ domain.AllocationRequest, _ time.Time) (agones.AllocatedServer, bool, error) { + return p.recovered, p.found, p.recoverErr +} + func (s *matchClaimSpy) FindProviderAllocation(_ context.Context, _ domain.AllocationRequest) (domain.Allocation, bool, error) { return s.recorded, s.recorded.AllocationID != "", s.recordErr } @@ -69,6 +80,19 @@ func TestWorkerRecoversDurableProviderAllocationWithoutCallingProvider(t *testin } } +func TestWorkerRecoversProviderAllocationBeforeIssuingSecondAllocation(t *testing.T) { + request := domain.AllocationRequest{AllocationID: "allocation-1", MatchID: "match-1", Region: "EU", Build: "build-1", Protocol: 1, Transport: "enet"} + recovered := agones.AllocatedServer{Allocation: domain.Allocation{AllocationID: request.AllocationID, MatchID: request.MatchID, ServerID: "server-recovered", Region: request.Region, Build: request.Build, Protocol: request.Protocol, Transport: request.Transport, State: domain.ServerAllocated}, Endpoint: "127.0.0.1:31001"} + claims := &matchClaimSpy{request: request, found: true} + provider := &recoverableProviderSpy{recovered: recovered, found: true} + durable := &durableSpy{} + worker := Worker{Claims: claims, Service: Service{Provider: provider, Durable: durable, Now: func() time.Time { return time.Unix(1_000, 0) }}, Now: func() time.Time { return time.Unix(1_000, 0) }} + processed, err := worker.RunOnce(context.Background()) + if err != nil || !processed || provider.calls != 0 || durable.calls != 1 || claims.bound.ServerID != "server-recovered" { + t.Fatalf("processed=%t err=%v provider_calls=%d durable_calls=%d bound=%+v", processed, err, provider.calls, durable.calls, claims.bound) + } +} + func TestWorkerDoesNothingWhenNoDurableMatchIsAvailable(t *testing.T) { claims := &matchClaimSpy{} worker := Worker{Claims: claims, Now: func() time.Time { return time.Unix(1_000, 0) }}