optimize ut in podopslifecycle controller

Author: shaofan-hs

go.mod

@@ -3,7 +3,6 @@ module kusionstack.io/operating
 go 1.19
 
 require (
-	github.com/davecgh/go-spew v1.1.1
 	github.com/docker/distribution v2.8.2+incompatible
 	github.com/go-logr/logr v1.2.4
 	github.com/google/uuid v1.3.0
@@ -50,6 +49,7 @@ require (
 	github.com/cespare/xxhash/v2 v2.2.0 // indirect
 	github.com/clbanning/mxj/v2 v2.5.5 // indirect
 	github.com/cyphar/filepath-securejoin v0.2.2 // indirect
+	github.com/davecgh/go-spew v1.1.1 // indirect
 	github.com/evanphx/json-patch v4.11.0+incompatible // indirect
 	github.com/form3tech-oss/jwt-go v3.2.3+incompatible // indirect
 	github.com/fsnotify/fsnotify v1.6.0 // indirect

pkg/controllers/podopslifecycle/podopslifecycle_controller.go

@@ -125,6 +125,8 @@ func (r *ReconcilePodOpsLifecycle) Reconcile(ctx context.Context, request reconc
 	if err != nil {
 		return reconcile.Result{}, err
 	}
+
+	// If all lifecycle is finished, or the is no lifecycle begined
 	if len(idToLabelsMap) == 0 {
 		updated, err := r.addServiceAvailable(pod)
 		if updated {
@@ -139,6 +141,7 @@ func (r *ReconcilePodOpsLifecycle) Reconcile(ctx context.Context, request reconc
 		}
 	}
 
+	// Get the state of pod managed by TransitionRule
 	state, err := r.podTransitionRuleManager.GetState(ctx, r.Client, pod)
 	if err != nil {
 		logger.Error(err, "failed to get pod state")
@@ -164,8 +167,8 @@ func (r *ReconcilePodOpsLifecycle) Reconcile(ctx context.Context, request reconc
 	}
 
 	expected := map[string]bool{
-		v1alpha1.PodPreparingLabelPrefix:  false, // set readiness gate to false, traffic off
-		v1alpha1.PodCompletingLabelPrefix: true,  // set readiness gate to true, traffic on
+		v1alpha1.PodPreparingLabelPrefix:  false, // Set readiness gate to false
+		v1alpha1.PodCompletingLabelPrefix: true,  // Set readiness gate to true
 	}
 	for _, labels := range idToLabelsMap {
 		for k, v := range expected {
@@ -175,7 +178,7 @@ func (r *ReconcilePodOpsLifecycle) Reconcile(ctx context.Context, request reconc
 
 			updated, err := r.updateServiceReadiness(ctx, pod, v)
 			if err != nil {
-				return reconcile.Result{}, err // only need set once
+				return reconcile.Result{}, err // Only need set once
 			}
 			if updated {
 				r.Recorder.Eventf(pod, corev1.EventTypeNormal, "ReadinessGate", "Set service ready readiness gate to %v", v)
@@ -186,6 +189,7 @@ func (r *ReconcilePodOpsLifecycle) Reconcile(ctx context.Context, request reconc
 	return reconcile.Result{}, nil
 }
 
+// addServiceAvailable try to add service available label to pod
 func (r *ReconcilePodOpsLifecycle) addServiceAvailable(pod *corev1.Pod) (bool, error) {
 	if pod.Labels == nil {
 		return false, nil
@@ -194,7 +198,8 @@ func (r *ReconcilePodOpsLifecycle) addServiceAvailable(pod *corev1.Pod) (bool, e
 		return false, nil
 	}
 
-	satisfied, notSatisfiedFinalizers, err := controllerutils.IsExpectedFinalizerSatisfied(pod) // whether all expected finalizers are satisfied
+	// Whether all expected finalizers are satisfied
+	satisfied, notSatisfiedFinalizers, err := controllerutils.IsExpectedFinalizerSatisfied(pod)
 	if err != nil {
 		return false, err
 	}
@@ -207,7 +212,7 @@ func (r *ReconcilePodOpsLifecycle) addServiceAvailable(pod *corev1.Pod) (bool, e
 		if !allDirty {
 			return false, nil
 		}
-		// all not satisfied expected finalizers are dirty, so actually the pod satisfied expected finalizer now
+		// All not satisfied finalizers are dirty, so actually the pod satisfied expected finalizers now
 	}
 
 	if !controllerutils.IsPodReady(pod) {
@@ -221,7 +226,7 @@ func (r *ReconcilePodOpsLifecycle) addServiceAvailable(pod *corev1.Pod) (bool, e
 }
 
 func (r *ReconcilePodOpsLifecycle) removeDirtyExpectedFinalizer(pod *corev1.Pod, notSatisfiedFinalizers map[string]string) (bool, error) {
-	var allDirty bool
+	var allDirty bool // Whether all not atisfied finalizers are dirty
 	dirtyExpectedFinalizer := make(map[string]string)
 
 	for expectedFlzKey, finalizer := range notSatisfiedFinalizers {
@@ -348,7 +353,7 @@ func (r *ReconcilePodOpsLifecycle) setServiceReadiness(pod *corev1.Pod, isReady
 	if !isReady {
 		status = corev1.ConditionFalse
 	}
-	if index == -1 { // append readiness gate
+	if index == -1 { // Append readiness gate
 		pod.Status.Conditions = append(pod.Status.Conditions, corev1.PodCondition{
 			Type:               v1alpha1.ReadinessGatePodServiceReady,
 			Status:             status,
@@ -362,7 +367,7 @@ func (r *ReconcilePodOpsLifecycle) setServiceReadiness(pod *corev1.Pod, isReady
 		return false, ""
 	}
 
-	// update readiness gate
+	// Update readiness gate
 	pod.Status.Conditions[index].Status = status
 	pod.Status.Conditions[index].LastTransitionTime = metav1.Now()
 	pod.Status.Conditions[index].Message = "updated by PodOpsLifecycle"

pkg/controllers/podopslifecycle/podopslifecycle_controller_test.go

@@ -27,8 +27,10 @@ import (
 	. "github.com/onsi/gomega"
 	appsv1 "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
+	"k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/types"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/envtest"
 	logf "sigs.k8s.io/controller-runtime/pkg/log"
@@ -88,15 +90,15 @@ var _ = BeforeSuite(func() {
 })
 
 var _ = AfterSuite(func() {
-	By("tearing down the test environment")
+	By("Tearing down the test environment")
 
 	cancel()
 
 	err := env.Stop()
 	Expect(err).NotTo(HaveOccurred())
 })
 
-var _ = Describe("podopslifecycle controller", func() {
+var _ = Describe("PodOpsLifecycle controller", func() {
 	var (
 		podSpec = corev1.PodSpec{
 			Containers: []corev1.Container{
@@ -111,15 +113,17 @@ var _ = Describe("podopslifecycle controller", func() {
 				},
 			},
 		}
+		name      = "test"
+		namespace = "default"
 		id        = "123"
 		timestamp = "1402144848"
 	)
 
 	AfterEach(func() {
 		pod := &corev1.Pod{
 			ObjectMeta: metav1.ObjectMeta{
-				Name:      "test",
-				Namespace: "default",
+				Name:      name,
+				Namespace: namespace,
 			},
 		}
 		err := mgr.GetClient().Delete(context.Background(), pod)
@@ -133,11 +137,11 @@ var _ = Describe("podopslifecycle controller", func() {
 		}
 	})
 
-	It("update pod with stage pre-check", func() {
+	It("Create a pod controlled by kusionstack", func() {
 		pod := &corev1.Pod{
 			ObjectMeta: metav1.ObjectMeta{
-				Name:      "test",
-				Namespace: "default",
+				Name:      name,
+				Namespace: namespace,
 				Labels:    map[string]string{v1alpha1.ControlledByKusionStackLabelKey: "true"},
 			},
 			Spec: podSpec,
@@ -147,22 +151,33 @@ var _ = Describe("podopslifecycle controller", func() {
 
 		<-request
 
+		// Not found the pod
 		pod = &corev1.Pod{}
 		err = mgr.GetAPIReader().Get(context.Background(), client.ObjectKey{
-			Name:      "test",
-			Namespace: "default",
+			Name:      name + "1",
+			Namespace: namespace,
+		}, pod)
+		Expect(errors.IsNotFound(err)).To(Equal(true))
+
+		_, err = podOpsLifecycle.Reconcile(context.Background(), reconcile.Request{types.NamespacedName{Name: name, Namespace: namespace}})
+		Expect(err).NotTo(HaveOccurred())
+
+		pod = &corev1.Pod{}
+		err = mgr.GetAPIReader().Get(context.Background(), client.ObjectKey{
+			Name:      name,
+			Namespace: namespace,
 		}, pod)
 		Expect(err).NotTo(HaveOccurred())
 		Expect(pod.Status.Conditions).To(HaveLen(1))
 		Expect(string(pod.Status.Conditions[0].Type)).To(Equal(v1alpha1.ReadinessGatePodServiceReady))
 		Expect(pod.Status.Conditions[0].Status).To(Equal(corev1.ConditionTrue))
 	})
 
-	It("create pod with label prepare", func() {
+	It("Create pod with label preparing", func() {
 		pod := &corev1.Pod{
 			ObjectMeta: metav1.ObjectMeta{
-				Name:      "test",
-				Namespace: "default",
+				Name:      name,
+				Namespace: namespace,
 				Labels: map[string]string{
 					v1alpha1.ControlledByKusionStackLabelKey:                   "true",
 					fmt.Sprintf("%s/%s", v1alpha1.PodOperatingLabelPrefix, id): timestamp,
@@ -177,8 +192,8 @@ var _ = Describe("podopslifecycle controller", func() {
 		pod = &corev1.Pod{}
 		Eventually(func() error {
 			if err := mgr.GetAPIReader().Get(context.Background(), client.ObjectKey{
-				Name:      "test",
-				Namespace: "default",
+				Name:      name,
+				Namespace: namespace,
 			}, pod); err != nil {
 				return fmt.Errorf("fail to get pod: %s", err)
 			}
@@ -196,14 +211,14 @@ var _ = Describe("podopslifecycle controller", func() {
 			}
 
 			return nil
-		}, 5*time.Second, 1*time.Second).Should(BeNil())
+		}, 3*time.Second, 200*time.Millisecond).Should(BeNil())
 	})
 
-	It("create pod with label complete", func() {
+	It("Create pod with label completing", func() {
 		pod := &corev1.Pod{
 			ObjectMeta: metav1.ObjectMeta{
-				Name:      "test",
-				Namespace: "default",
+				Name:      name,
+				Namespace: namespace,
 				Labels: map[string]string{
 					v1alpha1.ControlledByKusionStackLabelKey:                    "true",
 					fmt.Sprintf("%s/%s", v1alpha1.PodOperateLabelPrefix, id):    timestamp,
@@ -220,8 +235,8 @@ var _ = Describe("podopslifecycle controller", func() {
 		pod = &corev1.Pod{}
 		Eventually(func() error {
 			if err := mgr.GetAPIReader().Get(context.Background(), client.ObjectKey{
-				Name:      "test",
-				Namespace: "default",
+				Name:      name,
+				Namespace: namespace,
 			}, pod); err != nil {
 				return fmt.Errorf("fail to get pod: %s", err)
 			}
@@ -239,14 +254,14 @@ var _ = Describe("podopslifecycle controller", func() {
 			}
 
 			return nil
-		}, 5*time.Second, 1*time.Second).Should(BeNil())
+		}, 3*time.Second, 200*time.Millisecond).Should(BeNil())
 	})
 
-	It("update pod with label complete", func() {
+	It("Update pod with label compling", func() {
 		pod := &corev1.Pod{
 			ObjectMeta: metav1.ObjectMeta{
-				Name:      "test",
-				Namespace: "default",
+				Name:      name,
+				Namespace: namespace,
 				Labels:    map[string]string{v1alpha1.ControlledByKusionStackLabelKey: "true"},
 			},
 			Spec: podSpec,
@@ -256,28 +271,31 @@ var _ = Describe("podopslifecycle controller", func() {
 
 		<-request
 
-		pod = &corev1.Pod{}
-		err = mgr.GetAPIReader().Get(context.Background(), client.ObjectKey{
-			Name:      "test",
-			Namespace: "default",
-		}, pod)
-		Expect(err).NotTo(HaveOccurred())
+		Eventually(func() error {
+			pod = &corev1.Pod{}
+			err = mgr.GetAPIReader().Get(context.Background(), client.ObjectKey{
+				Name:      name,
+				Namespace: namespace,
+			}, pod)
+			if err != nil {
+				return fmt.Errorf("fail to get pod: %v", err)
+			}
 
-		pod.ObjectMeta.Labels = map[string]string{
-			v1alpha1.ControlledByKusionStackLabelKey:                    "true",
-			fmt.Sprintf("%s/%s", v1alpha1.PodOperateLabelPrefix, id):    timestamp,
-			fmt.Sprintf("%s/%s", v1alpha1.PodCompletingLabelPrefix, id): timestamp,
-		}
-		err = mgr.GetClient().Update(context.Background(), pod)
-		Expect(err).NotTo(HaveOccurred())
+			pod.ObjectMeta.Labels = map[string]string{
+				v1alpha1.ControlledByKusionStackLabelKey:                    "true",
+				fmt.Sprintf("%s/%s", v1alpha1.PodOperateLabelPrefix, id):    timestamp,
+				fmt.Sprintf("%s/%s", v1alpha1.PodCompletingLabelPrefix, id): timestamp,
+			}
+			return mgr.GetClient().Update(context.Background(), pod)
+		}, 3*time.Second, 200*time.Millisecond).Should(BeNil())
 
 		pod = &corev1.Pod{}
 		Eventually(func() error {
 			if err := mgr.GetAPIReader().Get(context.Background(), client.ObjectKey{
-				Name:      "test",
-				Namespace: "default",
+				Name:      name,
+				Namespace: namespace,
 			}, pod); err != nil {
-				return fmt.Errorf("fail to get pod: %s", err)
+				return fmt.Errorf("fail to get pod: %v", err)
 			}
 
 			if len(pod.Status.Conditions) != 1 {
@@ -293,7 +311,7 @@ var _ = Describe("podopslifecycle controller", func() {
 			}
 
 			return nil
-		}, 5*time.Second, 1*time.Second).Should(BeNil())
+		}, 3*time.Second, 200*time.Millisecond).Should(BeNil())
 	})
 })
 

pkg/controllers/podopslifecycle/predicate.go

@@ -24,7 +24,7 @@ import (
 type NeedOpsLifecycle func(oldPod, newPod *corev1.Pod) bool
 
 type PodPredicate struct {
-	NeedOpsLifecycle // check if pod need use lifecycle
+	NeedOpsLifecycle // Check if pod need be reconciled
 }
 
 func (pp *PodPredicate) Create(evt event.CreateEvent) bool {