fix(multiplayer): recover ambiguous agones allocations

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{}

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)

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) }}