Eliminate Try{Update,Patch}* (gardener#4799)

* Eliminate TryPatch*

* Eliminate TryUpdate

* Eliminate TryUpdateStatus

extensions/pkg/controller/backupentry/reconciler.go

@@ -265,7 +265,7 @@ func (r *reconciler) migrate(ctx context.Context, be *extensionsv1alpha1.BackupE
 	}
 
 	r.logger.Info("Removing all finalizers", "backupentry", kutil.ObjectName(be))
-	if err := extensionscontroller.DeleteAllFinalizers(ctx, r.client, be); err != nil {
+	if err := controllerutils.RemoveAllFinalizers(ctx, r.client, r.client, be); err != nil {
 		return reconcile.Result{}, fmt.Errorf("error removing all finalizers from backupentry: %+v", err)
 	}
 

extensions/pkg/controller/containerruntime/reconciler.go

@@ -219,7 +219,7 @@ func (r *reconciler) migrate(ctx context.Context, cr *extensionsv1alpha1.Contain
 	}
 
 	r.logger.Info("Removing all finalizers", "containerruntime", kutil.ObjectName(cr))
-	if err := extensionscontroller.DeleteAllFinalizers(ctx, r.client, cr); err != nil {
+	if err := controllerutils.RemoveAllFinalizers(ctx, r.client, r.client, cr); err != nil {
 		return reconcile.Result{}, fmt.Errorf("error removing finalizers from the containerruntime: %+v", err)
 	}
 

extensions/pkg/controller/controlplane/reconciler.go

@@ -203,7 +203,7 @@ func (r *reconciler) migrate(ctx context.Context, cp *extensionsv1alpha1.Control
 	}
 
 	r.logger.Info("Removing all finalizers.", "controlplane", kutil.ObjectName(cp))
-	if err := extensionscontroller.DeleteAllFinalizers(ctx, r.client, cp); err != nil {
+	if err := controllerutils.RemoveAllFinalizers(ctx, r.client, r.client, cp); err != nil {
 		return reconcile.Result{}, fmt.Errorf("error removing finalizers from controlplane: %+v", err)
 	}
 

extensions/pkg/controller/dnsrecord/reconciler.go

@@ -196,7 +196,7 @@ func (r *reconciler) migrate(ctx context.Context, dns *extensionsv1alpha1.DNSRec
 	}
 
 	r.logger.Info("Removing all finalizers", "dnsrecord", kutil.ObjectName(dns))
-	if err := extensionscontroller.DeleteAllFinalizers(ctx, r.client, dns); err != nil {
+	if err := controllerutils.RemoveAllFinalizers(ctx, r.client, r.client, dns); err != nil {
 		return reconcile.Result{}, fmt.Errorf("error removing finalizers from dnsrecord: %+v", err)
 	}
 

extensions/pkg/controller/extension/reconciler.go

@@ -233,7 +233,7 @@ func (r *reconciler) migrate(ctx context.Context, ex *extensionsv1alpha1.Extensi
 	}
 
 	r.logger.Info("Removing all finalizers", "extension", kutil.ObjectName(ex))
-	if err := extensionscontroller.DeleteAllFinalizers(ctx, r.client, ex); err != nil {
+	if err := controllerutils.RemoveAllFinalizers(ctx, r.client, r.client, ex); err != nil {
 		return reconcile.Result{}, fmt.Errorf("error removing finalizers from extension: %+v", err)
 	}
 

extensions/pkg/controller/healthcheck/reconciler.go

@@ -23,7 +23,6 @@ import (
 	"k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
-	"k8s.io/client-go/util/retry"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/log"
 	"sigs.k8s.io/controller-runtime/pkg/manager"
@@ -36,7 +35,6 @@ import (
 	gardenv1beta1constants "github.com/gardener/gardener/pkg/apis/core/v1beta1/constants"
 	gardencorev1beta1helper "github.com/gardener/gardener/pkg/apis/core/v1beta1/helper"
 	extensionsv1alpha1 "github.com/gardener/gardener/pkg/apis/extensions/v1alpha1"
-	"github.com/gardener/gardener/pkg/controllerutils"
 	kutil "github.com/gardener/gardener/pkg/utils/kubernetes"
 )
 
@@ -268,19 +266,17 @@ type condition struct {
 }
 
 func (r *reconciler) updateExtensionConditions(ctx context.Context, extension extensionsv1alpha1.Object, conditions ...condition) error {
-	return controllerutils.TryPatchStatus(ctx, retry.DefaultBackoff, r.client, extension, func() error {
-		for _, cond := range conditions {
-			now := metav1.Now()
-			if c := gardencorev1beta1helper.GetCondition(extension.GetExtensionStatus().GetConditions(), gardencorev1beta1.ConditionType(cond.healthConditionType)); c != nil {
-				cond.builder.WithOldCondition(*c)
-			}
-			updatedCondition, _ := cond.builder.WithNowFunc(func() metav1.Time { return now }).Build()
-			// always update - the Gardenlet expects a recent health check
-			updatedCondition.LastUpdateTime = now
-			extension.GetExtensionStatus().SetConditions(gardencorev1beta1helper.MergeConditions(extension.GetExtensionStatus().GetConditions(), updatedCondition))
+	for _, cond := range conditions {
+		now := metav1.Now()
+		if c := gardencorev1beta1helper.GetCondition(extension.GetExtensionStatus().GetConditions(), gardencorev1beta1.ConditionType(cond.healthConditionType)); c != nil {
+			cond.builder.WithOldCondition(*c)
 		}
-		return nil
-	})
+		updatedCondition, _ := cond.builder.WithNowFunc(func() metav1.Time { return now }).Build()
+		// always update - the Gardenlet expects a recent health check
+		updatedCondition.LastUpdateTime = now
+		extension.GetExtensionStatus().SetConditions(gardencorev1beta1helper.MergeConditions(extension.GetExtensionStatus().GetConditions(), updatedCondition))
+	}
+	return r.client.Status().Update(ctx, extension)
 }
 
 func (r *reconciler) resultWithRequeue() reconcile.Result {

extensions/pkg/controller/network/reconciler.go

@@ -216,7 +216,7 @@ func (r *reconciler) migrate(ctx context.Context, network *extensionsv1alpha1.Ne
 	}
 
 	r.logger.Info("Removing all finalizers", "network", kutil.ObjectName(network))
-	if err := extensionscontroller.DeleteAllFinalizers(ctx, r.client, network); err != nil {
+	if err := controllerutils.RemoveAllFinalizers(ctx, r.client, r.client, network); err != nil {
 		return reconcile.Result{}, fmt.Errorf("error removing finalizers from the network: %+v", err)
 	}
 

extensions/pkg/controller/operatingsystemconfig/reconciler.go

@@ -264,8 +264,8 @@ func (r *reconciler) migrate(ctx context.Context, osc *extensionsv1alpha1.Operat
 	}
 
 	r.logger.Info("Removing finalizer.", "osc", osc.Name)
-	if err := extensionscontroller.DeleteAllFinalizers(ctx, r.client, osc); err != nil {
-		return reconcile.Result{}, fmt.Errorf("Error removing all finalizers from operatingsystemconfig: %+v", err)
+	if err := controllerutils.RemoveAllFinalizers(ctx, r.client, r.client, osc); err != nil {
+		return reconcile.Result{}, fmt.Errorf("error removing all finalizers from operatingsystemconfig: %+v", err)
 	}
 
 	if err := extensionscontroller.RemoveAnnotation(ctx, r.client, osc, v1beta1constants.GardenerOperation); err != nil {

extensions/pkg/controller/status.go

@@ -18,15 +18,13 @@ import (
 	"context"
 	"fmt"
 
-	gardencorev1beta1 "github.com/gardener/gardener/pkg/apis/core/v1beta1"
-	gardencorev1beta1helper "github.com/gardener/gardener/pkg/apis/core/v1beta1/helper"
-	extensionsv1alpha1 "github.com/gardener/gardener/pkg/apis/extensions/v1alpha1"
-	"github.com/gardener/gardener/pkg/controllerutils"
-
 	"github.com/go-logr/logr"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"k8s.io/client-go/util/retry"
 	"sigs.k8s.io/controller-runtime/pkg/client"
+
+	gardencorev1beta1 "github.com/gardener/gardener/pkg/apis/core/v1beta1"
+	gardencorev1beta1helper "github.com/gardener/gardener/pkg/apis/core/v1beta1/helper"
+	extensionsv1alpha1 "github.com/gardener/gardener/pkg/apis/extensions/v1alpha1"
 )
 
 // LastOperation creates a new LastOperation from the given parameters.
@@ -96,12 +94,10 @@ func (s *statusUpdater) Processing(ctx context.Context, obj extensionsv1alpha1.O
 
 	s.logger.Info(description, s.logKeysAndValues(obj)...)
 
-	return controllerutils.TryUpdateStatus(ctx, retry.DefaultBackoff, s.client, obj, func() error {
-		lastOp := LastOperation(lastOperationType, gardencorev1beta1.LastOperationStateProcessing, 1, description)
-
-		obj.GetExtensionStatus().SetLastOperation(lastOp)
-		return nil
-	})
+	patch := client.MergeFrom(obj.DeepCopyObject().(client.Object))
+	lastOp := LastOperation(lastOperationType, gardencorev1beta1.LastOperationStateProcessing, 1, description)
+	obj.GetExtensionStatus().SetLastOperation(lastOp)
+	return s.client.Status().Patch(ctx, obj, patch)
 }
 
 func (s *statusUpdater) Error(ctx context.Context, obj extensionsv1alpha1.Object, err error, lastOperationType gardencorev1beta1.LastOperationType, description string) error {
@@ -112,14 +108,13 @@ func (s *statusUpdater) Error(ctx context.Context, obj extensionsv1alpha1.Object
 	errDescription := gardencorev1beta1helper.FormatLastErrDescription(fmt.Errorf("%s: %v", description, err))
 	s.logger.Error(fmt.Errorf(errDescription), "error", s.logKeysAndValues(obj)...)
 
-	return controllerutils.TryUpdateStatus(ctx, retry.DefaultBackoff, s.client, obj, func() error {
-		lastOp, lastErr := ReconcileError(lastOperationType, errDescription, 50, gardencorev1beta1helper.ExtractErrorCodes(gardencorev1beta1helper.DetermineError(err, err.Error()))...)
+	lastOp, lastErr := ReconcileError(lastOperationType, errDescription, 50, gardencorev1beta1helper.ExtractErrorCodes(gardencorev1beta1helper.DetermineError(err, err.Error()))...)
 
-		obj.GetExtensionStatus().SetObservedGeneration(obj.GetGeneration())
-		obj.GetExtensionStatus().SetLastOperation(lastOp)
-		obj.GetExtensionStatus().SetLastError(lastErr)
-		return nil
-	})
+	patch := client.MergeFrom(obj.DeepCopyObject().(client.Object))
+	obj.GetExtensionStatus().SetObservedGeneration(obj.GetGeneration())
+	obj.GetExtensionStatus().SetLastOperation(lastOp)
+	obj.GetExtensionStatus().SetLastError(lastErr)
+	return s.client.Status().Patch(ctx, obj, patch)
 }
 
 func (s *statusUpdater) Success(ctx context.Context, obj extensionsv1alpha1.Object, lastOperationType gardencorev1beta1.LastOperationType, description string) error {
@@ -129,14 +124,12 @@ func (s *statusUpdater) Success(ctx context.Context, obj extensionsv1alpha1.Obje
 
 	s.logger.Info(description, s.logKeysAndValues(obj)...)
 
-	return controllerutils.TryUpdateStatus(ctx, retry.DefaultBackoff, s.client, obj, func() error {
-		lastOp, lastErr := ReconcileSucceeded(lastOperationType, description)
-
-		obj.GetExtensionStatus().SetObservedGeneration(obj.GetGeneration())
-		obj.GetExtensionStatus().SetLastOperation(lastOp)
-		obj.GetExtensionStatus().SetLastError(lastErr)
-		return nil
-	})
+	patch := client.MergeFrom(obj.DeepCopyObject().(client.Object))
+	lastOp, lastErr := ReconcileSucceeded(lastOperationType, description)
+	obj.GetExtensionStatus().SetObservedGeneration(obj.GetGeneration())
+	obj.GetExtensionStatus().SetLastOperation(lastOp)
+	obj.GetExtensionStatus().SetLastError(lastErr)
+	return s.client.Status().Patch(ctx, obj, patch)
 }
 
 func (s *statusUpdater) logKeysAndValues(obj metav1.Object) []interface{} {

extensions/pkg/controller/status_test.go

@@ -19,19 +19,18 @@ import (
 	"fmt"
 	"strings"
 
-	. "github.com/gardener/gardener/extensions/pkg/controller"
-	gardencorev1beta1 "github.com/gardener/gardener/pkg/apis/core/v1beta1"
-	extensionsv1alpha1 "github.com/gardener/gardener/pkg/apis/extensions/v1alpha1"
-	mockclient "github.com/gardener/gardener/pkg/mock/controller-runtime/client"
-	kutil "github.com/gardener/gardener/pkg/utils/kubernetes"
-
 	"github.com/go-logr/logr"
 	"github.com/golang/mock/gomock"
 	. "github.com/onsi/ginkgo"
 	. "github.com/onsi/gomega"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	logzap "sigs.k8s.io/controller-runtime/pkg/log/zap"
+
+	. "github.com/gardener/gardener/extensions/pkg/controller"
+	gardencorev1beta1 "github.com/gardener/gardener/pkg/apis/core/v1beta1"
+	extensionsv1alpha1 "github.com/gardener/gardener/pkg/apis/extensions/v1alpha1"
+	mockclient "github.com/gardener/gardener/pkg/mock/controller-runtime/client"
 )
 
 var _ = Describe("Status", func() {
@@ -71,20 +70,10 @@ var _ = Describe("Status", func() {
 	})
 
 	Describe("#Processing", func() {
-		It("should return an error if the Get() call fails", func() {
+		It("should return an error if the Patch() call fails", func() {
 			gomock.InOrder(
 				c.EXPECT().Status().Return(c),
-				c.EXPECT().Get(ctx, kutil.Key(obj.GetNamespace(), obj.GetName()), gomock.AssignableToTypeOf(&extensionsv1alpha1.Infrastructure{})).Return(fakeErr),
-			)
-
-			Expect(statusUpdater.Processing(ctx, obj, lastOpType, lastOpDesc)).To(MatchError(fakeErr))
-		})
-
-		It("should return an error if the Update() call fails", func() {
-			gomock.InOrder(
-				c.EXPECT().Status().Return(c),
-				c.EXPECT().Get(ctx, kutil.Key(obj.GetNamespace(), obj.GetName()), gomock.AssignableToTypeOf(&extensionsv1alpha1.Infrastructure{})),
-				c.EXPECT().Update(ctx, gomock.AssignableToTypeOf(&extensionsv1alpha1.Infrastructure{})).Return(fakeErr),
+				c.EXPECT().Patch(ctx, gomock.AssignableToTypeOf(&extensionsv1alpha1.Infrastructure{}), gomock.Any()).Return(fakeErr),
 			)
 
 			Expect(statusUpdater.Processing(ctx, obj, lastOpType, lastOpDesc)).To(MatchError(fakeErr))
@@ -93,8 +82,7 @@ var _ = Describe("Status", func() {
 		It("should update the last operation as expected", func() {
 			gomock.InOrder(
 				c.EXPECT().Status().Return(c),
-				c.EXPECT().Get(ctx, kutil.Key(obj.GetNamespace(), obj.GetName()), gomock.AssignableToTypeOf(&extensionsv1alpha1.Infrastructure{})),
-				c.EXPECT().Update(ctx, gomock.AssignableToTypeOf(&extensionsv1alpha1.Infrastructure{})).Do(func(ctx context.Context, obj extensionsv1alpha1.Object, opts ...client.UpdateOption) {
+				c.EXPECT().Patch(ctx, gomock.AssignableToTypeOf(&extensionsv1alpha1.Infrastructure{}), gomock.Any()).Do(func(ctx context.Context, obj extensionsv1alpha1.Object, patch client.Patch, opts ...client.PatchOption) {
 					lastOperation := obj.GetExtensionStatus().GetLastOperation()
 
 					Expect(lastOperation.Type).To(Equal(lastOpType))
@@ -109,20 +97,10 @@ var _ = Describe("Status", func() {
 	})
 
 	Describe("#Error", func() {
-		It("should return an error if the Get() call fails", func() {
-			gomock.InOrder(
-				c.EXPECT().Status().Return(c),
-				c.EXPECT().Get(ctx, kutil.Key(obj.GetNamespace(), obj.GetName()), gomock.AssignableToTypeOf(&extensionsv1alpha1.Infrastructure{})).Return(fakeErr),
-			)
-
-			Expect(statusUpdater.Error(ctx, obj, fakeErr, lastOpType, lastOpDesc)).To(MatchError(fakeErr))
-		})
-
-		It("should return an error if the Update() call fails", func() {
+		It("should return an error if the Patch() call fails", func() {
 			gomock.InOrder(
 				c.EXPECT().Status().Return(c),
-				c.EXPECT().Get(ctx, kutil.Key(obj.GetNamespace(), obj.GetName()), gomock.AssignableToTypeOf(&extensionsv1alpha1.Infrastructure{})),
-				c.EXPECT().Update(ctx, gomock.AssignableToTypeOf(&extensionsv1alpha1.Infrastructure{})).Return(fakeErr),
+				c.EXPECT().Patch(ctx, gomock.AssignableToTypeOf(&extensionsv1alpha1.Infrastructure{}), gomock.Any()).Return(fakeErr),
 			)
 
 			Expect(statusUpdater.Error(ctx, obj, fakeErr, lastOpType, lastOpDesc)).To(MatchError(fakeErr))
@@ -131,8 +109,7 @@ var _ = Describe("Status", func() {
 		It("should update the last operation as expected (w/o error codes)", func() {
 			gomock.InOrder(
 				c.EXPECT().Status().Return(c),
-				c.EXPECT().Get(ctx, kutil.Key(obj.GetNamespace(), obj.GetName()), gomock.AssignableToTypeOf(&extensionsv1alpha1.Infrastructure{})),
-				c.EXPECT().Update(ctx, gomock.AssignableToTypeOf(&extensionsv1alpha1.Infrastructure{})).Do(func(ctx context.Context, obj extensionsv1alpha1.Object, opts ...client.UpdateOption) {
+				c.EXPECT().Patch(ctx, gomock.AssignableToTypeOf(&extensionsv1alpha1.Infrastructure{}), gomock.Any()).Do(func(ctx context.Context, obj extensionsv1alpha1.Object, patch client.Patch, opts ...client.PatchOption) {
 					var (
 						description = strings.Title(lastOpDesc) + ": " + fakeErr.Error()
 
@@ -162,8 +139,7 @@ var _ = Describe("Status", func() {
 
 			gomock.InOrder(
 				c.EXPECT().Status().Return(c),
-				c.EXPECT().Get(ctx, kutil.Key(obj.GetNamespace(), obj.GetName()), gomock.AssignableToTypeOf(&extensionsv1alpha1.Infrastructure{})),
-				c.EXPECT().Update(ctx, gomock.AssignableToTypeOf(&extensionsv1alpha1.Infrastructure{})).Do(func(ctx context.Context, obj extensionsv1alpha1.Object, opts ...client.UpdateOption) {
+				c.EXPECT().Patch(ctx, gomock.AssignableToTypeOf(&extensionsv1alpha1.Infrastructure{}), gomock.Any()).Do(func(ctx context.Context, obj extensionsv1alpha1.Object, patch client.Patch, opts ...client.PatchOption) {
 					var (
 						description = strings.Title(lastOpDesc) + ": " + err.Error()
 
@@ -190,20 +166,10 @@ var _ = Describe("Status", func() {
 	})
 
 	Describe("#Success", func() {
-		It("should return an error if the Get() call fails", func() {
-			gomock.InOrder(
-				c.EXPECT().Status().Return(c),
-				c.EXPECT().Get(ctx, kutil.Key(obj.GetNamespace(), obj.GetName()), gomock.AssignableToTypeOf(&extensionsv1alpha1.Infrastructure{})).Return(fakeErr),
-			)
-
-			Expect(statusUpdater.Success(ctx, obj, lastOpType, lastOpDesc)).To(MatchError(fakeErr))
-		})
-
-		It("should return an error if the Update() call fails", func() {
+		It("should return an error if the Patch() call fails", func() {
 			gomock.InOrder(
 				c.EXPECT().Status().Return(c),
-				c.EXPECT().Get(ctx, kutil.Key(obj.GetNamespace(), obj.GetName()), gomock.AssignableToTypeOf(&extensionsv1alpha1.Infrastructure{})),
-				c.EXPECT().Update(ctx, gomock.AssignableToTypeOf(&extensionsv1alpha1.Infrastructure{})).Return(fakeErr),
+				c.EXPECT().Patch(ctx, gomock.AssignableToTypeOf(&extensionsv1alpha1.Infrastructure{}), gomock.Any()).Return(fakeErr),
 			)
 
 			Expect(statusUpdater.Success(ctx, obj, lastOpType, lastOpDesc)).To(MatchError(fakeErr))
@@ -212,8 +178,7 @@ var _ = Describe("Status", func() {
 		It("should update the last operation as expected", func() {
 			gomock.InOrder(
 				c.EXPECT().Status().Return(c),
-				c.EXPECT().Get(ctx, kutil.Key(obj.GetNamespace(), obj.GetName()), gomock.AssignableToTypeOf(&extensionsv1alpha1.Infrastructure{})),
-				c.EXPECT().Update(ctx, gomock.AssignableToTypeOf(&extensionsv1alpha1.Infrastructure{})).Do(func(ctx context.Context, obj extensionsv1alpha1.Object, opts ...client.UpdateOption) {
+				c.EXPECT().Patch(ctx, gomock.AssignableToTypeOf(&extensionsv1alpha1.Infrastructure{}), gomock.Any()).Do(func(ctx context.Context, obj extensionsv1alpha1.Object, patch client.Patch, opts ...client.PatchOption) {
 					var (
 						lastOperation      = obj.GetExtensionStatus().GetLastOperation()
 						lastError          = obj.GetExtensionStatus().GetLastError()

extensions/pkg/controller/utils.go

@@ -24,7 +24,6 @@ import (
 	v1beta1constants "github.com/gardener/gardener/pkg/apis/core/v1beta1/constants"
 	extensionsv1alpha1 "github.com/gardener/gardener/pkg/apis/extensions/v1alpha1"
 	resourcesv1alpha1 "github.com/gardener/gardener/pkg/apis/resources/v1alpha1"
-	"github.com/gardener/gardener/pkg/controllerutils"
 	kutil "github.com/gardener/gardener/pkg/utils/kubernetes"
 
 	"github.com/Masterminds/semver"
@@ -34,7 +33,6 @@ import (
 	utilruntime "k8s.io/apimachinery/pkg/util/runtime"
 	autoscalingv1beta2 "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/apis/autoscaling.k8s.io/v1beta2"
 	"k8s.io/client-go/kubernetes/scheme"
-	"k8s.io/client-go/util/retry"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/controller"
 	"sigs.k8s.io/controller-runtime/pkg/manager"
@@ -85,14 +83,6 @@ func (a *AddToManagerBuilder) AddToManager(m manager.Manager) error {
 	return nil
 }
 
-// DeleteAllFinalizers removes all finalizers from the object and issues an  update.
-func DeleteAllFinalizers(ctx context.Context, client client.Client, obj client.Object) error {
-	return controllerutils.TryUpdate(ctx, retry.DefaultBackoff, client, obj, func() error {
-		obj.SetFinalizers(nil)
-		return nil
-	})
-}
-
 // GetSecretByReference returns the Secret object matching the given SecretReference.
 var GetSecretByReference = kutil.GetSecretByReference