From deeb54584a45e47c26fe1b71bd0e4e1c57afe6b2 Mon Sep 17 00:00:00 2001 From: Francisco Mejia Date: Tue, 21 Aug 2018 14:22:44 -0400 Subject: [PATCH 1/4] Add unit tests for sticky lua module --- .../nginx/lua/test/balancer/sticky_test.lua | 153 ++++++++++++++++++ 1 file changed, 153 insertions(+) create mode 100644 rootfs/etc/nginx/lua/test/balancer/sticky_test.lua diff --git a/rootfs/etc/nginx/lua/test/balancer/sticky_test.lua b/rootfs/etc/nginx/lua/test/balancer/sticky_test.lua new file mode 100644 index 000000000..a3b0d28d9 --- /dev/null +++ b/rootfs/etc/nginx/lua/test/balancer/sticky_test.lua @@ -0,0 +1,153 @@ +local sticky = require("balancer.sticky") +local cookie = require("resty.cookie") +local util = require("util") + +function get_mocked_ngx_env() + local _ngx = { + var = {}, + } + setmetatable(_ngx, {__index = _G.ngx}) + return _ngx +end + +function get_mocked_cookie_new() + return function(self) + return { + get = function(self, n) return nil, "error" end, + set = function(self, n) return true, "" end + } + end +end + +_G.ngx = get_mocked_ngx_env() +cookie.new = get_mocked_cookie_new() + +local function get_test_backend() + return { + name = "access-router-production-web-80", + endpoints = { + { address = "10.184.7.40", port = "8080", maxFails = 0, failTimeout = 0 }, + }, + sessionAffinityConfig = { + name = "cookie", + cookieSessionAffinity = { name = "test_name", hash = "sha1" } + }, + } +end + +describe("Sticky", function() + local test_backend = get_test_backend() + local test_backend_endpoint= test_backend.endpoints[1].address .. ":" .. test_backend.endpoints[1].port + + describe("new(backend)", function() + context("when backend specifies cookie name", function() + it("should return an instance containing the corresponding cookie name", function() + local sticky_balancer_instance = sticky:new(test_backend) + local test_backend_cookie_name = test_backend.sessionAffinityConfig.cookieSessionAffinity.name + assert.equal(sticky_balancer_instance.cookie_name, test_backend_cookie_name) + end) + end) + + context("when backend does not specify cookie name", function() + it("should return an instance with 'route' as cookie name", function() + local temp_backend = util.deepcopy(test_backend) + temp_backend.sessionAffinityConfig.cookieSessionAffinity.name = nil + local sticky_balancer_instance = sticky:new(temp_backend) + local default_cookie_name = "route" + assert.equal(sticky_balancer_instance.cookie_name, default_cookie_name) + end) + end) + + context("when backend specifies hash function", function() + it("should return an instance with the corresponding hash implementation", function() + local sticky_balancer_instance = sticky:new(test_backend) + local test_backend_hash_fn = test_backend.sessionAffinityConfig.cookieSessionAffinity.hash + local test_backend_hash_implementation = util[test_backend_hash_fn .. "_digest"] + assert.equal(sticky_balancer_instance.digest_func, test_backend_hash_implementation) + end) + end) + + context("when backend does not specify hash function", function() + it("should return an instance with the default implementation (md5)", function() + local temp_backend = util.deepcopy(test_backend) + temp_backend.sessionAffinityConfig.cookieSessionAffinity.hash = nil + local sticky_balancer_instance = sticky:new(temp_backend) + local default_hash_fn = "md5" + local default_hash_implementation = util[default_hash_fn .. "_digest"] + assert.equal(sticky_balancer_instance.digest_func, default_hash_implementation) + end) + end) + end) + + describe("balance()", function() + local mocked_cookie_new = cookie.new + + before_each(function() + package.loaded["balancer.sticky"] = nil + sticky = require("balancer.sticky") + end) + + after_each(function() + cookie.new = mocked_cookie_new + end) + + context("when client doesn't have a cookie set", function() + it("should pick an endpoint for the client", function() + local sticky_balancer_instance = sticky:new(test_backend) + local ip, port = sticky_balancer_instance:balance() + assert.truthy(ip) + assert.truthy(port) + assert.equal(ip .. ":" .. port, test_backend_endpoint) + end) + + it("should set a cookie on the client", function() + local s = {} + cookie.new = function(self) + local test_backend_hash_fn = test_backend.sessionAffinityConfig.cookieSessionAffinity.hash + local cookie_instance = { + set = function(self, payload) + assert.equal(payload.key, test_backend.sessionAffinityConfig.cookieSessionAffinity.name) + assert.equal(payload.value, util[test_backend_hash_fn .. "_digest"](test_backend_endpoint)) + assert.equal(payload.path, "/") + assert.equal(payload.domain, nil) + assert.equal(payload.httponly, true) + return true, nil + end, + get = function(k) return false end, + } + s = spy.on(cookie_instance, "set") + return cookie_instance, false + end + local sticky_balancer_instance = sticky:new(get_test_backend()) + assert.has_no.errors(function() sticky_balancer_instance:balance() end) + assert.spy(s).was_called() + end) + end) + + context("when client has a cookie set", function() + it("should not set a cookie", function() + local s = {} + cookie.new = function(self) + local return_obj = { + set = function(v) return false, nil end, + get = function(k) return test_backend_endpoint end, + } + s = spy.on(return_obj, "set") + return return_obj, false + end + local sticky_balancer_instance = sticky:new(test_backend) + assert.has_no.errors(function() sticky_balancer_instance:balance() end) + assert.spy(s).was_not_called() + end) + + it("should return the correct endpoint for the client", function() + local sticky_balancer_instance = sticky:new(test_backend) + local ip, port = sticky_balancer_instance:balance() + assert.truthy(ip) + assert.truthy(port) + assert.equal(ip .. ":" .. port, test_backend_endpoint) + end) + end) + end) +end) + From aec649b05409fc09c4e1f75257f89dc9ac8d26ea Mon Sep 17 00:00:00 2001 From: Francisco Mejia Date: Tue, 21 Aug 2018 15:07:32 -0400 Subject: [PATCH 2/4] Update tests to account for balance() return value --- rootfs/etc/nginx/lua/test/balancer/sticky_test.lua | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/rootfs/etc/nginx/lua/test/balancer/sticky_test.lua b/rootfs/etc/nginx/lua/test/balancer/sticky_test.lua index a3b0d28d9..f484721e2 100644 --- a/rootfs/etc/nginx/lua/test/balancer/sticky_test.lua +++ b/rootfs/etc/nginx/lua/test/balancer/sticky_test.lua @@ -94,10 +94,8 @@ describe("Sticky", function() context("when client doesn't have a cookie set", function() it("should pick an endpoint for the client", function() local sticky_balancer_instance = sticky:new(test_backend) - local ip, port = sticky_balancer_instance:balance() - assert.truthy(ip) - assert.truthy(port) - assert.equal(ip .. ":" .. port, test_backend_endpoint) + local peer = sticky_balancer_instance:balance() + assert.equal(peer, test_backend_endpoint) end) it("should set a cookie on the client", function() @@ -142,10 +140,8 @@ describe("Sticky", function() it("should return the correct endpoint for the client", function() local sticky_balancer_instance = sticky:new(test_backend) - local ip, port = sticky_balancer_instance:balance() - assert.truthy(ip) - assert.truthy(port) - assert.equal(ip .. ":" .. port, test_backend_endpoint) + local peer = sticky_balancer_instance:balance() + assert.equal(peer, test_backend_endpoint) end) end) end) From d306c3ab1a33df41038ff68834597d071798078e Mon Sep 17 00:00:00 2001 From: Francisco Mejia Date: Thu, 23 Aug 2018 14:02:42 -0400 Subject: [PATCH 3/4] Refactor ngx mock and indent using 2 spaces --- .../nginx/lua/test/balancer/sticky_test.lua | 251 +++++++++--------- 1 file changed, 124 insertions(+), 127 deletions(-) diff --git a/rootfs/etc/nginx/lua/test/balancer/sticky_test.lua b/rootfs/etc/nginx/lua/test/balancer/sticky_test.lua index f484721e2..d6b89fae0 100644 --- a/rootfs/etc/nginx/lua/test/balancer/sticky_test.lua +++ b/rootfs/etc/nginx/lua/test/balancer/sticky_test.lua @@ -2,148 +2,145 @@ local sticky = require("balancer.sticky") local cookie = require("resty.cookie") local util = require("util") -function get_mocked_ngx_env() - local _ngx = { - var = {}, - } - setmetatable(_ngx, {__index = _G.ngx}) - return _ngx +function mock_ngx(mock) + local _ngx = mock + setmetatable(_ngx, {__index = _G.ngx}) + _G.ngx = _ngx end function get_mocked_cookie_new() - return function(self) - return { - get = function(self, n) return nil, "error" end, - set = function(self, n) return true, "" end - } - end + return function(self) + return { + get = function(self, n) return nil, "error" end, + set = function(self, n) return true, "" end + } + end end -_G.ngx = get_mocked_ngx_env() +mock_ngx({ var = {} }) cookie.new = get_mocked_cookie_new() local function get_test_backend() - return { - name = "access-router-production-web-80", - endpoints = { - { address = "10.184.7.40", port = "8080", maxFails = 0, failTimeout = 0 }, - }, - sessionAffinityConfig = { - name = "cookie", - cookieSessionAffinity = { name = "test_name", hash = "sha1" } - }, - } + return { + name = "access-router-production-web-80", + endpoints = { + { address = "10.184.7.40", port = "8080", maxFails = 0, failTimeout = 0 }, + }, + sessionAffinityConfig = { + name = "cookie", + cookieSessionAffinity = { name = "test_name", hash = "sha1" } + }, + } end describe("Sticky", function() - local test_backend = get_test_backend() - local test_backend_endpoint= test_backend.endpoints[1].address .. ":" .. test_backend.endpoints[1].port + local test_backend = get_test_backend() + local test_backend_endpoint= test_backend.endpoints[1].address .. ":" .. test_backend.endpoints[1].port - describe("new(backend)", function() - context("when backend specifies cookie name", function() - it("should return an instance containing the corresponding cookie name", function() - local sticky_balancer_instance = sticky:new(test_backend) - local test_backend_cookie_name = test_backend.sessionAffinityConfig.cookieSessionAffinity.name - assert.equal(sticky_balancer_instance.cookie_name, test_backend_cookie_name) - end) - end) + describe("new(backend)", function() + context("when backend specifies cookie name", function() + it("returns an instance containing the corresponding cookie name", function() + local sticky_balancer_instance = sticky:new(test_backend) + local test_backend_cookie_name = test_backend.sessionAffinityConfig.cookieSessionAffinity.name + assert.equal(sticky_balancer_instance.cookie_name, test_backend_cookie_name) + end) + end) - context("when backend does not specify cookie name", function() - it("should return an instance with 'route' as cookie name", function() - local temp_backend = util.deepcopy(test_backend) - temp_backend.sessionAffinityConfig.cookieSessionAffinity.name = nil - local sticky_balancer_instance = sticky:new(temp_backend) - local default_cookie_name = "route" - assert.equal(sticky_balancer_instance.cookie_name, default_cookie_name) - end) - end) + context("when backend does not specify cookie name", function() + it("returns an instance with 'route' as cookie name", function() + local temp_backend = util.deepcopy(test_backend) + temp_backend.sessionAffinityConfig.cookieSessionAffinity.name = nil + local sticky_balancer_instance = sticky:new(temp_backend) + local default_cookie_name = "route" + assert.equal(sticky_balancer_instance.cookie_name, default_cookie_name) + end) + end) - context("when backend specifies hash function", function() - it("should return an instance with the corresponding hash implementation", function() - local sticky_balancer_instance = sticky:new(test_backend) - local test_backend_hash_fn = test_backend.sessionAffinityConfig.cookieSessionAffinity.hash - local test_backend_hash_implementation = util[test_backend_hash_fn .. "_digest"] - assert.equal(sticky_balancer_instance.digest_func, test_backend_hash_implementation) - end) - end) - - context("when backend does not specify hash function", function() - it("should return an instance with the default implementation (md5)", function() - local temp_backend = util.deepcopy(test_backend) - temp_backend.sessionAffinityConfig.cookieSessionAffinity.hash = nil - local sticky_balancer_instance = sticky:new(temp_backend) - local default_hash_fn = "md5" - local default_hash_implementation = util[default_hash_fn .. "_digest"] - assert.equal(sticky_balancer_instance.digest_func, default_hash_implementation) - end) - end) + context("when backend specifies hash function", function() + it("returns an instance with the corresponding hash implementation", function() + local sticky_balancer_instance = sticky:new(test_backend) + local test_backend_hash_fn = test_backend.sessionAffinityConfig.cookieSessionAffinity.hash + local test_backend_hash_implementation = util[test_backend_hash_fn .. "_digest"] + assert.equal(sticky_balancer_instance.digest_func, test_backend_hash_implementation) + end) end) - describe("balance()", function() - local mocked_cookie_new = cookie.new - - before_each(function() - package.loaded["balancer.sticky"] = nil - sticky = require("balancer.sticky") - end) - - after_each(function() - cookie.new = mocked_cookie_new - end) - - context("when client doesn't have a cookie set", function() - it("should pick an endpoint for the client", function() - local sticky_balancer_instance = sticky:new(test_backend) - local peer = sticky_balancer_instance:balance() - assert.equal(peer, test_backend_endpoint) - end) - - it("should set a cookie on the client", function() - local s = {} - cookie.new = function(self) - local test_backend_hash_fn = test_backend.sessionAffinityConfig.cookieSessionAffinity.hash - local cookie_instance = { - set = function(self, payload) - assert.equal(payload.key, test_backend.sessionAffinityConfig.cookieSessionAffinity.name) - assert.equal(payload.value, util[test_backend_hash_fn .. "_digest"](test_backend_endpoint)) - assert.equal(payload.path, "/") - assert.equal(payload.domain, nil) - assert.equal(payload.httponly, true) - return true, nil - end, - get = function(k) return false end, - } - s = spy.on(cookie_instance, "set") - return cookie_instance, false - end - local sticky_balancer_instance = sticky:new(get_test_backend()) - assert.has_no.errors(function() sticky_balancer_instance:balance() end) - assert.spy(s).was_called() - end) - end) - - context("when client has a cookie set", function() - it("should not set a cookie", function() - local s = {} - cookie.new = function(self) - local return_obj = { - set = function(v) return false, nil end, - get = function(k) return test_backend_endpoint end, - } - s = spy.on(return_obj, "set") - return return_obj, false - end - local sticky_balancer_instance = sticky:new(test_backend) - assert.has_no.errors(function() sticky_balancer_instance:balance() end) - assert.spy(s).was_not_called() - end) - - it("should return the correct endpoint for the client", function() - local sticky_balancer_instance = sticky:new(test_backend) - local peer = sticky_balancer_instance:balance() - assert.equal(peer, test_backend_endpoint) - end) - end) + context("when backend does not specify hash function", function() + it("returns an instance with the default implementation (md5)", function() + local temp_backend = util.deepcopy(test_backend) + temp_backend.sessionAffinityConfig.cookieSessionAffinity.hash = nil + local sticky_balancer_instance = sticky:new(temp_backend) + local default_hash_fn = "md5" + local default_hash_implementation = util[default_hash_fn .. "_digest"] + assert.equal(sticky_balancer_instance.digest_func, default_hash_implementation) + end) end) -end) + end) + describe("balance()", function() + local mocked_cookie_new = cookie.new + + before_each(function() + package.loaded["balancer.sticky"] = nil + sticky = require("balancer.sticky") + end) + + after_each(function() + cookie.new = mocked_cookie_new + end) + + context("when client doesn't have a cookie set", function() + it("picks an endpoint for the client", function() + local sticky_balancer_instance = sticky:new(test_backend) + local peer = sticky_balancer_instance:balance() + assert.equal(peer, test_backend_endpoint) + end) + + it("sets a cookie on the client", function() + local s = {} + cookie.new = function(self) + local test_backend_hash_fn = test_backend.sessionAffinityConfig.cookieSessionAffinity.hash + local cookie_instance = { + set = function(self, payload) + assert.equal(payload.key, test_backend.sessionAffinityConfig.cookieSessionAffinity.name) + assert.equal(payload.value, util[test_backend_hash_fn .. "_digest"](test_backend_endpoint)) + assert.equal(payload.path, "/") + assert.equal(payload.domain, nil) + assert.equal(payload.httponly, true) + return true, nil + end, + get = function(k) return false end, + } + s = spy.on(cookie_instance, "set") + return cookie_instance, false + end + local sticky_balancer_instance = sticky:new(get_test_backend()) + assert.has_no.errors(function() sticky_balancer_instance:balance() end) + assert.spy(s).was_called() + end) + end) + + context("when client has a cookie set", function() + it("does not set a cookie", function() + local s = {} + cookie.new = function(self) + local return_obj = { + set = function(v) return false, nil end, + get = function(k) return test_backend_endpoint end, + } + s = spy.on(return_obj, "set") + return return_obj, false + end + local sticky_balancer_instance = sticky:new(test_backend) + assert.has_no.errors(function() sticky_balancer_instance:balance() end) + assert.spy(s).was_not_called() + end) + + it("returns the correct endpoint for the client", function() + local sticky_balancer_instance = sticky:new(test_backend) + local peer = sticky_balancer_instance:balance() + assert.equal(peer, test_backend_endpoint) + end) + end) + end) +end) \ No newline at end of file From 52f7393781ff0453ec864bb698132186450f62db Mon Sep 17 00:00:00 2001 From: Francisco Mejia Date: Thu, 23 Aug 2018 14:09:08 -0400 Subject: [PATCH 4/4] Add reset_ngx method to sticky_test.lua --- .../etc/nginx/lua/test/balancer/sticky_test.lua | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/rootfs/etc/nginx/lua/test/balancer/sticky_test.lua b/rootfs/etc/nginx/lua/test/balancer/sticky_test.lua index d6b89fae0..9346d6573 100644 --- a/rootfs/etc/nginx/lua/test/balancer/sticky_test.lua +++ b/rootfs/etc/nginx/lua/test/balancer/sticky_test.lua @@ -2,12 +2,18 @@ local sticky = require("balancer.sticky") local cookie = require("resty.cookie") local util = require("util") +local original_ngx = ngx + function mock_ngx(mock) local _ngx = mock setmetatable(_ngx, {__index = _G.ngx}) _G.ngx = _ngx end +local function reset_ngx() + _G.ngx = original_ngx +end + function get_mocked_cookie_new() return function(self) return { @@ -17,7 +23,6 @@ function get_mocked_cookie_new() end end -mock_ngx({ var = {} }) cookie.new = get_mocked_cookie_new() local function get_test_backend() @@ -34,6 +39,14 @@ local function get_test_backend() end describe("Sticky", function() + before_each(function() + mock_ngx({ var = {} }) + end) + + after_each(function() + reset_ngx() + end) + local test_backend = get_test_backend() local test_backend_endpoint= test_backend.endpoints[1].address .. ":" .. test_backend.endpoints[1].port