diff --git a/pkg/controllers/gameserverset/gameserverset_controller_test.go b/pkg/controllers/gameserverset/gameserverset_controller_test.go index e691c00b..26dccf57 100644 --- a/pkg/controllers/gameserverset/gameserverset_controller_test.go +++ b/pkg/controllers/gameserverset/gameserverset_controller_test.go @@ -100,6 +100,87 @@ func TestInitAsts(t *testing.T) { }, }, }, + + { + gss: &gameKruiseV1alpha1.GameServerSet{ + TypeMeta: metav1.TypeMeta{ + Kind: "GameServerSet", + APIVersion: "game.kruise.io/v1alpha1", + }, + ObjectMeta: metav1.ObjectMeta{ + Namespace: "xxx", + Name: "case1", + UID: "xxx1", + }, + Spec: gameKruiseV1alpha1.GameServerSetSpec{ + Replicas: ptr.To[int32](4), + ReserveGameServerIds: []int{0}, + UpdateStrategy: gameKruiseV1alpha1.UpdateStrategy{ + Type: apps.RollingUpdateStatefulSetStrategyType, + RollingUpdate: &gameKruiseV1alpha1.RollingUpdateStatefulSetStrategy{}, + }, + }, + }, + asts: &kruiseV1beta1.StatefulSet{ + TypeMeta: metav1.TypeMeta{ + Kind: "StatefulSet", + APIVersion: "apps.kruise.io/v1beta1", + }, + ObjectMeta: metav1.ObjectMeta{ + Namespace: "xxx", + Name: "case1", + OwnerReferences: []metav1.OwnerReference{ + { + APIVersion: "game.kruise.io/v1alpha1", + Kind: "GameServerSet", + Name: "case1", + UID: "xxx1", + Controller: ptr.To[bool](true), + BlockOwnerDeletion: ptr.To[bool](true), + }, + }, + ResourceVersion: "1", + }, + Spec: kruiseV1beta1.StatefulSetSpec{ + Replicas: ptr.To[int32](4), + ReserveOrdinals: []int{0}, + PodManagementPolicy: apps.ParallelPodManagement, + ServiceName: "case1", + Selector: &metav1.LabelSelector{ + MatchLabels: map[string]string{gameKruiseV1alpha1.GameServerOwnerGssKey: "case1"}, + }, + UpdateStrategy: kruiseV1beta1.StatefulSetUpdateStrategy{ + Type: apps.RollingUpdateStatefulSetStrategyType, + RollingUpdate: &kruiseV1beta1.RollingUpdateStatefulSetStrategy{ + UnorderedUpdate: &kruiseV1beta1.UnorderedUpdateStrategy{ + PriorityStrategy: &appspub.UpdatePriorityStrategy{ + OrderPriority: []appspub.UpdatePriorityOrderTerm{ + { + OrderedKey: gameKruiseV1alpha1.GameServerUpdatePriorityKey, + }, + }, + }, + }, + }, + }, + Template: corev1.PodTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{ + gameKruiseV1alpha1.GameServerOwnerGssKey: "case1", + }, + }, + Spec: corev1.PodSpec{ + ReadinessGates: []corev1.PodReadinessGate{ + { + ConditionType: appspub.InPlaceUpdateReady, + }, + }, + }, + }, + ScaleStrategy: &kruiseV1beta1.StatefulSetScaleStrategy{}, + }, + }, + }, } for i, test := range tests { diff --git a/pkg/controllers/gameserverset/gameserverset_manager.go b/pkg/controllers/gameserverset/gameserverset_manager.go index a357ab02..1549bd33 100644 --- a/pkg/controllers/gameserverset/gameserverset_manager.go +++ b/pkg/controllers/gameserverset/gameserverset_manager.go @@ -124,10 +124,11 @@ func (manager *GameServerSetManager) GameServerScale() error { notExistIds := util.GetSliceInANotInB(asts.Spec.ReserveOrdinals, reserveIds) gssReserveIds := gss.Spec.ReserveGameServerIds - klog.Infof("GameServers %s/%s already has %d replicas, expect to have %d replicas.", gss.GetNamespace(), gss.GetName(), currentReplicas, expectedReplicas) + klog.Infof("GameServers %s/%s already has %d replicas, expect to have %d replicas; With newExplicit: %v; oldExplicit: %v; oldImplicit: %v", + gss.GetNamespace(), gss.GetName(), currentReplicas, expectedReplicas, gssReserveIds, reserveIds, notExistIds) manager.eventRecorder.Eventf(gss, corev1.EventTypeNormal, ScaleReason, "scale from %d to %d", currentReplicas, expectedReplicas) - newManageIds, newReserveIds := computeToScaleGs(gssReserveIds, reserveIds, notExistIds, expectedReplicas, podList, gss.Spec.ScaleStrategy.ScaleDownStrategyType) + newManageIds, newReserveIds := computeToScaleGs(gssReserveIds, reserveIds, notExistIds, expectedReplicas, podList) if gss.Spec.GameServerTemplate.ReclaimPolicy == gameKruiseV1alpha1.DeleteGameServerReclaimPolicy { err := SyncGameServer(gss, c, newManageIds, util.GetIndexListFromPodList(podList)) @@ -163,82 +164,59 @@ func (manager *GameServerSetManager) GameServerScale() error { return nil } -func computeToScaleGs(gssReserveIds, reserveIds, notExistIds []int, expectedReplicas int, pods []corev1.Pod, scaleDownType gameKruiseV1alpha1.ScaleDownStrategyType) ([]int, []int) { - workloadManageIds := util.GetIndexListFromPodList(pods) - - var toAdd []int - var toDelete []int - - // 1. compute reserved GameServerIds, firstly - - // 1.a. to delete those new reserved GameServers already in workloadManageIds - toDelete = util.GetSliceInAandInB(util.GetSliceInANotInB(gssReserveIds, reserveIds), workloadManageIds) - - // 1.b. to add those remove-reserved GameServers already in workloadManageIds - existLastIndex := -1 - if len(workloadManageIds) != 0 { - sort.Ints(workloadManageIds) - existLastIndex = workloadManageIds[len(workloadManageIds)-1] - } - for _, id := range util.GetSliceInANotInB(reserveIds, gssReserveIds) { - if existLastIndex > id { - toAdd = append(toAdd, id) +// computeToScaleGs is to compute what the id list the pods should be existed in cluster, and what the asts reserve id list should be. +// reserveIds is the explicit id list. +// notExistIds is the implicit id list. +// gssReserveIds is the newest explicit id list. +// pods is the pods that managed by gss now. +func computeToScaleGs(gssReserveIds, reserveIds, notExistIds []int, expectedReplicas int, pods []corev1.Pod) ([]int, []int) { + // 1. Get newest implicit list & explicit. + newAddExplicit := util.GetSliceInANotInB(gssReserveIds, reserveIds) + newDeleteExplicit := util.GetSliceInANotInB(reserveIds, gssReserveIds) + newImplicit := util.GetSliceInANotInB(notExistIds, newAddExplicit) + newImplicit = append(newImplicit, newDeleteExplicit...) + newExplicit := gssReserveIds + + // 2. Remove the pods ids is in newExplicit. + var workloadManageIds []int + var newPods []corev1.Pod + for _, pod := range pods { + index := util.GetIndexFromGsName(pod.Name) + if util.IsNumInList(index, newExplicit) { + continue } - } - // 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 + workloadManageIds = append(workloadManageIds, index) + newPods = append(newPods, pod) } - // 2. compute remain GameServerIds, secondly + // 3. Continue to delete or add pods based on the current and expected number of pods. + existReplicas := len(workloadManageIds) - numToAdd := expectedReplicas - len(pods) + len(toDelete) - len(toAdd) - if numToAdd < 0 { - - // 2.a to delete GameServers according to DeleteSequence - sortedGs := util.DeleteSequenceGs(pods) - sort.Sort(sortedGs) - toDelete = append(toDelete, util.GetIndexListFromPodList(sortedGs[:-numToAdd])...) - } else { - - // 2.b to add GameServers, firstly add those in add notExistIds, secondly add those in future sequence - numNotExist := len(notExistIds) - if numNotExist < numToAdd { - toAdd = append(toAdd, notExistIds...) - times := 0 - for i := existLastIndex + 1; times < numToAdd-numNotExist; i++ { - if !util.IsNumInList(i, gssReserveIds) { - toAdd = append(toAdd, i) - times++ - } + if existReplicas < expectedReplicas { + // Add pods. + num := 0 + var toAdd []int + for i := 0; num < expectedReplicas-existReplicas; i++ { + if util.IsNumInList(i, workloadManageIds) || util.IsNumInList(i, newExplicit) { + continue } - } else { - toAdd = append(toAdd, notExistIds[:numToAdd]...) - } - } - - newManageIds := append(workloadManageIds, util.GetSliceInANotInB(toAdd, workloadManageIds)...) - newManageIds = util.GetSliceInANotInB(newManageIds, toDelete) - - if scaleDownType == gameKruiseV1alpha1.ReserveIdsScaleDownStrategyType { - return newManageIds, append(gssReserveIds, util.GetSliceInANotInB(toDelete, gssReserveIds)...) - } - - var newReserveIds []int - if len(newManageIds) != 0 { - sort.Ints(newManageIds) - for i := 0; i < newManageIds[len(newManageIds)-1]; i++ { - if !util.IsNumInList(i, newManageIds) { - newReserveIds = append(newReserveIds, i) + if util.IsNumInList(i, newImplicit) { + newImplicit = util.GetSliceInANotInB(newImplicit, []int{i}) } + toAdd = append(toAdd, i) + num++ } + workloadManageIds = append(workloadManageIds, toAdd...) + } else if existReplicas > expectedReplicas { + // Delete pods. + sortedGs := util.DeleteSequenceGs(newPods) + sort.Sort(sortedGs) + toDelete := util.GetIndexListFromPodList(sortedGs[:existReplicas-expectedReplicas]) + workloadManageIds = util.GetSliceInANotInB(workloadManageIds, toDelete) + newImplicit = append(newImplicit, toDelete...) } - return newManageIds, newReserveIds + return workloadManageIds, append(newImplicit, newExplicit...) } func SyncGameServer(gss *gameKruiseV1alpha1.GameServerSet, c client.Client, newManageIds, oldManageIds []int) error { diff --git a/pkg/controllers/gameserverset/gameserverset_manager_test.go b/pkg/controllers/gameserverset/gameserverset_manager_test.go index 8322d55b..6dac2f8b 100644 --- a/pkg/controllers/gameserverset/gameserverset_manager_test.go +++ b/pkg/controllers/gameserverset/gameserverset_manager_test.go @@ -37,21 +37,20 @@ func init() { func TestComputeToScaleGs(t *testing.T) { tests := []struct { - newGssReserveIds []int - oldGssreserveIds []int - notExistIds []int - expectedReplicas int - scaleDownStrategyType gameKruiseV1alpha1.ScaleDownStrategyType - pods []corev1.Pod - newReserveIds []int - newManageIds []int + newGssReserveIds []int + oldGssreserveIds []int + notExistIds []int + expectedReplicas int + pods []corev1.Pod + newReserveIds []int + newManageIds []int }{ + // case 0 { - newGssReserveIds: []int{2, 3, 4}, - oldGssreserveIds: []int{2, 3}, - notExistIds: []int{5}, - expectedReplicas: 3, - scaleDownStrategyType: gameKruiseV1alpha1.GeneralScaleDownStrategyType, + newGssReserveIds: []int{2, 3, 4}, + oldGssreserveIds: []int{2, 3}, + notExistIds: []int{5}, + expectedReplicas: 3, pods: []corev1.Pod{ { ObjectMeta: metav1.ObjectMeta{ @@ -93,12 +92,12 @@ func TestComputeToScaleGs(t *testing.T) { newReserveIds: []int{2, 3, 4, 5}, newManageIds: []int{0, 1, 6}, }, + // case 1 { - newGssReserveIds: []int{0, 2, 3}, - oldGssreserveIds: []int{0, 4, 5}, - notExistIds: []int{}, - expectedReplicas: 3, - scaleDownStrategyType: gameKruiseV1alpha1.GeneralScaleDownStrategyType, + newGssReserveIds: []int{0, 2, 3}, + oldGssreserveIds: []int{0, 4, 5}, + notExistIds: []int{}, + expectedReplicas: 3, pods: []corev1.Pod{ { ObjectMeta: metav1.ObjectMeta{ @@ -149,12 +148,12 @@ func TestComputeToScaleGs(t *testing.T) { newReserveIds: []int{0, 2, 3, 4, 5}, newManageIds: []int{1, 6, 7}, }, + // case 2 { - newGssReserveIds: []int{0}, - oldGssreserveIds: []int{0, 4, 5}, - notExistIds: []int{}, - expectedReplicas: 1, - scaleDownStrategyType: gameKruiseV1alpha1.GeneralScaleDownStrategyType, + newGssReserveIds: []int{0}, + oldGssreserveIds: []int{0, 4, 5}, + notExistIds: []int{}, + expectedReplicas: 1, pods: []corev1.Pod{ { ObjectMeta: metav1.ObjectMeta{ @@ -202,15 +201,15 @@ func TestComputeToScaleGs(t *testing.T) { }, }, }, - newReserveIds: []int{0}, + newReserveIds: []int{0, 2, 3, 4, 5, 6, 7}, newManageIds: []int{1}, }, + // case 3 { - newGssReserveIds: []int{0, 2, 3}, - oldGssreserveIds: []int{0, 4, 5}, - notExistIds: []int{}, - expectedReplicas: 4, - scaleDownStrategyType: gameKruiseV1alpha1.GeneralScaleDownStrategyType, + newGssReserveIds: []int{0, 2, 3}, + oldGssreserveIds: []int{0, 4, 5}, + notExistIds: []int{}, + expectedReplicas: 4, pods: []corev1.Pod{ { ObjectMeta: metav1.ObjectMeta{ @@ -261,12 +260,12 @@ func TestComputeToScaleGs(t *testing.T) { newReserveIds: []int{0, 2, 3, 5}, newManageIds: []int{1, 4, 6, 7}, }, + // case 4 { - newGssReserveIds: []int{0, 3, 5}, - oldGssreserveIds: []int{0, 3, 5}, - notExistIds: []int{}, - expectedReplicas: 1, - scaleDownStrategyType: gameKruiseV1alpha1.ReserveIdsScaleDownStrategyType, + newGssReserveIds: []int{0, 3, 5}, + oldGssreserveIds: []int{0, 3, 5}, + notExistIds: []int{}, + expectedReplicas: 1, pods: []corev1.Pod{ { ObjectMeta: metav1.ObjectMeta{ @@ -308,12 +307,12 @@ func TestComputeToScaleGs(t *testing.T) { newReserveIds: []int{0, 3, 5, 2, 4, 6}, newManageIds: []int{1}, }, + // case 5 { - newGssReserveIds: []int{1, 2}, - oldGssreserveIds: []int{}, - notExistIds: []int{1, 2}, - expectedReplicas: 2, - scaleDownStrategyType: gameKruiseV1alpha1.GeneralScaleDownStrategyType, + newGssReserveIds: []int{1, 2}, + oldGssreserveIds: []int{}, + notExistIds: []int{1, 2}, + expectedReplicas: 2, pods: []corev1.Pod{ { ObjectMeta: metav1.ObjectMeta{ @@ -337,36 +336,97 @@ func TestComputeToScaleGs(t *testing.T) { newReserveIds: []int{1, 2}, newManageIds: []int{0, 3}, }, + // case 6 { - newGssReserveIds: []int{}, - oldGssreserveIds: []int{}, - notExistIds: []int{}, - expectedReplicas: 3, - scaleDownStrategyType: gameKruiseV1alpha1.GeneralScaleDownStrategyType, - pods: []corev1.Pod{}, - newReserveIds: []int{}, - newManageIds: []int{0, 1, 2}, + newGssReserveIds: []int{}, + oldGssreserveIds: []int{}, + notExistIds: []int{}, + expectedReplicas: 3, + pods: []corev1.Pod{}, + newReserveIds: []int{}, + newManageIds: []int{0, 1, 2}, }, + // case 7 { - newGssReserveIds: []int{1, 2}, - oldGssreserveIds: []int{}, - notExistIds: []int{}, - expectedReplicas: 3, - scaleDownStrategyType: gameKruiseV1alpha1.GeneralScaleDownStrategyType, - pods: []corev1.Pod{}, - newReserveIds: []int{1, 2}, - newManageIds: []int{0, 3, 4}, + newGssReserveIds: []int{1, 2}, + oldGssreserveIds: []int{}, + notExistIds: []int{}, + expectedReplicas: 3, + pods: []corev1.Pod{}, + newReserveIds: []int{1, 2}, + newManageIds: []int{0, 3, 4}, + }, + // case 8 + { + newGssReserveIds: []int{0}, + oldGssreserveIds: []int{}, + notExistIds: []int{0}, + expectedReplicas: 1, + pods: []corev1.Pod{}, + newReserveIds: []int{0}, + newManageIds: []int{1}, + }, + // case 9 + { + newGssReserveIds: []int{}, + oldGssreserveIds: []int{1}, + notExistIds: []int{}, + expectedReplicas: 2, + pods: []corev1.Pod{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "xxx-0", + 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", + }, + }, + }, + }, + newReserveIds: []int{1}, + newManageIds: []int{0, 2}, + }, + // case 10 + { + newGssReserveIds: []int{0}, + oldGssreserveIds: []int{}, + notExistIds: []int{2, 3, 4}, + expectedReplicas: 4, + pods: []corev1.Pod{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "xxx-1", + Labels: map[string]string{ + gameKruiseV1alpha1.GameServerOpsStateKey: string(gameKruiseV1alpha1.None), + gameKruiseV1alpha1.GameServerDeletePriorityKey: "0", + }, + }, + }, + }, + newReserveIds: []int{0}, + newManageIds: []int{1, 2, 3, 4}, }, } for i, test := range tests { - newManageIds, newReserveIds := computeToScaleGs(test.newGssReserveIds, test.oldGssreserveIds, test.notExistIds, test.expectedReplicas, test.pods, test.scaleDownStrategyType) + t.Logf("case %d : newGssReserveIds: %v ; oldGssreserveIds: %v ; notExistIds: %v ; expectedReplicas: %d; pods: %v", i, test.newGssReserveIds, test.oldGssreserveIds, test.notExistIds, test.expectedReplicas, test.pods) + newManageIds, newReserveIds := computeToScaleGs(test.newGssReserveIds, test.oldGssreserveIds, test.notExistIds, test.expectedReplicas, test.pods) if !util.IsSliceEqual(newReserveIds, test.newReserveIds) { t.Errorf("case %d: expect newNotExistIds %v but got %v", i, test.newReserveIds, newReserveIds) } if !util.IsSliceEqual(newManageIds, test.newManageIds) { t.Errorf("case %d: expect newManageIds %v but got %v", i, test.newManageIds, newManageIds) } + t.Logf("case %d : newManageIds: %v ; newReserveIds: %v", i, newManageIds, newReserveIds) } }