make NEG tests more resilient to API failures and add more failure logging

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.

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())
 	}