From 011b2cde33e76af4acee4d2682b27e0383cc6afa Mon Sep 17 00:00:00 2001 From: Matt Liggett Date: Fri, 17 Jun 2016 10:49:23 -0700 Subject: [PATCH 1/5] Use DNS TTL --- test/e2e/federated-service.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/test/e2e/federated-service.go b/test/e2e/federated-service.go index ec2f448805d..705713ee369 100644 --- a/test/e2e/federated-service.go +++ b/test/e2e/federated-service.go @@ -50,6 +50,9 @@ const ( FederatedServicePod = "federated-service-test-pod" DefaultFederationName = "federation" + + // We use this to decide how long to wait for our DNS probes to succeed. + DNSTTL = 180 * time.Second ) var _ = framework.KubeDescribe("Service [Feature:Federation]", func() { @@ -345,6 +348,6 @@ func discoverService(f *framework.Framework, name string) { } return logerr(fmt.Errorf("exited %d", status.State.Terminated.ExitCode)) - }, time.Minute*2, time.Second*2). + }, DNSTTL, time.Second*2). Should(BeNil(), "%q should exit 0, but it never did", command) } From 2b3e9d5a9835347e2fb25c8b9832ee00c5e75119 Mon Sep 17 00:00:00 2001 From: Matt Liggett Date: Fri, 17 Jun 2016 10:49:45 -0700 Subject: [PATCH 2/5] Add a "this should not work" comment. --- test/e2e/federated-service.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/e2e/federated-service.go b/test/e2e/federated-service.go index 705713ee369..76363278868 100644 --- a/test/e2e/federated-service.go +++ b/test/e2e/federated-service.go @@ -176,6 +176,8 @@ var _ = framework.KubeDescribe("Service [Feature:Federation]", func() { framework.SkipUnlessFederated(f.Client) // Delete a federated service shard in the default e2e Kubernetes cluster. + // TODO(mml): This should not work: #27623. We should use a load + // balancer with actual back-ends, some of which we delete or disable. err := f.Clientset_1_3.Core().Services(f.Namespace.Name).Delete(FederatedServiceName, &api.DeleteOptions{}) Expect(err).NotTo(HaveOccurred()) waitForFederatedServiceShard(f.Clientset_1_3, f.Namespace.Name, nil, 0) From dd7626bbf1785f8d540cd731ce997d65d9eae4b9 Mon Sep 17 00:00:00 2001 From: Matt Liggett Date: Fri, 17 Jun 2016 11:39:39 -0700 Subject: [PATCH 3/5] refactor and re-add exists --- test/e2e/federated-service.go | 94 +++++++++++++++++++---------------- 1 file changed, 52 insertions(+), 42 deletions(-) diff --git a/test/e2e/federated-service.go b/test/e2e/federated-service.go index 76363278868..69987755270 100644 --- a/test/e2e/federated-service.go +++ b/test/e2e/federated-service.go @@ -167,7 +167,7 @@ var _ = framework.KubeDescribe("Service [Feature:Federation]", func() { // pods, perhaps in the BeforeEach, and then just poll until we get // successes/failures from them all. for _, name := range svcDNSNames { - discoverService(f, name) + discoverService(f, name, true) } }) @@ -191,7 +191,7 @@ var _ = framework.KubeDescribe("Service [Feature:Federation]", func() { fmt.Sprintf("%s.%s.%s.svc.cluster.local.", FederatedServiceName, f.Namespace.Name, federationName), } for _, name := range svcDNSNames { - discoverService(f, name) + discoverService(f, name, true) } // TODO(mml): Unclear how to make this meaningful and not terribly @@ -286,8 +286,50 @@ func createService(fcs *federation_release_1_3.Clientset, clusterClientSets []*r } } -func discoverService(f *framework.Framework, name string) { +func podExitCodeDetector(f *framework.Framework, name string, code int32) func() error { + // If we ever get any container logs, stash them here. + logs := "" + + logerr := func(err error) error { + if err == nil { + return nil + } + if logs == "" { + return err + } + return fmt.Errorf("%s (%v)", logs, err) + } + + return func() error { + pod, err := f.Client.Pods(f.Namespace.Name).Get(name) + if err != nil { + return logerr(err) + } + if len(pod.Status.ContainerStatuses) < 1 { + return logerr(fmt.Errorf("no container statuses")) + } + + // Best effort attempt to grab pod logs for debugging + logs, err = framework.GetPodLogs(f.Client, f.Namespace.Name, name, pod.Spec.Containers[0].Name) + if err != nil { + framework.Logf("Cannot fetch pod logs: %v", err) + } + + status := pod.Status.ContainerStatuses[0] + if status.State.Terminated == nil { + return logerr(fmt.Errorf("container is not in terminated state")) + } + if status.State.Terminated.ExitCode == code { + return nil + } + + return logerr(fmt.Errorf("exited %d", status.State.Terminated.ExitCode)) + } +} + +func discoverService(f *framework.Framework, name string, exists bool) { command := []string{"sh", "-c", fmt.Sprintf("until nslookup '%s'; do sleep 1; done", name)} + By(fmt.Sprintf("Looking up %q", name)) defer f.Client.Pods(f.Namespace.Name).Delete(FederatedServicePod, api.NewDeleteOptions(0)) @@ -312,44 +354,12 @@ func discoverService(f *framework.Framework, name string) { Expect(err). NotTo(HaveOccurred(), "Trying to create pod to run %q", command) - // If we ever get any container logs, stash them here. - logs := "" - - logerr := func(err error) error { - if err == nil { - return nil - } - if logs == "" { - return err - } - return fmt.Errorf("%s (%v)", logs, err) + if exists { + // TODO(mml): Eventually check the IP address is correct, too. + Eventually(podExitCodeDetector(f, FederatedServicePod, 0), DNSTTL, time.Second*2). + Should(BeNil(), "%q should exit 0, but it never did", command) + } else { + Consistently(podExitCodeDetector(f, FederatedServicePod, 0), DNSTTL, time.Second*2). + ShouldNot(BeNil(), "%q should never exit 0, but it did", command) } - - // TODO(mml): Eventually check the IP address is correct, too. - Eventually(func() error { - pod, err := f.Client.Pods(f.Namespace.Name).Get(FederatedServicePod) - if err != nil { - return logerr(err) - } - if len(pod.Status.ContainerStatuses) < 1 { - return logerr(fmt.Errorf("no container statuses")) - } - - // Best effort attempt to grab pod logs for debugging - logs, err = framework.GetPodLogs(f.Client, f.Namespace.Name, FederatedServicePod, "federated-service-discovery-container") - if err != nil { - framework.Logf("Cannot fetch pod logs: %v", err) - } - - status := pod.Status.ContainerStatuses[0] - if status.State.Terminated == nil { - return logerr(fmt.Errorf("container is not in terminated state")) - } - if status.State.Terminated.ExitCode == 0 { - return nil - } - - return logerr(fmt.Errorf("exited %d", status.State.Terminated.ExitCode)) - }, DNSTTL, time.Second*2). - Should(BeNil(), "%q should exit 0, but it never did", command) } From 7cf7717d4c6612828a336d84158cd438d541fb7d Mon Sep 17 00:00:00 2001 From: Matt Liggett Date: Fri, 17 Jun 2016 11:38:53 -0700 Subject: [PATCH 4/5] Move the slow part of the test into a Slow block. --- test/e2e/federated-service.go | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/test/e2e/federated-service.go b/test/e2e/federated-service.go index 69987755270..80d08c53c2b 100644 --- a/test/e2e/federated-service.go +++ b/test/e2e/federated-service.go @@ -194,11 +194,9 @@ var _ = framework.KubeDescribe("Service [Feature:Federation]", func() { discoverService(f, name, true) } - // TODO(mml): Unclear how to make this meaningful and not terribly - // slow. How long (how many minutes?) do we verify that a given DNS - // lookup *doesn't* work before we call it a success? For now, - // commenting out. - /* + // TODO(mml): This currently takes 9 minutes. Consider reducing the + // TTL and/or running the pods in parallel. + Context("[Slow]", func() { localSvcDNSNames := []string{ FederatedServiceName, fmt.Sprintf("%s.%s", FederatedServiceName, f.Namespace.Name), @@ -207,7 +205,7 @@ var _ = framework.KubeDescribe("Service [Feature:Federation]", func() { for _, name := range localSvcDNSNames { discoverService(f, name, false) } - */ + }) }) }) }) From b0a4fdea4da07a236694886b6d556f49b1cfa941 Mon Sep 17 00:00:00 2001 From: Matt Liggett Date: Fri, 17 Jun 2016 11:41:36 -0700 Subject: [PATCH 5/5] tidy --- test/e2e/federated-service.go | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/test/e2e/federated-service.go b/test/e2e/federated-service.go index 80d08c53c2b..056a78576c8 100644 --- a/test/e2e/federated-service.go +++ b/test/e2e/federated-service.go @@ -329,8 +329,6 @@ func discoverService(f *framework.Framework, name string, exists bool) { command := []string{"sh", "-c", fmt.Sprintf("until nslookup '%s'; do sleep 1; done", name)} By(fmt.Sprintf("Looking up %q", name)) - defer f.Client.Pods(f.Namespace.Name).Delete(FederatedServicePod, api.NewDeleteOptions(0)) - pod := &api.Pod{ ObjectMeta: api.ObjectMeta{ Name: FederatedServicePod, @@ -349,8 +347,8 @@ func discoverService(f *framework.Framework, name string, exists bool) { } _, err := f.Client.Pods(f.Namespace.Name).Create(pod) - Expect(err). - NotTo(HaveOccurred(), "Trying to create pod to run %q", command) + Expect(err).NotTo(HaveOccurred(), "Trying to create pod to run %q", command) + defer f.Client.Pods(f.Namespace.Name).Delete(FederatedServicePod, api.NewDeleteOptions(0)) if exists { // TODO(mml): Eventually check the IP address is correct, too.