Merge pull request kubernetes#96553 from AlexeyPerevalov/FixesKubelet…

…CrashEmptyTopology Fixes sigfault in case of empty TopologyInfo
c3y1huang · Nov 21, 2020 · 8c7cd8a · 8c7cd8a
2 parents 8095565 + 5e6aed4
commit 8c7cd8a
Show file tree

Hide file tree

Showing 2 changed files with 19 additions and 3 deletions.
diff --git 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
@@ -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,18 +666,21 @@ type TestResource struct {
 	resourceName     string
 	resourceQuantity resource.Quantity
 	devs             checkpoint.DevicesPerNUMA
+	topology         bool
 }
 
 func TestPodContainerDeviceAllocation(t *testing.T) {
 	res1 := TestResource{
 		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