Skip to content

Commit

Permalink
scheduler: fixed a bug where resource calculation did not account cor…
Browse files Browse the repository at this point in the history
…rectly for poststart tasks (#24297)

Fixes a bug in the AllocatedResources.Comparable method, which resulted in
reporting less required resources than actually expected. This could result in
overscheduling of allocations on a single node  and overlapping cgroup cpusets.
  • Loading branch information
mvegter authored Nov 5, 2024
1 parent f75e2c2 commit 8545e1c
Show file tree
Hide file tree
Showing 3 changed files with 68 additions and 0 deletions.
3 changes: 3 additions & 0 deletions .changelog/24297.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:bug
scheduler: fixed a bug where resource calculation did not account correctly for poststart tasks
```
4 changes: 4 additions & 0 deletions nomad/structs/structs.go
Original file line number Diff line number Diff line change
Expand Up @@ -3859,6 +3859,7 @@ func (a *AllocatedResources) Comparable() *ComparableResources {
prestartSidecarTasks := &AllocatedTaskResources{}
prestartEphemeralTasks := &AllocatedTaskResources{}
main := &AllocatedTaskResources{}
poststartTasks := &AllocatedTaskResources{}
poststopTasks := &AllocatedTaskResources{}

for taskName, r := range a.Tasks {
Expand All @@ -3871,12 +3872,15 @@ func (a *AllocatedResources) Comparable() *ComparableResources {
} else {
prestartEphemeralTasks.Add(r)
}
} else if lc.Hook == TaskLifecycleHookPoststart {
poststartTasks.Add(r)
} else if lc.Hook == TaskLifecycleHookPoststop {
poststopTasks.Add(r)
}
}

// update this loop to account for lifecycle hook
main.Add(poststartTasks)
prestartEphemeralTasks.Max(main)
prestartEphemeralTasks.Max(poststopTasks)
prestartSidecarTasks.Add(prestartEphemeralTasks)
Expand Down
61 changes: 61 additions & 0 deletions nomad/structs/structs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7759,6 +7759,67 @@ func TestComparableResources_Superset(t *testing.T) {
}
}

func TestAllocatedResources_Comparable_Flattened(t *testing.T) {
ci.Parallel(t)

allocationResources := AllocatedResources{
TaskLifecycles: map[string]*TaskLifecycleConfig{
"prestart-task": {
Hook: TaskLifecycleHookPrestart,
Sidecar: false,
},
"prestart-sidecar-task": {
Hook: TaskLifecycleHookPrestart,
Sidecar: true,
},
"poststart-task": {
Hook: TaskLifecycleHookPoststart,
Sidecar: false,
},
"poststop-task": {
Hook: TaskLifecycleHookPoststop,
Sidecar: false,
},
},
Tasks: map[string]*AllocatedTaskResources{
"prestart-task": {
Cpu: AllocatedCpuResources{
CpuShares: 2000,
ReservedCores: []uint16{0, 1},
},
},
"prestart-sidecar-task": {
Cpu: AllocatedCpuResources{
CpuShares: 2000,
ReservedCores: []uint16{2, 3},
},
},
"main-task": {
Cpu: AllocatedCpuResources{
CpuShares: 1000,
ReservedCores: []uint16{4},
},
},
"poststart-task": {
Cpu: AllocatedCpuResources{
CpuShares: 2000,
ReservedCores: []uint16{5, 6},
},
},
"poststop-task": {
Cpu: AllocatedCpuResources{
CpuShares: 2000,
ReservedCores: []uint16{7, 8},
},
},
},
}

// The output of Flattened should return the resource required during the execution of the largest lifecycle
must.Eq(t, 5000, allocationResources.Comparable().Flattened.Cpu.CpuShares)
must.Len(t, 5, allocationResources.Comparable().Flattened.Cpu.ReservedCores)
}

func requireErrors(t *testing.T, err error, expected ...string) {
t.Helper()
require.Error(t, err)
Expand Down

0 comments on commit 8545e1c

Please sign in to comment.