fix, comments

apis/apps/v1alpha1/types.go

@@ -19,8 +19,8 @@ package v1alpha1
 const (
 	PodOperationProtectionFinalizerPrefix = "prot.lifecycle.kafed.kusionstack.io"
 
-	PodOpsLifecyclePreCheckStage  = "precheck"
-	PodOpsLifecyclePostCheckStage = "postcheck"
+	PodOpsLifecyclePreCheckStage  = "pre-check"
+	PodOpsLifecyclePostCheckStage = "post-check"
 )
 
 // +kubebuilder:object:generate=false

pkg/controllers/podopslifecycle/podopslifecycle_controller.go

@@ -136,10 +136,8 @@ func (r *ReconcilePodOpsLifecycle) Reconcile(ctx context.Context, request reconc
 			if err != nil {
 				r.logger.Errorf("failed to update pod %s: %s", key, err)
 				expectation.DeleteExpectations(key)
-
-				return reconcile.Result{}, err
 			}
-			return reconcile.Result{}, nil
+			return reconcile.Result{}, err
 		}
 	}
 

pkg/webhook/server/generic/pod/opslifecycle/mutating.go

@@ -51,7 +51,12 @@ func (lc *OpsLifecycle) Mutating(ctx context.Context, c client.Client, oldPod, n
 			}
 
 			// clean up these labels with id
