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

fix: invalid error after passive health check is changed #5589

Merged
merged 3 commits into from
Nov 24, 2021
Merged

fix: invalid error after passive health check is changed #5589

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
44cb583
fix: invalid error after passive health check is changed
shuaijinchao Nov 23, 2021
839b611
chore: update schema defined
shuaijinchao Nov 23, 2021
8cad3e6
chore: update health-check document parameter range
shuaijinchao Nov 24, 2021
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
22 changes: 18 additions & 4 deletions apisix/schema_def.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,7 @@ local health_checker = {
},
successes = {
type = "integer",
minimum = 1,
minimum = 0,
maximum = 254,
default = 5
}
Expand All @@ -259,24 +259,38 @@ local health_checker = {
},
tcp_failures = {
type = "integer",
minimum = 1,
minimum = 0,
maximum = 254,
default = 2
},
timeouts = {
type = "integer",
minimum = 1,
minimum = 0,
maximum = 254,
default = 7
},
http_failures = {
type = "integer",
minimum = 1,
minimum = 0,
maximum = 254,
default = 5
},
}
}
},
default = {
type = "http",
healthy = {
http_statuses = { 200, 201, 202, 203, 204, 205, 206, 207, 208, 226,
300, 301, 302, 303, 304, 305, 306, 307, 308 },
successes = 0,
},
unhealthy = {
http_statuses = { 429, 500, 503 },
tcp_failures = 0,
Copy link
Member

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?

Copy link
Member Author

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

Copy link
Member Author

Choose a reason for hiding this comment

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

document updated

timeouts = 0,
http_failures = 0,
},
}
}
},
Expand Down
8 changes: 4 additions & 4 deletions docs/en/latest/health-check.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,11 +55,11 @@ it whether this unique node is healthy or not.
| upstream.checks.active.unhealthy.tcp_failures | Active check (unhealthy node) | integer | `1` to `254` | 2 | Active check (unhealthy node) TCP type check, determine the number of times that the node is not healthy. |
| upstream.checks.active.unhealthy.timeouts | Active check (unhealthy node) | integer | `1` to `254` | 3 | Active check (unhealthy node) to determine the number of timeouts for unhealthy nodes. |
| upstream.checks.passive.healthy.http_statuses | Passive check (healthy node) | array | `200` to `599` | [200, 201, 202, 203, 204, 205, 206, 207, 208, 226, 300, 301, 302, 303, 304, 305, 306, 307, 308] | Passive check (healthy node) HTTP or HTTPS type check, the HTTP status code of the healthy node. |
| upstream.checks.passive.healthy.successes | Passive check (healthy node) | integer | `1` to `254` | 5 | Passive checks (healthy node) determine the number of times a node is healthy. |
| upstream.checks.passive.healthy.successes | Passive check (healthy node) | integer | `0` to `254` | 5 | Passive checks (healthy node) determine the number of times a node is healthy. |
| upstream.checks.passive.unhealthy.http_statuses | Passive check (unhealthy node) | array | `200` to `599` | [429, 500, 503] | Passive check (unhealthy node) HTTP or HTTPS type check, the HTTP status code of the non-healthy node. |
| upstream.checks.passive.unhealthy.tcp_failures | Passive check (unhealthy node) | integer | `1` to `254` | 2 | Passive check (unhealthy node) When TCP type is checked, determine the number of times that the node is not healthy. |
| upstream.checks.passive.unhealthy.timeouts | Passive check (unhealthy node) | integer | `1` to `254` | 7 | Passive checks (unhealthy node) determine the number of timeouts for unhealthy nodes. |
| upstream.checks.passive.unhealthy.http_failures | Passive check (unhealthy node) | integer | `1` to `254` | 5 | Passive check (unhealthy node) The number of times that the node is not healthy during HTTP or HTTPS type checking. |
| upstream.checks.passive.unhealthy.tcp_failures | Passive check (unhealthy node) | integer | `0` to `254` | 2 | Passive check (unhealthy node) When TCP type is checked, determine the number of times that the node is not healthy. |
| upstream.checks.passive.unhealthy.timeouts | Passive check (unhealthy node) | integer | `0` to `254` | 7 | Passive checks (unhealthy node) determine the number of timeouts for unhealthy nodes. |
| upstream.checks.passive.unhealthy.http_failures | Passive check (unhealthy node) | integer | `0` to `254` | 5 | Passive check (unhealthy node) The number of times that the node is not healthy during HTTP or HTTPS type checking. |

### Configuration example

