decoupling update and finalizer/label add/remove

pkg/controllers/operationjob/replace/replace.go

@@ -27,6 +27,7 @@ import (
 	appsv1alpha1 "kusionstack.io/kube-api/apps/v1alpha1"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/controller"
+	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
 	"sigs.k8s.io/controller-runtime/pkg/source"
 
 	. "kusionstack.io/kuperator/pkg/controllers/operationjob/opscore"
@@ -74,8 +75,9 @@ func (p *PodReplaceHandler) OperateTargets(ctx context.Context, candidates []*Op
 		if !replaceTriggered {
 			// add replace indicate label on origin pod to trigger replace
 			candidate.Pod.Labels[appsv1alpha1.PodReplaceIndicationLabelKey] = "true"
-			// add finalizer on origin pod before trigger replace, note that AddFinalizer always do update pod
-			if err := controllerutils.AddFinalizer(ctx, p.client, candidate.Pod, OperationJobReplacePodFinalizer); err != nil {
+			// add finalizer on origin pod before trigger replace
+			controllerutil.AddFinalizer(candidate.Pod, OperationJobReplacePodFinalizer)
+			if err := ojutils.UpdatePodWithRetry(ctx, p.client, candidate.Pod); err != nil {
 				retErr := fmt.Errorf("fail to add %s finalizer or replace indicate label to origin pod %s/%s : %s", OperationJobReplacePodFinalizer, candidate.Pod.Namespace, candidate.Pod.Name, err.Error())
 				ojutils.SetOpsStatusError(candidate, ojutils.ReasonUpdateObjectFailed, retErr.Error())
 				return retErr
@@ -165,11 +167,17 @@ func (p *PodReplaceHandler) ReleaseTargets(ctx context.Context, candidates []*Op
 			return nil
 		}
 
+		// skip if replace is canceled and finalizer is removed
+		if _, exist := candidate.Pod.Labels[appsv1alpha1.PodReplaceIndicationLabelKey]; !exist &&
+			!controllerutils.ContainsFinalizer(candidate.Pod, OperationJobReplacePodFinalizer) {
+			return nil
+		}
+
 		// try to remove replace label from origin pod
 		delete(candidate.Pod.Labels, appsv1alpha1.PodReplaceIndicationLabelKey)
-
-		// try to remove finalizer from origin pod, note that RemoveFinalizer always do update pod
-		if err := controllerutils.RemoveFinalizer(ctx, p.client, candidate.Pod, OperationJobReplacePodFinalizer); err != nil {
+		// try to remove finalizer from origin pod
+		controllerutil.RemoveFinalizer(candidate.Pod, OperationJobReplacePodFinalizer)
+		if err := ojutils.UpdatePodWithRetry(ctx, p.client, candidate.Pod); err != nil {
 			retErr := fmt.Errorf("fail to remove %s finalizer or replace indicate label from origin pod %s/%s : %s", OperationJobReplacePodFinalizer, candidate.Pod.Namespace, candidate.Pod.Name, err.Error())
 			ojutils.SetOpsStatusError(candidate, ojutils.ReasonUpdateObjectFailed, retErr.Error())
 			return retErr

pkg/controllers/operationjob/utils/common.go

@@ -17,9 +17,12 @@ limitations under the License.
 package utils
 
 import (
+	"context"
+
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/apimachinery/pkg/util/sets"
+	"k8s.io/client-go/util/retry"
 	"k8s.io/client-go/util/workqueue"
 	appsv1alpha1 "kusionstack.io/kube-api/apps/v1alpha1"
 	"sigs.k8s.io/controller-runtime/pkg/client"
@@ -84,3 +87,16 @@ func SetOpsStatusError(candidate *opscore.OpsCandidate, reason string, message s
 		Message: message,
 	}
 }
+
+func UpdatePodWithRetry(ctx context.Context, c client.Client, obj client.Object) error {
+	return retry.RetryOnConflict(retry.DefaultRetry, func() error {
+		var updateErr error
+		if updateErr = c.Update(ctx, obj); updateErr == nil {
+			return nil
+		}
+		if err := c.Get(ctx, types.NamespacedName{Namespace: obj.GetNamespace(), Name: obj.GetName()}, obj); err != nil {
+			return err
+		}
+		return updateErr
+	})
+}