e2e: topomgr: validate all containers in pod

Up until now, the test validated the alignment of resources
only in the first container in a pod. That was just an overlook.
With this patch, we validate all the containers in a given pod.

Signed-off-by: Francesco Romani <fromani@redhat.com>

test/e2e_node/numa_alignment.go

@@ -89,7 +89,7 @@ func getCPUsPerNUMANode(nodeNum int) ([]int, error) {
 	return cpus.ToSlice(), nil
 }
 
-func getCPUToNUMANodeMapFromEnv(f *framework.Framework, pod *v1.Pod, environ map[string]string, numaNodes int) (map[int]int, error) {
+func getCPUToNUMANodeMapFromEnv(f *framework.Framework, pod *v1.Pod, cnt *v1.Container, environ map[string]string, numaNodes int) (map[int]int, error) {
 	var cpuIDs []int
 	cpuListAllowedEnvVar := "CPULIST_ALLOWED"
 
@@ -103,12 +103,12 @@ func getCPUToNUMANodeMapFromEnv(f *framework.Framework, pod *v1.Pod, environ map
 		}
 	}
 	if len(cpuIDs) == 0 {
-		return nil, fmt.Errorf("variable %q found in environ", cpuListAllowedEnvVar)
+		return nil, fmt.Errorf("variable %q not found in environ", cpuListAllowedEnvVar)
 	}
 
 	cpusPerNUMA := make(map[int][]int)
 	for numaNode := 0; numaNode < numaNodes; numaNode++ {
-		nodeCPUList := f.ExecCommandInContainer(pod.Name, pod.Spec.Containers[0].Name,
+		nodeCPUList := f.ExecCommandInContainer(pod.Name, cnt.Name,
 			"/bin/cat", fmt.Sprintf("/sys/devices/system/node/node%d/cpulist", numaNode))
 
 		cpus, err := cpuset.Parse(nodeCPUList)
@@ -138,7 +138,7 @@ func getCPUToNUMANodeMapFromEnv(f *framework.Framework, pod *v1.Pod, environ map
 	return CPUMap, nil
 }
 
-func getPCIDeviceToNumaNodeMapFromEnv(f *framework.Framework, pod *v1.Pod, environ map[string]string) (map[string]int, error) {
+func getPCIDeviceToNumaNodeMapFromEnv(f *framework.Framework, pod *v1.Pod, cnt *v1.Container, environ map[string]string) (map[string]int, error) {
 	pciDevPrefix := "PCIDEVICE_"
 	// at this point we don't care which plugin selected the device,
 	// we only need to know which devices were assigned to the POD.
@@ -153,14 +153,11 @@ func getPCIDeviceToNumaNodeMapFromEnv(f *framework.Framework, pod *v1.Pod, envir
 		// a single plugin can allocate more than a single device
 		pciDevs := strings.Split(value, ",")
 		for _, pciDev := range pciDevs {
-			pciDevNUMANode := f.ExecCommandInContainer(pod.Name, pod.Spec.Containers[0].Name,
+			pciDevNUMANode := f.ExecCommandInContainer(pod.Name, cnt.Name,
 				"/bin/cat", fmt.Sprintf("/sys/bus/pci/devices/%s/numa_node", pciDev))
 			NUMAPerDev[pciDev] = numaNodeFromSysFsEntry(pciDevNUMANode)
 		}
 	}
-	if len(NUMAPerDev) == 0 {
-		return nil, fmt.Errorf("no PCI devices found in environ")
-	}
 	return NUMAPerDev, nil
 }
 
@@ -180,22 +177,30 @@ func makeEnvMap(logs string) (map[string]string, error) {
 	return envMap, nil
 }
 
-func checkNUMAAlignment(f *framework.Framework, pod *v1.Pod, logs string, numaNodes int) (numaPodResources, error) {
+func containerWantsDevices(cnt *v1.Container, hwinfo testEnvHWInfo) bool {
+	_, found := cnt.Resources.Requests[v1.ResourceName(hwinfo.sriovResourceName)]
+	return found
+}
+
+func checkNUMAAlignment(f *framework.Framework, pod *v1.Pod, cnt *v1.Container, logs string, hwinfo testEnvHWInfo) (numaPodResources, error) {
 	podEnv, err := makeEnvMap(logs)
 	if err != nil {
 		return numaPodResources{}, err
 	}
 
-	CPUToNUMANode, err := getCPUToNUMANodeMapFromEnv(f, pod, podEnv, numaNodes)
+	CPUToNUMANode, err := getCPUToNUMANodeMapFromEnv(f, pod, cnt, podEnv, hwinfo.numaNodes)
 	if err != nil {
 		return numaPodResources{}, err
 	}
 
-	PCIDevsToNUMANode, err := getPCIDeviceToNumaNodeMapFromEnv(f, pod, podEnv)
+	PCIDevsToNUMANode, err := getPCIDeviceToNumaNodeMapFromEnv(f, pod, cnt, podEnv)
 	if err != nil {
 		return numaPodResources{}, err
 	}
 
+	if containerWantsDevices(cnt, hwinfo) && len(PCIDevsToNUMANode) == 0 {
+		return numaPodResources{}, fmt.Errorf("no PCI devices found in environ")
+	}
 	numaRes := numaPodResources{
 		CPUToNUMANode:     CPUToNUMANode,
 		PCIDevsToNUMANode: PCIDevsToNUMANode,

test/e2e_node/topology_manager_test.go

@@ -31,10 +31,12 @@ import (
 
 	"k8s.io/apimachinery/pkg/api/resource"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	runtimeapi "k8s.io/cri-api/pkg/apis/runtime/v1alpha2"
 	kubeletconfig "k8s.io/kubernetes/pkg/kubelet/apis/config"
 	"k8s.io/kubernetes/pkg/kubelet/cm/cpumanager"
 	"k8s.io/kubernetes/pkg/kubelet/cm/cpuset"
 	"k8s.io/kubernetes/pkg/kubelet/cm/topologymanager"
+	"k8s.io/kubernetes/pkg/kubelet/types"
 	"k8s.io/kubernetes/test/e2e/framework"
 	e2enode "k8s.io/kubernetes/test/e2e/framework/node"
 	e2epod "k8s.io/kubernetes/test/e2e/framework/pod"
@@ -304,16 +306,17 @@ func deletePodInNamespace(f *framework.Framework, namespace, name string) {
 	framework.ExpectNoError(err)
 }
 
-func validatePodAlignment(f *framework.Framework, pod *v1.Pod, numaNodes int) {
+func validatePodAlignment(f *framework.Framework, pod *v1.Pod, hwinfo testEnvHWInfo) {
-	ginkgo.By("validating the Gu pod")
+	for _, cnt := range pod.Spec.Containers {
-	logs, err := e2epod.GetPodLogs(f.ClientSet, f.Namespace.Name, pod.Name, pod.Spec.Containers[0].Name)
+		ginkgo.By(fmt.Sprintf("validating the container %s on Gu pod %s", cnt.Name, pod.Name))
-	framework.ExpectNoError(err, "expected log not found in container [%s] of pod [%s]",
-		pod.Spec.Containers[0].Name, pod.Name)
 
-	framework.Logf("got pod logs: %v", logs)
+		logs, err := e2epod.GetPodLogs(f.ClientSet, f.Namespace.Name, pod.Name, cnt.Name)
-	numaRes, err := checkNUMAAlignment(f, pod, logs, numaNodes)
+		framework.ExpectNoError(err, "expected log not found in container [%s] of pod [%s]", cnt.Name, pod.Name)
-	framework.ExpectNoError(err, "NUMA Alignment check failed for [%s] of pod [%s]: %s",
+
-		pod.Spec.Containers[0].Name, pod.Name, numaRes.String())
+		framework.Logf("got pod logs: %v", logs)
+		numaRes, err := checkNUMAAlignment(f, pod, &cnt, logs, hwinfo)
+		framework.ExpectNoError(err, "NUMA Alignment check failed for [%s] of pod [%s]: %s", cnt.Name, pod.Name, numaRes.String())
+	}
 }
 
 func runTopologyManagerPolicySuiteTests(f *framework.Framework) {
@@ -542,21 +545,27 @@ func runTopologyManagerPolicySuiteTests(f *framework.Framework) {
 	waitForContainerRemoval(pod2.Spec.Containers[0].Name, pod2.Name, pod2.Namespace)
 }
 
-func runTopologyManagerPositiveTest(f *framework.Framework, numaNodes, numPods int, cpuAmount, sriovResourceName, deviceAmount string) {
+func waitForAllContainerRemoval(podName, podNS string) {
+	rs, _, err := getCRIClient()
+	framework.ExpectNoError(err)
+	gomega.Eventually(func() bool {
+		containers, err := rs.ListContainers(&runtimeapi.ContainerFilter{
+			LabelSelector: map[string]string{
+				types.KubernetesPodNameLabel:      podName,
+				types.KubernetesPodNamespaceLabel: podNS,
+			},
+		})
+		if err != nil {
+			return false
+		}
+		return len(containers) == 0
+	}, 2*time.Minute, 1*time.Second).Should(gomega.BeTrue())
+}
+
+func runTopologyManagerPositiveTest(f *framework.Framework, numPods int, ctnAttrs []tmCtnAttribute, hwinfo testEnvHWInfo) {
 	var pods []*v1.Pod
 
 	for podID := 0; podID < numPods; podID++ {
-		ctnAttrs := []tmCtnAttribute{
-			{
-				ctnName:       "gu-container",
-				cpuRequest:    cpuAmount,
-				cpuLimit:      cpuAmount,
-				deviceName:    sriovResourceName,
-				deviceRequest: deviceAmount,
-				deviceLimit:   deviceAmount,
-			},
-		}
-
 		podName := fmt.Sprintf("gu-pod-%d", podID)
 		framework.Logf("creating pod %s attrs %v", podName, ctnAttrs)
 		pod := makeTopologyManagerTestPod(podName, numalignCmd, ctnAttrs)
@@ -566,30 +575,19 @@ func runTopologyManagerPositiveTest(f *framework.Framework, numaNodes, numPods i
 	}
 
 	for podID := 0; podID < numPods; podID++ {
-		validatePodAlignment(f, pods[podID], numaNodes)
+		validatePodAlignment(f, pods[podID], hwinfo)
 	}
 
 	for podID := 0; podID < numPods; podID++ {
 		pod := pods[podID]
-		framework.Logf("deleting the pod %s/%s and waiting for container %s removal",
+		framework.Logf("deleting the pod %s/%s and waiting for container removal",
-			pod.Namespace, pod.Name, pod.Spec.Containers[0].Name)
+			pod.Namespace, pod.Name)
 		deletePods(f, []string{pod.Name})
-		waitForContainerRemoval(pod.Spec.Containers[0].Name, pod.Name, pod.Namespace)
+		waitForAllContainerRemoval(pod.Name, pod.Namespace)
 	}
 }
 
-func runTopologyManagerNegativeTest(f *framework.Framework, numaNodes, numPods int, cpuAmount, sriovResourceName, deviceAmount string) {
+func runTopologyManagerNegativeTest(f *framework.Framework, numPods int, ctnAttrs []tmCtnAttribute, hwinfo testEnvHWInfo) {
-	ctnAttrs := []tmCtnAttribute{
-		{
-			ctnName:       "gu-container",
-			cpuRequest:    cpuAmount,
-			cpuLimit:      cpuAmount,
-			deviceName:    sriovResourceName,
-			deviceRequest: deviceAmount,
-			deviceLimit:   deviceAmount,
-		},
-	}
-
 	podName := "gu-pod"
 	framework.Logf("creating pod %s attrs %v", podName, ctnAttrs)
 	pod := makeTopologyManagerTestPod(podName, numalignCmd, ctnAttrs)
@@ -682,6 +680,11 @@ func teardownSRIOVConfigOrFail(f *framework.Framework, dpPod *v1.Pod) {
 	waitForContainerRemoval(dpPod.Spec.Containers[0].Name, dpPod.Name, dpPod.Namespace)
 }
 
+type testEnvHWInfo struct {
+	numaNodes         int
+	sriovResourceName string
+}
+
 func runTopologyManagerNodeAlignmentSuiteTests(f *framework.Framework, configMap *v1.ConfigMap, reservedSystemCPUs string, numaNodes, coreCount int) {
 	threadsPerCore := 1
 	if isHTEnabled() {
@@ -689,40 +692,107 @@ func runTopologyManagerNodeAlignmentSuiteTests(f *framework.Framework, configMap
 	}
 
 	dpPod, sriovResourceName, sriovResourceAmount := setupSRIOVConfigOrFail(f, configMap)
+	hwinfo := testEnvHWInfo{
+		numaNodes:         numaNodes,
+		sriovResourceName: sriovResourceName,
+	}
 
 	// could have been a loop, we unroll it to explain the testcases
+	var ctnAttrs []tmCtnAttribute
 
 	// simplest case
 	ginkgo.By(fmt.Sprintf("Successfully admit one guaranteed pod with 1 core, 1 %s device", sriovResourceName))
-	runTopologyManagerPositiveTest(f, numaNodes, 1, "1000m", sriovResourceName, "1")
+	ctnAttrs = []tmCtnAttribute{
+		{
+			ctnName:       "gu-container",
+			cpuRequest:    "1000m",
+			cpuLimit:      "1000m",
+			deviceName:    sriovResourceName,
+			deviceRequest: "1",
+			deviceLimit:   "1",
+		},
+	}
+	runTopologyManagerPositiveTest(f, 1, ctnAttrs, hwinfo)
 
 	ginkgo.By(fmt.Sprintf("Successfully admit one guaranteed pod with 2 cores, 1 %s device", sriovResourceName))
-	runTopologyManagerPositiveTest(f, numaNodes, 1, "2000m", sriovResourceName, "1")
+	ctnAttrs = []tmCtnAttribute{
+		{
+			ctnName:       "gu-container",
+			cpuRequest:    "2000m",
+			cpuLimit:      "2000m",
+			deviceName:    sriovResourceName,
+			deviceRequest: "1",
+			deviceLimit:   "1",
+		},
+	}
+	runTopologyManagerPositiveTest(f, 1, ctnAttrs, hwinfo)
 
 	if reservedSystemCPUs != "" {
 		// to avoid false negatives, we have put reserved CPUs in such a way there is at least a NUMA node
 		// with 1+ SRIOV devices and not reserved CPUs.
 		numCores := threadsPerCore * coreCount
+		allCoresReq := fmt.Sprintf("%dm", numCores*1000)
 		ginkgo.By(fmt.Sprintf("Successfully admit an entire socket (%d cores), 1 %s device", numCores, sriovResourceName))
-		runTopologyManagerPositiveTest(f, numaNodes, 1, fmt.Sprintf("%dm", numCores*1000), sriovResourceName, "1")
+		ctnAttrs = []tmCtnAttribute{
+			{
+				ctnName:       "gu-container",
+				cpuRequest:    allCoresReq,
+				cpuLimit:      allCoresReq,
+				deviceName:    sriovResourceName,
+				deviceRequest: "1",
+				deviceLimit:   "1",
+			},
+		}
+		runTopologyManagerPositiveTest(f, 1, ctnAttrs, hwinfo)
 	}
 
 	if sriovResourceAmount > 1 {
 		// no matter how busses are connected to NUMA nodes and SRIOV devices are installed, this function
 		// preconditions must ensure the following can be fulfilled
 		ginkgo.By(fmt.Sprintf("Successfully admit two guaranteed pods, each with 1 core, 1 %s device", sriovResourceName))
-		runTopologyManagerPositiveTest(f, numaNodes, 2, "1000m", sriovResourceName, "1")
+		ctnAttrs = []tmCtnAttribute{
+			{
+				ctnName:       "gu-container",
+				cpuRequest:    "1000m",
+				cpuLimit:      "1000m",
+				deviceName:    sriovResourceName,
+				deviceRequest: "1",
+				deviceLimit:   "1",
+			},
+		}
+		runTopologyManagerPositiveTest(f, 2, ctnAttrs, hwinfo)
 
 		ginkgo.By(fmt.Sprintf("Successfully admit two guaranteed pods, each with 2 cores, 1 %s device", sriovResourceName))
-		runTopologyManagerPositiveTest(f, numaNodes, 2, "2000m", sriovResourceName, "1")
+		ctnAttrs = []tmCtnAttribute{
+			{
+				ctnName:       "gu-container",
+				cpuRequest:    "2000m",
+				cpuLimit:      "2000m",
+				deviceName:    sriovResourceName,
+				deviceRequest: "1",
+				deviceLimit:   "1",
+			},
+		}
+		runTopologyManagerPositiveTest(f, 2, ctnAttrs, hwinfo)
 
 		// testing more complex conditions require knowledge about the system cpu+bus topology
 	}
 
 	// overflow NUMA node capacity: cores
 	numCores := 1 + (threadsPerCore * coreCount)
+	excessCoresReq := fmt.Sprintf("%dm", numCores*1000)
 	ginkgo.By(fmt.Sprintf("Trying to admit a guaranteed pods, with %d cores, 1 %s device - and it should be rejected", numCores, sriovResourceName))
-	runTopologyManagerNegativeTest(f, numaNodes, 1, fmt.Sprintf("%dm", numCores*1000), sriovResourceName, "1")
+	ctnAttrs = []tmCtnAttribute{
+		{
+			ctnName:       "gu-container",
+			cpuRequest:    excessCoresReq,
+			cpuLimit:      excessCoresReq,
+			deviceName:    sriovResourceName,
+			deviceRequest: "1",
+			deviceLimit:   "1",
+		},
+	}
+	runTopologyManagerNegativeTest(f, 1, ctnAttrs, hwinfo)
 
 	teardownSRIOVConfigOrFail(f, dpPod)
 }