Expand Down
8 changes: 4 additions & 4 deletions docs/zh/latest/health-check.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,11 +53,11 @@ Apache APISIX 的健康检查使用 [lua-resty-healthcheck](https://github.com/K
| upstream.checks.active.unhealthy.tcp_failures | 主动检查（非健康节点） | integer | `1` 至 `254` | 2 | 主动检查（非健康节点）TCP 类型检查时，确定节点非健康的次数。 |
| upstream.checks.active.unhealthy.timeouts | 主动检查（非健康节点） | integer | `1` 至 `254` | 3 | 主动检查（非健康节点）确定节点非健康的超时次数。 |
| upstream.checks.passive.healthy.http_statuses | 被动检查（健康节点） | array | `200` 至 `599` | [200, 201, 202, 203, 204, 205, 206, 207, 208, 226, 300, 301, 302, 303, 304, 305, 306, 307, 308] | 被动检查（健康节点） HTTP 或 HTTPS 类型检查时，健康节点的HTTP状态码。 |
| upstream.checks.passive.healthy.successes | 被动检查（健康节点） | integer | `1` 至 `254` | 5 | 被动检查（健康节点）确定节点健康的次数。 |
| upstream.checks.passive.healthy.successes | 被动检查（健康节点） | integer | `0` 至 `254` | 5 | 被动检查（健康节点）确定节点健康的次数。 |
| upstream.checks.passive.unhealthy.http_statuses | 被动检查（非健康节点） | array | `200` 至 `599` | [429, 500, 503] | 被动检查（非健康节点） HTTP 或 HTTPS 类型检查时，非健康节点的HTTP状态码。 |
| upstream.checks.passive.unhealthy.tcp_failures | 被动检查（非健康节点） | integer | `1` 至 `254` | 2 | 被动检查（非健康节点）TCP 类型检查时，确定节点非健康的次数。 |
| upstream.checks.passive.unhealthy.timeouts | 被动检查（非健康节点） | integer | `1` 至 `254` | 7 | 被动检查（非健康节点）确定节点非健康的超时次数。 |
| upstream.checks.passive.unhealthy.http_failures | 被动检查（非健康节点） | integer | `1` 至 `254` | 5 | 被动检查（非健康节点）HTTP 或 HTTPS 类型检查时，确定节点非健康的次数。 |
| upstream.checks.passive.unhealthy.tcp_failures | 被动检查（非健康节点） | integer | `0` 至 `254` | 2 | 被动检查（非健康节点）TCP 类型检查时，确定节点非健康的次数。 |
| upstream.checks.passive.unhealthy.timeouts | 被动检查（非健康节点） | integer | `0` 至 `254` | 7 | 被动检查（非健康节点）确定节点非健康的超时次数。 |
| upstream.checks.passive.unhealthy.http_failures | 被动检查（非健康节点） | integer | `0` 至 `254` | 5 | 被动检查（非健康节点）HTTP 或 HTTPS 类型检查时，确定节点非健康的次数。 |

### 配置示例：

Expand Down
114 changes: 61 additions & 53 deletions t/admin/health-check.t
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,15 @@ add_block_preprocessor(sub {
_EOC_

$block->set_value("init_by_lua_block", $init_by_lua_block);

if ((!defined $block->error_log) && (!defined $block->no_error_log)) {
$block->set_value("no_error_log", "[error]");
}

if (!defined $block->request) {
$block->set_value("request", "GET /t");
}

});

run_tests;
Expand Down Expand Up @@ -90,12 +99,8 @@ __DATA__
ngx.say(body)
}
}
--- request
GET /t
--- response_body
passed
--- no_error_log
[error]



Expand Down Expand Up @@ -137,12 +142,8 @@ passed
ngx.say(body)
}
}
--- request
GET /t
--- response_body
passed
--- no_error_log
[error]



Expand All @@ -166,13 +167,9 @@ passed
ngx.print(body)
}
}
--- request
GET /t
--- error_code: 400
--- response_body
{"error_msg":"invalid configuration: property \"upstream\" validation failed: property \"checks\" validation failed: property \"active\" validation failed: property \"healthy\" validation failed: property \"successes\" validation failed: expected 255 to be smaller than 254"}
--- no_error_log
[error]



Expand All @@ -196,13 +193,9 @@ GET /t
ngx.print(body)
}
}
--- request
GET /t
--- error_code: 400
--- response_body
{"error_msg":"invalid configuration: property \"upstream\" validation failed: property \"checks\" validation failed: property \"active\" validation failed: property \"healthy\" validation failed: property \"successes\" validation failed: expected 0 to be greater than 1"}
--- no_error_log
[error]



