🌱 Improve logging in e2e tests

test/e2e/common.go

@@ -69,6 +69,11 @@ const (
 	// Out-of-service Taint test actions.
 	oostAdded   = "added"
 	oostRemoved = "removed"
+	// Log collection paths.
+	beforePivotLogCollectionPath  = "before-pivot"
+	afterPivotLogCollectionPath   = "after-pivot"
+	afterRePivotLogCollectionPath = "after-re-pivot"
+	beforeDeleteLogCollectionPath = "before-delete"
 )
 
 func Byf(format string, a ...any) {
@@ -141,11 +146,16 @@ func DumpSpecResourcesAndCleanup(ctx context.Context, specName string, bootstrap
 
 	bootstrapClusterProxy.CollectWorkloadClusterLogs(ctx, namespace, clusterName, artifactFolder)
 
-	By("Fetch logs from target cluster")
-	err := FetchClusterLogs(targetClusterProxy, clusterLogCollectionBasePath)
-	if err != nil {
-		Logf("Error: %v", err)
-	}
+	By("Fetch manifest before deleting clusters")
+	FetchManifestsAndLogs(func() FetchManifestsAndLogsInput {
+		return FetchManifestsAndLogsInput{
+			BootstrapClusterProxy: bootstrapClusterProxy,
+			WorkloadClusterProxy:  targetClusterProxy,
+			ArtifactFolder:        artifactFolder,
+			LogCollectionPath:     beforeDeleteLogCollectionPath,
+		}
+	})
+
 	// Dumps all the resources in the spec namespace, then cleanups the cluster object and the spec namespace itself.
 	By(fmt.Sprintf("Dumping all the Cluster API resources in the %q namespace", namespace))
 	// Dump all Cluster API related resources to artifacts before deleting them.

test/e2e/logcollector.go

@@ -108,7 +108,7 @@ func (Metal3LogCollector) CollectMachinePoolLog(_ context.Context, _ client.Clie
 // FetchManifests fetches relevant Metal3, CAPI, and Kubernetes core resources
 // and dumps them to a file.
 func FetchManifests(clusterProxy framework.ClusterProxy, outputPath string) error {
-	outputPath = filepath.Join(outputPath, clusterProxy.GetName())
+	outputPath = filepath.Join(outputPath, "manifests")
 	ctx := context.Background()
 	restConfig := clusterProxy.GetRESTConfig()
 	dynamicClient, err := dynamic.NewForConfig(restConfig)
@@ -159,6 +159,7 @@ func FetchManifests(clusterProxy framework.ClusterProxy, outputPath string) erro
 		"m3data",
 		"m3dataclaim",
 		"m3datatemplate",
+		"ironic",
 	}
 	client := clusterProxy.GetClient()
 
@@ -193,10 +194,10 @@ func FetchManifests(clusterProxy framework.ClusterProxy, outputPath string) erro
 // FetchClusterLogs fetches logs from all pods in the cluster and writes them
 // to files.
 func FetchClusterLogs(clusterProxy framework.ClusterProxy, outputPath string) error {
+	outputPath = filepath.Join(outputPath, "controller_logs")
 	ctx := context.Background()
-	baseDir := filepath.Join(outputPath, clusterProxy.GetName())
 	// Ensure the base directory exists
-	if err := os.MkdirAll(baseDir, 0o750); err != nil {
+	if err := os.MkdirAll(outputPath, 0o750); err != nil {
 		return fmt.Errorf("couldn't create directory: %w", err)
 	}
 
@@ -206,7 +207,7 @@ func FetchClusterLogs(clusterProxy framework.ClusterProxy, outputPath string) er
 	// Print the Pods' information to file
 	// This does the same thing as:
 	// kubectl --kubeconfig="${KUBECONFIG_WORKLOAD}" get pods -A
-	outputFile := filepath.Join(baseDir, "pods.log")
+	outputFile := filepath.Join(outputPath, "pods.log")
 	file, err := os.Create(outputFile)
 	if err != nil {
 		return fmt.Errorf("failed to create output file: %w", err)
@@ -270,7 +271,7 @@ func FetchClusterLogs(clusterProxy framework.ClusterProxy, outputPath string) er
 			}
 
 			machineName := pod.Spec.NodeName
-			podDir := filepath.Join(baseDir, "machines", machineName, namespace.Name, pod.Name)
+			podDir := filepath.Join(outputPath, "machines", machineName, namespace.Name, pod.Name)
 			if err = os.MkdirAll(podDir, 0o750); err != nil {
 				return fmt.Errorf("couldn't create directory: %w", err)
 			}
@@ -394,3 +395,37 @@ func DumpGVR(ctx context.Context, dynamicClient *dynamic.DynamicClient, gvr sche
 	}
 	return nil
 }
+
+type FetchManifestsAndLogsInput struct {
+	BootstrapClusterProxy framework.ClusterProxy
+	WorkloadClusterProxy  framework.ClusterProxy
+	ArtifactFolder        string
+	LogCollectionPath     string
+}
+
+func FetchManifestsAndLogs(inputGetter func() FetchManifestsAndLogsInput) {
+	input := inputGetter()
+	By("Fetch logs and manifests for path: " + input.LogCollectionPath)
+	By("Fetch manifests from cluster " + input.WorkloadClusterProxy.GetName())
+	err := FetchManifests(input.WorkloadClusterProxy, filepath.Join(input.ArtifactFolder, workloadClusterLogCollectionBasePath, input.WorkloadClusterProxy.GetName(), input.LogCollectionPath))
+	if err != nil {
+		Logf("Error fetching manifests for workload cluster: %v", err)
+	}
+
+	By("Fetch manifests from cluster " + input.BootstrapClusterProxy.GetName())
+	err = FetchManifests(input.BootstrapClusterProxy, filepath.Join(input.ArtifactFolder, bootstrapClusterLogCollectionBasePath, input.LogCollectionPath))
+	if err != nil {
+		Logf("Error fetching manifests for bootstrap: %v", err)
+	}
+
+	By("Fetch logs from cluster " + input.WorkloadClusterProxy.GetName())
+	err = FetchClusterLogs(input.WorkloadClusterProxy, filepath.Join(input.ArtifactFolder, workloadClusterLogCollectionBasePath, input.WorkloadClusterProxy.GetName(), input.LogCollectionPath))
+	if err != nil {
+		Logf("Error fetching logs from workload cluster: %v", err)
+	}
+	By("Fetch logs from cluster " + input.BootstrapClusterProxy.GetName())
+	err = FetchClusterLogs(input.BootstrapClusterProxy, filepath.Join(input.ArtifactFolder, bootstrapClusterLogCollectionBasePath, input.LogCollectionPath))
+	if err != nil {
+		Logf("Error fetching logs from bootstrap cluster: %v", err)
+	}
+}

test/e2e/pivoting.go

@@ -16,7 +16,6 @@ import (
 	. "github.com/onsi/gomega"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"k8s.io/apimachinery/pkg/runtime"
 	clusterv1 "sigs.k8s.io/cluster-api/api/core/v1beta2"
 	clusterctlv1 "sigs.k8s.io/cluster-api/cmd/clusterctl/api/v1alpha3"
 	"sigs.k8s.io/cluster-api/cmd/clusterctl/client/config"
@@ -26,17 +25,18 @@ import (
 )
 
 const (
-	bmoPath                      = "BMOPATH"
-	ironicTLSSetup               = "IRONIC_TLS_SETUP"
-	ironicBasicAuth              = "IRONIC_BASIC_AUTH"
-	ironicKeepalived             = "IRONIC_KEEPALIVED"
-	ironicMariadb                = "IRONIC_USE_MARIADB"
-	Kind                         = "kind"
-	NamePrefix                   = "NAMEPREFIX"
-	restartContainerCertUpdate   = "RESTART_CONTAINER_CERTIFICATE_UPDATED"
-	ironicNamespace              = "IRONIC_NAMESPACE"
-	clusterLogCollectionBasePath = "/tmp/target_cluster_logs"
-	Metal3ipamProviderName       = "metal3"
+	bmoPath                               = "BMOPATH"
+	ironicTLSSetup                        = "IRONIC_TLS_SETUP"
+	ironicBasicAuth                       = "IRONIC_BASIC_AUTH"
+	ironicKeepalived                      = "IRONIC_KEEPALIVED"
+	ironicMariadb                         = "IRONIC_USE_MARIADB"
+	Kind                                  = "kind"
+	NamePrefix                            = "NAMEPREFIX"
+	restartContainerCertUpdate            = "RESTART_CONTAINER_CERTIFICATE_UPDATED"
+	ironicNamespace                       = "IRONIC_NAMESPACE"
+	workloadClusterLogCollectionBasePath  = "workload-cluster-logs"
+	bootstrapClusterLogCollectionBasePath = "bootstrap-cluster-logs"
+	Metal3ipamProviderName                = "metal3"
 )
 
 type PivotingInput struct {
@@ -63,11 +63,14 @@ func pivoting(ctx context.Context, inputGetter func() PivotingInput) {
 	ListMachines(ctx, input.BootstrapClusterProxy.GetClient(), client.InNamespace(input.Namespace))
 	ListNodes(ctx, input.TargetCluster.GetClient())
 
-	By("Fetch logs from target cluster before pivot")
-	err := FetchClusterLogs(input.TargetCluster, filepath.Join(clusterLogCollectionBasePath, "beforePivot"))
-	if err != nil {
-		Logf("Error: %v", err)
-	}
+	FetchManifestsAndLogs(func() FetchManifestsAndLogsInput {
+		return FetchManifestsAndLogsInput{
+			BootstrapClusterProxy: input.BootstrapClusterProxy,
+			WorkloadClusterProxy:  input.TargetCluster,
+			ArtifactFolder:        input.ArtifactFolder,
+			LogCollectionPath:     beforePivotLogCollectionPath,
+		}
+	})
 
 	ironicContainers := []string{
 		"ironic",
@@ -83,17 +86,11 @@ func pivoting(ctx context.Context, inputGetter func() PivotingInput) {
 	}
 
 	By("Fetch container logs")
-	bootstrapCluster := os.Getenv("BOOTSTRAP_CLUSTER")
 	fetchContainerLogs(&generalContainers, input.ArtifactFolder, input.E2EConfig.MustGetVariable("CONTAINER_RUNTIME"))
-	if bootstrapCluster == Kind {
+	if input.BootstrapClusterProxy.GetName() == Kind {
 		fetchContainerLogs(&ironicContainers, input.ArtifactFolder, input.E2EConfig.MustGetVariable("CONTAINER_RUNTIME"))
 	}
 
-	By("Fetch manifest for bootstrap cluster before pivot")
-	err = FetchManifests(input.BootstrapClusterProxy, "/tmp/manifests/")
-	if err != nil {
-		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()
 	os.Setenv("KUBECONFIG_WORKLOAD", kconfigPathWorkload)
@@ -109,7 +106,7 @@ func pivoting(ctx context.Context, inputGetter func() PivotingInput) {
 
 	By("Remove Ironic containers from the source cluster")
 	ironicDeploymentType := IronicDeploymentTypeBMO
-	if bootstrapCluster == Kind {
+	if input.BootstrapClusterProxy.GetName() == Kind {
 		ironicDeploymentType = IronicDeploymentTypeLocal
 	} else if GetBoolVariable(input.E2EConfig, "USE_IRSO") {
 		ironicDeploymentType = IronicDeploymentTypeIrSO
@@ -130,7 +127,7 @@ func pivoting(ctx context.Context, inputGetter func() PivotingInput) {
 			Name: input.E2EConfig.MustGetVariable(ironicNamespace),
 		},
 	}
-	_, err = targetClusterClientSet.CoreV1().Namespaces().Create(ctx, ironicNamespaceObj, metav1.CreateOptions{})
+	_, err := targetClusterClientSet.CoreV1().Namespaces().Create(ctx, ironicNamespaceObj, metav1.CreateOptions{})
 	Expect(err).ToNot(HaveOccurred(), "Unable to create the Ironic namespace")
 
 	By("Initialize Provider component in target cluster")
@@ -367,19 +364,14 @@ func rePivoting(ctx context.Context, inputGetter func() RePivotingInput) {
 	numberOfControlplane := int(*input.E2EConfig.MustGetInt32PtrVariable("CONTROL_PLANE_MACHINE_COUNT"))
 	numberOfAllBmh := numberOfWorkers + numberOfControlplane
 
-	By("Fetch logs from target cluster after pivot")
-	err := FetchClusterLogs(input.TargetCluster, filepath.Join(clusterLogCollectionBasePath, "afterPivot"))
-	if err != nil {
-		Logf("Error: %v", err)
-	}
-
-	By("Fetch manifest for workload cluster after pivot")
-	workloadClusterProxy := framework.NewClusterProxy("workload-cluster-after-pivot", os.Getenv("KUBECONFIG"), runtime.NewScheme())
-	err = FetchManifests(workloadClusterProxy, "/tmp/manifests/")
-	if err != nil {
-		Logf("Error fetching manifests for workload cluster after pivot: %v", err)
-	}
-	os.Unsetenv("KUBECONFIG_WORKLOAD")
+	FetchManifestsAndLogs(func() FetchManifestsAndLogsInput {
+		return FetchManifestsAndLogsInput{
+			BootstrapClusterProxy: input.BootstrapClusterProxy,
+			WorkloadClusterProxy:  input.TargetCluster,
+			ArtifactFolder:        input.ArtifactFolder,
+			LogCollectionPath:     afterPivotLogCollectionPath,
+		}
+	})
 
 	By("Remove Ironic deployment from target cluster")
 	ironicDeploymentType := IronicDeploymentTypeBMO
@@ -401,14 +393,14 @@ func rePivoting(ctx context.Context, inputGetter func() RePivotingInput) {
 		//#nosec G204:gosec
 		cmd := exec.Command("sh", "-c", "export CONTAINER_RUNTIME=docker; "+ironicCommand)
 		var stdoutStderr []byte
-		stdoutStderr, err = cmd.CombinedOutput()
+		stdoutStderr, err := cmd.CombinedOutput()
 		Logf("Output: %s", stdoutStderr)
 		Expect(err).ToNot(HaveOccurred(), "Cannot run local ironic")
 	} else {
 		By("Install Ironic in the bootstrap cluster")
 		ironicKustomization := input.E2EConfig.MustGetVariable("IRONIC_RELEASE_PR_TEST")
 		ironicDeployLogFolder := filepath.Join(os.TempDir(), "source_cluster_logs", "ironic-deploy-logs", input.TargetCluster.GetName())
-		err = BuildAndApplyKustomization(ctx, &BuildAndApplyKustomizationInput{
+		err := BuildAndApplyKustomization(ctx, &BuildAndApplyKustomizationInput{
 			Kustomization:       ironicKustomization,
 			ClusterProxy:        input.BootstrapClusterProxy,
 			WaitForDeployment:   true,
@@ -425,7 +417,7 @@ func rePivoting(ctx context.Context, inputGetter func() RePivotingInput) {
 	bmoKustomization := input.E2EConfig.MustGetVariable("BMO_RELEASE_PR_TEST")
 	bmoDeployLogFolder := filepath.Join(os.TempDir(), "source_cluster_logs", "bmo-deploy-logs", input.TargetCluster.GetName())
 	By(fmt.Sprintf("Installing BMO from kustomization %s on the source cluster", bmoKustomization))
-	err = BuildAndApplyKustomization(ctx, &BuildAndApplyKustomizationInput{
+	err := BuildAndApplyKustomization(ctx, &BuildAndApplyKustomizationInput{
 		Kustomization:       bmoKustomization,
 		ClusterProxy:        input.BootstrapClusterProxy,
 		WaitForDeployment:   true,
@@ -496,11 +488,15 @@ func rePivoting(ctx context.Context, inputGetter func() RePivotingInput) {
 		Intervals: input.E2EConfig.GetIntervals(input.SpecName, "wait-machine-running"),
 	})
 
-	By("Fetch manifest for bootstrap cluster after re-pivot")
-	err = FetchManifests(input.BootstrapClusterProxy, "/tmp/manifests/")
-	if err != nil {
-		Logf("Error fetching manifests for bootstrap cluster before pivot: %v", err)
-	}
+	FetchManifestsAndLogs(func() FetchManifestsAndLogsInput {
+		return FetchManifestsAndLogsInput{
+			BootstrapClusterProxy: input.BootstrapClusterProxy,
+			WorkloadClusterProxy:  input.TargetCluster,
+			ArtifactFolder:        input.ArtifactFolder,
+			LogCollectionPath:     afterRePivotLogCollectionPath,
+		}
+	})
+
 	os.Unsetenv("KUBECONFIG_BOOTSTRAP")
 
 	By("RE-PIVOTING TEST PASSED!")