scheduler: fixed a bug where the bandwidth of reserved cores were not taken into account

Author: mvegter

.changelog/26768.txt

@@ -1,3 +1,3 @@
 ```release-note:bug
-client: fixed a bug where the bandwidth of reserved cores were not taken into account
+scheduler: fixed a bug where the bandwidth of reserved cores were not taken into account
 ```

client/client_test.go

@@ -1691,17 +1691,17 @@ func TestClient_UpdateNodeFromDevicesAccumulates(t *testing.T) {
 func TestClient_UpdateNodeFromFingerprintCalculatesReservedResources(t *testing.T) {
 	ci.Parallel(t)
 
-	// Client without network configured updates to match fingerprint
 	client, cleanup := TestClient(t, func(c *config.Config) {
+		// Parsed version of the client reserved field
 		c.Node.ReservedResources.Cpu = structs.NodeReservedCpuResources{
 			CpuShares:        100,
 			ReservedCpuCores: []uint16{0},
 		}
 	})
 	defer cleanup()
 
-	// Set up a basic topology where the first core is and an
-	// additionally 100 MHz are reserved
+	// Set up a basic topology where the first core is reserved and
+	// an additionally 100 MHz are reserved
 	basicTopology := structs.MockBasicTopology()
 	basicTopology.Cores[0].Disable = true
 	basicTopology.OverrideWitholdCompute = 100
@@ -1720,7 +1720,9 @@ func TestClient_UpdateNodeFromFingerprintCalculatesReservedResources(t *testing.
 
 	expectedReservedResources := &structs.NodeReservedResources{
 		Cpu: structs.NodeReservedCpuResources{
-			CpuShares:        3_600, // 1 * 3500 MHz and 100 MHz
+			// set by fingerprinting callback, once the total compute has been detected, it should have converted the
+			// client reserved configuration into the effective reserved bandwidth (1 core * 3500 MHz + 100 MHz)
+			CpuShares:        3_600,
 			ReservedCpuCores: []uint16{0},
 		},
 		Memory: structs.NodeReservedMemoryResources{

client/lib/numalib/topology.go

@@ -282,17 +282,17 @@ func (st *Topology) NumECores() int {
 	return total
 }
 
-// TotalCores returns the number of logical cores detected. The UsableCores will
+// AllCores returns the set of logical cores detected. The UsableCores will
 // be equal to or less than this value.
-func (st *Topology) TotalCores() *idset.Set[hw.CoreID] {
+func (st *Topology) AllCores() *idset.Set[hw.CoreID] {
 	result := idset.Empty[hw.CoreID]()
 	for _, cpu := range st.Cores {
 		result.Insert(cpu.ID)
 	}
 	return result
 }
 
-// UsableCores returns the number of logical cores usable by the Nomad client
+// UsableCores returns the set of logical cores usable by the Nomad client
 // for running tasks. Nomad must subtract off any reserved cores (reserved.cores)
 // and/or must mask the cpuset to the one set in config (config.reservable_cores).
 func (st *Topology) UsableCores() *idset.Set[hw.CoreID] {

nomad/structs/funcs_test.go

@@ -149,7 +149,9 @@ func node2k() *Node {
 		},
 		ReservedResources: &NodeReservedResources{
 			Cpu: NodeReservedCpuResources{
-				CpuShares:        3000, // set by fingerprinting callback
+				// set by fingerprinting callback, topology of 1000 MHz * 4 cores (4000 MHz), of which 2 cores are reserved
+				// plus 1000 MHz of reserved amount of CPU, effectively a total of 3000 MHz of reserved CPU
+				CpuShares:        3000,
 				ReservedCpuCores: []uint16{2, 3},
 			},
 			Memory: NodeReservedMemoryResources{
@@ -214,9 +216,10 @@ func TestAllocsFit(t *testing.T) {
 	must.Eq(t, 1024, used.Flattened.Memory.MemoryMB)
 
 	// Should not fit second allocation
-	fit, _, used, err = AllocsFit(n, []*Allocation{a1, a1}, nil, false)
+	fit, dim, used, err = AllocsFit(n, []*Allocation{a1, a1}, nil, false)
 	must.NoError(t, err)
 	must.False(t, fit)
+	must.Eq(t, "cpu", dim)
 	must.Eq(t, 2000, used.Flattened.Cpu.CpuShares)
 	must.Eq(t, 2048, used.Flattened.Memory.MemoryMB)
 
@@ -261,38 +264,6 @@ func TestAllocsFit(t *testing.T) {
 	must.Eq(t, 1000, used.Flattened.Cpu.CpuShares)
 	must.Eq(t, []uint16{0}, used.Flattened.Cpu.ReservedCores)
 	must.Eq(t, 1024, used.Flattened.Memory.MemoryMB)
-
-	a3 := &Allocation{
-		AllocatedResources: &AllocatedResources{
-			Tasks: map[string]*AllocatedTaskResources{
-				"web": {
-					Cpu: AllocatedCpuResources{
-						CpuShares: 1000,
-					},
-					Memory: AllocatedMemoryResources{
-						MemoryMB: 512,
-					},
-				},
-			},
-		},
-	}
-
-	// Should fit one allocation
-	fit, dim, used, err = AllocsFit(n, []*Allocation{a3}, nil, false)
-	must.NoError(t, err)
-	must.True(t, fit, must.Sprintf("failed for dimension %q", dim))
-	must.Eq(t, 1000, used.Flattened.Cpu.CpuShares)
-	must.Eq(t, []uint16{}, used.Flattened.Cpu.ReservedCores)
-	must.Eq(t, 512, used.Flattened.Memory.MemoryMB)
-
-	// Should not fit second allocation
-	fit, dim, used, err = AllocsFit(n, []*Allocation{a3, a3}, nil, false)
-	must.NoError(t, err)
-	must.False(t, fit)
-	must.Eq(t, "cpu", dim)
-	must.Eq(t, 2000, used.Flattened.Cpu.CpuShares)
-	must.Eq(t, []uint16{}, used.Flattened.Cpu.ReservedCores)
-	must.Eq(t, 1024, used.Flattened.Memory.MemoryMB)
 }
 
 func TestAllocsFit_Cores(t *testing.T) {
@@ -721,7 +692,9 @@ func TestScoreFitBinPack(t *testing.T) {
 	node.NodeResources.Compatibility()
 	node.ReservedResources = &NodeReservedResources{
 		Cpu: NodeReservedCpuResources{
-			CpuShares:        6144, // set by fingerprinting callback
+			// set by fingerprinting callback, topology of 2048 MHz * 4 cores (8192 MHz), of which 2 cores are reserved
+			// plus 2048 MHz of reserved amount of CPU, effectively a total of 6144 MHz of reserved CPU
+			CpuShares:        6144,
 			ReservedCpuCores: []uint16{2, 3},
 		},
 		Memory: NodeReservedMemoryResources{

nomad/structs/structs.go

@@ -3244,8 +3244,8 @@ func (n *NodeResources) Comparable() *ComparableResources {
 		return nil
 	}
 
-	totalCores := n.Processors.Topology.TotalCores().Slice()
-	cores := helper.ConvertSlice(totalCores, func(id hw.CoreID) uint16 {
+	allCores := n.Processors.Topology.AllCores().Slice()
+	cores := helper.ConvertSlice(allCores, func(id hw.CoreID) uint16 {
 		return uint16(id)
 	})