remove Patch calls

Using the controller runtime Patch API with controller runtime cached client seems to be a pretty bad fit At least given the way the CR API is designed where an old object is compared to new. I like patch in theory though and will revisit Signed-off-by: Michael Henriksen <mhenriks@redhat.com>
mhenriks · Jun 26, 2023 · 2e75b5a · 2e75b5a
1 parent d173e08
commit 2e75b5a
Show file tree

Hide file tree

Showing 2 changed files with 4 additions and 25 deletions.
diff --git a/pkg/controller/common/util.go b/pkg/controller/common/util.go
@@ -1833,17 +1833,6 @@ type PatchArgs struct {
 	OldObj client.Object
 }
 
-// MergePatch patches a resource
-func MergePatch(ctx context.Context, args *PatchArgs) error {
-	patch := client.MergeFrom(args.OldObj)
-	bs, err := patch.Data(args.Obj)
-	if err != nil {
-		return err
-	}
-	args.Log.V(3).Info("Merge patch", "patch", string(bs))
-	return args.Client.Patch(ctx, args.Obj, patch)
-}
-
 // GetAnnotatedEventSource returns resource referenced by AnnEventSource annotations
 func GetAnnotatedEventSource(ctx context.Context, c client.Client, obj client.Object) (client.Object, error) {
 	esk, ok := obj.GetAnnotations()[AnnEventSourceKind]

diff --git a/pkg/controller/populators/clone-populator.go b/pkg/controller/populators/clone-populator.go
@@ -311,7 +311,7 @@ func (r *ClonePopulatorReconciler) reconcileDone(ctx context.Context, log logr.L
 	log.V(1).Info("removing finalizer")
 	claimCpy := pvc.DeepCopy()
 	cc.RemoveFinalizer(claimCpy, cloneFinalizer)
-	return reconcile.Result{}, r.patchClaim(ctx, log, pvc, claimCpy)
+	return reconcile.Result{}, r.client.Update(ctx, claimCpy)
 }
 
 func (r *ClonePopulatorReconciler) initTargetClaim(ctx context.Context, log logr.Logger, pvc *corev1.PersistentVolumeClaim, vcs *cdiv1.VolumeCloneSource, cs cdiv1.CDICloneStrategy) (bool, error) {
@@ -324,7 +324,7 @@ func (r *ClonePopulatorReconciler) initTargetClaim(ctx context.Context, log logr
 	cc.AddFinalizer(claimCpy, cloneFinalizer)
 
 	if !apiequality.Semantic.DeepEqual(pvc, claimCpy) {
-		if err := r.patchClaim(ctx, log, pvc, claimCpy); err != nil {
+		if err := r.client.Update(ctx, claimCpy); err != nil {
 			return false, err
 		}
 
@@ -367,7 +367,7 @@ func (r *ClonePopulatorReconciler) updateClonePhase(ctx context.Context, log log
 	r.addRunningAnnotations(claimCpy, phase, mergedAnnotations)
 
 	if !apiequality.Semantic.DeepEqual(pvc, claimCpy) {
-		return r.patchClaim(ctx, log, pvc, claimCpy)
+		return r.client.Update(ctx, claimCpy)
 	}
 
 	return nil
@@ -381,7 +381,7 @@ func (r *ClonePopulatorReconciler) updateClonePhaseError(ctx context.Context, lo
 	r.addRunningAnnotations(claimCpy, clone.ErrorPhaseName, nil)
 
 	if !apiequality.Semantic.DeepEqual(pvc, claimCpy) {
-		if err := r.patchClaim(ctx, log, pvc, claimCpy); err != nil {
+		if err := r.client.Update(ctx, claimCpy); err != nil {
 			log.V(1).Info("error setting error annotations")
 		}
 	}
@@ -424,16 +424,6 @@ func (r *ClonePopulatorReconciler) addRunningAnnotations(pvc *corev1.PersistentV
 	cc.AddAnnotation(pvc, cc.AnnRunningConditionReason, reason)
 }
 
-func (r *ClonePopulatorReconciler) patchClaim(ctx context.Context, log logr.Logger, oldObj, obj *corev1.PersistentVolumeClaim) error {
-	args := &cc.PatchArgs{
-		Client: r.client,
-		Log:    log,
-		OldObj: oldObj,
-		Obj:    obj,
-	}
-	return cc.MergePatch(ctx, args)
-}
-
 func (r *ClonePopulatorReconciler) getVolumeCloneSource(ctx context.Context, log logr.Logger, pvc *corev1.PersistentVolumeClaim) (*cdiv1.VolumeCloneSource, error) {
 	if !IsPVCDataSourceRefKind(pvc, cdiv1.VolumeCloneSourceRef) {
 		return nil, fmt.Errorf("pvc %s/%s does not refer to a %s", pvc.Namespace, pvc.Name, cdiv1.VolumeCloneSourceRef)