Merge pull request #1409 from qmhu/feat/patch-replace-update-more

feat:Use patch to replace update in generic client for multi cases
kubernetes-retired · May 27, 2021 · 82b2011 · 82b2011
2 parents 5030a05 + 9d988f5
commit 82b2011
Show file tree

Hide file tree

Showing 7 changed files with 33 additions and 13 deletions.
diff --git a/pkg/controller/sync/dispatch/unmanaged.go b/pkg/controller/sync/dispatch/unmanaged.go
@@ -28,7 +28,7 @@ import (
 	"k8s.io/apimachinery/pkg/util/runtime"
 	"k8s.io/klog/v2"
 
-	"sigs.k8s.io/controller-runtime/pkg/client"
+	runtimeclient "sigs.k8s.io/controller-runtime/pkg/client"
 
 	"sigs.k8s.io/kubefed/pkg/client/generic"
 	"sigs.k8s.io/kubefed/pkg/controller/sync/status"
@@ -43,7 +43,7 @@ const eventTemplate = "%s %s %q in cluster %q"
 type UnmanagedDispatcher interface {
 	OperationDispatcher
 
-	Delete(clusterName string, opts ...client.DeleteOption)
+	Delete(clusterName string, opts ...runtimeclient.DeleteOption)
 	RemoveManagedLabel(clusterName string, clusterObj *unstructured.Unstructured)
 }
 
@@ -74,7 +74,7 @@ func (d *unmanagedDispatcherImpl) Wait() (bool, error) {
 	return d.dispatcher.Wait()
 }
 
-func (d *unmanagedDispatcherImpl) Delete(clusterName string, opts ...client.DeleteOption) {
+func (d *unmanagedDispatcherImpl) Delete(clusterName string, opts ...runtimeclient.DeleteOption) {
 	start := time.Now()
 	d.dispatcher.incrementOperationsInitiated()
 	const op = "delete"
@@ -120,10 +120,11 @@ func (d *unmanagedDispatcherImpl) RemoveManagedLabel(clusterName string, cluster
 
 		// Avoid mutating the resource in the informer cache
 		updateObj := clusterObj.DeepCopy()
+		patch := runtimeclient.MergeFrom(updateObj.DeepCopy())
 
 		util.RemoveManagedLabel(updateObj)
 
-		err := client.Update(context.Background(), updateObj)
+		err := client.Patch(context.Background(), updateObj, patch)
 		if err != nil {
 			if d.recorder == nil {
 				wrappedErr := d.wrapOperationError(err, clusterName, op)

diff --git a/pkg/kubefedctl/disable.go b/pkg/kubefedctl/disable.go
@@ -33,6 +33,7 @@ import (
 	"k8s.io/apimachinery/pkg/util/wait"
 	"k8s.io/client-go/rest"
 	"k8s.io/klog/v2"
+	runtimeclient "sigs.k8s.io/controller-runtime/pkg/client"
 
 	"sigs.k8s.io/kubefed/pkg/apis/core/typeconfig"
 	fedv1b1 "sigs.k8s.io/kubefed/pkg/apis/core/v1beta1"
@@ -262,8 +263,9 @@ func checkFederatedTypeConfigExists(client genericclient.Client, typeConfig *fed
 
 func disablePropagation(client genericclient.Client, typeConfig *fedv1b1.FederatedTypeConfig, typeConfigName ctlutil.QualifiedName, write func(string)) error {
 	if typeConfig.GetPropagationEnabled() {
+		patch := runtimeclient.MergeFrom(typeConfig.DeepCopy())
 		typeConfig.Spec.Propagation = fedv1b1.PropagationDisabled
-		err := client.Update(context.TODO(), typeConfig)
+		err := client.Patch(context.TODO(), typeConfig, patch)
 		if err != nil {
 			return errors.Wrapf(err, "Error disabling propagation for FederatedTypeConfig %q", typeConfigName)
 		}

diff --git a/pkg/kubefedctl/enable/enable.go b/pkg/kubefedctl/enable/enable.go
@@ -34,6 +34,7 @@ import (
 	pkgruntime "k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/client-go/rest"
 	"k8s.io/klog/v2"
+	runtimeclient "sigs.k8s.io/controller-runtime/pkg/client"
 
 	"sigs.k8s.io/kubefed/pkg/apis/core/typeconfig"
 	fedv1b1 "sigs.k8s.io/kubefed/pkg/apis/core/v1beta1"
@@ -338,9 +339,10 @@ func CreateResources(cmdOut io.Writer, config *rest.Config, resources *typeResou
 			}
 		}
 	} else {
+		patch := runtimeclient.MergeFrom(existingTypeConfig.DeepCopy())
 		existingTypeConfig.Spec = concreteTypeConfig.Spec
 		if !dryRun {
-			err = client.Update(context.TODO(), existingTypeConfig)
+			err := client.Patch(context.TODO(), existingTypeConfig, patch)
 			if err != nil {
 				return errors.Wrapf(err, "Error updating FederatedTypeConfig %q", concreteTypeConfig.Name)
 			}

diff --git a/pkg/kubefedctl/join.go b/pkg/kubefedctl/join.go
@@ -37,6 +37,7 @@ import (
 	kubeclient "k8s.io/client-go/kubernetes"
 	"k8s.io/client-go/rest"
 	"k8s.io/klog/v2"
+	runtimeclient "sigs.k8s.io/controller-runtime/pkg/client"
 
 	fedv1b1 "sigs.k8s.io/kubefed/pkg/apis/core/v1beta1"
 	genericclient "sigs.k8s.io/kubefed/pkg/client/generic"
@@ -361,8 +362,9 @@ func createKubeFedCluster(client genericclient.Client, joiningClusterName, apiEn
 	case err == nil && errorOnExisting:
 		return nil, errors.Errorf("federated cluster %s already exists in host cluster", joiningClusterName)
 	case err == nil:
+		patch := runtimeclient.MergeFrom(existingFedCluster.DeepCopy())
 		existingFedCluster.Spec = fedCluster.Spec
-		err = client.Update(context.TODO(), existingFedCluster)
+		err := client.Patch(context.TODO(), existingFedCluster, patch)
 		if err != nil {
 			klog.V(2).Infof("Could not update federated cluster %s due to %v", fedCluster.Name, err)
 			return nil, err

diff --git a/test/common/typeconfig.go b/test/common/typeconfig.go
@@ -19,6 +19,8 @@ package common
 import (
 	"context"
 
+	runtimeclient "sigs.k8s.io/controller-runtime/pkg/client"
+
 	"sigs.k8s.io/kubefed/pkg/apis/core/typeconfig"
 	fedv1b1 "sigs.k8s.io/kubefed/pkg/apis/core/v1beta1"
 	client "sigs.k8s.io/kubefed/pkg/client/generic"
@@ -34,8 +36,12 @@ func GetTypeConfig(genericClient client.Client, name, namespace string) (typecon
 	return typeConfig, nil
 }
 
-func UpdateTypeConfig(genericClient client.Client, typeConfig *fedv1b1.FederatedTypeConfig, namespace string) error {
-	err := genericClient.Update(context.Background(), typeConfig)
+func EnableStatusCollection(genericClient client.Client, typeConfig *fedv1b1.FederatedTypeConfig) error {
+	patch := runtimeclient.MergeFrom(typeConfig.DeepCopy())
+	statusCollection := fedv1b1.StatusCollectionEnabled
+	typeConfig.Spec.StatusCollection = &statusCollection
+
+	err := genericClient.Patch(context.Background(), typeConfig, patch)
 	if err != nil {
 		return err
 	}

diff --git a/test/e2e/crud.go b/test/e2e/crud.go
@@ -284,11 +284,9 @@ func getCrudTestInput(f framework.KubeFedFramework, tl common.TestLogger,
 		tl.Logf("TypeConfig name: %s", tc.GetName())
 		if tc.GetName() == typeName {
 			tl.Logf("Enabling remote status collection for %v", typeConfig.GetFederatedType().Kind)
-			statusCollection := v1beta1.StatusCollectionEnabled
-			tc.Spec.StatusCollection = &statusCollection
-			err = common.UpdateTypeConfig(client, tc, f.KubeFedSystemNamespace())
+			err = common.EnableStatusCollection(client, tc)
 			if err != nil {
-				tl.Fatalf("Error updating the federatedtypeconfig %q: %v", typeConfigName, err)
+				tl.Fatalf("Error enabling the federatedtypeconfig %q: %v", typeConfigName, err)
 			}
 		}
 	}

diff --git a/test/e2e/validation.go b/test/e2e/validation.go
@@ -20,6 +20,8 @@ import (
 	"context"
 	"fmt"
 
+	runtimeclient "sigs.k8s.io/controller-runtime/pkg/client"
+
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	pkgruntime "k8s.io/apimachinery/pkg/runtime"
@@ -84,6 +86,13 @@ func runValidationResourceTests(f framework.KubeFedFramework, vrt validationReso
 			f.Logger().Fatalf("Expected error updating invalid %s = %+v", resourceName, vrt)
 		}
 
+		By(fmt.Sprintf("Patching with an invalid %s", resourceName))
+		patch := runtimeclient.MergeFrom(validObj.DeepCopyObject())
+		err = client.Patch(context.TODO(), invalidObj, patch)
+		if err == nil {
+			f.Logger().Fatalf("Expected error patching invalid %s = %+v", resourceName, vrt)
+		}
+
 		// Immediately delete the created test resource to avoid errors in
 		// other e2e tests that rely on the original e2e testing setup. For
 		// example for KubeFedCluster, delete the test cluster we just