Skip to content
This repository has been archived by the owner on Apr 25, 2023. It is now read-only.

feat:Use patch to replace update in generic client for multi cases #1409

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions pkg/controller/sync/dispatch/unmanaged.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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)
}

Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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)
Expand Down
4 changes: 3 additions & 1 deletion pkg/kubefedctl/disable.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)
}
Expand Down
4 changes: 3 additions & 1 deletion pkg/kubefedctl/enable/enable.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)
}
Expand Down
4 changes: 3 additions & 1 deletion pkg/kubefedctl/join.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand Down
10 changes: 8 additions & 2 deletions test/common/typeconfig.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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
}
Expand Down
6 changes: 2 additions & 4 deletions test/e2e/crud.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
}
Expand Down
9 changes: 9 additions & 0 deletions test/e2e/validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand Down