From 624d17ff1194ed3fe8280f24221955430c12b6e2 Mon Sep 17 00:00:00 2001 From: Kagaya Date: Mon, 21 Apr 2025 15:31:16 +0800 Subject: [PATCH] feat: support range type for ReserveIDs (#209) --- apis/v1alpha1/gameserverset_types.go | 16 +- apis/v1alpha1/zz_generated.deepcopy.go | 2 +- .../bases/game.kruise.io_gameserversets.yaml | 5 +- go.mod | 6 +- go.sum | 4 +- .../gameserverset_controller_test.go | 5 +- .../gameserverset/gameserverset_manager.go | 75 +++-- .../gameserverset_manager_test.go | 300 ++++++++++-------- pkg/util/gameserver.go | 5 + pkg/util/set.go | 199 ++++++++++++ pkg/util/set_test.go | 272 ++++++++++++++++ pkg/util/slice.go | 25 ++ pkg/util/slice_test.go | 113 ++++++- pkg/webhook/validating_gss.go | 50 ++- test/e2e/framework/framework.go | 2 +- 15 files changed, 889 insertions(+), 190 deletions(-) create mode 100644 pkg/util/set.go create mode 100644 pkg/util/set_test.go diff --git a/apis/v1alpha1/gameserverset_types.go b/apis/v1alpha1/gameserverset_types.go index 052d397..537e4ff 100644 --- a/apis/v1alpha1/gameserverset_types.go +++ b/apis/v1alpha1/gameserverset_types.go @@ -50,14 +50,14 @@ type GameServerSetSpec struct { Replicas *int32 `json:"replicas"` // INSERT ADDITIONAL SPEC FIELDS - desired state of cluster // Important: Run "make" to regenerate code after modifying this file - GameServerTemplate GameServerTemplate `json:"gameServerTemplate,omitempty"` - ServiceName string `json:"serviceName,omitempty"` - ReserveGameServerIds []int `json:"reserveGameServerIds,omitempty"` - ServiceQualities []ServiceQuality `json:"serviceQualities,omitempty"` - UpdateStrategy UpdateStrategy `json:"updateStrategy,omitempty"` - ScaleStrategy ScaleStrategy `json:"scaleStrategy,omitempty"` - Network *Network `json:"network,omitempty"` - Lifecycle *appspub.Lifecycle `json:"lifecycle,omitempty"` + GameServerTemplate GameServerTemplate `json:"gameServerTemplate,omitempty"` + ServiceName string `json:"serviceName,omitempty"` + ReserveGameServerIds []intstr.IntOrString `json:"reserveGameServerIds,omitempty"` + ServiceQualities []ServiceQuality `json:"serviceQualities,omitempty"` + UpdateStrategy UpdateStrategy `json:"updateStrategy,omitempty"` + ScaleStrategy ScaleStrategy `json:"scaleStrategy,omitempty"` + Network *Network `json:"network,omitempty"` + Lifecycle *appspub.Lifecycle `json:"lifecycle,omitempty"` } type GameServerTemplate struct { diff --git a/apis/v1alpha1/zz_generated.deepcopy.go b/apis/v1alpha1/zz_generated.deepcopy.go index 39c64ca..eed47b0 100644 --- a/apis/v1alpha1/zz_generated.deepcopy.go +++ b/apis/v1alpha1/zz_generated.deepcopy.go @@ -189,7 +189,7 @@ func (in *GameServerSetSpec) DeepCopyInto(out *GameServerSetSpec) { in.GameServerTemplate.DeepCopyInto(&out.GameServerTemplate) if in.ReserveGameServerIds != nil { in, out := &in.ReserveGameServerIds, &out.ReserveGameServerIds - *out = make([]int, len(*in)) + *out = make([]intstr.IntOrString, len(*in)) copy(*out, *in) } if in.ServiceQualities != nil { diff --git a/config/crd/bases/game.kruise.io_gameserversets.yaml b/config/crd/bases/game.kruise.io_gameserversets.yaml index f01c545..b7e6684 100644 --- a/config/crd/bases/game.kruise.io_gameserversets.yaml +++ b/config/crd/bases/game.kruise.io_gameserversets.yaml @@ -605,7 +605,10 @@ spec: type: integer reserveGameServerIds: items: - type: integer + anyOf: + - type: integer + - type: string + x-kubernetes-int-or-string: true type: array scaleStrategy: properties: diff --git a/go.mod b/go.mod index 59a7371..04852c3 100644 --- a/go.mod +++ b/go.mod @@ -2,6 +2,8 @@ module github.com/openkruise/kruise-game go 1.22.0 +toolchain go1.22.12 + require ( github.com/BurntSushi/toml v1.2.1 github.com/aws-controllers-k8s/elbv2-controller v0.0.9 @@ -9,8 +11,9 @@ require ( github.com/kr/pretty v0.3.1 github.com/onsi/ginkgo v1.16.5 github.com/onsi/gomega v1.32.0 - github.com/openkruise/kruise-api v1.7.1 + github.com/openkruise/kruise-api v1.8.0 github.com/prometheus/client_golang v1.18.0 + golang.org/x/exp v0.0.0-20220722155223-a9213eeb770e google.golang.org/grpc v1.58.3 google.golang.org/protobuf v1.33.0 k8s.io/api v0.30.10 @@ -62,7 +65,6 @@ require ( github.com/spf13/pflag v1.0.5 // indirect go.uber.org/multierr v1.11.0 // indirect go.uber.org/zap v1.26.0 // indirect - golang.org/x/exp v0.0.0-20220722155223-a9213eeb770e // indirect golang.org/x/mod v0.17.0 // indirect golang.org/x/net v0.24.0 // indirect golang.org/x/oauth2 v0.17.0 // indirect diff --git a/go.sum b/go.sum index 9731843..81c62cb 100644 --- a/go.sum +++ b/go.sum @@ -110,8 +110,8 @@ github.com/onsi/gomega v1.7.1/go.mod h1:XdKZgCCFLUoM/7CFJVPcG8C1xQ1AJ0vpAezJrB7J github.com/onsi/gomega v1.10.1/go.mod h1:iN09h71vgCQne3DLsj+A5owkum+a2tYe+TOCB1ybHNo= github.com/onsi/gomega v1.32.0 h1:JRYU78fJ1LPxlckP6Txi/EYqJvjtMrDC04/MM5XRHPk= github.com/onsi/gomega v1.32.0/go.mod h1:a4x4gW6Pz2yK1MAmvluYme5lvYTn61afQ2ETw/8n4Lg= -github.com/openkruise/kruise-api v1.7.1 h1:pF+tPHWY1SS0X7sXTOIHZ5sNb5h5MBy1D7h6bJI5yW8= -github.com/openkruise/kruise-api v1.7.1/go.mod h1:ZD94u+GSQGtKrDfFhMVpQhzjr7g7UlXhYfRoNp/EhJs= +github.com/openkruise/kruise-api v1.8.0 h1:DoUb873uuf2Bhoajim+9tb/X0eFpwIxRydc4Awfeeiw= +github.com/openkruise/kruise-api v1.8.0/go.mod h1:XRpoTk7VFgh9r5HRUZurwhiC3cpCf5BX8X4beZLcIfA= github.com/pkg/diff v0.0.0-20210226163009-20ebb0f2a09e/go.mod h1:pJLUxLENpZxwdsKMEsNbx1VGcRFpLqf3715MtcvvzbA= github.com/pkg/errors v0.9.1 h1:FEBLx1zS214owpjy7qsBeixbURkuhQAwrK5UwLGTwt4= github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= diff --git a/pkg/controllers/gameserverset/gameserverset_controller_test.go b/pkg/controllers/gameserverset/gameserverset_controller_test.go index 79488f6..4d1224a 100644 --- a/pkg/controllers/gameserverset/gameserverset_controller_test.go +++ b/pkg/controllers/gameserverset/gameserverset_controller_test.go @@ -13,6 +13,7 @@ import ( corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" + "k8s.io/apimachinery/pkg/util/intstr" "k8s.io/utils/ptr" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/fake" @@ -115,7 +116,7 @@ func TestInitAsts(t *testing.T) { }, Spec: gameKruiseV1alpha1.GameServerSetSpec{ Replicas: ptr.To[int32](4), - ReserveGameServerIds: []int{0}, + ReserveGameServerIds: []intstr.IntOrString{intstr.FromInt(0)}, UpdateStrategy: gameKruiseV1alpha1.UpdateStrategy{ Type: apps.RollingUpdateStatefulSetStrategyType, RollingUpdate: &gameKruiseV1alpha1.RollingUpdateStatefulSetStrategy{}, @@ -144,7 +145,7 @@ func TestInitAsts(t *testing.T) { }, Spec: kruiseV1beta1.StatefulSetSpec{ Replicas: ptr.To[int32](4), - ReserveOrdinals: []int{0}, + ReserveOrdinals: []intstr.IntOrString{intstr.FromInt(0)}, PodManagementPolicy: apps.ParallelPodManagement, ServiceName: "case1", Selector: &metav1.LabelSelector{ diff --git a/pkg/controllers/gameserverset/gameserverset_manager.go b/pkg/controllers/gameserverset/gameserverset_manager.go index 1549bd3..80eb534 100644 --- a/pkg/controllers/gameserverset/gameserverset_manager.go +++ b/pkg/controllers/gameserverset/gameserverset_manager.go @@ -18,6 +18,10 @@ package gameserverset import ( "context" + "sort" + "strconv" + "sync" + kruiseV1alpha1 "github.com/openkruise/kruise-api/apps/v1alpha1" kruiseV1beta1 "github.com/openkruise/kruise-api/apps/v1beta1" corev1 "k8s.io/api/core/v1" @@ -26,14 +30,12 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/json" + "k8s.io/apimachinery/pkg/util/sets" "k8s.io/client-go/tools/record" "k8s.io/client-go/util/retry" "k8s.io/klog/v2" "k8s.io/utils/ptr" "sigs.k8s.io/controller-runtime/pkg/client" - "sort" - "strconv" - "sync" gameKruiseV1alpha1 "github.com/openkruise/kruise-game/apis/v1alpha1" "github.com/openkruise/kruise-game/pkg/util" @@ -96,13 +98,18 @@ func (manager *GameServerSetManager) GetReplicasAfterKilling() *int32 { return ptr.To[int32](*gss.Spec.Replicas - int32(toKill)) } +// IsNeedToScale checks if the GameServerSet need to scale, +// return True when the replicas or reserveGameServerIds is changed func (manager *GameServerSetManager) IsNeedToScale() bool { gss := manager.gameServerSet asts := manager.asts + gssSpecReserveIds := util.GetReserveOrdinalIntSet(gss.Spec.ReserveGameServerIds) // no need to scale return !(*gss.Spec.Replicas == *asts.Spec.Replicas && - util.IsSliceEqual(util.StringToIntSlice(gss.GetAnnotations()[gameKruiseV1alpha1.GameServerSetReserveIdsKey], ","), gss.Spec.ReserveGameServerIds)) + util.StringToOrdinalIntSet( + gss.GetAnnotations()[gameKruiseV1alpha1.GameServerSetReserveIdsKey], ",", + ).Equal(gssSpecReserveIds)) } func (manager *GameServerSetManager) GameServerScale() error { @@ -120,9 +127,11 @@ func (manager *GameServerSetManager) GameServerScale() error { currentReplicas := len(podList) expectedReplicas := int(*gss.Spec.Replicas) as := gss.GetAnnotations() - reserveIds := util.StringToIntSlice(as[gameKruiseV1alpha1.GameServerSetReserveIdsKey], ",") - notExistIds := util.GetSliceInANotInB(asts.Spec.ReserveOrdinals, reserveIds) - gssReserveIds := gss.Spec.ReserveGameServerIds + specReserveIds := util.GetReserveOrdinalIntSet(asts.Spec.ReserveOrdinals) + reserveIds := util.GetReserveOrdinalIntSet( + util.StringToIntStrSlice(as[gameKruiseV1alpha1.GameServerSetReserveIdsKey], ",")) + notExistIds := util.GetSetInANotInB(specReserveIds, reserveIds) + gssReserveIds := util.GetReserveOrdinalIntSet(gss.Spec.ReserveGameServerIds) 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) @@ -131,13 +140,13 @@ func (manager *GameServerSetManager) GameServerScale() error { newManageIds, newReserveIds := computeToScaleGs(gssReserveIds, reserveIds, notExistIds, expectedReplicas, podList) if gss.Spec.GameServerTemplate.ReclaimPolicy == gameKruiseV1alpha1.DeleteGameServerReclaimPolicy { - err := SyncGameServer(gss, c, newManageIds, util.GetIndexListFromPodList(podList)) + err := SyncGameServer(gss, c, newManageIds, util.GetIndexSetFromPodList(podList)) if err != nil { return err } } - asts.Spec.ReserveOrdinals = newReserveIds + asts.Spec.ReserveOrdinals = util.OrdinalSetToIntStrSlice(newReserveIds) asts.Spec.Replicas = gss.Spec.Replicas asts.Spec.ScaleStrategy = &kruiseV1beta1.StatefulSetScaleStrategy{ MaxUnavailable: gss.Spec.ScaleStrategy.MaxUnavailable, @@ -152,8 +161,8 @@ func (manager *GameServerSetManager) GameServerScale() error { gssReserveIds = newReserveIds } gssAnnotations := make(map[string]string) - gssAnnotations[gameKruiseV1alpha1.GameServerSetReserveIdsKey] = util.IntSliceToString(gssReserveIds, ",") - patchGss := map[string]interface{}{"spec": map[string]interface{}{"reserveGameServerIds": gssReserveIds}, "metadata": map[string]map[string]string{"annotations": gssAnnotations}} + gssAnnotations[gameKruiseV1alpha1.GameServerSetReserveIdsKey] = util.OrdinalSetToString(gssReserveIds) + patchGss := map[string]interface{}{"spec": map[string]interface{}{"reserveGameServerIds": util.OrdinalSetToIntStrSlice(gssReserveIds)}, "metadata": map[string]map[string]string{"annotations": gssAnnotations}} patchGssBytes, _ := json.Marshal(patchGss) err = c.Patch(ctx, gss, client.RawPatch(types.MergePatchType, patchGssBytes)) if err != nil { @@ -169,23 +178,23 @@ func (manager *GameServerSetManager) GameServerScale() error { // 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) { +func computeToScaleGs(gssReserveIds, reserveIds, notExistIds sets.Set[int], expectedReplicas int, pods []corev1.Pod) (workloadManageIds sets.Set[int], newReverseIds sets.Set[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...) + newAddExplicit := util.GetSetInANotInB(gssReserveIds, reserveIds) + newDeleteExplicit := util.GetSetInANotInB(reserveIds, gssReserveIds) + newImplicit := util.GetSetInANotInB(notExistIds, newAddExplicit) + newImplicit = newImplicit.Union(newDeleteExplicit) newExplicit := gssReserveIds // 2. Remove the pods ids is in newExplicit. - var workloadManageIds []int + workloadManageIds = sets.New[int]() var newPods []corev1.Pod for _, pod := range pods { index := util.GetIndexFromGsName(pod.Name) - if util.IsNumInList(index, newExplicit) { + if newExplicit.Has(index) { continue } - workloadManageIds = append(workloadManageIds, index) + workloadManageIds.Insert(index) newPods = append(newPods, pod) } @@ -197,38 +206,38 @@ func computeToScaleGs(gssReserveIds, reserveIds, notExistIds []int, expectedRepl num := 0 var toAdd []int for i := 0; num < expectedReplicas-existReplicas; i++ { - if util.IsNumInList(i, workloadManageIds) || util.IsNumInList(i, newExplicit) { + if workloadManageIds.Has(i) || newExplicit.Has(i) { continue } - if util.IsNumInList(i, newImplicit) { - newImplicit = util.GetSliceInANotInB(newImplicit, []int{i}) + if newImplicit.Has(i) { + newImplicit.Delete(i) } toAdd = append(toAdd, i) num++ } - workloadManageIds = append(workloadManageIds, toAdd...) + workloadManageIds.Insert(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...) + toDelete := util.GetIndexSetFromPodList(sortedGs[:existReplicas-expectedReplicas]) + workloadManageIds = util.GetSetInANotInB(workloadManageIds, toDelete) + newImplicit = newImplicit.Union(toDelete) } - return workloadManageIds, append(newImplicit, newExplicit...) + return workloadManageIds, newImplicit.Union(newExplicit) } -func SyncGameServer(gss *gameKruiseV1alpha1.GameServerSet, c client.Client, newManageIds, oldManageIds []int) error { +func SyncGameServer(gss *gameKruiseV1alpha1.GameServerSet, c client.Client, newManageIds, oldManageIds sets.Set[int]) error { ctx, cancel := context.WithCancel(context.Background()) defer cancel() - addIds := util.GetSliceInANotInB(newManageIds, oldManageIds) - deleteIds := util.GetSliceInANotInB(oldManageIds, newManageIds) + addIds := util.GetSetInANotInB(newManageIds, oldManageIds) + deleteIds := util.GetSetInANotInB(oldManageIds, newManageIds) errch := make(chan error, len(addIds)+len(deleteIds)) var wg sync.WaitGroup - for _, gsId := range append(addIds, deleteIds...) { + for _, gsId := range addIds.Union(deleteIds).UnsortedList() { wg.Add(1) id := gsId go func(ctx context.Context) { @@ -249,7 +258,7 @@ func SyncGameServer(gss *gameKruiseV1alpha1.GameServerSet, c client.Client, newM return } - if util.IsNumInList(id, addIds) && gs.GetLabels()[gameKruiseV1alpha1.GameServerDeletingKey] == "true" { + if addIds.Has(id) && gs.GetLabels()[gameKruiseV1alpha1.GameServerDeletingKey] == "true" { gsLabels := make(map[string]string) gsLabels[gameKruiseV1alpha1.GameServerDeletingKey] = "false" patchGs := map[string]interface{}{"metadata": map[string]map[string]string{"labels": gsLabels}} @@ -266,7 +275,7 @@ func SyncGameServer(gss *gameKruiseV1alpha1.GameServerSet, c client.Client, newM klog.Infof("GameServer %s/%s DeletingKey turn into false", gss.Namespace, gsName) } - if util.IsNumInList(id, deleteIds) && gs.GetLabels()[gameKruiseV1alpha1.GameServerDeletingKey] != "true" { + if deleteIds.Has(id) && gs.GetLabels()[gameKruiseV1alpha1.GameServerDeletingKey] != "true" { gsLabels := make(map[string]string) gsLabels[gameKruiseV1alpha1.GameServerDeletingKey] = "true" patchGs := map[string]interface{}{"metadata": map[string]map[string]string{"labels": gsLabels}} diff --git a/pkg/controllers/gameserverset/gameserverset_manager_test.go b/pkg/controllers/gameserverset/gameserverset_manager_test.go index 8b316f0..ebfb4a4 100644 --- a/pkg/controllers/gameserverset/gameserverset_manager_test.go +++ b/pkg/controllers/gameserverset/gameserverset_manager_test.go @@ -2,6 +2,7 @@ package gameserverset import ( "context" + "fmt" "reflect" "strconv" "testing" @@ -14,7 +15,9 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" + "k8s.io/apimachinery/pkg/util/intstr" utilruntime "k8s.io/apimachinery/pkg/util/runtime" + "k8s.io/apimachinery/pkg/util/sets" "k8s.io/client-go/tools/record" "k8s.io/utils/ptr" "sigs.k8s.io/controller-runtime/pkg/client" @@ -37,19 +40,19 @@ func init() { func TestComputeToScaleGs(t *testing.T) { tests := []struct { - newGssReserveIds []int - oldGssreserveIds []int - notExistIds []int + newGssReserveIds sets.Set[int] + oldGssreserveIds sets.Set[int] + notExistIds sets.Set[int] expectedReplicas int pods []corev1.Pod - newReserveIds []int - newManageIds []int + newReserveIds sets.Set[int] + newManageIds sets.Set[int] }{ // case 0 { - newGssReserveIds: []int{2, 3, 4}, - oldGssreserveIds: []int{2, 3}, - notExistIds: []int{5}, + newGssReserveIds: sets.New(2, 3, 4), + oldGssreserveIds: sets.New(2, 3), + notExistIds: sets.New(5), expectedReplicas: 3, pods: []corev1.Pod{ { @@ -89,14 +92,14 @@ func TestComputeToScaleGs(t *testing.T) { }, }, }, - newReserveIds: []int{2, 3, 4, 5}, - newManageIds: []int{0, 1, 6}, + newReserveIds: sets.New(2, 3, 4, 5), + newManageIds: sets.New(0, 1, 6), }, // case 1 { - newGssReserveIds: []int{0, 2, 3}, - oldGssreserveIds: []int{0, 4, 5}, - notExistIds: []int{}, + newGssReserveIds: sets.New(0, 2, 3), + oldGssreserveIds: sets.New(0, 4, 5), + notExistIds: sets.New[int](), expectedReplicas: 3, pods: []corev1.Pod{ { @@ -145,14 +148,14 @@ func TestComputeToScaleGs(t *testing.T) { }, }, }, - newReserveIds: []int{0, 2, 3, 4, 5}, - newManageIds: []int{1, 6, 7}, + newReserveIds: sets.New(0, 2, 3, 4, 5), + newManageIds: sets.New(1, 6, 7), }, // case 2 { - newGssReserveIds: []int{0}, - oldGssreserveIds: []int{0, 4, 5}, - notExistIds: []int{}, + newGssReserveIds: sets.New(0), + oldGssreserveIds: sets.New(0, 4, 5), + notExistIds: sets.New[int](), expectedReplicas: 1, pods: []corev1.Pod{ { @@ -201,14 +204,14 @@ func TestComputeToScaleGs(t *testing.T) { }, }, }, - newReserveIds: []int{0, 2, 3, 4, 5, 6, 7}, - newManageIds: []int{1}, + newReserveIds: sets.New(0, 2, 3, 4, 5, 6, 7), + newManageIds: sets.New(1), }, // case 3 { - newGssReserveIds: []int{0, 2, 3}, - oldGssreserveIds: []int{0, 4, 5}, - notExistIds: []int{}, + newGssReserveIds: sets.New(0, 2, 3), + oldGssreserveIds: sets.New(0, 4, 5), + notExistIds: sets.New[int](), expectedReplicas: 4, pods: []corev1.Pod{ { @@ -257,14 +260,14 @@ func TestComputeToScaleGs(t *testing.T) { }, }, }, - newReserveIds: []int{0, 2, 3, 5}, - newManageIds: []int{1, 4, 6, 7}, + newReserveIds: sets.New(0, 2, 3, 5), + newManageIds: sets.New(1, 4, 6, 7), }, // case 4 { - newGssReserveIds: []int{0, 3, 5}, - oldGssreserveIds: []int{0, 3, 5}, - notExistIds: []int{}, + newGssReserveIds: sets.New(0, 3, 5), + oldGssreserveIds: sets.New(0, 3, 5), + notExistIds: sets.New[int](), expectedReplicas: 1, pods: []corev1.Pod{ { @@ -304,14 +307,14 @@ func TestComputeToScaleGs(t *testing.T) { }, }, }, - newReserveIds: []int{0, 3, 5, 2, 4, 6}, - newManageIds: []int{1}, + newReserveIds: sets.New(0, 3, 5, 2, 4, 6), + newManageIds: sets.New(1), }, // case 5 { - newGssReserveIds: []int{1, 2}, - oldGssreserveIds: []int{}, - notExistIds: []int{1, 2}, + newGssReserveIds: sets.New(1, 2), + oldGssreserveIds: sets.New[int](), + notExistIds: sets.New(1, 2), expectedReplicas: 2, pods: []corev1.Pod{ { @@ -333,44 +336,44 @@ func TestComputeToScaleGs(t *testing.T) { }, }, }, - newReserveIds: []int{1, 2}, - newManageIds: []int{0, 3}, + newReserveIds: sets.New(1, 2), + newManageIds: sets.New(0, 3), }, // case 6 { - newGssReserveIds: []int{}, - oldGssreserveIds: []int{}, - notExistIds: []int{}, + newGssReserveIds: sets.New[int](), + oldGssreserveIds: sets.New[int](), + notExistIds: sets.New[int](), expectedReplicas: 3, pods: []corev1.Pod{}, - newReserveIds: []int{}, - newManageIds: []int{0, 1, 2}, + newReserveIds: sets.New[int](), + newManageIds: sets.New(0, 1, 2), }, // case 7 { - newGssReserveIds: []int{1, 2}, - oldGssreserveIds: []int{}, - notExistIds: []int{}, + newGssReserveIds: sets.New(1, 2), + oldGssreserveIds: sets.New[int](), + notExistIds: sets.New[int](), expectedReplicas: 3, pods: []corev1.Pod{}, - newReserveIds: []int{1, 2}, - newManageIds: []int{0, 3, 4}, + newReserveIds: sets.New(1, 2), + newManageIds: sets.New(0, 3, 4), }, // case 8 { - newGssReserveIds: []int{0}, - oldGssreserveIds: []int{}, - notExistIds: []int{0}, + newGssReserveIds: sets.New(0), + oldGssreserveIds: sets.New[int](), + notExistIds: sets.New(0), expectedReplicas: 1, pods: []corev1.Pod{}, - newReserveIds: []int{0}, - newManageIds: []int{1}, + newReserveIds: sets.New(0), + newManageIds: sets.New(1), }, // case 9 { - newGssReserveIds: []int{}, - oldGssreserveIds: []int{1}, - notExistIds: []int{}, + newGssReserveIds: sets.New[int](), + oldGssreserveIds: sets.New(1), + notExistIds: sets.New[int](), expectedReplicas: 2, pods: []corev1.Pod{ { @@ -392,14 +395,14 @@ func TestComputeToScaleGs(t *testing.T) { }, }, }, - newReserveIds: []int{1}, - newManageIds: []int{0, 2}, + newReserveIds: sets.New(1), + newManageIds: sets.New(0, 2), }, // case 10 { - newGssReserveIds: []int{0}, - oldGssreserveIds: []int{}, - notExistIds: []int{2, 3, 4}, + newGssReserveIds: sets.New(0), + oldGssreserveIds: sets.New[int](), + notExistIds: sets.New(2, 3, 4), expectedReplicas: 4, pods: []corev1.Pod{ { @@ -412,31 +415,35 @@ func TestComputeToScaleGs(t *testing.T) { }, }, }, - newReserveIds: []int{0}, - newManageIds: []int{1, 2, 3, 4}, + newReserveIds: sets.New(0), + newManageIds: sets.New(1, 2, 3, 4), }, } for i, test := range tests { - 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) + t.Run(fmt.Sprintf("case %d", i), func(t *testing.T) { + 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 !newReserveIds.Equal(test.newReserveIds) { + t.Errorf("case %d: expect newReserveIds %v but got %v", i, test.newReserveIds, newReserveIds) + } + if !newManageIds.Equal(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) + }) } } func TestIsNeedToScale(t *testing.T) { tests := []struct { + name string gss *gameKruiseV1alpha1.GameServerSet asts *kruiseV1beta1.StatefulSet result bool }{ { + name: "case 0", gss: &gameKruiseV1alpha1.GameServerSet{ Spec: gameKruiseV1alpha1.GameServerSetSpec{ Replicas: ptr.To[int32](5), @@ -453,13 +460,14 @@ func TestIsNeedToScale(t *testing.T) { result: false, }, { + name: "case 1", gss: &gameKruiseV1alpha1.GameServerSet{ ObjectMeta: metav1.ObjectMeta{ Annotations: map[string]string{gameKruiseV1alpha1.GameServerSetReserveIdsKey: "1,5"}, }, Spec: gameKruiseV1alpha1.GameServerSetSpec{ Replicas: ptr.To[int32](5), - ReserveGameServerIds: []int{1, 5}, + ReserveGameServerIds: []intstr.IntOrString{intstr.FromInt(1), intstr.FromInt(5)}, }, }, asts: &kruiseV1beta1.StatefulSet{ @@ -472,16 +480,36 @@ func TestIsNeedToScale(t *testing.T) { }, result: false, }, + { + name: "case 2", + gss: &gameKruiseV1alpha1.GameServerSet{ + Spec: gameKruiseV1alpha1.GameServerSetSpec{ + Replicas: ptr.To[int32](5), + ReserveGameServerIds: []intstr.IntOrString{intstr.FromInt(1), intstr.FromInt(5)}, + }, + }, + asts: &kruiseV1beta1.StatefulSet{ + Spec: kruiseV1beta1.StatefulSetSpec{ + Replicas: ptr.To[int32](5), + }, + Status: kruiseV1beta1.StatefulSetStatus{ + Replicas: int32(5), + }, + }, + result: true, + }, } for _, test := range tests { - manager := &GameServerSetManager{ - gameServerSet: test.gss, - asts: test.asts, - } - actual := manager.IsNeedToScale() - if actual != test.result { - t.Errorf("expect spec %v but got %v", test.result, actual) - } + t.Run(test.name, func(t *testing.T) { + manager := &GameServerSetManager{ + gameServerSet: test.gss, + asts: test.asts, + } + actual := manager.IsNeedToScale() + if actual != test.result { + t.Errorf("expect spec %v but got %v", test.result, actual) + } + }) } } @@ -489,14 +517,15 @@ func TestGameServerScale(t *testing.T) { recorder := record.NewFakeRecorder(100) tests := []struct { + name string gss *gameKruiseV1alpha1.GameServerSet asts *kruiseV1beta1.StatefulSet podList []corev1.Pod - astsReserveIds []int + astsReserveIds sets.Set[int] gssReserveIds string }{ - // case0: scale down without reserveIds { + name: "case0: scale down without reserveIds", gss: &gameKruiseV1alpha1.GameServerSet{ ObjectMeta: metav1.ObjectMeta{ Namespace: "xxx", @@ -505,7 +534,7 @@ func TestGameServerScale(t *testing.T) { }, Spec: gameKruiseV1alpha1.GameServerSetSpec{ Replicas: ptr.To[int32](3), - ReserveGameServerIds: []int{1}, + ReserveGameServerIds: []intstr.IntOrString{intstr.FromInt(1)}, }, }, asts: &kruiseV1beta1.StatefulSet{ @@ -515,7 +544,7 @@ func TestGameServerScale(t *testing.T) { }, Spec: kruiseV1beta1.StatefulSetSpec{ Replicas: ptr.To[int32](4), - ReserveOrdinals: []int{1}, + ReserveOrdinals: []intstr.IntOrString{intstr.FromInt(1)}, }, Status: kruiseV1beta1.StatefulSetStatus{ Replicas: int32(4), @@ -559,11 +588,11 @@ func TestGameServerScale(t *testing.T) { }, }, }, - astsReserveIds: []int{1, 2}, + astsReserveIds: sets.New(1, 2), gssReserveIds: "1", }, - // case1: scale down with reserveIds { + name: "case1: scale down with reserveIds", gss: &gameKruiseV1alpha1.GameServerSet{ ObjectMeta: metav1.ObjectMeta{ Namespace: "xxx", @@ -572,7 +601,7 @@ func TestGameServerScale(t *testing.T) { }, Spec: gameKruiseV1alpha1.GameServerSetSpec{ Replicas: ptr.To[int32](3), - ReserveGameServerIds: []int{1, 0}, + ReserveGameServerIds: []intstr.IntOrString{intstr.FromInt(1), intstr.FromInt(0)}, }, }, asts: &kruiseV1beta1.StatefulSet{ @@ -582,7 +611,7 @@ func TestGameServerScale(t *testing.T) { }, Spec: kruiseV1beta1.StatefulSetSpec{ Replicas: ptr.To[int32](4), - ReserveOrdinals: []int{1}, + ReserveOrdinals: []intstr.IntOrString{intstr.FromInt(1)}, }, Status: kruiseV1beta1.StatefulSetStatus{ Replicas: int32(4), @@ -626,11 +655,11 @@ func TestGameServerScale(t *testing.T) { }, }, }, - astsReserveIds: []int{1, 0}, - gssReserveIds: "1,0", + astsReserveIds: sets.New(0, 1), + gssReserveIds: "0,1", }, - // case2: scale up with reserveIds { + name: "case2: scale up with reserveIds", gss: &gameKruiseV1alpha1.GameServerSet{ ObjectMeta: metav1.ObjectMeta{ Namespace: "xxx", @@ -639,7 +668,7 @@ func TestGameServerScale(t *testing.T) { }, Spec: gameKruiseV1alpha1.GameServerSetSpec{ Replicas: ptr.To[int32](5), - ReserveGameServerIds: []int{}, + ReserveGameServerIds: []intstr.IntOrString{}, }, }, asts: &kruiseV1beta1.StatefulSet{ @@ -649,7 +678,7 @@ func TestGameServerScale(t *testing.T) { }, Spec: kruiseV1beta1.StatefulSetSpec{ Replicas: ptr.To[int32](4), - ReserveOrdinals: []int{1}, + ReserveOrdinals: []intstr.IntOrString{intstr.FromInt(1)}, }, Status: kruiseV1beta1.StatefulSetStatus{ Replicas: int32(4), @@ -696,8 +725,8 @@ func TestGameServerScale(t *testing.T) { astsReserveIds: nil, gssReserveIds: "", }, - // case3: scale up with both reserveIds and others { + name: "case3: scale up with both reserveIds and others", gss: &gameKruiseV1alpha1.GameServerSet{ ObjectMeta: metav1.ObjectMeta{ Namespace: "xxx", @@ -706,7 +735,7 @@ func TestGameServerScale(t *testing.T) { }, Spec: gameKruiseV1alpha1.GameServerSetSpec{ Replicas: ptr.To[int32](5), - ReserveGameServerIds: []int{}, + ReserveGameServerIds: []intstr.IntOrString{}, }, }, asts: &kruiseV1beta1.StatefulSet{ @@ -716,7 +745,7 @@ func TestGameServerScale(t *testing.T) { }, Spec: kruiseV1beta1.StatefulSetSpec{ Replicas: ptr.To[int32](3), - ReserveOrdinals: []int{1, 3}, + ReserveOrdinals: []intstr.IntOrString{intstr.FromInt(1), intstr.FromInt(3)}, }, Status: kruiseV1beta1.StatefulSetStatus{ Replicas: int32(3), @@ -757,41 +786,44 @@ func TestGameServerScale(t *testing.T) { } for _, test := range tests { - objs := []client.Object{test.asts, test.gss} - c := fake.NewClientBuilder().WithScheme(scheme).WithObjects(objs...).Build() - manager := &GameServerSetManager{ - gameServerSet: test.gss, - asts: test.asts, - podList: test.podList, - eventRecorder: recorder, - client: c, - } + t.Run(test.name, func(t *testing.T) { + objs := []client.Object{test.asts, test.gss} + c := fake.NewClientBuilder().WithScheme(scheme).WithObjects(objs...).Build() + manager := &GameServerSetManager{ + gameServerSet: test.gss, + asts: test.asts, + podList: test.podList, + eventRecorder: recorder, + client: c, + } - if err := manager.GameServerScale(); err != nil { - t.Error(err) - } + if err := manager.GameServerScale(); err != nil { + t.Error(err) + } - updateAsts := &kruiseV1beta1.StatefulSet{} - if err := manager.client.Get(context.TODO(), types.NamespacedName{ - Namespace: test.asts.Namespace, - Name: test.asts.Name, - }, updateAsts); err != nil { - t.Error(err) - } - if !util.IsSliceEqual(updateAsts.Spec.ReserveOrdinals, test.astsReserveIds) { - t.Errorf("expect asts ReserveOrdinals %v but got %v", test.astsReserveIds, updateAsts.Spec.ReserveOrdinals) - } + updateAsts := &kruiseV1beta1.StatefulSet{} + if err := manager.client.Get(context.TODO(), types.NamespacedName{ + Namespace: test.asts.Namespace, + Name: test.asts.Name, + }, updateAsts); err != nil { + t.Error(err) + } + gotIds := util.GetReserveOrdinalIntSet(updateAsts.Spec.ReserveOrdinals) + if !gotIds.Equal(test.astsReserveIds) { + t.Errorf("expect asts ReserveOrdinals %v but got %v", test.astsReserveIds, gotIds) + } - updateGss := &gameKruiseV1alpha1.GameServerSet{} - if err := manager.client.Get(context.TODO(), types.NamespacedName{ - Namespace: test.gss.Namespace, - Name: test.gss.Name, - }, updateGss); err != nil { - t.Error(err) - } - if updateGss.GetAnnotations()[gameKruiseV1alpha1.GameServerSetReserveIdsKey] != test.gssReserveIds { - t.Errorf("expect asts ReserveOrdinals %v but got %v", test.gssReserveIds, updateGss.GetAnnotations()[gameKruiseV1alpha1.GameServerSetReserveIdsKey]) - } + updateGss := &gameKruiseV1alpha1.GameServerSet{} + if err := manager.client.Get(context.TODO(), types.NamespacedName{ + Namespace: test.gss.Namespace, + Name: test.gss.Name, + }, updateGss); err != nil { + t.Error(err) + } + if updateGss.GetAnnotations()[gameKruiseV1alpha1.GameServerSetReserveIdsKey] != test.gssReserveIds { + t.Errorf("expect asts ReserveOrdinals %v but got %v", test.gssReserveIds, updateGss.GetAnnotations()[gameKruiseV1alpha1.GameServerSetReserveIdsKey]) + } + }) } } @@ -799,8 +831,8 @@ func TestSyncGameServer(t *testing.T) { tests := []struct { gss *gameKruiseV1alpha1.GameServerSet gsList []*gameKruiseV1alpha1.GameServer - newManageIds []int - oldManageIds []int + newManageIds sets.Set[int] + oldManageIds sets.Set[int] IdsLabelTure []int IdsLabelFalse []int }{ @@ -850,8 +882,8 @@ func TestSyncGameServer(t *testing.T) { }, }, }, - oldManageIds: []int{0, 2, 3, 4}, - newManageIds: []int{0, 1}, + oldManageIds: sets.New(0, 2, 3, 4), + newManageIds: sets.New(0, 1), IdsLabelTure: []int{2, 3, 4}, IdsLabelFalse: []int{}, }, @@ -865,8 +897,8 @@ func TestSyncGameServer(t *testing.T) { }, }, gsList: []*gameKruiseV1alpha1.GameServer{}, - oldManageIds: []int{}, - newManageIds: []int{0, 1, 3}, + oldManageIds: sets.New[int](), + newManageIds: sets.New(0, 1, 3), IdsLabelTure: []int{}, IdsLabelFalse: []int{}, }, @@ -891,8 +923,8 @@ func TestSyncGameServer(t *testing.T) { }, }, }, - oldManageIds: []int{}, - newManageIds: []int{0}, + oldManageIds: sets.New[int](), + newManageIds: sets.New(0), IdsLabelTure: []int{}, IdsLabelFalse: []int{0}, }, diff --git a/pkg/util/gameserver.go b/pkg/util/gameserver.go index 2f8e013..a73131d 100644 --- a/pkg/util/gameserver.go +++ b/pkg/util/gameserver.go @@ -27,6 +27,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/intstr" + "k8s.io/apimachinery/pkg/util/sets" "k8s.io/utils/ptr" "sigs.k8s.io/controller-runtime/pkg/client" @@ -97,6 +98,10 @@ func GetIndexListFromPodList(podList []corev1.Pod) []int { return indexList } +func GetIndexSetFromPodList(podList []corev1.Pod) sets.Set[int] { + return sets.New[int](GetIndexListFromPodList(podList)...) +} + func GetIndexListFromGsList(gsList []gameKruiseV1alpha1.GameServer) []int { var indexList []int for i := 0; i < len(gsList); i++ { diff --git a/pkg/util/set.go b/pkg/util/set.go new file mode 100644 index 0000000..6299a0e --- /dev/null +++ b/pkg/util/set.go @@ -0,0 +1,199 @@ +/* +Copyright 2022 The Kruise Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package util + +import ( + "fmt" + "slices" + "strconv" + "strings" + + "k8s.io/apimachinery/pkg/util/intstr" + "k8s.io/apimachinery/pkg/util/sets" + "k8s.io/klog/v2" + + "golang.org/x/exp/constraints" +) + +// see github.com/openkruise/kruise/pkg/util/api/asts.go + +// ParseRange parses the start and end value from a string like "1-3" +func ParseRange(s string) (start int, end int, err error) { + split := strings.Split(s, "-") + if len(split) != 2 { + return 0, 0, fmt.Errorf("invalid range %s", s) + } + start, err = strconv.Atoi(strings.TrimSpace(split[0])) + if err != nil { + return + } + end, err = strconv.Atoi(strings.TrimSpace(split[1])) + if err != nil { + return + } + if start > end { + return 0, 0, fmt.Errorf("invalid range %s", s) + } + return +} + +// GetReserveOrdinalIntSet returns a set of ints from parsed reserveOrdinal +func GetReserveOrdinalIntSet(r []intstr.IntOrString) sets.Set[int] { + values := sets.New[int]() + for _, elem := range r { + if elem.Type == intstr.Int { + values.Insert(int(elem.IntVal)) + } else { + start, end, err := ParseRange(elem.StrVal) + if err != nil { + klog.ErrorS(err, "invalid range reserveOrdinal found, an empty slice will be returned", "reserveOrdinal", elem.StrVal) + return nil + } + for i := start; i <= end; i++ { + values.Insert(i) + } + } + } + return values +} + +// StringToOrdinalIntSet convert a string to a set of ordinals, +// support ranged ordinals like "1-3,5-7,10" +// eg, "1, 2-5, 7" -> {1, 2, 3, 4, 5, 7} +func StringToOrdinalIntSet(str string, delimiter string) sets.Set[int] { + ret := sets.New[int]() + if str == "" { + return ret + } + + strList := strings.Split(str, delimiter) + if len(strList) == 0 { + return ret + } + + for _, s := range strList { + if strings.Contains(s, "-") { + start, end, err := ParseRange(s) + if err != nil { + klog.ErrorS(err, "invalid range found, skip", "range", s) + continue + } + for i := start; i <= end; i++ { + ret.Insert(i) + } + } else { + num, err := strconv.Atoi(strings.TrimSpace(s)) + if err != nil { + klog.ErrorS(err, "invalid number found, skip", "number", s) + continue + } + ret.Insert(num) + } + } + + return ret +} + +// OrdinalSetToIntStrSlice convert a set of oridinals to a ranged intstr slice +// e.g. {1, 2, 5, 6, 7, 10} -> ["1", "2", "5-7", 10] +func OrdinalSetToIntStrSlice[T constraints.Integer](s sets.Set[T]) []intstr.IntOrString { + if s.Len() == 0 { + return nil + } + + // get all ordinals and sort them + ordinals := s.UnsortedList() + slices.Sort(ordinals) + + var ret []intstr.IntOrString + if len(ordinals) == 0 { + return ret + } + + // Initialize sequence tracking + start := ordinals[0] + end := start + + // Process all ordinals + for i := 1; i < len(ordinals); i++ { + curr := ordinals[i] + if curr == end+1 { + // Continue the current sequence + end = curr + } else { + // Add the completed sequence to results + appendSequence(&ret, start, end) + // Start a new sequence + start = curr + end = curr + } + } + + // Handle the final sequence + appendSequence(&ret, start, end) + + return ret +} + +// Helper function to append a sequence to the result slice +func appendSequence[T constraints.Integer](ret *[]intstr.IntOrString, start, end T) { + if end < start { + start, end = end, start + } + switch { + case start == end: + *ret = append(*ret, intstr.FromInt(int(start))) + case end-start == 1: + *ret = append(*ret, intstr.FromInt(int(start)), intstr.FromInt(int(end))) + default: + *ret = append(*ret, intstr.FromString(fmt.Sprintf("%d-%d", start, end))) + } +} + +// OrdinalSetToString convert a set of ordinals to a string with default delimiter ",", +// e.g. {1, 2, 5, 6, 7, 10} -> "1,2,5-7,10" +func OrdinalSetToString(s sets.Set[int]) string { + return intSetToString(s, ",") +} + +func intSetToString(s sets.Set[int], delimiter string) string { + if s.Len() == 0 { + return "" + } + // get all ordinals and sort them + ss := OrdinalSetToIntStrSlice(s) + ret := make([]string, 0, len(ss)) + for _, elem := range ss { + if elem.Type == intstr.Int { + ret = append(ret, strconv.Itoa(int(elem.IntVal))) + } else { + ret = append(ret, elem.StrVal) + } + } + return strings.Join(ret, delimiter) +} + +// GetSetInANotInB returns a set of elements that are in set a but not in set b +func GetSetInANotInB[T comparable](a, b sets.Set[T]) sets.Set[T] { + ret := sets.New[T]() + for elem := range a { + if !b.Has(elem) { + ret.Insert(elem) + } + } + return ret +} diff --git a/pkg/util/set_test.go b/pkg/util/set_test.go new file mode 100644 index 0000000..eeb0677 --- /dev/null +++ b/pkg/util/set_test.go @@ -0,0 +1,272 @@ +/* +Copyright 2022 The Kruise Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package util + +import ( + "reflect" + "testing" + + "k8s.io/apimachinery/pkg/util/intstr" + "k8s.io/apimachinery/pkg/util/sets" +) + +func TestOrdinalSetToIntStrSlice(t *testing.T) { + tests := []struct { + name string + input sets.Set[int] + expected []intstr.IntOrString + }{ + { + name: "single element", + input: sets.New(5), + expected: []intstr.IntOrString{intstr.FromInt(5)}, + }, + { + name: "continuous elements", + input: sets.New(1, 2, 3, 4, 5), + expected: []intstr.IntOrString{intstr.FromString("1-5")}, + }, + { + name: "multiple continuous elements", + input: sets.New(1, 2, 3, 5, 6, 7), + expected: []intstr.IntOrString{ + intstr.FromString("1-3"), + intstr.FromString("5-7"), + }, + }, + { + name: "multiple continuous elements with single element", + input: sets.New(1, 2, 3, 5, 7, 8, 9, 11), + expected: []intstr.IntOrString{ + intstr.FromString("1-3"), + intstr.FromInt(5), + intstr.FromString("7-9"), + intstr.FromInt(11), + }, + }, + { + name: "unsorted continuous elements", + input: sets.New(3, 1, 2, 4, 8, 6, 7), + expected: []intstr.IntOrString{ + intstr.FromString("1-4"), + intstr.FromString("6-8"), + }, + }, + { + name: "non-continuous elements", + input: sets.New(1, 2, 5, 7, 9), + expected: []intstr.IntOrString{ + intstr.FromInt(1), + intstr.FromInt(2), + intstr.FromInt(5), + intstr.FromInt(7), + intstr.FromInt(9), + }, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + result := OrdinalSetToIntStrSlice(tc.input) + + if !reflect.DeepEqual(result, tc.expected) { + t.Errorf("OrdinalSetToIntStrSlice(%v) = %v, expected %v", tc.input.UnsortedList(), result, tc.expected) + } + }) + } +} + +// test OrdinalSetToIntStrSlice with different types +func TestOrdinalSetToIntStrSliceWithDifferentTypes(t *testing.T) { + int32Set := sets.New[int32](1, 2, 3, 5) + expected := []intstr.IntOrString{ + intstr.FromString("1-3"), + intstr.FromInt(5), + } + + result := OrdinalSetToIntStrSlice(int32Set) + if !reflect.DeepEqual(result, expected) { + t.Errorf("use int32 type test failed: got %v, expected %v", result, expected) + } + + // 测试uint类型 + uintSet := sets.New[uint](10, 11, 12, 15) + expected = []intstr.IntOrString{ + intstr.FromString("10-12"), + intstr.FromInt(15), + } + + result = OrdinalSetToIntStrSlice(uintSet) + if !reflect.DeepEqual(result, expected) { + t.Errorf("use uint type test failed: got %v, expected %v", result, expected) + } +} + +func TestStringToOrdinalIntSet(t *testing.T) { + tests := []struct { + name string + str string + delimiter string + expected sets.Set[int] + }{ + { + name: "empty string", + str: "", + delimiter: ",", + expected: sets.New[int](), + }, + { + name: "single number", + str: "5", + delimiter: ",", + expected: sets.New(5), + }, + { + name: "multiple numbers", + str: "1,3,5,7", + delimiter: ",", + expected: sets.New(1, 3, 5, 7), + }, + { + name: "single range", + str: "1-5", + delimiter: ",", + expected: sets.New(1, 2, 3, 4, 5), + }, + { + name: "multiple ranges", + str: "1-3,7-9", + delimiter: ",", + expected: sets.New(1, 2, 3, 7, 8, 9), + }, + { + name: "mixed numbers and ranges", + str: "1-3,5,7-9,11", + delimiter: ",", + expected: sets.New(1, 2, 3, 5, 7, 8, 9, 11), + }, + { + name: "with spaces", + str: "1-3, 5, 7-9, 11", + delimiter: ",", + expected: sets.New(1, 2, 3, 5, 7, 8, 9, 11), + }, + { + name: "different delimiter", + str: "1-3;5;7-9;11", + delimiter: ";", + expected: sets.New(1, 2, 3, 5, 7, 8, 9, 11), + }, + { + name: "invalid number", + str: "1,abc,3", + delimiter: ",", + expected: sets.New(1, 3), + }, + { + name: "invalid range", + str: "1-3,5-abc,7-9", + delimiter: ",", + expected: sets.New(1, 2, 3, 7, 8, 9), + }, + { + name: "inverted range", + str: "1-3,5-2,7-9", + delimiter: ",", + expected: sets.New(1, 2, 3, 7, 8, 9), + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + result := StringToOrdinalIntSet(tc.str, tc.delimiter) + + if !result.Equal(tc.expected) { + t.Errorf("StringToOrdinalIntSet(%q, %q) = %v, expected %v", + tc.str, tc.delimiter, result.UnsortedList(), tc.expected.UnsortedList()) + } + }) + } +} + +func TestIntSetToString(t *testing.T) { + tests := []struct { + name string + set sets.Set[int] + delimiter string + expected string + }{ + { + name: "empty set", + set: sets.New[int](), + delimiter: ",", + expected: "", + }, + { + name: "single element", + set: sets.New(5), + delimiter: ",", + expected: "5", + }, + { + name: "multiple elements", + set: sets.New(1, 3, 5, 7), + delimiter: ",", + expected: "1,3,5,7", + }, + { + name: "continuous elements", + set: sets.New(1, 2, 3, 4, 5), + delimiter: ",", + expected: "1-5", + }, + { + name: "mixed continuous and single elements", + set: sets.New(1, 2, 3, 5, 7, 8, 9, 11), + delimiter: ",", + expected: "1-3,5,7-9,11", + }, + { + name: "unsorted elements", + set: sets.New(5, 3, 1, 2, 4), + delimiter: ",", + expected: "1-5", + }, + { + name: "different delimiter", + set: sets.New(1, 2, 3, 5, 7), + delimiter: ";", + expected: "1-3;5;7", + }, + { + name: "non-continuous elements", + set: sets.New(1, 2, 5), + delimiter: ",", + expected: "1,2,5", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + result := intSetToString(tc.set, tc.delimiter) + if result != tc.expected { + t.Errorf("intSetToString(%v, %q) = %q, expected %q", + tc.set.UnsortedList(), tc.delimiter, result, tc.expected) + } + }) + } +} diff --git a/pkg/util/slice.go b/pkg/util/slice.go index 56f0859..4174b54 100644 --- a/pkg/util/slice.go +++ b/pkg/util/slice.go @@ -21,6 +21,8 @@ import ( "sort" "strconv" "strings" + + "k8s.io/apimachinery/pkg/util/intstr" ) func IsNumInList(num int, list []int) bool { @@ -91,6 +93,29 @@ func StringToIntSlice(str string, delimiter string) []int { return retSlice } +func StringToIntStrSlice(str string, delimiter string) []intstr.IntOrString { + if str == "" || delimiter == "" { + return nil + } + strList := strings.Split(str, delimiter) + if len(strList) == 0 { + return nil + } + var retSlice []intstr.IntOrString + for _, item := range strList { + if item == "" { + continue + } + val, err := strconv.Atoi(item) + if err != nil { + retSlice = append(retSlice, intstr.FromString(strings.TrimSpace(item))) + } else { + retSlice = append(retSlice, intstr.FromInt(val)) + } + } + return retSlice +} + func StringToInt32Slice(str string, delimiter string) []int32 { if str == "" { return nil diff --git a/pkg/util/slice_test.go b/pkg/util/slice_test.go index 5a9a8d1..0587560 100644 --- a/pkg/util/slice_test.go +++ b/pkg/util/slice_test.go @@ -16,7 +16,11 @@ limitations under the License. package util -import "testing" +import ( + "testing" + + "k8s.io/apimachinery/pkg/util/intstr" +) func TestIsNumInList(t *testing.T) { tests := []struct { @@ -331,3 +335,110 @@ func TestIsHasNegativeNum(t *testing.T) { } } } + +func TestStringToIntStrSlice(t *testing.T) { + tests := []struct { + name string + str string + delimiter string + result []intstr.IntOrString + }{ + { + name: "mixed int and string values", + str: "4,test,1", + delimiter: ",", + result: []intstr.IntOrString{ + intstr.FromInt(4), + intstr.FromString("test"), + intstr.FromInt(1), + }, + }, + { + name: "only int values", + str: "4,5,1", + delimiter: ",", + result: []intstr.IntOrString{ + intstr.FromInt(4), + intstr.FromInt(5), + intstr.FromInt(1), + }, + }, + { + name: "only string values", + str: "a,b,c", + delimiter: ",", + result: []intstr.IntOrString{ + intstr.FromString("a"), + intstr.FromString("b"), + intstr.FromString("c"), + }, + }, + { + name: "empty string", + str: "", + delimiter: ",", + result: nil, + }, + { + name: "empty delimiter", + str: "1,2,3", + delimiter: "", + result: nil, + }, + { + name: "empty parts", + str: "1,,3", + delimiter: ",", + result: []intstr.IntOrString{ + intstr.FromInt(1), + intstr.FromInt(3), + }, + }, + { + name: "different delimiter", + str: "1:test:3", + delimiter: ":", + result: []intstr.IntOrString{ + intstr.FromInt(1), + intstr.FromString("test"), + intstr.FromInt(3), + }, + }, + { + name: "reversed ids slice", + str: "1,2-5,6,7-10", + delimiter: ",", + result: []intstr.IntOrString{ + intstr.FromInt(1), + intstr.FromString("2-5"), + intstr.FromInt(6), + intstr.FromString("7-10"), + }, + }, + { + name: "has space in the string", + str: "1, 2-3, 4", + delimiter: ",", + result: []intstr.IntOrString{ + intstr.FromInt(1), + intstr.FromString("2-3"), + intstr.FromInt(4), + }, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + actual := StringToIntStrSlice(test.str, test.delimiter) + if len(actual) != len(test.result) { + t.Errorf("expect length %v but got %v", len(test.result), len(actual)) + return + } + for i := range len(actual) { + if test.result[i].String() != actual[i].String() { + t.Errorf("index %d: expect %v but got %v", i, test.result[i], actual[i]) + } + } + }) + } +} diff --git a/pkg/webhook/validating_gss.go b/pkg/webhook/validating_gss.go index 899a5c4..ecd32f8 100644 --- a/pkg/webhook/validating_gss.go +++ b/pkg/webhook/validating_gss.go @@ -25,6 +25,8 @@ import ( "github.com/openkruise/kruise-game/cloudprovider/manager" "github.com/openkruise/kruise-game/pkg/util" admissionv1 "k8s.io/api/admission/v1" + "k8s.io/apimachinery/pkg/util/intstr" + "k8s.io/apimachinery/pkg/util/sets" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/webhook/admission" ) @@ -65,12 +67,50 @@ func (gvh *GssValidaatingHandler) Handle(ctx context.Context, req admission.Requ func validatingGss(gss *gamekruiseiov1alpha1.GameServerSet, client client.Client) (bool, string) { // validate reserveGameServerIds - rgsIds := gss.Spec.ReserveGameServerIds - if util.IsRepeat(rgsIds) { - return false, fmt.Sprintf("reserveGameServerIds should not be repeat. Now it is %v", rgsIds) + vset := sets.Set[int]{} + + validate := func(ids intstr.IntOrString) (bool, string) { + switch ids.Type { + case intstr.Int: + id := ids.IntVal + if id < 0 { + return false, fmt.Sprintf("reserveGameServerIds should be greater or equal to 0. Now it is %d", id) + } + if vset.Has(int(id)) { + return false, fmt.Sprintf("reserveGameServerIds should not be repeat. Now it is %d", id) + } + vset.Insert(int(id)) + case intstr.String: + start, end, err := util.ParseRange(ids.StrVal) + if err != nil { + return false, fmt.Sprintf("invalid range reserveGameServerIds found, an empty slice will be returned: %s", ids.StrVal) + } + if start < 0 { + return false, fmt.Sprintf("reserveGameServerIds should be greater or equal to 0. Now it is %d", start) + } + if end < 0 { + return false, fmt.Sprintf("reserveGameServerIds should be greater or equal to 0. Now it is %d", end) + } + if start > end { + return false, fmt.Sprintf("invalid range reserveGameServerIds found, an empty slice will be returned: %s", ids.StrVal) + } + if vset.Has(int(start)) || vset.Has(int(end)) { + return false, fmt.Sprintf("reserveGameServerIds should not be repeat. Now it is %d-%d", start, end) + } + for i := start; i <= end; i++ { + if vset.Has(int(i)) { + return false, fmt.Sprintf("reserveGameServerIds should not be repeat. Now it is %d-%d", start, end) + } + vset.Insert(int(i)) + } + } + return true, "" } - if util.IsHasNegativeNum(rgsIds) { - return false, fmt.Sprintf("reserveGameServerIds should be greater or equal to 0. Now it is %v", rgsIds) + + for _, id := range gss.Spec.ReserveGameServerIds { + if ok, reason := validate(id); !ok { + return false, reason + } } return true, "general validating success" diff --git a/test/e2e/framework/framework.go b/test/e2e/framework/framework.go index 90447ba..3d00fae 100644 --- a/test/e2e/framework/framework.go +++ b/test/e2e/framework/framework.go @@ -124,7 +124,7 @@ func (f *Framework) DeployGssWithServiceQualities() (*gamekruiseiov1alpha1.GameS return f.client.CreateGameServerSet(gss) } -func (f *Framework) GameServerScale(gss *gamekruiseiov1alpha1.GameServerSet, desireNum int, reserveGsId *int) (*gamekruiseiov1alpha1.GameServerSet, error) { +func (f *Framework) GameServerScale(gss *gamekruiseiov1alpha1.GameServerSet, desireNum int, reserveGsId *intstr.IntOrString) (*gamekruiseiov1alpha1.GameServerSet, error) { // TODO: change patch type newReserves := gss.Spec.ReserveGameServerIds if reserveGsId != nil {