Skip to content

Commit 600e8b0

Browse files
freehanfreehan
committed
make NEG tests more resilient to API failures and add more failure logging
1 parent 428a8e0 commit 600e8b0

File tree

2 files changed

+91
-38
lines changed

2 files changed

+91
-38
lines changed

‎test/e2e/framework/providers/gce/ingress.go‎

Lines changed: 63 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -58,8 +58,13 @@ const (
5858
// GCE only allows names < 64 characters, and the loadbalancer controller inserts
5959
// a single character of padding.
6060
nameLenLimit = 62
61+
62+
negBackend = backendType("networkEndpointGroup")
63+
igBackend = backendType("instanceGroup")
6164
)
6265

66+
type backendType string
67+
6368
// IngressController manages implementation details of Ingress on GCE/GKE.
6469
type IngressController struct {
6570
Ns string
@@ -610,28 +615,53 @@ func (cont *IngressController) isHTTPErrorCode(err error, code int) bool {
610615
return ok && apiErr.Code == code
611616
}
612617

613-
// BackendServiceUsingNEG returns true only if all global backend service with matching nodeports pointing to NEG as backend
614-
func (cont *IngressController) BackendServiceUsingNEG(svcPorts map[string]v1.ServicePort) (bool, error) {
615-
return cont.backendMode(svcPorts, "networkEndpointGroups")
618+
// WaitForNegBackendService waits for the expected backend service to become
619+
func (cont *IngressController) WaitForNegBackendService(svcPorts map[string]v1.ServicePort) error {
620+
return wait.Poll(5*time.Second, 1*time.Minute, func() (bool, error) {
621+
err := cont.verifyBackendMode(svcPorts, negBackend)
622+
if err != nil {
623+
framework.Logf("Err while checking if backend service is using NEG: %v", err)
624+
return false, nil
625+
}
626+
return true, nil
627+
})
628+
}
629+
630+
// WaitForIgBackendService returns true only if all global backend service with matching svcPorts pointing to IG as backend
631+
func (cont *IngressController) WaitForIgBackendService(svcPorts map[string]v1.ServicePort) error {
632+
return wait.Poll(5*time.Second, 1*time.Minute, func() (bool, error) {
633+
err := cont.verifyBackendMode(svcPorts, igBackend)
634+
if err != nil {
635+
framework.Logf("Err while checking if backend service is using IG: %v", err)
636+
return false, nil
637+
}
638+
return true, nil
639+
})
640+
}
641+
642+
// BackendServiceUsingNEG returns true only if all global backend service with matching svcPorts pointing to NEG as backend
643+
func (cont *IngressController) BackendServiceUsingNEG(svcPorts map[string]v1.ServicePort) error {
644+
return cont.verifyBackendMode(svcPorts, negBackend)
616645
}
617646

618647
// BackendServiceUsingIG returns true only if all global backend service with matching svcPorts pointing to IG as backend
619-
func (cont *IngressController) BackendServiceUsingIG(svcPorts map[string]v1.ServicePort) (bool, error) {
620-
return cont.backendMode(svcPorts, "instanceGroups")
648+
func (cont *IngressController) BackendServiceUsingIG(svcPorts map[string]v1.ServicePort) error {
649+
return cont.verifyBackendMode(svcPorts, igBackend)
621650
}
622651

623-
func (cont *IngressController) backendMode(svcPorts map[string]v1.ServicePort, keyword string) (bool, error) {
652+
func (cont *IngressController) verifyBackendMode(svcPorts map[string]v1.ServicePort, backendType backendType) error {
624653
gceCloud := cont.Cloud.Provider.(*Provider).gceCloud
625654
beList, err := gceCloud.ListGlobalBackendServices()
626655
if err != nil {
627-
return false, fmt.Errorf("failed to list backend services: %v", err)
656+
return fmt.Errorf("failed to list backend services: %v", err)
628657
}
629658

630659
hcList, err := gceCloud.ListHealthChecks()
631660
if err != nil {
632-
return false, fmt.Errorf("failed to list health checks: %v", err)
661+
return fmt.Errorf("failed to list health checks: %v", err)
633662
}
634663

664+
// Generate short UID
635665
uid := cont.UID
636666
if len(uid) > 8 {
637667
uid = uid[:8]
@@ -641,13 +671,22 @@ func (cont *IngressController) backendMode(svcPorts map[string]v1.ServicePort, k
641671
for svcName, sp := range svcPorts {
642672
match := false
643673
bsMatch := &compute.BackendService{}
644-
// Non-NEG BackendServices are named with the Nodeport in the name.
645674
// NEG BackendServices' names contain the a sha256 hash of a string.
675+
// This logic is copied from the ingress-gce namer.
676+
// WARNING: This needs to adapt if the naming convention changed.
646677
negString := strings.Join([]string{uid, cont.Ns, svcName, fmt.Sprintf("%v", sp.Port)}, ";")
647678
negHash := fmt.Sprintf("%x", sha256.Sum256([]byte(negString)))[:8]
648679
for _, bs := range beList {
649-
if strings.Contains(bs.Name, strconv.Itoa(int(sp.NodePort))) ||
650-
strings.Contains(bs.Name, negHash) {
680+
// Non-NEG BackendServices are named with the Nodeport in the name.
681+
if backendType == igBackend && strings.Contains(bs.Name, strconv.Itoa(int(sp.NodePort))) {
682+
match = true
683+
bsMatch = bs
684+
matchingBackendService++
685+
break
686+
}
687+
688+
// NEG BackendServices' names contain the a sha256 hash of a string.
689+
if backendType == negBackend && strings.Contains(bs.Name, negHash) {
651690
match = true
652691
bsMatch = bs
653692
matchingBackendService++
@@ -657,8 +696,8 @@ func (cont *IngressController) backendMode(svcPorts map[string]v1.ServicePort, k
657696

658697
if match {
659698
for _, be := range bsMatch.Backends {
660-
if !strings.Contains(be.Group, keyword) {
661-
return false, nil
699+
if !strings.Contains(be.Group, string(backendType)) {
700+
return fmt.Errorf("expect to find backends with type %q, but got backend group: %v", backendType, be.Group)
662701
}
663702
}
664703

@@ -672,11 +711,20 @@ func (cont *IngressController) backendMode(svcPorts map[string]v1.ServicePort, k
672711
}
673712

674713
if !hcMatch {
675-
return false, fmt.Errorf("missing healthcheck for backendservice: %v", bsMatch.Name)
714+
return fmt.Errorf("missing healthcheck for backendservice: %v", bsMatch.Name)
676715
}
677716
}
678717
}
679-
return matchingBackendService == len(svcPorts), nil
718+
719+
if matchingBackendService != len(svcPorts) {
720+
beNames := []string{}
721+
for _, be := range beList {
722+
beNames = append(beNames, be.Name)
723+
}
724+
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)
725+
}
726+
727+
return nil
680728
}
681729

682730
// Cleanup cleans up cloud resources.

‎test/e2e/network/ingress.go‎

Lines changed: 28 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -507,9 +507,7 @@ var _ = SIGDescribe("Loadbalancing: L7", func() {
507507
t.Execute()
508508
By(t.ExitLog)
509509
jig.WaitForIngress(true)
510-
usingNeg, err := gceController.BackendServiceUsingNEG(jig.GetServicePorts(false))
511-
Expect(err).NotTo(HaveOccurred())
512-
Expect(usingNeg).To(BeTrue())
510+
Expect(gceController.WaitForNegBackendService(jig.GetServicePorts(false))).NotTo(HaveOccurred())
513511
}
514512
})
515513

@@ -518,9 +516,7 @@ var _ = SIGDescribe("Loadbalancing: L7", func() {
518516
By("Create a basic HTTP ingress using NEG")
519517
jig.CreateIngress(filepath.Join(ingress.IngressManifestPath, "neg"), ns, map[string]string{}, map[string]string{})
520518
jig.WaitForIngress(true)
521-
usingNEG, err := gceController.BackendServiceUsingNEG(jig.GetServicePorts(false))
522-
Expect(err).NotTo(HaveOccurred())
523-
Expect(usingNEG).To(BeTrue())
519+
Expect(gceController.WaitForNegBackendService(jig.GetServicePorts(false))).NotTo(HaveOccurred())
524520

525521
By("Switch backend service to use IG")
526522
svcList, err := f.ClientSet.CoreV1().Services(ns).List(metav1.ListOptions{})
@@ -531,7 +527,11 @@ var _ = SIGDescribe("Loadbalancing: L7", func() {
531527
Expect(err).NotTo(HaveOccurred())
532528
}
533529
err = wait.Poll(5*time.Second, framework.LoadBalancerPollTimeout, func() (bool, error) {
534-
return gceController.BackendServiceUsingIG(jig.GetServicePorts(false))
530+
if err := gceController.BackendServiceUsingIG(jig.GetServicePorts(false)); err != nil {
531+
framework.Logf("Failed to verify IG backend service: %v", err)
532+
return false, nil
533+
}
534+
return true, nil
535535
})
536536
Expect(err).NotTo(HaveOccurred(), "Expect backend service to target IG, but failed to observe")
537537
jig.WaitForIngress(true)
@@ -545,21 +545,22 @@ var _ = SIGDescribe("Loadbalancing: L7", func() {
545545
Expect(err).NotTo(HaveOccurred())
546546
}
547547
err = wait.Poll(5*time.Second, framework.LoadBalancerPollTimeout, func() (bool, error) {
548-
return gceController.BackendServiceUsingNEG(jig.GetServicePorts(false))
548+
if err := gceController.BackendServiceUsingNEG(jig.GetServicePorts(false)); err != nil {
549+
framework.Logf("Failed to verify NEG backend service: %v", err)
550+
return false, nil
551+
}
552+
return true, nil
549553
})
550554
Expect(err).NotTo(HaveOccurred(), "Expect backend service to target NEG, but failed to observe")
551555
jig.WaitForIngress(true)
552556
})
553557

554558
It("should be able to create a ClusterIP service", func() {
555-
var err error
556559
By("Create a basic HTTP ingress using NEG")
557560
jig.CreateIngress(filepath.Join(ingress.IngressManifestPath, "neg-clusterip"), ns, map[string]string{}, map[string]string{})
558561
jig.WaitForIngress(true)
559562
svcPorts := jig.GetServicePorts(false)
560-
usingNEG, err := gceController.BackendServiceUsingNEG(svcPorts)
561-
Expect(err).NotTo(HaveOccurred())
562-
Expect(usingNEG).To(BeTrue(), "Expect backend service to be using NEG. But not.")
563+
Expect(gceController.WaitForNegBackendService(svcPorts)).NotTo(HaveOccurred())
563564

564565
// ClusterIP ServicePorts have no NodePort
565566
for _, sp := range svcPorts {
@@ -592,9 +593,7 @@ var _ = SIGDescribe("Loadbalancing: L7", func() {
592593
jig.CreateIngress(filepath.Join(ingress.IngressManifestPath, "neg"), ns, map[string]string{}, map[string]string{})
593594
jig.WaitForIngress(true)
594595
jig.WaitForIngressToStable()
595-
usingNEG, err := gceController.BackendServiceUsingNEG(jig.GetServicePorts(false))
596-
Expect(err).NotTo(HaveOccurred())
597-
Expect(usingNEG).To(BeTrue())
596+
Expect(gceController.WaitForNegBackendService(jig.GetServicePorts(false))).NotTo(HaveOccurred())
598597
// initial replicas number is 1
599598
scaleAndValidateNEG(1)
600599

@@ -618,9 +617,7 @@ var _ = SIGDescribe("Loadbalancing: L7", func() {
618617
jig.CreateIngress(filepath.Join(ingress.IngressManifestPath, "neg"), ns, map[string]string{}, map[string]string{})
619618
jig.WaitForIngress(true)
620619
jig.WaitForIngressToStable()
621-
usingNEG, err := gceController.BackendServiceUsingNEG(jig.GetServicePorts(false))
622-
Expect(err).NotTo(HaveOccurred())
623-
Expect(usingNEG).To(BeTrue())
620+
Expect(gceController.WaitForNegBackendService(jig.GetServicePorts(false))).NotTo(HaveOccurred())
624621

625622
By(fmt.Sprintf("Scale backend replicas to %d", replicas))
626623
scale, err := f.ClientSet.AppsV1().Deployments(ns).GetScale(name, metav1.GetOptions{})
@@ -732,9 +729,7 @@ var _ = SIGDescribe("Loadbalancing: L7", func() {
732729
By("Create a basic HTTP ingress using NEG")
733730
jig.CreateIngress(filepath.Join(ingress.IngressManifestPath, "neg-exposed"), ns, map[string]string{}, map[string]string{})
734731
jig.WaitForIngress(true)
735-
usingNEG, err := gceController.BackendServiceUsingNEG(jig.GetServicePorts(false))
736-
Expect(err).NotTo(HaveOccurred())
737-
Expect(usingNEG).To(BeTrue())
732+
Expect(gceController.WaitForNegBackendService(jig.GetServicePorts(false))).NotTo(HaveOccurred())
738733
// initial replicas number is 1
739734
scaleAndValidateExposedNEG(1)
740735

@@ -1127,7 +1122,12 @@ func detectNegAnnotation(f *framework.Framework, jig *ingress.TestJig, gceContro
11271122

11281123
// if we expect no NEGs, then we should be using IGs
11291124
if negs == 0 {
1130-
return gceController.BackendServiceUsingIG(jig.GetServicePorts(false))
1125+
err := gceController.BackendServiceUsingIG(jig.GetServicePorts(false))
1126+
if err != nil {
1127+
framework.Logf("Failed to validate IG backend service: %v", err)
1128+
return false, nil
1129+
}
1130+
return true, nil
11311131
}
11321132

11331133
var status ingress.NegStatus
@@ -1160,7 +1160,12 @@ func detectNegAnnotation(f *framework.Framework, jig *ingress.TestJig, gceContro
11601160
}
11611161
}
11621162

1163-
return gceController.BackendServiceUsingNEG(jig.GetServicePorts(false))
1163+
err = gceController.BackendServiceUsingNEG(jig.GetServicePorts(false))
1164+
if err != nil {
1165+
framework.Logf("Failed to validate NEG backend service: %v", err)
1166+
return false, nil
1167+
}
1168+
return true, nil
11641169
}); err != nil {
11651170
Expect(err).NotTo(HaveOccurred())
11661171
}

0 commit comments

Comments
 (0)