-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
fix: invalid error after passive health check is changed #5589
Conversation
|
||
|
||
|
||
=== TEST 13: number type timeout | ||
=== TEST 13: only active |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
=== TEST 13: only active | |
=== TEST 13: change active & passive to only active |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The purpose of this test is to verify that the default passive configuration will be added in the case of active only. I think this title description is not very accurate, what do you think?
}, | ||
unhealthy = { | ||
http_statuses = { 429, 500, 503 }, | ||
tcp_failures = 0, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it necessary to set a zero default? Do we need to update the doc?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changing the value to 0 will disable the passive health check, which is necessary when only the active check is configured, documentation needs to be updated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
document updated
This issue is introduced in the bugfix: apache#5589 This PR fixes the bug added by that bugfix, and also introduce a new test (TEST 6) to cover issue apache#4077. The fix of that issue is submitted in api7/lua-resty-healthcheck#5 which fixes the bug in an appropriate way. Signed-off-by: spacewander <[email protected]>
What this PR does / why we need it:
FIX #4077
Pre-submission checklist: