From d96fb3c86917bd5aee93fd8767ea528020ab270a Mon Sep 17 00:00:00 2001 From: Julien Date: Fri, 26 Jan 2024 21:18:49 -0500 Subject: [PATCH] SDK server not clearing lists on update (#3606) * fix for the sdk server not clearing lists on update * new unit tests for the gsListUpdates queue --------- Co-authored-by: igooch --- pkg/sdkserver/sdkserver.go | 10 ++- pkg/sdkserver/sdkserver_test.go | 114 +++++++++++++++++++------------- 2 files changed, 77 insertions(+), 47 deletions(-) diff --git a/pkg/sdkserver/sdkserver.go b/pkg/sdkserver/sdkserver.go index 52c97ced19..d1d685ddd8 100644 --- a/pkg/sdkserver/sdkserver.go +++ b/pkg/sdkserver/sdkserver.go @@ -1230,8 +1230,8 @@ func (s *SDKServer) updateList(ctx context.Context) error { // Cache a copy of the successfully updated gameserver s.gsCopy = gs - // Clear the gsCounterUpdates - s.gsCounterUpdates = map[string]counterUpdateRequest{} + // Clear the gsListUpdates + s.gsListUpdates = map[string]listUpdateRequest{} return nil } @@ -1429,3 +1429,9 @@ func (s *SDKServer) NewSDKServerContext(ctx context.Context) context.Context { }() return sdkCtx } + +func (s *SDKServer) gsListUpdatesLen() int { + s.gsUpdateMutex.RLock() + defer s.gsUpdateMutex.RUnlock() + return len(s.gsListUpdates) +} diff --git a/pkg/sdkserver/sdkserver_test.go b/pkg/sdkserver/sdkserver_test.go index 6157692e5b..0bb0fa09de 100644 --- a/pkg/sdkserver/sdkserver_test.go +++ b/pkg/sdkserver/sdkserver_test.go @@ -1351,18 +1351,20 @@ func TestSDKServerAddListValue(t *testing.T) { } fixtures := map[string]struct { - listName string - requests []*alpha.AddListValueRequest - want agonesv1.ListStatus - updateErrs []bool - updated bool + listName string + requests []*alpha.AddListValueRequest + want agonesv1.ListStatus + updateErrs []bool + updated bool + expectedUpdatesQueueLen int }{ "Add value": { - listName: "foo", - requests: []*alpha.AddListValueRequest{{Name: "foo", Value: "five"}}, - want: agonesv1.ListStatus{Values: []string{"one", "two", "three", "four", "five"}, Capacity: int64(10)}, - updateErrs: []bool{false}, - updated: true, + listName: "foo", + requests: []*alpha.AddListValueRequest{{Name: "foo", Value: "five"}}, + want: agonesv1.ListStatus{Values: []string{"one", "two", "three", "four", "five"}, Capacity: int64(10)}, + updateErrs: []bool{false}, + updated: true, + expectedUpdatesQueueLen: 0, }, "Add multiple values including duplicates": { listName: "bar", @@ -1372,9 +1374,10 @@ func TestSDKServerAddListValue(t *testing.T) { {Name: "bar", Value: "five"}, {Name: "bar", Value: "zero"}, }, - want: agonesv1.ListStatus{Values: []string{"one", "two", "three", "four", "five", "zero"}, Capacity: int64(10)}, - updateErrs: []bool{false, true, true, false}, - updated: true, + want: agonesv1.ListStatus{Values: []string{"one", "two", "three", "four", "five", "zero"}, Capacity: int64(10)}, + updateErrs: []bool{false, true, true, false}, + updated: true, + expectedUpdatesQueueLen: 0, }, "Add multiple values past capacity": { listName: "baz", @@ -1391,8 +1394,9 @@ func TestSDKServerAddListValue(t *testing.T) { Values: []string{"one", "two", "three", "four", "five", "six", "seven", "eight", "nine", "ten"}, Capacity: int64(10), }, - updateErrs: []bool{false, false, false, false, false, false, true}, - updated: true, + updateErrs: []bool{false, false, false, false, false, false, true}, + updated: true, + expectedUpdatesQueueLen: 0, }, } @@ -1478,6 +1482,10 @@ func TestSDKServerAddListValue(t *testing.T) { } } + // on an update, confirms that the update queue list contains the right amount of items + glu := sc.gsListUpdatesLen() + assert.Equal(t, testCase.expectedUpdatesQueueLen, glu) + cancel() wg.Wait() }) @@ -1499,18 +1507,20 @@ func TestSDKServerRemoveListValue(t *testing.T) { } fixtures := map[string]struct { - listName string - requests []*alpha.RemoveListValueRequest - want agonesv1.ListStatus - updateErrs []bool - updated bool + listName string + requests []*alpha.RemoveListValueRequest + want agonesv1.ListStatus + updateErrs []bool + updated bool + expectedUpdatesQueueLen int }{ "Remove value": { - listName: "foo", - requests: []*alpha.RemoveListValueRequest{{Name: "foo", Value: "two"}}, - want: agonesv1.ListStatus{Values: []string{"one", "three", "four"}, Capacity: int64(100)}, - updateErrs: []bool{false}, - updated: true, + listName: "foo", + requests: []*alpha.RemoveListValueRequest{{Name: "foo", Value: "two"}}, + want: agonesv1.ListStatus{Values: []string{"one", "three", "four"}, Capacity: int64(100)}, + updateErrs: []bool{false}, + updated: true, + expectedUpdatesQueueLen: 0, }, "Remove multiple values including duplicates": { listName: "bar", @@ -1519,9 +1529,10 @@ func TestSDKServerRemoveListValue(t *testing.T) { {Name: "bar", Value: "three"}, {Name: "bar", Value: "two"}, }, - want: agonesv1.ListStatus{Values: []string{"one", "four"}, Capacity: int64(100)}, - updateErrs: []bool{false, false, true}, - updated: true, + want: agonesv1.ListStatus{Values: []string{"one", "four"}, Capacity: int64(100)}, + updateErrs: []bool{false, false, true}, + updated: true, + expectedUpdatesQueueLen: 0, }, "Remove all values": { listName: "baz", @@ -1531,9 +1542,10 @@ func TestSDKServerRemoveListValue(t *testing.T) { {Name: "baz", Value: "four"}, {Name: "baz", Value: "one"}, }, - want: agonesv1.ListStatus{Values: []string{}, Capacity: int64(100)}, - updateErrs: []bool{false, false, false, false}, - updated: true, + want: agonesv1.ListStatus{Values: []string{}, Capacity: int64(100)}, + updateErrs: []bool{false, false, false, false}, + updated: true, + expectedUpdatesQueueLen: 0, }, } @@ -1619,6 +1631,10 @@ func TestSDKServerRemoveListValue(t *testing.T) { } } + // on an update, confirms that the update queue list contains the right amount of items + glu := sc.gsListUpdatesLen() + assert.Equal(t, testCase.expectedUpdatesQueueLen, glu) + cancel() wg.Wait() }) @@ -1640,11 +1656,12 @@ func TestSDKServerUpdateList(t *testing.T) { } fixtures := map[string]struct { - listName string - request *alpha.UpdateListRequest - want agonesv1.ListStatus - updateErr bool - updated bool + listName string + request *alpha.UpdateListRequest + want agonesv1.ListStatus + updateErr bool + updated bool + expectedUpdatesQueueLen int }{ "set capacity to max": { listName: "foo", @@ -1655,9 +1672,10 @@ func TestSDKServerUpdateList(t *testing.T) { }, UpdateMask: &fieldmaskpb.FieldMask{Paths: []string{"capacity"}}, }, - want: agonesv1.ListStatus{Values: []string{"one", "two", "three", "four"}, Capacity: int64(1000)}, - updateErr: false, - updated: true, + want: agonesv1.ListStatus{Values: []string{"one", "two", "three", "four"}, Capacity: int64(1000)}, + updateErr: false, + updated: true, + expectedUpdatesQueueLen: 0, }, "set capacity to min values are truncated": { listName: "bar", @@ -1668,9 +1686,10 @@ func TestSDKServerUpdateList(t *testing.T) { }, UpdateMask: &fieldmaskpb.FieldMask{Paths: []string{"capacity"}}, }, - want: agonesv1.ListStatus{Values: []string{}, Capacity: int64(0)}, - updateErr: false, - updated: true, + want: agonesv1.ListStatus{Values: []string{}, Capacity: int64(0)}, + updateErr: false, + updated: true, + expectedUpdatesQueueLen: 0, }, "set capacity past max": { listName: "baz", @@ -1681,9 +1700,10 @@ func TestSDKServerUpdateList(t *testing.T) { }, UpdateMask: &fieldmaskpb.FieldMask{Paths: []string{"capacity"}}, }, - want: agonesv1.ListStatus{Values: []string{"one", "two", "three", "four"}, Capacity: int64(100)}, - updateErr: true, - updated: false, + want: agonesv1.ListStatus{Values: []string{"one", "two", "three", "four"}, Capacity: int64(100)}, + updateErr: true, + updated: false, + expectedUpdatesQueueLen: 0, }, } @@ -1767,6 +1787,10 @@ func TestSDKServerUpdateList(t *testing.T) { } } + // on an update, confirm that the update queue list contains the right amount of items + glu := sc.gsListUpdatesLen() + assert.Equal(t, testCase.expectedUpdatesQueueLen, glu) + cancel() wg.Wait() })