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

WIP: Smart Node Drain #4005

Closed
wants to merge 221 commits into from
Closed

WIP: Smart Node Drain #4005

wants to merge 221 commits into from

Conversation

dadgar
Copy link
Contributor

@dadgar dadgar commented Mar 19, 2018

This PR introduces a new stanza called migrate that allows a job to specify its run time requirements which are now taken into account during cluster maintenance operations. The new node drainer will taken into account the migrate strategy into account and attempt to avoid service down time.

dadgar and others added 19 commits March 19, 2018 10:34
This PR allows marking a node as eligible for scheduling while toggling
drain. By default the `nomad node drain -disable` commmand will mark it
as eligible but the drainer will maintain in-eligibility.
allow -detach like other commands
Also delay "node complete" after the node has been marked complete to
capture a few more alloc events. There are other ways to implement this
that could trade off correctness for responsiveness as technically a
node is considered drained when all of its allocs have been marked to
stop and not when they've actually stopped (which may not happen for a
long time).
@dadgar dadgar changed the title Smart Node Drain WIP: Smart Node Drain Mar 19, 2018
@@ -455,9 +455,9 @@ func mergeAutocompleteFlags(flags ...complete.Flags) complete.Flags {
return merged
}

// sanitizeUUIDPrefix is used to sanitize a UUID prefix. The returned result
// sanitizeUUIDPrefix is used to sanatize a UUID prefix. The returned result
Copy link
Member

Choose a reason for hiding this comment

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

bad merge

@@ -735,6 +741,82 @@ func TestParse(t *testing.T) {
},
false,
},
{
Copy link
Member

Choose a reason for hiding this comment

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

duplicated. bad merge?

@schmichael
Copy link
Member

I took another crack at rebasing to try to remove the ugly wip commits and fixup the couple (minor) issues I found in this version: #4010 The other benefit of the alternate approach is it's rebased on master and should be easy to continue to rebase if necessary (it shouldn't be 😬 ).

I looked at git diff f-drain-rebased2 and I think all of the differences are whitespace or test ordering. This thing has gotten too unwieldy... 😞

@schmichael schmichael closed this Mar 20, 2018
@schmichael schmichael deleted the f-drain-rebased branch March 20, 2018 18:12
@github-actions
Copy link

github-actions bot commented Mar 9, 2023

I'm going to lock this pull request because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 9, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants