Unified ExecCommand and ExecCommandToString

Refactor all calls to the unified function: ExecCommand. Added a util ToString function that will be used for the calls that require the output in a string format. Signed-off-by: Ronny Baturov <[email protected]>
openshift · Jun 5, 2024 · 362196e · 362196e
1 parent 21653f0
commit 362196e
Show file tree

Hide file tree

Showing 15 changed files with 222 additions and 114 deletions.
diff --git a/test/e2e/performanceprofile/functests/11_mixedcpus/mixedcpus.go b/test/e2e/performanceprofile/functests/11_mixedcpus/mixedcpus.go
@@ -81,7 +81,8 @@ var _ = Describe("Mixedcpus", Ordered, func() {
 			// test arbitrary one should be good enough
 			worker := &workers[0]
 			cmd := isFileExistCmd(kubeletMixedCPUsConfigFile)
-			found, err := nodes.ExecCommandToString(ctx, cmd, worker)
+			out, err := nodes.ExecCommand(ctx, worker, cmd)
+			found := testutils.ToString(out)
 			Expect(err).ToNot(HaveOccurred(), "failed to execute command on node; cmd=%q node=%q", cmd, worker)
 			Expect(found).To(Equal("true"), "file not found; file=%q", kubeletMixedCPUsConfigFile)
 		})
