fix: add marginal conditions to avoid fatal errors (openkruise#49)

Signed-off-by: ChrisLiu <[email protected]>
CloudNativeGame · Apr 11, 2023 · f283461 · f283461
1 parent 9204359
commit f283461
Show file tree

Hide file tree

Showing 4 changed files with 205 additions and 2 deletions.
diff --git a/pkg/controllers/gameserverset/gameserverset_manager.go b/pkg/controllers/gameserverset/gameserverset_manager.go
@@ -153,6 +153,14 @@ func computeToScaleGs(gssReserveIds, reserveIds, notExistIds []int, expectedRepl
 			toAdd = append(toAdd, id)
 		}
 	}
+	// those remove-reserved GameServers will only be added when expansion is required
+	if len(toDelete)-len(pods)+expectedReplicas > 0 {
+		index := util.Min(len(toAdd), len(toDelete)-len(pods)+expectedReplicas)
+		sort.Ints(toAdd)
+		toAdd = toAdd[:index]
+	} else {
+		toAdd = nil
+	}
 
 	// 2. compute remain GameServerIds, secondly
 

diff --git a/pkg/controllers/gameserverset/gameserverset_manager_test.go b/pkg/controllers/gameserverset/gameserverset_manager_test.go
@@ -83,12 +83,174 @@ func TestComputeToScaleGs(t *testing.T) {
 			},
 			newNotExistIds: []int{5},
 		},
