-
Notifications
You must be signed in to change notification settings - Fork 4.4k
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
Support node and service maintenance mode #606
Conversation
|
||
// Ensure maintenance mode is not already enabled | ||
if _, ok := a.state.Checks()[serviceMaintCheckID]; ok { | ||
return nil |
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.
Should we log something here for information sake?
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.
Yeah, I missed that. Thanks!
|
@sethvargo I do think in this context that PUT is the correct verb, because the resource we are updating is the maintenance state of the given service or node, and not the service as a whole. If we do use PATCH at some point, I'd suggest we do that in a v2 API, since the current set of methods all use PUT in v1. |
I'm going to merge this in. @armon if you want to change the endpoints or anything else feel free, or let me know! |
Support node and service maintenance mode
|
||
var enable bool | ||
raw := params.Get("enable") | ||
switch raw { |
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 should use strconv.ParseBool to be a bit more flexible to things like case.
Looks really good. Minor feedback, could we also add the API client updates for this too? One thing worth discussing is a potential CLI command to easily toggle the node/services. It seems like it would be useful. @sethvargo @mitchellh thoughts? |
@armon CLI tool seems useful. Is that something you envision in consul itself or a plugin of sorts? I'm thinking it should be a separate project. If I'm using a monitoring service, I probably just want a CLI with some auth flags as opposed to all of Consul. |
I was thinking as a built-in, like "consul lock". |
* Update disruptionbudget api version policy/v1beta1 is deprecated in Kubernetes 1.21+
This introduces maintenance mode at both a node and service level. Maintenance mode can be invoked at either scope at runtime to immediately bring the node out of service. This is mostly health check sugar, which works by registering a critical health check right away. Maintenance mode can be enabled or disabled idempotently using the HTTP API endpoints. Maintenance mode is also persistent, so nodes and services will not re-enter the pool unless explicitly told to do so.