Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add canary-by-cookie-value annotation #4200

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 3 additions & 0 deletions docs/user-guide/nginx-configuration/annotations.md
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ You can add these Kubernetes annotations to specific Ingress objects to customiz
|[nginx.ingress.kubernetes.io/canary-by-header](#canary)|string|
|[nginx.ingress.kubernetes.io/canary-by-header-value](#canary)|string
|[nginx.ingress.kubernetes.io/canary-by-cookie](#canary)|string|
|[nginx.ingress.kubernetes.io/canary-by-cookie-value](#canary)|string
|[nginx.ingress.kubernetes.io/canary-weight](#canary)|number|
|[nginx.ingress.kubernetes.io/client-body-buffer-size](#client-body-buffer-size)|string|
|[nginx.ingress.kubernetes.io/configuration-snippet](#configuration-snippet)|string|
Expand Down Expand Up @@ -118,6 +119,8 @@ In some cases, you may want to "canary" a new set of changes by sending a small

* `nginx.ingress.kubernetes.io/canary-by-cookie`: The cookie to use for notifying the Ingress to route the request to the service specified in the Canary Ingress. When the cookie value is set to `always`, it will be routed to the canary. When the cookie is set to `never`, it will never be routed to the canary. For any other value, the cookie will be ignored and the request compared against the other canary rules by precedence.

* `nginx.ingress.kubernetes.io/canary-by-cookie-value`: The cookie value (it is [regular expression of Lua](https://www.lua.org/pil/20.2.html)) to match for notifying the Ingress to route the request to the service specified in the Canary Ingress. When the cookie value is set to this value, it will be routed to the canary. If cookie value doesn't match as a regular expression, the cookie will be ignored and the request compared against the other canary rules by precedence. This annotation has to be used together with . The annotation is an extension of the `nginx.ingress.kubernetes.io/canary-by-cookie` to allow customizing the cookie value instead of using hardcoded values. It doesn't have any effect if the `nginx.ingress.kubernetes.io/canary-by-cookie` annotation is not defined.

* `nginx.ingress.kubernetes.io/canary-weight`: The integer based (0 - 100) percent of random requests that should be routed to the service specified in the canary Ingress. A weight of 0 implies that no requests will be sent to the service in the Canary ingress by this canary rule. A weight of 100 means implies all requests will be sent to the alternative service specified in the Ingress.

Canary rules are evaluated in order of precedence. Precedence is as follows:
Expand Down
8 changes: 7 additions & 1 deletion internal/ingress/annotations/canary/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ type Config struct {
Header string
HeaderValue string
Cookie string
CookieValue string
}

// NewParser parses the ingress for canary related annotations
Expand Down Expand Up @@ -73,7 +74,12 @@ func (c canary) Parse(ing *networking.Ingress) (interface{}, error) {
config.Cookie = ""
}

if !config.Enabled && (config.Weight > 0 || len(config.Header) > 0 || len(config.HeaderValue) > 0 || len(config.Cookie) > 0) {
config.CookieValue, err = parser.GetStringAnnotation("canary-by-cookie-value", ing)
if err != nil {
config.CookieValue = ""
}

if !config.Enabled && (config.Weight > 0 || len(config.Header) > 0 || len(config.HeaderValue) > 0 || len(config.Cookie) > 0 || len(config.CookieValue) > 0) {
return nil, errors.NewInvalidAnnotationConfiguration("canary", "configured but not enabled")
}

Expand Down
2 changes: 2 additions & 0 deletions internal/ingress/controller/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -707,6 +707,7 @@ func (n *NGINXController) createUpstreams(data []*ingress.Ingress, du *ingress.B
Header: anns.Canary.Header,
HeaderValue: anns.Canary.HeaderValue,
Cookie: anns.Canary.Cookie,
CookieValue: anns.Canary.CookieValue,
}
}

Expand Down Expand Up @@ -772,6 +773,7 @@ func (n *NGINXController) createUpstreams(data []*ingress.Ingress, du *ingress.B
Header: anns.Canary.Header,
HeaderValue: anns.Canary.HeaderValue,
Cookie: anns.Canary.Cookie,
CookieValue: anns.Canary.CookieValue,
}
}

Expand Down
2 changes: 2 additions & 0 deletions internal/ingress/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,8 @@ type TrafficShapingPolicy struct {
HeaderValue string `json:"headerValue"`
// Cookie on which to redirect requests to this backend
Cookie string `json:"cookie"`
// CookieValue (regular expression of Lua) on which to redirect requests to this backend
CookieValue string `json:"cookieValue"`
}

// HashInclude defines if a field should be used or not to calculate the hash
Expand Down
3 changes: 3 additions & 0 deletions internal/ingress/types_equals.go
Original file line number Diff line number Diff line change
Expand Up @@ -248,6 +248,9 @@ func (tsp1 TrafficShapingPolicy) Equal(tsp2 TrafficShapingPolicy) bool {
if tsp1.Cookie != tsp2.Cookie {
return false
}
if tsp1.CookieValue != tsp2.CookieValue {
return false
}

return true
}
Expand Down
6 changes: 5 additions & 1 deletion rootfs/etc/nginx/lua/balancer.lua
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,11 @@ local function route_to_alternative_balancer(balancer)
local target_cookie = traffic_shaping_policy.cookie
local cookie = ngx.var["cookie_" .. target_cookie]
if cookie then
if cookie == "always" then
if traffic_shaping_policy.cookieValue and #traffic_shaping_policy.cookieValue > 0 then
if cookie:find(traffic_shaping_policy.cookieValue) ~= nil then
return true
end
elseif cookie == "always" then
return true
elseif cookie == "never" then
return false
Expand Down
56 changes: 50 additions & 6 deletions rootfs/etc/nginx/lua/test/balancer_test.lua
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,8 @@ local function reset_backends()
weight = 0,
header = "",
headerValue = "",
cookie = ""
cookie = "",
cookieValue = "",
},
},
{ name = "my-dummy-app-1", ["load-balance"] = "round_robin", },
Expand Down Expand Up @@ -135,34 +136,77 @@ describe("Balancer", function()
context("canary by cookie", function()
it("returns correct result for given cookies", function()
backend.trafficShapingPolicy.cookie = "canaryCookie"
balancer.sync_backend(backend)
local test_patterns = {
-- with no cookie value setting
{
case_title = "cookie_value is 'always'",
case_title = "requested cookie value is 'always'",
cookie_value = "",
request_cookie_name = "canaryCookie",
request_cookie_value = "always",
expected_result = true,
},
{
case_title = "cookie_value is 'never'",
case_title = "requested cookie value is 'never'",
cookie_value = "",
request_cookie_name = "canaryCookie",
request_cookie_value = "never",
expected_result = false,
},
{
case_title = "cookie_value is undefined",
case_title = "requested cookie value is undefined",
cookie_value = "",
request_cookie_name = "canaryCookie",
request_cookie_value = "foo",
expected_result = false,
},
{
case_title = "cookie_name is undefined",
case_title = "requested cookie name is undefined",
cookie_value = "",
request_cookie_name = "foo",
request_cookie_value = "always",
expected_result = false
},
-- with cookie value setting
{
case_title = "cookie value is set and requested cookie value is 'always'",
cookie_value = "foo",
request_cookie_name = "canaryCookie",
request_cookie_value = "always",
expected_result = false,
},
{
case_title = "cookie value is set and requested cookie name is undefined",
cookie_value = "foo",
request_cookie_name = "bar",
request_cookie_value = "foo",
expected_result = false,
},
{
case_title = "cookie value is set and requested cookie value exactly matches",
cookie_value = "foo",
request_cookie_name = "canaryCookie",
request_cookie_value = "foo",
expected_result = true,
},
{
case_title = "cookie value is set and requested cookie value matches as regular expression",
cookie_value = "foo",
request_cookie_name = "canaryCookie",
request_cookie_value = "barfoobar",
expected_result = true,
},
{
case_title = "regular expression is set as cookie value and requested cookie value matches",
cookie_value = "^foo%d[0-1]$",
request_cookie_name = "canaryCookie",
request_cookie_value = "foo91",
expected_result = true,
},
}
for _, test_pattern in pairs(test_patterns) do
reset_balancer()
backend.trafficShapingPolicy.cookieValue = test_pattern.cookie_value
balancer.sync_backend(backend)
mock_ngx({ var = {
["cookie_" .. test_pattern.request_cookie_name] = test_pattern.request_cookie_value,
request_uri = "/"
Expand Down
80 changes: 79 additions & 1 deletion test/e2e/annotations/canary.go
Original file line number Diff line number Diff line change
Expand Up @@ -580,7 +580,7 @@ var _ = framework.IngressNginxDescribe("Annotations - canary", func() {
})
})

Context("when canaried by cookie", func() {
Context("when canaried by cookie with no value", func() {
It("should route requests to the correct upstream", func() {
host := "foo"
annotations := map[string]string{}
Expand Down Expand Up @@ -645,6 +645,84 @@ var _ = framework.IngressNginxDescribe("Annotations - canary", func() {
})
})

Context("when canaried by cookie with value", func() {
It("should route requests to the correct upstream", func() {
host := "foo"
annotations := map[string]string{}

ing := framework.NewSingleIngress(host, "/", host, f.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-cookie": "Canary-By-Cookie",
"nginx.ingress.kubernetes.io/canary-by-cookie-value": "^DoCanary$",
}

canaryIngName := fmt.Sprintf("%v-canary", host)

canaryIng := framework.NewSingleIngress(canaryIngName, "/", host, f.Namespace, "http-svc-canary",
80, &canaryAnnotations)
f.EnsureIngress(canaryIng)

time.Sleep(waitForLuaSync)

By("routing requests to the canary upstream when cookie is set to 'DoCanary'")
resp, body, errs := gorequest.New().
Get(f.GetURL(framework.HTTP)).
Set("Host", host).
AddCookie(&http.Cookie{Name: "Canary-By-Cookie", Value: "DoCanary"}).
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 cookie is set to 'always'")
resp, body, errs = gorequest.New().
Get(f.GetURL(framework.HTTP)).
Set("Host", host).
AddCookie(&http.Cookie{Name: "Canary-By-Cookie", Value: "always"}).
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 'never'")

resp, body, errs = gorequest.New().
Get(f.GetURL(framework.HTTP)).
Set("Host", host).
AddCookie(&http.Cookie{Name: "Canary-By-Cookie", 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.GetURL(framework.HTTP)).
Set("Host", host).
AddCookie(&http.Cookie{Name: "Canary-By-Cookie", 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() {
Expand Down