Integrate UpdateStatus function

Signed-off-by: whitewindmills <[email protected]>

pkg/controllers/cronfederatedhpa/cronfederatedhpa_job.go

@@ -17,6 +17,7 @@ package cronfederatedhpa
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"time"
 
@@ -32,6 +33,7 @@ import (
 	"k8s.io/client-go/util/retry"
 	"k8s.io/klog/v2"
 	"sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
 
 	autoscalingv1alpha1 "github.com/karmada-io/karmada/pkg/apis/autoscaling/v1alpha1"
 	"github.com/karmada-io/karmada/pkg/metrics"
@@ -252,26 +254,30 @@ func (c *ScalingJob) addFailedExecutionHistory(
 		return exists
 	}
 
-	return retry.RetryOnConflict(retry.DefaultRetry, func() (err error) {
-		// If this history not exist, it means the rule is suspended or deleted, so just ignore it.
-		if exists := addFailedHistoryFunc(); !exists {
+	var operationResult controllerutil.OperationResult
+	if err := retry.RetryOnConflict(retry.DefaultRetry, func() (err error) {
+		var exists bool
+		operationResult, err = helper.UpdateStatus(context.Background(), c.client, cronFHPA, func() error {
+			// If this history not exist, it means the rule deleted, so just ignore it.
+			if exists = addFailedHistoryFunc(); !exists {
+				return errors.New("no such rule")
+			}
 			return nil
+		})
+		if exists && err != nil {
+			return err
 		}
+		return nil
+	}); err != nil {
+		klog.Errorf("Failed to add failed history record to CronFederatedHPA(%s/%s): %v", cronFHPA.Namespace, cronFHPA.Name, err)
+		return err
+	}
 
-		updateErr := c.client.Status().Update(context.Background(), cronFHPA)
-		if updateErr == nil {
-			klog.V(4).Infof("CronFederatedHPA(%s/%s) status has been updated successfully", cronFHPA.Namespace, cronFHPA.Name)
-			return nil
-		}
+	if operationResult == controllerutil.OperationResultUpdatedStatusOnly {
+		klog.V(4).Infof("CronFederatedHPA(%s/%s) status has been updated successfully", cronFHPA.Namespace, cronFHPA.Name)
+	}
 
-		updated := &autoscalingv1alpha1.CronFederatedHPA{}
-		if err = c.client.Get(context.Background(), client.ObjectKey{Namespace: cronFHPA.Namespace, Name: cronFHPA.Name}, updated); err == nil {
-			cronFHPA = updated
-		} else {
-			klog.Errorf("Get CronFederatedHPA(%s/%s) failed: %v", cronFHPA.Namespace, cronFHPA.Name, err)
-		}
-		return updateErr
-	})
+	return nil
 }
 
 func (c *ScalingJob) addSuccessExecutionHistory(
@@ -307,24 +313,28 @@ func (c *ScalingJob) addSuccessExecutionHistory(
 		return exists
 	}
 
-	return retry.RetryOnConflict(retry.DefaultRetry, func() (err error) {
-		// If this history not exist, it means the rule deleted, so just ignore it.
-		if exists := addSuccessHistoryFunc(); !exists {
+	var operationResult controllerutil.OperationResult
+	if err := retry.RetryOnConflict(retry.DefaultRetry, func() (err error) {
+		var exists bool
+		operationResult, err = helper.UpdateStatus(context.Background(), c.client, cronFHPA, func() error {
+			// If this history not exist, it means the rule deleted, so just ignore it.
+			if exists = addSuccessHistoryFunc(); !exists {
+				return errors.New("no such rule")
+			}
 			return nil
-		}
-
-		updateErr := c.client.Status().Update(context.Background(), cronFHPA)
-		if updateErr == nil {
-			klog.V(4).Infof("CronFederatedHPA(%s/%s) status has been updated successfully", cronFHPA.Namespace, cronFHPA.Name)
+		})
+		if exists && err != nil {
 			return err
 		}
+		return nil
+	}); err != nil {
+		klog.Errorf("Failed to add success history record to CronFederatedHPA(%s/%s): %v", cronFHPA.Namespace, cronFHPA.Name, err)
+		return err
+	}
 
-		updated := &autoscalingv1alpha1.CronFederatedHPA{}
-		if err = c.client.Get(context.Background(), client.ObjectKey{Namespace: cronFHPA.Namespace, Name: cronFHPA.Name}, updated); err == nil {
-			cronFHPA = updated
-		} else {
-			klog.Errorf("Get CronFederatedHPA(%s/%s) failed: %v", cronFHPA.Namespace, cronFHPA.Name, err)
-		}
-		return updateErr
-	})
+	if operationResult == controllerutil.OperationResultUpdatedStatusOnly {
+		klog.V(4).Infof("CronFederatedHPA(%s/%s) status has been updated successfully", cronFHPA.Namespace, cronFHPA.Name)
+	}
+
+	return nil
 }

pkg/controllers/execution/execution_controller.go

@@ -258,18 +258,11 @@ func (c *Controller) updateAppliedCondition(work *workv1alpha1.Work, status meta
 	}
 
 	return retry.RetryOnConflict(retry.DefaultRetry, func() (err error) {
-		meta.SetStatusCondition(&work.Status.Conditions, newWorkAppliedCondition)
-		updateErr := c.Status().Update(context.TODO(), work)
-		if updateErr == nil {
+		_, err = helper.UpdateStatus(context.Background(), c.Client, work, func() error {
+			meta.SetStatusCondition(&work.Status.Conditions, newWorkAppliedCondition)
 			return nil
-		}
-		updated := &workv1alpha1.Work{}
-		if err = c.Get(context.TODO(), client.ObjectKey{Namespace: work.Namespace, Name: work.Name}, updated); err == nil {
-			work = updated
-		} else {
-			klog.Errorf("Failed to get the updated work(%s/%s), err: %v", work.Namespace, work.Name, err)
-		}
-		return updateErr
+		})
+		return err
 	})
 }
 

pkg/controllers/federatedresourcequota/federated_resource_quota_status_controller.go

@@ -160,20 +160,11 @@ func (c *StatusController) collectQuotaStatus(quota *policyv1alpha1.FederatedRes
 	}
 
 	return retry.RetryOnConflict(retry.DefaultRetry, func() error {
-		quota.Status = *quotaStatus
-		updateErr := c.Status().Update(context.TODO(), quota)
-		if updateErr == nil {
+		_, err = helper.UpdateStatus(context.Background(), c.Client, quota, func() error {
+			quota.Status = *quotaStatus
 			return nil
-		}
-
-		updated := &policyv1alpha1.FederatedResourceQuota{}
-		if err = c.Get(context.TODO(), client.ObjectKey{Namespace: quota.Namespace, Name: quota.Name}, updated); err == nil {
-			quota = updated
-		} else {
-			klog.Errorf("Failed to get updated  federatedResourceQuota(%s): %v", klog.KObj(quota).String(), err)
-		}
-
-		return updateErr
+		})
+		return err
 	})
 }
 

