From e7fc1ba31b1853e9b8393c3f334ad27396b044dd Mon Sep 17 00:00:00 2001 From: Justin Kulikauskas Date: Thu, 19 Jan 2023 17:52:01 -0500 Subject: [PATCH 1/3] Add test for configpolicy cleanup during uninstall Tests that the ConfigurationPolicy CRD does not become stuck due to pruneObjectBehavior finalizers when the controller is uninstalled. Refs: - https://issues.redhat.com/browse/ACM-2923 Signed-off-by: Justin Kulikauskas --- test/configuration_policy_prune.go | 136 ++++++++++++++++++++++++----- 1 file changed, 116 insertions(+), 20 deletions(-) diff --git a/test/configuration_policy_prune.go b/test/configuration_policy_prune.go index e7e26bbc..92d45ab9 100644 --- a/test/configuration_policy_prune.go +++ b/test/configuration_policy_prune.go @@ -5,6 +5,7 @@ package test import ( "errors" + "os" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" @@ -20,6 +21,26 @@ func ConfigPruneBehavior(labels ...string) bool { pruneConfigMapYaml string = "../resources/configuration_policy_prune/configmap-only.yaml" ) + cleanPolicy := func(policyName, policyYaml string) func() { + return func() { + _, err := OcHub( + "delete", "-f", policyYaml, + "--ignore-not-found", + ) + Expect(err).To(BeNil()) + _, err = OcManaged( + "delete", + "events", + "-n", + ClusterNamespace, + "--field-selector=involvedObject.name="+ + UserNamespace+"."+policyName, + "--ignore-not-found", + ) + Expect(err).To(BeNil()) + } + } + pruneTestCreatedByPolicy := func(policyName, policyYaml string, cmShouldBeDeleted bool) { clientManagedDynamic := NewKubeClientDynamic("", KubeconfigManaged, "") @@ -270,26 +291,6 @@ func ConfigPruneBehavior(labels ...string) bool { BeforeEach(cleanConfigMap) AfterAll(cleanConfigMap) - cleanPolicy := func(policyName, policyYaml string) func() { - return func() { - _, err := OcHub( - "delete", "-f", policyYaml, - "--ignore-not-found", - ) - Expect(err).To(BeNil()) - _, err = OcManaged( - "delete", - "events", - "-n", - ClusterNamespace, - "--field-selector=involvedObject.name="+ - UserNamespace+"."+policyName, - "--ignore-not-found", - ) - Expect(err).To(BeNil()) - } - } - Describe("Test DeleteAll pruning", func() { policyName := "cm-policy-prune-all" policyYaml := "../resources/configuration_policy_prune/cm-policy-prune-all.yaml" @@ -361,5 +362,100 @@ func ConfigPruneBehavior(labels ...string) bool { }) }) + Describe("GRC: [P1][Sev1][policy-grc] Test cleanup during controller removal", Ordered, func() { + var configpolicyDeploymentYaml string + var configpolicyCRDYaml string + + clientManagedDynamic := NewKubeClientDynamic("", KubeconfigManaged, "") + policyName := "cm-policy-prune-all" + policyYaml := "../resources/configuration_policy_prune/cm-policy-prune-all.yaml" + pruneFinalizer := "policy.open-cluster-management.io/delete-related-objects" + + BeforeAll(func() { + var err error + + configpolicyDeploymentYaml, err = OcManaged("get", "deployment", "-o=yaml", + "--namespace=open-cluster-management-agent-addon", "config-policy-controller") + Expect(err).To(BeNil()) + + configpolicyCRDYaml, err = OcManaged("get", "crd", "-o=yaml", + "configurationpolicies.policy.open-cluster-management.io") + Expect(err).To(BeNil()) + }) + + AfterAll(func() { + By("Re-applying the config policy controller deployment") + tmpDeployFile, err := os.CreateTemp("", "config-policy-controller-*.yaml") + Expect(err).To(BeNil()) + + defer os.Remove(tmpDeployFile.Name()) + + _, err = tmpDeployFile.WriteString(configpolicyDeploymentYaml) + Expect(err).To(BeNil()) + + Expect(tmpDeployFile.Close()).To(BeNil()) + + _, err = OcManaged("apply", "-f", tmpDeployFile.Name()) + Expect(err).To(BeNil()) + + By("Re-applying the configuration policy CRD") + tmpCRDFile, err := os.CreateTemp("", "config-policy-crd-*.yaml") + Expect(err).To(BeNil()) + + defer os.Remove(tmpCRDFile.Name()) + + _, err = tmpCRDFile.WriteString(configpolicyCRDYaml) + Expect(err).To(BeNil()) + + Expect(tmpCRDFile.Close()).To(BeNil()) + + _, err = OcManaged("apply", "-f", tmpCRDFile.Name()) + Expect(err).To(BeNil()) + }) + + AfterAll(cleanPolicy(policyName, policyYaml)) + + It("Should have the finalizer on the deployment", func() { + DoCreatePolicyTest(policyYaml, GvrConfigurationPolicy) + + By("Checking for the finalizer") + Eventually(func(g Gomega) { + deployment := utils.GetWithTimeout( + clientManagedDynamic, + GvrDeployment, + "config-policy-controller", + "open-cluster-management-agent-addon", + true, + DefaultTimeoutSeconds, + ) + + g.Expect(deployment.GetFinalizers()).Should(ContainElement(pruneFinalizer)) + }, DefaultTimeoutSeconds, 1).Should(Succeed()) + }) + + It("Should eventually remove the deployment despite the finalizer", func() { + // Ignoring error because it probably should take more than 1 second, + // but we check whether it eventually succeeds separately. + // nolint: errcheck + OcManaged("delete", "deployment", "--timeout=1s", + "--namespace=open-cluster-management-agent-addon", "config-policy-controller") + + utils.GetWithTimeout( + clientManagedDynamic, + GvrDeployment, + "config-policy-controller", + "open-cluster-management-agent-addon", + false, + DefaultTimeoutSeconds, + ) + }) + + It("Should remove the CRD in a timely manner", func() { + _, err := OcManaged("delete", "crd", "--timeout=15s", + "configurationpolicies.policy.open-cluster-management.io") + Expect(err).To(BeNil()) + }) + }) + return true } From 89ea08f7072d9d7078f8cc29637cf921ba7a038c Mon Sep 17 00:00:00 2001 From: Justin Kulikauskas Date: Thu, 19 Jan 2023 22:08:49 -0500 Subject: [PATCH 2/3] Collect more logs when debugging When a selector is given, `kubectl logs` will only get 10 lines. Now it will get all the logs for the container. Signed-off-by: Justin Kulikauskas --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 5696b464..3261e73a 100644 --- a/Makefile +++ b/Makefile @@ -332,7 +332,7 @@ e2e-debug-kind: e2e-debug -@for APP in $(KIND_COMPONENTS); do\ for CONTAINER in $$(kubectl get pod -l $(KIND_COMPONENT_SELECTOR)=$${APP} -n $(KIND_MANAGED_NAMESPACE) -o jsonpath={.items[*].spec.containers[*].name} --kubeconfig=$(PWD)/kubeconfig_$(MANAGED_CLUSTER_NAME)); do\ echo "* Logs for Label: $(KIND_COMPONENT_SELECTOR)=$${APP}, Container: $${CONTAINER}" > $(DEBUG_DIR)/managed_logs_$${CONTAINER}.log;\ - kubectl logs -l $(KIND_COMPONENT_SELECTOR)=$${APP} -n $(KIND_MANAGED_NAMESPACE) -c $${CONTAINER} --kubeconfig=$(PWD)/kubeconfig_$(MANAGED_CLUSTER_NAME) >> $(DEBUG_DIR)/managed_logs_$${CONTAINER}.log;\ + kubectl logs -l $(KIND_COMPONENT_SELECTOR)=$${APP} -n $(KIND_MANAGED_NAMESPACE) -c $${CONTAINER} --kubeconfig=$(PWD)/kubeconfig_$(MANAGED_CLUSTER_NAME) --tail=-1 >> $(DEBUG_DIR)/managed_logs_$${CONTAINER}.log;\ done;\ done From fa6c07caafd40fcbf278f072ba6f78779f6ebd58 Mon Sep 17 00:00:00 2001 From: Justin Kulikauskas Date: Mon, 23 Jan 2023 10:36:35 -0500 Subject: [PATCH 3/3] Improve cleanPolicy helper func Now it targets the correct namespace, and puts some extra info into the ginkgo writer, which will be printed if the spec fails. Signed-off-by: Justin Kulikauskas --- test/configuration_policy_prune.go | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/test/configuration_policy_prune.go b/test/configuration_policy_prune.go index 92d45ab9..903307e6 100644 --- a/test/configuration_policy_prune.go +++ b/test/configuration_policy_prune.go @@ -23,20 +23,18 @@ func ConfigPruneBehavior(labels ...string) bool { cleanPolicy := func(policyName, policyYaml string) func() { return func() { - _, err := OcHub( - "delete", "-f", policyYaml, - "--ignore-not-found", - ) + By("Cleaning up policy " + policyName + ", ignoring if not found") + + outHub, err := OcHub("delete", "-f", policyYaml, "-n", UserNamespace, "--ignore-not-found") + GinkgoWriter.Printf("cleanPolicy OcHub output: %v\n", outHub) Expect(err).To(BeNil()) - _, err = OcManaged( - "delete", - "events", - "-n", - ClusterNamespace, - "--field-selector=involvedObject.name="+ - UserNamespace+"."+policyName, + + outManaged, err := OcManaged( + "delete", "events", "-n", ClusterNamespace, + "--field-selector=involvedObject.name="+UserNamespace+"."+policyName, "--ignore-not-found", ) + GinkgoWriter.Printf("cleanPolicy OcManaged output: %v\n", outManaged) Expect(err).To(BeNil()) } }