make canary ingresses independent of the order they were applied

2018-12-05 15:12:55 -05:00 · 2018-12-05 15:12:55 -05:00 · d5840a4071
commit d5840a4071
parent 58057a2d25
2 changed files with 430 additions and 19 deletions
--- 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
--- a/test/e2e/annotations/canary.go
+++ b/test/e2e/annotations/canary.go
@ -43,8 +43,8 @@ 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{}

@ -70,17 +70,102 @@ var _ = framework.IngressNginxDescribe("Annotations - canary", func() {
 			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).ShouldNot(Equal(http.StatusNotFound))
-			Expect(body).Should(ContainSubstring("http-svc-canary"))
+			Expect(resp.StatusCode).Should(Equal(http.StatusNotFound))
 		})

-		It("should not route requests to the canary pod if header is set to 'never'", func() {
+		/*
+
+			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{}

@ -105,6 +190,122 @@ var _ = framework.IngressNginxDescribe("Annotations - canary", func() {

 			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{}
+
+			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)
+
+			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).
@ -112,13 +313,153 @@ 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"))
+			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 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"))
+		})
+	})
+
+	Context("when canaried by header", 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)
+
+			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 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).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"))
-		})

-		It("should not route requests to the canary pod if cookie is set to 'never'", func() {
+			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"))
+		})
+	})
+
+	// 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"))
+
 		})
 	})