From 3c46262d2424d36cc19b77daa8aa11b54ce85a6e Mon Sep 17 00:00:00 2001 From: Yecheng Fu Date: Fri, 16 Oct 2020 18:44:58 +0800 Subject: [PATCH 1/2] add canary-weight-total annotation --- .../nginx-configuration/annotations.md | 5 ++- internal/ingress/annotations/canary/main.go | 6 ++++ internal/ingress/controller/controller.go | 1 + internal/ingress/types.go | 11 +++++-- rootfs/etc/nginx/lua/balancer.lua | 6 +++- rootfs/etc/nginx/lua/test/balancer_test.lua | 14 ++++++++ test/e2e/annotations/canary.go | 33 +++++++++++++++++++ 7 files changed, 71 insertions(+), 5 deletions(-) diff --git a/docs/user-guide/nginx-configuration/annotations.md b/docs/user-guide/nginx-configuration/annotations.md index 19bd3947f3..46cf3462a3 100755 --- a/docs/user-guide/nginx-configuration/annotations.md +++ b/docs/user-guide/nginx-configuration/annotations.md @@ -41,6 +41,7 @@ You can add these Kubernetes annotations to specific Ingress objects to customiz |[nginx.ingress.kubernetes.io/canary-by-header-pattern](#canary)|string| |[nginx.ingress.kubernetes.io/canary-by-cookie](#canary)|string| |[nginx.ingress.kubernetes.io/canary-weight](#canary)|number| +|[nginx.ingress.kubernetes.io/canary-weight-total](#canary)|number| |[nginx.ingress.kubernetes.io/client-body-buffer-size](#client-body-buffer-size)|string| |[nginx.ingress.kubernetes.io/configuration-snippet](#configuration-snippet)|string| |[nginx.ingress.kubernetes.io/custom-http-errors](#custom-http-errors)|[]int| @@ -138,7 +139,9 @@ 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-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. +* `nginx.ingress.kubernetes.io/canary-weight`: The integer based (0 - ) 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 means implies all requests will be sent to the alternative service specified in the Ingress. `` defaults to 100, and can be increased via `nginx.ingress.kubernetes.io/canary-weight-total`. + +* `nginx.ingress.kubernetes.io/canary-weight-total`: The total weight of traffic. If unspecified, it defaults to 100. Canary rules are evaluated in order of precedence. Precedence is as follows: `canary-by-header -> canary-by-cookie -> canary-weight` diff --git a/internal/ingress/annotations/canary/main.go b/internal/ingress/annotations/canary/main.go index 3930b84d77..d9e53b3b81 100644 --- a/internal/ingress/annotations/canary/main.go +++ b/internal/ingress/annotations/canary/main.go @@ -32,6 +32,7 @@ type canary struct { type Config struct { Enabled bool Weight int + WeightTotal int Header string HeaderValue string HeaderPattern string @@ -59,6 +60,11 @@ func (c canary) Parse(ing *networking.Ingress) (interface{}, error) { config.Weight = 0 } + config.WeightTotal, err = parser.GetIntAnnotation("canary-weight-total", ing) + if err != nil { + config.WeightTotal = 100 + } + config.Header, err = parser.GetStringAnnotation("canary-by-header", ing) if err != nil { config.Header = "" diff --git a/internal/ingress/controller/controller.go b/internal/ingress/controller/controller.go index fb06a58c6e..06eea9d7df 100644 --- a/internal/ingress/controller/controller.go +++ b/internal/ingress/controller/controller.go @@ -891,6 +891,7 @@ func (n *NGINXController) createUpstreams(data []*ingress.Ingress, du *ingress.B upstreams[defBackend].NoServer = true upstreams[defBackend].TrafficShapingPolicy = ingress.TrafficShapingPolicy{ Weight: anns.Canary.Weight, + WeightTotal: anns.Canary.WeightTotal, Header: anns.Canary.Header, HeaderValue: anns.Canary.HeaderValue, HeaderPattern: anns.Canary.HeaderPattern, diff --git a/internal/ingress/types.go b/internal/ingress/types.go index 033fa9cc46..78c2245ff9 100644 --- a/internal/ingress/types.go +++ b/internal/ingress/types.go @@ -111,10 +111,15 @@ type Backend struct { // alternative backend // +k8s:deepcopy-gen=true type TrafficShapingPolicy struct { - // Weight (0-100) of traffic to redirect to the backend. - // e.g. Weight 20 means 20% of traffic will be redirected to the backend and 80% will remain - // with the other backend. 0 weight will not send any traffic to this backend + // Weight (0-) of traffic to redirect to the backend. + // e.g. defaults to 100, weight 20 means 20% of traffic will be + // redirected to the backend and 80% will remain with the other backend. If + // is set to 1000, weight 2 means 0.2% of traffic will be + // redirected to the backend and 99.8% will remain with the other backend. + // 0 weight will not send any traffic to this backend Weight int `json:"weight"` + // The total weight of traffic (>= 100). If unspecified, it defaults to 100. + WeightTotal int `json:"weightTotal"` // Header on which to redirect requests to this backend Header string `json:"header"` // HeaderValue on which to redirect requests to this backend diff --git a/rootfs/etc/nginx/lua/balancer.lua b/rootfs/etc/nginx/lua/balancer.lua index e83257a6f9..b6c420c9f9 100644 --- a/rootfs/etc/nginx/lua/balancer.lua +++ b/rootfs/etc/nginx/lua/balancer.lua @@ -259,7 +259,11 @@ local function route_to_alternative_balancer(balancer) end end - if math.random(100) <= traffic_shaping_policy.weight then + local weightTotal = 100 + if traffic_shaping_policy.weightTotal ~= nil and traffic_shaping_policy.weightTotal > 100 then + weightTotal = traffic_shaping_policy.weightTotal + end + if math.random(weightTotal) <= traffic_shaping_policy.weight then return true end diff --git a/rootfs/etc/nginx/lua/test/balancer_test.lua b/rootfs/etc/nginx/lua/test/balancer_test.lua index 4f40bc6ae5..2d42ad3306 100644 --- a/rootfs/etc/nginx/lua/test/balancer_test.lua +++ b/rootfs/etc/nginx/lua/test/balancer_test.lua @@ -203,6 +203,20 @@ describe("Balancer", function() balancer.sync_backend(backend) assert.equal(false, balancer.route_to_alternative_balancer(_primaryBalancer)) end) + + it("returns true when weight is 1000 and weight total is 1000", function() + backend.trafficShapingPolicy.weight = 1000 + backend.trafficShapingPolicy.weightTotal = 1000 + balancer.sync_backend(backend) + assert.equal(true, balancer.route_to_alternative_balancer(_primaryBalancer)) + end) + + it("returns false when weight is 0 and weight total is 1000", function() + backend.trafficShapingPolicy.weight = 1000 + backend.trafficShapingPolicy.weightTotal = 1000 + balancer.sync_backend(backend) + assert.equal(true, balancer.route_to_alternative_balancer(_primaryBalancer)) + end) end) describe("canary by cookie", function() diff --git a/test/e2e/annotations/canary.go b/test/e2e/annotations/canary.go index d189c972db..31e7404341 100644 --- a/test/e2e/annotations/canary.go +++ b/test/e2e/annotations/canary.go @@ -773,6 +773,39 @@ var _ = framework.DescribeAnnotation("canary-*", func() { Contains(canaryService) }) + ginkgo.It("should route requests only to canary if canary weight is equal to canary weight total", 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": "1000", + "nginx.ingress.kubernetes.io/canary-weight-total": "1000", + } + + canaryIng := framework.NewSingleIngress(canaryIngName, "/", host, + f.Namespace, canaryService, 80, canaryAnnotations) + f.EnsureIngress(canaryIng) + + f.HTTPTestClient(). + GET("/"). + WithHeader("Host", host). + Expect(). + Status(http.StatusOK). + 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{} From 6b8f3d65160fcd7a856da2a3c71a6c7a91df71cb Mon Sep 17 00:00:00 2001 From: 2339478391 <2339478391@qq.com> Date: Tue, 21 Dec 2021 13:52:19 +0800 Subject: [PATCH 2/2] consistency --- internal/ingress/annotations/canary/main.go | 32 ++++++++++++++----- internal/ingress/controller/controller.go | 26 +++++++++------- internal/ingress/types.go | 4 +++ internal/ingress/types_equals.go | 6 ++++ rootfs/etc/nginx/lua/balancer.lua | 34 +++++++++++++++++++++ rootfs/etc/nginx/lua/balancer/hashcode.lua | 17 +++++++++++ 6 files changed, 100 insertions(+), 19 deletions(-) create mode 100644 rootfs/etc/nginx/lua/balancer/hashcode.lua diff --git a/internal/ingress/annotations/canary/main.go b/internal/ingress/annotations/canary/main.go index d9e53b3b81..8ca883127d 100644 --- a/internal/ingress/annotations/canary/main.go +++ b/internal/ingress/annotations/canary/main.go @@ -30,13 +30,15 @@ type canary struct { // Config returns the configuration rules for setting up the Canary type Config struct { - Enabled bool - Weight int - WeightTotal int - Header string - HeaderValue string - HeaderPattern string - Cookie string + Enabled bool + Weight int + WeightTotal int + Header string + HeaderValue string + HeaderPattern string + Cookie string + CanaryConsistency string + HashSeed string } // NewParser parses the ingress for canary related annotations @@ -85,8 +87,22 @@ func (c canary) Parse(ing *networking.Ingress) (interface{}, error) { config.Cookie = "" } + config.CanaryConsistency, err = parser.GetStringAnnotation("canary-consistency", ing) + if err != nil { + config.CanaryConsistency = "" + } + + if config.CanaryConsistency != "header" && config.CanaryConsistency != "cookie" { + config.CanaryConsistency = "" + } + + config.HashSeed, err = parser.GetStringAnnotation("canary-hash-seed", ing) + if err != nil { + config.HashSeed = "" + } + if !config.Enabled && (config.Weight > 0 || len(config.Header) > 0 || len(config.HeaderValue) > 0 || len(config.Cookie) > 0 || - len(config.HeaderPattern) > 0) { + len(config.HeaderPattern) > 0 || len(config.CanaryConsistency) > 0 || len(config.HashSeed) > 0) { return nil, errors.NewInvalidAnnotationConfiguration("canary", "configured but not enabled") } diff --git a/internal/ingress/controller/controller.go b/internal/ingress/controller/controller.go index 06eea9d7df..de26d2afec 100644 --- a/internal/ingress/controller/controller.go +++ b/internal/ingress/controller/controller.go @@ -890,12 +890,14 @@ func (n *NGINXController) createUpstreams(data []*ingress.Ingress, du *ingress.B if anns.Canary.Enabled { upstreams[defBackend].NoServer = true upstreams[defBackend].TrafficShapingPolicy = ingress.TrafficShapingPolicy{ - Weight: anns.Canary.Weight, - WeightTotal: anns.Canary.WeightTotal, - Header: anns.Canary.Header, - HeaderValue: anns.Canary.HeaderValue, - HeaderPattern: anns.Canary.HeaderPattern, - Cookie: anns.Canary.Cookie, + Weight: anns.Canary.Weight, + WeightTotal: anns.Canary.WeightTotal, + Header: anns.Canary.Header, + HeaderValue: anns.Canary.HeaderValue, + HeaderPattern: anns.Canary.HeaderPattern, + Cookie: anns.Canary.Cookie, + CanaryConsistency: anns.Canary.CanaryConsistency, + HashSeed: anns.Canary.HashSeed, } } @@ -962,11 +964,13 @@ func (n *NGINXController) createUpstreams(data []*ingress.Ingress, du *ingress.B if anns.Canary.Enabled { upstreams[name].NoServer = true upstreams[name].TrafficShapingPolicy = ingress.TrafficShapingPolicy{ - Weight: anns.Canary.Weight, - Header: anns.Canary.Header, - HeaderValue: anns.Canary.HeaderValue, - HeaderPattern: anns.Canary.HeaderPattern, - Cookie: anns.Canary.Cookie, + Weight: anns.Canary.Weight, + Header: anns.Canary.Header, + HeaderValue: anns.Canary.HeaderValue, + HeaderPattern: anns.Canary.HeaderPattern, + Cookie: anns.Canary.Cookie, + CanaryConsistency: anns.Canary.CanaryConsistency, + HashSeed: anns.Canary.HashSeed, } } diff --git a/internal/ingress/types.go b/internal/ingress/types.go index 78c2245ff9..2bd505d492 100644 --- a/internal/ingress/types.go +++ b/internal/ingress/types.go @@ -128,6 +128,10 @@ type TrafficShapingPolicy struct { HeaderPattern string `json:"headerPattern"` // Cookie on which to redirect requests to this backend Cookie string `json:"cookie"` + // the value can be "header" or "cookie", which one will be keeping consistency + CanaryConsistency string `json:"canaryConsistency"` + //it's optional, the different seeds can result different hashcode + HashSeed string `json:"hashSeed"` } // HashInclude defines if a field should be used or not to calculate the hash diff --git a/internal/ingress/types_equals.go b/internal/ingress/types_equals.go index 18ffa90424..0ee20aaeaf 100644 --- a/internal/ingress/types_equals.go +++ b/internal/ingress/types_equals.go @@ -260,6 +260,12 @@ func (tsp1 TrafficShapingPolicy) Equal(tsp2 TrafficShapingPolicy) bool { if tsp1.Cookie != tsp2.Cookie { return false } + if tsp1.CanaryConsistency != tsp2.CanaryConsistency { + return false + } + if tsp1.HashSeed != tsp2.HashSeed { + return false + } return true } diff --git a/rootfs/etc/nginx/lua/balancer.lua b/rootfs/etc/nginx/lua/balancer.lua index b6c420c9f9..0b62716940 100644 --- a/rootfs/etc/nginx/lua/balancer.lua +++ b/rootfs/etc/nginx/lua/balancer.lua @@ -17,6 +17,7 @@ local tostring = tostring local pairs = pairs local math = math local ngx = ngx +local hashcode = require("balancer.hashcode").str_hash_to_int -- measured in seconds -- for an Nginx worker to pick up the new list of upstream peers @@ -223,6 +224,39 @@ local function route_to_alternative_balancer(balancer) return false end + if traffic_shaping_policy.canaryConsistency and traffic_shaping_policy.canaryConsistency ~= "" + then + local seed = traffic_shaping_policy.hashSeed + if not seed then + seed = "" + end + + if traffic_shaping_policy.canaryConsistency == "header" then + local target_header = util.replace_special_char(traffic_shaping_policy.header, + "-", "_") + local header_value = ngx.var["http_" .. target_header] + if header_value then + local hash_value = math.abs(hashcode(header_value..seed)) + if hash_value % 100 < traffic_shaping_policy.weight then + return true + end + end + end + + if traffic_shaping_policy.canaryConsistency == "cookie" then + local target_cookie = traffic_shaping_policy.cookie + local cookie = ngx.var["cookie_" .. target_cookie] + if cookie then + local hash_value = math.abs(hashcode(cookie..seed)) + if hash_value % 100 < traffic_shaping_policy.weight then + return true + end + end + end + + return false + end + local target_header = util.replace_special_char(traffic_shaping_policy.header, "-", "_") local header = ngx.var["http_" .. target_header] diff --git a/rootfs/etc/nginx/lua/balancer/hashcode.lua b/rootfs/etc/nginx/lua/balancer/hashcode.lua new file mode 100644 index 0000000000..c859751c46 --- /dev/null +++ b/rootfs/etc/nginx/lua/balancer/hashcode.lua @@ -0,0 +1,17 @@ +local string = string +local _M = {} + +function _M.str_hash_to_int(str) + local h = 0 + local l = #str + if l > 0 then + local i = 0 + while i < l do + h = 31 * h + string.byte(str, i + 1) + i = i + 1 + end + end + return h +end + +return _M