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

refactor(balancer) rename balancer_address to balancer_data #3502

Merged
merged 1 commit into from
May 31, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 28 additions & 26 deletions kong/init.lua
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@ end

require("kong.globalpatches")()

local ip = require "kong.tools.ip"
local DB = require "kong.db"
local dns = require "kong.tools.dns"
local utils = require "kong.tools.utils"
Expand Down Expand Up @@ -155,6 +154,7 @@ local Kong = {}
function Kong.init()
local pl_path = require "pl.path"
local conf_loader = require "kong.conf_loader"
local ip = require "kong.tools.ip"

-- retrieve kong_config
local conf_path = pl_path.join(ngx.config.prefix(), ".kong_env")
Expand Down Expand Up @@ -308,64 +308,66 @@ end

function Kong.balancer()
local ctx = ngx.ctx
local addr = ctx.balancer_address
local tries = addr.tries
local balancer_data = ctx.balancer_data
local tries = balancer_data.tries
local current_try = {}
addr.try_count = addr.try_count + 1
tries[addr.try_count] = current_try
balancer_data.try_count = balancer_data.try_count + 1
tries[balancer_data.try_count] = current_try

runloop.balancer.before()

if addr.try_count > 1 then
-- only call balancer on retry, first one is done in `runloop.access.after` which runs
-- in the ACCESS context and hence has less limitations than this BALANCER context
-- where the retries are executed
if balancer_data.try_count > 1 then
-- only call balancer on retry, first one is done in `runloop.access.after`
-- which runs in the ACCESS context and hence has less limitations than
-- this BALANCER context where the retries are executed

-- record failure data
local previous_try = tries[addr.try_count - 1]
local previous_try = tries[balancer_data.try_count - 1]
previous_try.state, previous_try.code = get_last_failure()

-- Report HTTP status for health checks
if addr.balancer then
local balancer = balancer_data.balancer
if balancer then
local ip, port = balancer_data.ip, balancer_data.port
if previous_try.state == "failed" then
addr.balancer.report_tcp_failure(addr.ip, addr.port)
balancer.report_tcp_failure(ip, port)
else
addr.balancer.report_http_status(addr.ip, addr.port, previous_try.code)
balancer.report_http_status(ip, port, previous_try.code)
end
end

local ok, err, errcode = balancer_execute(addr)
local ok, err, errcode = balancer_execute(balancer_data)
if not ok then
ngx_log(ngx_ERR, "failed to retry the dns/balancer resolver for ",
tostring(addr.host), "' with: ", tostring(err))
tostring(balancer_data.host), "' with: ", tostring(err))
return ngx.exit(errcode)
end

else
-- first try, so set the max number of retries
local retries = addr.retries
local retries = balancer_data.retries
if retries > 0 then
set_more_tries(retries)
end
end

current_try.ip = addr.ip
current_try.port = addr.port
current_try.ip = balancer_data.ip
current_try.port = balancer_data.port

-- set the targets as resolved
ngx_log(ngx_DEBUG, "setting address (try ", addr.try_count, "): ",
addr.ip, ":", addr.port)
local ok, err = set_current_peer(addr.ip, addr.port)
ngx_log(ngx_DEBUG, "setting address (try ", balancer_data.try_count, "): ",
balancer_data.ip, ":", balancer_data.port)
local ok, err = set_current_peer(balancer_data.ip, balancer_data.port)
if not ok then
ngx_log(ngx_ERR, "failed to set the current peer (address: ",
tostring(addr.ip), " port: ", tostring(addr.port),"): ",
tostring(err))
tostring(balancer_data.ip), " port: ", tostring(balancer_data.port),
"): ", tostring(err))
return ngx.exit(500)
end

