diff --git a/internal/ingress/controller/controller.go b/internal/ingress/controller/controller.go index d02bee72b..6f5b46cf6 100644 --- a/internal/ingress/controller/controller.go +++ b/internal/ingress/controller/controller.go @@ -1199,22 +1199,27 @@ func mergeAlternativeBackends(ing *ingress.Ingress, upstreams map[string]*ingres altUps := upstreams[upsName] - merged := false + if altUps == nil { + klog.Warningf("alternative backend %s has already been removed", upsName) + } else { - for _, loc := range servers[defServerName].Locations { - priUps := upstreams[loc.Backend] + merged := false - if canMergeBackend(priUps, altUps) { - klog.V(2).Infof("matching backend %v found for alternative backend %v", - priUps.Name, altUps.Name) + for _, loc := range servers[defServerName].Locations { + priUps := upstreams[loc.Backend] - merged = mergeAlternativeBackend(priUps, altUps) + if canMergeBackend(priUps, altUps) { + klog.V(2).Infof("matching backend %v found for alternative backend %v", + priUps.Name, altUps.Name) + + merged = mergeAlternativeBackend(priUps, altUps) + } } - } - if !merged { - klog.Warningf("unable to find real backend for alternative backend %v. Deleting.", altUps.Name) - delete(upstreams, altUps.Name) + if !merged { + klog.Warningf("unable to find real backend for alternative backend %v. Deleting.", altUps.Name) + delete(upstreams, altUps.Name) + } } } @@ -1225,7 +1230,7 @@ func mergeAlternativeBackends(ing *ingress.Ingress, upstreams map[string]*ingres altUps := upstreams[upsName] if altUps == nil { - klog.Errorf("alternative backend %s has already be removed", upsName) + klog.Warningf("alternative backend %s has already been removed", upsName) continue } diff --git a/test/e2e/annotations/canary.go b/test/e2e/annotations/canary.go index bcab689be..7a9a237c9 100644 --- a/test/e2e/annotations/canary.go +++ b/test/e2e/annotations/canary.go @@ -759,27 +759,24 @@ var _ = framework.IngressNginxDescribe("Annotations - canary", func() { }) }) - Context("Single canary Ingress with multiple paths to same backend", func() { - It("should work", func() { - host := "foo" - canaryIngName := fmt.Sprintf("%v-canary", host) - annotations := map[string]string{ - "nginx.ingress.kubernetes.io/canary": "true", - "nginx.ingress.kubernetes.io/canary-by-header": "CanaryByHeader", - } + It("does not crash when canary ingress has multiple paths to the same non-matching backend", func() { + host := "foo" + canaryIngName := fmt.Sprintf("%v-canary", host) + annotations := map[string]string{ + "nginx.ingress.kubernetes.io/canary": "true", + "nginx.ingress.kubernetes.io/canary-by-header": "CanaryByHeader", + } - paths := []string{"/foo", "/bar"} + paths := []string{"/foo", "/bar"} + ing := framework.NewSingleIngressWithMultiplePaths(canaryIngName, paths, host, f.Namespace, "httpy-svc-canary", 80, &annotations) + f.EnsureIngress(ing) - ing := framework.NewSingleIngressWithMultiplePaths(canaryIngName, paths, host, f.Namespace, "httpy-svc-canary", 80, &annotations) - f.EnsureIngress(ing) + ing = framework.NewSingleIngress(host, "/", host, f.Namespace, "http-svc", 80, nil) + f.EnsureIngress(ing) - ing = framework.NewSingleIngress(host, "/", host, f.Namespace, "http-svc", 80, nil) - f.EnsureIngress(ing) - - f.WaitForNginxServer(host, - func(server string) bool { - return Expect(server).Should(ContainSubstring("server_name foo")) - }) - }) + f.WaitForNginxServer(host, + func(server string) bool { + return Expect(server).Should(ContainSubstring("server_name foo")) + }) }) })