Merge pull request #3775 from chethanv28/revert-patch-cr-changes

Revert "Use Patch operation for CnsFileAccessConfig updates"

Author: chethanv28

pkg/kubernetes/kubernetes.go

@@ -19,7 +19,6 @@ package kubernetes
 import (
 	"context"
 	"embed"
-	"encoding/json"
 	"flag"
 	"fmt"
 	"net"
@@ -38,8 +37,6 @@ import (
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apimachinery/pkg/runtime/serializer"
-	apitypes "k8s.io/apimachinery/pkg/types"
-	"k8s.io/apimachinery/pkg/util/strategicpatch"
 	"k8s.io/apimachinery/pkg/util/wait"
 	utilyaml "k8s.io/apimachinery/pkg/util/yaml"
 	clientset "k8s.io/client-go/kubernetes"
@@ -830,46 +827,3 @@ func RemoveFinalizer(ctx context.Context, c client.Client, obj client.Object, fi
 	log.Info("Removing finalizer from object.")
 	return c.Update(ctx, obj)
 }
-
-// PatchObject patches a Kubernetes object using strategic merge patch.
-// This function creates a patch between the original and modified objects and applies it using the client.
-// It returns an error if the patch operation fails.
-func PatchObject(ctx context.Context, k8sClient client.Client, original, modified client.Object) error {
-	log := logger.GetLogger(ctx)
-
-	// Marshal the original object
-	oldData, err := json.Marshal(original)
-	if err != nil {
-		log.Errorf("PatchObject: Failed to marshal original object %s/%s: %v",
-			original.GetNamespace(), original.GetName(), err)
-		return err
-	}
-
-	// Marshal the modified object
-	newData, err := json.Marshal(modified)
-	if err != nil {
-		log.Errorf("PatchObject: Failed to marshal modified object %s/%s: %v",
-			modified.GetNamespace(), modified.GetName(), err)
-		return err
-	}
-
-	// Create strategic merge patch
-	patchBytes, err := strategicpatch.CreateTwoWayMergePatch(oldData, newData, original)
-	if err != nil {
-		log.Errorf("PatchObject: Error creating strategic merge patch for object %s/%s: %v",
-			original.GetNamespace(), original.GetName(), err)
-		return err
-	}
-
-	// Apply the patch
-	patch := client.RawPatch(apitypes.StrategicMergePatchType, patchBytes)
-	if err := k8sClient.Patch(ctx, original, patch); err != nil {
-		log.Errorf("PatchObject: Failed to patch object %s/%s: %v",
-			original.GetNamespace(), original.GetName(), err)
-		return err
-	}
-
-	log.Debugf("PatchObject: Successfully patched object %s/%s",
-		original.GetNamespace(), original.GetName())
-	return nil
-}

pkg/syncer/cnsoperator/controller/cnsfileaccessconfig/cnsfileaccessconfig_controller.go

@@ -532,7 +532,7 @@ func addPvcFinalizer(ctx context.Context,
 			pvc.Namespace)
 	}
 
-	err = k8s.PatchObject(ctx, client, original, pvc)
+	err = util.PatchObject(ctx, client, original, pvc)
 	if err != nil {
 		return fmt.Errorf("failed to add finalizer %s on PVC %s in namespace %s", cnsoperatortypes.CNSPvcFinalizer,
 			instance.Spec.PvcName, instance.Namespace)
@@ -606,7 +606,7 @@ func removeFinalizerFromPVC(ctx context.Context, client client.Client,
 		return err
 	}
 
-	err = k8s.PatchObject(ctx, client, original, pvc)
+	err = util.PatchObject(ctx, client, original, pvc)
 	if err != nil {
 		return fmt.Errorf("failed to remove finalizer %s from PVC %s in namespace %s",
 			cnsoperatortypes.CNSPvcFinalizer, pvcName, pvc.Namespace)
@@ -875,7 +875,7 @@ func updateCnsFileAccessConfig(ctx context.Context, client client.Client,
 	instance *v1a1.CnsFileAccessConfig) error {
 	log := logger.GetLogger(ctx)
 	original := instance.DeepCopy()
-	err := k8s.PatchObject(ctx, client, original, instance)
+	err := util.PatchObject(ctx, client, original, instance)
 	if err != nil {
 		log.Errorf("failed to patch CnsFileAccessConfig instance: %q on namespace: %q. Error: %+v",
 			instance.Name, instance.Namespace, err)

pkg/syncer/cnsoperator/controller/cnsvolumemetadata/cnsvolumemetadata_controller.go

@@ -28,7 +28,6 @@ import (
 	"sigs.k8s.io/vsphere-csi-driver/v3/pkg/csi/service/common"
 	"sigs.k8s.io/vsphere-csi-driver/v3/pkg/csi/service/common/commonco"
 	"sigs.k8s.io/vsphere-csi-driver/v3/pkg/csi/service/logger"
-	k8s "sigs.k8s.io/vsphere-csi-driver/v3/pkg/kubernetes"
 	cnsoperatorutil "sigs.k8s.io/vsphere-csi-driver/v3/pkg/syncer/cnsoperator/util"
 
 	"github.com/davecgh/go-spew/spew"
@@ -56,6 +55,7 @@ import (
 	volumes "sigs.k8s.io/vsphere-csi-driver/v3/pkg/common/cns-lib/volume"
 	cnsvsphere "sigs.k8s.io/vsphere-csi-driver/v3/pkg/common/cns-lib/vsphere"
 	csitypes "sigs.k8s.io/vsphere-csi-driver/v3/pkg/csi/types"
+	k8s "sigs.k8s.io/vsphere-csi-driver/v3/pkg/kubernetes"
 	cnsoperatortypes "sigs.k8s.io/vsphere-csi-driver/v3/pkg/syncer/cnsoperator/types"
 )
 
@@ -234,7 +234,7 @@ func (r *ReconcileCnsVolumeMetadata) Reconcile(ctx context.Context,
 			recordEvent(ctx, r, instance, v1.EventTypeWarning, msg)
 			// Update instance.status fields with the errors per volume.
 			original := instance.DeepCopy()
-			if err = k8s.PatchObject(ctx, r.client, original, instance); err != nil {
+			if err = cnsoperatorutil.PatchObject(ctx, r.client, original, instance); err != nil {
 				msg := fmt.Sprintf("ReconcileCnsVolumeMetadata: Failed to patch status for %q. "+
 					"Err: %v.", instance.Name, err)
 				recordEvent(ctx, r, instance, v1.EventTypeWarning, msg)
@@ -249,7 +249,7 @@ func (r *ReconcileCnsVolumeMetadata) Reconcile(ctx context.Context,
 				log.Debugf("ReconcileCnsVolumeMetadata: Removing finalizer %q for instance %q", finalizer, instance.Name)
 				original := instance.DeepCopy()
 				instance.Finalizers = append(instance.Finalizers[:index], instance.Finalizers[index+1:]...)
-				if err = k8s.PatchObject(ctx, r.client, original, instance); err != nil {
+				if err = cnsoperatorutil.PatchObject(ctx, r.client, original, instance); err != nil {
 					msg := fmt.Sprintf("ReconcileCnsVolumeMetadata: Failed to remove finalizer %q for %q. "+
 						"Err: %v. Requeueing request.", finalizer, instance.Name, err)
 					recordEvent(ctx, r, instance, v1.EventTypeWarning, msg)
@@ -280,7 +280,7 @@ func (r *ReconcileCnsVolumeMetadata) Reconcile(ctx context.Context,
 	if !isFinalizerSet {
 		original := instance.DeepCopy()
 		instance.Finalizers = append(instance.Finalizers, cnsoperatortypes.CNSFinalizer)
-		if err = k8s.PatchObject(ctx, r.client, original, instance); err != nil {
+		if err = cnsoperatorutil.PatchObject(ctx, r.client, original, instance); err != nil {
 			msg := fmt.Sprintf("ReconcileCnsVolumeMetadata: Failed to add finalizer %q for %q. "+
 				"Err: %v. Requeueing request.", cnsoperatortypes.CNSFinalizer, instance.Name, err)
 			recordEvent(ctx, r, instance, v1.EventTypeWarning, msg)
@@ -297,7 +297,7 @@ func (r *ReconcileCnsVolumeMetadata) Reconcile(ctx context.Context,
 			// Update instance.status fields on supervisor API server and requeue
 			// the request.
 			original := instance.DeepCopy()
-			_ = k8s.PatchObject(ctx, r.client, original, instance)
+			_ = cnsoperatorutil.PatchObject(ctx, r.client, original, instance)
 			return reconcile.Result{RequeueAfter: timeout}, nil
 		}
 		// Successfully updated CNS.
@@ -307,7 +307,7 @@ func (r *ReconcileCnsVolumeMetadata) Reconcile(ctx context.Context,
 		recordEvent(ctx, r, instance, v1.EventTypeNormal, msg)
 		// Update instance.status fields on supervisor API server.
 		original := instance.DeepCopy()
-		if err = k8s.PatchObject(ctx, r.client, original, instance); err != nil {
+		if err = cnsoperatorutil.PatchObject(ctx, r.client, original, instance); err != nil {
 			msg := fmt.Sprintf("ReconcileCnsVolumeMetadata: Failed to patch status for %q. "+
 				"Err: %v. Requeueing request.", instance.Name, err)
 			recordEvent(ctx, r, instance, v1.EventTypeWarning, msg)

pkg/syncer/cnsoperator/controller/cnsvolumemetadata/cnsvolumemetadata_controller_test.go

@@ -29,8 +29,8 @@ import (
 
 	cnsoperatorapis "sigs.k8s.io/vsphere-csi-driver/v3/pkg/apis/cnsoperator"
 	cnsvolumemetadatav1alpha1 "sigs.k8s.io/vsphere-csi-driver/v3/pkg/apis/cnsoperator/cnsvolumemetadata/v1alpha1"
-	k8s "sigs.k8s.io/vsphere-csi-driver/v3/pkg/kubernetes"
 	cnsoperatortypes "sigs.k8s.io/vsphere-csi-driver/v3/pkg/syncer/cnsoperator/types"
+	cnsoperatorutil "sigs.k8s.io/vsphere-csi-driver/v3/pkg/syncer/cnsoperator/util"
 )
 
 func TestCnsVolumeMetadata_PatchOperations(t *testing.T) {
@@ -211,7 +211,7 @@ func TestCnsVolumeMetadata_PatchOperations(t *testing.T) {
 			fakeClient, original, modified := tt.setupFunc()
 
 			// Execute the patch operation using our common utility
-			err := k8s.PatchObject(ctx, fakeClient, original, modified)
+			err := cnsoperatorutil.PatchObject(ctx, fakeClient, original, modified)
 
 			// Validate results
 			if tt.expectError {
@@ -253,7 +253,7 @@ func TestCnsVolumeMetadata_PatchErrorHandling(t *testing.T) {
 		modified.Finalizers = append(modified.Finalizers, cnsoperatortypes.CNSFinalizer)
 
 		// Execute the patch operation - should fail
-		err := k8s.PatchObject(ctx, fakeClient, original, modified)
+		err := cnsoperatorutil.PatchObject(ctx, fakeClient, original, modified)
 		assert.Error(t, err)
 		assert.Contains(t, err.Error(), "not found")
 	})
@@ -282,7 +282,7 @@ func TestCnsVolumeMetadata_PatchErrorHandling(t *testing.T) {
 		}
 
 		// This should succeed as the objects are valid
-		err := k8s.PatchObject(ctx, fakeClient, original, modified)
+		err := cnsoperatorutil.PatchObject(ctx, fakeClient, original, modified)
 		assert.NoError(t, err)
 
 		// Verify the patch was applied

pkg/syncer/cnsoperator/util/util.go

@@ -18,6 +18,7 @@ package util
 
 import (
 	"context"
+	"encoding/json"
 	"errors"
 	"fmt"
 	"os"
@@ -30,6 +31,7 @@ import (
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	apitypes "k8s.io/apimachinery/pkg/types"
+	"k8s.io/apimachinery/pkg/util/strategicpatch"
 	"k8s.io/client-go/dynamic"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	cnsvsphere "sigs.k8s.io/vsphere-csi-driver/v3/pkg/common/cns-lib/vsphere"
@@ -397,3 +399,46 @@ func GetMaxWorkerThreads(ctx context.Context, key string, defaultVal int) int {
 	}
 	return workerThreads
 }
+
+// PatchObject patches a Kubernetes object using strategic merge patch.
+// This function creates a patch between the original and modified objects and applies it using the client.
+// It returns an error if the patch operation fails.
+func PatchObject(ctx context.Context, k8sClient client.Client, original, modified client.Object) error {
+	log := logger.GetLogger(ctx)
+
+	// Marshal the original object
+	oldData, err := json.Marshal(original)
+	if err != nil {
+		log.Errorf("PatchObject: Failed to marshal original object %s/%s: %v",
+			original.GetNamespace(), original.GetName(), err)
+		return err
+	}
+
+	// Marshal the modified object
+	newData, err := json.Marshal(modified)
+	if err != nil {
+		log.Errorf("PatchObject: Failed to marshal modified object %s/%s: %v",
+			modified.GetNamespace(), modified.GetName(), err)
+		return err
+	}
+
+	// Create strategic merge patch
+	patchBytes, err := strategicpatch.CreateTwoWayMergePatch(oldData, newData, original)
+	if err != nil {
+		log.Errorf("PatchObject: Error creating strategic merge patch for object %s/%s: %v",
+			original.GetNamespace(), original.GetName(), err)
+		return err
+	}
+
+	// Apply the patch
+	patch := client.RawPatch(apitypes.StrategicMergePatchType, patchBytes)
+	if err := k8sClient.Patch(ctx, original, patch); err != nil {
+		log.Errorf("PatchObject: Failed to patch object %s/%s: %v",
+			original.GetNamespace(), original.GetName(), err)
+		return err
+	}
+
+	log.Debugf("PatchObject: Successfully patched object %s/%s",
+		original.GetNamespace(), original.GetName())
+	return nil
+}

pkg/syncer/cnsoperator/util/util_test.go

@@ -32,7 +32,6 @@ import (
 	ctrlruntimefake "sigs.k8s.io/controller-runtime/pkg/client/fake"
 	cnsoperatorapis "sigs.k8s.io/vsphere-csi-driver/v3/pkg/apis/cnsoperator"
 	cnsvolumemetadatav1alpha1 "sigs.k8s.io/vsphere-csi-driver/v3/pkg/apis/cnsoperator/cnsvolumemetadata/v1alpha1"
-	k8s "sigs.k8s.io/vsphere-csi-driver/v3/pkg/kubernetes"
 )
 
 func TestGetSnatIpFromNamespaceNetworkInfo(t *testing.T) {
@@ -346,7 +345,7 @@ func TestPatchObject(t *testing.T) {
 		t.Run(tt.name, func(t *testing.T) {
 			fakeClient, original, modified := tt.setupFunc()
 
-			err := k8s.PatchObject(ctx, fakeClient, original, modified)
+			err := PatchObject(ctx, fakeClient, original, modified)
 
 			if tt.expectError {
 				assert.Error(t, err)

pkg/syncer/pvcsi_fullsync.go

@@ -41,6 +41,7 @@ import (
 	cnsconfig "sigs.k8s.io/vsphere-csi-driver/v3/pkg/common/config"
 	"sigs.k8s.io/vsphere-csi-driver/v3/pkg/common/prometheus"
 	"sigs.k8s.io/vsphere-csi-driver/v3/pkg/csi/service/logger"
+	cnsoperatorutil "sigs.k8s.io/vsphere-csi-driver/v3/pkg/syncer/cnsoperator/util"
 )
 
 var (
@@ -145,7 +146,7 @@ func PvcsiFullSync(ctx context.Context, metadataSyncer *metadataSyncInformer) er
 				log.Infof("FullSync: Patching CnsVolumeMetadata %v on the supervisor cluster", guestObject.Name)
 				original := supervisorObject.DeepCopy()
 				supervisorObject.Spec = guestObject.Spec
-				if err := k8s.PatchObject(ctx, metadataSyncer.cnsOperatorClient, original,
+				if err := cnsoperatorutil.PatchObject(ctx, metadataSyncer.cnsOperatorClient, original,
 					supervisorObject); err != nil {
 					log.Warnf("FullSync: Failed to patch CnsVolumeMetadata %v. Err: %v", supervisorObject.Name, err)
 				}

pkg/syncer/pvcsi_metadatasyncer.go

@@ -28,7 +28,7 @@ import (
 	cnsvolumemetadatav1alpha1 "sigs.k8s.io/vsphere-csi-driver/v3/pkg/apis/cnsoperator/cnsvolumemetadata/v1alpha1"
 	cnsconfig "sigs.k8s.io/vsphere-csi-driver/v3/pkg/common/config"
 	"sigs.k8s.io/vsphere-csi-driver/v3/pkg/csi/service/logger"
-	k8s "sigs.k8s.io/vsphere-csi-driver/v3/pkg/kubernetes"
+	cnsoperatorutil "sigs.k8s.io/vsphere-csi-driver/v3/pkg/syncer/cnsoperator/util"
 )
 
 // pvcsiVolumeUpdated updates persistent volume claim and persistent volume
@@ -80,7 +80,7 @@ func pvcsiVolumeUpdated(ctx context.Context, resourceType interface{},
 	newMetadata.ResourceVersion = currentMetadata.ResourceVersion
 	newMetadata.Namespace = supervisorNamespace
 	log.Debugf("pvCSI VolumeUpdated: Invoking patch on CnsVolumeMetadata with spec: %+v", spew.Sdump(newMetadata))
-	if err := k8s.PatchObject(ctx, metadataSyncer.cnsOperatorClient, currentMetadata,
+	if err := cnsoperatorutil.PatchObject(ctx, metadataSyncer.cnsOperatorClient, currentMetadata,
 		newMetadata); err != nil {
 		log.Errorf("pvCSI VolumeUpdated: Failed to patch CnsVolumeMetadata: %v. Error: %v", newMetadata.Name, err)
 		return