fix compute domain controller deletion

Author: enoodle

internal/compute-domain-controller/computedomain_controller.go

@@ -38,6 +38,16 @@ const (
 	DefaultComputeDomainAllocationMode = "Single"
 )
 
+// isOwnedBy checks if the ResourceClaimTemplate is owned by the given ComputeDomain
+func isOwnedBy(template *resourceapi.ResourceClaimTemplate, domain *computedomainv1beta1.ComputeDomain) bool {
+	for _, owner := range template.OwnerReferences {
+		if owner.Name == domain.Name && owner.Kind == "ComputeDomain" {
+			return true
+		}
+	}
+	return false
+}
+
 // ComputeDomainReconciler watches ComputeDomain resources and keeps the
 // associated ResourceClaimTemplates in sync.
 type ComputeDomainReconciler struct {
@@ -135,7 +145,7 @@ func (r *ComputeDomainReconciler) ensureTemplate(
 				"resource.nvidia.com/computeDomainTarget": templateType,
 			},
 			Finalizers: []string{
-				"resource.nvidia.com/computeDomain",
+				consts.ComputeDomainFinalizer,
 			},
 		},
 		Spec: resourceapi.ResourceClaimTemplateSpec{
@@ -186,12 +196,28 @@ func (r *ComputeDomainReconciler) ensureTemplate(
 }
 
 func (r *ComputeDomainReconciler) deleteResourceClaimTemplates(ctx context.Context, domain *computedomainv1beta1.ComputeDomain) error {
-	template := &resourceapi.ResourceClaimTemplate{
-		ObjectMeta: metav1.ObjectMeta{
-			Name:      domain.Name,
-			Namespace: domain.Namespace,
-		},
+	template := &resourceapi.ResourceClaimTemplate{}
+	key := client.ObjectKey{Namespace: domain.Namespace, Name: domain.Name}
+	if err := r.Get(ctx, key, template); err != nil {
+		if apierrors.IsNotFound(err) {
+			return nil
+		}
+		return err
+	}
+
+	if !isOwnedBy(template, domain) {
+		return nil
 	}
+
+	// Remove our finalizer
+	if controllerutil.ContainsFinalizer(template, consts.ComputeDomainFinalizer) {
+		controllerutil.RemoveFinalizer(template, consts.ComputeDomainFinalizer)
+		if err := r.Update(ctx, template); err != nil {
+			return err
+		}
+	}
+
+	// Now delete the template
 	if err := r.Delete(ctx, template); err != nil && !apierrors.IsNotFound(err) {
 		return err
 	}

internal/compute-domain-controller/computedomain_controller_test.go

@@ -19,6 +19,7 @@ package computedomaincontroller_test
 import (
 	"context"
 	"testing"
+	"time"
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
@@ -44,6 +45,7 @@ func TestComputeDomainReconciler_Reconcile(t *testing.T) {
 
 	tests := map[string]struct {
 		computeDomain            *computedomainv1beta1.ComputeDomain
+		existingObjects          []client.Object
 		expectedWorkloadTemplate bool
 		expectedFinalizer        bool
 	}{
@@ -58,12 +60,46 @@ func TestComputeDomainReconciler_Reconcile(t *testing.T) {
 			expectedWorkloadTemplate: true,
 			expectedFinalizer:        true,
 		},
+		"deleted ComputeDomain removes templates": {
+			computeDomain: &computedomainv1beta1.ComputeDomain{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:              "test-domain",
+					Namespace:         "default",
+					UID:               "test-uid",
+					DeletionTimestamp: &metav1.Time{Time: time.Now()},
+					Finalizers:        []string{consts.ComputeDomainFinalizer},
+				},
+			},
+			existingObjects: []client.Object{
+				&resourceapi.ResourceClaimTemplate{
+					ObjectMeta: metav1.ObjectMeta{
+						Name:      "test-domain",
+						Namespace: "default",
+						Labels: map[string]string{
+							"resource.nvidia.com/computeDomain": "test-domain",
+						},
+						Finalizers: []string{consts.ComputeDomainFinalizer},
+						OwnerReferences: []metav1.OwnerReference{
+							{
+								Kind: "ComputeDomain",
+								Name: "test-domain",
+							},
+						},
+					},
+				},
+			},
+			expectedWorkloadTemplate: false,
+			expectedFinalizer:        false,
+		},
 	}
 
 	for name, test := range tests {
 		t.Run(name, func(t *testing.T) {
 			// Setup
 			objs := []client.Object{test.computeDomain}
+			if len(test.existingObjects) > 0 {
+				objs = append(objs, test.existingObjects...)
+			}
 
 			fakeClient := fake.NewClientBuilder().
 				WithScheme(scheme).
@@ -89,13 +125,13 @@ func TestComputeDomainReconciler_Reconcile(t *testing.T) {
 			require.NoError(t, err)
 			assert.Equal(t, ctrl.Result{}, result)
 
+			workloadTemplate := &resourceapi.ResourceClaimTemplate{}
+			err = fakeClient.Get(context.Background(), types.NamespacedName{
+				Name:      "test-domain",
+				Namespace: "default",
+			}, workloadTemplate)
 			// Check ResourceClaimTemplates
 			if test.expectedWorkloadTemplate {
-				workloadTemplate := &resourceapi.ResourceClaimTemplate{}
-				err := fakeClient.Get(context.Background(), types.NamespacedName{
-					Name:      "test-domain",
-					Namespace: "default",
-				}, workloadTemplate)
 				assert.NoError(t, err)
 				// Check config section
 				assert.Len(t, workloadTemplate.Spec.Spec.Devices.Config, 1)
@@ -112,7 +148,14 @@ func TestComputeDomainReconciler_Reconcile(t *testing.T) {
 				// Check labels copied into generated claims
 				assert.Equal(t, test.computeDomain.GetName(), workloadTemplate.Spec.Labels["nvidia.com/computeDomain"])
 				// Check finalizers
-				assert.Contains(t, workloadTemplate.Finalizers, "resource.nvidia.com/computeDomain")
+				assert.Contains(t, workloadTemplate.Finalizers, consts.ComputeDomainFinalizer)
+			} else {
+				assert.Error(t, err)
+				assert.NoError(t, client.IgnoreNotFound(err))
+			}
+
+			if !test.computeDomain.DeletionTimestamp.IsZero() {
+				return
 			}
 
 			// Check finalizer