Skip to content

Commit

Permalink
refactor: Use Patch instead of Update to set/remove finalizers
Browse files Browse the repository at this point in the history
  • Loading branch information
Baarsgaard committed Dec 27, 2024
1 parent 4633e22 commit b778a52
Show file tree
Hide file tree
Showing 4 changed files with 87 additions and 37 deletions.
54 changes: 54 additions & 0 deletions controllers/controller_shared.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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))
}
24 changes: 11 additions & 13 deletions controllers/grafanaalertrulegroup_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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")
}
}
}()

Expand Down
23 changes: 11 additions & 12 deletions controllers/grafanacontactpoint_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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")
}
}
}()

Expand Down
23 changes: 11 additions & 12 deletions controllers/notificationpolicy_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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")
}
}
}()

Expand Down

0 comments on commit b778a52

Please sign in to comment.