-
Notifications
You must be signed in to change notification settings - Fork 84
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
func: add metadata for drained nodes #637
Conversation
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.
Looking great. I've added a couple of questions, mostly about readability.
Co-authored-by: James Rasell <jrasell@users.noreply.github.com>
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, pending the change to the const naming. We will also need a changelog entry, either in this PR or a follow up to broadcast the goodness.
sdk/helper/scaleutils/node_drain.go
Outdated
DrainSpec: spec, | ||
MarkEligible: false, | ||
Meta: map[string]string{ | ||
NODE_DRAINED_META_KEY: NODE_DRAINED_META_VALUE, |
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.
NODE_DRAINED_META_KEY: NODE_DRAINED_META_VALUE, | |
nodeDrainedMetaKey: nodeDrainedMetaValue, |
Co-authored-by: James Rasell <jrasell@users.noreply.github.com>
Co-authored-by: James Rasell <jrasell@users.noreply.github.com>
Issue found here
The nomad autoscaler should be able to set up specific metadata for nodes that are being drain, in order to pick up where it left off in case of a failure. This PR introduces the change. It also adds an interface to make decoupling from the nomad cluster easier for testing on this feature and maybe some more in the future.