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

Jump to bottom

test: reformatted the test cases by latest tool reindex. #22

Closed
wants to merge 5 commits into from
Closed

test: reformatted the test cases by latest tool reindex. #22

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
a207199
chore(reindex) reformat the test suite.
membphis Jul 12, 2019
09d94e9
test(active_probes) use ca-bundle.trust.crt if fails to find ca-certi…
membphis Jul 12, 2019
d130d7a
optimize(healthchecks): cache local variables.
membphis Jul 12, 2019
272d4ba
test(tls_active_probes): removed useless double quotes.
membphis Jul 12, 2019
94a53db
change(utils): used original `reindex` version.
membphis Jul 17, 2019
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
18 changes: 15 additions & 3 deletions lib/resty/healthcheck.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
-- @author Hisham Muhammad, Thijs Schreijer
-- @license Apache 2.0

local ngx = ngx
membphis marked this conversation as resolved.
Show resolved Hide resolved
local ERR = ngx.ERR
local WARN = ngx.WARN
local DEBUG = ngx.DEBUG
Expand All @@ -38,6 +39,17 @@ local resty_lock = require ("resty.lock")
local re_find = ngx.re.find
local bit = require("bit")
local ngx_now = ngx.now
local error = error
local print = print
local require = require
local string = string
local assert = assert
local tonumber = tonumber
local type = type
local math = math
local next = next
local pairs = pairs
local table = table

-- constants
local EVENT_SOURCE_PREFIX = "lua-resty-healthcheck"
Expand Down Expand Up @@ -503,8 +515,8 @@ local function incr_counter(self, health_report, ip, port, hostname, limit, ctr_
local ctr = ctr_get(multictr, ctr_type)

self:log(WARN, health_report, " ", COUNTER_NAMES[ctr_type],
" increment (", ctr, "/", limit, ") for '", hostname or "",
"(", ip, ":", port, ")'")
" increment (", ctr, "/", limit, ") for '", hostname or "",
"(", ip, ":", port, ")'")

local new_multictr
if ctr_type == CTR_SUCCESS then
Expand Down Expand Up @@ -1011,7 +1023,7 @@ end
-- Log a message specific to this checker
-- @param level standard ngx log level constant
function checker:log(level, ...)
ngx_log(level, worker_color(self.LOG_PREFIX), ...)
return ngx_log(level, worker_color(self.LOG_PREFIX), ...)
Copy link
Contributor

Choose a reason for hiding this comment

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

this does not seem necessary, as we don't care about the return of ngx.log (is there any? the docs don't say so)

Copy link
Contributor Author

@membphis membphis Jul 13, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

We use lua's tail call feature here so that we can see the caller's line number in the error log.

please take a look at this example, pay more attention about the output of error log:

$ resty -e 'local function log1(...)
                ngx.log(ngx.ERR, ...)
            end

            local function log2(...)
                return ngx.log(ngx.ERR, ...)
            end

            log1("first")
            log2("second")'
2019/07/12 21:04:01 [error] 40647#40647: *2 [lua] (command line -e):2: log1(): first, context: ngx.timer
2019/07/12 21:04:01 [error] 40647#40647: *2 [lua] (command line -e):10: second, context: ngx.timer

I think we more love the implementation of log2, which can record the original source code line number.

Copy link
Member

Choose a reason for hiding this comment

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

@membphis that is a neat trick to get the proper line logged. It has been very annoying to only get the lines from the log function. Definitely one to remember and reuse elsewhere. Thx.

Copy link
Contributor

Choose a reason for hiding this comment

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

@membphis @Tieske while Lua does have a tail call feature, losing the line number info when performing a tail call is a LuaJIT-specific (mis)behavior. Here's an equivalent non-resty specific script:

local function x()
   return debug.getinfo(1, "l")
end

local function y()
   local x = debug.getinfo(1, "l")
   return x
end

print(x().currentline)
print(y().currentline)

With Lua 5.3 this prints "2 6", as it should; with resty this prints "10 6".

It may be a "neat trick" that you managed to get a useful behavior out of the fact that LuaJIT loses debugging information, but I would be wary of relying on such undefined behavior. A future version of the compiler may fix this bug and then line number info will change in surprising ways.

Copy link
Contributor

Choose a reason for hiding this comment

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

To clarify, I'm not saying this change should be removed from this PR — just that I'm not a fan of it :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It seems that we can keep this improvement. After all, the project works in the openresty environment, we can only use luajit.

end


Expand Down
3 changes: 3 additions & 0 deletions lib/resty/healthcheck/utils.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@
-- @author Hisham Muhammad, Thijs Schreijer
-- @license Apache 2.0

local assert = assert
local type = type

local timer_at = ngx.timer.at

local _M = {}
Expand Down
12 changes: 12 additions & 0 deletions t/00-new.t
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ GET /t
--- error_log
please configure



