From 600e8b0ebacfc7ee0f5692ea904ade81c6f16551 Mon Sep 17 00:00:00 2001 From: Minhan Xia Date: Mon, 25 Mar 2019 13:28:41 -0700 Subject: [PATCH] make NEG tests more resilient to API failures and add more failure logging --- test/e2e/framework/providers/gce/ingress.go | 78 +++++++++++++++++---- test/e2e/network/ingress.go | 51 ++++++++------ 2 files changed, 91 insertions(+), 38 deletions(-) diff --git a/test/e2e/framework/providers/gce/ingress.go b/test/e2e/framework/providers/gce/ingress.go index 6519df34ae..e7b34307ce 100644 --- a/test/e2e/framework/providers/gce/ingress.go +++ b/test/e2e/framework/providers/gce/ingress.go @@ -58,8 +58,13 @@ const ( // GCE only allows names < 64 characters, and the loadbalancer controller inserts // a single character of padding. nameLenLimit = 62 + + negBackend = backendType("networkEndpointGroup") + igBackend = backendType("instanceGroup") ) +type backendType string + // IngressController manages implementation details of Ingress on GCE/GKE. type IngressController struct { Ns string @@ -610,28 +615,53 @@ func (cont *IngressController) isHTTPErrorCode(err error, code int) bool { return ok && apiErr.Code == code } -// BackendServiceUsingNEG returns true only if all global backend service with matching nodeports pointing to NEG as backend -func (cont *IngressController) BackendServiceUsingNEG(svcPorts map[string]v1.ServicePort) (bool, error) { - return cont.backendMode(svcPorts, "networkEndpointGroups") +// WaitForNegBackendService waits for the expected backend service to become +func (cont *IngressController) WaitForNegBackendService(svcPorts map[string]v1.ServicePort) error { + return wait.Poll(5*time.Second, 1*time.Minute, func() (bool, error) { + err := cont.verifyBackendMode(svcPorts, negBackend) + if err != nil { + framework.Logf("Err while checking if backend service is using NEG: %v", err) + return false, nil + } + return true, nil + }) +} + +// WaitForIgBackendService returns true only if all global backend service with matching svcPorts pointing to IG as backend +func (cont *IngressController) WaitForIgBackendService(svcPorts map[string]v1.ServicePort) error { + return wait.Poll(5*time.Second, 1*time.Minute, func() (bool, error) { + err := cont.verifyBackendMode(svcPorts, igBackend) + if err != nil { + framework.Logf("Err while checking if backend service is using IG: %v", err) + return false, nil + } + return true, nil + }) +} + +// BackendServiceUsingNEG returns true only if all global backend service with matching svcPorts pointing to NEG as backend +func (cont *IngressController) BackendServiceUsingNEG(svcPorts map[string]v1.ServicePort) error { + return cont.verifyBackendMode(svcPorts, negBackend) } // BackendServiceUsingIG returns true only if all global backend service with matching svcPorts pointing to IG as backend -func (cont *IngressController) BackendServiceUsingIG(svcPorts map[string]v1.ServicePort) (bool, error) { - return cont.backendMode(svcPorts, "instanceGroups") +func (cont *IngressController) BackendServiceUsingIG(svcPorts map[string]v1.ServicePort) error { + return cont.verifyBackendMode(svcPorts, igBackend) } -func (cont *IngressController) backendMode(svcPorts map[string]v1.ServicePort, keyword string) (bool, error) { +func (cont *IngressController) verifyBackendMode(svcPorts map[string]v1.ServicePort, backendType backendType) error { gceCloud := cont.Cloud.Provider.(*Provider).gceCloud beList, err := gceCloud.ListGlobalBackendServices() if err != nil { - return false, fmt.Errorf("failed to list backend services: %v", err) + return fmt.Errorf("failed to list backend services: %v", err) } hcList, err := gceCloud.ListHealthChecks() if err != nil { - return false, fmt.Errorf("failed to list health checks: %v", err) + return fmt.Errorf("failed to list health checks: %v", err) } + // Generate short UID uid := cont.UID if len(uid) > 8 { uid = uid[:8] @@ -641,13 +671,22 @@ func (cont *IngressController) backendMode(svcPorts map[string]v1.ServicePort, k for svcName, sp := range svcPorts { match := false bsMatch := &compute.BackendService{} - // Non-NEG BackendServices are named with the Nodeport in the name. // NEG BackendServices' names contain the a sha256 hash of a string. + // This logic is copied from the ingress-gce namer. + // WARNING: This needs to adapt if the naming convention changed. negString := strings.Join([]string{uid, cont.Ns, svcName, fmt.Sprintf("%v", sp.Port)}, ";") negHash := fmt.Sprintf("%x", sha256.Sum256([]byte(negString)))[:8] for _, bs := range beList { - if strings.Contains(bs.Name, strconv.Itoa(int(sp.NodePort))) || - strings.Contains(bs.Name, negHash) { + // Non-NEG BackendServices are named with the Nodeport in the name. + if backendType == igBackend && strings.Contains(bs.Name, strconv.Itoa(int(sp.NodePort))) { + match = true + bsMatch = bs + matchingBackendService++ + break + } + + // NEG BackendServices' names contain the a sha256 hash of a string. + if backendType == negBackend && strings.Contains(bs.Name, negHash) { match = true bsMatch = bs matchingBackendService++ @@ -657,8 +696,8 @@ func (cont *IngressController) backendMode(svcPorts map[string]v1.ServicePort, k if match { for _, be := range bsMatch.Backends { - if !strings.Contains(be.Group, keyword) { - return false, nil + if !strings.Contains(be.Group, string(backendType)) { + return fmt.Errorf("expect to find backends with type %q, but got backend group: %v", backendType, be.Group) } } @@ -672,11 +711,20 @@ func (cont *IngressController) backendMode(svcPorts map[string]v1.ServicePort, k } if !hcMatch { - return false, fmt.Errorf("missing healthcheck for backendservice: %v", bsMatch.Name) + return fmt.Errorf("missing healthcheck for backendservice: %v", bsMatch.Name) } } } - return matchingBackendService == len(svcPorts), nil + + if matchingBackendService != len(svcPorts) { + beNames := []string{} + for _, be := range beList { + beNames = append(beNames, be.Name) + } + return fmt.Errorf("expect %d backend service with backend type: %v, but got %d matching backend service. Expect backend services for service ports: %v, but got backend services: %v", len(svcPorts), backendType, matchingBackendService, svcPorts, beNames) + } + + return nil } // Cleanup cleans up cloud resources. diff --git a/test/e2e/network/ingress.go b/test/e2e/network/ingress.go index cda2cbca68..d6bd4a3dcb 100644 --- a/test/e2e/network/ingress.go +++ b/test/e2e/network/ingress.go @@ -507,9 +507,7 @@ var _ = SIGDescribe("Loadbalancing: L7", func() { t.Execute() By(t.ExitLog) jig.WaitForIngress(true) - usingNeg, err := gceController.BackendServiceUsingNEG(jig.GetServicePorts(false)) - Expect(err).NotTo(HaveOccurred()) - Expect(usingNeg).To(BeTrue()) + Expect(gceController.WaitForNegBackendService(jig.GetServicePorts(false))).NotTo(HaveOccurred()) } }) @@ -518,9 +516,7 @@ var _ = SIGDescribe("Loadbalancing: L7", func() { By("Create a basic HTTP ingress using NEG") jig.CreateIngress(filepath.Join(ingress.IngressManifestPath, "neg"), ns, map[string]string{}, map[string]string{}) jig.WaitForIngress(true) - usingNEG, err := gceController.BackendServiceUsingNEG(jig.GetServicePorts(false)) - Expect(err).NotTo(HaveOccurred()) - Expect(usingNEG).To(BeTrue()) + Expect(gceController.WaitForNegBackendService(jig.GetServicePorts(false))).NotTo(HaveOccurred()) By("Switch backend service to use IG") svcList, err := f.ClientSet.CoreV1().Services(ns).List(metav1.ListOptions{}) @@ -531,7 +527,11 @@ var _ = SIGDescribe("Loadbalancing: L7", func() { Expect(err).NotTo(HaveOccurred()) } err = wait.Poll(5*time.Second, framework.LoadBalancerPollTimeout, func() (bool, error) { - return gceController.BackendServiceUsingIG(jig.GetServicePorts(false)) + if err := gceController.BackendServiceUsingIG(jig.GetServicePorts(false)); err != nil { + framework.Logf("Failed to verify IG backend service: %v", err) + return false, nil + } + return true, nil }) Expect(err).NotTo(HaveOccurred(), "Expect backend service to target IG, but failed to observe") jig.WaitForIngress(true) @@ -545,21 +545,22 @@ var _ = SIGDescribe("Loadbalancing: L7", func() { Expect(err).NotTo(HaveOccurred()) } err = wait.Poll(5*time.Second, framework.LoadBalancerPollTimeout, func() (bool, error) { - return gceController.BackendServiceUsingNEG(jig.GetServicePorts(false)) + if err := gceController.BackendServiceUsingNEG(jig.GetServicePorts(false)); err != nil { + framework.Logf("Failed to verify NEG backend service: %v", err) + return false, nil + } + return true, nil }) Expect(err).NotTo(HaveOccurred(), "Expect backend service to target NEG, but failed to observe") jig.WaitForIngress(true) }) It("should be able to create a ClusterIP service", func() { - var err error By("Create a basic HTTP ingress using NEG") jig.CreateIngress(filepath.Join(ingress.IngressManifestPath, "neg-clusterip"), ns, map[string]string{}, map[string]string{}) jig.WaitForIngress(true) svcPorts := jig.GetServicePorts(false) - usingNEG, err := gceController.BackendServiceUsingNEG(svcPorts) - Expect(err).NotTo(HaveOccurred()) - Expect(usingNEG).To(BeTrue(), "Expect backend service to be using NEG. But not.") + Expect(gceController.WaitForNegBackendService(svcPorts)).NotTo(HaveOccurred()) // ClusterIP ServicePorts have no NodePort for _, sp := range svcPorts { @@ -592,9 +593,7 @@ var _ = SIGDescribe("Loadbalancing: L7", func() { jig.CreateIngress(filepath.Join(ingress.IngressManifestPath, "neg"), ns, map[string]string{}, map[string]string{}) jig.WaitForIngress(true) jig.WaitForIngressToStable() - usingNEG, err := gceController.BackendServiceUsingNEG(jig.GetServicePorts(false)) - Expect(err).NotTo(HaveOccurred()) - Expect(usingNEG).To(BeTrue()) + Expect(gceController.WaitForNegBackendService(jig.GetServicePorts(false))).NotTo(HaveOccurred()) // initial replicas number is 1 scaleAndValidateNEG(1) @@ -618,9 +617,7 @@ var _ = SIGDescribe("Loadbalancing: L7", func() { jig.CreateIngress(filepath.Join(ingress.IngressManifestPath, "neg"), ns, map[string]string{}, map[string]string{}) jig.WaitForIngress(true) jig.WaitForIngressToStable() - usingNEG, err := gceController.BackendServiceUsingNEG(jig.GetServicePorts(false)) - Expect(err).NotTo(HaveOccurred()) - Expect(usingNEG).To(BeTrue()) + Expect(gceController.WaitForNegBackendService(jig.GetServicePorts(false))).NotTo(HaveOccurred()) By(fmt.Sprintf("Scale backend replicas to %d", replicas)) scale, err := f.ClientSet.AppsV1().Deployments(ns).GetScale(name, metav1.GetOptions{}) @@ -732,9 +729,7 @@ var _ = SIGDescribe("Loadbalancing: L7", func() { By("Create a basic HTTP ingress using NEG") jig.CreateIngress(filepath.Join(ingress.IngressManifestPath, "neg-exposed"), ns, map[string]string{}, map[string]string{}) jig.WaitForIngress(true) - usingNEG, err := gceController.BackendServiceUsingNEG(jig.GetServicePorts(false)) - Expect(err).NotTo(HaveOccurred()) - Expect(usingNEG).To(BeTrue()) + Expect(gceController.WaitForNegBackendService(jig.GetServicePorts(false))).NotTo(HaveOccurred()) // initial replicas number is 1 scaleAndValidateExposedNEG(1) @@ -1127,7 +1122,12 @@ func detectNegAnnotation(f *framework.Framework, jig *ingress.TestJig, gceContro // if we expect no NEGs, then we should be using IGs if negs == 0 { - return gceController.BackendServiceUsingIG(jig.GetServicePorts(false)) + err := gceController.BackendServiceUsingIG(jig.GetServicePorts(false)) + if err != nil { + framework.Logf("Failed to validate IG backend service: %v", err) + return false, nil + } + return true, nil } var status ingress.NegStatus @@ -1160,7 +1160,12 @@ func detectNegAnnotation(f *framework.Framework, jig *ingress.TestJig, gceContro } } - return gceController.BackendServiceUsingNEG(jig.GetServicePorts(false)) + err = gceController.BackendServiceUsingNEG(jig.GetServicePorts(false)) + if err != nil { + framework.Logf("Failed to validate NEG backend service: %v", err) + return false, nil + } + return true, nil }); err != nil { Expect(err).NotTo(HaveOccurred()) }