kubelet: consider real CPU topology when check Core/Socket free

kubernetes · Apr 30, 2024 · ec22153 · ec22153
1 parent d07a576
commit ec22153
Show file tree

Hide file tree

Showing 6 changed files with 462 additions and 15 deletions.
diff --git a/pkg/kubelet/cm/cpumanager/cpu_assignment.go b/pkg/kubelet/cm/cpumanager/cpu_assignment.go
@@ -251,13 +251,13 @@ func (a *cpuAccumulator) isNUMANodeFree(numaID int) bool {
 // Returns true if the supplied socket is fully available in `a.details`.
 // "fully available" means that all the CPUs in it are free.
 func (a *cpuAccumulator) isSocketFree(socketID int) bool {
-	return a.details.CPUsInSockets(socketID).Size() == a.topo.CPUsPerSocket()
+	return a.details.CPUsInSockets(socketID).Size() == a.topo.CPUDetails.CPUsInSockets(socketID).Size()
 }
 
 // Returns true if the supplied core is fully available in `a.details`.
 // "fully available" means that all the CPUs in it are free.
 func (a *cpuAccumulator) isCoreFree(coreID int) bool {
-	return a.details.CPUsInCores(coreID).Size() == a.topo.CPUsPerCore()
+	return a.details.CPUsInCores(coreID).Size() == a.topo.CPUDetails.CPUsInCores(coreID).Size()
 }
 
 // Returns free NUMA Node IDs as a slice sorted by sortAvailableNUMANodes().

diff --git a/pkg/kubelet/cm/cpumanager/cpu_assignment_test.go b/pkg/kubelet/cm/cpumanager/cpu_assignment_test.go
@@ -110,6 +110,18 @@ func TestCPUAccumulatorFreeSockets(t *testing.T) {
 			mustParseCPUSet(t, "0-40,42-49,51-68,71-79"),
 			[]int{},
 		},
+		{
+			"multi numa, dual socket, HT, last CPU offline, socket-0 free",
+			topoDualSocketMultiNumaPerSocketHTLargeWithSingleCPUOffline,
+			mustParseCPUSet(t, "0-63,128-191"),
+			[]int{0},
+		},
+		{
+			"multi numa, dual socket, HT, last CPU offline, socket-1 free",
+			topoDualSocketMultiNumaPerSocketHTLargeWithSingleCPUOffline,
+			mustParseCPUSet(t, "64-127,192-254"),
+			[]int{1},
+		},
 	}
 
 	for _, tc := range testCases {
@@ -119,7 +131,6 @@ func TestCPUAccumulatorFreeSockets(t *testing.T) {
 			sort.Ints(result)
 			if !reflect.DeepEqual(result, tc.expect) {
 				t.Errorf("expected %v to equal %v", result, tc.expect)
-
 			}
 		})
 	}
@@ -210,6 +221,24 @@ func TestCPUAccumulatorFreeNUMANodes(t *testing.T) {
 			mustParseCPUSet(t, "0-9,11-59,61-79"),
 			[]int{},
 		},
