End-to-end tests for canary affinity (#7529)

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**
 

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().
+			for i := 0; i < 50; i++ {
-				GET("/").
+				f.HTTPTestClient().
-				WithHeader("Host", host).
+					GET("/").
-				WithCookie("Canary-By-Cookie", "always").
+					WithHeader("Host", host).
-				Expect().
+					WithCookie("Canary-By-Cookie", "always").
-				Status(http.StatusOK).
+					Expect().
-				Body().Contains(canaryService)
+					Status(http.StatusOK).
+					Body().Contains(canaryService)
+			}
 
 			ginkgo.By("routing requests to the mainline upstream when cookie is set to 'never'")
-			f.HTTPTestClient().
+			for i := 0; i < 50; i++ {
-				GET("/").
+				f.HTTPTestClient().
-				WithHeader("Host", host).
+					GET("/").
-				WithCookie("Canary-By-Cookie", "never").
+					WithHeader("Host", host).
-				Expect().
+					WithCookie("Canary-By-Cookie", "never").
-				Status(http.StatusOK).
+					Expect().
-				Body().Contains(framework.EchoService).NotContains(canaryService)
+					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().
+			for i := 0; i < 50; i++ {
-				GET("/").
+				// This test relies on canary cookie not parsing into the valid
-				WithHeader("Host", host).
+				// affinity data and canary weight not being specified at all.
-				WithCookie("Canary-By-Cookie", "badcookievalue").
+				f.HTTPTestClient().
-				Expect().
+					GET("/").
-				Status(http.StatusOK).
+					WithHeader("Host", host).
-				Body().Contains(framework.EchoService).NotContains(canaryService)
+					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")
+		ginkgo.It("should route requests only to canary if canary weight is 100", func() {
+			host := "foo"
+			annotations := map[string]string{}
 
-			newAnnotations := map[string]string{
+			ing := framework.NewSingleIngress(host, "/", host,
-				"nginx.ingress.kubernetes.io/canary":        "true",
+				f.Namespace, framework.EchoService, 80, annotations)
-				"nginx.ingress.kubernetes.io/canary-weight": "100",
+			f.EnsureIngress(ing)
-			}
-
-			modIng := framework.NewSingleIngress(canaryIngName, "/", host,
-				f.Namespace, canaryService, 80, newAnnotations)
-
-			f.UpdateIngress(modIng)
 
 			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)
+}

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)
 		}
 	})
 })