add work mutating in ctrlutil.CreateOrUpdateWork()

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

pkg/controllers/ctrlutil/work.go

@@ -21,6 +21,7 @@ import (
 	"encoding/json"
 	"fmt"
 
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/runtime"
@@ -32,10 +33,11 @@ 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/util"
+	"github.com/karmada-io/karmada/pkg/util/mutating"
 )
 
 // 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 {
 	if workMeta.Labels[util.PropagationInstruction] != util.PropagationInstructionSuppressed {
 		resource = resource.DeepCopy()
 		// set labels
@@ -72,10 +74,35 @@ func CreateOrUpdateWork(ctx context.Context, client client.Client, workMeta meta
 
 	applyWorkOptions(work, options)
 
+	// get existing work to get the existing permanent ID
+	existingWork := &workv1alpha1.Work{}
+	err = c.Get(ctx, client.ObjectKeyFromObject(work), existingWork)
+	if err != nil {
+		if apierrors.IsNotFound(err) {
+			klog.V(2).Infof("Work %s/%s not found, will create it.", work.GetNamespace(), work.GetName())
+		} else {
+			klog.Errorf("Failed to get work %s/%s, error: %v", work.GetNamespace(), work.GetName(), err)
+			return err
+		}
+	}
+	if existingWork.Labels[workv1alpha2.WorkPermanentIDLabel] != "" {
+		if work.Labels == nil {
+			work.Labels = make(map[string]string)
+		}
+		work.Labels[workv1alpha2.WorkPermanentIDLabel] = existingWork.Labels[workv1alpha2.WorkPermanentIDLabel]
+	}
+	// mutate work here to let the deepEqual() have chance to return true, to reduce the HTTP UPDATE requests.
+	// otherwise it will always return false, because the informer cache is already mutated.
+	err = mutating.MutateWork(work)
+	if err != nil {
+		klog.Errorf("Failed to mutate work(%s/%s), error: %v", resource.GetNamespace(), resource.GetName(), err)
+		return err
+	}
+
 	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())
 			}

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/mutating/work.go

@@ -0,0 +1,75 @@
+/*
+Copyright 2020 The Karmada Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package mutating
+
+import (
+	"encoding/json"
+
+	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
+	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/klog/v2"
+
+	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"
+)
+
+// MutateWork mutates the Work object.
+// It's not contained the part of adding the permanent ID to work's label, because adding a random string will be hard to test.
+func MutateWork(work *workv1alpha1.Work) error {
+	var manifests []workv1alpha1.Manifest
+	for _, manifest := range work.Spec.Workload.Manifests {
+		workloadObj := &unstructured.Unstructured{}
+		err := json.Unmarshal(manifest.Raw, workloadObj)
+		if err != nil {
+			klog.Errorf("Failed to unmarshal the work(%s/%s) manifest to Unstructured, err: %v", work.Namespace, work.Name, err)
+			return err
+		}
+
+		err = prune.RemoveIrrelevantFields(workloadObj, prune.RemoveJobTTLSeconds)
+		if err != nil {
+			klog.Errorf("Failed to remove irrelevant fields for the work(%s/%s), err: %v", work.Namespace, work.Name, err)
+			return err
+		}
+
+		// Skip label/annotate the workload of Work that is not intended to be propagated.
+		if work.Labels[util.PropagationInstruction] != util.PropagationInstructionSuppressed {
+			setLabelsAndAnnotationsForWorkload(workloadObj, work)
+		}
+
+		workloadJSON, err := json.Marshal(workloadObj)
+		if err != nil {
+			klog.Errorf("Failed to marshal workload of the work(%s/%s), err: %s", work.Namespace, work.Name, err)
+			return err
+		}
+		manifests = append(manifests, workv1alpha1.Manifest{RawExtension: runtime.RawExtension{Raw: workloadJSON}})
+	}
+	work.Spec.Workload.Manifests = manifests
+	return nil
+}
+
+// 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

@@ -22,15 +22,13 @@ import (
 	"net/http"
 
 	"github.com/google/uuid"
-	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
-	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/klog/v2"
 	"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
 
 	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/mutating"
 )
 
 // MutatingAdmission mutates API request if necessary.
@@ -55,49 +53,15 @@ func (a *MutatingAdmission) Handle(_ context.Context, req admission.Request) adm
 		util.MergeLabel(work, workv1alpha2.WorkPermanentIDLabel, uuid.New().String())
 	}
 
-	var manifests []workv1alpha1.Manifest
-
-	for _, manifest := range work.Spec.Workload.Manifests {
-		workloadObj := &unstructured.Unstructured{}
-		err := json.Unmarshal(manifest.Raw, workloadObj)
-		if err != nil {
-			klog.Errorf("Failed to unmarshal the work(%s/%s) manifest to Unstructured, err: %v", work.Namespace, work.Name, err)
-			return admission.Errored(http.StatusInternalServerError, err)
-		}
-
-		err = prune.RemoveIrrelevantFields(workloadObj, prune.RemoveJobTTLSeconds)
-		if err != nil {
-			klog.Errorf("Failed to remove irrelevant fields for the work(%s/%s), err: %v", work.Namespace, work.Name, err)
-			return admission.Errored(http.StatusInternalServerError, err)
-		}
-
-		// Skip label/annotate the workload of Work that is not intended to be propagated.
-		if work.Labels[util.PropagationInstruction] != util.PropagationInstructionSuppressed {
-			setLabelsAndAnnotationsForWorkload(workloadObj, work)
-		}
-
-		workloadJSON, err := workloadObj.MarshalJSON()
-		if err != nil {
-			klog.Errorf("Failed to marshal workload of the work(%s/%s), err: %s", work.Namespace, work.Name, err)
-			return admission.Errored(http.StatusInternalServerError, err)
-		}
-		manifests = append(manifests, workv1alpha1.Manifest{RawExtension: runtime.RawExtension{Raw: workloadJSON}})
+	err = mutating.MutateWork(work)
+	if err != nil {
+		return admission.Errored(http.StatusInternalServerError, err)
 	}
 
-	work.Spec.Workload.Manifests = manifests
 	marshaledBytes, err := json.Marshal(work)
 	if err != nil {
 		return admission.Errored(http.StatusInternalServerError, err)
 	}
 
 	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)
-}