From b778a52b29ca9f54edf8245bc57900b3d6ca8dd8 Mon Sep 17 00:00:00 2001 From: ste Date: Wed, 18 Dec 2024 12:36:02 +0100 Subject: [PATCH] refactor: Use Patch instead of Update to set/remove finalizers --- controllers/controller_shared.go | 54 +++++++++++++++++++ .../grafanaalertrulegroup_controller.go | 24 ++++----- controllers/grafanacontactpoint_controller.go | 23 ++++---- controllers/notificationpolicy_controller.go | 23 ++++---- 4 files changed, 87 insertions(+), 37 deletions(-) diff --git a/controllers/controller_shared.go b/controllers/controller_shared.go index 8c112737d..878af6572 100644 --- a/controllers/controller_shared.go +++ b/controllers/controller_shared.go @@ -19,6 +19,7 @@ import ( "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/event" "sigs.k8s.io/controller-runtime/pkg/predicate" @@ -366,3 +367,56 @@ func getReferencedValue(ctx context.Context, cl client.Client, cr metav1.ObjectM } } } + +// Add finalizer through a MergePatch +// Avoids updating the entire object and only changes the finalizers +func addFinalizer(ctx context.Context, cl client.Client, cr client.Object) error { + crFinalizers := cr.GetFinalizers() + + // Already exists, skip the update + if slices.Contains(crFinalizers, grafanaFinalizer) { + return nil + } + + // Append finalizer and create patch + crFinalizers = append(crFinalizers, grafanaFinalizer) + patch, err := json.Marshal(map[string]interface{}{"metadata": map[string]interface{}{"finalizers": crFinalizers}}) + if err != nil { + return err + } + return cl.Patch(ctx, cr, client.RawPatch(types.MergePatchType, patch)) +} + +// Remove finalizer through a MergePatch +// Avoids updating the entire object and only changes the finalizers +func removeFinalizer(ctx context.Context, cl client.Client, cr client.Object) error { + crFinalizers := cr.GetFinalizers() + length := len(crFinalizers) + + // Skip patching if finalizers is empty + if length == 0 { + return nil + } + + // Shift items in array if it contains and is not equal to grafanaFinalizer + index := 0 + for i := 0; i < length; i++ { + if crFinalizers[i] == grafanaFinalizer { + continue + } + crFinalizers[index] = crFinalizers[i] + index++ + } + + // Skip patching if finalizer length is unchanged + if index == length { + return nil + } + + // Create patch using slice + patch, err := json.Marshal(map[string]interface{}{"metadata": map[string]interface{}{"finalizers": crFinalizers[:index]}}) + if err != nil { + return err + } + return cl.Patch(ctx, cr, client.RawPatch(types.MergePatchType, patch)) +} diff --git a/controllers/grafanaalertrulegroup_controller.go b/controllers/grafanaalertrulegroup_controller.go index 72a39a074..ab730b489 100644 --- a/controllers/grafanaalertrulegroup_controller.go +++ b/controllers/grafanaalertrulegroup_controller.go @@ -76,16 +76,13 @@ func (r *GrafanaAlertRuleGroupReconciler) Reconcile(ctx context.Context, req ctr } if group.GetDeletionTimestamp() != nil { + // Check if resource needs clean up if controllerutil.ContainsFinalizer(group, grafanaFinalizer) { - // still need to clean up - err := r.finalize(ctx, group) - if err != nil { - return ctrl.Result{}, fmt.Errorf("cleaning up alert rule group: %w", err) + if err := r.finalize(ctx, group); err != nil { + return ctrl.Result{}, fmt.Errorf("failed to finalize GrafanaAlertRuleGroup: %w", err) } - controllerutil.RemoveFinalizer(group, grafanaFinalizer) - if err := r.Update(ctx, group); err != nil { - r.Log.Error(err, "failed to remove finalizer") - return ctrl.Result{}, err + if err := removeFinalizer(ctx, r.Client, group); err != nil { + return ctrl.Result{}, fmt.Errorf("failed to remove finalizer: %w", err) } } return ctrl.Result{}, nil @@ -96,12 +93,13 @@ func (r *GrafanaAlertRuleGroupReconciler) Reconcile(ctx context.Context, req ctr r.Log.Error(err, "updating status") } if meta.IsStatusConditionTrue(group.Status.Conditions, conditionNoMatchingInstance) { - controllerutil.RemoveFinalizer(group, grafanaFinalizer) + if err := removeFinalizer(ctx, r.Client, group); err != nil { + r.Log.Error(err, "failed to remove finalizer") + } } else { - controllerutil.AddFinalizer(group, grafanaFinalizer) - } - if err := r.Update(ctx, group); err != nil { - r.Log.Error(err, "failed to set finalizer") + if err := addFinalizer(ctx, r.Client, group); err != nil { + r.Log.Error(err, "failed to set finalizer") + } } }() diff --git a/controllers/grafanacontactpoint_controller.go b/controllers/grafanacontactpoint_controller.go index a62456824..1340b447b 100644 --- a/controllers/grafanacontactpoint_controller.go +++ b/controllers/grafanacontactpoint_controller.go @@ -82,15 +82,13 @@ func (r *GrafanaContactPointReconciler) Reconcile(ctx context.Context, req ctrl. } if contactPoint.GetDeletionTimestamp() != nil { + // Check if resource needs clean up if controllerutil.ContainsFinalizer(contactPoint, grafanaFinalizer) { - err := r.finalize(ctx, contactPoint) - if err != nil { - return ctrl.Result{RequeueAfter: RequeueDelay}, fmt.Errorf("failed to finalize GrafanaContactPoint: %w", err) + if err := r.finalize(ctx, contactPoint); err != nil { + return ctrl.Result{}, fmt.Errorf("failed to finalize GrafanaContactPoint: %w", err) } - controllerutil.RemoveFinalizer(contactPoint, grafanaFinalizer) - if err := r.Update(ctx, contactPoint); err != nil { - r.Log.Error(err, "failed to remove finalizer") - return ctrl.Result{RequeueAfter: RequeueDelay}, fmt.Errorf("failed to update GrafanaContactPoint: %w", err) + if err := removeFinalizer(ctx, r.Client, contactPoint); err != nil { + return ctrl.Result{}, fmt.Errorf("failed to remove finalizer: %w", err) } } return ctrl.Result{}, nil @@ -101,12 +99,13 @@ func (r *GrafanaContactPointReconciler) Reconcile(ctx context.Context, req ctrl. r.Log.Error(err, "updating status") } if meta.IsStatusConditionTrue(contactPoint.Status.Conditions, conditionNoMatchingInstance) { - controllerutil.RemoveFinalizer(contactPoint, grafanaFinalizer) + if err := removeFinalizer(ctx, r.Client, contactPoint); err != nil { + r.Log.Error(err, "failed to remove finalizer") + } } else { - controllerutil.AddFinalizer(contactPoint, grafanaFinalizer) - } - if err := r.Update(ctx, contactPoint); err != nil { - r.Log.Error(err, "failed to set finalizer") + if err := addFinalizer(ctx, r.Client, contactPoint); err != nil { + r.Log.Error(err, "failed to set finalizer") + } } }() diff --git a/controllers/notificationpolicy_controller.go b/controllers/notificationpolicy_controller.go index 7fa34cf21..0f8188691 100644 --- a/controllers/notificationpolicy_controller.go +++ b/controllers/notificationpolicy_controller.go @@ -80,15 +80,13 @@ func (r *GrafanaNotificationPolicyReconciler) Reconcile(ctx context.Context, req } if notificationPolicy.GetDeletionTimestamp() != nil { + // Check if resource needs clean up if controllerutil.ContainsFinalizer(notificationPolicy, grafanaFinalizer) { - err := r.finalize(ctx, notificationPolicy) - if err != nil { - return ctrl.Result{RequeueAfter: RequeueDelay}, fmt.Errorf("failed to finalize GrafanaNotificationPolicy: %w", err) + if err := r.finalize(ctx, notificationPolicy); err != nil { + return ctrl.Result{}, fmt.Errorf("failed to finalize GrafanaNotificationPolicy: %w", err) } - controllerutil.RemoveFinalizer(notificationPolicy, grafanaFinalizer) - if err := r.Update(ctx, notificationPolicy); err != nil { - r.Log.Error(err, "failed to remove finalizer") - return ctrl.Result{RequeueAfter: RequeueDelay}, fmt.Errorf("failed to update GrafanaNotificationPolicy: %w", err) + if err := removeFinalizer(ctx, r.Client, notificationPolicy); err != nil { + return ctrl.Result{}, fmt.Errorf("failed to remove finalizer: %w", err) } } return ctrl.Result{}, nil @@ -99,12 +97,13 @@ func (r *GrafanaNotificationPolicyReconciler) Reconcile(ctx context.Context, req r.Log.Error(err, "updating status") } if meta.IsStatusConditionTrue(notificationPolicy.Status.Conditions, conditionNoMatchingInstance) { - controllerutil.RemoveFinalizer(notificationPolicy, grafanaFinalizer) + if err := removeFinalizer(ctx, r.Client, notificationPolicy); err != nil { + r.Log.Error(err, "failed to remove finalizer") + } } else { - controllerutil.AddFinalizer(notificationPolicy, grafanaFinalizer) - } - if err := r.Update(ctx, notificationPolicy); err != nil { - r.Log.Error(err, "failed to set finalizer") + if err := addFinalizer(ctx, r.Client, notificationPolicy); err != nil { + r.Log.Error(err, "failed to set finalizer") + } } }()