fix: Namespacesync controller should reconcile an updated namespace (#775)

**What problem does this PR solve?**:
The namespacesync controller did not reconcile a namespace when it was
updated. We anticipated namespaces to meet the criteria for "target
namespace" when they were created. We also wanted to avoid reconciling
namespaces unnecessarily.

This PR makes the controller reconcile an updated namespace, if the
answer to question "is this a target namespace," has changed. It also
adds an integration test (which fails without the subsequent fix
commit).

**Which issue(s) this PR fixes**:
Fixes #

**How Has This Been Tested?**:
<!--
Please describe the tests that you ran to verify your changes.
Provide output from the tests and any manual steps needed to replicate
the tests.
-->

**Special notes for your reviewer**:
<!--
Use this to provide any additional information to the reviewers.
This may include:
- Best way to review the PR.
- Where the author wants the most review attention on.
- etc.
-->

Author: dlipovetsky

cmd/main.go

@@ -173,7 +173,7 @@ func main() {
 			Client:                      mgr.GetClient(),
 			UnstructuredCachingClient:   unstructuredCachingClient,
 			SourceClusterClassNamespace: namespacesyncOptions.SourceNamespace,
-			TargetNamespaceFilter:       namespacesync.NamespaceHasLabelKey(namespacesyncOptions.TargetNamespaceLabelKey),
+			IsTargetNamespace:           namespacesync.NamespaceHasLabelKey(namespacesyncOptions.TargetNamespaceLabelKey),
 		}).SetupWithManager(
 			signalCtx,
 			mgr,

pkg/controllers/namespacesync/controller.go

@@ -28,17 +28,17 @@ type Reconciler struct {
 	// SourceClusterClassNamespace is the namespace from which ClusterClasses are copied.
 	SourceClusterClassNamespace string
 
-	// TargetNamespaceFilter determines whether ClusterClasses should be copied to a given namespace.
-	TargetNamespaceFilter func(ns *corev1.Namespace) bool
+	// IsTargetNamespace determines whether ClusterClasses should be copied to a given namespace.
+	IsTargetNamespace func(ns *corev1.Namespace) bool
 }
 
 func (r *Reconciler) SetupWithManager(
 	ctx context.Context,
 	mgr ctrl.Manager,
 	options controller.Options,
 ) error {
-	if r.TargetNamespaceFilter == nil {
-		return fmt.Errorf("target Namespace filter is nil")
+	if r.IsTargetNamespace == nil {
+		return fmt.Errorf("define IsTargetNamespace function to use controller")
 	}
 
 	err := ctrl.NewControllerManagedBy(mgr).
@@ -52,12 +52,22 @@ func (r *Reconciler) SetupWithManager(
 						if !ok {
 							return false
 						}
-						return r.TargetNamespaceFilter(ns)
+						return r.IsTargetNamespace(ns)
 					},
 					UpdateFunc: func(e event.UpdateEvent) bool {
 						// Called when an object is already in the cache, and it is either updated,
 						// or fetched as part of a re-list (aka re-sync).
-						return false
+						nsOld, ok := e.ObjectOld.(*corev1.Namespace)
+						if !ok {
+							return false
+						}
+						nsNew, ok := e.ObjectNew.(*corev1.Namespace)
+						if !ok {
+							return false
+						}
+						// Only reconcile the namespace if the answer to the question "Is this a
+						// target namespace?" changed from no to yes.
+						return !r.IsTargetNamespace(nsOld) && r.IsTargetNamespace(nsNew)
 					},
 					DeleteFunc: func(e event.DeleteEvent) bool {
 						// Ignore deletes.
@@ -93,7 +103,7 @@ func (r *Reconciler) clusterClassToNamespaces(ctx context.Context, o client.Obje
 	rs := []ctrl.Request{}
 	for i := range namespaceList.Items {
 		ns := &namespaceList.Items[i]
-		if r.TargetNamespaceFilter(ns) {
+		if r.IsTargetNamespace(ns) {
 			rs = append(rs,
 				ctrl.Request{
 					NamespacedName: client.ObjectKeyFromObject(ns),

pkg/controllers/namespacesync/controller_test.go

@@ -17,6 +17,38 @@ import (
 	"github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/internal/test/builder"
 )
 
+func TestReconcileExistingNamespaceWithUpdatedLabels(t *testing.T) {
+	g := NewWithT(t)
+	timeout := 5 * time.Second
+
+	sourceClusterClassName, cleanup, err := createUniqueClusterClassAndTemplates(
+		sourceClusterClassNamespace,
+	)
+	g.Expect(err).ToNot(HaveOccurred())
+	defer func() {
+		g.Expect(cleanup()).To(Succeed())
+	}()
+
+	// Create namespace without label
+	targetNamespace, err := env.CreateNamespace(ctx, "target", map[string]string{})
+	g.Expect(err).ToNot(HaveOccurred())
+
+	// Label the namespace
+	targetNamespace.Labels[targetNamespaceLabelKey] = ""
+	err = env.Update(ctx, targetNamespace)
+	g.Expect(err).ToNot(HaveOccurred())
+
+	g.Eventually(func() error {
+		return verifyClusterClassAndTemplates(
+			env.Client,
+			sourceClusterClassName,
+			targetNamespace.Name,
+		)
+	},
+		timeout,
+	).Should(Succeed())
+}
+
 func TestReconcileNewNamespaces(t *testing.T) {
 	g := NewWithT(t)
 	timeout := 5 * time.Second

pkg/controllers/namespacesync/suite_test.go

@@ -58,7 +58,7 @@ func TestMain(m *testing.M) {
 			Client:                      mgr.GetClient(),
 			UnstructuredCachingClient:   unstructuredCachingClient,
 			SourceClusterClassNamespace: sourceClusterClassNamespace,
-			TargetNamespaceFilter:       NamespaceHasLabelKey(targetNamespaceLabelKey),
+			IsTargetNamespace:           NamespaceHasLabelKey(targetNamespaceLabelKey),
 		}).SetupWithManager(ctx, mgr, controller.Options{MaxConcurrentReconciles: 1}); err != nil {
 			panic(fmt.Sprintf("unable to create reconciler: %v", err))
 		}