Merge pull request #2381 from Nordix/lentzi90/e2e-logging

metal3-io-bot · web-flow · commit a8a536956a7c · 2025-03-13T09:59:35.000+02:00
🌱 E2E: Unify logging and cleanup
diff --git a/test/e2e/common.go b/test/e2e/common.go
@@ -561,7 +561,7 @@ func GenerateIPPoolPreallocations(ctx context.Context, ippool ipamv1.IPPool, poo
 	Expect(c.List(ctx, &m3MachineList, &client.ListOptions{})).To(Succeed())
 	newAllocations := make(map[string]ipamv1.IPAddressStr)
 	for m3dataPoolName, ipaddress := range allocations {
-		fmt.Println("datapoolName:", m3dataPoolName, "=>", "ipaddress:", ipaddress)
+		Logf("datapoolName:", m3dataPoolName, "=>", "ipaddress:", ipaddress)
 		BMHName := strings.Split(m3dataPoolName, "-"+poolName)[0]
 		Logf("poolName: %s", poolName)
 		Logf("BMHName: %s", BMHName)
@@ -950,10 +950,10 @@ func KubectlDelete(ctx context.Context, kubeconfigPath string, resources []byte,
 		testexec.WithStdin(rbytes),
 	)
 
-	fmt.Printf("Running kubectl %s\n", strings.Join(aargs, " "))
+	Logf("Running kubectl %s\n", strings.Join(aargs, " "))
 	stdout, stderr, err := deleteCmd.Run(ctx)
-	fmt.Printf("stderr:\n%s\n", string(stderr))
-	fmt.Printf("stdout:\n%s\n", string(stdout))
+	Logf("stderr:\n%s", string(stderr))
+	Logf("stdout:\n%s", string(stdout))
 	return err
 }
 
diff --git a/test/e2e/logcollector.go b/test/e2e/logcollector.go
@@ -184,7 +184,7 @@ func FetchManifests(clusterProxy framework.ClusterProxy, outputPath string) erro
 			}
 		}
 	}
-	fmt.Printf("Successfully collected manifests for cluster %s.", clusterProxy.GetName())
+	Logf("Successfully collected manifests for cluster %s.", clusterProxy.GetName())
 	return nil
 }
 
@@ -244,7 +244,7 @@ func FetchClusterLogs(clusterProxy framework.ClusterProxy, outputPath string) er
 		// Get all pods in the namespace
 		pods, err := clientset.CoreV1().Pods(namespace.Name).List(ctx, metav1.ListOptions{})
 		if err != nil {
-			fmt.Printf("couldn't list pods in namespace %s: %v", namespace.Name, err)
+			Logf("couldn't list pods in namespace %s: %v", namespace.Name, err)
 			continue
 		}
 		for _, pod := range pods.Items {
@@ -259,13 +259,16 @@ func FetchClusterLogs(clusterProxy framework.ClusterProxy, outputPath string) er
 			}
 			podDescription, err := podDescriber.Describe(namespace.Name, pod.Name, describerSettings)
 			if err != nil {
-				fmt.Printf("couldn't describe pod %s in namespace %s: %v", pod.Name, namespace.Name, err)
+				Logf("couldn't describe pod %s in namespace %s: %v", pod.Name, namespace.Name, err)
 				continue
 			}
 
 			machineName := pod.Spec.NodeName
 			podDir := filepath.Join(baseDir, "machines", machineName, namespace.Name, pod.Name)
-			writeToFile([]byte(podDescription), "stdout_describe.log", podDir)
+			err = writeToFile([]byte(podDescription), "stdout_describe.log", podDir)
+			if err != nil {
+				return fmt.Errorf("couldn't write to file: %v", err)
+			}
 
 			// Get containers of the Pod
 			for _, container := range pod.Spec.Containers {
@@ -274,13 +277,13 @@ func FetchClusterLogs(clusterProxy framework.ClusterProxy, outputPath string) er
 
 				err := CollectContainerLogs(ctx, namespace.Name, pod.Name, container.Name, clientset, containerDir)
 				if err != nil {
-					fmt.Printf("Error %v.", err)
+					Logf("Error %v.", err)
 					continue
 				}
 			}
 		}
 	}
