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.

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

reverseproxy: active health check allows configurable health_passes and health_fails #6154

Merged
merged 4 commits into from
Mar 20, 2024

Conversation

ottenhoff
Copy link
Contributor

To fix issue #6152

Please let me know if this is directionally correct.

@mholt
Copy link
Member

mholt commented Mar 14, 2024

Thanks for the PR!

This is directionally correct, yes 😄 It's looking good actually. Does it work well for you?

@ottenhoff ottenhoff marked this pull request as ready for review March 20, 2024 00:48
@ottenhoff
Copy link
Contributor Author

Thanks! The previous code didn't reset the counter after marking a Host (un)healthy. Now this is working well in my basic testing and matches behavior of other load balancers

Copy link
Member

@mholt mholt left a comment

Choose a reason for hiding this comment

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

Gotcha, thanks for the update!

Did another quick pass and it's looking good but I have a few minor quibbles over naming and capitalization 🙃

modules/caddyhttp/reverseproxy/hosts.go Outdated Show resolved Hide resolved
modules/caddyhttp/reverseproxy/hosts.go Outdated Show resolved Hide resolved
@mholt mholt added the under review 🧐 Review is pending before merging label Mar 20, 2024
Copy link
Member

@mholt mholt left a comment

Choose a reason for hiding this comment

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

This looks good now. Thank you for the contribution!!

@mholt mholt merged commit e65b97f into caddyserver:master Mar 20, 2024
25 checks passed
@mholt mholt added this to the v2.8.0 milestone Mar 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
under review 🧐 Review is pending before merging
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants