From e623d33343566abba9196652f3873488e20ee10d Mon Sep 17 00:00:00 2001 From: Clayton Coleman Date: Fri, 7 Aug 2015 12:40:59 -0400 Subject: [PATCH] e2e test cases should clean up more effectively Graceful deletion exposes weakness in cleanup paths, add common hooks for waiting for namespcae deletion to complete, use direct delection where necessary, and add some debug output for finding cleanup flags (namespaces that aren't fully deleted) --- test/e2e/framework.go | 3 ++- test/e2e/kubectl.go | 4 +-- test/e2e/pods.go | 13 +++++---- test/e2e/service.go | 10 ++++++- test/e2e/util.go | 62 ++++++++++++++++++++++++++++++++++++++----- 5 files changed, 77 insertions(+), 15 deletions(-) diff --git a/test/e2e/framework.go b/test/e2e/framework.go index b2167d7f1b..de9b69406a 100644 --- a/test/e2e/framework.go +++ b/test/e2e/framework.go @@ -94,7 +94,8 @@ func (f *Framework) afterEach() { } By(fmt.Sprintf("Destroying namespace %q for this suite.", f.Namespace.Name)) - if err := f.Client.Namespaces().Delete(f.Namespace.Name); err != nil { + + if err := deleteNS(f.Client, f.Namespace.Name); err != nil { Failf("Couldn't delete ns %q: %s", f.Namespace.Name, err) } // Paranoia-- prevent reuse! diff --git a/test/e2e/kubectl.go b/test/e2e/kubectl.go index 6a21e668b3..9369a0928c 100644 --- a/test/e2e/kubectl.go +++ b/test/e2e/kubectl.go @@ -78,7 +78,7 @@ var _ = Describe("Kubectl client", func() { AfterEach(func() { By(fmt.Sprintf("Destroying namespace for this suite %v", ns)) - if err := c.Namespaces().Delete(ns); err != nil { + if err := deleteNS(c, ns); err != nil { Failf("Couldn't delete ns %s", err) } }) @@ -467,7 +467,7 @@ var _ = Describe("Kubectl client", func() { } } if !found { - Failf("Added annation not found") + Failf("Added annotation not found") } }) }) diff --git a/test/e2e/pods.go b/test/e2e/pods.go index bf28677edb..4fc7b2d70d 100644 --- a/test/e2e/pods.go +++ b/test/e2e/pods.go @@ -246,14 +246,11 @@ var _ = Describe("Pods", func() { } By("deleting the pod") - podClient.Delete(pod.Name, nil) - pods, err = podClient.List(labels.SelectorFromSet(labels.Set(map[string]string{"time": value})), fields.Everything()) - if err != nil { + if err := podClient.Delete(pod.Name, nil); err != nil { Failf("Failed to delete pod: %v", err) } - Expect(len(pods.Items)).To(Equal(0)) - By("veryfying pod deletion was observed") + By("verifying pod deletion was observed") deleted := false timeout := false timer := time.After(podStartTimeout) @@ -270,6 +267,12 @@ var _ = Describe("Pods", func() { if !deleted { Fail("Failed to observe pod deletion") } + + pods, err = podClient.List(labels.SelectorFromSet(labels.Set(map[string]string{"time": value})), fields.Everything()) + if err != nil { + Fail(fmt.Sprintf("Failed to list pods to verify deletion: %v", err)) + } + Expect(len(pods.Items)).To(Equal(0)) }) It("should be updated", func() { diff --git a/test/e2e/service.go b/test/e2e/service.go index 982f6239a0..40fcadd8e0 100644 --- a/test/e2e/service.go +++ b/test/e2e/service.go @@ -62,7 +62,7 @@ var _ = Describe("Services", func() { AfterEach(func() { for _, ns := range namespaces { By(fmt.Sprintf("Destroying namespace %v", ns)) - if err := c.Namespaces().Delete(ns); err != nil { + if err := deleteNS(c, ns); err != nil { Failf("Couldn't delete namespace %s: %s", ns, err) } } @@ -1096,6 +1096,14 @@ func validateEndpointsOrFail(c *client.Client, namespace, serviceName string, ex } Logf("Unexpected number of endpoints: found %v, expected %v (%v elapsed, ignoring for 5s)", portsByIp, expectedEndpoints, time.Since(start)) } + + if pods, err := c.Pods(api.NamespaceAll).List(labels.Everything(), fields.Everything()); err == nil { + for _, pod := range pods.Items { + Logf("Pod %s\t%s\t%s\t%s", pod.Namespace, pod.Name, pod.Spec.NodeName, pod.DeletionTimestamp) + } + } else { + Logf("Can't list pod debug info: %v", err) + } Failf("Timed out waiting for service %s in namespace %s to expose endpoints %v (%v elapsed)", serviceName, namespace, expectedEndpoints, serviceStartTimeout) } diff --git a/test/e2e/util.go b/test/e2e/util.go index 3449417c70..8ec5dee609 100644 --- a/test/e2e/util.go +++ b/test/e2e/util.go @@ -497,7 +497,7 @@ func deleteTestingNS(c *client.Client) error { for _, ns := range namespaces.Items { if strings.HasPrefix(ns.ObjectMeta.Name, "e2e-tests-") { if ns.Status.Phase == api.NamespaceActive { - return fmt.Errorf("Namespace %s is active", ns) + return fmt.Errorf("Namespace %s is active", ns.ObjectMeta.Name) } terminating++ } @@ -509,6 +509,51 @@ func deleteTestingNS(c *client.Client) error { return fmt.Errorf("Waiting for terminating namespaces to be deleted timed out") } +// deleteNS deletes the provided namespace, waits for it to be completely deleted, and then checks +// whether there are any pods remaining in a non-terminating state. +func deleteNS(c *client.Client, namespace string) error { + if err := c.Namespaces().Delete(namespace); err != nil { + return err + } + + err := wait.Poll(1*time.Second, 2*time.Minute, func() (bool, error) { + if _, err := c.Namespaces().Get(namespace); err != nil { + if apierrs.IsNotFound(err) { + return true, nil + } + Logf("Error while waiting for namespace to be terminated: %v", err) + return false, nil + } + return false, nil + }) + + // check for pods that were not deleted + remaining := []string{} + missingTimestamp := false + if pods, perr := c.Pods(namespace).List(labels.Everything(), fields.Everything()); perr == nil { + for _, pod := range pods.Items { + Logf("Pod %s %s on node %s remains, has deletion timestamp %s", namespace, pod.Name, pod.Spec.NodeName, pod.DeletionTimestamp) + remaining = append(remaining, pod.Name) + if pod.DeletionTimestamp == nil { + missingTimestamp = true + } + } + } + + // a timeout occured + if err != nil { + if missingTimestamp { + return fmt.Errorf("namespace %s was not deleted within limit: %v, some pods were not marked with a deletion timestamp, pods remaining: %v", namespace, err, remaining) + } + return fmt.Errorf("namespace %s was not deleted within limit: %v, pods remaining: %v", namespace, err, remaining) + } + // pods were not deleted but the namespace was deleted + if len(remaining) > 0 { + return fmt.Errorf("pods remained within namespace %s after deletion: %v", namespace, remaining) + } + return nil +} + func waitForPodRunningInNamespace(c *client.Client, podName string, namespace string) error { return waitForPodCondition(c, namespace, podName, "running", podStartTimeout, func(pod *api.Pod) (bool, error) { if pod.Status.Phase == api.PodRunning { @@ -545,12 +590,10 @@ func waitForPodSuccessInNamespace(c *client.Client, podName string, contName str if ci.State.Terminated.ExitCode == 0 { By("Saw pod success") return true, nil - } else { - return true, fmt.Errorf("pod '%s' terminated with failure: %+v", podName, ci.State.Terminated) } - } else { - Logf("Nil State.Terminated for container '%s' in pod '%s' in namespace '%s' so far", contName, podName, namespace) + return true, fmt.Errorf("pod '%s' terminated with failure: %+v", podName, ci.State.Terminated) } + Logf("Nil State.Terminated for container '%s' in pod '%s' in namespace '%s' so far", contName, podName, namespace) } return false, nil }) @@ -959,7 +1002,7 @@ func tryKill(cmd *exec.Cmd) { func testContainerOutputInNamespace(scenarioName string, c *client.Client, pod *api.Pod, containerIndex int, expectedOutput []string, ns string) { By(fmt.Sprintf("Creating a pod to test %v", scenarioName)) - defer c.Pods(ns).Delete(pod.Name, nil) + defer c.Pods(ns).Delete(pod.Name, api.NewDeleteOptions(0)) if _, err := c.Pods(ns).Create(pod); err != nil { Failf("Failed to create pod: %v", err) } @@ -1243,6 +1286,13 @@ func RunRC(config RCConfig) error { } if oldRunning != config.Replicas { + if pods, err := config.Client.Pods(api.NamespaceAll).List(labels.Everything(), fields.Everything()); err == nil { + for _, pod := range pods.Items { + Logf("Pod %s\t%s\t%s\t%s", pod.Namespace, pod.Name, pod.Spec.NodeName, pod.DeletionTimestamp) + } + } else { + Logf("Can't list pod debug info: %v", err) + } return fmt.Errorf("Only %d pods started out of %d", oldRunning, config.Replicas) } return nil