Skip to content

Commit

Permalink
SDK server not clearing lists on update (#3606)
Browse files Browse the repository at this point in the history
* fix for the sdk server not clearing lists on update

* new unit tests for the gsListUpdates queue

---------

Co-authored-by: igooch <igooch@google.com>
  • Loading branch information
jlory and igooch authored Jan 27, 2024
1 parent 1215925 commit d96fb3c
Show file tree
Hide file tree
Showing 2 changed files with 77 additions and 47 deletions.
10 changes: 8 additions & 2 deletions pkg/sdkserver/sdkserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down Expand Up @@ -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)
}
114 changes: 69 additions & 45 deletions pkg/sdkserver/sdkserver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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",
Expand All @@ -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,
},
}

Expand Down Expand Up @@ -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()
})
Expand All @@ -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",
Expand All @@ -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",
Expand All @@ -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,
},
}

Expand Down Expand Up @@ -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()
})
Expand All @@ -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",
Expand All @@ -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",
Expand All @@ -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",
Expand All @@ -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,
},
}

Expand Down Expand Up @@ -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()
})
Expand Down

0 comments on commit d96fb3c

Please sign in to comment.