Skip to content

Commit

Permalink
add canary-weight-total annotation (kubernetes#6338)
Browse files Browse the repository at this point in the history
  • Loading branch information
cofyc authored and rchshld committed May 17, 2023
1 parent 02ca7ed commit dadcb65
Show file tree
Hide file tree
Showing 7 changed files with 71 additions and 5 deletions.
5 changes: 4 additions & 1 deletion docs/user-guide/nginx-configuration/annotations.md
Original file line number Diff line number Diff line change
Expand Up @@ -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|
Expand Down Expand Up @@ -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 - <weight-total>) 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 <weight-total> means implies all requests will be sent to the alternative service specified in the Ingress. `<weight-total>` 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`
Expand Down
6 changes: 6 additions & 0 deletions internal/ingress/annotations/canary/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ type canary struct {
type Config struct {
Enabled bool
Weight int
WeightTotal int
Header string
HeaderValue string
HeaderPattern string
Expand Down Expand Up @@ -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 = ""
Expand Down
1 change: 1 addition & 0 deletions internal/ingress/controller/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -910,6 +910,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,
Expand Down
11 changes: 8 additions & 3 deletions internal/ingress/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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-<WeightTotal>) of traffic to redirect to the backend.
// e.g. <WeightTotal> defaults to 100, weight 20 means 20% of traffic will be
// redirected to the backend and 80% will remain with the other backend. If
// <WeightTotal> 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
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 @@ -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

Expand Down
14 changes: 14 additions & 0 deletions rootfs/etc/nginx/lua/test/balancer_test.lua
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
33 changes: 33 additions & 0 deletions test/e2e/annotations/canary.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{}
Expand Down

0 comments on commit dadcb65

Please sign in to comment.