-
-
Notifications
You must be signed in to change notification settings - Fork 5.7k
Adds definitions to backend kv template for health checking #1644
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
Conversation
emilevauge
left a comment
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.
Great!
Minor improvement possible in the template ;)
Other than that, looks good!
templates/kv.tmpl
Outdated
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.
We may move {{$interval := Get "30s" . "/healthcheck/" "interval"}} below {{with $healthCheck}} ?
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 followed the same format as the load balancer lines above (where {{$sticky := Get "false" . "/loadbalancer/" "sticky"}} is outside the with). I can move it inside the {{with $healthCheck}} if you want.
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.
It's not mandatory, but it would be better :) And yes, we could do the same thing with sticky.
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.
@emilevauge I moved the interval Get into the with block.
ldez
left a comment
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.
Thanks!
LGTM
emilevauge
left a comment
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.
Thanks @zachomedia
LGTM
dff989c to
5917e1a
Compare
Description
This PR adds definitions to the KV template to load backend health check information (path and interval).
I've only tested this change with the etcd backend.