fix: use Score as primary sort key in GPU scheduler policy

pkg/scheduler/numa_sort_test.go

@@ -0,0 +1,114 @@
+/*
+Copyright 2024 The HAMi Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package scheduler
+
+import (
+	"sort"
+	"testing"
+
+	"gotest.tools/v3/assert"
+	corev1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+
+	"github.com/Project-HAMi/HAMi/pkg/device"
+	"github.com/Project-HAMi/HAMi/pkg/device/nvidia"
+	"github.com/Project-HAMi/HAMi/pkg/scheduler/policy"
+)
+
+// sortedDevices mirrors what fitInDevices does: it wraps the devices in a
+// DeviceUsageList (carrying Policy and NumaBind), sorts them, and returns the
+// sorted device slice in the same order Fit() will iterate.
+func sortedDevices(devs []*device.DeviceUsage, scores []float32, policyName string, numaBind bool) []*device.DeviceUsage {
+	dl := policy.DeviceUsageList{Policy: policyName, NumaBind: numaBind}
+	for i, d := range devs {
+		dl.DeviceLists = append(dl.DeviceLists, &policy.DeviceListsScore{Device: d, Score: scores[i]})
+	}
+	sort.Sort(dl)
+	out := make([]*device.DeviceUsage, 0, len(dl.DeviceLists))
+	for _, ds := range dl.DeviceLists {
+		out = append(out, ds.Device)
+	}
+	return out
+}
+
+func numaTestNvidia() *nvidia.NvidiaGPUDevices {
+	return nvidia.InitNvidiaDevice(nvidia.NvidiaConfig{
+		ResourceCountName:            "nvidia.com/gpu",
+		ResourceMemoryName:           "nvidia.com/gpumem",
+		ResourceCoreName:             "nvidia.com/gpucores",
+		ResourceMemoryPercentageName: "nvidia.com/gpumem-percentage",
+	})
+}
+
+// A 2-card numa-bind request fits only on NUMA 0 (two devices there, one on
+// NUMA 1 with a middle score). Fit needs a contiguous same-NUMA run, so the
+// sort decides the outcome: score-primary interleaves NUMA 1 and fails;
+// NumaBind grouping keeps NUMA 0 together and fits.
+func TestNumaBindSortPreservesAffinity(t *testing.T) {
+	nv := numaTestNvidia()
+	mk := func(id string, numa int) *device.DeviceUsage {
+		return &device.DeviceUsage{
+			ID: id, Count: 10, Used: 0, Totalmem: 8192, Totalcore: 100,
+			Type: nvidia.NvidiaGPUDevice, Health: true, Numa: numa,
+		}
+	}
+	// A(n0,score5), B(n0,score1), C(n1,score3): score-primary spread interleaves
+	// C between A and B.
+	devs := []*device.DeviceUsage{mk("A_n0", 0), mk("B_n0", 0), mk("C_n1", 1)}
+	scores := []float32{5, 1, 3}
+
+	req := device.ContainerDeviceRequest{Nums: 2, Memreq: 100, Coresreq: 10, Type: nvidia.NvidiaGPUDevice}
+	pod := &corev1.Pod{ObjectMeta: metav1.ObjectMeta{
+		Annotations: map[string]string{nvidia.NumaBind: "true"},
+	}}
+
+	// Without NUMA grouping the same-NUMA run is broken and Fit fails.
+	unfit, _, _ := nv.Fit(sortedDevices(devs, scores, "spread", false), req, pod, &device.NodeInfo{}, &device.PodDevices{})
+	assert.Equal(t, unfit, false)
+
+	// With NUMA grouping (our fix) the two NUMA-0 devices stay contiguous and fit.
+	fit, result, _ := nv.Fit(sortedDevices(devs, scores, "spread", true), req, pod, &device.NodeInfo{}, &device.PodDevices{})
+	assert.Equal(t, fit, true)
+	assert.Equal(t, len(result[nvidia.NvidiaGPUDevice]), 2)
+	for _, r := range result[nvidia.NvidiaGPUDevice] {
+		assert.Assert(t, r.UUID == "A_n0" || r.UUID == "B_n0")
+	}
+}
+
+// #1806: without numa-bind, Score wins across NUMA. The idlest device is on
+// the lower NUMA id, which the old NUMA-primary sort would skip. Score-primary
+// picks the globally idlest device regardless of NUMA.
+func TestScorePrimarySelectsAcrossNuma(t *testing.T) {
+	nv := numaTestNvidia()
+	mk := func(id string, numa int) *device.DeviceUsage {
+		return &device.DeviceUsage{
+			ID: id, Count: 10, Used: 0, Totalmem: 8192, Totalcore: 100,
+			Type: nvidia.NvidiaGPUDevice, Health: true, Numa: numa,
+		}
+	}
+	// P(n0,score1)=idlest, Q(n1,score8), R(n1,score5). spread must pick P.
+	devs := []*device.DeviceUsage{mk("P_n0", 0), mk("Q_n1", 1), mk("R_n1", 1)}
+	scores := []float32{1, 8, 5}
+
+	req := device.ContainerDeviceRequest{Nums: 1, Memreq: 100, Coresreq: 10, Type: nvidia.NvidiaGPUDevice}
+	pod := &corev1.Pod{ObjectMeta: metav1.ObjectMeta{}}
+
+	fit, result, _ := nv.Fit(sortedDevices(devs, scores, "spread", false), req, pod, &device.NodeInfo{}, &device.PodDevices{})
+	assert.Equal(t, fit, true)
+	assert.Equal(t, len(result[nvidia.NvidiaGPUDevice]), 1)
+	assert.Equal(t, result[nvidia.NvidiaGPUDevice][0].UUID, "P_n0")
+}

pkg/scheduler/policy/gpu_policy.go

@@ -32,6 +32,9 @@ type DeviceListsScore struct {
 type DeviceUsageList struct {
 	DeviceLists []*DeviceListsScore
 	Policy      string
+	// NumaBind groups devices by NUMA so Fit can accumulate a same-NUMA run.
+	// When false, Score is the primary key and NUMA only breaks ties (#1806).
+	NumaBind bool
 }
 
 func (l DeviceUsageList) Len() int {
@@ -43,17 +46,37 @@ func (l DeviceUsageList) Swap(i, j int) {
 }
 
 func (l DeviceUsageList) Less(i, j int) bool {
-	if l.Policy == util.GPUSchedulerPolicyBinpack.String() {
-		if l.DeviceLists[i].Device.Numa == l.DeviceLists[j].Device.Numa {
-			return l.DeviceLists[i].Score < l.DeviceLists[j].Score
+	si, sj := l.DeviceLists[i].Score, l.DeviceLists[j].Score
+	ni, nj := l.DeviceLists[i].Device.Numa, l.DeviceLists[j].Device.Numa
+	binpack := l.Policy == util.GPUSchedulerPolicyBinpack.String()
+
+	// numa-bind: keep NUMA groups contiguous for Fit's same-NUMA accumulation.
+	if l.NumaBind {
+		if binpack {
+			if ni == nj {
+				return si < sj
+			}
+			return ni > nj
 		}
-		return l.DeviceLists[i].Device.Numa > l.DeviceLists[j].Device.Numa
+		// default policy is spread
+		if ni == nj {
+			return si > sj
+		}
+		return ni < nj
+	}
+
+	// score primary, NUMA tiebreaker (#1806).
+	if binpack {
+		if si != sj {
+			return si < sj
+		}
+		return ni < nj
 	}
 	// default policy is spread
-	if l.DeviceLists[i].Device.Numa == l.DeviceLists[j].Device.Numa {
-		return l.DeviceLists[i].Score > l.DeviceLists[j].Score
+	if si != sj {
+		return si > sj
 	}
-	return l.DeviceLists[i].Device.Numa < l.DeviceLists[j].Device.Numa
+	return ni < nj
 }
 
 func (l DeviceUsageList) DeepCopy() DeviceUsageList {
@@ -67,6 +90,7 @@ func (l DeviceUsageList) DeepCopy() DeviceUsageList {
 	return DeviceUsageList{
 		DeviceLists: deviceLists,
 		Policy:      l.Policy,
+		NumaBind:    l.NumaBind,
 	}
 }
 

pkg/scheduler/policy/gpu_policy_test.go

@@ -124,6 +124,7 @@ func TestDeviceUsageList_Less(t *testing.T) {
 	tests := []struct {
 		name         string
 		policy       string
+		numaBind     bool
 		deviceLists  []*DeviceListsScore
 		expectedLess bool
 	}{
@@ -137,7 +138,7 @@ func TestDeviceUsageList_Less(t *testing.T) {
 			expectedLess: true,
 		},
 		{
-			name:   "Binpack policy with different Numa true",
+			name:   "Binpack: score primary, lower score sorts first across NUMA",
 			policy: "binpack",
 			deviceLists: []*DeviceListsScore{
 				{Device: &device.DeviceUsage{Numa: 1, Used: 10}, Score: 10},
@@ -146,16 +147,34 @@ func TestDeviceUsageList_Less(t *testing.T) {
 			expectedLess: true,
 		},
 		{
-			name:   "Binpack policy with different Numa false",
+			name:   "Binpack: score primary, higher score sorts last even on lower NUMA",
 			policy: "binpack",
 			deviceLists: []*DeviceListsScore{
 				{Device: &device.DeviceUsage{Numa: 0, Used: 10}, Score: 10},
 				{Device: &device.DeviceUsage{Numa: 1, Used: 20}, Score: 20},
 			},
+			expectedLess: true,
+		},
+		{
+			name:   "Binpack: NUMA tiebreaker when scores equal",
+			policy: "binpack",
+			deviceLists: []*DeviceListsScore{
+				{Device: &device.DeviceUsage{Numa: 0}, Score: 15},
+				{Device: &device.DeviceUsage{Numa: 1}, Score: 15},
+			},
+			expectedLess: true,
+		},
+		{
+			name:   "Binpack: score primary over NUMA, high-NUMA high-score sorts last",
+			policy: "binpack",
+			deviceLists: []*DeviceListsScore{
+				{Device: &device.DeviceUsage{Numa: 1}, Score: 20},
+				{Device: &device.DeviceUsage{Numa: 0}, Score: 10},
+			},
 			expectedLess: false,
 		},
 		{
-			name:   "Spread policy with same Numa",
+			name:   "Spread: score primary, higher score sorts first",
 			policy: "spread",
 			deviceLists: []*DeviceListsScore{
 				{Device: &device.DeviceUsage{Numa: 0, Used: 10}, Score: 10},
@@ -164,7 +183,7 @@ func TestDeviceUsageList_Less(t *testing.T) {
 			expectedLess: false,
 		},
 		{
-			name:   "Spread policy with different Numa false",
+			name:   "Spread: score primary, lower score sorts last across NUMA",
 			policy: "spread",
 			deviceLists: []*DeviceListsScore{
 				{Device: &device.DeviceUsage{Numa: 1, Used: 10}, Score: 10},
@@ -173,11 +192,61 @@ func TestDeviceUsageList_Less(t *testing.T) {
 			expectedLess: false,
 		},
 		{
-			name:   "Spread policy with different Numa true",
+			name:   "Spread: score primary over NUMA, high-NUMA high-score sorts first",
 			policy: "spread",
 			deviceLists: []*DeviceListsScore{
-				{Device: &device.DeviceUsage{Numa: 0, Used: 10}, Score: 10},
-				{Device: &device.DeviceUsage{Numa: 1, Used: 20}, Score: 20},
+				{Device: &device.DeviceUsage{Numa: 1}, Score: 20},
+				{Device: &device.DeviceUsage{Numa: 0}, Score: 10},
+			},
+			expectedLess: true,
+		},
+		{
+			name:   "Spread: NUMA tiebreaker when scores equal",
+			policy: "spread",
+			deviceLists: []*DeviceListsScore{
+				{Device: &device.DeviceUsage{Numa: 0}, Score: 15},
+				{Device: &device.DeviceUsage{Numa: 1}, Score: 15},
+			},
+			expectedLess: true,
+		},
+		// NumaBind=true: NUMA is primary, Score orders within a NUMA node.
+		{
+			name:     "Binpack+NumaBind: NUMA primary, higher NUMA sorts first regardless of Score",
+			policy:   "binpack",
+			numaBind: true,
+			deviceLists: []*DeviceListsScore{
+				{Device: &device.DeviceUsage{Numa: 1}, Score: 5},
+				{Device: &device.DeviceUsage{Numa: 0}, Score: 99},
+			},
+			expectedLess: true,
+		},
+		{
+			name:     "Binpack+NumaBind: same NUMA falls back to lower Score first",
+			policy:   "binpack",
+			numaBind: true,
+			deviceLists: []*DeviceListsScore{
+				{Device: &device.DeviceUsage{Numa: 0}, Score: 10},
+				{Device: &device.DeviceUsage{Numa: 0}, Score: 20},
+			},
+			expectedLess: true,
+		},
+		{
+			name:     "Spread+NumaBind: NUMA primary, lower NUMA sorts first regardless of Score",
+			policy:   "spread",
+			numaBind: true,
+			deviceLists: []*DeviceListsScore{
+				{Device: &device.DeviceUsage{Numa: 0}, Score: 5},
+				{Device: &device.DeviceUsage{Numa: 1}, Score: 99},
+			},
+			expectedLess: true,
+		},
+		{
+			name:     "Spread+NumaBind: same NUMA falls back to higher Score first",
+			policy:   "spread",
+			numaBind: true,
+			deviceLists: []*DeviceListsScore{
+				{Device: &device.DeviceUsage{Numa: 0}, Score: 20},
+				{Device: &device.DeviceUsage{Numa: 0}, Score: 10},
 			},
 			expectedLess: true,
 		},
@@ -187,6 +256,7 @@ func TestDeviceUsageList_Less(t *testing.T) {
 		t.Run(tt.name, func(t *testing.T) {
 			l := DeviceUsageList{
 				Policy:      tt.policy,
+				NumaBind:    tt.numaBind,
 				DeviceLists: tt.deviceLists,
 			}
 			i, j := 0, 1

pkg/scheduler/scheduler.go

@@ -42,6 +42,7 @@ import (
 	extenderv1 "k8s.io/kube-scheduler/extender/v1"
 
 	"github.com/Project-HAMi/HAMi/pkg/device"
+	"github.com/Project-HAMi/HAMi/pkg/device/nvidia"
 	"github.com/Project-HAMi/HAMi/pkg/scheduler/config"
 	"github.com/Project-HAMi/HAMi/pkg/scheduler/policy"
 	"github.com/Project-HAMi/HAMi/pkg/util"
@@ -527,13 +528,27 @@ func (s *Scheduler) InspectAllNodesUsage() *map[string]*NodeUsage {
 	return &snapshot
 }
 
+// numaBindingRequested reports whether the pod requests numa-bind affinity.
+func numaBindingRequested(task *corev1.Pod) bool {
+	if task == nil {
+		return false
+	}
+	v, ok := task.Annotations[nvidia.NumaBind]
+	if !ok {
+		return false
+	}
+	enforce, _ := strconv.ParseBool(v)
+	return enforce
+}
+
 func buildNodeUsage(node *device.NodeInfo, task *corev1.Pod) *NodeUsage {
 	userGPUPolicy := util.GetGPUSchedulerPolicyByPod(device.GPUSchedulerPolicy, task)
 	nodeUsage := &NodeUsage{
 		Node:     node.Node,
 		NodeInfo: node,
 		Devices: policy.DeviceUsageList{
 			Policy:      userGPUPolicy,
+			NumaBind:    numaBindingRequested(task),
 			DeviceLists: make([]*policy.DeviceListsScore, 0),
 		},
 	}