Removes unnecessary error messages when merging two lists

Adds a const for the List maximum capacity

pkg/apis/agones/v1/gameserver.go

@@ -23,6 +23,7 @@ import (
 	"agones.dev/agones/pkg"
 	"agones.dev/agones/pkg/apis"
 	"agones.dev/agones/pkg/apis/agones"
+	"agones.dev/agones/pkg/util/apiserver"
 	"agones.dev/agones/pkg/util/runtime"
 	"github.com/mattbaird/jsonpatch"
 	"github.com/pkg/errors"
@@ -919,7 +920,7 @@ func (gs *GameServer) UpdateCounterCapacity(name string, capacity int64) error {
 
 // UpdateListCapacity updates the ListStatus Capacity to the given capacity.
 func (gs *GameServer) UpdateListCapacity(name string, capacity int64) error {
-	if capacity < 0 || capacity > 1000 {
+	if capacity < 0 || capacity > apiserver.ListMaxCapacity {
 		return errors.Errorf("unable to UpdateListCapacity: Name %s, Capacity %d. Capacity must be between 0 and 1000, inclusive", name, capacity)
 	}
 	if list, ok := gs.Status.Lists[name]; ok {
@@ -932,19 +933,12 @@ func (gs *GameServer) UpdateListCapacity(name string, capacity int64) error {
 
 // AppendListValues adds unique values to the ListStatus Values list.
 func (gs *GameServer) AppendListValues(name string, values []string) error {
-	if len(values) == 0 {
-		return errors.Errorf("unable to AppendListValues: Name %s, Values %s. No values to append", name, values)
+	if values == nil {
+		return errors.Errorf("unable to AppendListValues: Name %s, Values %s. Values must not be nil", name, values)
 	}
 	if list, ok := gs.Status.Lists[name]; ok {
 		mergedList := MergeRemoveDuplicates(list.Values, values)
-		if len(mergedList) > int(list.Capacity) {
-			return errors.Errorf("unable to AppendListValues: Name %s, Values %s. Appended list length %d exceeds list capacity %d", name, values, len(mergedList), list.Capacity)
-		}
-		// If all given values are duplicates we give an error warning.
-		if len(mergedList) == len(list.Values) {
-			return errors.Errorf("unable to AppendListValues: Name %s, Values %s. All appended values are duplicates of the existing list", name, values)
-		}
-		// If only some values are duplicates, those duplicate values are silently dropped.
+		// Any duplicate values are silently dropped.
 		list.Values = mergedList
 		// Truncate values if more than capacity
 		if len(list.Values) > int(list.Capacity) {

pkg/apis/agones/v1/gameserver_test.go

@@ -1897,6 +1897,8 @@ func TestGameServerUpdateListCapacity(t *testing.T) {
 func TestGameServerAppendListValues(t *testing.T) {
 	t.Parallel()
 
+	var nilSlice []string
+
 	testCases := map[string]struct {
 		gs      GameServer
 		name    string
@@ -1960,37 +1962,22 @@ func TestGameServerAppendListValues(t *testing.T) {
 			},
 			wantErr: false,
 		},
-		"append no values no-op with error": {
+		"append nil values": {
 			gs: GameServer{Status: GameServerStatus{
 				Lists: map[string]ListStatus{
 					"blings": {
 						Values:   []string{"bling1"},
 						Capacity: 10,
 					}}}},
 			name:   "blings",
-			values: []string{},
+			values: nilSlice,
 			want: ListStatus{
 				Values:   []string{"bling1"},
 				Capacity: 10,
 			},
 			wantErr: true,
 		},
-		"append only duplicates no-op with error": {
-			gs: GameServer{Status: GameServerStatus{
-				Lists: map[string]ListStatus{
-					"slings": {
-						Values:   []string{"slings1", "sling2"},
-						Capacity: 4,
-					}}}},
-			name:   "blings",
-			values: []string{},
-			want: ListStatus{
-				Values:   []string{"slings1", "sling2"},
-				Capacity: 4,
-			},
-			wantErr: true,
-		},
-		"append too many values no-op with error": {
+		"append too many values truncates list": {
 			gs: GameServer{Status: GameServerStatus{
 				Lists: map[string]ListStatus{
 					"bananaslugs": {
@@ -2000,10 +1987,10 @@ func TestGameServerAppendListValues(t *testing.T) {
 			name:   "bananaslugs",
 			values: []string{"bananaslug4", "bananaslug5", "bananaslug6"},
 			want: ListStatus{
-				Values:   []string{"bananaslugs1", "bananaslug2", "bananaslug3"},
+				Values:   []string{"bananaslugs1", "bananaslug2", "bananaslug3", "bananaslug4", "bananaslug5"},
 				Capacity: 5,
 			},
-			wantErr: true,
+			wantErr: false,
 		},
 	}
 

pkg/apis/allocation/v1/gameserverallocation_test.go

@@ -1006,7 +1006,7 @@ func TestGameServerListActions(t *testing.T) {
 					}}}},
 			wantErr: false,
 		},
-		"update list values and capacity - value add fails": {
+		"update list values and capacity - value add truncates silently": {
 			la: ListAction{
 				AddValues: []string{"fairy1", "fairy3"},
 				Capacity:  int64Pointer(2),
@@ -1024,7 +1024,7 @@ func TestGameServerListActions(t *testing.T) {
 						Values:   []string{"fairy1", "fairy2"},
 						Capacity: 2,
 					}}}},
-			wantErr: true,
+			wantErr: false,
 		},
 	}
 

pkg/gameserverallocations/allocator_test.go

@@ -397,7 +397,7 @@ func TestAllocatorApplyAllocationToGameServerCountsListsActions(t *testing.T) {
 				}},
 			wantLists: map[string]agonesv1.ListStatus{
 				"players": {
-					Values:   []string{},
+					Values:   []string{"x7un"},
 					Capacity: 1,
 				}},
 		},

pkg/sdkserver/localsdk.go

@@ -29,6 +29,7 @@ import (
 	"agones.dev/agones/pkg/sdk"
 	"agones.dev/agones/pkg/sdk/alpha"
 	"agones.dev/agones/pkg/sdk/beta"
+	"agones.dev/agones/pkg/util/apiserver"
 	"agones.dev/agones/pkg/util/runtime"
 	"github.com/fsnotify/fsnotify"
 	"github.com/mennanov/fmutils"
@@ -661,8 +662,7 @@ func (l *LocalSDKServer) UpdateList(ctx context.Context, in *alpha.UpdateListReq
 		return nil, errors.Errorf("INVALID_ARGUMENT. Field Mask Path(s): %v are invalid for List. Use valid field name(s): %v", in.UpdateMask.GetPaths(), in.List.ProtoReflect().Descriptor().Fields())
 	}
 
-	// TODO: Pull in variable Max Capacity from CRD instead of hard-coded number here.
-	if in.List.Capacity < 0 || in.List.Capacity > 1000 {
+	if in.List.Capacity < 0 || in.List.Capacity > apiserver.ListMaxCapacity {
 		return nil, errors.Errorf("OUT_OF_RANGE. Capacity must be within range [0,1000]. Found Capacity: %d", in.List.Capacity)
 	}
 

pkg/sdkserver/sdkserver.go

@@ -33,6 +33,7 @@ import (
 	"agones.dev/agones/pkg/sdk"
 	"agones.dev/agones/pkg/sdk/alpha"
 	"agones.dev/agones/pkg/sdk/beta"
+	"agones.dev/agones/pkg/util/apiserver"
 	"agones.dev/agones/pkg/util/logfields"
 	"agones.dev/agones/pkg/util/runtime"
 	"agones.dev/agones/pkg/util/workerqueue"
@@ -1066,8 +1067,7 @@ func (s *SDKServer) UpdateList(ctx context.Context, in *alpha.UpdateListRequest)
 	s.gsUpdateMutex.Lock()
 	defer s.gsUpdateMutex.Unlock()
 
-	// TODO: Pull in variable Max Capacity from CRD instead of hard-coded number here.
-	if in.List.Capacity < 0 || in.List.Capacity > 1000 {
+	if in.List.Capacity < 0 || in.List.Capacity > apiserver.ListMaxCapacity {
 		return nil, errors.Errorf("out of range. Capacity must be within range [0,1000]. Found Capacity: %d", in.List.Capacity)
 	}
 

pkg/util/apiserver/apiserver.go

@@ -59,6 +59,11 @@ const (
 	AcceptHeader = "Accept"
 )
 
+const (
+	// ListMaxCapacity is the maximum capacity for List in the gamerserver spec and status CRDs.
+	ListMaxCapacity = int64(1000)
+)
+
 func init() {
 	Scheme.AddUnversionedTypes(unversionedVersion, unversionedTypes...)
 }