Expand All @@ -226,13 +219,9 @@ GET /t
ngx.print(body)
}
}
--- request
GET /t
--- error_code: 400
--- response_body
{"error_msg":"invalid configuration: property \"upstream\" validation failed: property \"checks\" validation failed: property \"passive\" validation failed: property \"unhealthy\" validation failed: property \"http_statuses\" validation failed: failed to validate item 2: expected 600 to be smaller than 599"}
--- no_error_log
[error]



Expand All @@ -254,13 +243,9 @@ GET /t
ngx.print(body)
}
}
--- request
GET /t
--- error_code: 400
--- response_body
{"error_msg":"invalid configuration: property \"upstream\" validation failed: property \"checks\" validation failed: property \"active\" validation failed: property \"type\" validation failed: matches none of the enum values"}
--- no_error_log
[error]



Expand All @@ -284,13 +269,9 @@ GET /t
ngx.print(body)
}
}
--- request
GET /t
--- error_code: 400
--- response_body
{"error_msg":"invalid configuration: property \"upstream\" validation failed: property \"checks\" validation failed: property \"active\" validation failed: property \"healthy\" validation failed: property \"http_statuses\" validation failed: expected unique items but items 1 and 2 are equal"}
--- no_error_log
[error]



Expand All @@ -314,13 +295,9 @@ GET /t
ngx.print(body)
}
}
--- request
GET /t
--- error_code: 400
--- response_body
{"error_msg":"invalid configuration: property \"upstream\" validation failed: property \"checks\" validation failed: property \"active\" validation failed: property \"unhealthy\" validation failed: property \"http_failures\" validation failed: wrong type: expected integer, got number"}
--- no_error_log
[error]



Expand Down Expand Up @@ -353,12 +330,8 @@ GET /t
ngx.say(body)
}
}
--- request
GET /t
--- response_body
passed
--- no_error_log
[error]



Expand Down Expand Up @@ -391,12 +364,8 @@ passed
ngx.say(body)
}
}
--- request
GET /t
--- response_body
passed
--- no_error_log
[error]



Expand Down Expand Up @@ -429,13 +398,9 @@ passed
ngx.print(body)
}
}
--- request
GET /t
--- error_code: 400
--- response_body
{"error_msg":"invalid configuration: property \"upstream\" validation failed: property \"checks\" validation failed: property \"active\" validation failed: property \"req_headers\" validation failed: failed to validate item 2: wrong type: expected string, got number"}
--- no_error_log
[error]



Expand Down Expand Up @@ -469,17 +434,64 @@ GET /t
ngx.print(body)
}
}
--- request
GET /t
--- error_code: 400
--- response_body
{"error_msg":"invalid configuration: property \"upstream\" validation failed: property \"checks\" validation failed: object matches none of the requireds: [\"active\"] or [\"active\",\"passive\"]"}
--- no_error_log
[error]



=== TEST 13: number type timeout
=== TEST 13: only active
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
=== TEST 13: only active
=== TEST 13: change active & passive to only active

Copy link
Member Author

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?

--- config
location /t {
content_by_lua_block {
local t = require("lib.test_admin").test

req_data.upstream.checks = json.decode([[{
"active": {
"http_path": "/status",
"host": "foo.com",
"healthy": {
"interval": 2,
"successes": 1
},
"unhealthy": {
"interval": 1,
"http_failures": 2
}
}
}]])
exp_data.node.value.upstream.checks.active = req_data.upstream.checks.active
exp_data.node.value.upstream.checks.passive = {
type = "http",
healthy = {
http_statuses = { 200, 201, 202, 203, 204, 205, 206, 207, 208, 226,
300, 301, 302, 303, 304, 305, 306, 307, 308 },
successes = 0,
},
unhealthy = {
http_statuses = { 429, 500, 503 },
tcp_failures = 0,
timeouts = 0,
http_failures = 0,
}
}

local code, body = t('/apisix/admin/routes/1',
ngx.HTTP_PUT,
req_data,
exp_data
)

ngx.status = code
ngx.say(body)
}
}
--- response_body
passed



=== TEST 14: number type timeout
--- config
location /t {
content_by_lua_block {
Expand Down Expand Up @@ -512,9 +524,5 @@ GET /t
ngx.say(body)
}
}
--- request
GET /t
--- response_body
passed
--- no_error_log
[error]