From 082ee009be2d1061f320df9ca584aa4dfe5da681 Mon Sep 17 00:00:00 2001 From: ChrisLiu <70144550+chrisliu1995@users.noreply.github.com> Date: Thu, 3 Nov 2022 17:53:40 +0800 Subject: [PATCH] Refactoring GameServer Lifecycle Management (#10) * refactor: gs will not be deleted due to pod deletion and opt format --- apis/v1alpha1/gameserver_types.go | 1 + .../gameserver/gameserver_controller.go | 54 ++++++++++------ .../gameserverset/gameserverset_manager.go | 61 +++++++++++++++++-- test/e2e/client/client.go | 8 +++ test/e2e/framework/framework.go | 19 ++++++ test/e2e/testcase/testcase.go | 22 +++++++ 6 files changed, 144 insertions(+), 21 deletions(-) diff --git a/apis/v1alpha1/gameserver_types.go b/apis/v1alpha1/gameserver_types.go index 406c0c0..da62434 100644 --- a/apis/v1alpha1/gameserver_types.go +++ b/apis/v1alpha1/gameserver_types.go @@ -27,6 +27,7 @@ const ( GameServerOpsStateKey = "game.kruise.io/gs-opsState" GameServerUpdatePriorityKey = "game.kruise.io/gs-update-priority" GameServerDeletePriorityKey = "game.kruise.io/gs-delete-priority" + GameServerDeletingKey = "game.kruise.io/gs-deleting" ) // GameServerSpec defines the desired state of GameServer diff --git a/pkg/controllers/gameserver/gameserver_controller.go b/pkg/controllers/gameserver/gameserver_controller.go index 50c627c..adba8c3 100644 --- a/pkg/controllers/gameserver/gameserver_controller.go +++ b/pkg/controllers/gameserver/gameserver_controller.go @@ -72,10 +72,7 @@ func add(mgr manager.Manager, r reconcile.Reconciler) error { klog.Error(err) return err } - if err = c.Watch(&source.Kind{Type: &gamekruiseiov1alpha1.GameServer{}}, &handler.EnqueueRequestForOwner{ - OwnerType: &corev1.Pod{}, - IsController: true, - }); err != nil { + if err = c.Watch(&source.Kind{Type: &gamekruiseiov1alpha1.GameServer{}}, &handler.EnqueueRequestForObject{}); err != nil { klog.Error(err) return err } @@ -151,28 +148,47 @@ func (r *GameServerReconciler) Reconcile(ctx context.Context, req ctrl.Request) // get pod pod := &corev1.Pod{} err := r.Get(ctx, namespacedName, pod) + podFound := true if err != nil { if errors.IsNotFound(err) { - return reconcile.Result{}, nil + podFound = false + } else { + klog.Errorf("failed to find pod %s in %s, because of %s.", namespacedName.Name, namespacedName.Namespace, err.Error()) + return reconcile.Result{}, err } - klog.Errorf("failed to find pod %s in %s, because of %s.", namespacedName.Name, namespacedName.Namespace, err.Error()) - return reconcile.Result{}, err } // get GameServer gs := &gamekruiseiov1alpha1.GameServer{} err = r.Get(ctx, namespacedName, gs) + gsFound := true if err != nil { if errors.IsNotFound(err) { - err := r.initGameServer(pod) - if err != nil && !errors.IsAlreadyExists(err) && !errors.IsNotFound(err) { - klog.Errorf("failed to create GameServer %s in %s, because of %s.", namespacedName.Name, namespacedName.Namespace, err.Error()) + gsFound = false + } else { + klog.Errorf("failed to find GameServer %s in %s, because of %s.", namespacedName.Name, namespacedName.Namespace, err.Error()) + return reconcile.Result{}, err + } + } + + if podFound && !gsFound { + err := r.initGameServer(pod) + if err != nil && !errors.IsAlreadyExists(err) && !errors.IsNotFound(err) { + klog.Errorf("failed to create GameServer %s in %s, because of %s.", namespacedName.Name, namespacedName.Namespace, err.Error()) + return reconcile.Result{}, err + } + return reconcile.Result{}, nil + } + + if !podFound { + if gsFound && gs.GetLabels()[gamekruiseiov1alpha1.GameServerDeletingKey] == "true" { + err := r.Client.Delete(context.Background(), gs) + if err != nil && !errors.IsNotFound(err) { + klog.Errorf("failed to delete GameServer %s in %s, because of %s.", namespacedName.Name, namespacedName.Namespace, err.Error()) return reconcile.Result{}, err } - return reconcile.Result{}, nil } - klog.Errorf("failed to find GameServer %s in %s, because of %s.", namespacedName.Name, namespacedName.Namespace, err.Error()) - return reconcile.Result{}, err + return reconcile.Result{}, nil } gsm := NewGameServerManager(gs, pod, r.Client) @@ -222,12 +238,16 @@ func (r *GameServerReconciler) initGameServer(pod *corev1.Pod) error { gs.Namespace = pod.GetNamespace() // set owner reference + gss, err := r.getGameServerSet(pod) + if err != nil { + return err + } ors := make([]metav1.OwnerReference, 0) or := metav1.OwnerReference{ - APIVersion: pod.APIVersion, - Kind: pod.Kind, - Name: pod.GetName(), - UID: pod.GetUID(), + APIVersion: gss.APIVersion, + Kind: gss.Kind, + Name: gss.GetName(), + UID: gss.GetUID(), Controller: pointer.BoolPtr(true), BlockOwnerDeletion: pointer.BoolPtr(true), } diff --git a/pkg/controllers/gameserverset/gameserverset_manager.go b/pkg/controllers/gameserverset/gameserverset_manager.go index 45e14c9..6cbc219 100644 --- a/pkg/controllers/gameserverset/gameserverset_manager.go +++ b/pkg/controllers/gameserverset/gameserverset_manager.go @@ -31,6 +31,9 @@ import ( "k8s.io/utils/pointer" "sigs.k8s.io/controller-runtime/pkg/client" "sort" + "strconv" + "sync" + "time" ) type Control interface { @@ -88,7 +91,7 @@ func (manager *GameServerSetManager) GameServerScale() error { klog.Infof("GameServers %s/%s already has %d replicas, expect to have %d replicas.", gss.GetNamespace(), gss.GetName(), currentReplicas, expectedReplicas) - newNotExistIds := computeToScaleGs(gssReserveIds, reserveIds, notExistIds, expectedReplicas, gsList) + newNotExistIds, deleteIds := computeToScaleGs(gssReserveIds, reserveIds, notExistIds, expectedReplicas, gsList) asts.Spec.ReserveOrdinals = append(gssReserveIds, newNotExistIds...) asts.Spec.Replicas = gss.Spec.Replicas @@ -111,10 +114,10 @@ func (manager *GameServerSetManager) GameServerScale() error { return err } - return nil + return manager.deleteGameServers(deleteIds) } -func computeToScaleGs(gssReserveIds, reserveIds, notExistIds []int, expectedReplicas int, pods []corev1.Pod) []int { +func computeToScaleGs(gssReserveIds, reserveIds, notExistIds []int, expectedReplicas int, pods []corev1.Pod) ([]int, []int) { workloadManageIds := util.GetIndexListFromPodList(pods) var toAdd []int @@ -176,7 +179,57 @@ func computeToScaleGs(gssReserveIds, reserveIds, notExistIds []int, expectedRepl } } - return newNotExistIds + deleteIds := util.GetSliceInANotInB(workloadManageIds, newManageIds) + + return newNotExistIds, deleteIds +} + +func (manager *GameServerSetManager) deleteGameServers(deleteIds []int) error { + gss := manager.gameServerSet + ctx, cancel := context.WithTimeout(context.Background(), 1*time.Second) + defer cancel() + + errch := make(chan error, len(deleteIds)) + var wg sync.WaitGroup + for _, id := range deleteIds { + wg.Add(1) + id := id + go func(ctx context.Context) { + defer wg.Done() + defer ctx.Done() + gsName := gss.GetName() + "-" + strconv.Itoa(id) + gs := &gameKruiseV1alpha1.GameServer{} + err := manager.client.Get(ctx, types.NamespacedName{ + Name: gsName, + Namespace: gss.GetNamespace(), + }, gs) + if err != nil { + if !errors.IsNotFound(err) { + errch <- err + } + return + } + newLabels := gs.GetLabels() + if newLabels == nil { + newLabels = make(map[string]string) + } + newLabels[gameKruiseV1alpha1.GameServerDeletingKey] = "true" + gs.SetLabels(newLabels) + err = manager.client.Update(ctx, gs) + if err != nil { + errch <- err + } + }(ctx) + } + wg.Wait() + close(errch) + err := <-errch + if err != nil { + klog.Errorf("failed to delete GameServers %s in %s because of %s.", gss.GetNamespace(), err.Error()) + return err + } + + return nil } func (manager *GameServerSetManager) IsNeedToUpdateWorkload() bool { diff --git a/test/e2e/client/client.go b/test/e2e/client/client.go index 7e863e1..24de22d 100644 --- a/test/e2e/client/client.go +++ b/test/e2e/client/client.go @@ -120,6 +120,10 @@ func (client *Client) PatchGameServer(gsName string, data []byte) (*gameKruiseV1 return client.kruisegameClient.GameV1alpha1().GameServers(Namespace).Patch(context.TODO(), gsName, types.MergePatchType, data, metav1.PatchOptions{}) } +func (client *Client) GetGameServer(gsName string) (*gameKruiseV1alpha1.GameServer, error) { + return client.kruisegameClient.GameV1alpha1().GameServers(Namespace).Get(context.TODO(), gsName, metav1.GetOptions{}) +} + func (client *Client) PatchGameServerSet(data []byte) (*gameKruiseV1alpha1.GameServerSet, error) { return client.kruisegameClient.GameV1alpha1().GameServerSets(Namespace).Patch(context.TODO(), GameServerSet, types.MergePatchType, data, metav1.PatchOptions{}) } @@ -135,3 +139,7 @@ func (client *Client) GetPodList(labelSelector string) (*corev1.PodList, error) func (client *Client) GetPod(podName string) (*corev1.Pod, error) { return client.kubeClint.CoreV1().Pods(Namespace).Get(context.TODO(), podName, metav1.GetOptions{}) } + +func (client *Client) DeletePod(podName string) error { + return client.kubeClint.CoreV1().Pods(Namespace).Delete(context.TODO(), podName, metav1.DeleteOptions{}) +} diff --git a/test/e2e/framework/framework.go b/test/e2e/framework/framework.go index f943732..efd6dd7 100644 --- a/test/e2e/framework/framework.go +++ b/test/e2e/framework/framework.go @@ -14,6 +14,7 @@ import ( "k8s.io/apimachinery/pkg/util/wait" clientset "k8s.io/client-go/kubernetes" restclient "k8s.io/client-go/rest" + "strconv" "strings" "time" ) @@ -253,6 +254,24 @@ func (f *Framework) WaitForGsDeletionPriorityUpdated(gsName string, deletionPrio }) } +func (f *Framework) DeletePodDirectly(index int) error { + gsName := client.GameServerSet + "-" + strconv.Itoa(index) + return f.client.DeletePod(gsName) +} + +func (f *Framework) ExpectGsCorrect(gsName, opsState, dp, up string) error { + gs, err := f.client.GetGameServer(gsName) + if err != nil { + return err + } + + if gs.Status.DeletionPriority.String() != dp || gs.Status.UpdatePriority.String() != up || string(gs.Spec.OpsState) != opsState { + return fmt.Errorf("current GameServer is wrong") + } + + return nil +} + func (f *Framework) WaitForGsUpdatePriorityUpdated(gsName string, updatePriority string) error { return wait.PollImmediate(5*time.Second, 1*time.Minute, func() (done bool, err error) { diff --git a/test/e2e/testcase/testcase.go b/test/e2e/testcase/testcase.go index c40fe05..05a301b 100644 --- a/test/e2e/testcase/testcase.go +++ b/test/e2e/testcase/testcase.go @@ -91,5 +91,27 @@ func RunTestCases(f *framework.Framework) { err = f.WaitForGsUpdatePriorityUpdated(gss.GetName()+"-2", "20") gomega.Expect(err).To(gomega.BeNil()) }) + + ginkgo.It("GameServer lifecycle", func() { + + // deploy + gss, err := f.DeployGameServerSet() + gomega.Expect(err).To(gomega.BeNil()) + + err = f.ExpectGssCorrect(gss, []int{0, 1, 2}) + gomega.Expect(err).To(gomega.BeNil()) + + _, err = f.ChangeGameServerDeletionPriority(gss.GetName()+"-1", "100") + gomega.Expect(err).To(gomega.BeNil()) + + err = f.WaitForGsDeletionPriorityUpdated(gss.GetName()+"-1", "100") + gomega.Expect(err).To(gomega.BeNil()) + + err = f.DeletePodDirectly(1) + gomega.Expect(err).To(gomega.BeNil()) + + err = f.ExpectGsCorrect(gss.GetName()+"-1", "None", "100", "0") + gomega.Expect(err).To(gomega.BeNil()) + }) }) }