-	fmt.Printf("Successfully collected logs for cluster %s.", clusterProxy.GetName())
+	Logf("Successfully collected logs for cluster %s.", clusterProxy.GetName())
 	return nil
 }
 
@@ -307,25 +310,29 @@ func CollectContainerLogs(ctx context.Context, namespace string, podName string,
 	}
 	podStr := buf.String()
 
-	writeToFile([]byte(podStr), "stdout.log", outputPath)
+	err = writeToFile([]byte(podStr), "stdout.log", outputPath)
+	if err != nil {
+		return fmt.Errorf("couldn't write to file: %v", err)
+	}
 
 	return nil
 }
 
 // writeToFile writes content to a file,
 // creating any missing directories in the path.
-func writeToFile(content []byte, fileName string, filePath string) {
+func writeToFile(content []byte, fileName string, filePath string) error {
 	// Create any missing directories in the path
 	err := os.MkdirAll(filePath, 0775)
 	if err != nil {
-		fmt.Printf("couldn't create directory: %v", err)
+		return fmt.Errorf("couldn't create directory: %v", err)
 	}
 	// Write content to file
 	file := filepath.Join(filePath, fileName)
 	err = os.WriteFile(file, content, 0600)
 	if err != nil {
-		fmt.Printf("couldn't write to file: %v", err)
+		return fmt.Errorf("couldn't write to file: %v", err)
 	}
+	return nil
 }
 
 // crdIsInList checks if a CustomResourceDefinition is in the provided list of
@@ -370,7 +377,10 @@ func DumpGVR(ctx context.Context, dynamicClient *dynamic.DynamicClient, gvr sche
 		if err != nil {
 			return fmt.Errorf("could not marshal content: %v", err)
 		}
-		writeToFile(content, fileName, filePath)
+		err = writeToFile(content, fileName, filePath)
+		if err != nil {
+			return fmt.Errorf("couldn't write to file: %v", err)
+		}
 	}
 	return nil
 }
