fix(multiplayer): fence provider allocation results

This commit is contained in:
Josh Creek
2026-09-03 00:19:03 +01:00
parent a15368ed29
commit bef71e1dcf
5 changed files with 87 additions and 12 deletions
+1 -1
View File
File diff suppressed because one or more lines are too long
+9 -1
View File
@@ -122,12 +122,20 @@ func (s Service) Allocate(ctx context.Context, request domain.AllocationRequest,
}
return agones.AllocatedServer{}, err
}
if _, err := s.Durable.RecordProviderAllocation(ctx, result.Allocation, now); err != nil {
if err := validateProviderAllocation(request, result); err != nil {
if s.Metrics != nil {
s.Metrics.ObserveFailure(request.Region)
}
return agones.AllocatedServer{}, err
}
recorded, err := s.Durable.RecordProviderAllocation(ctx, result.Allocation, now)
if err != nil {
if s.Metrics != nil {
s.Metrics.ObserveFailure(request.Region)
}
return agones.AllocatedServer{}, err
}
result.Allocation = recorded
if s.Metrics != nil {
s.Metrics.ObserveSuccess(request.Region)
}
+56 -5
View File
@@ -25,6 +25,7 @@ func (p *providerSpy) Allocate(_ context.Context, _ domain.AllocationRequest, _
type durableSpy struct {
calls int
allocation domain.Allocation
result domain.Allocation
err error
}
@@ -51,15 +52,19 @@ func (r *rosterSpy) PublishRoster(_ context.Context, _ domain.Assignment, _ []do
func (d *durableSpy) RecordProviderAllocation(_ context.Context, allocation domain.Allocation, _ time.Time) (domain.Allocation, error) {
d.calls++
d.allocation = allocation
if d.result.AllocationID != "" {
return d.result, d.err
}
return allocation, d.err
}
func TestServiceDurablyRecordsProviderAllocationBeforeReturning(t *testing.T) {
provider := &providerSpy{result: agones.AllocatedServer{Allocation: domain.Allocation{AllocationID: "a", MatchID: "m", ServerID: "gs", State: domain.ServerAllocated}, Endpoint: "127.0.0.1:7777"}}
request := domain.AllocationRequest{AllocationID: "a", MatchID: "m", Region: "EU", Build: "b", Protocol: 1, Transport: "enet"}
provider := &providerSpy{result: agones.AllocatedServer{Allocation: domain.Allocation{AllocationID: "a", MatchID: "m", ServerID: "gs", Region: "EU", Build: "b", Protocol: 1, Transport: "enet", State: domain.ServerAllocated}, Endpoint: "127.0.0.1:7777"}}
durable := &durableSpy{}
metrics := NewMetrics()
service := Service{Provider: provider, Durable: durable, Metrics: metrics, Now: func() time.Time { return time.Unix(1000, 0) }}
result, err := service.Allocate(context.Background(), domain.AllocationRequest{AllocationID: "a", MatchID: "m", Region: "EU", Build: "b", Protocol: 1, Transport: "enet"}, map[string]string{"region": "EU"})
result, err := service.Allocate(context.Background(), request, map[string]string{"region": "EU"})
if err != nil || result.Endpoint == "" || durable.calls != 1 || durable.allocation.ServerID != "gs" {
t.Fatalf("result=%+v err=%v durable=%+v", result, err, durable)
}
@@ -69,8 +74,54 @@ func TestServiceDurablyRecordsProviderAllocationBeforeReturning(t *testing.T) {
}
}
func TestServiceRejectsMismatchedFreshProviderAllocationBeforePersistence(t *testing.T) {
request := domain.AllocationRequest{AllocationID: "allocation-1", MatchID: "match-1", Playlist: domain.Ranked, Region: "EU", Build: "build-1", Protocol: 1, ArenaPath: "res://scenes/arena_01.tscn", Transport: "enet"}
base := agones.AllocatedServer{Allocation: domain.Allocation{AllocationID: request.AllocationID, MatchID: request.MatchID, ServerID: "server-1", Region: request.Region, Build: request.Build, Protocol: request.Protocol, ArenaPath: request.ArenaPath, Transport: request.Transport, State: domain.ServerAllocated}, Endpoint: "127.0.0.1:7777"}
for name, mutate := range map[string]func(*agones.AllocatedServer){
"allocation id": func(r *agones.AllocatedServer) { r.Allocation.AllocationID = "other" },
"match": func(r *agones.AllocatedServer) { r.Allocation.MatchID = "other" },
"region": func(r *agones.AllocatedServer) { r.Allocation.Region = "NA" },
"build": func(r *agones.AllocatedServer) { r.Allocation.Build = "other" },
"protocol": func(r *agones.AllocatedServer) { r.Allocation.Protocol++ },
"arena": func(r *agones.AllocatedServer) { r.Allocation.ArenaPath = "res://scenes/arena_02.tscn" },
"transport": func(r *agones.AllocatedServer) { r.Allocation.Transport = "steam_sdr" },
"server": func(r *agones.AllocatedServer) { r.Allocation.ServerID = "" },
"state": func(r *agones.AllocatedServer) { r.Allocation.State = domain.ServerReady },
"endpoint": func(r *agones.AllocatedServer) { r.Endpoint = "" },
} {
t.Run(name, func(t *testing.T) {
result := base
mutate(&result)
durable := &durableSpy{}
service := Service{Provider: &providerSpy{result: result}, Durable: durable, Now: func() time.Time { return time.Unix(1000, 0) }}
if _, err := service.Allocate(context.Background(), request, nil); err == nil {
t.Fatal("mismatched provider result accepted")
}
if durable.calls != 0 {
t.Fatalf("mismatched result reached durable store %d times", durable.calls)
}
})
}
}
func TestServiceReturnsCanonicalDurableAllocation(t *testing.T) {
now := time.Unix(1000, 0)
request := domain.AllocationRequest{AllocationID: "a", MatchID: "m", Region: "EU", Build: "b", Protocol: 1, Transport: "enet"}
providerAllocation := domain.Allocation{AllocationID: "a", MatchID: "m", ServerID: "gs", Region: "EU", Build: "b", Protocol: 1, Transport: "enet", State: domain.ServerAllocated}
canonical := providerAllocation
canonical.AllocatedAt = now
service := Service{
Provider: &providerSpy{result: agones.AllocatedServer{Allocation: providerAllocation, Endpoint: "127.0.0.1:7777"}},
Durable: &durableSpy{result: canonical}, Now: func() time.Time { return now },
}
result, err := service.Allocate(context.Background(), request, nil)
if err != nil || result.Allocation != canonical {
t.Fatalf("result=%+v err=%v, want canonical %+v", result, err, canonical)
}
}
func TestServiceDoesNotReturnProviderResultAfterDurableFailure(t *testing.T) {
provider := &providerSpy{result: agones.AllocatedServer{Allocation: domain.Allocation{AllocationID: "a", State: domain.ServerAllocated}, Endpoint: "127.0.0.1:7777"}}
provider := &providerSpy{result: agones.AllocatedServer{Allocation: domain.Allocation{AllocationID: "a", MatchID: "m", ServerID: "gs", Region: "EU", Build: "b", Protocol: 1, Transport: "enet", State: domain.ServerAllocated}, Endpoint: "127.0.0.1:7777"}}
durable := &durableSpy{err: errors.New("database unavailable")}
service := Service{Provider: provider, Durable: durable, Now: func() time.Time { return time.Unix(1000, 0) }}
result, err := service.Allocate(context.Background(), domain.AllocationRequest{AllocationID: "a", MatchID: "m", Region: "EU", Build: "b", Protocol: 1, Transport: "enet"}, map[string]string{"region": "EU"})
@@ -113,7 +164,7 @@ func TestServiceDoesNotConsumeSharedQuotaWhenReconcilingProviderResult(t *testin
func TestServiceDoesNotDoubleChargeQuotaAfterProviderResultRecovery(t *testing.T) {
quota := &quotaSpy{}
durable := &durableSpy{err: errors.New("recording unavailable")}
provider := &providerSpy{result: agones.AllocatedServer{Allocation: domain.Allocation{AllocationID: "a", MatchID: "m", Region: "EU", State: domain.ServerAllocated}, Endpoint: "127.0.0.1:7777"}}
provider := &providerSpy{result: agones.AllocatedServer{Allocation: domain.Allocation{AllocationID: "a", MatchID: "m", ServerID: "gs", Region: "EU", Build: "b", Protocol: 1, Transport: "enet", State: domain.ServerAllocated}, Endpoint: "127.0.0.1:7777"}}
service := Service{Provider: provider, Durable: durable, Quota: quota, Now: func() time.Time { return time.Unix(1000, 0) }}
request := domain.AllocationRequest{AllocationID: "a", MatchID: "m", Region: "EU", Build: "b", Protocol: 1, Transport: "enet"}
if _, err := service.Allocate(context.Background(), request, nil); err == nil {
@@ -136,7 +187,7 @@ func TestServiceAllocatesOnlyUnanimouslyAcceptedMatchingProposal(t *testing.T) {
{PlayerID: "player-b", Response: domain.AcceptedResponse},
},
}
provider := &providerSpy{result: agones.AllocatedServer{Allocation: domain.Allocation{AllocationID: "a", MatchID: "m", State: domain.ServerAllocated}, Endpoint: "127.0.0.1:7777"}}
provider := &providerSpy{result: agones.AllocatedServer{Allocation: domain.Allocation{AllocationID: "a", MatchID: "m", ServerID: "gs", Region: "EU", Build: "b", Protocol: 1, Transport: "enet", State: domain.ServerAllocated}, Endpoint: "127.0.0.1:7777"}}
durable := &durableSpy{}
service := Service{Provider: provider, Durable: durable, Now: func() time.Time { return time.Unix(1000, 0) }}
request := domain.AllocationRequest{AllocationID: "a", MatchID: "m", Region: "EU", Build: "b", Protocol: 1, Transport: "enet"}
+6 -5
View File
@@ -50,13 +50,14 @@ func (w Worker) RunOnce(ctx context.Context) (bool, error) {
return true, fmt.Errorf("recover provider allocation for match %s: %w", request.MatchID, err)
}
if found {
if err := validateRecoveredAllocation(request, recovered); err != nil {
if err := validateProviderAllocation(request, recovered); err != nil {
return true, fmt.Errorf("recovered provider allocation for match %s: %w", request.MatchID, err)
}
if _, err := w.Service.RecordProviderAllocation(ctx, recovered, w.Now()); err != nil {
recorded, err := w.Service.RecordProviderAllocation(ctx, recovered, w.Now())
if err != nil {
return true, fmt.Errorf("record recovered allocation for match %s: %w", request.MatchID, err)
}
allocation = recovered.Allocation
allocation = recorded
} else {
result, err := w.Service.Allocate(ctx, request, AllocationLabels(request))
if err != nil {
@@ -78,10 +79,10 @@ func (w Worker) RunOnce(ctx context.Context) (bool, error) {
return true, nil
}
func validateRecoveredAllocation(request domain.AllocationRequest, result agones.AllocatedServer) error {
func validateProviderAllocation(request domain.AllocationRequest, result agones.AllocatedServer) error {
allocation := result.Allocation
if result.Endpoint == "" || allocation.State != domain.ServerAllocated || allocation.AllocationID != request.AllocationID || allocation.MatchID != request.MatchID || allocation.ServerID == "" || allocation.Region != request.Region || allocation.Build != request.Build || allocation.Protocol != request.Protocol || allocation.Transport != request.Transport || allocation.ArenaPath != request.ArenaPath {
return fmt.Errorf("recovered allocation does not match request")
return fmt.Errorf("provider allocation does not match request")
}
return nil
}
+15
View File
@@ -93,6 +93,21 @@ func TestWorkerRecoversProviderAllocationBeforeIssuingSecondAllocation(t *testin
}
}
func TestWorkerBindsCanonicalRecordedRecovery(t *testing.T) {
now := time.Unix(1_000, 0)
request := domain.AllocationRequest{AllocationID: "allocation-1", MatchID: "match-1", Region: "EU", Build: "build-1", Protocol: 1, Transport: "enet"}
providerAllocation := 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}
canonical := providerAllocation
canonical.AllocatedAt = now
claims := &matchClaimSpy{request: request, found: true}
provider := &recoverableProviderSpy{recovered: agones.AllocatedServer{Allocation: providerAllocation, Endpoint: "127.0.0.1:31001"}, found: true}
worker := Worker{Claims: claims, Service: Service{Provider: provider, Durable: &durableSpy{result: canonical}, Now: func() time.Time { return now }}, Now: func() time.Time { return now }}
processed, err := worker.RunOnce(context.Background())
if err != nil || !processed || claims.bound != canonical {
t.Fatalf("processed=%t err=%v bound=%+v, want %+v", processed, err, claims.bound, canonical)
}
}
func TestWorkerRejectsRecoveredAllocationForDifferentCompatibility(t *testing.T) {
request := domain.AllocationRequest{AllocationID: "allocation-1", MatchID: "match-1", Region: "EU", Build: "build-1", Protocol: 1, Transport: "enet"}
claims := &matchClaimSpy{request: request, found: true}