pkg/controllers/mcs/service_import_controller.go

@@ -32,6 +32,7 @@ import (
 
 	"github.com/karmada-io/karmada/pkg/events"
 	"github.com/karmada-io/karmada/pkg/util"
+	"github.com/karmada-io/karmada/pkg/util/helper"
 	"github.com/karmada-io/karmada/pkg/util/names"
 )
 
@@ -153,24 +154,15 @@ func (c *ServiceImportController) updateServiceStatus(svcImport *mcsv1alpha1.Ser
 	}
 
 	err := retry.RetryOnConflict(retry.DefaultRetry, func() (err error) {
-		derivedService.Status = corev1.ServiceStatus{
-			LoadBalancer: corev1.LoadBalancerStatus{
-				Ingress: ingress,
-			},
-		}
-		updateErr := c.Status().Update(context.TODO(), derivedService)
-		if updateErr == nil {
+		_, err = helper.UpdateStatus(context.Background(), c.Client, derivedService, func() error {
+			derivedService.Status = corev1.ServiceStatus{
+				LoadBalancer: corev1.LoadBalancerStatus{
+					Ingress: ingress,
+				},
+			}
 			return nil
-		}
-
-		updated := &corev1.Service{}
-		if err = c.Get(context.TODO(), client.ObjectKey{Namespace: derivedService.Namespace, Name: derivedService.Name}, updated); err == nil {
-			derivedService = updated
-		} else {
-			klog.Errorf("Failed to get updated service %s/%s: %v", derivedService.Namespace, derivedService.Name, err)
-		}
-
-		return updateErr
+		})
+		return err
 	})
 
 	if err != nil {

pkg/controllers/multiclusterservice/endpointslice_dispatch_controller.go

@@ -127,18 +127,11 @@ func (c *EndpointsliceDispatchController) updateEndpointSliceDispatched(mcs *net
 	}
 
 	return retry.RetryOnConflict(retry.DefaultRetry, func() (err error) {
-		meta.SetStatusCondition(&mcs.Status.Conditions, EndpointSliceCollected)
-		updateErr := c.Status().Update(context.TODO(), mcs)
-		if updateErr == nil {
+		_, err = helper.UpdateStatus(context.Background(), c.Client, mcs, func() error {
+			meta.SetStatusCondition(&mcs.Status.Conditions, EndpointSliceCollected)
 			return nil
-		}
-		updated := &networkingv1alpha1.MultiClusterService{}
-		if err = c.Get(context.TODO(), client.ObjectKey{Namespace: mcs.Namespace, Name: mcs.Name}, updated); err == nil {
-			mcs = updated
-		} else {
-			klog.Errorf("Failed to get updated MultiClusterService %s/%s: %v", mcs.Namespace, mcs.Name, err)
-		}
-		return updateErr
+		})
+		return err
 	})
 }
 

