diff --git a/docs/user-guide/nginx-configuration/annotations.md b/docs/user-guide/nginx-configuration/annotations.md index 421065b118..67bc10c58f 100755 --- a/docs/user-guide/nginx-configuration/annotations.md +++ b/docs/user-guide/nginx-configuration/annotations.md @@ -141,7 +141,7 @@ In some cases, you may want to "canary" a new set of changes by sending a small Canary rules are evaluated in order of precedence. Precedence is as follows: `canary-by-header -> canary-by-cookie -> canary-weight` -**Note** that when you mark an ingress as canary, then all the other non-canary annotations will be ignored (inherited from the corresponding main ingress) except `nginx.ingress.kubernetes.io/load-balance`, `nginx.ingress.kubernetes.io/upstream-hash-by`, and [annotations related to session affinity](#session-affinity). If you want to restore the original behavior of canaries when session affinity was ignored, set `nginx.ingress.kubernetes.io/affinity-canary-behavior` annotation with value `legacy` on the non-canary ingress definition. +**Note** that when you mark an ingress as canary, then all the other non-canary annotations will be ignored (inherited from the corresponding main ingress) except `nginx.ingress.kubernetes.io/load-balance`, `nginx.ingress.kubernetes.io/upstream-hash-by`, and [annotations related to session affinity](#session-affinity). If you want to restore the original behavior of canaries when session affinity was ignored, set `nginx.ingress.kubernetes.io/affinity-canary-behavior` annotation with value `legacy` on the canary ingress definition. **Known Limitations** diff --git a/test/e2e/annotations/canary.go b/test/e2e/annotations/canary.go index fe3e1544f6..07b307abb1 100644 --- a/test/e2e/annotations/canary.go +++ b/test/e2e/annotations/canary.go @@ -18,10 +18,14 @@ package annotations import ( "fmt" + "math" "net/http" + "reflect" + "regexp" "strings" "github.com/onsi/ginkgo" + "github.com/stretchr/testify/assert" "k8s.io/ingress-nginx/test/e2e/framework" ) @@ -30,12 +34,12 @@ const ( canaryService = "echo-canary" ) -var _ = framework.DescribeAnnotation("canary", func() { +var _ = framework.DescribeAnnotation("canary-*", func() { f := framework.NewDefaultFramework("canary") ginkgo.BeforeEach(func() { // Deployment for main backend - f.NewEchoDeployment() + f.NewEchoDeploymentWithReplicas(1) // Deployment for canary backend f.NewEchoDeploymentWithNameAndReplicas(canaryService, 1) @@ -637,7 +641,7 @@ var _ = framework.DescribeAnnotation("canary", func() { }) ginkgo.Context("when canaried by cookie", func() { - ginkgo.It("should route requests to the correct upstream", func() { + ginkgo.It("respects always and never values", func() { host := "foo" annotations := map[string]string{} @@ -662,37 +666,44 @@ var _ = framework.DescribeAnnotation("canary", func() { f.EnsureIngress(canaryIng) ginkgo.By("routing requests to the canary upstream when cookie is set to 'always'") - f.HTTPTestClient(). - GET("/"). - WithHeader("Host", host). - WithCookie("Canary-By-Cookie", "always"). - Expect(). - Status(http.StatusOK). - Body().Contains(canaryService) + for i := 0; i < 50; i++ { + f.HTTPTestClient(). + GET("/"). + WithHeader("Host", host). + WithCookie("Canary-By-Cookie", "always"). + Expect(). + Status(http.StatusOK). + Body().Contains(canaryService) + } ginkgo.By("routing requests to the mainline upstream when cookie is set to 'never'") - f.HTTPTestClient(). - GET("/"). - WithHeader("Host", host). - WithCookie("Canary-By-Cookie", "never"). - Expect(). - Status(http.StatusOK). - Body().Contains(framework.EchoService).NotContains(canaryService) + for i := 0; i < 50; i++ { + f.HTTPTestClient(). + GET("/"). + WithHeader("Host", host). + WithCookie("Canary-By-Cookie", "never"). + Expect(). + Status(http.StatusOK). + Body().Contains(framework.EchoService).NotContains(canaryService) + } ginkgo.By("routing requests to the mainline upstream when cookie is set to anything else") - f.HTTPTestClient(). - GET("/"). - WithHeader("Host", host). - WithCookie("Canary-By-Cookie", "badcookievalue"). - Expect(). - Status(http.StatusOK). - Body().Contains(framework.EchoService).NotContains(canaryService) + for i := 0; i < 50; i++ { + // This test relies on canary cookie not parsing into the valid + // affinity data and canary weight not being specified at all. + f.HTTPTestClient(). + GET("/"). + WithHeader("Host", host). + WithCookie("Canary-By-Cookie", "badcookievalue"). + Expect(). + Status(http.StatusOK). + Body().Contains(framework.EchoService).NotContains(canaryService) + } }) }) - // TODO: add testing for canary-weight 0 < weight < 100 ginkgo.Context("when canaried by weight", func() { - ginkgo.It("should route requests to the correct upstream", func() { + ginkgo.It("should route requests only to mainline if canary weight is 0", func() { host := "foo" annotations := map[string]string{} @@ -720,7 +731,6 @@ var _ = framework.DescribeAnnotation("canary", func() { return strings.Contains(server, "server_name foo") }) - ginkgo.By("returning requests from the mainline only when weight is equal to 0") f.HTTPTestClient(). GET("/"). WithHeader("Host", host). @@ -729,24 +739,31 @@ var _ = framework.DescribeAnnotation("canary", func() { Body(). Contains(framework.EchoService). NotContains(canaryService) + }) - ginkgo.By("returning requests from the canary only when weight is equal to 100") - - newAnnotations := map[string]string{ - "nginx.ingress.kubernetes.io/canary": "true", - "nginx.ingress.kubernetes.io/canary-weight": "100", - } - - modIng := framework.NewSingleIngress(canaryIngName, "/", host, - f.Namespace, canaryService, 80, newAnnotations) + ginkgo.It("should route requests only to canary if canary weight is 100", func() { + host := "foo" + annotations := map[string]string{} - f.UpdateIngress(modIng) + ing := framework.NewSingleIngress(host, "/", host, + f.Namespace, framework.EchoService, 80, annotations) + f.EnsureIngress(ing) f.WaitForNginxServer(host, func(server string) bool { return strings.Contains(server, "server_name foo") }) + canaryIngName := fmt.Sprintf("%v-canary", host) + canaryAnnotations := map[string]string{ + "nginx.ingress.kubernetes.io/canary": "true", + "nginx.ingress.kubernetes.io/canary-weight": "100", + } + + canaryIng := framework.NewSingleIngress(canaryIngName, "/", host, + f.Namespace, canaryService, 80, canaryAnnotations) + f.EnsureIngress(canaryIng) + f.HTTPTestClient(). GET("/"). WithHeader("Host", host). @@ -755,6 +772,32 @@ var _ = framework.DescribeAnnotation("canary", func() { Body(). Contains(canaryService) }) + + ginkgo.It("should route requests evenly split between mainline and canary if canary weight is 50", func() { + host := "foo" + annotations := map[string]string{} + + ing := framework.NewSingleIngress(host, "/", host, + f.Namespace, framework.EchoService, 80, annotations) + f.EnsureIngress(ing) + + f.WaitForNginxServer(host, + func(server string) bool { + return strings.Contains(server, "server_name foo") + }) + + canaryIngName := fmt.Sprintf("%v-canary", host) + canaryAnnotations := map[string]string{ + "nginx.ingress.kubernetes.io/canary": "true", + "nginx.ingress.kubernetes.io/canary-weight": "50", + } + + canaryIng := framework.NewSingleIngress(canaryIngName, "/", host, + f.Namespace, canaryService, 80, canaryAnnotations) + f.EnsureIngress(canaryIng) + + TestEvenMainlineCanaryDistribution(f, host) + }) }) ginkgo.Context("Single canary Ingress", func() { @@ -832,4 +875,211 @@ var _ = framework.DescribeAnnotation("canary", func() { return strings.Contains(server, "server_name foo") }) }) + + ginkgo.Context("canary affinity behavior", func() { + host := "foo" + affinityCookieName := "aff" + canaryIngName := fmt.Sprintf("%v-canary", host) + + ginkgo.It("always routes traffic to canary if first request was affinitized to canary (default behavior)", func() { + annotations := map[string]string{ + "nginx.ingress.kubernetes.io/affinity": "cookie", + "nginx.ingress.kubernetes.io/session-cookie-name": affinityCookieName, + } + + ing := framework.NewSingleIngress(host, "/", host, + f.Namespace, framework.EchoService, 80, annotations) + f.EnsureIngress(ing) + + f.WaitForNginxServer(host, + func(server string) bool { + return strings.Contains(server, "server_name foo") + }) + + // Canary weight is 1% to ensure affinity cookie does its job. + // affinity-canary-behavior annotation is not explicitly configured. + canaryAnnotations := map[string]string{ + "nginx.ingress.kubernetes.io/canary": "true", + "nginx.ingress.kubernetes.io/canary-by-header": "ForceCanary", + "nginx.ingress.kubernetes.io/canary-by-header-value": "yes", + "nginx.ingress.kubernetes.io/canary-weight": "1", + } + + canaryIng := framework.NewSingleIngress(canaryIngName, "/", host, + f.Namespace, canaryService, 80, canaryAnnotations) + f.EnsureIngress(canaryIng) + + // This request will produce affinity cookie coming from the canary + // backend. + forcedRequestToCanary := f.HTTPTestClient(). + GET("/"). + WithHeader("Host", host). + WithHeader("ForceCanary", "yes"). + Expect(). + Status(http.StatusOK) + + // Make sure we got response from canary. + forcedRequestToCanary. + Body().Contains(canaryService) + + affinityCookie := forcedRequestToCanary. + Cookie(affinityCookieName) + + // As long as affinity cookie is present, all requests will be + // routed to a specific backend. + for i := 0; i < 50; i++ { + f.HTTPTestClient(). + GET("/"). + WithHeader("Host", host). + WithCookie(affinityCookieName, affinityCookie.Raw().Value). + Expect(). + Status(http.StatusOK). + Body().Contains(canaryService) + } + }) + + ginkgo.It("always routes traffic to canary if first request was affinitized to canary (explicit sticky behavior)", func() { + annotations := map[string]string{ + "nginx.ingress.kubernetes.io/affinity": "cookie", + "nginx.ingress.kubernetes.io/session-cookie-name": affinityCookieName, + } + + ing := framework.NewSingleIngress(host, "/", host, + f.Namespace, framework.EchoService, 80, annotations) + f.EnsureIngress(ing) + + f.WaitForNginxServer(host, + func(server string) bool { + return strings.Contains(server, "server_name foo") + }) + + // Canary weight is 1% to ensure affinity cookie does its job. + // Explicitly set affinity-canary-behavior annotation to "sticky". + canaryAnnotations := map[string]string{ + "nginx.ingress.kubernetes.io/canary": "true", + "nginx.ingress.kubernetes.io/canary-by-header": "ForceCanary", + "nginx.ingress.kubernetes.io/canary-by-header-value": "yes", + "nginx.ingress.kubernetes.io/canary-weight": "1", + "nginx.ingress.kubernetes.io/affinity-canary-behavior": "sticky", + } + + canaryIng := framework.NewSingleIngress(canaryIngName, "/", host, + f.Namespace, canaryService, 80, canaryAnnotations) + f.EnsureIngress(canaryIng) + + // This request will produce affinity cookie coming from the canary + // backend. + forcedRequestToCanary := f.HTTPTestClient(). + GET("/"). + WithHeader("Host", host). + WithHeader("ForceCanary", "yes"). + Expect(). + Status(http.StatusOK) + + // Make sure we got response from canary. + forcedRequestToCanary. + Body().Contains(canaryService) + + affinityCookie := forcedRequestToCanary. + Cookie(affinityCookieName) + + // As long as affinity cookie is present, all requests will be + // routed to a specific backend. + for i := 0; i < 50; i++ { + f.HTTPTestClient(). + GET("/"). + WithHeader("Host", host). + WithCookie(affinityCookieName, affinityCookie.Raw().Value). + Expect(). + Status(http.StatusOK). + Body().Contains(canaryService) + } + }) + + ginkgo.It("routes traffic to either mainline or canary backend (legacy behavior)", func() { + annotations := map[string]string{ + "nginx.ingress.kubernetes.io/affinity": "cookie", + "nginx.ingress.kubernetes.io/session-cookie-name": affinityCookieName, + } + + ing := framework.NewSingleIngress(host, "/", host, + f.Namespace, framework.EchoService, 80, annotations) + f.EnsureIngress(ing) + + f.WaitForNginxServer(host, + func(server string) bool { + return strings.Contains(server, "server_name foo") + }) + + // Canary weight is 50% to ensure requests are going there. + // Explicitly set affinity-canary-behavior annotation to "legacy". + canaryAnnotations := map[string]string{ + "nginx.ingress.kubernetes.io/canary": "true", + "nginx.ingress.kubernetes.io/canary-by-header": "ForceCanary", + "nginx.ingress.kubernetes.io/canary-by-header-value": "yes", + "nginx.ingress.kubernetes.io/canary-weight": "50", + "nginx.ingress.kubernetes.io/affinity-canary-behavior": "legacy", + } + + canaryIng := framework.NewSingleIngress(canaryIngName, "/", host, + f.Namespace, canaryService, 80, canaryAnnotations) + f.EnsureIngress(canaryIng) + + // This request will produce affinity cookie coming from the canary + // backend. + forcedRequestToCanary := f.HTTPTestClient(). + GET("/"). + WithHeader("Host", host). + WithHeader("ForceCanary", "yes"). + Expect(). + Status(http.StatusOK) + + // Make sure we got response from canary. + forcedRequestToCanary. + Body().Contains(canaryService) + + // Legacy behavior results in affinity cookie not being set in + // response. + for _, c := range forcedRequestToCanary.Cookies().Iter() { + if c.String().Raw() == affinityCookieName { + ginkgo.GinkgoT().Error("Affinity cookie is present in response, but was not expected.") + } + } + + TestEvenMainlineCanaryDistribution(f, host) + }) + }) + }) + +// This method assumes canary weight being configured at 50%. +func TestEvenMainlineCanaryDistribution(f *framework.Framework, host string) { + re := regexp.MustCompile(fmt.Sprintf(`%s.*`, framework.EchoService)) + replicaRequestCount := map[string]int{} + + for i := 0; i < 200; i++ { + body := f.HTTPTestClient(). + GET("/"). + WithHeader("Host", host). + Expect(). + Status(http.StatusOK).Body().Raw() + + replica := re.FindString(body) + assert.NotEmpty(ginkgo.GinkgoT(), replica) + + if _, ok := replicaRequestCount[replica]; !ok { + replicaRequestCount[replica] = 1 + } else { + replicaRequestCount[replica]++ + } + } + + keys := reflect.ValueOf(replicaRequestCount).MapKeys() + + assert.Equal(ginkgo.GinkgoT(), 2, len(keys)) + + // The implmentation of choice by weight doesn't guarantee exact + // number of requests, so verify if request imbalance is within an + // acceptable range. + assert.LessOrEqual(ginkgo.GinkgoT(), math.Abs(float64(replicaRequestCount[keys[0].String()]-replicaRequestCount[keys[1].String()]))/math.Max(float64(replicaRequestCount[keys[0].String()]), float64(replicaRequestCount[keys[1].String()])), 0.2) +} diff --git a/test/e2e/loadbalance/round_robin.go b/test/e2e/loadbalance/round_robin.go index 9e37d1596f..f035005dd2 100644 --- a/test/e2e/loadbalance/round_robin.go +++ b/test/e2e/loadbalance/round_robin.go @@ -66,7 +66,7 @@ var _ = framework.DescribeSetting("[Load Balancer] round-robin", func() { } for _, v := range replicaRequestCount { - assert.Equal(ginkgo.GinkgoT(), v, 200) + assert.Equal(ginkgo.GinkgoT(), 200, v) } }) })