From e2a759b94a1230e29d9f0a5672390cbf2350c8e5 Mon Sep 17 00:00:00 2001 From: Justin Kulikauskas Date: Wed, 22 Oct 2025 16:48:09 -0400 Subject: [PATCH] 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 --- pkg/common/namespace_selection.go | 9 +- test/e2e/case14_selection_test.go | 107 +++++++++++++++--- test/e2e/case29_trigger_uninstall_test.go | 4 +- .../case14_configmaps_policy.yaml | 22 ++++ 4 files changed, 122 insertions(+), 20 deletions(-) create mode 100644 test/resources/case14_namespaces/case14_configmaps_policy.yaml diff --git a/pkg/common/namespace_selection.go b/pkg/common/namespace_selection.go index 422fd398..77a76adc 100644 --- a/pkg/common/namespace_selection.go +++ b/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. diff --git a/test/e2e/case14_selection_test.go b/test/e2e/case14_selection_test.go index 9b4070be..36bf1aea 100644 --- a/test/e2e/case14_selection_test.go +++ b/test/e2e/case14_selection_test.go @@ -4,7 +4,8 @@ package e2e import ( - "context" + "strconv" + "time" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" @@ -16,12 +17,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 +51,18 @@ var _ = Describe("Test policy compliance with namespace selection", Ordered, fun }, } - BeforeAll(func() { + BeforeAll(func(ctx SpecContext) { 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 +136,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 SpecContext) { 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 +239,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 SpecContext) { 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 +269,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 SpecContext) { + 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 SpecContext) { + 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) + }) +}) diff --git a/test/e2e/case29_trigger_uninstall_test.go b/test/e2e/case29_trigger_uninstall_test.go index ecf4c0dd..d8b630b9 100644 --- a/test/e2e/case29_trigger_uninstall_test.go +++ b/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( diff --git a/test/resources/case14_namespaces/case14_configmaps_policy.yaml b/test/resources/case14_namespaces/case14_configmaps_policy.yaml new file mode 100644 index 00000000..30103b2a --- /dev/null +++ b/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