-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
feat(balancer) upstream healthcheck headers #8255
Conversation
@@ -73,6 +73,14 @@ local check_verify_certificate = Schema.define { | |||
required = true, | |||
} | |||
|
|||
local check_headers = Schema.define { | |||
type = "array", |
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.
I saw the unique=true
in array, so was just thinking should this rather be a set
then? Is the order a requirement?
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.
Oh, didn't notice that.
I'd like to replace this custom type with typedef.headers
.
bd64c8b
to
fe2beba
Compare
fe2beba
to
2bdcc6b
Compare
Thanks @flrgh, I've created following PR to lua-resty-heathcheck |
The PR to upstream lib is merged, we will be waiting for lua-resty-heathcheck 1.5.x release. |
The lua-resty-healthcheck version is bumped to 1.5.0, 4ceda40 It's ready to merge. |
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.
👍 lgtm!
I think it would be nice to see an integration test case using an upstream with healthcheck headers (even if we're just doubling up on testing the lua-resty-healthcheck API at that point), but IMO this is perfectly good-to-go as-is.
(Actually... @Hayk-S I wonder if the upstream healthcheck headers case would be a good candidate for E2E testing?)
@flrgh I think there will be no harm from additional tests. I will probably get back to you for more details on this. |
Summary
Add option to set upstream active healthcheck HTTP headers.
see:
Full changelog
Issues resolved
ref:
FT-2195