-			for _, v := range []string{v1alpha1.PodOperatingLabelPrefix, v1alpha1.PodOperationTypeLabelPrefix, v1alpha1.PodPreCheckLabelPrefix, v1alpha1.PodPreCheckedLabelPrefix, v1alpha1.PodPrepareLabelPrefix, v1alpha1.PodOperateLabelPrefix} {
+			for _, v := range []string{v1alpha1.PodOperatingLabelPrefix,
+				v1alpha1.PodOperationTypeLabelPrefix,
+				v1alpha1.PodPreCheckLabelPrefix,
+				v1alpha1.PodPreCheckedLabelPrefix,
+				v1alpha1.PodPrepareLabelPrefix,
+				v1alpha1.PodOperateLabelPrefix} {
 				delete(newPod.Labels, fmt.Sprintf("%s/%s", v, id))
 			}
 
@@ -67,7 +72,7 @@ func (lc *OpsLifecycle) Mutating(ctx context.Context, c client.Client, oldPod, n
 					lc.addLabelWithTime(newPod, fmt.Sprintf("%s/%s", v1alpha1.PodPrepareLabelPrefix, id))
 				}
 				if _, ok := labels[v1alpha1.PodOperateLabelPrefix]; !ok {
-					if ready, _, _ := lc.readyToUpgrade(newPod); ready {
+					if ready, _ := lc.readyToUpgrade(newPod); ready {
 						lc.addLabelWithTime(newPod, fmt.Sprintf("%s/%s", v1alpha1.PodOperateLabelPrefix, id))
 					}
 				}
@@ -116,7 +121,12 @@ func (lc *OpsLifecycle) Mutating(ctx context.Context, c client.Client, oldPod, n
 		}
 		if satisfied { // all operations are done and all expected finalizers are satisfied, then remove all unuseful labels, and add service available label
 			for id := range newIdToLabelsMap {
-				for _, v := range []string{v1alpha1.PodOperateLabelPrefix, v1alpha1.PodOperatedLabelPrefix, v1alpha1.PodDoneOperationTypeLabelPrefix, v1alpha1.PodPostCheckLabelPrefix, v1alpha1.PodPostCheckedLabelPrefix, v1alpha1.PodCompleteLabelPrefix} {
+				for _, v := range []string{v1alpha1.PodOperateLabelPrefix,
+					v1alpha1.PodOperatedLabelPrefix,
+					v1alpha1.PodDoneOperationTypeLabelPrefix,
+					v1alpha1.PodPostCheckLabelPrefix,
+					v1alpha1.PodPostCheckedLabelPrefix,
+					v1alpha1.PodCompleteLabelPrefix} {
 					delete(newPod.Labels, fmt.Sprintf("%s/%s", v, id))
 				}
 			}
@@ -132,7 +142,9 @@ func (lc *OpsLifecycle) Mutating(ctx context.Context, c client.Client, oldPod, n
 		}
 
 		for id, labels := range newIdToLabelsMap {
-			for _, v := range []string{v1alpha1.PodPreCheckLabelPrefix, v1alpha1.PodPreCheckedLabelPrefix, v1alpha1.PodPrepareLabelPrefix} {
+			for _, v := range []string{v1alpha1.PodPreCheckLabelPrefix,
+				v1alpha1.PodPreCheckedLabelPrefix,
+				v1alpha1.PodPrepareLabelPrefix} {
 				delete(newPod.Labels, fmt.Sprintf("%s/%s", v, id))
 			}
 

pkg/webhook/server/generic/pod/opslifecycle/webhook.go

@@ -49,7 +49,7 @@ var (
 	}
 )
 
-type ReadyToUpgrade func(pod *corev1.Pod) (bool, []string, *time.Duration)
+type ReadyToUpgrade func(pod *corev1.Pod) (bool, []string)
 type SatisfyExpectedFinalizers func(pod *corev1.Pod) (bool, []string, error)
 type TimeLabelValue func() string
 
@@ -70,7 +70,7 @@ func New() *OpsLifecycle {
 }
 
 func (lc *OpsLifecycle) Name() string {
-	return "PodLifecycleWebhook"
+	return "PodOpsLifecycleWebhook"
 }
 
 func (lc *OpsLifecycle) addLabelWithTime(pod *corev1.Pod, key string) {
@@ -134,9 +134,9 @@ func podAvailableConditions(pod *corev1.Pod) (*v1alpha1.PodAvailableConditions,
 	return availableConditions, nil
 }
 
-func hasNoBlockingFinalizer(pod *corev1.Pod) (bool, []string, *time.Duration) {
+func hasNoBlockingFinalizer(pod *corev1.Pod) (bool, []string) {
 	if pod == nil {
-		return true, nil, nil
+		return true, nil
 	}
 
 	hasReadinessGate := false
@@ -150,11 +150,11 @@ func hasNoBlockingFinalizer(pod *corev1.Pod) (bool, []string, *time.Duration) {
 	}
 	if !hasReadinessGate {
 		// if has no service-ready ReadinessGate, treat it as normal pod.
-		return true, nil, nil
+		return true, nil
 	}
 
 	if pod.ObjectMeta.Finalizers == nil || len(pod.ObjectMeta.Finalizers) == 0 {
-		return true, nil, nil
+		return true, nil
 	}
 
 	var finalizers []string
@@ -165,9 +165,8 @@ func hasNoBlockingFinalizer(pod *corev1.Pod) (bool, []string, *time.Duration) {
 	}
 
 	if len(finalizers) > 0 {
-		requeneAfter := time.Duration(waitingForLifecycleSeconds) * time.Second
-		return false, finalizers, &requeneAfter
+		return false, finalizers
 	}
 
-	return true, nil, nil
+	return true, nil
 }

pkg/webhook/server/generic/pod/opslifecycle/webhook_test.go

@@ -20,7 +20,6 @@ import (
 	"context"
 	"fmt"
 	"testing"
-	"time"
 
 	"github.com/stretchr/testify/assert"
 	admissionv1 "k8s.io/api/admission/v1"
@@ -489,12 +488,12 @@ func TestMutating(t *testing.T) {
 	}
 }
 
-func readyToUpgradeReturnTrue(pod *corev1.Pod) (bool, []string, *time.Duration) {
-	return true, nil, nil
+func readyToUpgradeReturnTrue(pod *corev1.Pod) (bool, []string) {
+	return true, nil
 }
 
-func readyToUpgradeReturnFalse(pod *corev1.Pod) (bool, []string, *time.Duration) {
-	return false, nil, nil
+func readyToUpgradeReturnFalse(pod *corev1.Pod) (bool, []string) {
+	return false, nil
 }
 
 func satifyExpectedFinalizersReturnTrue(pod *corev1.Pod) (bool, []string, error) {