fix: Copy ClusterClass and Templates only if they do not already exist

pkg/controllers/namespacesync/copy.go

@@ -8,6 +8,7 @@ import (
 	"fmt"
 
 	corev1 "k8s.io/api/core/v1"
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
 	"sigs.k8s.io/controller-runtime/pkg/client"
@@ -29,17 +30,10 @@ func copyClusterClassAndTemplates(
 			return fmt.Errorf("failed to get reference: %w", err)
 		}
 
-		// Copy Template to target namespace
+		// Copy Template to target namespace, if it does not exist there.
 		targetTemplate := copyObjectForCreate(sourceTemplate, sourceTemplate.GetName(), namespace)
-
-		//nolint:gocritic // Inline error is checked.
-		if err := w.Create(ctx, targetTemplate); client.IgnoreAlreadyExists(err) != nil {
-			return fmt.Errorf(
-				"failed to create %s %s: %w",
-				targetTemplate.GetKind(),
-				client.ObjectKeyFromObject(targetTemplate),
-				err,
-			)
+		if err = createIfNotExists(ctx, templateReader, w, targetTemplate); err != nil {
+			return fmt.Errorf("failed to create template: %w", err)
 		}
 
 		// Update reference to point to newly created Template
@@ -51,14 +45,32 @@ func copyClusterClassAndTemplates(
 		return fmt.Errorf("error processing references: %w", err)
 	}
 
-	//nolint:gocritic // Inline error is checked.
-	if err := w.Create(ctx, target); client.IgnoreAlreadyExists(err) != nil {
-		return fmt.Errorf(
-			"failed to create %s %s: %w",
-			target.Kind,
-			client.ObjectKeyFromObject(target),
-			err,
-		)
+	// Copy ClusterClass to target namespace, if it does not exist there.
+	if err := createIfNotExists(ctx, templateReader, w, target); err != nil {
+		return fmt.Errorf("failed to create cluster class: %w", err)
+	}
+	return nil
+}
+
+func createIfNotExists(ctx context.Context, r client.Reader, w client.Writer, obj client.Object) error {
+	key := client.ObjectKeyFromObject(obj)
+	gvk := obj.GetObjectKind().GroupVersionKind().String()
+
+	// Check if the resource exists.
+	// We do not need the object itself, just the metadata, so we use PartialObjectMetadata.
+	partial := &metav1.PartialObjectMetadata{}
+	partial.SetGroupVersionKind(obj.GetObjectKind().GroupVersionKind())
+	err := r.Get(ctx, key, partial)
+
+	if apierrors.IsNotFound(err) {
+		// The resource does not exist, so create it.
+		if err := w.Create(ctx, obj); err != nil {
+			return fmt.Errorf("failed to create %s %s: %w", gvk, key, err)
+		}
+		return nil
+	}
+	if err != nil {
+		return fmt.Errorf("failed to check if %s %s exists: %w", gvk, key, err)
 	}
 	return nil
 }

pkg/controllers/namespacesync/copy_test.go

@@ -15,6 +15,7 @@ import (
 	clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/client/fake"
+	"sigs.k8s.io/controller-runtime/pkg/client/interceptor"
 
 	"github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/internal/test/builder"
 )
@@ -24,53 +25,50 @@ const (
 	targetNamespace = "target-ns"
 )
 
-var errFakeCreate = errors.New("fake create error")
-
-type mockWriter struct {
-	client.Writer
-	createErrOnKind string
-	createdObjects  []client.Object
-}
-
-func (m *mockWriter) Create(
-	ctx context.Context,
-	obj client.Object,
-	opts ...client.CreateOption,
-) error {
-	if m.createErrOnKind != "" && obj.GetObjectKind().GroupVersionKind().Kind == m.createErrOnKind {
-		return errFakeCreate
-	}
-	m.createdObjects = append(m.createdObjects, obj)
-	// Fake setting of UID to simulate a real API server create.
-	obj.SetUID("fake-uid")
-	return nil
-}
+var (
+	errFakeCreate = errors.New("fake create error")
+	errFakeGet    = errors.New("fake get error")
+)
 
 func TestCopyClusterClassAndTemplates(t *testing.T) {
 	g := NewWithT(t)
 	ctx := context.Background()
 
 	testCases := []struct {
-		name            string
-		createErrOnKind string
-		expectErr       error
-		expectNumCopies int
+		name                string
+		interceptorFuncs    interceptor.Funcs
+		kindThatFailsCreate string
+		kindThatFailsGet    string
+		expectErr           error
+		expectNumCopies     int
 	}{
 		{
 			name:            "should succeed if all objects are created",
 			expectNumCopies: 6, // 1 ClusterClass + 5 templates
 		},
 		{
-			name:            "should fail if creating a template fails",
-			createErrOnKind: "GenericInfrastructureClusterTemplate",
-			expectErr:       errFakeCreate,
-			expectNumCopies: 0, // The first template create will fail.
+			name:                "should fail if creating a template fails",
+			kindThatFailsCreate: "GenericInfrastructureClusterTemplate",
+			expectErr:           errFakeCreate,
+			expectNumCopies:     0, // The first template create will fail.
+		},
+		{
+			name:                "should fail if creating the clusterclass fails",
+			kindThatFailsCreate: "ClusterClass",
+			expectErr:           errFakeCreate,
+			expectNumCopies:     5, // All 5 templates are created before ClusterClass.
+		},
+		{
+			name:             "should fail if getting a template fails",
+			kindThatFailsGet: "GenericInfrastructureClusterTemplate",
+			expectErr:        errFakeGet,
+			expectNumCopies:  0, // The first template get will fail.
 		},
 		{
-			name:            "should fail if creating the clusterclass fails",
-			createErrOnKind: "ClusterClass",
-			expectErr:       errFakeCreate,
-			expectNumCopies: 5, // All 5 templates are created before ClusterClass.
+			name:             "should fail if getting the clusterclass fails",
+			kindThatFailsGet: "ClusterClass",
+			expectErr:        errFakeGet,
+			expectNumCopies:  5, // All 5 templates are created before ClusterClass.
 		},
 	}
 
@@ -85,15 +83,23 @@ func TestCopyClusterClassAndTemplates(t *testing.T) {
 				initObjs = append(initObjs, template)
 			}
 
-			fakeReader := fake.NewClientBuilder().WithRuntimeObjects(initObjs...).Build()
-			mockWriter := &mockWriter{
-				createErrOnKind: tc.createErrOnKind,
-			}
+			createdObjs := []client.Object{}
+			fakeClient := fake.
+				NewClientBuilder().
+				WithInterceptorFuncs(
+					interceptors(
+						&createdObjs,
+						tc.kindThatFailsGet,
+						tc.kindThatFailsCreate,
+					),
+				).
+				WithRuntimeObjects(initObjs...).
+				Build()
 
 			err := copyClusterClassAndTemplates(
 				ctx,
-				mockWriter,
-				fakeReader,
+				fakeClient,
+				fakeClient,
 				sourceClusterClass,
 				targetNamespace,
 			)
@@ -105,18 +111,39 @@ func TestCopyClusterClassAndTemplates(t *testing.T) {
 				g.Expect(err).ToNot(HaveOccurred())
 			}
 
-			g.Expect(len(mockWriter.createdObjects)).To(Equal(tc.expectNumCopies))
+			g.Expect(len(createdObjs)).To(Equal(tc.expectNumCopies))
 
-			for _, obj := range mockWriter.createdObjects {
+			for _, obj := range createdObjs {
 				g.Expect(obj.GetNamespace()).To(Equal(targetNamespace))
 				g.Expect(obj.GetOwnerReferences()).To(BeEmpty())
 				g.Expect(obj.GetUID()).ToNot(BeEmpty())
-				g.Expect(obj.GetResourceVersion()).To(BeEmpty())
 			}
 		})
 	}
 }
 
+func interceptors(createdObjs *[]client.Object, kindThatFailsGet, kindThatFailsCreate string) interceptor.Funcs {
+	return interceptor.Funcs{
+		Get: func(ctx context.Context, c client.WithWatch, key client.ObjectKey, obj client.Object, opts ...client.GetOption) error {
+			if kindThatFailsGet != "" && obj.GetObjectKind().GroupVersionKind().Kind == kindThatFailsGet {
+				return errFakeGet
+			}
+			return c.Get(ctx, key, obj, opts...)
+		},
+		Create: func(ctx context.Context, c client.WithWatch, obj client.Object, opts ...client.CreateOption) error {
+			if kindThatFailsCreate != "" && obj.GetObjectKind().GroupVersionKind().Kind == kindThatFailsCreate {
+				return errFakeCreate
+			}
+
+			// Fake setting of UID to simulate a real API server create.
+			obj.SetUID("fake-uid")
+
+			*createdObjs = append(*createdObjs, obj)
+			return c.Create(ctx, obj, opts...)
+		},
+	}
+}
+
 // newTestClusterClassAndTemplates is a helper to generate a valid ClusterClass with all its referenced templates.
 func newTestClusterClassAndTemplates(
 	namespace,