add finaliser to GPUCluster CR using PATCH

Signed-off-by: Tariq Ibrahim <tibrahim@nvidia.com>
Co-authored-by: Rajath Agasthya <ragasthya@nvidia.com>

Author: tariq1890

controllers/gpucluster_controller.go

@@ -45,6 +45,7 @@ import (
 	"github.com/NVIDIA/gpu-operator/internal/conditions"
 	"github.com/NVIDIA/gpu-operator/internal/consts"
 	"github.com/NVIDIA/gpu-operator/internal/state"
+	"github.com/NVIDIA/gpu-operator/internal/utils"
 )
 
 // gpuClusterFinalizer holds the GPUCluster until reconcileDelete has ordered teardown.
@@ -97,11 +98,9 @@ func (r *GPUClusterReconciler) Reconcile(ctx context.Context, req ctrl.Request)
 	if !instance.DeletionTimestamp.IsZero() {
 		return r.reconcileDelete(ctx, instance)
 	}
-	if !controllerutil.ContainsFinalizer(instance, gpuClusterFinalizer) {
-		controllerutil.AddFinalizer(instance, gpuClusterFinalizer)
-		if err := r.Update(ctx, instance); err != nil {
-			return ctrl.Result{}, fmt.Errorf("error adding finalizer: %w", err)
-		}
+
+	if err := utils.EnsureFinalizer(ctx, r.Client, instance, gpuClusterFinalizer); err != nil {
+		return ctrl.Result{}, fmt.Errorf("error adding finalizer to GPUCluster %s: %w", req.NamespacedName, err)
 	}
 
 	// GPUCluster (DRA stack) may coexist with a ClusterPolicy (device-plugin

internal/utils/utils.go

@@ -17,6 +17,7 @@
 package utils
 
 import (
+	"context"
 	"fmt"
 	"hash"
 	"hash/fnv"
@@ -29,6 +30,8 @@ import (
 
 	"github.com/davecgh/go-spew/spew"
 	"k8s.io/apimachinery/pkg/util/rand"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
 )
 
 // GetFilesWithSuffix returns all files under a given base directory that have a specific suffix
@@ -163,3 +166,15 @@ func WriteFileAtomically(path, content string) error {
 	}
 	return nil
 }
+
+// EnsureFinalizer adds a finalizer to an object that has not been marked for deletion.
+// It is idempotent and returns an error only if an attempt to add the finalizer has failed.
+func EnsureFinalizer(ctx context.Context, c client.Client, o client.Object, finalizer string) error {
+	if !o.GetDeletionTimestamp().IsZero() || controllerutil.ContainsFinalizer(o, finalizer) {
+		return nil
+	}
+
+	original := o.DeepCopyObject().(client.Object)
+	controllerutil.AddFinalizer(o, finalizer)
+	return c.Patch(ctx, o, client.MergeFromWithOptions(original, client.MergeFromWithOptimisticLock{}))
+}

internal/utils/utils_test.go

@@ -17,10 +17,17 @@
 package utils
 
 import (
+	"context"
 	"reflect"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
+	corev1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"sigs.k8s.io/controller-runtime/pkg/client/fake"
+	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
+
+	"k8s.io/client-go/kubernetes/scheme"
 )
 
 func TestGetObjectHash(t *testing.T) {
@@ -190,3 +197,51 @@ func TestGetStringHash(t *testing.T) {
 		assert.Equal(t, tc.expected, actual)
 	}
 }
+
+func TestEnsureFinalizer(t *testing.T) {
+	const testFinalizer = "test.io/finalizer"
+
+	s := scheme.Scheme
+
+	newObj := func(name string, finalizers ...string) *corev1.ConfigMap {
+		return &corev1.ConfigMap{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:            name,
+				Namespace:       "default",
+				ResourceVersion: "1",
+				Finalizers:      finalizers,
+			},
+		}
+	}
+
+	t.Run("adds finalizer when not present", func(t *testing.T) {
+		obj := newObj("obj")
+		c := fake.NewClientBuilder().WithScheme(s).WithObjects(obj).Build()
+
+		err := EnsureFinalizer(context.Background(), c, obj, testFinalizer)
+		assert.NoError(t, err)
+		assert.True(t, controllerutil.ContainsFinalizer(obj, testFinalizer))
+	})
+
+	t.Run("no-op when finalizer already present", func(t *testing.T) {
+		obj := newObj("obj", testFinalizer)
+		// object not registered in client — Patch would fail if reached
+		c := fake.NewClientBuilder().WithScheme(s).Build()
+
+		err := EnsureFinalizer(context.Background(), c, obj, testFinalizer)
+		assert.NoError(t, err)
+		assert.True(t, controllerutil.ContainsFinalizer(obj, testFinalizer))
+	})
+
+	t.Run("no-op when object is being deleted", func(t *testing.T) {
+		now := metav1.Now()
+		obj := newObj("obj")
+		obj.DeletionTimestamp = &now
+		// object not registered in client — Patch would fail if reached
+		c := fake.NewClientBuilder().WithScheme(s).Build()
+
+		err := EnsureFinalizer(context.Background(), c, obj, testFinalizer)
+		assert.NoError(t, err)
+		assert.False(t, controllerutil.ContainsFinalizer(obj, testFinalizer))
+	})
+}