Refactoring GameServer Lifecycle Management (#10)

* refactor: gs will not be deleted due to pod deletion and opt format

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

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{
+	if err = c.Watch(&source.Kind{Type: &gamekruiseiov1alpha1.GameServer{}}, &handler.EnqueueRequestForObject{}); err != nil {
-		OwnerType:    &corev1.Pod{},
-		IsController: true,
-	}); 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)
+			gsFound = false
-			if err != nil && !errors.IsAlreadyExists(err) && !errors.IsNotFound(err) {
+		} else {
-				klog.Errorf("failed to create GameServer %s in %s, because of %s.", namespacedName.Name, namespacedName.Namespace, err.Error())
+			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{}, nil
-		return reconcile.Result{}, err
 	}
 
 	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,
+		APIVersion:         gss.APIVersion,
-		Kind:               pod.Kind,
+		Kind:               gss.Kind,
-		Name:               pod.GetName(),
+		Name:               gss.GetName(),
-		UID:                pod.GetUID(),
+		UID:                gss.GetUID(),
 		Controller:         pointer.BoolPtr(true),
 		BlockOwnerDeletion: pointer.BoolPtr(true),
 	}

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 {

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{})
+}

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) {

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())
+		})
 	})
 }