Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion pkg/scheduler/scheduler.go
Original file line number Diff line number Diff line change
Expand Up @@ -847,7 +847,13 @@ func (s *Scheduler) getNodesUsage(nodes *[]string, task *corev1.Pod) (*map[strin
failedNodes[nodeID] = "node unregistered"
continue
}
cachenodeMap[node.ID] = overallnodeMap[node.ID]
usage, ok := overallnodeMap[node.ID]
Comment thread
adity1raut marked this conversation as resolved.
if !ok {
klog.V(5).InfoS("node usage not found in snapshot", "node", nodeID)
failedNodes[nodeID] = "node usage unavailable"
continue
}
cachenodeMap[node.ID] = usage
}
return &cachenodeMap, &overallnodeMap, failedNodes, nil
}
Expand Down
41 changes: 41 additions & 0 deletions pkg/scheduler/scheduler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,47 @@ func Test_getNodesUsage(t *testing.T) {
assert.Equal(t, v.Devices.DeviceLists[0].Device.Usedcores, int32(20))
}

// Regression: ListNodes() silently skips nodes whose Node field is nil, while GetNode()
// does not apply that same filter and returns success. Before the fix, getNodesUsage()
// blindly assigned cachenodeMap[id] = overallnodeMap[id] for every node GetNode() accepted,
// so a node present in nodeManager but absent from the ListNodes() snapshot produced a nil
// *NodeUsage entry with no corresponding failedNodes record. That nil is later dereferenced
// unconditionally by calcScoreWithOptions (viewStatus(*node)), panicking the scheduler.
func Test_getNodesUsage_NodeMissingFromSnapshotIsNotCachedAsNil(t *testing.T) {
nodeMage := newNodeManager()
nodeMage.addNode("node1", &device.NodeInfo{
ID: "node1",
Node: nil,
Devices: map[string][]device.DeviceInfo{
nvidia.NvidiaGPUDevice: {{
ID: "GPU0",
Index: 0,
Count: 10,
Devmem: 1024,
Devcore: 100,
Numa: 1,
Mode: "hami",
Health: true,
}},
},
})
podMap := device.NewPodManager()
s := Scheduler{
nodeManager: nodeMage,
podManager: podMap,
}
nodes := []string{"node1"}
cachenodeMap, _, failedNodes, err := s.getNodesUsage(&nodes, nil)
if err != nil {
t.Fatal(err)
}
assert.Equal(t, 0, len(*cachenodeMap))
v, present := (*cachenodeMap)["node1"]
assert.Assert(t, !present, "node1 must not be cached at all, got %v", v)
_, failed := failedNodes["node1"]
assert.Assert(t, failed, "node1 should be recorded as a failed node")
}

func Test_getNodesUsage_StalePodDeviceAllocation(t *testing.T) {
t.Run("StaleOnly", func(t *testing.T) {
nodeMage := newNodeManager()
Expand Down
Loading