From 69babe66fecc012739cde55c0cbf60a6ddd3584e Mon Sep 17 00:00:00 2001 From: ChrisLiu <70144550+chrisliu1995@users.noreply.github.com> Date: Wed, 27 Mar 2024 10:25:07 +0800 Subject: [PATCH] replace patch asts with update (#131) Signed-off-by: ChrisLiu --- .../gameserverset/gameserverset_manager.go | 19 ++-- .../gameserverset_manager_test.go | 107 ++++++++++++++++++ 2 files changed, 117 insertions(+), 9 deletions(-) diff --git a/pkg/controllers/gameserverset/gameserverset_manager.go b/pkg/controllers/gameserverset/gameserverset_manager.go index 4703ff8d..ba0a053a 100644 --- a/pkg/controllers/gameserverset/gameserverset_manager.go +++ b/pkg/controllers/gameserverset/gameserverset_manager.go @@ -27,6 +27,7 @@ import ( "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/json" "k8s.io/client-go/tools/record" + "k8s.io/client-go/util/retry" "k8s.io/klog/v2" "k8s.io/utils/pointer" "sigs.k8s.io/controller-runtime/pkg/client" @@ -326,16 +327,16 @@ func (manager *GameServerSetManager) UpdateWorkload() error { asts := manager.asts // sync with Advanced StatefulSet - asts = util.GetNewAstsFromGss(gss.DeepCopy(), asts) - astsAns := asts.GetAnnotations() - astsAns[gameKruiseV1alpha1.AstsHashKey] = util.GetAstsHash(manager.gameServerSet) + retryErr := retry.RetryOnConflict(retry.DefaultRetry, func() error { + asts = util.GetNewAstsFromGss(gss.DeepCopy(), asts) + astsAns := asts.GetAnnotations() + astsAns[gameKruiseV1alpha1.AstsHashKey] = util.GetAstsHash(manager.gameServerSet) + asts.SetAnnotations(astsAns) - patchAsts := map[string]interface{}{"metadata": map[string]map[string]string{"annotations": astsAns}, "spec": asts.Spec} - patchAstsBytes, err := json.Marshal(patchAsts) - if err != nil { - return err - } - return manager.client.Patch(context.TODO(), asts, client.RawPatch(types.MergePatchType, patchAstsBytes)) + return manager.client.Update(context.TODO(), asts) + }) + + return retryErr } func (manager *GameServerSetManager) SyncPodProbeMarker() error { diff --git a/pkg/controllers/gameserverset/gameserverset_manager_test.go b/pkg/controllers/gameserverset/gameserverset_manager_test.go index 37c70144..e64d8cd6 100644 --- a/pkg/controllers/gameserverset/gameserverset_manager_test.go +++ b/pkg/controllers/gameserverset/gameserverset_manager_test.go @@ -2,10 +2,12 @@ package gameserverset import ( "context" + appspub "github.com/openkruise/kruise-api/apps/pub" kruiseV1alpha1 "github.com/openkruise/kruise-api/apps/v1alpha1" kruiseV1beta1 "github.com/openkruise/kruise-api/apps/v1beta1" gameKruiseV1alpha1 "github.com/openkruise/kruise-game/apis/v1alpha1" "github.com/openkruise/kruise-game/pkg/util" + apps "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" @@ -13,6 +15,7 @@ import ( utilruntime "k8s.io/apimachinery/pkg/util/runtime" "k8s.io/client-go/tools/record" "k8s.io/utils/pointer" + "reflect" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/fake" "strconv" @@ -1060,3 +1063,107 @@ func TestNumberToKill(t *testing.T) { } } } + +func TestGameServerSetManager_UpdateWorkload(t *testing.T) { + tests := []struct { + gss *gameKruiseV1alpha1.GameServerSet + asts *kruiseV1beta1.StatefulSet + newAsts *kruiseV1beta1.StatefulSet + }{ + { + gss: &gameKruiseV1alpha1.GameServerSet{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "xxx", + Name: "case0", + }, + Spec: gameKruiseV1alpha1.GameServerSetSpec{ + GameServerTemplate: gameKruiseV1alpha1.GameServerTemplate{}, + }, + }, + asts: &kruiseV1beta1.StatefulSet{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "xxx", + Name: "case0", + Annotations: map[string]string{gameKruiseV1alpha1.AstsHashKey: "xx"}, + }, + Spec: kruiseV1beta1.StatefulSetSpec{ + Template: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Affinity: &corev1.Affinity{ + NodeAffinity: &corev1.NodeAffinity{ + PreferredDuringSchedulingIgnoredDuringExecution: []corev1.PreferredSchedulingTerm{ + { + Weight: 1, + Preference: corev1.NodeSelectorTerm{ + MatchFields: []corev1.NodeSelectorRequirement{ + { + Key: "role", + Operator: corev1.NodeSelectorOpIn, + Values: []string{"test"}, + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + newAsts: &kruiseV1beta1.StatefulSet{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "xxx", + Name: "case0", + Annotations: map[string]string{gameKruiseV1alpha1.AstsHashKey: "xxx"}, + }, + Spec: kruiseV1beta1.StatefulSetSpec{ + ScaleStrategy: &kruiseV1beta1.StatefulSetScaleStrategy{ + MaxUnavailable: nil, + }, + PodManagementPolicy: apps.ParallelPodManagement, + Template: corev1.PodTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{gameKruiseV1alpha1.GameServerOwnerGssKey: "case0"}, + }, + Spec: corev1.PodSpec{ + ReadinessGates: []corev1.PodReadinessGate{ + { + ConditionType: appspub.InPlaceUpdateReady, + }, + }, + }, + }, + }, + }, + }, + } + recorder := record.NewFakeRecorder(100) + + 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, + eventRecorder: recorder, + client: c, + } + + if err := manager.UpdateWorkload(); 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 !reflect.DeepEqual(updateAsts.Spec, test.newAsts.Spec) { + t.Errorf("expect new asts spec %v but got %v", test.newAsts.Spec, updateAsts.Spec) + } + } +}