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.

Sign up for GitHub

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

I improved healthcheck library so that it supports contains / exact matc... #3

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

I improved healthcheck library so that it supports contains / exact matc... #3

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 2 additions & 0 deletions README.markdown
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,8 @@ http {
rise = 2, -- # of successive successes before turning a peer up
valid_statuses = {200, 302}, -- a list valid HTTP status code
concurrency = 10, -- concurrency level for test requests
http_version = "1.1", -- the http protocol version used for request format. Only 1.0 or 1.1 are supported values. By default 1.1 is used.
expected_body = "Server_Ok", -- the body we expect to receive. If you want contains instead of exact matching use % (e.g %Server_Ok%)
}
if not ok then
ngx.log(ngx.ERR, "failed to spawn health checker: ", err)
Expand Down
46 changes: 45 additions & 1 deletion lib/resty/upstream/healthcheck.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,15 @@ local fmod = math.fmod
local spawn = ngx.thread.spawn
local wait = ngx.thread.wait

local http_descriptor = {}
http_descriptor["1.0"] = {
delimiter_body = "\r\n"
}

http_descriptor["1.1"] = {
delimiter_body = "\r\n\r\n"
}

local _M = {
_VERSION = '0.03'
}
Expand Down Expand Up @@ -199,6 +208,25 @@ local function peer_ok(ctx, is_backup, id, peer)
end
end

--- This method is used to detect if a http message body matches the expected body specified
-- in an upstream healtcheck. If the expected_body is a contains expression (%) then it tries
-- to read from socket till it gets the string from %%. Otherwise it does an exact read of bytes
-- and compare the result with the expected body.
local function is_expectedbody(socket, expected_body, http_version)
local readuntilnew = socket:receiveuntil(http_descriptor[http_version]["delimiter_body"])
local headers = readuntilnew()

if not string.find(expected_body, "%%") then
local body = socket:receive(string.len(expected_body))

return body == expected_body
end

expected_body = string.gsub(expected_body, "%%", "")

return socket:receiveuntil(expected_body)()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this is the right thing to do. Since it introduces unnecessary delay (till timeout) if the body does not contain the expected pattern, which is rather wasteful. Also, this approach will fail when the chunked encoding is used and a chunk boundary happens to split the part matching the pattern.

end

local function check_peer(ctx, id, peer, is_backup)
local u = ctx.upstream
local ok, err
Expand Down Expand Up @@ -260,7 +288,17 @@ local function check_peer(ctx, id, peer, is_backup)
end
peer_fail(ctx, is_backup, id, peer)
else
peer_ok(ctx, is_backup, id, peer)
if ctx.expected_body then
if not is_expectedbody(sock, ctx.expected_body, ctx.http_version) then
errlog(string.format("Received body from %s does not match %s",
name, ctx.expected_body))
peer_fail(ctx, is_backup, id, peer)
else
peer_ok(ctx, is_backup, id, peer)
end
else
peer_ok(ctx, is_backup, id, peer)
end
end
end
else
Expand Down Expand Up @@ -600,8 +638,14 @@ function _M.spawn_checker(opts)
statuses = statuses,
version = 0,
concurrency = concur,
http_version = opts.http_version or "1.1",
expected_body = opts.expected_body
}

if ctx.http_version ~= "1.1" or ctx.http_version ~= "1.0" then
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shouldn't this be an AND operator?

You probably want;

if ctx.http_version ~= "1.0" then
    ctx.http_version = "1.1"
end

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just commented on the line, but actually the checks should be done further up, before the ctx table is created.

and then it should be something like;

local http_allowed = { ["1.0"] = true, ["1.1"] = true }

local http_version = opts.http_version or "1.1"
if not http_allowed[http_version] then
    return nil, "invalid http version: "..tostring(http_version)
end

ctx.http_version = "1.1"
end

local ok, err = new_timer(0, check, ctx)
if not ok then
return nil, "failed to create timer: " .. err
Expand Down