+		{
+			newGssReserveIds: []int{0, 2, 3},
+			oldGssreserveIds: []int{0, 4, 5},
+			notExistIds:      []int{},
+			expectedReplicas: 3,
+			pods: []corev1.Pod{
+				{
+					ObjectMeta: metav1.ObjectMeta{
+						Name: "xxx-1",
+						Labels: map[string]string{
+							gameKruiseV1alpha1.GameServerOpsStateKey:       string(gameKruiseV1alpha1.None),
+							gameKruiseV1alpha1.GameServerDeletePriorityKey: "0",
+						},
+					},
+				},
+				{
+					ObjectMeta: metav1.ObjectMeta{
+						Name: "xxx-2",
+						Labels: map[string]string{
+							gameKruiseV1alpha1.GameServerOpsStateKey:       string(gameKruiseV1alpha1.None),
+							gameKruiseV1alpha1.GameServerDeletePriorityKey: "0",
+						},
+					},
+				},
+				{
+					ObjectMeta: metav1.ObjectMeta{
+						Name: "xxx-3",
+						Labels: map[string]string{
+							gameKruiseV1alpha1.GameServerOpsStateKey:       string(gameKruiseV1alpha1.None),
+							gameKruiseV1alpha1.GameServerDeletePriorityKey: "0",
+						},
+					},
+				},
+				{
+					ObjectMeta: metav1.ObjectMeta{
+						Name: "xxx-6",
+						Labels: map[string]string{
+							gameKruiseV1alpha1.GameServerOpsStateKey:       string(gameKruiseV1alpha1.None),
+							gameKruiseV1alpha1.GameServerDeletePriorityKey: "0",
+						},
+					},
+				},
+				{
+					ObjectMeta: metav1.ObjectMeta{
+						Name: "xxx-7",
+						Labels: map[string]string{
+							gameKruiseV1alpha1.GameServerOpsStateKey:       string(gameKruiseV1alpha1.None),
+							gameKruiseV1alpha1.GameServerDeletePriorityKey: "0",
+						},
+					},
+				},
+			},
+			newNotExistIds: []int{4, 5},
+		},
+		{
+			newGssReserveIds: []int{0},
+			oldGssreserveIds: []int{0, 4, 5},
+			notExistIds:      []int{},
+			expectedReplicas: 1,
+			pods: []corev1.Pod{
+				{
+					ObjectMeta: metav1.ObjectMeta{
+						Name: "xxx-1",
+						Labels: map[string]string{
+							gameKruiseV1alpha1.GameServerOpsStateKey:       string(gameKruiseV1alpha1.None),
+							gameKruiseV1alpha1.GameServerDeletePriorityKey: "0",
+						},
+					},
+				},
+				{
+					ObjectMeta: metav1.ObjectMeta{
+						Name: "xxx-2",
+						Labels: map[string]string{
+							gameKruiseV1alpha1.GameServerOpsStateKey:       string(gameKruiseV1alpha1.None),
+							gameKruiseV1alpha1.GameServerDeletePriorityKey: "0",
+						},
+					},
+				},
+				{
+					ObjectMeta: metav1.ObjectMeta{
+						Name: "xxx-3",
+						Labels: map[string]string{
+							gameKruiseV1alpha1.GameServerOpsStateKey:       string(gameKruiseV1alpha1.None),
+							gameKruiseV1alpha1.GameServerDeletePriorityKey: "0",
+						},
+					},
+				},
+				{
+					ObjectMeta: metav1.ObjectMeta{
+						Name: "xxx-6",
+						Labels: map[string]string{
+							gameKruiseV1alpha1.GameServerOpsStateKey:       string(gameKruiseV1alpha1.None),
+							gameKruiseV1alpha1.GameServerDeletePriorityKey: "0",
+						},
+					},
+				},
+				{
+					ObjectMeta: metav1.ObjectMeta{
+						Name: "xxx-7",
+						Labels: map[string]string{
+							gameKruiseV1alpha1.GameServerOpsStateKey:       string(gameKruiseV1alpha1.None),
+							gameKruiseV1alpha1.GameServerDeletePriorityKey: "0",
+						},
+					},
+				},
+			},
+			newNotExistIds: nil,
+		},
+		{
+			newGssReserveIds: []int{0, 2, 3},
+			oldGssreserveIds: []int{0, 4, 5},
+			notExistIds:      []int{},
+			expectedReplicas: 4,
+			pods: []corev1.Pod{
+				{
+					ObjectMeta: metav1.ObjectMeta{
+						Name: "xxx-1",
+						Labels: map[string]string{
+							gameKruiseV1alpha1.GameServerOpsStateKey:       string(gameKruiseV1alpha1.None),
+							gameKruiseV1alpha1.GameServerDeletePriorityKey: "0",
+						},
+					},
+				},
+				{
+					ObjectMeta: metav1.ObjectMeta{
+						Name: "xxx-2",
+						Labels: map[string]string{
+							gameKruiseV1alpha1.GameServerOpsStateKey:       string(gameKruiseV1alpha1.None),
+							gameKruiseV1alpha1.GameServerDeletePriorityKey: "0",
+						},
+					},
+				},
+				{
+					ObjectMeta: metav1.ObjectMeta{
+						Name: "xxx-3",
+						Labels: map[string]string{
+							gameKruiseV1alpha1.GameServerOpsStateKey:       string(gameKruiseV1alpha1.None),
+							gameKruiseV1alpha1.GameServerDeletePriorityKey: "0",
+						},
+					},
+				},
+				{
+					ObjectMeta: metav1.ObjectMeta{
+						Name: "xxx-6",
+						Labels: map[string]string{
+							gameKruiseV1alpha1.GameServerOpsStateKey:       string(gameKruiseV1alpha1.None),
+							gameKruiseV1alpha1.GameServerDeletePriorityKey: "0",
+						},
+					},
+				},
+				{
+					ObjectMeta: metav1.ObjectMeta{
+						Name: "xxx-7",
+						Labels: map[string]string{
+							gameKruiseV1alpha1.GameServerOpsStateKey:       string(gameKruiseV1alpha1.None),
+							gameKruiseV1alpha1.GameServerDeletePriorityKey: "0",
+						},
+					},
+				},
+			},
+			newNotExistIds: []int{5},
+		},
 	}
 
-	for _, test := range tests {
+	for i, test := range tests {
 		newNotExistIds := computeToScaleGs(test.newGssReserveIds, test.oldGssreserveIds, test.notExistIds, test.expectedReplicas, test.pods)
 		if !util.IsSliceEqual(newNotExistIds, test.newNotExistIds) {
-			t.Errorf("expect newNotExistIds %v but got %v", test.newNotExistIds, newNotExistIds)
+			t.Errorf("case %d: expect newNotExistIds %v but got %v", i, test.newNotExistIds, newNotExistIds)
 		}
 	}
 }

diff --git a/pkg/util/math.go b/pkg/util/math.go
@@ -0,0 +1,8 @@
+package util
+
+func Min(a, b int) int {
+	if a < b {
+		return a
+	}
+	return b
+}
diff --git a/pkg/util/math_test.go b/pkg/util/math_test.go
@@ -0,0 +1,25 @@
+package util
+
+import "testing"
+
+func TestMin(t *testing.T) {
+	tests := []struct {
+		a      int
+		b      int
+		result int
+	}{
+		{
+			a:      1,
+			b:      0,
+			result: 0,
+		},
+	}
+
+	for _, test := range tests {
+		expect := test.result
+		actual := Min(test.a, test.b)
+		if expect != actual {
+			t.Errorf("expect %v but got %v", expect, actual)
+		}
+	}
+}