From d5840a407137a3725dd61a9d2ba986a5a6021eb5 Mon Sep 17 00:00:00 2001 From: Conor Landry Date: Wed, 5 Dec 2018 15:12:55 -0500 Subject: [PATCH] make canary ingresses independent of the order they were applied --- internal/ingress/controller/controller.go | 23 +- test/e2e/annotations/canary.go | 422 +++++++++++++++++++++- 2 files changed, 428 insertions(+), 17 deletions(-) diff --git a/internal/ingress/controller/controller.go b/internal/ingress/controller/controller.go index c05e0af77f..11dd371760 100644 --- a/internal/ingress/controller/controller.go +++ b/internal/ingress/controller/controller.go @@ -389,6 +389,8 @@ func (n *NGINXController) getBackendServers(ingresses []*ingress.Ingress) ([]*in upstreams := n.createUpstreams(ingresses, du) servers := n.createServers(ingresses, upstreams, du) + var canaryIngresses []*ingress.Ingress + for _, ing := range ingresses { ingKey := k8s.MetaNamespaceKey(ing) anns := ing.ParsedAnnotations @@ -561,9 +563,15 @@ func (n *NGINXController) getBackendServers(ingresses []*ingress.Ingress) ([]*in } } + // set aside canary ingresses to merge later if anns.Canary.Enabled { - klog.Infof("Canary ingress %v detected. Finding eligible backends to merge into.", ing.Name) - mergeAlternativeBackends(ing, upstreams, servers) + canaryIngresses = append(canaryIngresses, ing) + } + } + + if nonCanaryIngressExists(ingresses, canaryIngresses) { + for _, canaryIng := range canaryIngresses { + mergeAlternativeBackends(canaryIng, upstreams, servers) } } @@ -1127,8 +1135,17 @@ func (n *NGINXController) createServers(data []*ingress.Ingress, return servers } +// OK to merge canary ingresses iff there exists one or more ingresses to potentially merge into +func nonCanaryIngressExists(ingresses []*ingress.Ingress, canaryIngresses []*ingress.Ingress) bool { + return len(ingresses)-len(canaryIngresses) > 0 +} + +// ensure that the following conditions are met +// 1) names of backends do not match and canary doesn't merge into itself +// 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.NoServer + return 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 diff --git a/test/e2e/annotations/canary.go b/test/e2e/annotations/canary.go index bb9502fc56..b92827454c 100644 --- a/test/e2e/annotations/canary.go +++ b/test/e2e/annotations/canary.go @@ -43,8 +43,230 @@ var _ = framework.IngressNginxDescribe("Annotations - canary", func() { f.NewDeployment("http-svc-canary", "gcr.io/kubernetes-e2e-test-images/echoserver:2.2", 8080, 1) }) - Context("when canaried by header", func() { - It("should route requests to the canary pod if header is set to 'always'", func() { + Context("when canary is created", func() { + It("should response with a 200 status from the mainline upstream when requests are made to the mainline ingress", func() { + host := "foo" + annotations := map[string]string{} + + ing := framework.NewSingleIngress(host, "/", host, f.IngressController.Namespace, "http-svc", 80, &annotations) + f.EnsureIngress(ing) + + f.WaitForNginxServer(host, + func(server string) bool { + return Expect(server).Should(ContainSubstring("server_name foo")) + }) + + canaryAnnotations := map[string]string{ + "nginx.ingress.kubernetes.io/canary": "true", + "nginx.ingress.kubernetes.io/canary-by-header": "CanaryByHeader", + } + + canaryIngName := fmt.Sprintf("%v-canary", host) + + canaryIng := framework.NewSingleIngress(canaryIngName, "/", host, f.IngressController.Namespace, "http-svc-canary", + 80, &canaryAnnotations) + f.EnsureIngress(canaryIng) + + time.Sleep(waitForLuaSync) + + resp, body, errs := gorequest.New(). + Get(f.IngressController.HTTPURL). + Set("Host", host). + End() + + Expect(errs).Should(BeEmpty()) + Expect(resp.StatusCode).Should(Equal(http.StatusOK)) + Expect(body).Should(ContainSubstring("http-svc")) + Expect(body).ShouldNot(ContainSubstring("http-svc-canary")) + }) + + It("should return 404 status for requests to the canary if no matching ingress is found", func() { + host := "foo" + + canaryAnnotations := map[string]string{ + "nginx.ingress.kubernetes.io/canary": "true", + "nginx.ingress.kubernetes.io/canary-by-header": "CanaryByHeader", + } + + canaryIngName := fmt.Sprintf("%v-canary", host) + + canaryIng := framework.NewSingleIngress(canaryIngName, "/", host, f.IngressController.Namespace, "http-svc-canary", + 80, &canaryAnnotations) + + f.EnsureIngress(canaryIng) + + time.Sleep(waitForLuaSync) + + resp, _, errs := gorequest.New(). + Get(f.IngressController.HTTPURL). + Set("Host", host). + Set("CanaryByHeader", "always"). + End() + + Expect(errs).Should(BeEmpty()) + Expect(resp.StatusCode).Should(Equal(http.StatusNotFound)) + }) + + /* + + TODO: This test needs improvements made to the e2e framework so that deployment updates work in order to successfully run + + It("should return the correct status codes when endpoints are unavailable", func() { + host := "foo" + annotations := map[string]string{} + + ing := framework.NewSingleIngress(host, "/", host, f.IngressController.Namespace, "http-svc", 80, &annotations) + f.EnsureIngress(ing) + + f.WaitForNginxServer(host, + func(server string) bool { + return Expect(server).Should(ContainSubstring("server_name foo")) + }) + + canaryAnnotations := map[string]string{ + "nginx.ingress.kubernetes.io/canary": "true", + "nginx.ingress.kubernetes.io/canary-by-header": "CanaryByHeader", + } + + canaryIngName := fmt.Sprintf("%v-canary", host) + + canaryIng := framework.NewSingleIngress(canaryIngName, "/", host, f.IngressController.Namespace, "http-svc-canary", + 80, &canaryAnnotations) + f.EnsureIngress(canaryIng) + + time.Sleep(waitForLuaSync) + + By("returning a 503 status when the mainline deployment has 0 replicas and a request is sent to the canary") + + f.NewEchoDeploymentWithReplicas(0) + + resp, _, errs := gorequest.New(). + Get(f.IngressController.HTTPURL). + Set("Host", host). + Set("CanaryByHeader", "always"). + End() + + Expect(errs).Should(BeEmpty()) + Expect(resp.StatusCode).Should(Equal(http.StatusServiceUnavailable)) + + By("returning a 200 status when the canary deployment has 0 replicas and a request is sent to the mainline ingress") + + f.NewEchoDeploymentWithReplicas(1) + f.NewDeployment("http-svc-canary", "gcr.io/kubernetes-e2e-test-images/echoserver:2.2", 8080, 0) + + resp, _, errs = gorequest.New(). + Get(f.IngressController.HTTPURL). + Set("Host", host). + Set("CanaryByHeader", "never"). + End() + + Expect(errs).Should(BeEmpty()) + Expect(resp.StatusCode).Should(Equal(http.StatusOK)) + }) + */ + + It("should route requests to the correct upstream if mainline ingress is created before the canary ingress", func() { + host := "foo" + annotations := map[string]string{} + + ing := framework.NewSingleIngress(host, "/", host, f.IngressController.Namespace, "http-svc", 80, &annotations) + f.EnsureIngress(ing) + + f.WaitForNginxServer(host, + func(server string) bool { + return Expect(server).Should(ContainSubstring("server_name foo")) + }) + + canaryAnnotations := map[string]string{ + "nginx.ingress.kubernetes.io/canary": "true", + "nginx.ingress.kubernetes.io/canary-by-header": "CanaryByHeader", + } + + canaryIngName := fmt.Sprintf("%v-canary", host) + + canaryIng := framework.NewSingleIngress(canaryIngName, "/", host, f.IngressController.Namespace, "http-svc-canary", + 80, &canaryAnnotations) + f.EnsureIngress(canaryIng) + + time.Sleep(waitForLuaSync) + + By("routing requests destined for the mainline ingress to the maineline upstream") + resp, body, errs := gorequest.New(). + Get(f.IngressController.HTTPURL). + Set("Host", host). + Set("CanaryByHeader", "never"). + End() + + Expect(errs).Should(BeEmpty()) + Expect(resp.StatusCode).Should(Equal(http.StatusOK)) + Expect(body).Should(ContainSubstring("http-svc")) + Expect(body).ShouldNot(ContainSubstring("http-svc-canary")) + + By("routing requests destined for the canary ingress to the canary upstream") + + resp, body, errs = gorequest.New(). + Get(f.IngressController.HTTPURL). + Set("Host", host). + Set("CanaryByHeader", "always"). + End() + + Expect(errs).Should(BeEmpty()) + Expect(resp.StatusCode).Should(Equal(http.StatusOK)) + Expect(body).Should(ContainSubstring("http-svc-canary")) + }) + + It("should route requests to the correct upstream if mainline ingress is created after the canary ingress", func() { + host := "foo" + + canaryAnnotations := map[string]string{ + "nginx.ingress.kubernetes.io/canary": "true", + "nginx.ingress.kubernetes.io/canary-by-header": "CanaryByHeader", + } + + canaryIngName := fmt.Sprintf("%v-canary", host) + + canaryIng := framework.NewSingleIngress(canaryIngName, "/", host, f.IngressController.Namespace, "http-svc-canary", + 80, &canaryAnnotations) + f.EnsureIngress(canaryIng) + + time.Sleep(waitForLuaSync) + + annotations := map[string]string{} + + ing := framework.NewSingleIngress(host, "/", host, f.IngressController.Namespace, "http-svc", 80, &annotations) + f.EnsureIngress(ing) + + f.WaitForNginxServer(host, + func(server string) bool { + return Expect(server).Should(ContainSubstring("server_name foo")) + }) + + By("routing requests destined for the mainline ingress to the mainelin upstream") + resp, body, errs := gorequest.New(). + Get(f.IngressController.HTTPURL). + Set("Host", host). + Set("CanaryByHeader", "never"). + End() + + Expect(errs).Should(BeEmpty()) + Expect(resp.StatusCode).Should(Equal(http.StatusOK)) + Expect(body).Should(ContainSubstring("http-svc")) + Expect(body).ShouldNot(ContainSubstring("http-svc-canary")) + + By("routing requests destined for the canary ingress to the canary upstream") + + resp, body, errs = gorequest.New(). + Get(f.IngressController.HTTPURL). + Set("Host", host). + Set("CanaryByHeader", "always"). + End() + + Expect(errs).Should(BeEmpty()) + Expect(resp.StatusCode).Should(Equal(http.StatusOK)) + Expect(body).Should(ContainSubstring("http-svc-canary")) + }) + + It("should route requests to the correct upstream if the mainline ingress is modified", func() { host := "foo" annotations := map[string]string{} @@ -69,18 +291,109 @@ var _ = framework.IngressNginxDescribe("Annotations - canary", func() { time.Sleep(waitForLuaSync) + modAnnotations := map[string]string{ + "foo": "bar", + } + + modIng := framework.NewSingleIngress(host, "/", host, f.IngressController.Namespace, "http-svc", 80, &modAnnotations) + + f.EnsureIngress(modIng) + + f.WaitForNginxServer(host, + func(server string) bool { + return Expect(server).Should(ContainSubstring("server_name foo")) + }) + + By("routing requests destined fro the mainline ingress to the mainline upstream") + resp, body, errs := gorequest.New(). + Get(f.IngressController.HTTPURL). + Set("Host", host). + Set("CanaryByHeader", "never"). + End() + + Expect(errs).Should(BeEmpty()) + Expect(resp.StatusCode).Should(Equal(http.StatusOK)) + Expect(body).Should(ContainSubstring("http-svc")) + Expect(body).ShouldNot(ContainSubstring("http-svc-canary")) + + By("routing requests destined for the canary ingress to the canary upstream") + + resp, body, errs = gorequest.New(). Get(f.IngressController.HTTPURL). Set("Host", host). Set("CanaryByHeader", "always"). End() Expect(errs).Should(BeEmpty()) - Expect(resp.StatusCode).ShouldNot(Equal(http.StatusNotFound)) + Expect(resp.StatusCode).Should(Equal(http.StatusOK)) + Expect(body).Should(ContainSubstring("http-svc-canary")) + }) + + It("should route requests to the correct upstream if the canary ingress is modified", func() { + host := "foo" + annotations := map[string]string{} + + ing := framework.NewSingleIngress(host, "/", host, f.IngressController.Namespace, "http-svc", 80, &annotations) + f.EnsureIngress(ing) + + f.WaitForNginxServer(host, + func(server string) bool { + return Expect(server).Should(ContainSubstring("server_name foo")) + }) + + canaryAnnotations := map[string]string{ + "nginx.ingress.kubernetes.io/canary": "true", + "nginx.ingress.kubernetes.io/canary-by-header": "CanaryByHeader", + } + + canaryIngName := fmt.Sprintf("%v-canary", host) + + canaryIng := framework.NewSingleIngress(canaryIngName, "/", host, f.IngressController.Namespace, "http-svc-canary", + 80, &canaryAnnotations) + f.EnsureIngress(canaryIng) + + time.Sleep(waitForLuaSync) + + modCanaryAnnotations := map[string]string{ + "nginx.ingress.kubernetes.io/canary": "true", + "nginx.ingress.kubernetes.io/canary-by-header": "CanaryByHeader2", + } + + modCanaryIng := framework.NewSingleIngress(canaryIngName, "/", host, f.IngressController.Namespace, "http-svc-canary", 80, &modCanaryAnnotations) + f.EnsureIngress(modCanaryIng) + + time.Sleep(waitForLuaSync) + + By("routing requests destined for the mainline ingress to the mainline upstream") + + resp, body, errs := gorequest.New(). + Get(f.IngressController.HTTPURL). + Set("Host", host). + Set("CanaryByHeader2", "never"). + End() + + Expect(errs).Should(BeEmpty()) + Expect(resp.StatusCode).Should(Equal(http.StatusOK)) + Expect(body).Should(ContainSubstring("http-svc")) + Expect(body).ShouldNot(ContainSubstring("http-svc-canary")) + + By("routing requests destined for the canary ingress to the canary upstream") + + resp, body, errs = gorequest.New(). + Get(f.IngressController.HTTPURL). + Set("Host", host). + Set("CanaryByHeader2", "always"). + End() + + Expect(errs).Should(BeEmpty()) + Expect(resp.StatusCode).Should(Equal(http.StatusOK)) Expect(body).Should(ContainSubstring("http-svc-canary")) }) + }) - It("should not route requests to the canary pod if header is set to 'never'", func() { + Context("when canaried by header", func() { + It("should route requests to the correct upstream", func() { host := "foo" annotations := map[string]string{} @@ -105,20 +418,48 @@ var _ = framework.IngressNginxDescribe("Annotations - canary", func() { time.Sleep(waitForLuaSync) + By("routing requests to the canary upstream when header is set to 'always'") + resp, body, errs := gorequest.New(). + Get(f.IngressController.HTTPURL). + Set("Host", host). + Set("CanaryByHeader", "always"). + End() + + Expect(errs).Should(BeEmpty()) + Expect(resp.StatusCode).Should(Equal(http.StatusOK)) + Expect(body).Should(ContainSubstring("http-svc-canary")) + + By("routing requests to the mainline upstream when header is set to 'never'") + + resp, body, errs = gorequest.New(). Get(f.IngressController.HTTPURL). Set("Host", host). Set("CanaryByHeader", "never"). End() Expect(errs).Should(BeEmpty()) - Expect(resp.StatusCode).ShouldNot(Equal(http.StatusNotFound)) + Expect(resp.StatusCode).Should(Equal(http.StatusOK)) + Expect(body).Should(ContainSubstring("http-svc")) + Expect(body).ShouldNot(ContainSubstring("http-svc-canary")) + + By("routing requests to the mainline upstream when header is set to anything else") + + resp, body, errs = gorequest.New(). + Get(f.IngressController.HTTPURL). + Set("Host", host). + Set("CanaryByHeader", "badheadervalue"). + End() + + Expect(errs).Should(BeEmpty()) + Expect(resp.StatusCode).Should(Equal(http.StatusOK)) + Expect(body).Should(ContainSubstring("http-svc")) Expect(body).ShouldNot(ContainSubstring("http-svc-canary")) }) }) Context("when canaried by cookie", func() { - It("should route requests to the canary pod if cookie is set to 'always'", func() { + It("should route requests to the correct upstream", func() { host := "foo" annotations := map[string]string{} @@ -143,6 +484,7 @@ var _ = framework.IngressNginxDescribe("Annotations - canary", func() { time.Sleep(waitForLuaSync) + By("routing requests to the canary upstream when cookie is set to 'always'") resp, body, errs := gorequest.New(). Get(f.IngressController.HTTPURL). Set("Host", host). @@ -150,27 +492,54 @@ var _ = framework.IngressNginxDescribe("Annotations - canary", func() { End() Expect(errs).Should(BeEmpty()) - Expect(resp.StatusCode).ShouldNot(Equal(http.StatusNotFound)) + Expect(resp.StatusCode).Should(Equal(http.StatusOK)) Expect(body).Should(ContainSubstring("http-svc-canary")) + + By("routing requests to the mainline upstream when cookie is set to 'never'") + + resp, body, errs = gorequest.New(). + Get(f.IngressController.HTTPURL). + Set("Host", host). + AddCookie(&http.Cookie{Name: "CanaryByCookie", Value: "never"}). + End() + + Expect(errs).Should(BeEmpty()) + Expect(resp.StatusCode).Should(Equal(http.StatusOK)) + Expect(body).Should(ContainSubstring("http-svc")) + Expect(body).ShouldNot(ContainSubstring("http-svc-canary")) + + By("routing requests to the mainline upstream when cookie is set to anything else") + + resp, body, errs = gorequest.New(). + Get(f.IngressController.HTTPURL). + Set("Host", host). + AddCookie(&http.Cookie{Name: "CanaryByCookie", Value: "badcookievalue"}). + End() + + Expect(errs).Should(BeEmpty()) + Expect(resp.StatusCode).Should(Equal(http.StatusOK)) + Expect(body).Should(ContainSubstring("http-svc")) + Expect(body).ShouldNot(ContainSubstring("http-svc-canary")) }) + }) - It("should not route requests to the canary pod if cookie is set to 'never'", func() { + // TODO: add testing for canary-weight 0 < weight < 100 + Context("when canaried by weight", func() { + It("should route requests to the correct upstream", func() { host := "foo" annotations := map[string]string{} ing := framework.NewSingleIngress(host, "/", host, f.IngressController.Namespace, "http-svc", 80, &annotations) f.EnsureIngress(ing) - Expect(ing).NotTo(BeNil()) - f.WaitForNginxServer(host, func(server string) bool { return Expect(server).Should(ContainSubstring("server_name foo")) }) canaryAnnotations := map[string]string{ - "nginx.ingress.kubernetes.io/canary": "true", - "nginx.ingress.kubernetes.io/canary-by-header": "CanaryByHeader", + "nginx.ingress.kubernetes.io/canary": "true", + "nginx.ingress.kubernetes.io/canary-weight": "0", } canaryIngName := fmt.Sprintf("%v-canary", host) @@ -181,15 +550,40 @@ var _ = framework.IngressNginxDescribe("Annotations - canary", func() { time.Sleep(waitForLuaSync) + By("returning requests from the mainline only when weight is equal to 0") + resp, body, errs := gorequest.New(). Get(f.IngressController.HTTPURL). Set("Host", host). - AddCookie(&http.Cookie{Name: "CanaryByCookie", Value: "never"}). End() Expect(errs).Should(BeEmpty()) - Expect(resp.StatusCode).ShouldNot(Equal(http.StatusNotFound)) + Expect(resp.StatusCode).Should(Equal(http.StatusOK)) + Expect(body).Should(ContainSubstring("http-svc")) Expect(body).ShouldNot(ContainSubstring("http-svc-canary")) + + By("returning requests from the canary only when weight is equal to 100") + + modCanaryAnnotations := map[string]string{ + "nginx.ingress.kubernetes.io/canary": "true", + "nginx.ingress.kubernetes.io/canary-weight": "100", + } + + modCanaryIng := framework.NewSingleIngress(canaryIngName, "/", host, f.IngressController.Namespace, "http-svc-canary", 80, &modCanaryAnnotations) + + f.EnsureIngress(modCanaryIng) + + time.Sleep(waitForLuaSync) + + resp, body, errs = gorequest.New(). + Get(f.IngressController.HTTPURL). + Set("Host", host). + End() + + Expect(errs).Should(BeEmpty()) + Expect(resp.StatusCode).Should(Equal(http.StatusOK)) + Expect(body).Should(ContainSubstring("http-svc-canary")) + }) })