pkg/controllers/multiclusterservice/mcs_controller.go

@@ -525,18 +525,11 @@ func (c *MCSController) updateMultiClusterServiceStatus(mcs *networkingv1alpha1.
 	}
 
 	return retry.RetryOnConflict(retry.DefaultRetry, func() (err error) {
-		meta.SetStatusCondition(&mcs.Status.Conditions, serviceAppliedCondition)
-		updateErr := c.Status().Update(context.TODO(), mcs)
-		if updateErr == nil {
+		_, err = helper.UpdateStatus(context.Background(), c.Client, mcs, func() error {
+			meta.SetStatusCondition(&mcs.Status.Conditions, serviceAppliedCondition)
 			return nil
-		}
-		updated := &networkingv1alpha1.MultiClusterService{}
-		if err = c.Get(context.TODO(), client.ObjectKey{Namespace: mcs.Namespace, Name: mcs.Name}, updated); err == nil {
-			mcs = updated
-		} else {
-			klog.Errorf("Failed to get updated MultiClusterService %s/%s: %v", mcs.Namespace, mcs.Name, err)
-		}
-		return updateErr
+		})
+		return err
 	})
 }
 

pkg/controllers/remediation/remedy_controller.go

@@ -18,10 +18,8 @@ package remediation
 
 import (
 	"context"
-	"reflect"
 
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
-	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/client-go/util/retry"
 	"k8s.io/klog/v2"
 	controllerruntime "sigs.k8s.io/controller-runtime"
@@ -33,6 +31,7 @@ import (
 	clusterv1alpha1 "github.com/karmada-io/karmada/pkg/apis/cluster/v1alpha1"
 	remedyv1alpha1 "github.com/karmada-io/karmada/pkg/apis/remedy/v1alpha1"
 	"github.com/karmada-io/karmada/pkg/sharedcli/ratelimiterflag"
+	"github.com/karmada-io/karmada/pkg/util/helper"
 )
 
 // ControllerName is the controller name that will be used when reporting events.
@@ -71,23 +70,12 @@ func (c *RemedyController) Reconcile(ctx context.Context, req controllerruntime.
 
 	actions := calculateActions(clusterRelatedRemedies, cluster)
 	err = retry.RetryOnConflict(retry.DefaultRetry, func() error {
-		if reflect.DeepEqual(actions, cluster.Status.RemedyActions) {
+		_, err = helper.UpdateStatus(context.Background(), c.Client, cluster, func() error {
+			cluster.Status.RemedyActions = actions
 			return nil
-		}
-		cluster.Status.RemedyActions = actions
-		updateErr := c.Client.Status().Update(ctx, cluster)
-		if updateErr == nil {
-			return nil
-		}
+		})
+		return err
 
-		updatedCluster := &clusterv1alpha1.Cluster{}
-		err = c.Client.Get(ctx, types.NamespacedName{Name: cluster.Name}, updatedCluster)
-		if err == nil {
-			cluster = updatedCluster
-		} else {
-			klog.Errorf("Failed to get updated cluster(%s): %v", cluster.Name, err)
-		}
-		return updateErr
 	})
 	if err != nil {
 		klog.Errorf("Failed to sync cluster(%s) remedy actions: %v", cluster.Name, err)

pkg/controllers/status/cluster_status_controller.go

@@ -277,19 +277,11 @@ func (c *ClusterStatusController) updateStatusIfNeeded(cluster *clusterv1alpha1.
 	if !equality.Semantic.DeepEqual(cluster.Status, currentClusterStatus) {
 		klog.V(4).Infof("Start to update cluster status: %s", cluster.Name)
 		err := retry.RetryOnConflict(retry.DefaultRetry, func() (err error) {
-			cluster.Status = currentClusterStatus
-			updateErr := c.Status().Update(context.TODO(), cluster)
-			if updateErr == nil {
+			_, err = helper.UpdateStatus(context.Background(), c.Client, cluster, func() error {
+				cluster.Status = currentClusterStatus
 				return nil
-			}
-
-			updated := &clusterv1alpha1.Cluster{}
-			if err = c.Get(context.TODO(), client.ObjectKey{Namespace: cluster.Namespace, Name: cluster.Name}, updated); err == nil {
-				cluster = updated
-			} else {
-				klog.Errorf("Failed to get updated cluster %s: %v", cluster.Name, err)
-			}
-			return updateErr
+			})
+			return err
 		})
 		if err != nil {
 			klog.Errorf("Failed to update health status of the member cluster: %v, err is : %v", cluster.Name, err)

pkg/controllers/status/work_status_controller.go

@@ -326,22 +326,11 @@ func (c *WorkStatusController) updateAppliedCondition(work *workv1alpha1.Work, s
 	}
 
 	err := retry.RetryOnConflict(retry.DefaultRetry, func() (err error) {
-		workStatus := work.Status.DeepCopy()
-		meta.SetStatusCondition(&work.Status.Conditions, newWorkAppliedCondition)
-		if reflect.DeepEqual(*workStatus, work.Status) {
+		_, err = helper.UpdateStatus(context.Background(), c.Client, work, func() error {
+			meta.SetStatusCondition(&work.Status.Conditions, newWorkAppliedCondition)
 			return nil
-		}
-		updateErr := c.Status().Update(context.TODO(), work)
-		if updateErr == nil {
-			return nil
-		}
-		updated := &workv1alpha1.Work{}
-		if err = c.Get(context.TODO(), client.ObjectKey{Namespace: work.Namespace, Name: work.Name}, updated); err == nil {
-			work = updated
-		} else {
-			klog.Errorf("Failed to get updated work %s/%s: %s", work.Namespace, work.Name, err.Error())
-		}
-		return updateErr
+		})
+		return err
 	})
 
 	if err != nil {
@@ -386,25 +375,12 @@ func (c *WorkStatusController) reflectStatus(work *workv1alpha1.Work, clusterObj
 		Health:     resourceHealth,
 	}
 
-	workCopy := work.DeepCopy()
 	return retry.RetryOnConflict(retry.DefaultRetry, func() (err error) {
-		manifestStatuses := c.mergeStatus(workCopy.Status.ManifestStatuses, manifestStatus)
-		if reflect.DeepEqual(workCopy.Status.ManifestStatuses, manifestStatuses) {
+		_, err = helper.UpdateStatus(context.Background(), c.Client, work, func() error {
+			work.Status.ManifestStatuses = c.mergeStatus(work.Status.ManifestStatuses, manifestStatus)
 			return nil
-		}
-		workCopy.Status.ManifestStatuses = manifestStatuses
-		updateErr := c.Status().Update(context.TODO(), workCopy)
-		if updateErr == nil {
-			return nil
-		}
-
-		updated := &workv1alpha1.Work{}
-		if err = c.Get(context.TODO(), client.ObjectKey{Namespace: workCopy.Namespace, Name: workCopy.Name}, updated); err == nil {
-			workCopy = updated
-		} else {
-			klog.Errorf("Failed to get updated work %s/%s: %v", workCopy.Namespace, workCopy.Name, err)
-		}
-		return updateErr
+		})
+		return err
 	})
 }