@@ -112,7 +113,8 @@ var _ = Describe("Mixedcpus", Ordered, func() {
 				"-c",
 				fmt.Sprintf("/bin/awk  -F '\"' '/shared_cpuset.*/ { print $2 }' %s", runtime.CRIORuntimeConfigFile),
 			}
-			cpus, err := nodes.ExecCommandToString(ctx, cmd, worker)
+			out, err := nodes.ExecCommand(ctx, worker, cmd)
+			cpus := testutils.ToString(out)
 			Expect(err).ToNot(HaveOccurred(), "failed to execute command on node; cmd=%q node=%q", cmd, worker)
 			cpus = strings.Trim(cpus, "\n")
 			crioShared := mustParse(cpus)
@@ -268,7 +270,7 @@ var _ = Describe("Mixedcpus", Ordered, func() {
 
 				cmd := kubeletRestartCmd()
 				// The command would fail since it aborts all the pods during restart
-				_, _ = nodes.ExecCommandToString(ctx, cmd, node)
+				_, _ = nodes.ExecCommand(ctx, node, cmd)
 				// check that the node is ready after we restart Kubelet
 				nodes.WaitForReadyOrFail("post restart", node.Name, 20*time.Minute, 3*time.Second)
 
@@ -616,7 +618,8 @@ func fetchSharedCPUsFromEnv(c *kubernetes.Clientset, p *corev1.Pod, containerNam
 // getCPUswithLoadBalanceDisabled Return cpus which are not in any scheduling domain
 func getCPUswithLoadBalanceDisabled(ctx context.Context, targetNode *corev1.Node) ([]string, error) {
 	cmd := []string{"/bin/bash", "-c", "cat /proc/schedstat"}
-	schedstatData, err := nodes.ExecCommandToString(ctx, cmd, targetNode)
+	out, err := nodes.ExecCommand(ctx, targetNode, cmd)
+	schedstatData := testutils.ToString(out)
 	if err != nil {
 		return nil, err
 	}

diff --git a/test/e2e/performanceprofile/functests/1_performance/cpu_management.go b/test/e2e/performanceprofile/functests/1_performance/cpu_management.go
@@ -120,8 +120,9 @@ var _ = Describe("[rfe_id:27363][performance] CPU Management", Ordered, func() {
 		It("[test_id:37862][crit:high][vendor:[email protected]][level:acceptance] Verify CPU affinity mask, CPU reservation and CPU isolation on worker node", func() {
 			By("checking isolated CPU")
 			cmd := []string{"cat", "/sys/devices/system/cpu/isolated"}
-			sysIsolatedCpus, err := nodes.ExecCommandToString(context.TODO(), cmd, workerRTNode)
+			out, err := nodes.ExecCommand(context.TODO(), workerRTNode, cmd)
 			Expect(err).ToNot(HaveOccurred())
+			sysIsolatedCpus := testutils.ToString(out)
 			if balanceIsolated {
 				Expect(sysIsolatedCpus).To(BeEmpty())
 			} else {
@@ -130,15 +131,17 @@ var _ = Describe("[rfe_id:27363][performance] CPU Management", Ordered, func() {
 
 			By("checking reserved CPU in kubelet config file")
 			cmd = []string{"cat", "/rootfs/etc/kubernetes/kubelet.conf"}
-			conf, err := nodes.ExecCommandToString(context.TODO(), cmd, workerRTNode)
+			out, err = nodes.ExecCommand(context.TODO(), workerRTNode, cmd)
 			Expect(err).ToNot(HaveOccurred(), "failed to cat kubelet.conf")
+			conf := testutils.ToString(out)
 			// kubelet.conf changed formatting, there is a space after colons atm. Let's deal with both cases with a regex
 			Expect(conf).To(MatchRegexp(fmt.Sprintf(`"reservedSystemCPUs": ?"%s"`, reservedCPU)))
 
 			By("checking CPU affinity mask for kernel scheduler")
 			cmd = []string{"/bin/bash", "-c", "taskset -pc 1"}
-			sched, err := nodes.ExecCommandToString(context.TODO(), cmd, workerRTNode)
+			out, err = nodes.ExecCommand(context.TODO(), workerRTNode, cmd)
 			Expect(err).ToNot(HaveOccurred(), "failed to execute taskset")
+			sched := testutils.ToString(out)
 			mask := strings.SplitAfter(sched, " ")
 			maskSet, err := cpuset.Parse(mask[len(mask)-1])
 			Expect(err).ToNot(HaveOccurred())
@@ -149,8 +152,9 @@ var _ = Describe("[rfe_id:27363][performance] CPU Management", Ordered, func() {
 		It("[test_id:34358] Verify rcu_nocbs kernel argument on the node", func() {
 			By("checking that cmdline contains rcu_nocbs with right value")
 			cmd := []string{"cat", "/proc/cmdline"}
-			cmdline, err := nodes.ExecCommandToString(context.TODO(), cmd, workerRTNode)
+			out, err := nodes.ExecCommand(context.TODO(), workerRTNode, cmd)
 			Expect(err).ToNot(HaveOccurred())
+			cmdline := testutils.ToString(out)
 			re := regexp.MustCompile(`rcu_nocbs=\S+`)
 			rcuNocbsArgument := re.FindString(cmdline)
 			Expect(rcuNocbsArgument).To(ContainSubstring("rcu_nocbs="))
@@ -159,13 +163,15 @@ var _ = Describe("[rfe_id:27363][performance] CPU Management", Ordered, func() {
 
 			By("checking that new rcuo processes are running on non_isolated cpu")
 			cmd = []string{"pgrep", "rcuo"}
-			rcuoList, err := nodes.ExecCommandToString(context.TODO(), cmd, workerRTNode)
+			out, err = nodes.ExecCommand(context.TODO(), workerRTNode, cmd)
 			Expect(err).ToNot(HaveOccurred())
+			rcuoList := testutils.ToString(out)
 			for _, rcuo := range strings.Split(rcuoList, "\n") {
 				// check cpu affinity mask
 				cmd = []string{"/bin/bash", "-c", fmt.Sprintf("taskset -pc %s", rcuo)}
-				taskset, err := nodes.ExecCommandToString(context.TODO(), cmd, workerRTNode)
+				out, err := nodes.ExecCommand(context.TODO(), workerRTNode, cmd)
 				Expect(err).ToNot(HaveOccurred())
+				taskset := testutils.ToString(out)
 				mask := strings.SplitAfter(taskset, " ")
 				maskSet, err := cpuset.Parse(mask[len(mask)-1])
 				Expect(err).ToNot(HaveOccurred())
@@ -246,11 +252,12 @@ var _ = Describe("[rfe_id:27363][performance] CPU Management", Ordered, func() {
 				"unexpected QoS Class for %s/%s: %s (looking for %s)",
 				updatedPod.Namespace, updatedPod.Name, updatedPod.Status.QOSClass, expectedQos)
 
-			output, err := nodes.ExecCommandToString(ctx,
-				[]string{"/bin/bash", "-c", "ps -o psr $(pgrep -n stress) | tail -1"},
+			out, err := nodes.ExecCommand(ctx,
 				workerRTNode,
+				[]string{"/bin/bash", "-c", "ps -o psr $(pgrep -n stress) | tail -1"},
 			)
 			Expect(err).ToNot(HaveOccurred(), "failed to get cpu of stress process")
+			output := testutils.ToString(out)
 			cpu, err := strconv.Atoi(strings.Trim(output, " "))
 			Expect(err).ToNot(HaveOccurred())
 
@@ -295,7 +302,7 @@ var _ = Describe("[rfe_id:27363][performance] CPU Management", Ordered, func() {
 					"systemctl restart kubelet",
 				}
 
-				_, _ = nodes.ExecCommandToString(ctx, kubeletRestartCmd, workerRTNode)
+				_, _ = nodes.ExecCommand(ctx, workerRTNode, kubeletRestartCmd)
 				nodes.WaitForReadyOrFail("post kubele restart", workerRTNode.Name, 20*time.Minute, 3*time.Second)
 				// giving kubelet more time to stabilize and initialize itself before
 				testlog.Infof("post restart: entering cooldown time: %v", restartCooldownTime)
@@ -370,8 +377,9 @@ var _ = Describe("[rfe_id:27363][performance] CPU Management", Ordered, func() {
 			// It may takes some time for the system to reschedule active IRQs
 			Eventually(func() bool {
 				getActiveIrq := []string{"/bin/bash", "-c", "for n in $(find /proc/irq/ -name smp_affinity_list); do echo $(cat $n); done"}
-				activeIrq, err := nodes.ExecCommandToString(context.TODO(), getActiveIrq, workerRTNode)
+				out, err := nodes.ExecCommand(context.TODO(), workerRTNode, getActiveIrq)
 				Expect(err).ToNot(HaveOccurred())
+				activeIrq := testutils.ToString(out)
 				Expect(activeIrq).ToNot(BeEmpty())
 				for _, irq := range strings.Split(activeIrq, "\n") {
 					irqAffinity, err := cpuset.Parse(irq)
@@ -424,17 +432,19 @@ var _ = Describe("[rfe_id:27363][performance] CPU Management", Ordered, func() {
 
 			Eventually(func() string {
 				cmd := []string{"/bin/bash", "-c", fmt.Sprintf("find %s -name *%s*", cpusetPath, podUID)}
-				podCgroup, err = nodes.ExecCommandToString(context.TODO(), cmd, workerRTNode)
+				out, err := nodes.ExecCommand(context.TODO(), workerRTNode, cmd)
 				Expect(err).ToNot(HaveOccurred())
+				podCgroup = testutils.ToString(out)
 				return podCgroup
 			}, cluster.ComputeTestTimeout(30*time.Second, RunningOnSingleNode), 5*time.Second).ShouldNot(BeEmpty(),
 				fmt.Sprintf("cannot find cgroup for pod %q", podUID))
 
 			containersCgroups := ""
 			Eventually(func() string {
 				cmd := []string{"/bin/bash", "-c", fmt.Sprintf("find %s -name crio-*", podCgroup)}
-				containersCgroups, err = nodes.ExecCommandToString(context.TODO(), cmd, workerRTNode)
+				out, err := nodes.ExecCommand(context.TODO(), workerRTNode, cmd)
 				Expect(err).ToNot(HaveOccurred())
+				containersCgroups = testutils.ToString(out)
 				return containersCgroups
 			}, cluster.ComputeTestTimeout(30*time.Second, RunningOnSingleNode), 5*time.Second).ShouldNot(BeEmpty(),
 				fmt.Sprintf("cannot find containers cgroups from pod cgroup %q", podCgroup))
@@ -454,9 +464,9 @@ var _ = Describe("[rfe_id:27363][performance] CPU Management", Ordered, func() {
 
 				By("Checking what CPU the infra container is using")
 				cmd := []string{"/bin/bash", "-c", fmt.Sprintf("cat %s/cpuset.cpus", dir)}
-				output, err := nodes.ExecCommandToString(context.TODO(), cmd, workerRTNode)
+				out, err := nodes.ExecCommand(context.TODO(), workerRTNode, cmd)
 				Expect(err).ToNot(HaveOccurred())
-
+				output := testutils.ToString(out)
 				cpus, err := cpuset.Parse(output)
 				Expect(err).ToNot(HaveOccurred())
 
@@ -710,8 +720,9 @@ var _ = Describe("[rfe_id:27363][performance] CPU Management", Ordered, func() {
 				Expect(err).ToNot(HaveOccurred(), "Unable to parse pod cpus")
 				kubepodsExclusiveCpus := fmt.Sprintf("%s/kubepods.slice/cpuset.cpus.exclusive", cgroupRoot)
 				cmd := []string{"cat", kubepodsExclusiveCpus}
-				exclusiveCpus, err := nodes.ExecCommandToString(ctx, cmd, targetNode)
+				out, err := nodes.ExecCommand(ctx, targetNode, cmd)
 				Expect(err).ToNot(HaveOccurred())
+				exclusiveCpus := testutils.ToString(out)
 				exclusiveCpuset, err := cpuset.Parse(exclusiveCpus)
 				Expect(err).ToNot(HaveOccurred(), "unable to parse cpuset.cpus.exclusive")
 				Expect(podCpuset.Equals(exclusiveCpuset)).To(BeTrue())
@@ -752,8 +763,9 @@ func checkForWorkloadPartitioning(ctx context.Context) bool {
 		"-c",
 		"echo CHECK ; /bin/grep -rEo 'activation_annotation.*target\\.workload\\.openshift\\.io/management.*' /etc/crio/crio.conf.d/ || true",
 	}
-	output, err := nodes.ExecCommandToString(ctx, cmd, workerRTNode)
+	out, err := nodes.ExecCommand(ctx, workerRTNode, cmd)
 	Expect(err).ToNot(HaveOccurred(), "Unable to check cluster for Workload Partitioning enabled")
+	output := testutils.ToString(out)
 	re := regexp.MustCompile(`activation_annotation.*target\.workload\.openshift\.io/management.*`)
 	return re.MatchString(fmt.Sprint(output))
 }
@@ -773,9 +785,9 @@ func checkPodHTSiblings(ctx context.Context, testpod *corev1.Pod) bool {
 	node, err := nodes.GetByName(testpod.Spec.NodeName)
 	Expect(err).ToNot(HaveOccurred(), "failed to get node %q", testpod.Spec.NodeName)
 	Expect(testpod.Spec.NodeName).ToNot(BeEmpty(), "testpod %s/%s still pending - no nodeName set", testpod.Namespace, testpod.Name)
-	output, err := nodes.ExecCommandToString(ctx, cmd, node)
+	out, err := nodes.ExecCommand(ctx, node, cmd)
 	Expect(err).ToNot(HaveOccurred(), "Unable to crictl inspect containerID %q", containerID)
-
+	output := testutils.ToString(out)
 	podcpus, err := cpuset.Parse(strings.Trim(output, "\n"))
 	Expect(err).ToNot(
 		HaveOccurred(), "Unable to cpuset.Parse pod allocated cpu set from output %s", output)
@@ -797,11 +809,12 @@ func checkPodHTSiblings(ctx context.Context, testpod *corev1.Pod) bool {
 		"-c",
 		fmt.Sprintf("/bin/cat %s | /bin/sort -u", hostHTSiblingPaths.String()),
 	}
-	output, err = nodes.ExecCommandToString(ctx, cmd, workerRTNode)
+	out, err = nodes.ExecCommand(ctx, workerRTNode, cmd)
 	Expect(err).ToNot(
 		HaveOccurred(),
 		"Unable to read host thread_siblings_list files",
 	)
+	output = testutils.ToString(out)
 
 	// output is newline seperated. Convert to cpulist format by replacing internal "\n" chars with ","
 	hostHTSiblings := strings.ReplaceAll(
@@ -978,10 +991,11 @@ func logEventsForPod(testPod *corev1.Pod) {
 // getCPUswithLoadBalanceDisabled Return cpus which are not in any scheduling domain
 func getCPUswithLoadBalanceDisabled(ctx context.Context, targetNode *corev1.Node) ([]string, error) {
 	cmd := []string{"/bin/bash", "-c", "cat /proc/schedstat"}
-	schedstatData, err := nodes.ExecCommandToString(ctx, cmd, targetNode)
+	out, err := nodes.ExecCommand(ctx, targetNode, cmd)
 	if err != nil {
 		return nil, err
 	}
+	schedstatData := testutils.ToString(out)
 
 	info, err := schedstat.ParseData(strings.NewReader(schedstatData))
 	if err != nil {

diff --git a/test/e2e/performanceprofile/functests/1_performance/irqbalance.go b/test/e2e/performanceprofile/functests/1_performance/irqbalance.go
@@ -313,8 +313,9 @@ var _ = Describe("[performance] Checking IRQBalance settings", Ordered, func() {
 			origBannedCPUsFile := "/etc/sysconfig/orig_irq_banned_cpus"
 			By(fmt.Sprintf("Checking content of %q on node %q", origBannedCPUsFile, node.Name))
 			fullPath := filepath.Join("/", "rootfs", origBannedCPUsFile)
-			out, err := nodes.ExecCommandToString(context.TODO(), []string{"/usr/bin/cat", fullPath}, node)
+			output, err := nodes.ExecCommand(context.TODO(), node, []string{"/usr/bin/cat", fullPath})
 			Expect(err).ToNot(HaveOccurred())
+			out := testutils.ToString(output)
 			out = strings.TrimSuffix(out, "\r\n")
 			Expect(out).To(Equal("0"), "file %s does not contain the expect output; expected=0 actual=%s", fullPath, out)
 		})
@@ -327,10 +328,11 @@ var _ = Describe("[performance] Checking IRQBalance settings", Ordered, func() {
 // require close attention. For the time being we reimplement a form of nodes.BannedCPUs which can handle empty ban list.
 func getIrqBalanceBannedCPUs(ctx context.Context, node *corev1.Node) (cpuset.CPUSet, error) {
 	cmd := []string{"cat", "/rootfs/etc/sysconfig/irqbalance"}
-	conf, err := nodes.ExecCommandToString(ctx, cmd, node)
+	out, err := nodes.ExecCommand(ctx, node, cmd)
 	if err != nil {
 		return cpuset.New(), err
 	}
+	conf := testutils.ToString(out)
 
 	keyValue := findIrqBalanceBannedCPUsVarFromConf(conf)
 	if len(keyValue) == 0 {
@@ -361,7 +363,12 @@ func getIrqBalanceBannedCPUs(ctx context.Context, node *corev1.Node) (cpuset.CPU
 
 func getIrqDefaultSMPAffinity(ctx context.Context, node *corev1.Node) (string, error) {
 	cmd := []string{"cat", "/rootfs/proc/irq/default_smp_affinity"}
-	return nodes.ExecCommandToString(ctx, cmd, node)
+	out, err := nodes.ExecCommand(ctx, node, cmd)
+	if err != nil {
+		return "", err
+	}
+	output := testutils.ToString(out)
+	return output, nil
 }
 
 func findIrqBalanceBannedCPUsVarFromConf(conf string) string {

diff --git a/test/e2e/performanceprofile/functests/1_performance/netqueues.go b/test/e2e/performanceprofile/functests/1_performance/netqueues.go
@@ -401,16 +401,18 @@ func getReservedCPUSize(CPU *performancev2.CPU) int {
 func getVendorID(ctx context.Context, node corev1.Node, device string) string {
 	cmd := []string{"bash", "-c",
 		fmt.Sprintf("cat /sys/class/net/%s/device/vendor", device)}
-	stdout, err := nodes.ExecCommandToString(ctx, cmd, &node)
+	out, err := nodes.ExecCommand(ctx, &node, cmd)
 	Expect(err).ToNot(HaveOccurred())
+	stdout := testutils.ToString(out)
 	return stdout
 }
 
 func getDeviceID(ctx context.Context, node corev1.Node, device string) string {
 	cmd := []string{"bash", "-c",
 		fmt.Sprintf("cat /sys/class/net/%s/device/device", device)}
-	stdout, err := nodes.ExecCommandToString(ctx, cmd, &node)
+	out, err := nodes.ExecCommand(ctx, &node, cmd)
 	Expect(err).ToNot(HaveOccurred())
+	stdout := testutils.ToString(out)
 	return stdout
 }