From 8a40e82ffb710469797f9effff2908b1a0b6fad5 Mon Sep 17 00:00:00 2001 From: Per Bernhardt Date: Mon, 4 Mar 2019 11:20:54 +0000 Subject: [PATCH] Fix panic on multiple non-matching canary --- internal/ingress/controller/controller.go | 7 +++++- test/e2e/annotations/canary.go | 24 +++++++++++++++++++++ test/e2e/framework/framework.go | 26 +++++++++++++++++++++++ 3 files changed, 56 insertions(+), 1 deletion(-) diff --git a/internal/ingress/controller/controller.go b/internal/ingress/controller/controller.go index 728e8b3a6..d02bee72b 100644 --- a/internal/ingress/controller/controller.go +++ b/internal/ingress/controller/controller.go @@ -1162,7 +1162,7 @@ func nonCanaryIngressExists(ingresses []*ingress.Ingress, canaryIngresses []*ing // 2) primary name is not the default upstream // 3) the primary has a server func canMergeBackend(primary *ingress.Backend, alternative *ingress.Backend) bool { - return primary.Name != alternative.Name && primary.Name != defUpstreamName && !primary.NoServer + return alternative != nil && primary.Name != alternative.Name && primary.Name != defUpstreamName && !primary.NoServer } // Performs the merge action and checks to ensure that one two alternative backends do not merge into each other @@ -1224,6 +1224,11 @@ 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) + continue + } + merged := false server, ok := servers[rule.Host] diff --git a/test/e2e/annotations/canary.go b/test/e2e/annotations/canary.go index 1c9aa9bac..bcab689be 100644 --- a/test/e2e/annotations/canary.go +++ b/test/e2e/annotations/canary.go @@ -758,4 +758,28 @@ 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", + } + + paths := []string{"/foo", "/bar"} + + 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) + + f.WaitForNginxServer(host, + func(server string) bool { + return Expect(server).Should(ContainSubstring("server_name foo")) + }) + }) + }) }) diff --git a/test/e2e/framework/framework.go b/test/e2e/framework/framework.go index cbdb454ef..04fa62704 100644 --- a/test/e2e/framework/framework.go +++ b/test/e2e/framework/framework.go @@ -367,6 +367,32 @@ func NewSingleIngress(name, path, host, ns, service string, port int, annotation return newSingleIngressWithRules(name, path, host, ns, service, port, annotations, nil) } +// NewSingleIngressWithMultiplePaths creates a simple ingress rule with multiple paths +func NewSingleIngressWithMultiplePaths(name string, paths []string, host, ns, service string, port int, annotations *map[string]string) *extensions.Ingress { + spec := extensions.IngressSpec{ + Rules: []extensions.IngressRule{ + { + Host: host, + IngressRuleValue: extensions.IngressRuleValue{ + HTTP: &extensions.HTTPIngressRuleValue{}, + }, + }, + }, + } + + for _, path := range paths { + spec.Rules[0].IngressRuleValue.HTTP.Paths = append(spec.Rules[0].IngressRuleValue.HTTP.Paths, extensions.HTTPIngressPath{ + Path: path, + Backend: extensions.IngressBackend{ + ServiceName: service, + ServicePort: intstr.FromInt(port), + }, + }) + } + + return newSingleIngress(name, ns, annotations, spec) +} + func newSingleIngressWithRules(name, path, host, ns, service string, port int, annotations *map[string]string, tlsHosts []string) *extensions.Ingress { spec := extensions.IngressSpec{