From 7e880d1babb0c77d45644572b00efbf5e1ac3a1f Mon Sep 17 00:00:00 2001 From: Swati Sehgal Date: Mon, 24 Oct 2022 16:21:28 +0100 Subject: [PATCH 1/4] node: e2e: ensure log rotation pod is deleted after test Some node e2e tests check for expected number of pods running on the node to verify the correct state of that node after running test scenarios. An example of such a check is in the device plugin end to end test here: [1]. If the node is not left in a clean state after an e2e test finishes running, it can lead to flaky tests because the node might have unexpected pods running on the node. In order to avoid that, we make sure that the test pods are cleaned up after the test runs. [1]: https://github.com/kubernetes/kubernetes/blob/master/test/e2e_node/device_plugin_test.go#L189-L190 Signed-off-by: Swati Sehgal --- test/e2e_node/container_log_rotation_test.go | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/test/e2e_node/container_log_rotation_test.go b/test/e2e_node/container_log_rotation_test.go index 56796fd469b..6a6a2708446 100644 --- a/test/e2e_node/container_log_rotation_test.go +++ b/test/e2e_node/container_log_rotation_test.go @@ -50,7 +50,8 @@ var _ = SIGDescribe("ContainerLogRotation [Slow] [Serial] [Disruptive]", func() initialConfig.ContainerLogMaxSize = testContainerLogMaxSize }) - ginkgo.It("should be rotated and limited to a fixed amount of files", func(ctx context.Context) { + var logRotationPod *v1.Pod + ginkgo.BeforeEach(func() { ginkgo.By("create log container") pod := &v1.Pod{ ObjectMeta: metav1.ObjectMeta{ @@ -72,10 +73,20 @@ var _ = SIGDescribe("ContainerLogRotation [Slow] [Serial] [Disruptive]", func() }, }, } - pod = e2epod.NewPodClient(f).CreateSync(pod) + logRotationPod = e2epod.NewPodClient(f).CreateSync(pod) + }) + + ginkgo.AfterEach(func() { + ginkgo.By("Deleting the log-rotation pod") + framework.Logf("Deleting pod: %s", logRotationPod.Name) + e2epod.NewPodClient(f).DeleteSync(logRotationPod.Name, metav1.DeleteOptions{}, time.Minute) + }) + + ginkgo.It("should be rotated and limited to a fixed amount of files", func(ctx context.Context) { + ginkgo.By("get container log path") - framework.ExpectEqual(len(pod.Status.ContainerStatuses), 1) - id := kubecontainer.ParseContainerID(pod.Status.ContainerStatuses[0].ContainerID).ID + framework.ExpectEqual(len(logRotationPod.Status.ContainerStatuses), 1) + id := kubecontainer.ParseContainerID(logRotationPod.Status.ContainerStatuses[0].ContainerID).ID r, _, err := getCRIClient() framework.ExpectNoError(err) resp, err := r.ContainerStatus(context.Background(), id, false) From a9e3689e63956bc7fe2715cff7d16119351f5308 Mon Sep 17 00:00:00 2001 From: Swati Sehgal Date: Mon, 24 Oct 2022 16:57:30 +0100 Subject: [PATCH 2/4] node: e2e: ensure clean cluster state before e2e tests are run Signed-off-by: Swati Sehgal --- test/e2e_node/device_plugin_test.go | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/test/e2e_node/device_plugin_test.go b/test/e2e_node/device_plugin_test.go index 242d665f82b..deedc654a66 100644 --- a/test/e2e_node/device_plugin_test.go +++ b/test/e2e_node/device_plugin_test.go @@ -94,7 +94,9 @@ func testDevicePlugin(f *framework.Framework, pluginSockDir string) { // and then use the same here devsLen := int64(2) var devicePluginPod, dptemplate *v1.Pod - + var v1alphaPodResources *kubeletpodresourcesv1alpha1.ListPodResourcesResponse + var v1PodResources *kubeletpodresourcesv1.ListPodResourcesResponse + var err error ginkgo.BeforeEach(func() { ginkgo.By("Wait for node to be ready") gomega.Eventually(func() bool { @@ -103,6 +105,18 @@ func testDevicePlugin(f *framework.Framework, pluginSockDir string) { return nodes == 1 }, time.Minute, time.Second).Should(gomega.BeTrue()) + v1alphaPodResources, err = getV1alpha1NodeDevices() + framework.ExpectNoError(err) + + v1PodResources, err = getV1NodeDevices() + framework.ExpectNoError(err) + + // Before we run the device plugin test, we need to ensure + // that the cluster is in a clean state and there are no + // pods running on this node. + framework.ExpectEqual(len(v1alphaPodResources.PodResources), 0) + framework.ExpectEqual(len(v1PodResources.PodResources), 0) + ginkgo.By("Scheduling a sample device plugin pod") data, err := e2etestfiles.Read(SampleDevicePluginDSYAML) if err != nil { @@ -175,10 +189,10 @@ func testDevicePlugin(f *framework.Framework, pluginSockDir string) { devID1 := parseLog(f, pod1.Name, pod1.Name, deviceIDRE) gomega.Expect(devID1).To(gomega.Not(gomega.Equal(""))) - v1alphaPodResources, err := getV1alpha1NodeDevices() + v1alphaPodResources, err = getV1alpha1NodeDevices() framework.ExpectNoError(err) - v1PodResources, err := getV1NodeDevices() + v1PodResources, err = getV1NodeDevices() framework.ExpectNoError(err) framework.Logf("v1alphaPodResources.PodResources:%+v\n", v1alphaPodResources.PodResources) From 62e4d39c2f3637e3b4fbc1e818ba781fd3ed0d5c Mon Sep 17 00:00:00 2001 From: Swati Sehgal Date: Mon, 12 Dec 2022 16:31:40 +0000 Subject: [PATCH 3/4] node: e2e: address review comments (2022/12/12) - use `ginkgo.DeferCleanup` instead of clean up in the AfterEach block - encourage use of ginkgo by not extending expect.go Signed-off-by: Swati Sehgal --- test/e2e_node/container_log_rotation_test.go | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/test/e2e_node/container_log_rotation_test.go b/test/e2e_node/container_log_rotation_test.go index 6a6a2708446..f281a156405 100644 --- a/test/e2e_node/container_log_rotation_test.go +++ b/test/e2e_node/container_log_rotation_test.go @@ -74,12 +74,7 @@ var _ = SIGDescribe("ContainerLogRotation [Slow] [Serial] [Disruptive]", func() }, } logRotationPod = e2epod.NewPodClient(f).CreateSync(pod) - }) - - ginkgo.AfterEach(func() { - ginkgo.By("Deleting the log-rotation pod") - framework.Logf("Deleting pod: %s", logRotationPod.Name) - e2epod.NewPodClient(f).DeleteSync(logRotationPod.Name, metav1.DeleteOptions{}, time.Minute) + ginkgo.DeferCleanup(e2epod.NewPodClient(f).DeleteSync, logRotationPod.Name, metav1.DeleteOptions{}, time.Minute) }) ginkgo.It("should be rotated and limited to a fixed amount of files", func(ctx context.Context) { From 213a6edc579c058ffdc625e1c0f2270776e5c944 Mon Sep 17 00:00:00 2001 From: Swati Sehgal Date: Tue, 13 Dec 2022 14:54:48 +0000 Subject: [PATCH 4/4] node: e2e: Add descriptive messages for operation/error checks Signed-off-by: Swati Sehgal --- test/e2e_node/container_log_rotation_test.go | 4 ++-- test/e2e_node/device_plugin_test.go | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/test/e2e_node/container_log_rotation_test.go b/test/e2e_node/container_log_rotation_test.go index f281a156405..aadd300d7d9 100644 --- a/test/e2e_node/container_log_rotation_test.go +++ b/test/e2e_node/container_log_rotation_test.go @@ -80,10 +80,10 @@ var _ = SIGDescribe("ContainerLogRotation [Slow] [Serial] [Disruptive]", func() ginkgo.It("should be rotated and limited to a fixed amount of files", func(ctx context.Context) { ginkgo.By("get container log path") - framework.ExpectEqual(len(logRotationPod.Status.ContainerStatuses), 1) + framework.ExpectEqual(len(logRotationPod.Status.ContainerStatuses), 1, "log rotation pod should have one container") id := kubecontainer.ParseContainerID(logRotationPod.Status.ContainerStatuses[0].ContainerID).ID r, _, err := getCRIClient() - framework.ExpectNoError(err) + framework.ExpectNoError(err, "should connect to CRI and obtain runtime service clients and image service client") resp, err := r.ContainerStatus(context.Background(), id, false) framework.ExpectNoError(err) logPath := resp.GetStatus().GetLogPath() diff --git a/test/e2e_node/device_plugin_test.go b/test/e2e_node/device_plugin_test.go index deedc654a66..84568950dad 100644 --- a/test/e2e_node/device_plugin_test.go +++ b/test/e2e_node/device_plugin_test.go @@ -106,16 +106,16 @@ func testDevicePlugin(f *framework.Framework, pluginSockDir string) { }, time.Minute, time.Second).Should(gomega.BeTrue()) v1alphaPodResources, err = getV1alpha1NodeDevices() - framework.ExpectNoError(err) + framework.ExpectNoError(err, "should get node local podresources by accessing the (v1alpha) podresources API endpoint") v1PodResources, err = getV1NodeDevices() - framework.ExpectNoError(err) + framework.ExpectNoError(err, "should get node local podresources by accessing the (v1) podresources API endpoint") // Before we run the device plugin test, we need to ensure // that the cluster is in a clean state and there are no // pods running on this node. - framework.ExpectEqual(len(v1alphaPodResources.PodResources), 0) - framework.ExpectEqual(len(v1PodResources.PodResources), 0) + gomega.Expect(v1alphaPodResources.PodResources).To(gomega.BeEmpty(), "should have no pod resources") + gomega.Expect(v1PodResources.PodResources).To(gomega.BeEmpty(), "should have no pod resources") ginkgo.By("Scheduling a sample device plugin pod") data, err := e2etestfiles.Read(SampleDevicePluginDSYAML)