Merge pull request #1595 from Nordix/lentzi90/e2e-common-machine-log-collection

🌱 E2e: Unify machine log collection

Author: k8s-ci-robot

test/e2e/shared/common.go

@@ -26,13 +26,13 @@ import (
 	"os"
 	"path"
 	"path/filepath"
+	"strings"
 	"time"
 
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/types"
-	"k8s.io/apimachinery/pkg/util/sets"
 	clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
 	expv1 "sigs.k8s.io/cluster-api/exp/api/v1beta1"
 	"sigs.k8s.io/cluster-api/test/framework"
@@ -57,6 +57,9 @@ func SetupSpecNamespace(ctx context.Context, specName string, e2eCtx *E2EContext
 	return namespace
 }
 
+// DumpSpecResourcesAndCleanup dumps all the resources in the spec namespace.
+// This includes OpenStack resources and all the CAPI/CAPO resources in Kubernetes.
+// It also then cleanups the cluster object and the spec namespace itself.
 func DumpSpecResourcesAndCleanup(ctx context.Context, specName string, namespace *corev1.Namespace, e2eCtx *E2EContext) {
 	Logf("Running DumpSpecResourcesAndCleanup for namespace %q", namespace.Name)
 	// Dump all Cluster API related resources to artifacts before deleting them.
@@ -65,9 +68,6 @@ func DumpSpecResourcesAndCleanup(ctx context.Context, specName string, namespace
 	dumpAllResources := func(directory ...string) {
 		dumpSpecResources(ctx, e2eCtx, namespace, directory...)
 		dumpOpenStack(ctx, e2eCtx, e2eCtx.Environment.BootstrapClusterProxy.GetName(), directory...)
-
-		Logf("Dumping all OpenStack server instances in the %q namespace", namespace.Name)
-		dumpMachines(ctx, e2eCtx, namespace, directory...)
 	}
 
 	dumpAllResources()
@@ -100,40 +100,8 @@ func DumpSpecResourcesAndCleanup(ctx context.Context, specName string, namespace
 	delete(e2eCtx.Environment.Namespaces, namespace)
 }
 
-func dumpMachines(ctx context.Context, e2eCtx *E2EContext, namespace *corev1.Namespace, directory ...string) {
-	cluster, err := ClusterForSpec(ctx, e2eCtx, namespace)
-	if err != nil {
-		_, _ = fmt.Fprintf(GinkgoWriter, "cannot dump machines, couldn't get cluster in namespace %s: %v\n", namespace.Name, err)
-		return
-	}
-	if cluster.Status.Bastion == nil || cluster.Status.Bastion.FloatingIP == "" {
-		_, _ = fmt.Fprintln(GinkgoWriter, "cannot dump machines, cluster doesn't has a bastion host (yet) with a floating ip")
-		return
-	}
-	machines, err := machinesForSpec(ctx, e2eCtx.Environment.BootstrapClusterProxy, namespace)
-	if err != nil {
-		_, _ = fmt.Fprintf(GinkgoWriter, "cannot dump machines, could not get machines: %v\n", err)
-		return
-	}
-
-	machineNames := sets.New[string]()
-	for _, machine := range machines.Items {
-		machineNames.Insert(machine.Name)
-	}
-	srvs, err := GetOpenStackServers(e2eCtx, cluster, machineNames)
-	if err != nil {
-		_, _ = fmt.Fprintf(GinkgoWriter, "cannot dump machines, could not get servers from OpenStack: %v\n", err)
-		return
-	}
-	for _, m := range machines.Items {
-		srv, ok := srvs[m.Name]
-		if !ok {
-			continue
-		}
-		dumpMachine(ctx, e2eCtx, m, srv, cluster.Status.Bastion.FloatingIP, directory...)
-	}
-}
-
+// ClusterForSpec returns the OpenStackCluster in the given namespace.
+// It is considered an error if more than 1 OpenStackCluster is found.
 func ClusterForSpec(ctx context.Context, e2eCtx *E2EContext, namespace *corev1.Namespace) (*infrav1.OpenStackCluster, error) {
 	lister := e2eCtx.Environment.BootstrapClusterProxy.GetClient()
 	list := new(infrav1.OpenStackClusterList)
@@ -146,88 +114,7 @@ func ClusterForSpec(ctx context.Context, e2eCtx *E2EContext, namespace *corev1.N
 	return &list.Items[0], nil
 }
 
-func machinesForSpec(ctx context.Context, clusterProxy framework.ClusterProxy, namespace *corev1.Namespace) (*infrav1.OpenStackMachineList, error) {
-	list := new(infrav1.OpenStackMachineList)
-	if err := clusterProxy.GetClient().List(ctx, list, client.InNamespace(namespace.GetName())); err != nil {
-		return nil, fmt.Errorf("error listing machines: %v", err)
-	}
-	return list, nil
-}
-
-func dumpMachine(ctx context.Context, e2eCtx *E2EContext, machine infrav1.OpenStackMachine, srv ServerExtWithIP, bastionIP string, directory ...string) {
-	paths := append([]string{e2eCtx.Settings.ArtifactFolder, "clusters", e2eCtx.Environment.BootstrapClusterProxy.GetName()}, directory...)
-	logPath := filepath.Join(paths...)
-	machineLogBase := path.Join(logPath, "instances", machine.Namespace, machine.Name)
-	metaLog := path.Join(machineLogBase, "instance.log")
-
-	if err := os.MkdirAll(filepath.Dir(metaLog), 0o750); err != nil {
-		_, _ = fmt.Fprintf(GinkgoWriter, "couldn't create directory %q for file: %s\n", metaLog, err)
-	}
-
-	f, err := os.OpenFile(metaLog, os.O_APPEND|os.O_CREATE|os.O_WRONLY, 0o644)
-	if err != nil {
-		_, _ = fmt.Fprintf(GinkgoWriter, "couldn't open file %q: %s\n", metaLog, err)
-		return
-	}
-	defer f.Close()
-
-	serverJSON, err := json.MarshalIndent(srv, "", "    ")
-	if err != nil {
-		_, _ = fmt.Fprintf(GinkgoWriter, "error marshalling server %v: %s", srv, err)
-	}
-	if err := os.WriteFile(path.Join(machineLogBase, "server.txt"), serverJSON, 0o600); err != nil {
-		_, _ = fmt.Fprintf(GinkgoWriter, "error writing server JSON %s: %s", serverJSON, err)
-	}
-
-	srvUser := e2eCtx.E2EConfig.GetVariable(SSHUserMachine)
-
-	_, _ = fmt.Fprintf(f, "instance found: %q\n", srv.ID)
-	executeCommands(
-		ctx,
-		e2eCtx.Settings.ArtifactFolder,
-		e2eCtx.Settings.Debug,
-		filepath.Dir(f.Name()),
-		srv.ip,
-		bastionIP,
-		srvUser,
-		[]command{
-			// don't do this for now, it just takes to long
-			// {
-			//	title: "systemd",
-			//	cmd:   "journalctl --no-pager --output=short-precise | grep -v  'audit:\\|audit\\['",
-			// },
-			{
-				title: "kern",
-				cmd:   "journalctl --no-pager --output=short-precise -k",
-			},
-			{
-				title: "containerd-info",
-				cmd:   "crictl --runtime-endpoint unix:///run/containerd/containerd.sock info",
-			},
-			{
-				title: "containerd-containers",
-				cmd:   "crictl --runtime-endpoint unix:///run/containerd/containerd.sock ps",
-			},
-			{
-				title: "containerd-pods",
-				cmd:   "crictl --runtime-endpoint unix:///run/containerd/containerd.sock pods",
-			},
-			{
-				title: "cloud-final",
-				cmd:   "journalctl --no-pager -u cloud-final",
-			},
-			{
-				title: "kubelet",
-				cmd:   "journalctl --no-pager -u kubelet.service",
-			},
-			{
-				title: "containerd",
-				cmd:   "journalctl --no-pager -u containerd.service",
-			},
-		},
-	)
-}
-
+// dumpSpecResources dumps all CAPI/CAPO resources to yaml.
 func dumpSpecResources(ctx context.Context, e2eCtx *E2EContext, namespace *corev1.Namespace, directory ...string) {
 	paths := append([]string{e2eCtx.Settings.ArtifactFolder, "clusters", e2eCtx.Environment.BootstrapClusterProxy.GetName(), "resources"}, directory...)
 	framework.DumpAllResources(ctx, framework.DumpAllResourcesInput{
@@ -287,26 +174,23 @@ func getOpenStackClusterFromMachine(ctx context.Context, client client.Client, m
 	return openStackCluster, err
 }
 
+// getIDFromProviderID returns the server ID part of a provider ID string.
+func getIDFromProviderID(providerID string) string {
+	return strings.TrimPrefix(providerID, "openstack:///")
+}
+
 type OpenStackLogCollector struct {
-	E2EContext E2EContext
+	E2EContext *E2EContext
 }
 
 // CollectMachineLog gets logs for the OpenStack resources related to the given machine.
 func (o OpenStackLogCollector) CollectMachineLog(ctx context.Context, managementClusterClient client.Client, m *clusterv1.Machine, outputPath string) error {
-	machineLogBase := path.Join(outputPath, "instances", m.Namespace, m.Name)
-	metaLog := path.Join(machineLogBase, "instance.log")
+	Logf("Collecting logs for machine %q and storing them in %q", m.ObjectMeta.Name, outputPath)
 
-	if err := os.MkdirAll(filepath.Dir(metaLog), 0o750); err != nil {
-		_, _ = fmt.Fprintf(GinkgoWriter, "couldn't create directory %q for file: %s\n", metaLog, err)
+	if err := os.MkdirAll(outputPath, 0o750); err != nil {
+		return fmt.Errorf("couldn't create directory %q for logs: %s", outputPath, err)
 	}
 
-	f, err := os.OpenFile(metaLog, os.O_APPEND|os.O_CREATE|os.O_WRONLY, 0o644)
-	if err != nil {
-		_, _ = fmt.Fprintf(GinkgoWriter, "couldn't open file %q: %s\n", metaLog, err)
-		return nil
-	}
-	defer f.Close()
-
 	openStackCluster, err := getOpenStackClusterFromMachine(ctx, managementClusterClient, m)
 	if err != nil {
 		return fmt.Errorf("error getting OpenStackCluster for Machine: %s", err)
@@ -317,30 +201,25 @@ func (o OpenStackLogCollector) CollectMachineLog(ctx context.Context, management
 	}
 	ip := m.Status.Addresses[0].Address
 
-	srvs, err := GetOpenStackServers(&o.E2EContext, openStackCluster, sets.New(m.Spec.InfrastructureRef.Name))
+	srv, err := GetOpenStackServerWithIP(o.E2EContext, getIDFromProviderID(*m.Spec.ProviderID), openStackCluster)
 	if err != nil {
-		return fmt.Errorf("cannot dump machines, could not get servers from OpenStack: %v", err)
-	}
-	if len(srvs) != 1 {
-		return fmt.Errorf("expected exactly 1 server but got %d", len(srvs))
+		return fmt.Errorf("error getting OpenStack server: %w", err)
 	}
-	srv := srvs[m.Spec.InfrastructureRef.Name]
 
 	serverJSON, err := json.MarshalIndent(srv, "", "    ")
 	if err != nil {
 		return fmt.Errorf("error marshalling server %v: %s", srv, err)
 	}
-	if err := os.WriteFile(path.Join(machineLogBase, "server.txt"), serverJSON, 0o600); err != nil {
+	if err := os.WriteFile(path.Join(outputPath, "server.txt"), serverJSON, 0o600); err != nil {
 		return fmt.Errorf("error writing server JSON %s: %s", serverJSON, err)
 	}
-	_, _ = fmt.Fprintf(f, "instance found: %q\n", srv.ID)
 
 	srvUser := o.E2EContext.E2EConfig.GetVariable(SSHUserMachine)
 	executeCommands(
 		ctx,
 		o.E2EContext.Settings.ArtifactFolder,
 		o.E2EContext.Settings.Debug,
-		filepath.Dir(f.Name()),
+		outputPath,
 		ip,
 		openStackCluster.Status.Bastion.FloatingIP,
 		srvUser,

test/e2e/shared/openstack.go

@@ -485,6 +485,41 @@ func GetOpenStackServers(e2eCtx *E2EContext, openStackCluster *infrav1.OpenStack
 	return srvs, nil
 }
 
+// GetOpenStackServer returns the server with the given ID along with the first
+// IP address it has in the OpenStackCluster network.
+func GetOpenStackServerWithIP(e2eCtx *E2EContext, id string, openStackCluster *infrav1.OpenStackCluster) (ServerExtWithIP, error) {
+	srvExtWithIP := ServerExtWithIP{}
+	providerClient, clientOpts, _, err := GetTenantProviderClient(e2eCtx)
+	if err != nil {
+		_, _ = fmt.Fprintf(GinkgoWriter, "error creating provider client: %s\n", err)
+		return srvExtWithIP, nil
+	}
+
+	computeClient, err := clients.NewComputeClient(providerClient, clientOpts)
+	if err != nil {
+		return srvExtWithIP, fmt.Errorf("unable to create compute client: %w", err)
+	}
+	srvExt, err := computeClient.GetServer(id)
+	if err != nil {
+		return srvExtWithIP, fmt.Errorf("unable to get server: %w", err)
+	}
+	srvExtWithIP.ServerExt = *srvExt
+
+	instanceStatus := compute.NewInstanceStatusFromServer(srvExt, logr.Discard())
+	instanceNS, err := instanceStatus.NetworkStatus()
+	if err != nil {
+		return srvExtWithIP, fmt.Errorf("error getting network status for server %s: %v", srvExt.Name, err)
+	}
+
+	ip := instanceNS.IP(openStackCluster.Status.Network.Name)
+	if ip == "" {
+		return srvExtWithIP, fmt.Errorf("error getting internal ip for server %s: internal ip doesn't exist (yet)", srvExt.Name)
+	}
+	srvExtWithIP.ip = ip
+
+	return srvExtWithIP, nil
+}
+
 func GetTenantProviderClient(e2eCtx *E2EContext) (*gophercloud.ProviderClient, *clientconfig.ClientOpts, *string, error) {
 	openstackCloud := e2eCtx.E2EConfig.GetVariable(OpenStackCloud)
 	return getProviderClient(e2eCtx, openstackCloud)

test/e2e/shared/suite.go

@@ -182,7 +182,7 @@ func AllNodesBeforeSuite(e2eCtx *E2EContext, data []byte) {
 	e2eCtx.Settings.ArtifactFolder = conf.ArtifactFolder
 	e2eCtx.Settings.ConfigPath = conf.ConfigPath
 	e2eCtx.Environment.ClusterctlConfigPath = conf.ClusterctlConfigPath
-	withLogCollector := framework.WithMachineLogCollector(OpenStackLogCollector{E2EContext: *e2eCtx})
+	withLogCollector := framework.WithMachineLogCollector(OpenStackLogCollector{E2EContext: e2eCtx})
 	e2eCtx.Environment.BootstrapClusterProxy = framework.NewClusterProxy("bootstrap", conf.KubeconfigPath, e2eCtx.Environment.Scheme, withLogCollector)
 	e2eCtx.E2EConfig = &conf.E2EConfig
 	e2eCtx.Settings.KubetestConfigFilePath = conf.KubetestConfigFilePath

test/e2e/suites/e2e/e2e_test.go

@@ -65,8 +65,9 @@ const specName = "e2e"
 
 var _ = Describe("e2e tests [PR-Blocking]", func() {
 	var (
-		namespace *corev1.Namespace
-		ctx       context.Context
+		namespace        *corev1.Namespace
+		clusterResources *clusterctl.ApplyClusterTemplateAndWaitResult
+		ctx              context.Context
 
 		// Cleanup functions which cannot run until after the cluster has been deleted
 		postClusterCleanup []func()
@@ -77,6 +78,7 @@ var _ = Describe("e2e tests [PR-Blocking]", func() {
 		ctx = context.TODO()
 		// Setup a Namespace where to host objects for this spec and create a watcher for the namespace events.
 		namespace = shared.SetupSpecNamespace(ctx, specName, e2eCtx)
+		clusterResources = new(clusterctl.ApplyClusterTemplateAndWaitResult)
 		Expect(e2eCtx.E2EConfig).ToNot(BeNil(), "Invalid argument. e2eConfig can't be nil when calling %s spec", specName)
 		Expect(e2eCtx.E2EConfig.Variables).To(HaveKey(shared.KubernetesVersion))
 		shared.SetEnvVar("USE_CI_ARTIFACTS", "true", false)
@@ -91,7 +93,8 @@ var _ = Describe("e2e tests [PR-Blocking]", func() {
 			configCluster.ControlPlaneMachineCount = pointer.Int64(3)
 			configCluster.WorkerMachineCount = pointer.Int64(1)
 			configCluster.Flavor = shared.FlavorDefault
-			md := createCluster(ctx, configCluster)
+			createCluster(ctx, configCluster, clusterResources)
+			md := clusterResources.MachineDeployments
 
 			workerMachines := framework.GetMachinesByMachineDeployments(ctx, framework.GetMachinesByMachineDeploymentsInput{
 				Lister:            e2eCtx.Environment.BootstrapClusterProxy.GetClient(),
@@ -169,7 +172,8 @@ var _ = Describe("e2e tests [PR-Blocking]", func() {
 			configCluster.ControlPlaneMachineCount = pointer.Int64(3)
 			configCluster.WorkerMachineCount = pointer.Int64(1)
 			configCluster.Flavor = shared.FlavorFlatcar
-			md := createCluster(ctx, configCluster)
+			createCluster(ctx, configCluster, clusterResources)
+			md := clusterResources.MachineDeployments
 
 			workerMachines := framework.GetMachinesByMachineDeployments(ctx, framework.GetMachinesByMachineDeploymentsInput{
 				Lister:            e2eCtx.Environment.BootstrapClusterProxy.GetClient(),
@@ -210,7 +214,8 @@ var _ = Describe("e2e tests [PR-Blocking]", func() {
 			configCluster.ControlPlaneMachineCount = pointer.Int64(1)
 			configCluster.WorkerMachineCount = pointer.Int64(1)
 			configCluster.Flavor = shared.FlavorWithoutLB
-			md := createCluster(ctx, configCluster)
+			createCluster(ctx, configCluster, clusterResources)
+			md := clusterResources.MachineDeployments
 
 			workerMachines := framework.GetMachinesByMachineDeployments(ctx, framework.GetMachinesByMachineDeploymentsInput{
 				Lister:            e2eCtx.Environment.BootstrapClusterProxy.GetClient(),
@@ -343,7 +348,8 @@ var _ = Describe("e2e tests [PR-Blocking]", func() {
 			configCluster.ControlPlaneMachineCount = pointer.Int64(1)
 			configCluster.WorkerMachineCount = pointer.Int64(1)
 			configCluster.Flavor = shared.FlavorMultiNetwork
-			md = createCluster(ctx, configCluster)
+			createCluster(ctx, configCluster, clusterResources)
+			md = clusterResources.MachineDeployments
 		})
 
 		It("should attach all machines to multiple networks", func() {
@@ -421,7 +427,7 @@ var _ = Describe("e2e tests [PR-Blocking]", func() {
 			configCluster.ControlPlaneMachineCount = pointer.Int64(1)
 			configCluster.WorkerMachineCount = pointer.Int64(0)
 			configCluster.Flavor = shared.FlavorWithoutLB
-			_ = createCluster(ctx, configCluster)
+			createCluster(ctx, configCluster, clusterResources)
 
 			shared.Logf("Creating Machine Deployment in an invalid Availability Zone")
 			mdInvalidAZName := clusterName + "-md-invalid-az"
@@ -474,7 +480,8 @@ var _ = Describe("e2e tests [PR-Blocking]", func() {
 			configCluster.ControlPlaneMachineCount = pointer.Int64(3)
 			configCluster.WorkerMachineCount = pointer.Int64(2)
 			configCluster.Flavor = shared.FlavorMultiAZ
-			md = createCluster(ctx, configCluster)
+			createCluster(ctx, configCluster, clusterResources)
+			md = clusterResources.MachineDeployments
 
 			var err error
 			cluster, err = shared.ClusterForSpec(ctx, e2eCtx, namespace)
@@ -595,6 +602,8 @@ var _ = Describe("e2e tests [PR-Blocking]", func() {
 
 	AfterEach(func() {
 		shared.SetEnvVar("USE_CI_ARTIFACTS", "false", false)
+		shared.Logf("Attempting to collect logs for cluster %q in namespace %q", clusterResources.Cluster.Name, namespace.Name)
+		e2eCtx.Environment.BootstrapClusterProxy.CollectWorkloadClusterLogs(ctx, namespace.Name, clusterResources.Cluster.Name, filepath.Join(e2eCtx.Settings.ArtifactFolder, "clusters", e2eCtx.Environment.BootstrapClusterProxy.GetName(), namespace.Name))
 		// Dumps all the resources in the spec namespace, then cleanups the cluster object and the spec namespace itself.
 		shared.DumpSpecResourcesAndCleanup(ctx, specName, namespace, e2eCtx)
 
@@ -605,17 +614,14 @@ var _ = Describe("e2e tests [PR-Blocking]", func() {
 	})
 })
 
-func createCluster(ctx context.Context, configCluster clusterctl.ConfigClusterInput) []*clusterv1.MachineDeployment {
-	result := &clusterctl.ApplyClusterTemplateAndWaitResult{}
+func createCluster(ctx context.Context, configCluster clusterctl.ConfigClusterInput, result *clusterctl.ApplyClusterTemplateAndWaitResult) {
 	clusterctl.ApplyClusterTemplateAndWait(ctx, clusterctl.ApplyClusterTemplateAndWaitInput{
 		ClusterProxy:                 e2eCtx.Environment.BootstrapClusterProxy,
 		ConfigCluster:                configCluster,
 		WaitForClusterIntervals:      e2eCtx.E2EConfig.GetIntervals(specName, "wait-cluster"),
 		WaitForControlPlaneIntervals: e2eCtx.E2EConfig.GetIntervals(specName, "wait-control-plane"),
 		WaitForMachineDeployments:    e2eCtx.E2EConfig.GetIntervals(specName, "wait-worker-nodes"),
 	}, result)
-
-	return result.MachineDeployments
 }
 
 func defaultConfigCluster(clusterName, namespace string) clusterctl.ConfigClusterInput {