Skip to content

Commit

Permalink
Fix in-place updates over ineligible nodes (#12264)
Browse files Browse the repository at this point in the history
  • Loading branch information
jorgemarey committed Apr 6, 2022
1 parent 12b7647 commit cf6ca95
Show file tree
Hide file tree
Showing 4 changed files with 63 additions and 2 deletions.
3 changes: 3 additions & 0 deletions .changelog/12264.txt
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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

0 comments on commit cf6ca95

Please sign in to comment.