Ensure new namespaces can trigger a reconcile

Previously, if the controller restarted, then policies with an
evaluationInterval would not always start watching the namespaces
specified in their namespaceSelectors. The intention was that new or
modified namespaces could trigger evaluations even outside of the
specified intervals, but this was not working.

Refs:
 - https://issues.redhat.com/browse/ACM-24595

Signed-off-by: Justin Kulikauskas <[email protected]>

Author: JustinKuli

pkg/common/namespace_selection.go

@@ -251,12 +251,17 @@ func (r *NamespaceSelectorReconciler) Get(objNS string, objName string, t policy
 }
 
 // HasUpdate indicates when the cached selection for this policy has been changed since the last
-// time that Get was called for that policy.
+// time that Get was called for that policy. Returns true if the selection isn't cached.
 func (r *NamespaceSelectorReconciler) HasUpdate(namespace string, name string) bool {
 	r.lock.RLock()
 	defer r.lock.RUnlock()
 
-	return r.selections[getKey(namespace, name)].hasUpdate
+	sel, ok := r.selections[getKey(namespace, name)]
+	if !ok {
+		return true
+	}
+
+	return sel.hasUpdate
 }
 
 // Stop tells the SelectorReconciler to stop updating the cached selection for the name.

test/e2e/case14_selection_test.go