=== TEST 2: new() requires 'name'
--- http_config eval: $::HttpConfig
--- config
Expand All @@ -55,6 +57,8 @@ GET /t
--- error_log
required option 'name' is missing



=== TEST 3: new() requires 'shm_name'
--- http_config eval: $::HttpConfig
--- config
Expand All @@ -76,6 +80,8 @@ GET /t
--- error_log
required option 'shm_name' is missing



=== TEST 4: new() fails with invalid shm
--- http_config eval: $::HttpConfig
--- config
Expand All @@ -98,6 +104,8 @@ GET /t
--- error_log
no shm found by name



=== TEST 5: new() initializes with default config
--- http_config eval: $::HttpConfig
--- config
Expand All @@ -119,6 +127,8 @@ GET /t
--- error_log
Healthchecker started!



=== TEST 6: new() only accepts http or tcp types
--- http_config eval: $::HttpConfig
--- config
Expand Down Expand Up @@ -154,6 +164,8 @@ true
true
false



=== TEST 7: new() deals with bad inputs
--- http_config eval: $::HttpConfig
--- config
Expand Down
8 changes: 8 additions & 0 deletions t/01-start-stop.t
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,8 @@ true
--- no_error_log
[error]



=== TEST 2: start() cannot start a second time using active health checks
--- http_config eval: $::HttpConfig
--- config
Expand Down Expand Up @@ -87,6 +89,8 @@ cannot start, 2 (of 2) timers are still running
--- no_error_log
[error]



=== TEST 3: start() is a no-op if active intervals are 0
--- http_config eval: $::HttpConfig
--- config
Expand Down Expand Up @@ -128,6 +132,8 @@ true
--- no_error_log
[error]



=== TEST 4: stop() stops health checks
--- http_config eval: $::HttpConfig
--- config
Expand Down Expand Up @@ -167,6 +173,8 @@ true
[error]
checking



=== TEST 5: start() restarts health checks
--- http_config eval: $::HttpConfig
--- config
Expand Down
5 changes: 3 additions & 2 deletions t/04-report_success.t
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,6 @@ run_tests();

__DATA__



