From bb6beb99e5ab7182cd02f64b26fdf9d69f255297 Mon Sep 17 00:00:00 2001 From: Francesco Romani Date: Fri, 14 Feb 2020 18:07:05 +0100 Subject: [PATCH 1/8] e2e: topomgr: early check to detect VFs, not PFs The e2e_node topology_manager check have a early, quick check to rule out systems without sriov device, thus skipping the tests. The first version of the ckeck detected PFs, (Physical Functions), under the assumption that VFs (Virtual Functions) were already been created. This works because, obviously, you can't have VFs without PFs. However, it's a little safer and easier to understand if we check firectly for VFs, bailing out from systems which don't provide them. Nothing changes for properly configured test systems. Signed-off-by: Francesco Romani --- test/e2e_node/topology_manager_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/e2e_node/topology_manager_test.go b/test/e2e_node/topology_manager_test.go index 51cf16b4bde..ce025d03c12 100644 --- a/test/e2e_node/topology_manager_test.go +++ b/test/e2e_node/topology_manager_test.go @@ -83,7 +83,7 @@ func detectCoresPerSocket() int { } func detectSRIOVDevices() int { - outData, err := exec.Command("/bin/sh", "-c", "ls /sys/bus/pci/devices/*/sriov_totalvfs | wc -w").Output() + outData, err := exec.Command("/bin/sh", "-c", "ls /sys/bus/pci/devices/*/physfn | wc -w").Output() framework.ExpectNoError(err) devCount, err := strconv.Atoi(strings.TrimSpace(string(outData))) @@ -728,7 +728,7 @@ func runTopologyManagerTests(f *framework.Framework) { e2eskipper.Skipf("this test is meant to run on a system with at least 4 cores per socket") } if sriovdevCount == 0 { - e2eskipper.Skipf("this test is meant to run on a system with at least one SRIOV device") + e2eskipper.Skipf("this test is meant to run on a system with at least one configured VF from SRIOV device") } configMap := getSRIOVDevicePluginConfigMap(framework.TestContext.SriovdpConfigMapFile) From 0c2827cb504a24173deaeaae538ed6512bed281e Mon Sep 17 00:00:00 2001 From: Francesco Romani Date: Mon, 17 Feb 2020 09:53:34 +0100 Subject: [PATCH 2/8] e2e: topomgr: remove single-numa node hack On single-NUMA node systems the numa_node of sriov devices was sometimes reported as "-1" instead of, say, 0. This makes some tests that should succeed[0] fail unexpectedly. The reporting works as expected on real multi-NUMA node systems. This small workaround was added to handle this corner case, but it makes overall the code less readable and a bit too lenient, hence we remove it. +++ [0] on a single NUMA node system some resources are obviously always aligned if the pod can be admitted. It boils down to the node capacity at pod admittal time. Signed-off-by: Francesco Romani --- test/e2e_node/numa_alignment.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/test/e2e_node/numa_alignment.go b/test/e2e_node/numa_alignment.go index 33edd964e8c..11a55ed6186 100644 --- a/test/e2e_node/numa_alignment.go +++ b/test/e2e_node/numa_alignment.go @@ -44,8 +44,7 @@ func (R *numaPodResources) CheckAlignment() bool { } } for _, devNode := range R.PCIDevsToNUMANode { - // TODO: explain -1 - if devNode != -1 && nodeNum != devNode { + if nodeNum != devNode { return false } } From ddc18eae67cc7973679c1c46bfb2a217148edda0 Mon Sep 17 00:00:00 2001 From: Francesco Romani Date: Fri, 14 Feb 2020 17:40:19 +0100 Subject: [PATCH 3/8] e2e: topomgr: autodetect NUMA position of VF devs Add autodetection code to figure out on which NUMA node are the devices attached to. This autodetection work under the assumption all the VFs in the system must be used for the tests. Should not this be the case, or in general to handle non-trivial configurations, we keep the annotations mechanism added to the SRIOV device plugin config map. Signed-off-by: Francesco Romani --- test/e2e_node/numa_alignment.go | 62 +++++++++++++++++++++++--- test/e2e_node/topology_manager_test.go | 42 ++++++++++++++++- 2 files changed, 97 insertions(+), 7 deletions(-) diff --git a/test/e2e_node/numa_alignment.go b/test/e2e_node/numa_alignment.go index 11a55ed6186..a1c17454d36 100644 --- a/test/e2e_node/numa_alignment.go +++ b/test/e2e_node/numa_alignment.go @@ -19,6 +19,8 @@ package e2enode import ( "fmt" "io/ioutil" + "os" + "path/filepath" "sort" "strconv" "strings" @@ -153,12 +155,7 @@ func getPCIDeviceToNumaNodeMapFromEnv(f *framework.Framework, pod *v1.Pod, envir for _, pciDev := range pciDevs { pciDevNUMANode := f.ExecCommandInContainer(pod.Name, pod.Spec.Containers[0].Name, "/bin/cat", fmt.Sprintf("/sys/bus/pci/devices/%s/numa_node", pciDev)) - - nodeNum, err := strconv.Atoi(pciDevNUMANode) - if err != nil { - return nil, err - } - NUMAPerDev[pciDev] = nodeNum + NUMAPerDev[pciDev] = numaNodeFromSysFsEntry(pciDevNUMANode) } } if len(NUMAPerDev) == 0 { @@ -209,3 +206,56 @@ func checkNUMAAlignment(f *framework.Framework, pod *v1.Pod, logs string, numaNo } return numaRes, nil } + +type pciDeviceInfo struct { + Address string + NUMANode int + IsPhysFn bool + IsVFn bool +} + +func getPCIDeviceInfo(sysPCIDir string) ([]pciDeviceInfo, error) { + var pciDevs []pciDeviceInfo + + entries, err := ioutil.ReadDir(sysPCIDir) + if err != nil { + return nil, err + } + + for _, entry := range entries { + isPhysFn := false + isVFn := false + if _, err := os.Stat(filepath.Join(sysPCIDir, entry.Name(), "sriov_numvfs")); err == nil { + isPhysFn = true + } else if !os.IsNotExist(err) { + // unexpected error. Bail out + return nil, err + } + if _, err := os.Stat(filepath.Join(sysPCIDir, entry.Name(), "physfn")); err == nil { + isVFn = true + } else if !os.IsNotExist(err) { + // unexpected error. Bail out + return nil, err + } + + content, err := ioutil.ReadFile(filepath.Join(sysPCIDir, entry.Name(), "numa_node")) + if err != nil { + return nil, err + } + + pciDevs = append(pciDevs, pciDeviceInfo{ + Address: entry.Name(), + NUMANode: numaNodeFromSysFsEntry(string(content)), + IsPhysFn: isPhysFn, + IsVFn: isVFn, + }) + } + + return pciDevs, nil +} + +func numaNodeFromSysFsEntry(content string) int { + nodeNum, err := strconv.Atoi(strings.TrimSpace(content)) + framework.ExpectNoError(err, "error detecting the device numa_node from sysfs: %v", err) + return nodeNum +} diff --git a/test/e2e_node/topology_manager_test.go b/test/e2e_node/topology_manager_test.go index ce025d03c12..6ec77a58cfb 100644 --- a/test/e2e_node/topology_manager_test.go +++ b/test/e2e_node/topology_manager_test.go @@ -134,7 +134,7 @@ func makeTopologyManagerTestPod(podName, podCmd string, tmCtnAttributes []tmCtnA } } -func findNUMANodeWithoutSRIOVDevices(configMap *v1.ConfigMap, numaNodes int) (int, bool) { +func findNUMANodeWithoutSRIOVDevicesFromConfigMap(configMap *v1.ConfigMap, numaNodes int) (int, bool) { for nodeNum := 0; nodeNum < numaNodes; nodeNum++ { value, ok := configMap.Annotations[fmt.Sprintf("pcidevice_node%d", nodeNum)] if !ok { @@ -154,6 +154,46 @@ func findNUMANodeWithoutSRIOVDevices(configMap *v1.ConfigMap, numaNodes int) (in return -1, false } +func findNUMANodeWithoutSRIOVDevicesFromSysfs(numaNodes int) (int, bool) { + pciDevs, err := getPCIDeviceInfo("/sys/bus/pci/devices") + if err != nil { + framework.Failf("error detecting the PCI device NUMA node: %v", err) + } + + pciPerNuma := make(map[int]int) + for _, pciDev := range pciDevs { + if pciDev.IsVFn { + pciPerNuma[pciDev.NUMANode]++ + } + } + + if len(pciPerNuma) == 0 { + // if we got this far we already passed a rough check that SRIOV devices + // are available in the box, so something is seriously wrong + framework.Failf("failed to find any VF devices from %v", pciDevs) + } + + for nodeNum := 0; nodeNum < numaNodes; nodeNum++ { + v := pciPerNuma[nodeNum] + if v == 0 { + framework.Logf("NUMA node %d has no SRIOV devices attached", nodeNum) + return nodeNum, true + } + framework.Logf("NUMA node %d has %d SRIOV devices attached", nodeNum, v) + } + return -1, false +} + +func findNUMANodeWithoutSRIOVDevices(configMap *v1.ConfigMap, numaNodes int) (int, bool) { + // if someone annotated the configMap, let's use this information + if nodeNum, found := findNUMANodeWithoutSRIOVDevicesFromConfigMap(configMap, numaNodes); found { + return nodeNum, found + } + // no annotations, try to autodetect + // NOTE: this assumes all the VFs in the box can be used for the tests. + return findNUMANodeWithoutSRIOVDevicesFromSysfs(numaNodes) +} + func configureTopologyManagerInKubelet(f *framework.Framework, oldCfg *kubeletconfig.KubeletConfiguration, policy string, configMap *v1.ConfigMap, numaNodes int) string { // Configure Topology Manager in Kubelet with policy. newCfg := oldCfg.DeepCopy() From 8e9d76f1b95be03e213f99509d052c7dcd58f492 Mon Sep 17 00:00:00 2001 From: Francesco Romani Date: Fri, 14 Feb 2020 17:11:07 +0100 Subject: [PATCH 4/8] 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 --- test/e2e_node/numa_alignment.go | 27 +++-- test/e2e_node/topology_manager_test.go | 156 ++++++++++++++++++------- 2 files changed, 129 insertions(+), 54 deletions(-) diff --git a/test/e2e_node/numa_alignment.go b/test/e2e_node/numa_alignment.go index a1c17454d36..70012f66548 100644 --- a/test/e2e_node/numa_alignment.go +++ b/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, diff --git a/test/e2e_node/topology_manager_test.go b/test/e2e_node/topology_manager_test.go index 6ec77a58cfb..16793885022 100644 --- a/test/e2e_node/topology_manager_test.go +++ b/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) { - ginkgo.By("validating the Gu pod") - logs, err := e2epod.GetPodLogs(f.ClientSet, f.Namespace.Name, pod.Name, pod.Spec.Containers[0].Name) - framework.ExpectNoError(err, "expected log not found in container [%s] of pod [%s]", - pod.Spec.Containers[0].Name, pod.Name) +func validatePodAlignment(f *framework.Framework, pod *v1.Pod, hwinfo testEnvHWInfo) { + for _, cnt := range pod.Spec.Containers { + ginkgo.By(fmt.Sprintf("validating the container %s on Gu pod %s", cnt.Name, pod.Name)) - framework.Logf("got pod logs: %v", logs) - numaRes, err := checkNUMAAlignment(f, pod, logs, numaNodes) - framework.ExpectNoError(err, "NUMA Alignment check failed for [%s] of pod [%s]: %s", - pod.Spec.Containers[0].Name, pod.Name, numaRes.String()) + logs, err := e2epod.GetPodLogs(f.ClientSet, f.Namespace.Name, pod.Name, cnt.Name) + framework.ExpectNoError(err, "expected log not found in container [%s] of pod [%s]", cnt.Name, pod.Name) + + 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", - pod.Namespace, pod.Name, pod.Spec.Containers[0].Name) + framework.Logf("deleting the pod %s/%s and waiting for container removal", + 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) { - ctnAttrs := []tmCtnAttribute{ - { - ctnName: "gu-container", - cpuRequest: cpuAmount, - cpuLimit: cpuAmount, - deviceName: sriovResourceName, - deviceRequest: deviceAmount, - deviceLimit: deviceAmount, - }, - } - +func runTopologyManagerNegativeTest(f *framework.Framework, numPods int, ctnAttrs []tmCtnAttribute, hwinfo testEnvHWInfo) { 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) } From 7c12251c7a6e88929db97c28199febdf467c384e Mon Sep 17 00:00:00 2001 From: Francesco Romani Date: Mon, 17 Feb 2020 10:11:01 +0100 Subject: [PATCH 5/8] e2e: topomgr: add multi-container tests Add tests to check alignment of pods which contains more than one container. Signed-off-by: Francesco Romani --- test/e2e_node/topology_manager_test.go | 63 ++++++++++++++++++++++++++ 1 file changed, 63 insertions(+) diff --git a/test/e2e_node/topology_manager_test.go b/test/e2e_node/topology_manager_test.go index 16793885022..a52a38772d2 100644 --- a/test/e2e_node/topology_manager_test.go +++ b/test/e2e_node/topology_manager_test.go @@ -778,6 +778,69 @@ func runTopologyManagerNodeAlignmentSuiteTests(f *framework.Framework, configMap // testing more complex conditions require knowledge about the system cpu+bus topology } + // multi-container tests + if sriovResourceAmount >= 4 { + ginkgo.By(fmt.Sprintf("Successfully admit one guaranteed pods, each with two containers, each with 2 cores, 1 %s device", sriovResourceName)) + ctnAttrs = []tmCtnAttribute{ + { + ctnName: "gu-container-0", + cpuRequest: "2000m", + cpuLimit: "2000m", + deviceName: sriovResourceName, + deviceRequest: "1", + deviceLimit: "1", + }, + { + ctnName: "gu-container-1", + cpuRequest: "2000m", + cpuLimit: "2000m", + deviceName: sriovResourceName, + deviceRequest: "1", + deviceLimit: "1", + }, + } + runTopologyManagerPositiveTest(f, 1, ctnAttrs, hwinfo) + + ginkgo.By(fmt.Sprintf("Successfully admit two guaranteed pods, each with two containers, each with 1 core, 1 %s device", sriovResourceName)) + ctnAttrs = []tmCtnAttribute{ + { + ctnName: "gu-container-0", + cpuRequest: "1000m", + cpuLimit: "1000m", + deviceName: sriovResourceName, + deviceRequest: "1", + deviceLimit: "1", + }, + { + ctnName: "gu-container-1", + 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 two containers, both with with 2 cores, one with 1 %s device", sriovResourceName)) + ctnAttrs = []tmCtnAttribute{ + { + ctnName: "gu-container-dev", + cpuRequest: "2000m", + cpuLimit: "2000m", + deviceName: sriovResourceName, + deviceRequest: "1", + deviceLimit: "1", + }, + { + ctnName: "gu-container-nodev", + cpuRequest: "2000m", + cpuLimit: "2000m", + }, + } + runTopologyManagerPositiveTest(f, 2, ctnAttrs, hwinfo) + } + // overflow NUMA node capacity: cores numCores := 1 + (threadsPerCore * coreCount) excessCoresReq := fmt.Sprintf("%dm", numCores*1000) From 833519f80b649209c62f1899783f0c9d055f6008 Mon Sep 17 00:00:00 2001 From: Francesco Romani Date: Wed, 19 Feb 2020 12:23:29 +0100 Subject: [PATCH 6/8] e2e: topomgr: properly clean up after completion Due to an oversight, the e2e topology manager tests were leaking a configmap and a serviceaccount. This patch ensures a proper cleanup Signed-off-by: Francesco Romani --- test/e2e_node/numa_alignment.go | 15 ++- test/e2e_node/topology_manager_test.go | 135 ++++++++++++++----------- 2 files changed, 85 insertions(+), 65 deletions(-) diff --git a/test/e2e_node/numa_alignment.go b/test/e2e_node/numa_alignment.go index 70012f66548..219681e8af0 100644 --- a/test/e2e_node/numa_alignment.go +++ b/test/e2e_node/numa_alignment.go @@ -177,18 +177,23 @@ func makeEnvMap(logs string) (map[string]string, error) { return envMap, nil } -func containerWantsDevices(cnt *v1.Container, hwinfo testEnvHWInfo) bool { - _, found := cnt.Resources.Requests[v1.ResourceName(hwinfo.sriovResourceName)] +type testEnvInfo struct { + numaNodes int + sriovResourceName string +} + +func containerWantsDevices(cnt *v1.Container, envInfo testEnvInfo) bool { + _, found := cnt.Resources.Requests[v1.ResourceName(envInfo.sriovResourceName)] return found } -func checkNUMAAlignment(f *framework.Framework, pod *v1.Pod, cnt *v1.Container, logs string, hwinfo testEnvHWInfo) (numaPodResources, error) { +func checkNUMAAlignment(f *framework.Framework, pod *v1.Pod, cnt *v1.Container, logs string, envInfo testEnvInfo) (numaPodResources, error) { podEnv, err := makeEnvMap(logs) if err != nil { return numaPodResources{}, err } - CPUToNUMANode, err := getCPUToNUMANodeMapFromEnv(f, pod, cnt, podEnv, hwinfo.numaNodes) + CPUToNUMANode, err := getCPUToNUMANodeMapFromEnv(f, pod, cnt, podEnv, envInfo.numaNodes) if err != nil { return numaPodResources{}, err } @@ -198,7 +203,7 @@ func checkNUMAAlignment(f *framework.Framework, pod *v1.Pod, cnt *v1.Container, return numaPodResources{}, err } - if containerWantsDevices(cnt, hwinfo) && len(PCIDevsToNUMANode) == 0 { + if containerWantsDevices(cnt, envInfo) && len(PCIDevsToNUMANode) == 0 { return numaPodResources{}, fmt.Errorf("no PCI devices found in environ") } numaRes := numaPodResources{ diff --git a/test/e2e_node/topology_manager_test.go b/test/e2e_node/topology_manager_test.go index a52a38772d2..aab3d586295 100644 --- a/test/e2e_node/topology_manager_test.go +++ b/test/e2e_node/topology_manager_test.go @@ -297,16 +297,7 @@ func findSRIOVResource(node *v1.Node) (string, int64) { return "", 0 } -func deletePodInNamespace(f *framework.Framework, namespace, name string) { - gp := int64(0) - deleteOptions := metav1.DeleteOptions{ - GracePeriodSeconds: &gp, - } - err := f.ClientSet.CoreV1().Pods(namespace).Delete(context.TODO(), name, &deleteOptions) - framework.ExpectNoError(err) -} - -func validatePodAlignment(f *framework.Framework, pod *v1.Pod, hwinfo testEnvHWInfo) { +func validatePodAlignment(f *framework.Framework, pod *v1.Pod, envInfo testEnvInfo) { for _, cnt := range pod.Spec.Containers { ginkgo.By(fmt.Sprintf("validating the container %s on Gu pod %s", cnt.Name, pod.Name)) @@ -314,7 +305,7 @@ func validatePodAlignment(f *framework.Framework, pod *v1.Pod, hwinfo testEnvHWI framework.ExpectNoError(err, "expected log not found in container [%s] of pod [%s]", cnt.Name, pod.Name) framework.Logf("got pod logs: %v", logs) - numaRes, err := checkNUMAAlignment(f, pod, &cnt, logs, hwinfo) + numaRes, err := checkNUMAAlignment(f, pod, &cnt, logs, envInfo) framework.ExpectNoError(err, "NUMA Alignment check failed for [%s] of pod [%s]: %s", cnt.Name, pod.Name, numaRes.String()) } } @@ -562,7 +553,7 @@ func waitForAllContainerRemoval(podName, podNS string) { }, 2*time.Minute, 1*time.Second).Should(gomega.BeTrue()) } -func runTopologyManagerPositiveTest(f *framework.Framework, numPods int, ctnAttrs []tmCtnAttribute, hwinfo testEnvHWInfo) { +func runTopologyManagerPositiveTest(f *framework.Framework, numPods int, ctnAttrs []tmCtnAttribute, envInfo testEnvInfo) { var pods []*v1.Pod for podID := 0; podID < numPods; podID++ { @@ -575,7 +566,7 @@ func runTopologyManagerPositiveTest(f *framework.Framework, numPods int, ctnAttr } for podID := 0; podID < numPods; podID++ { - validatePodAlignment(f, pods[podID], hwinfo) + validatePodAlignment(f, pods[podID], envInfo) } for podID := 0; podID < numPods; podID++ { @@ -587,7 +578,7 @@ func runTopologyManagerPositiveTest(f *framework.Framework, numPods int, ctnAttr } } -func runTopologyManagerNegativeTest(f *framework.Framework, numPods int, ctnAttrs []tmCtnAttribute, hwinfo testEnvHWInfo) { +func runTopologyManagerNegativeTest(f *framework.Framework, numPods int, ctnAttrs []tmCtnAttribute, envInfo testEnvInfo) { podName := "gu-pod" framework.Logf("creating pod %s attrs %v", podName, ctnAttrs) pod := makeTopologyManagerTestPod(podName, numalignCmd, ctnAttrs) @@ -636,7 +627,16 @@ func getSRIOVDevicePluginConfigMap(cmFile string) *v1.ConfigMap { return readConfigMapV1OrDie(cmData) } -func setupSRIOVConfigOrFail(f *framework.Framework, configMap *v1.ConfigMap) (*v1.Pod, string, int64) { +type sriovData struct { + configMap *v1.ConfigMap + serviceAccount *v1.ServiceAccount + pod *v1.Pod + + resourceName string + resourceAmount int64 +} + +func setupSRIOVConfigOrFail(f *framework.Framework, configMap *v1.ConfigMap) sriovData { var err error ginkgo.By(fmt.Sprintf("Creating configMap %v/%v", metav1.NamespaceSystem, configMap.Name)) @@ -670,19 +670,34 @@ func setupSRIOVConfigOrFail(f *framework.Framework, configMap *v1.ConfigMap) (*v }, 2*time.Minute, framework.Poll).Should(gomega.BeTrue()) framework.Logf("Successfully created device plugin pod, detected %d SRIOV device %q", sriovResourceAmount, sriovResourceName) - return dpPod, sriovResourceName, sriovResourceAmount + return sriovData{ + configMap: configMap, + serviceAccount: serviceAccount, + pod: dpPod, + resourceName: sriovResourceName, + resourceAmount: sriovResourceAmount, + } } -func teardownSRIOVConfigOrFail(f *framework.Framework, dpPod *v1.Pod) { - framework.Logf("deleting the SRIOV device plugin pod %s/%s and waiting for container %s removal", - dpPod.Namespace, dpPod.Name, dpPod.Spec.Containers[0].Name) - deletePodInNamespace(f, dpPod.Namespace, dpPod.Name) - waitForContainerRemoval(dpPod.Spec.Containers[0].Name, dpPod.Name, dpPod.Namespace) -} +func teardownSRIOVConfigOrFail(f *framework.Framework, sd sriovData) { + var err error + gp := int64(0) + deleteOptions := metav1.DeleteOptions{ + GracePeriodSeconds: &gp, + } -type testEnvHWInfo struct { - numaNodes int - sriovResourceName string + ginkgo.By("Delete SRIOV device plugin pod %s/%s") + err = f.ClientSet.CoreV1().Pods(sd.pod.Namespace).Delete(context.TODO(), sd.pod.Name, &deleteOptions) + framework.ExpectNoError(err) + waitForContainerRemoval(sd.pod.Spec.Containers[0].Name, sd.pod.Name, sd.pod.Namespace) + + ginkgo.By(fmt.Sprintf("Deleting configMap %v/%v", metav1.NamespaceSystem, sd.configMap.Name)) + err = f.ClientSet.CoreV1().ConfigMaps(metav1.NamespaceSystem).Delete(context.TODO(), sd.configMap.Name, &deleteOptions) + framework.ExpectNoError(err) + + ginkgo.By(fmt.Sprintf("Deleting serviceAccount %v/%v", metav1.NamespaceSystem, sd.serviceAccount.Name)) + err = f.ClientSet.CoreV1().ServiceAccounts(metav1.NamespaceSystem).Delete(context.TODO(), sd.serviceAccount.Name, &deleteOptions) + framework.ExpectNoError(err) } func runTopologyManagerNodeAlignmentSuiteTests(f *framework.Framework, configMap *v1.ConfigMap, reservedSystemCPUs string, numaNodes, coreCount int) { @@ -691,102 +706,102 @@ func runTopologyManagerNodeAlignmentSuiteTests(f *framework.Framework, configMap threadsPerCore = 2 } - dpPod, sriovResourceName, sriovResourceAmount := setupSRIOVConfigOrFail(f, configMap) - hwinfo := testEnvHWInfo{ + sd := setupSRIOVConfigOrFail(f, configMap) + envInfo := testEnvInfo{ numaNodes: numaNodes, - sriovResourceName: sriovResourceName, + sriovResourceName: sd.resourceName, } // 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)) + ginkgo.By(fmt.Sprintf("Successfully admit one guaranteed pod with 1 core, 1 %s device", sd.resourceName)) ctnAttrs = []tmCtnAttribute{ { ctnName: "gu-container", cpuRequest: "1000m", cpuLimit: "1000m", - deviceName: sriovResourceName, + deviceName: sd.resourceName, deviceRequest: "1", deviceLimit: "1", }, } - runTopologyManagerPositiveTest(f, 1, ctnAttrs, hwinfo) + runTopologyManagerPositiveTest(f, 1, ctnAttrs, envInfo) - ginkgo.By(fmt.Sprintf("Successfully admit one guaranteed pod with 2 cores, 1 %s device", sriovResourceName)) + ginkgo.By(fmt.Sprintf("Successfully admit one guaranteed pod with 2 cores, 1 %s device", sd.resourceName)) ctnAttrs = []tmCtnAttribute{ { ctnName: "gu-container", cpuRequest: "2000m", cpuLimit: "2000m", - deviceName: sriovResourceName, + deviceName: sd.resourceName, deviceRequest: "1", deviceLimit: "1", }, } - runTopologyManagerPositiveTest(f, 1, ctnAttrs, hwinfo) + runTopologyManagerPositiveTest(f, 1, ctnAttrs, envInfo) 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)) + ginkgo.By(fmt.Sprintf("Successfully admit an entire socket (%d cores), 1 %s device", numCores, sd.resourceName)) ctnAttrs = []tmCtnAttribute{ { ctnName: "gu-container", cpuRequest: allCoresReq, cpuLimit: allCoresReq, - deviceName: sriovResourceName, + deviceName: sd.resourceName, deviceRequest: "1", deviceLimit: "1", }, } - runTopologyManagerPositiveTest(f, 1, ctnAttrs, hwinfo) + runTopologyManagerPositiveTest(f, 1, ctnAttrs, envInfo) } - if sriovResourceAmount > 1 { + if sd.resourceAmount > 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)) + ginkgo.By(fmt.Sprintf("Successfully admit two guaranteed pods, each with 1 core, 1 %s device", sd.resourceName)) ctnAttrs = []tmCtnAttribute{ { ctnName: "gu-container", cpuRequest: "1000m", cpuLimit: "1000m", - deviceName: sriovResourceName, + deviceName: sd.resourceName, deviceRequest: "1", deviceLimit: "1", }, } - runTopologyManagerPositiveTest(f, 2, ctnAttrs, hwinfo) + runTopologyManagerPositiveTest(f, 2, ctnAttrs, envInfo) - ginkgo.By(fmt.Sprintf("Successfully admit two guaranteed pods, each with 2 cores, 1 %s device", sriovResourceName)) + ginkgo.By(fmt.Sprintf("Successfully admit two guaranteed pods, each with 2 cores, 1 %s device", sd.resourceName)) ctnAttrs = []tmCtnAttribute{ { ctnName: "gu-container", cpuRequest: "2000m", cpuLimit: "2000m", - deviceName: sriovResourceName, + deviceName: sd.resourceName, deviceRequest: "1", deviceLimit: "1", }, } - runTopologyManagerPositiveTest(f, 2, ctnAttrs, hwinfo) + runTopologyManagerPositiveTest(f, 2, ctnAttrs, envInfo) // testing more complex conditions require knowledge about the system cpu+bus topology } // multi-container tests - if sriovResourceAmount >= 4 { - ginkgo.By(fmt.Sprintf("Successfully admit one guaranteed pods, each with two containers, each with 2 cores, 1 %s device", sriovResourceName)) + if sd.resourceAmount >= 4 { + ginkgo.By(fmt.Sprintf("Successfully admit one guaranteed pods, each with two containers, each with 2 cores, 1 %s device", sd.resourceName)) ctnAttrs = []tmCtnAttribute{ { ctnName: "gu-container-0", cpuRequest: "2000m", cpuLimit: "2000m", - deviceName: sriovResourceName, + deviceName: sd.resourceName, deviceRequest: "1", deviceLimit: "1", }, @@ -794,20 +809,20 @@ func runTopologyManagerNodeAlignmentSuiteTests(f *framework.Framework, configMap ctnName: "gu-container-1", cpuRequest: "2000m", cpuLimit: "2000m", - deviceName: sriovResourceName, + deviceName: sd.resourceName, deviceRequest: "1", deviceLimit: "1", }, } - runTopologyManagerPositiveTest(f, 1, ctnAttrs, hwinfo) + runTopologyManagerPositiveTest(f, 1, ctnAttrs, envInfo) - ginkgo.By(fmt.Sprintf("Successfully admit two guaranteed pods, each with two containers, each with 1 core, 1 %s device", sriovResourceName)) + ginkgo.By(fmt.Sprintf("Successfully admit two guaranteed pods, each with two containers, each with 1 core, 1 %s device", sd.resourceName)) ctnAttrs = []tmCtnAttribute{ { ctnName: "gu-container-0", cpuRequest: "1000m", cpuLimit: "1000m", - deviceName: sriovResourceName, + deviceName: sd.resourceName, deviceRequest: "1", deviceLimit: "1", }, @@ -815,20 +830,20 @@ func runTopologyManagerNodeAlignmentSuiteTests(f *framework.Framework, configMap ctnName: "gu-container-1", cpuRequest: "1000m", cpuLimit: "1000m", - deviceName: sriovResourceName, + deviceName: sd.resourceName, deviceRequest: "1", deviceLimit: "1", }, } - runTopologyManagerPositiveTest(f, 2, ctnAttrs, hwinfo) + runTopologyManagerPositiveTest(f, 2, ctnAttrs, envInfo) - ginkgo.By(fmt.Sprintf("Successfully admit two guaranteed pods, each with two containers, both with with 2 cores, one with 1 %s device", sriovResourceName)) + ginkgo.By(fmt.Sprintf("Successfully admit two guaranteed pods, each with two containers, both with with 2 cores, one with 1 %s device", sd.resourceName)) ctnAttrs = []tmCtnAttribute{ { ctnName: "gu-container-dev", cpuRequest: "2000m", cpuLimit: "2000m", - deviceName: sriovResourceName, + deviceName: sd.resourceName, deviceRequest: "1", deviceLimit: "1", }, @@ -838,26 +853,26 @@ func runTopologyManagerNodeAlignmentSuiteTests(f *framework.Framework, configMap cpuLimit: "2000m", }, } - runTopologyManagerPositiveTest(f, 2, ctnAttrs, hwinfo) + runTopologyManagerPositiveTest(f, 2, ctnAttrs, envInfo) } // 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)) + ginkgo.By(fmt.Sprintf("Trying to admit a guaranteed pods, with %d cores, 1 %s device - and it should be rejected", numCores, sd.resourceName)) ctnAttrs = []tmCtnAttribute{ { ctnName: "gu-container", cpuRequest: excessCoresReq, cpuLimit: excessCoresReq, - deviceName: sriovResourceName, + deviceName: sd.resourceName, deviceRequest: "1", deviceLimit: "1", }, } - runTopologyManagerNegativeTest(f, 1, ctnAttrs, hwinfo) + runTopologyManagerNegativeTest(f, 1, ctnAttrs, envInfo) - teardownSRIOVConfigOrFail(f, dpPod) + teardownSRIOVConfigOrFail(f, sd) } func runTopologyManagerTests(f *framework.Framework) { From a249b93687a723431ab41dc9168b9dadcd389a6c Mon Sep 17 00:00:00 2001 From: Francesco Romani Date: Thu, 20 Feb 2020 10:31:09 +0100 Subject: [PATCH 7/8] e2e: topomgr: address reviewer comments Signed-off-by: Francesco Romani --- test/e2e_node/numa_alignment.go | 17 +++++++++-------- test/e2e_node/topology_manager_test.go | 19 +++++++++++-------- 2 files changed, 20 insertions(+), 16 deletions(-) diff --git a/test/e2e_node/numa_alignment.go b/test/e2e_node/numa_alignment.go index 219681e8af0..88005d4b9b4 100644 --- a/test/e2e_node/numa_alignment.go +++ b/test/e2e_node/numa_alignment.go @@ -182,29 +182,30 @@ type testEnvInfo struct { sriovResourceName string } -func containerWantsDevices(cnt *v1.Container, envInfo testEnvInfo) bool { +func containerWantsDevices(cnt *v1.Container, envInfo *testEnvInfo) bool { _, found := cnt.Resources.Requests[v1.ResourceName(envInfo.sriovResourceName)] return found } -func checkNUMAAlignment(f *framework.Framework, pod *v1.Pod, cnt *v1.Container, logs string, envInfo testEnvInfo) (numaPodResources, error) { +func checkNUMAAlignment(f *framework.Framework, pod *v1.Pod, cnt *v1.Container, logs string, envInfo *testEnvInfo) (*numaPodResources, error) { + var err error podEnv, err := makeEnvMap(logs) if err != nil { - return numaPodResources{}, err + return nil, err } CPUToNUMANode, err := getCPUToNUMANodeMapFromEnv(f, pod, cnt, podEnv, envInfo.numaNodes) if err != nil { - return numaPodResources{}, err + return nil, err } PCIDevsToNUMANode, err := getPCIDeviceToNumaNodeMapFromEnv(f, pod, cnt, podEnv) if err != nil { - return numaPodResources{}, err + return nil, err } if containerWantsDevices(cnt, envInfo) && len(PCIDevsToNUMANode) == 0 { - return numaPodResources{}, fmt.Errorf("no PCI devices found in environ") + return nil, fmt.Errorf("no PCI devices found in environ") } numaRes := numaPodResources{ CPUToNUMANode: CPUToNUMANode, @@ -212,9 +213,9 @@ func checkNUMAAlignment(f *framework.Framework, pod *v1.Pod, cnt *v1.Container, } aligned := numaRes.CheckAlignment() if !aligned { - return numaRes, fmt.Errorf("NUMA resources not aligned") + err = fmt.Errorf("NUMA resources not aligned") } - return numaRes, nil + return &numaRes, err } type pciDeviceInfo struct { diff --git a/test/e2e_node/topology_manager_test.go b/test/e2e_node/topology_manager_test.go index aab3d586295..9099931db50 100644 --- a/test/e2e_node/topology_manager_test.go +++ b/test/e2e_node/topology_manager_test.go @@ -297,7 +297,7 @@ func findSRIOVResource(node *v1.Node) (string, int64) { return "", 0 } -func validatePodAlignment(f *framework.Framework, pod *v1.Pod, envInfo testEnvInfo) { +func validatePodAlignment(f *framework.Framework, pod *v1.Pod, envInfo *testEnvInfo) { for _, cnt := range pod.Spec.Containers { ginkgo.By(fmt.Sprintf("validating the container %s on Gu pod %s", cnt.Name, pod.Name)) @@ -306,7 +306,10 @@ func validatePodAlignment(f *framework.Framework, pod *v1.Pod, envInfo testEnvIn framework.Logf("got pod logs: %v", logs) numaRes, err := checkNUMAAlignment(f, pod, &cnt, logs, envInfo) - framework.ExpectNoError(err, "NUMA Alignment check failed for [%s] of pod [%s]: %s", cnt.Name, pod.Name, numaRes.String()) + framework.ExpectNoError(err, "NUMA Alignment check failed for [%s] of pod [%s]", cnt.Name, pod.Name) + if numaRes != nil { + framework.Logf("NUMA resources for %s/%s: %s", pod.Name, cnt.Name, numaRes.String()) + } } } @@ -553,7 +556,7 @@ func waitForAllContainerRemoval(podName, podNS string) { }, 2*time.Minute, 1*time.Second).Should(gomega.BeTrue()) } -func runTopologyManagerPositiveTest(f *framework.Framework, numPods int, ctnAttrs []tmCtnAttribute, envInfo testEnvInfo) { +func runTopologyManagerPositiveTest(f *framework.Framework, numPods int, ctnAttrs []tmCtnAttribute, envInfo *testEnvInfo) { var pods []*v1.Pod for podID := 0; podID < numPods; podID++ { @@ -578,7 +581,7 @@ func runTopologyManagerPositiveTest(f *framework.Framework, numPods int, ctnAttr } } -func runTopologyManagerNegativeTest(f *framework.Framework, numPods int, ctnAttrs []tmCtnAttribute, envInfo testEnvInfo) { +func runTopologyManagerNegativeTest(f *framework.Framework, numPods int, ctnAttrs []tmCtnAttribute, envInfo *testEnvInfo) { podName := "gu-pod" framework.Logf("creating pod %s attrs %v", podName, ctnAttrs) pod := makeTopologyManagerTestPod(podName, numalignCmd, ctnAttrs) @@ -636,7 +639,7 @@ type sriovData struct { resourceAmount int64 } -func setupSRIOVConfigOrFail(f *framework.Framework, configMap *v1.ConfigMap) sriovData { +func setupSRIOVConfigOrFail(f *framework.Framework, configMap *v1.ConfigMap) *sriovData { var err error ginkgo.By(fmt.Sprintf("Creating configMap %v/%v", metav1.NamespaceSystem, configMap.Name)) @@ -670,7 +673,7 @@ func setupSRIOVConfigOrFail(f *framework.Framework, configMap *v1.ConfigMap) sri }, 2*time.Minute, framework.Poll).Should(gomega.BeTrue()) framework.Logf("Successfully created device plugin pod, detected %d SRIOV device %q", sriovResourceAmount, sriovResourceName) - return sriovData{ + return &sriovData{ configMap: configMap, serviceAccount: serviceAccount, pod: dpPod, @@ -679,7 +682,7 @@ func setupSRIOVConfigOrFail(f *framework.Framework, configMap *v1.ConfigMap) sri } } -func teardownSRIOVConfigOrFail(f *framework.Framework, sd sriovData) { +func teardownSRIOVConfigOrFail(f *framework.Framework, sd *sriovData) { var err error gp := int64(0) deleteOptions := metav1.DeleteOptions{ @@ -707,7 +710,7 @@ func runTopologyManagerNodeAlignmentSuiteTests(f *framework.Framework, configMap } sd := setupSRIOVConfigOrFail(f, configMap) - envInfo := testEnvInfo{ + envInfo := &testEnvInfo{ numaNodes: numaNodes, sriovResourceName: sd.resourceName, } From 64904d0ab8b880172dbe6630194f5abd070530be Mon Sep 17 00:00:00 2001 From: Francesco Romani Date: Tue, 18 Feb 2020 19:26:01 +0100 Subject: [PATCH 8/8] e2e: topomgr: extend tests to all the policies Per https://github.com/kubernetes/enhancements/blob/master/keps/sig-node/0035-20190130-topology-manager.md#multi-numa-systems-tests we validate only the results for single-numa node policy, because the is no a simple and reliable way to validate the allocation performed by the other policies. Signed-off-by: Francesco Romani --- test/e2e_node/numa_alignment.go | 1 + test/e2e_node/topology_manager_test.go | 58 +++++++++++++++----------- 2 files changed, 35 insertions(+), 24 deletions(-) diff --git a/test/e2e_node/numa_alignment.go b/test/e2e_node/numa_alignment.go index 88005d4b9b4..c12033dfb50 100644 --- a/test/e2e_node/numa_alignment.go +++ b/test/e2e_node/numa_alignment.go @@ -180,6 +180,7 @@ func makeEnvMap(logs string) (map[string]string, error) { type testEnvInfo struct { numaNodes int sriovResourceName string + policy string } func containerWantsDevices(cnt *v1.Container, envInfo *testEnvInfo) bool { diff --git a/test/e2e_node/topology_manager_test.go b/test/e2e_node/topology_manager_test.go index 9099931db50..eb2ceb04430 100644 --- a/test/e2e_node/topology_manager_test.go +++ b/test/e2e_node/topology_manager_test.go @@ -568,8 +568,12 @@ func runTopologyManagerPositiveTest(f *framework.Framework, numPods int, ctnAttr pods = append(pods, pod) } - for podID := 0; podID < numPods; podID++ { - validatePodAlignment(f, pods[podID], envInfo) + // per https://github.com/kubernetes/enhancements/blob/master/keps/sig-node/0035-20190130-topology-manager.md#multi-numa-systems-tests + // we can do a menaingful validation only when using the single-numa node policy + if envInfo.policy == topologymanager.PolicySingleNumaNode { + for podID := 0; podID < numPods; podID++ { + validatePodAlignment(f, pods[podID], envInfo) + } } for podID := 0; podID < numPods; podID++ { @@ -703,7 +707,7 @@ func teardownSRIOVConfigOrFail(f *framework.Framework, sd *sriovData) { framework.ExpectNoError(err) } -func runTopologyManagerNodeAlignmentSuiteTests(f *framework.Framework, configMap *v1.ConfigMap, reservedSystemCPUs string, numaNodes, coreCount int) { +func runTopologyManagerNodeAlignmentSuiteTests(f *framework.Framework, configMap *v1.ConfigMap, reservedSystemCPUs string, numaNodes, coreCount int, policy string) { threadsPerCore := 1 if isHTEnabled() { threadsPerCore = 2 @@ -713,6 +717,7 @@ func runTopologyManagerNodeAlignmentSuiteTests(f *framework.Framework, configMap envInfo := &testEnvInfo{ numaNodes: numaNodes, sriovResourceName: sd.resourceName, + policy: policy, } // could have been a loop, we unroll it to explain the testcases @@ -859,22 +864,24 @@ func runTopologyManagerNodeAlignmentSuiteTests(f *framework.Framework, configMap runTopologyManagerPositiveTest(f, 2, ctnAttrs, envInfo) } - // 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, sd.resourceName)) - ctnAttrs = []tmCtnAttribute{ - { - ctnName: "gu-container", - cpuRequest: excessCoresReq, - cpuLimit: excessCoresReq, - deviceName: sd.resourceName, - deviceRequest: "1", - deviceLimit: "1", - }, + // this is the only policy that can guarantee reliable rejects + if policy == topologymanager.PolicySingleNumaNode { + // 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, sd.resourceName)) + ctnAttrs = []tmCtnAttribute{ + { + ctnName: "gu-container", + cpuRequest: excessCoresReq, + cpuLimit: excessCoresReq, + deviceName: sd.resourceName, + deviceRequest: "1", + deviceLimit: "1", + }, + } + runTopologyManagerNegativeTest(f, 1, ctnAttrs, envInfo) } - runTopologyManagerNegativeTest(f, 1, ctnAttrs, envInfo) - teardownSRIOVConfigOrFail(f, sd) } @@ -927,15 +934,18 @@ func runTopologyManagerTests(f *framework.Framework) { oldCfg, err = getCurrentKubeletConfig() framework.ExpectNoError(err) - policy := topologymanager.PolicySingleNumaNode + var policies = []string{topologymanager.PolicySingleNumaNode, topologymanager.PolicyRestricted, + topologymanager.PolicyBestEffort, topologymanager.PolicyNone} - // Configure Topology Manager - ginkgo.By(fmt.Sprintf("by configuring Topology Manager policy to %s", policy)) - framework.Logf("Configuring topology Manager policy to %s", policy) + for _, policy := range policies { + // Configure Topology Manager + ginkgo.By(fmt.Sprintf("by configuring Topology Manager policy to %s", policy)) + framework.Logf("Configuring topology Manager policy to %s", policy) - reservedSystemCPUs := configureTopologyManagerInKubelet(f, oldCfg, policy, configMap, numaNodes) + reservedSystemCPUs := configureTopologyManagerInKubelet(f, oldCfg, policy, configMap, numaNodes) - runTopologyManagerNodeAlignmentSuiteTests(f, configMap, reservedSystemCPUs, numaNodes, coreCount) + runTopologyManagerNodeAlignmentSuiteTests(f, configMap, reservedSystemCPUs, numaNodes, coreCount, policy) + } // restore kubelet config setOldKubeletConfig(f, oldCfg)