diff --git a/pkg/kubelet/cm/devicemanager/topology_hints.go b/pkg/kubelet/cm/devicemanager/topology_hints.go index 8846122a3d7..267aedf8779 100644 --- a/pkg/kubelet/cm/devicemanager/topology_hints.go +++ b/pkg/kubelet/cm/devicemanager/topology_hints.go @@ -152,12 +152,27 @@ func (m *ManagerImpl) getAvailableDevices(resource string) sets.Set[string] { } func (m *ManagerImpl) generateDeviceTopologyHints(resource string, available sets.Set[string], reusable sets.Set[string], request int) []topologymanager.TopologyHint { - // Initialize minAffinitySize to include all NUMA Nodes - minAffinitySize := len(m.numaNodes) + // Narrow the bitmask iteration to NUMA nodes that actually host + // devices for this resource. On platforms where the OS exposes many + // NUMA nodes that carry no devices (e.g. NVIDIA GB200 with 36 NUMA + // nodes, most hosting only GPU HBM), iterating all machine NUMA + // nodes would enumerate O(2^n) subsets. Restricting to device- + // bearing nodes reduces n to the number of nodes that matter. + // Because device-less nodes never contribute to devicesInMask, + // excluding them does not change minAffinitySize or Preferred + // flag computation. + numaNodes := m.deviceNUMANodes(resource) + if len(numaNodes) == 0 { + numaNodes = m.numaNodes + } + + // Initialize minAffinitySize to the number of NUMA nodes under + // consideration; it will be narrowed as satisfying masks are found. + minAffinitySize := len(numaNodes) // Iterate through all combinations of NUMA Nodes and build hints from them. hints := []topologymanager.TopologyHint{} - bitmask.IterateBitMasks(m.numaNodes, func(mask bitmask.BitMask) { + bitmask.IterateBitMasks(numaNodes, func(mask bitmask.BitMask) { // First, update minAffinitySize for the current request size. devicesInMask := 0 for _, device := range m.allDevices[resource] { @@ -218,6 +233,28 @@ func (m *ManagerImpl) generateDeviceTopologyHints(resource string, available set return hints } +// deviceNUMANodes returns the sorted list of NUMA node IDs that host at least +// one device for the given resource. The returned set is guaranteed to be a +// subset of m.numaNodes: any NUMA IDs reported by device plugins that are not +// known to cadvisor are logged and dropped. +// The caller must hold m.mutex. +func (m *ManagerImpl) deviceNUMANodes(resource string) []int { + nodesWithDevices := sets.New[int]() + for _, device := range m.allDevices[resource] { + nodesWithDevices.Insert(m.getNUMANodeIds(device.Topology)...) + } + + knownNodes := sets.New[int](m.numaNodes...) + unknown := nodesWithDevices.Difference(knownNodes) + if unknown.Len() > 0 { + klog.TODO().Info("Ignoring NUMA node IDs reported by device plugin that are unknown to cadvisor", + "resource", resource, "unknownNodes", sets.List(unknown), "knownNodes", m.numaNodes) + nodesWithDevices = nodesWithDevices.Intersection(knownNodes) + } + + return sets.List(nodesWithDevices) +} + func (m *ManagerImpl) getNUMANodeIds(topology *pluginapi.TopologyInfo) []int { if topology == nil { return nil diff --git a/pkg/kubelet/cm/devicemanager/topology_hints_test.go b/pkg/kubelet/cm/devicemanager/topology_hints_test.go index 77c6cffa43c..04d70a3e2d9 100644 --- a/pkg/kubelet/cm/devicemanager/topology_hints_test.go +++ b/pkg/kubelet/cm/devicemanager/topology_hints_test.go @@ -980,6 +980,367 @@ func TestGetPodTopologyHints(t *testing.T) { } } +func TestDeviceNUMANodes(t *testing.T) { + resource := "testdevice" + deviceOnNode := func(id string, node int) *pluginapi.Device { + return &pluginapi.Device{ + ID: id, + Topology: &pluginapi.TopologyInfo{Nodes: []*pluginapi.NUMANode{{ID: int64(node)}}}, + } + } + + t.Run("collects NUMA nodes from all devices", func(t *testing.T) { + m := ManagerImpl{ + allDevices: NewResourceDeviceInstances(), + numaNodes: []int{0, 3, 5, 7}, + } + m.allDevices[resource] = DeviceInstances{ + "a": deviceOnNode("a", 3), + "b": deviceOnNode("b", 5), + } + + nodes := m.deviceNUMANodes(resource) + expected := []int{3, 5} + if !reflect.DeepEqual(nodes, expected) { + t.Fatalf("expected nodes %v, got %v", expected, nodes) + } + }) + + t.Run("device without topology is ignored", func(t *testing.T) { + m := ManagerImpl{ + allDevices: NewResourceDeviceInstances(), + numaNodes: []int{0, 4}, + } + m.allDevices[resource] = DeviceInstances{ + "a": deviceOnNode("a", 4), + "b": {ID: "b", Topology: nil}, + } + + nodes := m.deviceNUMANodes(resource) + expected := []int{4} + if !reflect.DeepEqual(nodes, expected) { + t.Fatalf("expected nodes %v, got %v", expected, nodes) + } + }) + + t.Run("returns empty when no device has topology", func(t *testing.T) { + m := ManagerImpl{ + allDevices: NewResourceDeviceInstances(), + } + m.allDevices[resource] = DeviceInstances{ + "a": {ID: "a", Topology: nil}, + "b": {ID: "b", Topology: nil}, + } + + nodes := m.deviceNUMANodes(resource) + if len(nodes) != 0 { + t.Fatalf("expected empty nodes, got %v", nodes) + } + }) + + t.Run("unknown NUMA IDs from device plugin are dropped", func(t *testing.T) { + m := ManagerImpl{ + allDevices: NewResourceDeviceInstances(), + numaNodes: []int{0, 1}, + } + m.allDevices[resource] = DeviceInstances{ + "a": deviceOnNode("a", 0), + "b": deviceOnNode("b", 99), + } + + nodes := m.deviceNUMANodes(resource) + expected := []int{0} + if !reflect.DeepEqual(nodes, expected) { + t.Fatalf("expected nodes %v (node 99 should be dropped), got %v", expected, nodes) + } + }) +} + +func TestGenerateDeviceTopologyHintsFiltersNUMANodes(t *testing.T) { + resource := "gpu" + + t.Run("two node machine, device on one node", func(t *testing.T) { + m := ManagerImpl{ + allDevices: NewResourceDeviceInstances(), + numaNodes: []int{0, 1}, + } + m.allDevices[resource] = DeviceInstances{ + "a": { + ID: "a", + Topology: &pluginapi.TopologyInfo{Nodes: []*pluginapi.NUMANode{{ID: 0}}}, + }, + } + + hints := m.generateDeviceTopologyHints(resource, sets.New[string]("a"), nil, 1) + + maskNode0, _ := bitmask.NewBitMask(0) + expected := []topologymanager.TopologyHint{ + {NUMANodeAffinity: maskNode0, Preferred: true}, + } + + if !reflect.DeepEqual(hints, expected) { + t.Fatalf("expected hints %v, got %v", expected, hints) + } + }) + + t.Run("large NUMA machine with devices on small subset", func(t *testing.T) { + allNUMA := make([]int, 34) + for i := range allNUMA { + allNUMA[i] = i + } + + m := ManagerImpl{ + allDevices: NewResourceDeviceInstances(), + numaNodes: allNUMA, + } + m.allDevices[resource] = DeviceInstances{ + "gpu0": { + ID: "gpu0", + Topology: &pluginapi.TopologyInfo{Nodes: []*pluginapi.NUMANode{{ID: 0}}}, + }, + "gpu1": { + ID: "gpu1", + Topology: &pluginapi.TopologyInfo{Nodes: []*pluginapi.NUMANode{{ID: 1}}}, + }, + } + + hints := m.generateDeviceTopologyHints(resource, sets.New[string]("gpu0", "gpu1"), nil, 1) + sort.SliceStable(hints, func(i, j int) bool { return hints[i].LessThan(hints[j]) }) + + maskNode0, _ := bitmask.NewBitMask(0) + maskNode1, _ := bitmask.NewBitMask(1) + maskBoth, _ := bitmask.NewBitMask(0, 1) + expected := []topologymanager.TopologyHint{ + {NUMANodeAffinity: maskNode0, Preferred: true}, + {NUMANodeAffinity: maskNode1, Preferred: true}, + {NUMANodeAffinity: maskBoth, Preferred: false}, + } + sort.SliceStable(expected, func(i, j int) bool { return expected[i].LessThan(expected[j]) }) + + if !reflect.DeepEqual(hints, expected) { + t.Fatalf("expected hints %v, got %v", expected, hints) + } + }) + + t.Run("devices span all NUMA nodes", func(t *testing.T) { + m := ManagerImpl{ + allDevices: NewResourceDeviceInstances(), + numaNodes: []int{0, 1}, + } + m.allDevices[resource] = DeviceInstances{ + "a": { + ID: "a", + Topology: &pluginapi.TopologyInfo{Nodes: []*pluginapi.NUMANode{{ID: 0}}}, + }, + "b": { + ID: "b", + Topology: &pluginapi.TopologyInfo{Nodes: []*pluginapi.NUMANode{{ID: 1}}}, + }, + } + + hints := m.generateDeviceTopologyHints(resource, sets.New[string]("a", "b"), nil, 1) + + fullMask, _ := bitmask.NewBitMask(0, 1) + fullMaskCount := 0 + for _, h := range hints { + if h.NUMANodeAffinity.IsEqual(fullMask) { + fullMaskCount++ + } + } + if fullMaskCount != 1 { + t.Fatalf("expected exactly one full-machine mask, got %d in hints: %v", fullMaskCount, hints) + } + }) + + t.Run("reusable device on filtered node", func(t *testing.T) { + m := ManagerImpl{ + allDevices: NewResourceDeviceInstances(), + numaNodes: []int{0, 1, 2, 3}, + } + m.allDevices[resource] = DeviceInstances{ + "a": { + ID: "a", + Topology: &pluginapi.TopologyInfo{Nodes: []*pluginapi.NUMANode{{ID: 0}}}, + }, + } + + hints := m.generateDeviceTopologyHints(resource, nil, sets.New[string]("a"), 1) + + expected := []topologymanager.TopologyHint{ + {NUMANodeAffinity: makeSocketMask(0), Preferred: true}, + } + if !reflect.DeepEqual(hints, expected) { + t.Fatalf("expected hints %v, got %v", expected, hints) + } + }) + + t.Run("reusable and available on different nodes", func(t *testing.T) { + m := ManagerImpl{ + allDevices: NewResourceDeviceInstances(), + numaNodes: []int{0, 1, 2, 3}, + } + m.allDevices[resource] = DeviceInstances{ + "a": { + ID: "a", + Topology: &pluginapi.TopologyInfo{Nodes: []*pluginapi.NUMANode{{ID: 0}}}, + }, + "b": { + ID: "b", + Topology: &pluginapi.TopologyInfo{Nodes: []*pluginapi.NUMANode{{ID: 1}}}, + }, + } + + hints := m.generateDeviceTopologyHints(resource, sets.New[string]("b"), sets.New[string]("a"), 2) + + expected := []topologymanager.TopologyHint{ + {NUMANodeAffinity: makeSocketMask(0, 1), Preferred: true}, + } + if !reflect.DeepEqual(hints, expected) { + t.Fatalf("expected hints %v, got %v", expected, hints) + } + }) +} + +// TestFilteredDeviceHintsMergeWithOtherProviders exercises policy.Merge with +// the device hints produced by our changed code, without needing to wire up +// real CPU/memory managers. +func TestFilteredDeviceHintsMergeWithOtherProviders(t *testing.T) { + t.Run("device on node 0", func(t *testing.T) { + numaNodes := []int{0, 1} + numaInfo := &topologymanager.NUMAInfo{ + Nodes: numaNodes, + NUMADistances: topologymanager.NUMADistances{}, + } + + m := ManagerImpl{ + allDevices: NewResourceDeviceInstances(), + numaNodes: numaNodes, + } + m.allDevices["gpu"] = DeviceInstances{ + "gpu0": { + ID: "gpu0", + Topology: &pluginapi.TopologyInfo{Nodes: []*pluginapi.NUMANode{{ID: 0}}}, + }, + } + + deviceHints := m.generateDeviceTopologyHints("gpu", sets.New[string]("gpu0"), nil, 1) + + providersHints := []map[string][]topologymanager.TopologyHint{ + {"gpu": deviceHints}, + {"cpu": { + {NUMANodeAffinity: makeSocketMask(0), Preferred: true}, + {NUMANodeAffinity: makeSocketMask(0, 1), Preferred: false}, + }}, + {"memory": { + {NUMANodeAffinity: makeSocketMask(0), Preferred: true}, + {NUMANodeAffinity: makeSocketMask(0, 1), Preferred: false}, + }}, + } + + assertMergePreferred(t, numaInfo, providersHints, makeSocketMask(0)) + }) + + t.Run("device on non-zero node only", func(t *testing.T) { + numaNodes := []int{0, 1} + numaInfo := &topologymanager.NUMAInfo{ + Nodes: numaNodes, + NUMADistances: topologymanager.NUMADistances{}, + } + + m := ManagerImpl{ + allDevices: NewResourceDeviceInstances(), + numaNodes: numaNodes, + } + m.allDevices["gpu"] = DeviceInstances{ + "gpu0": { + ID: "gpu0", + Topology: &pluginapi.TopologyInfo{Nodes: []*pluginapi.NUMANode{{ID: 1}}}, + }, + } + + deviceHints := m.generateDeviceTopologyHints("gpu", sets.New[string]("gpu0"), nil, 1) + + providersHints := []map[string][]topologymanager.TopologyHint{ + {"gpu": deviceHints}, + {"cpu": { + {NUMANodeAffinity: makeSocketMask(1), Preferred: true}, + {NUMANodeAffinity: makeSocketMask(0, 1), Preferred: false}, + }}, + {"memory": { + {NUMANodeAffinity: makeSocketMask(1), Preferred: true}, + {NUMANodeAffinity: makeSocketMask(0, 1), Preferred: false}, + }}, + } + + assertMergePreferred(t, numaInfo, providersHints, makeSocketMask(1)) + }) + + t.Run("high NUMA node IDs without node 0", func(t *testing.T) { + numaNodes := []int{3, 7} + numaInfo := &topologymanager.NUMAInfo{ + Nodes: numaNodes, + NUMADistances: topologymanager.NUMADistances{}, + } + + m := ManagerImpl{ + allDevices: NewResourceDeviceInstances(), + numaNodes: numaNodes, + } + m.allDevices["gpu"] = DeviceInstances{ + "gpu0": { + ID: "gpu0", + Topology: &pluginapi.TopologyInfo{Nodes: []*pluginapi.NUMANode{{ID: 7}}}, + }, + } + + deviceHints := m.generateDeviceTopologyHints("gpu", sets.New[string]("gpu0"), nil, 1) + + providersHints := []map[string][]topologymanager.TopologyHint{ + {"gpu": deviceHints}, + {"cpu": { + {NUMANodeAffinity: makeSocketMask(7), Preferred: true}, + {NUMANodeAffinity: makeSocketMask(3, 7), Preferred: false}, + }}, + {"memory": { + {NUMANodeAffinity: makeSocketMask(7), Preferred: true}, + {NUMANodeAffinity: makeSocketMask(3, 7), Preferred: false}, + }}, + } + + assertMergePreferred(t, numaInfo, providersHints, makeSocketMask(7)) + }) +} + +func assertMergePreferred(t *testing.T, numaInfo *topologymanager.NUMAInfo, providersHints []map[string][]topologymanager.TopologyHint, expectedMask bitmask.BitMask) { + t.Helper() + tCtx := ktesting.Init(t) + for _, policyName := range []string{"best-effort", "restricted"} { + t.Run(policyName, func(t *testing.T) { + var policy topologymanager.Policy + switch policyName { + case "best-effort": + policy = topologymanager.NewBestEffortPolicy(numaInfo, topologymanager.PolicyOptions{}) + case "restricted": + policy = topologymanager.NewRestrictedPolicy(numaInfo, topologymanager.PolicyOptions{}) + } + + bestHint, admit := policy.Merge(tCtx.Logger(), providersHints) + if !admit { + t.Fatalf("expected pod to be admitted under %s policy", policyName) + } + if bestHint.NUMANodeAffinity == nil { + t.Fatalf("expected non-nil NUMANodeAffinity") + } + if !bestHint.NUMANodeAffinity.IsEqual(expectedMask) { + t.Fatalf("expected best hint %v, got %v", expectedMask, bestHint.NUMANodeAffinity) + } + if !bestHint.Preferred { + t.Fatalf("expected best hint to be preferred") + } + }) + } +} + type topologyHintTestCase struct { description string pod *v1.Pod @@ -1052,10 +1413,6 @@ func getCommonTestCases() []topologyHintTestCase { NUMANodeAffinity: makeSocketMask(1), Preferred: true, }, - { - NUMANodeAffinity: makeSocketMask(0, 1), - Preferred: false, - }, }, }, }, @@ -1273,10 +1630,6 @@ func getCommonTestCases() []topologyHintTestCase { NUMANodeAffinity: makeSocketMask(0), Preferred: true, }, - { - NUMANodeAffinity: makeSocketMask(0, 1), - Preferred: false, - }, }, }, }, @@ -1349,10 +1702,6 @@ func getCommonTestCases() []topologyHintTestCase { NUMANodeAffinity: makeSocketMask(0), Preferred: true, }, - { - NUMANodeAffinity: makeSocketMask(0, 1), - Preferred: false, - }, }, }, },