From 5e6aed4137815854ad98e43173e8d49dd2c4db72 Mon Sep 17 00:00:00 2001 From: Alexey Perevalov Date: Fri, 13 Nov 2020 11:51:47 +0300 Subject: [PATCH] Fixes sigfault in case of empty TopologyInfo Device plugin which implements v1beta interface can return nil in Topology field For example nvidia-gpu-deviceplugin https://github.com/NVIDIA/k8s-device-plugin/blob/3520254b75df872c6a3468ab2f74deba7c63c09b/nvidia.go#L14://github.com/NVIDIA/k8s-device-plugin/blob/3520254b75df872c6a3468ab2f74deba7c63c09b/nvidia.go#L147 Signed-off-by: Alexey Perevalov --- pkg/kubelet/cm/devicemanager/manager.go | 4 ++++ pkg/kubelet/cm/devicemanager/manager_test.go | 18 +++++++++++++++--- 2 files changed, 19 insertions(+), 3 deletions(-) diff --git a/pkg/kubelet/cm/devicemanager/manager.go b/pkg/kubelet/cm/devicemanager/manager.go index 99d96984d5ca6..95cf058f1a78a 100644 --- a/pkg/kubelet/cm/devicemanager/manager.go +++ b/pkg/kubelet/cm/devicemanager/manager.go @@ -905,6 +905,10 @@ func (m *ManagerImpl) allocateContainerResources(pod *v1.Pod, container *v1.Cont // Update internal cached podDevices state. m.mutex.Lock() for dev := range allocDevices { + if m.allDevices[resource][dev].Topology == nil || len(m.allDevices[resource][dev].Topology.Nodes) == 0 { + allocDevicesWithNUMA[0] = append(allocDevicesWithNUMA[0], dev) + continue + } for idx := range m.allDevices[resource][dev].Topology.Nodes { node := m.allDevices[resource][dev].Topology.Nodes[idx] allocDevicesWithNUMA[node.ID] = append(allocDevicesWithNUMA[node.ID], dev) diff --git a/pkg/kubelet/cm/devicemanager/manager_test.go b/pkg/kubelet/cm/devicemanager/manager_test.go index 8f1ec0bd3624e..9034498c66f4f 100644 --- a/pkg/kubelet/cm/devicemanager/manager_test.go +++ b/pkg/kubelet/cm/devicemanager/manager_test.go @@ -656,7 +656,7 @@ func getTestManager(tmpDir string, activePods ActivePodsFunc, testRes []TestReso opts: nil, } } - testManager.allDevices[res.resourceName] = makeDevice(res.devs) + testManager.allDevices[res.resourceName] = makeDevice(res.devs, res.topology) } return testManager, nil @@ -666,6 +666,7 @@ type TestResource struct { resourceName string resourceQuantity resource.Quantity devs checkpoint.DevicesPerNUMA + topology bool } func TestPodContainerDeviceAllocation(t *testing.T) { @@ -673,11 +674,13 @@ func TestPodContainerDeviceAllocation(t *testing.T) { resourceName: "domain1.com/resource1", resourceQuantity: *resource.NewQuantity(int64(2), resource.DecimalSI), devs: checkpoint.DevicesPerNUMA{0: []string{"dev1", "dev2"}}, + topology: true, } res2 := TestResource{ resourceName: "domain2.com/resource2", resourceQuantity: *resource.NewQuantity(int64(1), resource.DecimalSI), devs: checkpoint.DevicesPerNUMA{0: []string{"dev3", "dev4"}}, + topology: false, } testResources := make([]TestResource, 2) testResources = append(testResources, res1) @@ -769,11 +772,13 @@ func TestInitContainerDeviceAllocation(t *testing.T) { resourceName: "domain1.com/resource1", resourceQuantity: *resource.NewQuantity(int64(2), resource.DecimalSI), devs: checkpoint.DevicesPerNUMA{0: []string{"dev1", "dev2"}}, + topology: false, } res2 := TestResource{ resourceName: "domain2.com/resource2", resourceQuantity: *resource.NewQuantity(int64(1), resource.DecimalSI), devs: checkpoint.DevicesPerNUMA{0: []string{"dev3", "dev4"}}, + topology: true, } testResources := make([]TestResource, 2) testResources = append(testResources, res1) @@ -922,6 +927,7 @@ func TestDevicePreStartContainer(t *testing.T) { resourceName: "domain1.com/resource1", resourceQuantity: *resource.NewQuantity(int64(2), resource.DecimalSI), devs: checkpoint.DevicesPerNUMA{0: []string{"dev1", "dev2"}}, + topology: false, } as := require.New(t) podsStub := activePodsStub{ @@ -1059,11 +1065,17 @@ func allocateStubFunc() func(devs []string) (*pluginapi.AllocateResponse, error) } } -func makeDevice(devOnNUMA checkpoint.DevicesPerNUMA) map[string]pluginapi.Device { +func makeDevice(devOnNUMA checkpoint.DevicesPerNUMA, topology bool) map[string]pluginapi.Device { res := make(map[string]pluginapi.Device) + var topologyInfo *pluginapi.TopologyInfo for node, devs := range devOnNUMA { + if topology { + topologyInfo = &pluginapi.TopologyInfo{Nodes: []*pluginapi.NUMANode{{ID: node}}} + } else { + topologyInfo = nil + } for idx := range devs { - res[devs[idx]] = pluginapi.Device{ID: devs[idx], Topology: &pluginapi.TopologyInfo{Nodes: []*pluginapi.NUMANode{{ID: node}}}} + res[devs[idx]] = pluginapi.Device{ID: devs[idx], Topology: topologyInfo} } } return res