diff --git a/controllers/templatesync/predicate.go b/controllers/templatesync/predicate.go new file mode 100644 index 00000000..a92ce602 --- /dev/null +++ b/controllers/templatesync/predicate.go @@ -0,0 +1,50 @@ +// Copyright (c) 2023 Red Hat, Inc. +// Copyright Contributors to the Open Cluster Management project + +package templatesync + +import ( + policiesv1 "open-cluster-management.io/governance-policy-propagator/api/v1" + "sigs.k8s.io/controller-runtime/pkg/event" + "sigs.k8s.io/controller-runtime/pkg/predicate" +) + +// templatePredicates filters out changes to policies that don't need to be +// considered by the template-sync controller. +func templatePredicates() predicate.Funcs { + return predicate.Funcs{ + UpdateFunc: func(e event.UpdateEvent) bool { + oldPolicy := e.ObjectOld.(*policiesv1.Policy) + updatedPolicy := e.ObjectNew.(*policiesv1.Policy) + + if oldPolicy.Generation != updatedPolicy.Generation { + // The spec changed - templates need to be updated. + return true + } + + if hasAnyDependencies(updatedPolicy) { + // if it has dependencies, and it's not currently Pending, then + // it needs to re-calculate if it *should* be Pending. + return updatedPolicy.Status.ComplianceState != "Pending" + } + + return false + }, + } +} + +// hasAnyDependencies returns true if the policy has any Dependencies or if +// any of its templates have any ExtraDependencies. +func hasAnyDependencies(pol *policiesv1.Policy) bool { + if len(pol.Spec.Dependencies) > 0 { + return true + } + + for _, tmpl := range pol.Spec.PolicyTemplates { + if len(tmpl.ExtraDependencies) > 0 { + return true + } + } + + return false +} diff --git a/controllers/templatesync/template_sync.go b/controllers/templatesync/template_sync.go index b7c12806..7ab5f618 100644 --- a/controllers/templatesync/template_sync.go +++ b/controllers/templatesync/template_sync.go @@ -37,7 +37,6 @@ import ( ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/handler" - "sigs.k8s.io/controller-runtime/pkg/predicate" "sigs.k8s.io/controller-runtime/pkg/reconcile" "sigs.k8s.io/controller-runtime/pkg/source" @@ -62,7 +61,7 @@ func (r *PolicyReconciler) Setup(mgr ctrl.Manager, depEvents *source.Channel) er return ctrl.NewControllerManagedBy(mgr). Named(ControllerName). For(&policiesv1.Policy{}). - WithEventFilter(predicate.GenerationChangedPredicate{}). + WithEventFilter(templatePredicates()). Watches(depEvents, &handler.EnqueueRequestForObject{}). Complete(r) } diff --git a/test/e2e/case12_ordering_test.go b/test/e2e/case12_ordering_test.go index e2e6a905..b19ed694 100644 --- a/test/e2e/case12_ordering_test.go +++ b/test/e2e/case12_ordering_test.go @@ -5,6 +5,7 @@ package e2e import ( "context" + "time" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" @@ -338,7 +339,46 @@ var _ = Describe("Test dependency logic in template sync", Ordered, func() { ) // should be noncompliant - template A is pending and B is noncompliant - By("Checking if policy status is pending") + By("Checking if policy status is noncompliant") Eventually(checkCompliance(case12Plc2TemplatesName), defaultTimeoutSeconds, 1).Should(Equal("NonCompliant")) }) + It("Should correct a late compliance event while the policy is pending", func() { + By("Creating a dep on hub cluster in ns:" + clusterNamespaceOnHub) + hubPolicyApplyAndDeferCleanup(case12DepYaml, case12DepName) + + By("Creating a policy on hub cluster in ns:" + clusterNamespaceOnHub) + hubPolicyApplyAndDeferCleanup(case12PolicyYaml, case12PolicyName) + + By("Generating a noncompliant event on the dependency") + generateEventOnPolicy( + case12DepName, + "namespace-foo-setup-configpolicy", + "there is violation", + "NonCompliant", + ) + + By("Checking if dependency status is noncompliant") + Eventually(checkCompliance(case12DepName), defaultTimeoutSeconds, 1).Should(Equal("NonCompliant")) + + By("Checking if policy status is pending") + Eventually(checkCompliance(case12PolicyName), defaultTimeoutSeconds*2, 1).Should(Equal("Pending")) + + By("Generating an (incorrect, late) compliance event on the policy") + generateEventOnPolicy( + case12PolicyName, + "case12-config-policy", + "No violation detected", + "Compliant", + ) + + // Sleep 5 seconds to avoid the Compliant status that only sometimes appears; + // otherwise, the next Eventually might finish *before* that status comes and goes. + time.Sleep(5 * time.Second) + + By("Checking if policy status is pending") + Eventually(checkCompliance(case12PolicyName), defaultTimeoutSeconds, 1).Should(Equal("Pending")) + + By("Checking if policy status is consistently pending") + Consistently(checkCompliance(case12PolicyName), "15s", 1).Should(Equal("Pending")) + }) })