+		{
+			"multi numa, dual socket, HT, last CPU offline, NUMA node-0 free",
+			topoDualSocketMultiNumaPerSocketHTLargeWithSingleCPUOffline,
+			mustParseCPUSet(t, "0-15,128-143"),
+			[]int{0},
+		},
+		{
+			"multi numa, dual socket, HT, last CPU offline, NUMA node-7 free",
+			topoDualSocketMultiNumaPerSocketHTLargeWithSingleCPUOffline,
+			mustParseCPUSet(t, "112-127,240-254"),
+			[]int{7},
+		},
+		{
+			"multi numa, dual socket, HT, last CPU offline, 0 NUMA nodes free(1 CPU consumed)",
+			topoDualSocketMultiNumaPerSocketHTLargeWithSingleCPUOffline,
+			mustParseCPUSet(t, "0-15,128-142"),
+			[]int{},
+		},
 	}
 
 	for _, tc := range testCases {
@@ -331,6 +360,18 @@ func TestCPUAccumulatorFreeCores(t *testing.T) {
 			cpuset.New(2, 3, 4, 5, 8, 9, 10, 11),
 			[]int{2, 4, 3, 5},
 		},
+		{
+			"multi numa, dual socket, HT, last CPU offline, 0 cores free (1 partially consumed)",
+			topoDualSocketMultiNumaPerSocketHTLargeWithSingleCPUOffline,
+			mustParseCPUSet(t, "0"),
+			[]int{},
+		},
+		{
+			"multi numa, dual socket, HT, last CPU offline, 1 cores free (1 online CPU, 1 offline CPU)",
+			topoDualSocketMultiNumaPerSocketHTLargeWithSingleCPUOffline,
+			mustParseCPUSet(t, "127"),
+			[]int{127},
+		},
 	}
 
 	for _, tc := range testCases {
@@ -854,6 +895,29 @@ func TestTakeByTopologyNUMADistributed(t *testing.T) {
 			"",
 			mustParseCPUSet(t, "43-47,75-79,96,101-105,171-174,203-206,229-232"),
 		},
+		// this case demonstrates that the policy does not guarantee Full Physical CPUs yet. Once the behavior changes, it can be deleted
+		// the provided available Numas are:
+		// - 0,1 both aliged 5 Cores, and 2 CPUs within 2 Cores
+		// - 2,3 both aliged 6 Cores
+		// the result chooses all 24 CPUs in Numa 0,1
+		{
+			"allocate 10 full cores distributed across first 2 NUMA nodes and 4 CPUs spilling over to each of NUMA 0,1",
+			topoDualSocketMultiNumaPerSocketHTLarge,
+			mustParseCPUSet(t, "0-5,129-134,16-21,144-149,32-37,160-165,48-53,176-181"),
+			24,
+			2,
+			"",
+			mustParseCPUSet(t, "0-5,16-21,129-134,144-149"),
+		},
+		{
+			"allocate 10 full cores distributed across first 2 NUMA nodes and 4 CPUs spilling over to each of NUMA 0,1(CPU 255 offline)",
+			topoDualSocketMultiNumaPerSocketHTLargeWithSingleCPUOffline,
+			mustParseCPUSet(t, "0-5,129-134,16-21,144-149,32-37,160-165,48-53,176-181"),
+			24,
+			2,
+			"",
+			mustParseCPUSet(t, "0-5,16-21,129-134,144-149"),
+		},
 	}...)
 
 	for _, tc := range testCases {

diff --git a/pkg/kubelet/cm/cpumanager/policy_static.go b/pkg/kubelet/cm/cpumanager/policy_static.go
@@ -310,7 +310,7 @@ func (p *staticPolicy) Allocate(s state.State, pod *v1.Pod, container *v1.Contai
 	}()
 
 	if p.options.FullPhysicalCPUsOnly {
-		CPUsPerCore := p.topology.CPUsPerCore()
+		CPUsPerCore := p.topology.MaxCPUsPerCore()
 		if (numCPUs % CPUsPerCore) != 0 {
 			// Since CPU Manager has been enabled requesting strict SMT alignment, it means a guaranteed pod can only be admitted
 			// if the CPU requested is a multiple of the number of virtual cpus per physical cores.
@@ -486,7 +486,7 @@ func (p *staticPolicy) takeByTopology(availableCPUs cpuset.CPUSet, numCPUs int)
 	if p.options.DistributeCPUsAcrossNUMA {
 		cpuGroupSize := 1
 		if p.options.FullPhysicalCPUsOnly {
-			cpuGroupSize = p.topology.CPUsPerCore()
+			cpuGroupSize = p.topology.MaxCPUsPerCore()
 		}
 		return takeByTopologyNUMADistributed(p.topology, availableCPUs, numCPUs, cpuGroupSize)
 	}