@@ -5,6 +5,8 @@ package e2e
 
 import (
 	"context"
+	"strconv"
+	"time"
 
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
@@ -16,12 +18,12 @@ import (
 	"open-cluster-management.io/config-policy-controller/test/utils"
 )
 
-const (
-	case14LimitRangeFile string = "../resources/case14_namespaces/case14_limitrange.yaml"
-	case14LimitRangeName string = "container-mem-limit-range"
-)
-
 var _ = Describe("Test policy compliance with namespace selection", Ordered, func() {
+	const (
+		case14LimitRangeFile string = "../resources/case14_namespaces/case14_limitrange.yaml"
+		case14LimitRangeName string = "container-mem-limit-range"
+	)
+
 	checkRelated := func(policy *unstructured.Unstructured) []interface{} {
 		related, _, err := unstructured.NestedSlice(policy.Object, "status", "relatedObjects")
 		if err != nil {
@@ -50,18 +52,18 @@ var _ = Describe("Test policy compliance with namespace selection", Ordered, fun
 		},
 	}
 
-	BeforeAll(func() {
+	BeforeAll(func(ctx context.Context) {
 		By("Create Namespaces if needed")
 		namespaces := clientManaged.CoreV1().Namespaces()
 		for _, ns := range testNamespaces {
-			if _, err := namespaces.Get(context.TODO(), ns, metav1.GetOptions{}); err != nil && errors.IsNotFound(err) {
-				Expect(namespaces.Create(context.TODO(), &corev1.Namespace{
+			if _, err := namespaces.Get(ctx, ns, metav1.GetOptions{}); err != nil && errors.IsNotFound(err) {
+				Expect(namespaces.Create(ctx, &corev1.Namespace{
 					ObjectMeta: metav1.ObjectMeta{
 						Name: ns,
 					},
 				}, metav1.CreateOptions{})).NotTo(BeNil())
 			}
-			Expect(namespaces.Get(context.TODO(), ns, metav1.GetOptions{})).NotTo(BeNil())
+			Expect(namespaces.Get(ctx, ns, metav1.GetOptions{})).NotTo(BeNil())
 		}
 	})
 
@@ -135,15 +137,15 @@ var _ = Describe("Test policy compliance with namespace selection", Ordered, fun
 		}
 	})
 
-	It("should be noncompliant after adding new matching namespace", func() {
+	It("should be noncompliant after adding new matching namespace", func(ctx context.Context) {
 		By("Creating namespace " + newNs)
 		namespaces := clientManaged.CoreV1().Namespaces()
-		Expect(namespaces.Create(context.TODO(), &corev1.Namespace{
+		Expect(namespaces.Create(ctx, &corev1.Namespace{
 			ObjectMeta: metav1.ObjectMeta{
 				Name: newNs,
 			},
 		}, metav1.CreateOptions{})).NotTo(BeNil())
-		Expect(namespaces.Get(context.TODO(), newNs, metav1.GetOptions{})).NotTo(BeNil())
+		Expect(namespaces.Get(ctx, newNs, metav1.GetOptions{})).NotTo(BeNil())
 		for _, policy := range policyTests {
 			By("Checking that " + policy.name + " is NonCompliant")
 			Eventually(func(g Gomega) {
@@ -238,12 +240,12 @@ var _ = Describe("Test policy compliance with namespace selection", Ordered, fun
 		}
 	})
 
-	It("should update relatedObjects after deleting a namespace", func() {
+	It("should update relatedObjects after deleting a namespace", func(ctx context.Context) {
 		By("Deleting namespace " + newNs)
 		namespaces := clientManaged.CoreV1().Namespaces()
-		Expect(namespaces.Delete(context.TODO(), newNs, metav1.DeleteOptions{})).To(Succeed())
+		Expect(namespaces.Delete(ctx, newNs, metav1.DeleteOptions{})).To(Succeed())
 		Eventually(func() bool {
-			_, err := namespaces.Get(context.TODO(), newNs, metav1.GetOptions{})
+			_, err := namespaces.Get(ctx, newNs, metav1.GetOptions{})
 
 			return errors.IsNotFound(err)
 		}, defaultTimeoutSeconds, 1).Should(BeTrue())
@@ -268,3 +270,77 @@ var _ = Describe("Test policy compliance with namespace selection", Ordered, fun
 		}
 	})
 })
+
+var _ = Describe("Test NS selection events after controller restart", Label("running-in-cluster"), Ordered, func() {
+	const (
+		configmapName = "case14-cm"
+		policyYAML    = "../resources/case14_namespaces/case14_configmaps_policy.yaml"
+		policyName    = "case14-configmaps"
+	)
+
+	BeforeAll(func(ctx context.Context) {
+		By("Create initial Namespaces")
+		namespaces := clientManaged.CoreV1().Namespaces()
+		for i := range 2 {
+			ns := "case14-" + strconv.Itoa(i)
+			if _, err := namespaces.Get(ctx, ns, metav1.GetOptions{}); err != nil && errors.IsNotFound(err) {
+				Expect(namespaces.Create(ctx, &corev1.Namespace{
+					ObjectMeta: metav1.ObjectMeta{
+						Name: ns,
+					},
+				}, metav1.CreateOptions{})).NotTo(BeNil())
+			}
+			Expect(namespaces.Get(ctx, ns, metav1.GetOptions{})).NotTo(BeNil())
+		}
+
+		By("Creating " + policyName + " on managed")
+		utils.Kubectl("apply", "-f", policyYAML, "-n", testNamespace)
+	})
+
+	AfterAll(func() {
+		By("Deleting namespaces")
+		for i := range 5 {
+			ns := "case14-" + strconv.Itoa(i)
+			utils.KubectlDelete("namespace", ns)
+		}
+
+		By("Deleting policy " + policyName)
+		utils.KubectlDelete("-f", policyYAML, "-n", testNamespace)
+	})
+
+	It("should initially be compliant with 2 related objects", func() {
+		Eventually(func(g Gomega) {
+			managedPlc := utils.GetWithTimeout(clientManagedDynamic, gvrConfigPolicy,
+				policyName, testNamespace, true, defaultTimeoutSeconds)
+
+			utils.CheckComplianceStatus(g, managedPlc, "Compliant")
+
+			related, found, err := unstructured.NestedSlice(managedPlc.Object, "status", "relatedObjects")
+			g.Expect(err).NotTo(HaveOccurred())
+			g.Expect(found).To(BeTrue())
+			g.Expect(related).To(HaveLen(2))
+		}, defaultTimeoutSeconds, 1).Should(Succeed())
+	})
+
+	It("should evaluate when a new namespace is created after restarting the controller", func(ctx context.Context) {
+		By("Deleting the controller pod")
+		utils.KubectlDelete("pods", "-l=name=config-policy-controller",
+			"-n=open-cluster-management-agent-addon", "--wait=true")
+		time.Sleep(time.Second)
+
+		By("Waiting for the deployment to be ready again")
+		utils.Kubectl("wait", "--for=condition=Available", "deployment", "config-policy-controller",
+			"-n=open-cluster-management-agent-addon")
+
+		By("Creating a new namespace")
+		Expect(clientManaged.CoreV1().Namespaces().Create(ctx, &corev1.Namespace{
+			ObjectMeta: metav1.ObjectMeta{
+				Name: "case14-3",
+			},
+		}, metav1.CreateOptions{})).NotTo(BeNil())
+
+		By("Checking for the policy to create the new configmap")
+		utils.GetWithTimeout(clientManagedDynamic, gvrConfigMap, "case14-cm",
+			"case14-3", true, defaultTimeoutSeconds)
+	})
+})

test/e2e/case29_trigger_uninstall_test.go

@@ -17,7 +17,7 @@ import (
 )
 
 // This test only works when the controller is running in the cluster.
-var _ = Describe("Clean up during uninstalls", Label("running-in-cluster"), Ordered, func() {
+var _ = Describe("Clean up during uninstalls", Label("running-in-cluster"), Serial, func() {
 	const (
 		configMapName        string = "case29-trigger-uninstall"
 		deploymentName       string = "config-policy-controller"
@@ -110,7 +110,7 @@ var _ = Describe("Clean up during uninstalls", Label("running-in-cluster"), Orde
 		Expect(policy2.GetFinalizers()).To(BeEmpty())
 	})
 
-	AfterAll(func() {
+	AfterEach(func() {
 		deleteConfigPolicies([]string{policyName, policy2Name})
 
 		err := clientManaged.CoreV1().ConfigMaps("default").Delete(

test/resources/case14_namespaces/case14_configmaps_policy.yaml

@@ -0,0 +1,22 @@
+apiVersion: policy.open-cluster-management.io/v1
+kind: ConfigurationPolicy
+metadata:
+  name: case14-configmaps
+spec:
+  evaluationInterval:
+    compliant: 30m
+    noncompliant: 30m
+  namespaceSelector:
+    include:
+      - 'case14-*'
+  object-templates:
+    - complianceType: musthave
+      objectDefinition:
+        apiVersion: v1
+        kind: ConfigMap
+        metadata:
+          name: case14-cm
+        data:
+          foo: bar
+  remediationAction: enforce
+  severity: high