diff --git a/test/e2e/pivoting.go b/test/e2e/pivoting.go
@@ -92,7 +92,7 @@ func pivoting(ctx context.Context, inputGetter func() PivotingInput) {
 	By("Fetch manifest for bootstrap cluster before pivot")
 	err = FetchManifests(input.BootstrapClusterProxy, "/tmp/manifests/")
 	if err != nil {
-		fmt.Printf("Error fetching manifests for bootstrap cluster before pivot: %v\n", err)
+		Logf("Error fetching manifests for bootstrap cluster before pivot: %v", err)
 	}
 	By("Fetch target cluster kubeconfig for target cluster log collection")
 	kconfigPathWorkload := input.TargetCluster.GetKubeconfigPath()
@@ -377,7 +377,7 @@ func rePivoting(ctx context.Context, inputGetter func() RePivotingInput) {
 	workloadClusterProxy := framework.NewClusterProxy("workload-cluster-after-pivot", os.Getenv("KUBECONFIG"), runtime.NewScheme())
 	err = FetchManifests(workloadClusterProxy, "/tmp/manifests/")
 	if err != nil {
-		fmt.Printf("Error fetching manifests for workload cluster after pivot: %v\n", err)
+		Logf("Error fetching manifests for workload cluster after pivot: %v", err)
 	}
 	os.Unsetenv("KUBECONFIG_WORKLOAD")
 
@@ -401,7 +401,7 @@ func rePivoting(ctx context.Context, inputGetter func() RePivotingInput) {
 		//#nosec G204:gosec
 		cmd := exec.Command("sh", "-c", "export CONTAINER_RUNTIME=docker; "+ironicCommand)
 		stdoutStderr, err := cmd.CombinedOutput()
-		fmt.Printf("%s\n", stdoutStderr)
+		Logf("Output: %s", stdoutStderr)
 		Expect(err).ToNot(HaveOccurred(), "Cannot run local ironic")
 	} else {
 		By("Install Ironic in the bootstrap cluster")
@@ -498,7 +498,7 @@ func rePivoting(ctx context.Context, inputGetter func() RePivotingInput) {
 	By("Fetch manifest for bootstrap cluster after re-pivot")
 	err = FetchManifests(input.BootstrapClusterProxy, "/tmp/manifests/")
 	if err != nil {
-		fmt.Printf("Error fetching manifests for bootstrap cluster before pivot: %v\n", err)
+		Logf("Error fetching manifests for bootstrap cluster before pivot: %v", err)
 	}
 	os.Unsetenv("KUBECONFIG_BOOTSTRAP")
 
diff --git a/test/e2e/upgrade_clusterctl_test.go b/test/e2e/upgrade_clusterctl_test.go
@@ -210,7 +210,7 @@ func preInitFunc(clusterProxy framework.ClusterProxy, bmoRelease string, ironicR
 	By("Fetch manifest for bootstrap cluster")
 	err := FetchManifests(clusterProxy, "/tmp/manifests/")
 	if err != nil {
-		fmt.Printf("Error fetching manifests for bootstrap cluster: %v\n", err)
+		Logf("Error fetching manifests for bootstrap cluster: %v", err)
 	}
 
 	By("Fetch target cluster kubeconfig for target cluster log collection")
@@ -220,8 +220,7 @@ func preInitFunc(clusterProxy framework.ClusterProxy, bmoRelease string, ironicR
 	kubeconfigPathTemp := "/tmp/kubeconfig-test1.yaml"
 	cmd := exec.Command("cp", kconfigPathWorkload, kubeconfigPathTemp) // #nosec G204:gosec
 	stdoutStderr, er := cmd.CombinedOutput()
-	Logf("%s\n", stdoutStderr)
-	Expect(er).ToNot(HaveOccurred(), "Cannot fetch target cluster kubeconfig")
+	Expect(er).ToNot(HaveOccurred(), "Cannot fetch target cluster kubeconfig: %s", stdoutStderr)
 	// install certmanager
 	installCertManager(clusterProxy)
 	// Remove ironic
@@ -364,7 +363,7 @@ func preCleanupManagementCluster(clusterProxy framework.ClusterProxy, ironicRele
 			//#nosec G204 -- We take the BMOPATH from a variable.
 			cmd := exec.Command("sh", "-c", "export CONTAINER_RUNTIME=docker; "+ironicCommand)
 			stdoutStderr, err := cmd.CombinedOutput()
-			fmt.Printf("%s\n", stdoutStderr)
+			Logf("Output: %s", stdoutStderr)
 			Expect(err).ToNot(HaveOccurred(), "Cannot run local ironic")
 		} else {
 			By("Install Ironic in the source cluster as deployments")

Original file line number	Diff line number	Diff line change
`@@ -184,7 +184,7 @@ func FetchManifests(clusterProxy framework.ClusterProxy, outputPath string) erro`
`184`	`184`	`}`
`185`	`185`	`}`
`186`	`186`	`}`
`187`		`- fmt.Printf("Successfully collected manifests for cluster %s.", clusterProxy.GetName())`
	`187`	`+ Logf("Successfully collected manifests for cluster %s.", clusterProxy.GetName())`
`188`	`188`	`return nil`
`189`	`189`	`}`
`190`	`190`
`@@ -244,7 +244,7 @@ func FetchClusterLogs(clusterProxy framework.ClusterProxy, outputPath string) er`
`244`	`244`	`// Get all pods in the namespace`
`245`	`245`	`pods, err := clientset.CoreV1().Pods(namespace.Name).List(ctx, metav1.ListOptions{})`
`246`	`246`	`if err != nil {`
`247`		`- fmt.Printf("couldn't list pods in namespace %s: %v", namespace.Name, err)`
	`247`	`+ Logf("couldn't list pods in namespace %s: %v", namespace.Name, err)`
`248`	`248`	`continue`
`249`	`249`	`}`
`250`	`250`	`for _, pod := range pods.Items {`
`@@ -259,13 +259,16 @@ func FetchClusterLogs(clusterProxy framework.ClusterProxy, outputPath string) er`
`259`	`259`	`}`
`260`	`260`	`podDescription, err := podDescriber.Describe(namespace.Name, pod.Name, describerSettings)`
`261`	`261`	`if err != nil {`
`262`		`- fmt.Printf("couldn't describe pod %s in namespace %s: %v", pod.Name, namespace.Name, err)`
	`262`	`+ Logf("couldn't describe pod %s in namespace %s: %v", pod.Name, namespace.Name, err)`
`263`	`263`	`continue`
`264`	`264`	`}`
`265`	`265`
`266`	`266`	`machineName := pod.Spec.NodeName`
`267`	`267`	`podDir := filepath.Join(baseDir, "machines", machineName, namespace.Name, pod.Name)`
`268`		`- writeToFile([]byte(podDescription), "stdout_describe.log", podDir)`
	`268`	`+ err = writeToFile([]byte(podDescription), "stdout_describe.log", podDir)`
	`269`	`+ if err != nil {`
	`270`	`+ return fmt.Errorf("couldn't write to file: %v", err)`
	`271`	`+ }`
`269`	`272`
`270`	`273`	`// Get containers of the Pod`
`271`	`274`	`for _, container := range pod.Spec.Containers {`
`@@ -274,13 +277,13 @@ func FetchClusterLogs(clusterProxy framework.ClusterProxy, outputPath string) er`
`274`	`277`
`275`	`278`	`err := CollectContainerLogs(ctx, namespace.Name, pod.Name, container.Name, clientset, containerDir)`
`276`	`279`	`if err != nil {`
`277`		`- fmt.Printf("Error %v.", err)`
	`280`	`+ Logf("Error %v.", err)`
`278`	`281`	`continue`
`279`	`282`	`}`
`280`	`283`	`}`
`281`	`284`	`}`
`282`	`285`	`}`
`283`		`- fmt.Printf("Successfully collected logs for cluster %s.", clusterProxy.GetName())`
	`286`	`+ Logf("Successfully collected logs for cluster %s.", clusterProxy.GetName())`
`284`	`287`	`return nil`
`285`	`288`	`}`
`286`	`289`
`@@ -307,25 +310,29 @@ func CollectContainerLogs(ctx context.Context, namespace string, podName string,`
`307`	`310`	`}`
`308`	`311`	`podStr := buf.String()`
`309`	`312`
`310`		`- writeToFile([]byte(podStr), "stdout.log", outputPath)`
	`313`	`+ err = writeToFile([]byte(podStr), "stdout.log", outputPath)`
	`314`	`+ if err != nil {`
	`315`	`+ return fmt.Errorf("couldn't write to file: %v", err)`
	`316`	`+ }`
`311`	`317`
`312`	`318`	`return nil`
`313`	`319`	`}`
`314`	`320`
`315`	`321`	`// writeToFile writes content to a file,`
`316`	`322`	`// creating any missing directories in the path.`
`317`		`-func writeToFile(content []byte, fileName string, filePath string) {`
	`323`	`+func writeToFile(content []byte, fileName string, filePath string) error {`
`318`	`324`	`// Create any missing directories in the path`
`319`	`325`	`err := os.MkdirAll(filePath, 0775)`
`320`	`326`	`if err != nil {`
`321`		`- fmt.Printf("couldn't create directory: %v", err)`
	`327`	`+ return fmt.Errorf("couldn't create directory: %v", err)`
`322`	`328`	`}`
`323`	`329`	`// Write content to file`
`324`	`330`	`file := filepath.Join(filePath, fileName)`
`325`	`331`	`err = os.WriteFile(file, content, 0600)`
`326`	`332`	`if err != nil {`
`327`		`- fmt.Printf("couldn't write to file: %v", err)`
	`333`	`+ return fmt.Errorf("couldn't write to file: %v", err)`
`328`	`334`	`}`
	`335`	`+ return nil`
`329`	`336`	`}`
`330`	`337`
`331`	`338`	`// crdIsInList checks if a CustomResourceDefinition is in the provided list of`
`@@ -370,7 +377,10 @@ func DumpGVR(ctx context.Context, dynamicClient *dynamic.DynamicClient, gvr sche`
`370`	`377`	`if err != nil {`
`371`	`378`	`return fmt.Errorf("could not marshal content: %v", err)`
`372`	`379`	`}`
`373`		`- writeToFile(content, fileName, filePath)`
	`380`	`+ err = writeToFile(content, fileName, filePath)`
	`381`	`+ if err != nil {`
	`382`	`+ return fmt.Errorf("couldn't write to file: %v", err)`
	`383`	`+ }`
`374`	`384`	`}`
`375`	`385`	`return nil`
`376`	`386`	`}`