fix(nvidia): collapse duplicate MIG candidates in topology scheduling

pkg/device/nvidia/device.go

@@ -814,14 +814,20 @@ func (nv *NvidiaGPUDevices) Fit(devices []*device.DeviceUsage, request device.Co
 			return true, tmpDevs, ""
 		}
 		if len(tmpDevs[k.Type]) > int(originReq) {
+			// A MIG card appears once per free slot; duplicates only inflate the
+			// combination space and win zero-score ties. Collapse when cards suffice.
+			candidates := tmpDevs
+			if distinct := distinctCardCandidates(tmpDevs[k.Type]); len(distinct) >= int(originReq) {
+				candidates = map[string]device.ContainerDevices{k.Type: distinct}
+			}
 			if originReq == 1 {
 				// If requesting a device, select the card with the worst connection to other cards (lowest total score).
-				lowestDevices := computeWorstSingleCard(nodeInfo, request, tmpDevs)
+				lowestDevices := computeWorstSingleCard(nodeInfo, request, candidates)
 				tmpDevs[k.Type] = lowestDevices
 				klog.V(5).InfoS("device allocate success", "pod", klog.KObj(pod), "worst device", lowestDevices)
 			} else {
 				// If requesting multiple devices, select the best combination of cards.
-				combinations := generateCombinations(request, tmpDevs)
+				combinations := generateCombinations(request, candidates)
 				combination := computeBestCombination(nodeInfo, combinations)
 				tmpDevs[k.Type] = combination
 				klog.V(5).InfoS("device allocate success", "pod", klog.KObj(pod), "best device combination", tmpDevs)
@@ -875,6 +881,21 @@ func generateCombinations(request device.ContainerDeviceRequest, tmpDevs map[str
 	return result
 }
 
+// distinctCardCandidates keeps the first candidate of each physical card,
+// preserving the order the Fit loop produced them in.
+func distinctCardCandidates(candidates device.ContainerDevices) device.ContainerDevices {
+	seen := make(map[string]struct{}, len(candidates))
+	distinct := make(device.ContainerDevices, 0, len(candidates))
+	for _, candidate := range candidates {
+		if _, ok := seen[candidate.UUID]; ok {
+			continue
+		}
+		seen[candidate.UUID] = struct{}{}
+		distinct = append(distinct, candidate)
+	}
+	return distinct
+}
+
 func getDevicePairScoreMap(nodeInfo *device.NodeInfo) map[string]*device.DevicePairScore {
 	deviceScoreMap := make(map[string]*device.DevicePairScore)
 

pkg/device/nvidia/device_test.go

@@ -2830,3 +2830,202 @@ func TestNodeDeleted_ReuseAfterDeletion(t *testing.T) {
 	assert.Equal(t, healthy, true, "re-created node should be healthy")
 	assert.Equal(t, needUpdate, true, "re-created node must trigger an update (stale bookkeeping was cleared)")
 }
+
+// migTopologyDevice builds the NVIDIA backend used by the MIG topology tests.
+func migTopologyDevice() *NvidiaGPUDevices {
+	return InitNvidiaDevice(NvidiaConfig{
+		ResourceCountName:            "nvidia.com/gpu",
+		ResourceMemoryName:           "nvidia.com/gpumem",
+		ResourceCoreName:             "nvidia.com/gpucores",
+		ResourceMemoryPercentageName: "nvidia.com/gpumem-percentage",
+	})
+}
+
+// migTopologyDevices builds MIG cards with seven single-slice placements each,
+// so the Fit loop contributes seven interchangeable candidates per card.
+func migTopologyDevices(ids ...string) []*device.DeviceUsage {
+	placements := make([]device.MigPlacement, 0, 7)
+	for start := range uint32(7) {
+		placements = append(placements, device.MigPlacement{Start: start, Size: 1})
+	}
+	devices := make([]*device.DeviceUsage, 0, len(ids))
+	for idx, id := range ids {
+		devices = append(devices, &device.DeviceUsage{
+			ID: id, Index: uint(idx), Used: 0, Count: 7,
+			Totalmem: 81920, Totalcore: 100, Type: NvidiaGPUDevice, Health: true,
+			Mode: MigMode,
+			MigProfiles: []device.MigProfile{
+				{Name: "1g.10gb", MemoryMB: 10240, Core: 14, SliceCount: 1, Placements: placements},
+			},
+		})
+	}
+	return devices
+}
+
+// migTopologyNodeInfo registers cards with no pair score, as on a node where
+// hami.io/node-nvidia-score was never published.
+func migTopologyNodeInfo(ids ...string) *device.NodeInfo {
+	infos := make([]device.DeviceInfo, 0, len(ids))
+	for _, id := range ids {
+		infos = append(infos, device.DeviceInfo{ID: id})
+	}
+	return &device.NodeInfo{Devices: map[string][]device.DeviceInfo{NvidiaGPUDevice: infos}}
+}
+
+func migTopologyPod() *corev1.Pod {
+	return &corev1.Pod{
+		ObjectMeta: metav1.ObjectMeta{
+			Annotations: map[string]string{util.GPUSchedulerPolicyAnnotationKey: util.GPUSchedulerPolicyTopology.String()},
+		},
+	}
+}
+
+func distinctUUIDs(devices device.ContainerDevices) []string {
+	seen := map[string]struct{}{}
+	out := make([]string, 0, len(devices))
+	for _, dev := range devices {
+		if _, ok := seen[dev.UUID]; ok {
+			continue
+		}
+		seen[dev.UUID] = struct{}{}
+		out = append(out, dev.UUID)
+	}
+	return out
+}
+
+// Without pair scores every combination ties at zero, so the first one generated
+// wins; that used to be several slots of one card with the others left idle.
+func TestFit_TopologyMigSpreadsAcrossCardsWithoutScores(t *testing.T) {
+	nv := migTopologyDevice()
+	devices := migTopologyDevices("dev-0", "dev-1", "dev-2")
+	nodeInfo := migTopologyNodeInfo("dev-0", "dev-1", "dev-2")
+
+	req := device.ContainerDeviceRequest{Nums: 2, Memreq: 10240, Coresreq: 10, Type: NvidiaGPUDevice}
+	fit, result, msg := nv.Fit(devices, req, migTopologyPod(), nodeInfo, &device.PodDevices{})
+
+	assert.Equal(t, fit, true, msg)
+	assert.Equal(t, len(result[NvidiaGPUDevice]), 2)
+	assert.Equal(t, len(distinctUUIDs(result[NvidiaGPUDevice])), 2,
+		"a 2-GPU request must land on two physical cards, got %v", distinctUUIDs(result[NvidiaGPUDevice]))
+}
+
+// The reported scenario: eight MIG cards, seven slots each, five GPUs requested.
+// The 56-entry pool enumerated C(56,5) and still packed every slot onto one card.
+func TestFit_TopologyMigSpreadsLargeRequest(t *testing.T) {
+	ids := []string{"dev-0", "dev-1", "dev-2", "dev-3", "dev-4", "dev-5", "dev-6", "dev-7"}
+	nv := migTopologyDevice()
+	devices := migTopologyDevices(ids...)
+	nodeInfo := migTopologyNodeInfo(ids...)
+
+	req := device.ContainerDeviceRequest{Nums: 5, Memreq: 10240, Coresreq: 10, Type: NvidiaGPUDevice}
+	fit, result, msg := nv.Fit(devices, req, migTopologyPod(), nodeInfo, &device.PodDevices{})
+
+	assert.Equal(t, fit, true, msg)
+	assert.Equal(t, len(result[NvidiaGPUDevice]), 5)
+	assert.Equal(t, len(distinctUUIDs(result[NvidiaGPUDevice])), 5,
+		"a 5-GPU request must land on five physical cards, got %v", distinctUUIDs(result[NvidiaGPUDevice]))
+}
+
+// Collapsing the pool must not change which cards topology scoring prefers: the
+// best-connected pair still wins when the node does publish pair scores.
+func TestFit_TopologyMigBestCombinationWithScores(t *testing.T) {
+	nv := migTopologyDevice()
+	devices := migTopologyDevices("dev-0", "dev-1", "dev-2")
+	nodeInfo := &device.NodeInfo{
+		Devices: map[string][]device.DeviceInfo{
+			NvidiaGPUDevice: {
+				{ID: "dev-0", DevicePairScore: device.DevicePairScore{Scores: map[string]int{"dev-1": 100, "dev-2": 200}}},
+				{ID: "dev-1", DevicePairScore: device.DevicePairScore{Scores: map[string]int{"dev-0": 100, "dev-2": 150}}},
+				{ID: "dev-2", DevicePairScore: device.DevicePairScore{Scores: map[string]int{"dev-0": 200, "dev-1": 150}}},
+			},
+		},
+	}
+
+	req := device.ContainerDeviceRequest{Nums: 2, Memreq: 10240, Coresreq: 10, Type: NvidiaGPUDevice}
+	fit, result, msg := nv.Fit(devices, req, migTopologyPod(), nodeInfo, &device.PodDevices{})
+
+	assert.Equal(t, fit, true, msg)
+	assert.Equal(t, len(result[NvidiaGPUDevice]), 2)
+	uuids := distinctUUIDs(result[NvidiaGPUDevice])
+	assert.Equal(t, len(uuids), 2, "got %v", uuids)
+	// dev-0 <-> dev-2 scores 200, the highest pair on this node.
+	assert.Assert(t, uuids[0] == "dev-0" || uuids[1] == "dev-0", "expected dev-0 in %v", uuids)
+	assert.Assert(t, uuids[0] == "dev-2" || uuids[1] == "dev-2", "expected dev-2 in %v", uuids)
+}
+
+// The single-GPU path still picks the least-connected card: duplicates scaled
+// every card's total by its slot count, which left the ranking intact.
+func TestFit_TopologyMigWorstSingleCard(t *testing.T) {
+	nv := migTopologyDevice()
+	devices := migTopologyDevices("dev-0", "dev-1", "dev-2")
+	nodeInfo := &device.NodeInfo{
+		Devices: map[string][]device.DeviceInfo{
+			NvidiaGPUDevice: {
+				{ID: "dev-0", DevicePairScore: device.DevicePairScore{Scores: map[string]int{"dev-1": 100, "dev-2": 200}}},
+				{ID: "dev-1", DevicePairScore: device.DevicePairScore{Scores: map[string]int{"dev-0": 100, "dev-2": 150}}},
+				{ID: "dev-2", DevicePairScore: device.DevicePairScore{Scores: map[string]int{"dev-0": 200, "dev-1": 150}}},
+			},
+		},
+	}
+
+	req := device.ContainerDeviceRequest{Nums: 1, Memreq: 10240, Coresreq: 10, Type: NvidiaGPUDevice}
+	fit, result, msg := nv.Fit(devices, req, migTopologyPod(), nodeInfo, &device.PodDevices{})
+
+	assert.Equal(t, fit, true, msg)
+	assert.Equal(t, len(result[NvidiaGPUDevice]), 1)
+	// dev-1 totals 250, below dev-0 (300) and dev-2 (350).
+	assert.Equal(t, result[NvidiaGPUDevice][0].UUID, "dev-1")
+}
+
+// With fewer cards than requested GPUs the full pool is kept, so MIG instances
+// on one card still satisfy the request instead of it being rejected.
+func TestFit_TopologyMigPacksOneCardWhenNoAlternative(t *testing.T) {
+	nv := migTopologyDevice()
+	devices := migTopologyDevices("dev-0")
+	nodeInfo := migTopologyNodeInfo("dev-0")
+
+	req := device.ContainerDeviceRequest{Nums: 2, Memreq: 10240, Coresreq: 10, Type: NvidiaGPUDevice}
+	fit, result, msg := nv.Fit(devices, req, migTopologyPod(), nodeInfo, &device.PodDevices{})
+
+	assert.Equal(t, fit, true, msg)
+	assert.Equal(t, len(result[NvidiaGPUDevice]), 2)
+	assert.Equal(t, len(distinctUUIDs(result[NvidiaGPUDevice])), 1,
+		"the only card on the node must supply both instances")
+}
+
+func TestDistinctCardCandidates(t *testing.T) {
+	tests := []struct {
+		name       string
+		candidates device.ContainerDevices
+		want       []string
+	}{
+		{
+			name: "collapses repeated cards and keeps first-seen order",
+			candidates: device.ContainerDevices{
+				{UUID: "dev-2", Idx: 2}, {UUID: "dev-2", Idx: 2},
+				{UUID: "dev-0", Idx: 0}, {UUID: "dev-2", Idx: 2},
+				{UUID: "dev-1", Idx: 1}, {UUID: "dev-0", Idx: 0},
+			},
+			want: []string{"dev-2", "dev-0", "dev-1"},
+		},
+		{
+			name:       "already distinct is unchanged",
+			candidates: device.ContainerDevices{{UUID: "dev-0"}, {UUID: "dev-1"}},
+			want:       []string{"dev-0", "dev-1"},
+		},
+		{
+			name:       "empty stays empty",
+			candidates: device.ContainerDevices{},
+			want:       []string{},
+		},
+	}
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			got := distinctCardCandidates(tc.candidates)
+			assert.Equal(t, len(got), len(tc.want))
+			for i := range tc.want {
+				assert.Equal(t, got[i].UUID, tc.want[i])
+			}
+		})
+	}
+}