From 2e75b5af2ab534be7009a821f38c02d5ab87540b Mon Sep 17 00:00:00 2001 From: Michael Henriksen Date: Sun, 25 Jun 2023 20:27:20 -0400 Subject: [PATCH] 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 --- pkg/controller/common/util.go | 11 ----------- pkg/controller/populators/clone-populator.go | 18 ++++-------------- 2 files changed, 4 insertions(+), 25 deletions(-) diff --git a/pkg/controller/common/util.go b/pkg/controller/common/util.go index ffbef3d13b..5f89b969a6 100644 --- 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 index 4d86e27c48..3245389bf5 100644 --- 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)