Merge pull request #6076 from ctripcloud/work-mutating

add work mutating in ctrlutil.CreateOrUpdateWork()

Author: karmada-bot

pkg/controllers/ctrlutil/work.go

@@ -31,13 +31,15 @@ import (
 
 	workv1alpha1 "github.com/karmada-io/karmada/pkg/apis/work/v1alpha1"
 	workv1alpha2 "github.com/karmada-io/karmada/pkg/apis/work/v1alpha2"
+	"github.com/karmada-io/karmada/pkg/resourceinterpreter/default/native/prune"
 	"github.com/karmada-io/karmada/pkg/util"
+	"github.com/karmada-io/karmada/pkg/util/helper"
 )
 
 // CreateOrUpdateWork creates a Work object if not exist, or updates if it already exists.
-func CreateOrUpdateWork(ctx context.Context, client client.Client, workMeta metav1.ObjectMeta, resource *unstructured.Unstructured, options ...WorkOption) error {
+func CreateOrUpdateWork(ctx context.Context, c client.Client, workMeta metav1.ObjectMeta, resource *unstructured.Unstructured, options ...WorkOption) error {
+	resource = resource.DeepCopy()
 	if workMeta.Labels[util.PropagationInstruction] != util.PropagationInstructionSuppressed {
-		resource = resource.DeepCopy()
 		// set labels
 		util.MergeLabel(resource, util.ManagedByKarmadaLabel, util.ManagedByKarmadaLabelValue)
 		// set annotations
@@ -48,42 +50,52 @@ func CreateOrUpdateWork(ctx context.Context, client client.Client, workMeta meta
 			util.MergeAnnotation(resource, workv1alpha2.ResourceConflictResolutionAnnotation, conflictResolution)
 		}
 	}
-
-	workloadJSON, err := json.Marshal(resource)
+	// Do the same thing as the mutating webhook does, remove the irrelevant fields for the resource.
+	// This is to avoid unnecessary updates to the Work object, especially when controller starts.
+	err := prune.RemoveIrrelevantFields(resource, prune.RemoveJobTTLSeconds)
 	if err != nil {
-		klog.Errorf("Failed to marshal workload(%s/%s), error: %v", resource.GetNamespace(), resource.GetName(), err)
+		klog.Errorf("Failed to prune irrelevant fields for resource %s/%s. Error: %v", resource.GetNamespace(), resource.GetName(), err)
 		return err
 	}
 
 	work := &workv1alpha1.Work{
 		ObjectMeta: workMeta,
-		Spec: workv1alpha1.WorkSpec{
-			Workload: workv1alpha1.WorkloadTemplate{
-				Manifests: []workv1alpha1.Manifest{
-					{
-						RawExtension: runtime.RawExtension{
-							Raw: workloadJSON,
-						},
-					},
-				},
-			},
-		},
 	}
 
 	applyWorkOptions(work, options)
 
 	runtimeObject := work.DeepCopy()
 	var operationResult controllerutil.OperationResult
 	err = retry.RetryOnConflict(retry.DefaultRetry, func() (err error) {
-		operationResult, err = controllerutil.CreateOrUpdate(ctx, client, runtimeObject, func() error {
+		operationResult, err = controllerutil.CreateOrUpdate(ctx, c, runtimeObject, func() error {
 			if !runtimeObject.DeletionTimestamp.IsZero() {
 				return fmt.Errorf("work %s/%s is being deleted", runtimeObject.GetNamespace(), runtimeObject.GetName())
 			}
 
-			runtimeObject.Spec = work.Spec
 			runtimeObject.Labels = util.DedupeAndMergeLabels(runtimeObject.Labels, work.Labels)
 			runtimeObject.Annotations = util.DedupeAndMergeAnnotations(runtimeObject.Annotations, work.Annotations)
 			runtimeObject.Finalizers = work.Finalizers
+			runtimeObject.Spec = work.Spec
+
+			// Do the same thing as the mutating webhook does, add the permanent ID to workload if not exist,
+			// This is to avoid unnecessary updates to the Work object, especially when controller starts.
+			if runtimeObject.Labels[util.PropagationInstruction] != util.PropagationInstructionSuppressed {
+				helper.SetLabelsAndAnnotationsForWorkload(resource, runtimeObject)
+			}
+			workloadJSON, err := json.Marshal(resource)
+			if err != nil {
+				klog.Errorf("Failed to marshal workload(%s/%s), error: %v", resource.GetNamespace(), resource.GetName(), err)
+				return err
+			}
+			runtimeObject.Spec.Workload = workv1alpha1.WorkloadTemplate{
+				Manifests: []workv1alpha1.Manifest{
+					{
+						RawExtension: runtime.RawExtension{
+							Raw: workloadJSON,
+						},
+					},
+				},
+			}
 			return nil
 		})
 		return err

pkg/controllers/multiclusterservice/endpointslice_dispatch_controller_test.go

@@ -604,7 +604,9 @@ func TestEnsureEndpointSliceWork(t *testing.T) {
                                                 "endpointslice.karmada.io/provision-cluster": "provider",
                                                 "work.karmada.io/name": "test-work",
                                                 "work.karmada.io/namespace": "karmada-es-consumer",
-                                                "resourcetemplate.karmada.io/uid": ""
+                                                "resourcetemplate.karmada.io/uid": "",
+                                                "resourcetemplate.karmada.io/managed-annotations": "endpointslice.karmada.io/provision-cluster,resourcetemplate.karmada.io/managed-annotations,resourcetemplate.karmada.io/managed-labels,resourcetemplate.karmada.io/uid,work.karmada.io/name,work.karmada.io/namespace",
+                                                "resourcetemplate.karmada.io/managed-labels":"endpointslice.kubernetes.io/managed-by,karmada.io/managed,kubernetes.io/service-name"
                                             }
                                         },
                                         "endpoints": [

pkg/util/helper/work.go

@@ -111,3 +111,14 @@ func IsWorkContains(manifests []workv1alpha1.Manifest, targetResource schema.Gro
 func IsWorkSuspendDispatching(work *workv1alpha1.Work) bool {
 	return ptr.Deref(work.Spec.SuspendDispatching, false)
 }
+
+// SetLabelsAndAnnotationsForWorkload sets the associated work object labels and annotations for workload.
+func SetLabelsAndAnnotationsForWorkload(workload *unstructured.Unstructured, work *workv1alpha1.Work) {
+	util.RecordManagedAnnotations(workload)
+	if work.Labels[workv1alpha2.WorkPermanentIDLabel] != "" {
+		workload.SetLabels(util.DedupeAndMergeLabels(workload.GetLabels(), map[string]string{
+			workv1alpha2.WorkPermanentIDLabel: work.Labels[workv1alpha2.WorkPermanentIDLabel],
+		}))
+	}
+	util.RecordManagedLabels(workload)
+}

pkg/webhook/work/mutating.go

@@ -31,6 +31,7 @@ import (
 	workv1alpha2 "github.com/karmada-io/karmada/pkg/apis/work/v1alpha2"
 	"github.com/karmada-io/karmada/pkg/resourceinterpreter/default/native/prune"
 	"github.com/karmada-io/karmada/pkg/util"
+	"github.com/karmada-io/karmada/pkg/util/helper"
 )
 
 // MutatingAdmission mutates API request if necessary.
@@ -42,6 +43,9 @@ type MutatingAdmission struct {
 var _ admission.Handler = &MutatingAdmission{}
 
 // Handle yields a response to an AdmissionRequest.
+// This function could affect the informer cache of controller-manager.
+// In controller-manager, redundant updates to Work are avoided as much as possible, and changes to informer cache will affect this behavior.
+// Therefore, if someone modify this function, please ensure that the relevant logic in controller-manager also be updated.
 func (a *MutatingAdmission) Handle(_ context.Context, req admission.Request) admission.Response {
 	work := &workv1alpha1.Work{}
 
@@ -73,7 +77,7 @@ func (a *MutatingAdmission) Handle(_ context.Context, req admission.Request) adm
 
 		// Skip label/annotate the workload of Work that is not intended to be propagated.
 		if work.Labels[util.PropagationInstruction] != util.PropagationInstructionSuppressed {
-			setLabelsAndAnnotationsForWorkload(workloadObj, work)
+			helper.SetLabelsAndAnnotationsForWorkload(workloadObj, work)
 		}
 
 		workloadJSON, err := workloadObj.MarshalJSON()
@@ -92,12 +96,3 @@ func (a *MutatingAdmission) Handle(_ context.Context, req admission.Request) adm
 
 	return admission.PatchResponseFromRaw(req.Object.Raw, marshaledBytes)
 }
-
-// setLabelsAndAnnotationsForWorkload sets the associated work object labels and annotations for workload.
-func setLabelsAndAnnotationsForWorkload(workload *unstructured.Unstructured, work *workv1alpha1.Work) {
-	util.RecordManagedAnnotations(workload)
-	workload.SetLabels(util.DedupeAndMergeLabels(workload.GetLabels(), map[string]string{
-		workv1alpha2.WorkPermanentIDLabel: work.Labels[workv1alpha2.WorkPermanentIDLabel],
-	}))
-	util.RecordManagedLabels(workload)
-}