Skip to content

Commit 031c5e5

Browse files
committed
Exclude draining endpoints from new sticky sessions
1 parent 0daef57 commit 031c5e5

File tree

2 files changed

+87
-5
lines changed

2 files changed

+87
-5
lines changed

rootfs/etc/nginx/lua/balancer/sticky.lua

Lines changed: 25 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ local ck = require("resty.cookie")
33
local ngx_balancer = require("ngx.balancer")
44
local split = require("util.split")
55
local same_site = require("util.same_site")
6+
local util = require("util")
67

78
local ngx = ngx
89
local pairs = pairs
@@ -24,7 +25,8 @@ function _M.new(self)
2425
alternative_backends = nil,
2526
cookie_session_affinity = nil,
2627
traffic_shaping_policy = nil,
27-
backend_key = nil
28+
backend_key = nil,
29+
draining_endpoints = nil
2830
}
2931

3032
setmetatable(o, self)
@@ -129,14 +131,20 @@ function _M.get_last_failure()
129131
return ngx_balancer.get_last_failure()
130132
end
131133

132-
local function get_failed_upstreams()
134+
local function get_upstreams_to_exclude(self)
133135
local indexed_upstream_addrs = {}
134-
local upstream_addrs = split.split_upstream_var(ngx.var.upstream_addr) or {}
136+
local failed_upstream_addrs = split.split_upstream_var(ngx.var.upstream_addr) or {}
135137

136-
for _, addr in ipairs(upstream_addrs) do
138+
for _, addr in ipairs(failed_upstream_addrs) do
137139
indexed_upstream_addrs[addr] = true
138140
end
139141

142+
if self.draining_endpoints then
143+
for addr, _ in pairs(self.draining_endpoints) do
144+
indexed_upstream_addrs[addr] = true
145+
end
146+
end
147+
140148
return indexed_upstream_addrs
141149
end
142150

@@ -188,7 +196,7 @@ function _M.balance(self)
188196

189197
local new_upstream
190198

191-
new_upstream, key = self:pick_new_upstream(get_failed_upstreams())
199+
new_upstream, key = self:pick_new_upstream(get_upstreams_to_exclude(self))
192200
if not new_upstream then
193201
ngx.log(ngx.WARN, string.format("failed to get new upstream; using upstream %s", new_upstream))
194202
elseif should_set_cookie(self) then
@@ -202,6 +210,18 @@ function _M.sync(self, backend)
202210
-- reload balancer nodes
203211
balancer_resty.sync(self, backend)
204212

213+
self.draining_endpoints = nil
214+
for _, endpoint in pairs(backend.endpoints) do
215+
if endpoint.isDraining then
216+
if not self.draining_endpoints then
217+
self.draining_endpoints = {}
218+
end
219+
220+
local endpoint_string = util.get_endpoint_string(endpoint)
221+
self.draining_endpoints[endpoint_string] = true
222+
end
223+
end
224+
205225
self.traffic_shaping_policy = backend.trafficShapingPolicy
206226
self.alternative_backends = backend.alternativeBackends
207227
self.cookie_session_affinity = backend.sessionAffinityConfig.cookieSessionAffinity

rootfs/etc/nginx/lua/test/balancer/sticky_test.lua

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -304,6 +304,68 @@ describe("Sticky", function()
304304
it("returns the correct endpoint for the client", function() test_correct_endpoint(sticky_balanced) end)
305305
it("returns the correct endpoint for the client", function() test_correct_endpoint(sticky_persistent) end)
306306
end)
307+
308+
-- Note below that endpoints are only marked this way when persistent
309+
describe("when an endpoint is draining", function()
310+
it("persists client with cookie to endpoint", function()
311+
local s = {}
312+
local cookie_jar = {}
313+
cookie.new = function(self)
314+
local cookie_instance = {
315+
set = function(self, ck)
316+
cookie_jar[ck.key] = ck.value
317+
return true, nil
318+
end,
319+
get = function(self, k) return cookie_jar[k] end,
320+
}
321+
s = spy.on(cookie_instance, "set")
322+
return cookie_instance, false
323+
end
324+
325+
local b = get_test_backend()
326+
b.sessionAffinityConfig.cookieSessionAffinity.locations = {}
327+
b.sessionAffinityConfig.cookieSessionAffinity.locations["test.com"] = {"/"}
328+
329+
local expectedPeer = b.endpoints[1]
330+
331+
local sticky_balancer_instance = sticky_persistent:new(b)
332+
local peer = sticky_balancer_instance:balance()
333+
assert.equal(peer, test_backend_endpoint)
334+
335+
expectedPeer.isDraining = true
336+
sticky_balancer_instance:sync(b)
337+
sticky_balancer_instance.TESTING = true
338+
peer = sticky_balancer_instance:balance()
339+
assert.equal(peer, test_backend_endpoint)
340+
end)
341+
342+
it("does not route client without cookie to endpoint", function()
343+
local s = {}
344+
local cookie_jar = {}
345+
cookie.new = function(self)
346+
local cookie_instance = {
347+
set = function(self, ck)
348+
cookie_jar[ck.key] = ck.value
349+
return true, nil
350+
end,
351+
get = function(self, k) return cookie_jar[k] end,
352+
}
353+
s = spy.on(cookie_instance, "set")
354+
return cookie_instance, false
355+
end
356+
357+
local b = get_test_backend()
358+
b.sessionAffinityConfig.cookieSessionAffinity.locations = {}
359+
b.sessionAffinityConfig.cookieSessionAffinity.locations["test.com"] = {"/"}
360+
361+
local expectedPeer = b.endpoints[1]
362+
expectedPeer.isDraining = true
363+
364+
local sticky_balancer_instance = sticky_persistent:new(b)
365+
local peer = sticky_balancer_instance:balance()
366+
assert.equal(peer, nil)
367+
end)
368+
end)
307369
end)
308370

309371
local function get_several_test_backends(change_on_failure)

0 commit comments

Comments
 (0)