ok, err = set_timeouts(addr.connect_timeout / 1000,
addr.send_timeout / 1000,
addr.read_timeout / 1000)
ok, err = set_timeouts(balancer_data.connect_timeout / 1000,
balancer_data.send_timeout / 1000,
balancer_data.read_timeout / 1000)
if not ok then
ngx_log(ngx_ERR, "could not set upstream timeouts: ", err)
end
Expand Down
2 changes: 1 addition & 1 deletion kong/plugins/log-serializers/basic.lua
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ function _M.serialize(ngx)
headers = ngx.resp.get_headers(),
size = ngx.var.bytes_sent
},
tries = (ngx.ctx.balancer_address or EMPTY).tries,
tries = (ngx.ctx.balancer_data or EMPTY).tries,
latencies = {
kong = (ngx.ctx.KONG_ACCESS_TIME or 0) +
(ngx.ctx.KONG_RECEIVE_TIME or 0) +
Expand Down
5 changes: 1 addition & 4 deletions kong/runloop/balancer.lua
Original file line number Diff line number Diff line change
Expand Up @@ -666,10 +666,7 @@ local create_hash = function(upstream)
if not identifier then
identifier = utils.uuid()

-- TODO: This should be added the `ngx.ctx.balancer_address`
-- structure, where other balancer-related values are stored,
-- when that is renamed/ refactored.
ctx.balancer_hash_cookie = {
ctx.balancer_data.hash_cookie = {
key = upstream.hash_on_cookie,
value = identifier,
path = upstream.hash_on_cookie_path
Expand Down
77 changes: 40 additions & 37 deletions kong/runloop/handler.lua
Original file line number Diff line number Diff line change
Expand Up @@ -523,52 +523,53 @@ return {
return responses.send(426, "Please use HTTPS protocol")
end

local balancer_address = {
type = upstream_url_t.type, -- the type of `host`; ipv4, ipv6 or name
host = upstream_url_t.host, -- target host per `upstream_url`
port = upstream_url_t.port, -- final target port
try_count = 0, -- retry counter
tries = {}, -- stores info per try
-- ip = nil, -- final target IP address
-- balancer = nil, -- the balancer object, in case of a balancer
-- hostname = nil, -- the hostname belonging to the final target IP
local balancer_data = {
type = upstream_url_t.type, -- type of 'host': ipv4, ipv6, name
host = upstream_url_t.host, -- target host per `upstream_url`
port = upstream_url_t.port, -- final target port
try_count = 0, -- retry counter
tries = {}, -- stores info per try
-- ip = nil, -- final target IP address
-- balancer = nil, -- the balancer object, if any
-- hostname = nil, -- hostname of the final target IP
-- hash_cookie = nil, -- if Upstream sets hash_on_cookie
}

-- TODO: this is probably not optimal
do
local retries = service.retries or api.retries
if retries ~= null then
balancer_address.retries = retries
balancer_data.retries = retries

else
balancer_address.retries = 5
balancer_data.retries = 5
end

local connect_timeout = service.connect_timeout or
api.upstream_connect_timeout
if connect_timeout ~= null then
balancer_address.connect_timeout = connect_timeout
balancer_data.connect_timeout = connect_timeout

else
balancer_address.connect_timeout = 60000
balancer_data.connect_timeout = 60000
end

local send_timeout = service.write_timeout or
api.upstream_send_timeout
if send_timeout ~= null then
balancer_address.send_timeout = send_timeout
balancer_data.send_timeout = send_timeout

else
balancer_address.send_timeout = 60000
balancer_data.send_timeout = 60000
end

local read_timeout = service.read_timeout or
api.upstream_read_timeout
if read_timeout ~= null then
balancer_address.read_timeout = read_timeout
balancer_data.read_timeout = read_timeout

else
balancer_address.read_timeout = 60000
balancer_data.read_timeout = 60000
end
end

Expand All @@ -577,7 +578,8 @@ return {
ctx.service = service
ctx.route = route
ctx.router_matches = match_t.matches
ctx.balancer_address = balancer_address
ctx.balancer_data = balancer_data
ctx.balancer_address = balancer_data -- for plugin backward compatibility

-- `scheme` is the scheme to use for the upstream call
-- `uri` is the URI with which to call upstream, as returned by the
Expand Down Expand Up @@ -627,11 +629,11 @@ return {
end
end

local ok, err, errcode = balancer.execute(ctx.balancer_address)
local ok, err, errcode = balancer.execute(ctx.balancer_data)
if not ok then
if errcode == 500 then
err = "failed the initial dns/balancer resolve for '" ..
ctx.balancer_address.host .. "' with: " ..
ctx.balancer_data.host .. "' with: " ..
tostring(err)
end
return responses.send(errcode, err)
Expand All @@ -642,14 +644,14 @@ return {
local upstream_host = var.upstream_host

if not upstream_host or upstream_host == "" then
local addr = ctx.balancer_address
upstream_host = addr.hostname
local balancer_data = ctx.balancer_data
upstream_host = balancer_data.hostname

local upstream_scheme = var.upstream_scheme
if upstream_scheme == "http" and addr.port ~= 80 or
upstream_scheme == "https" and addr.port ~= 443
if upstream_scheme == "http" and balancer_data.port ~= 80 or
upstream_scheme == "https" and balancer_data.port ~= 443
then
upstream_host = upstream_host .. ":" .. addr.port
upstream_host = upstream_host .. ":" .. balancer_data.port
end

var.upstream_host = upstream_host
Expand All @@ -669,14 +671,14 @@ return {
},
balancer = {
before = function()
local addr = ngx.ctx.balancer_address
local current_try = addr.tries[addr.try_count]
local balancer_data = ngx.ctx.balancer_data
local current_try = balancer_data.tries[balancer_data.try_count]
current_try.balancer_start = get_now()
end,
after = function ()
local ctx = ngx.ctx
local addr = ctx.balancer_address
local current_try = addr.tries[addr.try_count]
local balancer_data = ctx.balancer_data
local current_try = balancer_data.tries[balancer_data.try_count]

-- record try-latency
local try_latency = get_now() - current_try.balancer_start
Expand Down Expand Up @@ -709,15 +711,15 @@ return {
end
end

local cookie_hash_data = ctx.balancer_hash_cookie
if cookie_hash_data then
local hash_cookie = ctx.balancer_data.hash_cookie
if hash_cookie then
local cookie = ck:new()
local ok, err = cookie:set(cookie_hash_data)
local ok, err = cookie:set(hash_cookie)

if not ok then
log(ngx.WARN, "failed to set the cookie for hash-based load balancing: ", err,
" (key=", cookie_hash_data.key,
", path=", cookie_hash_data.path, ")")
" (key=", hash_cookie.key,
", path=", hash_cookie.path, ")")
end
end
end
Expand Down Expand Up @@ -761,13 +763,14 @@ return {
log = {
after = function(ctx)
reports.log()
local addr = ctx.balancer_address
local balancer_data = ctx.balancer_data

-- If response was produced by an upstream (ie, not by a Kong plugin)
if ctx.KONG_PROXIED == true then
-- Report HTTP status for health checks
if addr and addr.balancer and addr.ip then
addr.balancer.report_http_status(addr.ip, addr.port, ngx.status)
if balancer_data and balancer_data.balancer and balancer_data.ip then
balancer_data.balancer.report_http_status(balancer_data.ip,
balancer_data.port, ngx.status)
end
end
end
Expand Down
16 changes: 10 additions & 6 deletions spec/01-unit/011-balancer_spec.lua
Original file line number Diff line number Diff line change
Expand Up @@ -510,21 +510,23 @@ describe("Balancer", function()
it("uses the cookie when present in the request", function()
local value = "some cookie value"
ngx.var.cookie_Foo = value
ngx.ctx.balancer_data = {}
local hash = balancer._create_hash({
hash_on = "cookie",
hash_on_cookie = "Foo",
})
assert.are.same(crc32(value), hash)
assert.is_nil(ngx.ctx.balancer_hash_cookie)
assert.is_nil(ngx.ctx.balancer_data.hash_cookie)
end)
it("creates the cookie when not present in the request", function()
ngx.ctx.balancer_data = {}
balancer._create_hash({
hash_on = "cookie",
hash_on_cookie = "Foo",
hash_on_cookie_path = "/",
})
assert.are.same(ngx.ctx.balancer_hash_cookie.key, "Foo")
assert.are.same(ngx.ctx.balancer_hash_cookie.path, "/")
assert.are.same(ngx.ctx.balancer_data.hash_cookie.key, "Foo")
assert.are.same(ngx.ctx.balancer_data.hash_cookie.path, "/")
end)
end)
it("multi-header", function()
Expand Down Expand Up @@ -587,23 +589,25 @@ describe("Balancer", function()
it("uses the cookie when present in the request", function()
local value = "some cookie value"
ngx.var.cookie_Foo = value
ngx.ctx.balancer_data = {}
local hash = balancer._create_hash({
hash_on = "consumer",
hash_fallback = "cookie",
hash_on_cookie = "Foo",
})
assert.are.same(crc32(value), hash)
assert.is_nil(ngx.ctx.balancer_hash_cookie)
assert.is_nil(ngx.ctx.balancer_data.hash_cookie)
end)
it("creates the cookie when not present in the request", function()
ngx.ctx.balancer_data = {}
balancer._create_hash({
hash_on = "consumer",
hash_fallback = "cookie",
hash_on_cookie = "Foo",
hash_on_cookie_path = "/",
})
assert.are.same(ngx.ctx.balancer_hash_cookie.key, "Foo")
assert.are.same(ngx.ctx.balancer_hash_cookie.path, "/")
assert.are.same(ngx.ctx.balancer_data.hash_cookie.key, "Foo")
assert.are.same(ngx.ctx.balancer_data.hash_cookie.path, "/")
end)
end)
end)
Expand Down
8 changes: 4 additions & 4 deletions spec/01-unit/012-log_serializer_spec.lua
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ describe("Log Serializer", function()
before_each(function()
ngx = {
ctx = {
balancer_address = {
balancer_data = {
tries = {
{
ip = "127.0.0.1",
Expand Down Expand Up @@ -114,7 +114,7 @@ describe("Log Serializer", function()
end)

it("serializes the tries and failure information", function()
ngx.ctx.balancer_address.tries = {
ngx.ctx.balancer_data.tries = {
{ ip = "127.0.0.1", port = 1234, state = "next", code = 502 },
{ ip = "127.0.0.1", port = 1234, state = "failed", code = nil },
{ ip = "127.0.0.1", port = 1234 },
Expand All @@ -140,8 +140,8 @@ describe("Log Serializer", function()
}, res.tries)
end)

it("does not fail when the 'balancer_address' structure is missing", function()
ngx.ctx.balancer_address = nil
it("does not fail when the 'balancer_data' structure is missing", function()
ngx.ctx.balancer_data = nil

local res = basic.serialize(ngx)
assert.is_table(res)
Expand Down