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

Fix in-place updates over ineligible nodes #12264

Merged
merged 2 commits into from Apr 6, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 3 additions & 0 deletions .changelog/12264.txt
@@ -0,0 +1,3 @@
```release-note:bug
scheduler: fixed a bug where in-place updates on ineligible nodes would be ignored
```
11 changes: 9 additions & 2 deletions nomad/plan_apply.go
Expand Up @@ -662,8 +662,6 @@ func evaluateNodePlan(snap *state.StateSnapshot, plan *structs.Plan, nodeID stri
return false, "node is disconnected and contains invalid updates", nil
} else if node.Status != structs.NodeStatusReady {
return false, "node is not ready for placements", nil
} else if node.SchedulingEligibility == structs.NodeSchedulingIneligible {
return false, "node is not eligible", nil
}

// Get the existing allocations that are non-terminal
Expand All @@ -672,6 +670,15 @@ func evaluateNodePlan(snap *state.StateSnapshot, plan *structs.Plan, nodeID stri
return false, "", fmt.Errorf("failed to get existing allocations for '%s': %v", nodeID, err)
}

// If nodeAllocations is a subset of the existing allocations we can continue,
// even if the node is not eligible, as only in-place updates or stop/evict are performed
if structs.AllocSubset(existingAlloc, plan.NodeAllocation[nodeID]) {
return true, "", nil
}
if node.SchedulingEligibility == structs.NodeSchedulingIneligible {
return false, "node is not eligible", nil
}

// Determine the proposed allocation by first removing allocations
// that are planned evictions and adding the new allocations.
var remove []*structs.Allocation
Expand Down
33 changes: 33 additions & 0 deletions nomad/plan_apply_test.go
Expand Up @@ -887,6 +887,39 @@ func TestPlanApply_EvalNodePlan_UpdateExisting(t *testing.T) {
}
}

func TestPlanApply_EvalNodePlan_UpdateExisting_Ineligible(t *testing.T) {
t.Parallel()
alloc := mock.Alloc()
state := testStateStore(t)
node := mock.Node()
node.ReservedResources = nil
node.Reserved = nil
node.SchedulingEligibility = structs.NodeSchedulingIneligible
alloc.NodeID = node.ID
alloc.AllocatedResources = structs.NodeResourcesToAllocatedResources(node.NodeResources)
state.UpsertNode(structs.MsgTypeTestSetup, 1000, node)
state.UpsertAllocs(structs.MsgTypeTestSetup, 1001, []*structs.Allocation{alloc})
snap, _ := state.Snapshot()

plan := &structs.Plan{
Job: alloc.Job,
NodeAllocation: map[string][]*structs.Allocation{
node.ID: {alloc},
},
}

fit, reason, err := evaluateNodePlan(snap, plan, node.ID)
if err != nil {
t.Fatalf("err: %v", err)
}
if !fit {
t.Fatalf("bad")
}
if reason != "" {
t.Fatalf("bad")
}
}

func TestPlanApply_EvalNodePlan_NodeFull_Evict(t *testing.T) {
ci.Parallel(t)
alloc := mock.Alloc()
Expand Down
18 changes: 18 additions & 0 deletions nomad/structs/funcs.go
Expand Up @@ -64,6 +64,24 @@ func RemoveAllocs(allocs []*Allocation, remove []*Allocation) []*Allocation {
return r
}

func AllocSubset(allocs []*Allocation, subset []*Allocation) bool {
if len(subset) == 0 {
return true
}
// Convert allocs into a map
allocMap := make(map[string]struct{})
for _, alloc := range allocs {
allocMap[alloc.ID] = struct{}{}
}

for _, alloc := range subset {
if _, ok := allocMap[alloc.ID]; !ok {
return false
}
}
return true
}

// FilterTerminalAllocs filters out all allocations in a terminal state and
// returns the latest terminal allocations.
func FilterTerminalAllocs(allocs []*Allocation) ([]*Allocation, map[string]*Allocation) {
Expand Down