=== TEST 1: report_success() recovers HTTP active + passive
--- http_config eval
qq{
Expand Down Expand Up @@ -96,6 +94,7 @@ healthy SUCCESS increment (3/3) for '(127.0.0.1:2116)'
event: target status '(127.0.0.1:2118)' from 'false' to 'true'



=== TEST 2: report_success() recovers TCP active = passive
--- http_config eval
qq{
Expand Down Expand Up @@ -172,6 +171,8 @@ healthy SUCCESS increment (2/3) for '(127.0.0.1:2116)'
healthy SUCCESS increment (3/3) for '(127.0.0.1:2116)'
event: target status '(127.0.0.1:2118)' from 'false' to 'true'



=== TEST 3: report_success() is a nop when active.healthy.sucesses == 0
--- http_config eval
qq{
Expand Down
4 changes: 2 additions & 2 deletions t/05-report_failure.t
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,6 @@ run_tests();

__DATA__



=== TEST 1: report_failure() fails HTTP active + passive
--- http_config eval
qq{
Expand Down Expand Up @@ -96,6 +94,7 @@ unhealthy HTTP increment (3/3) for '(127.0.0.1:2113)'
event: target status '(127.0.0.1:2113)' from 'true' to 'false'



=== TEST 2: report_failure() fails TCP active + passive
--- http_config eval
qq{
Expand Down Expand Up @@ -171,6 +170,7 @@ unhealthy TCP increment (2/2) for '(127.0.0.1:2113)'
event: target status '(127.0.0.1:2113)' from 'true' to 'false'



=== TEST 3: report_failure() is a nop when failure counters == 0
--- http_config eval
qq{
Expand Down
8 changes: 5 additions & 3 deletions t/06-report_http_status.t
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,6 @@ run_tests();

__DATA__



=== TEST 1: report_http_status() failures active + passive
--- http_config eval
qq{
Expand Down Expand Up @@ -178,6 +176,7 @@ healthy SUCCESS increment (4/4) for '(127.0.0.1:2113)'
event: target status '(127.0.0.1:2113)' from 'false' to 'true'



=== TEST 3: report_http_status() with success is a nop when passive.healthy.successes == 0
--- http_config eval
qq{
Expand Down Expand Up @@ -245,6 +244,7 @@ healthy SUCCESS increment
event: target status '127.0.0.1 (127.0.0.1:2119)' from 'false' to 'true'



=== TEST 4: report_http_status() with success is a nop when active.healthy.successes == 0
--- http_config eval
qq{
Expand Down Expand Up @@ -312,6 +312,7 @@ healthy SUCCESS increment
event: target status '127.0.0.1 (127.0.0.1:2119)' from 'false' to 'true'



=== TEST 5: report_http_status() with failure is a nop when passive.unhealthy.http_failures == 0
--- http_config eval
qq{
Expand Down Expand Up @@ -379,7 +380,8 @@ unhealthy HTTP increment
event: target status '127.0.0.1 (127.0.0.1:2119)' from 'true' to 'false'


=== TEST 4: report_http_status() with success is a nop when active.unhealthy.http_failures == 0

=== TEST 6: report_http_status() with success is a nop when active.unhealthy.http_failures == 0
--- http_config eval
qq{
$::HttpConfig
Expand Down
3 changes: 1 addition & 2 deletions t/07-report_tcp_failure.t
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,6 @@ run_tests();

__DATA__



=== TEST 1: report_tcp_failure() active + passive
--- http_config eval
qq{
Expand Down Expand Up @@ -96,6 +94,7 @@ unhealthy TCP increment (3/3) for '(127.0.0.1:2113)'
event: target status '(127.0.0.1:2113)' from 'true' to 'false'



=== TEST 2: report_tcp_failure() for active is a nop when active.unhealthy.tcp_failures == 0
--- http_config eval
qq{
Expand Down
3 changes: 1 addition & 2 deletions t/08-report_timeout.t
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,6 @@ run_tests();

__DATA__



=== TEST 1: report_timeout() active + passive
--- http_config eval
qq{
Expand Down Expand Up @@ -94,6 +92,7 @@ unhealthy TIMEOUT increment (2/2) for '(127.0.0.1:2113)'
event: target status '(127.0.0.1:2113)' from 'true' to 'false'



=== TEST 2: report_timeout() for active is a nop when active.unhealthy.timeouts == 0
--- http_config eval
qq{
Expand Down
7 changes: 4 additions & 3 deletions t/09-active_probes.t
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,6 @@ run_tests();

__DATA__



=== TEST 1: active probes, http node failing
--- http_config eval
qq{
Expand Down Expand Up @@ -127,6 +125,8 @@ healthy SUCCESS increment (3/3) for '(127.0.0.1:2114)'
event: target status '(127.0.0.1:2114)' from 'false' to 'true'
checking unhealthy targets: nothing to do



=== TEST 3: active probes, custom http status (regression test for pre-filled defaults)
--- http_config eval
qq{
Expand Down Expand Up @@ -183,6 +183,7 @@ unhealthy HTTP increment (3/3) for '(127.0.0.1:2114)'
event: target status '(127.0.0.1:2114)' from 'true' to 'false'



=== TEST 4: active probes, custom http status, node failing
--- http_config eval
qq{
Expand Down Expand Up @@ -295,6 +296,7 @@ event: target status 'example.com(127.0.0.1:2114)' from 'false' to 'true'
checking unhealthy targets: nothing to do



=== TEST 6: active probes, tcp node failing
--- http_config eval
qq{
Expand Down Expand Up @@ -395,4 +397,3 @@ healthy SUCCESS increment (2/3) for '(127.0.0.1:2114)'
healthy SUCCESS increment (3/3) for '(127.0.0.1:2114)'
event: target status '(127.0.0.1:2114)' from 'false' to 'true'
checking unhealthy targets: nothing to do

2 changes: 0 additions & 2 deletions t/10-garbagecollect.t
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,6 @@ run_tests();

__DATA__



=== TEST 1: garbage collect the checker object
--- http_config eval
qq{
Expand Down
4 changes: 4 additions & 0 deletions t/11-clear.t
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,8 @@ initial target list (9 targets)
initial target list (10 targets)
initial target list (11 targets)



=== TEST 2: clear() clears the list, other checkers get notified and clear too
--- http_config eval: $::HttpConfig
--- config
Expand Down Expand Up @@ -115,6 +117,8 @@ checking unhealthy targets: nothing to do
--- no_error_log
checking unhealthy targets: #10



=== TEST 3: clear() resets counters
--- http_config eval
qq{
Expand Down
11 changes: 10 additions & 1 deletion t/12-set_target_status.t
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,9 @@ GET /t
true
false
true



=== TEST 2: set_target_status() restores node after passive check disables it
--- http_config eval
qq{
Expand Down Expand Up @@ -86,6 +89,9 @@ GET /t
true
false
true



=== TEST 3: set_target_status() resets the failure counters
--- http_config eval
qq{
Expand Down Expand Up @@ -129,7 +135,10 @@ GET /t
true
true
false
=== TEST 3: set_target_status() resets the success counters



=== TEST 4: set_target_status() resets the success counters
--- http_config eval
qq{
$::HttpConfig
Expand Down
2 changes: 0 additions & 2 deletions t/13-integration.t
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,6 @@ run_tests();

__DATA__



=== TEST 1: ensure counters work properly
--- http_config eval
qq{
Expand Down
Loading