diff --git a/test/e2e/common.go b/test/e2e/common.go index c1166e22de76..c6c683558ed2 100644 --- a/test/e2e/common.go +++ b/test/e2e/common.go @@ -63,7 +63,7 @@ func dumpSpecResourcesAndCleanup(ctx context.Context, specName string, clusterPr Byf("Dumping logs from the %q workload cluster", cluster.Name) // Dump all the logs from the workload cluster before deleting them. - clusterProxy.CollectWorkloadClusterLogs(ctx, cluster.Namespace, cluster.Name, filepath.Join(artifactFolder, "clusters", cluster.Name, "machines")) + clusterProxy.CollectWorkloadClusterLogs(ctx, cluster.Namespace, cluster.Name, filepath.Join(artifactFolder, "clusters", cluster.Name)) Byf("Dumping all the Cluster API resources in the %q namespace", namespace.Name) diff --git a/test/framework/cluster_proxy.go b/test/framework/cluster_proxy.go index 85387c4e0e44..ab058284915c 100644 --- a/test/framework/cluster_proxy.go +++ b/test/framework/cluster_proxy.go @@ -18,11 +18,13 @@ package framework import ( "context" + "errors" "fmt" "net/url" "os" "path" goruntime "runtime" + expv1 "sigs.k8s.io/cluster-api/exp/api/v1alpha4" "strings" . "github.com/onsi/gomega" @@ -81,6 +83,7 @@ type ClusterLogCollector interface { // CollectMachineLog collects log from a machine. // TODO: describe output folder struct CollectMachineLog(ctx context.Context, managementClusterClient client.Client, m *clusterv1.Machine, outputPath string) error + CollectMachinePoolLog(ctx context.Context, managementClusterClient client.Client, m *expv1.MachinePool, outputPath string) error } // Option is a configuration option supplied to NewClusterProxy. @@ -226,22 +229,33 @@ func (p *clusterProxy) CollectWorkloadClusterLogs(ctx context.Context, namespace for i := range machines.Items { m := &machines.Items[i] - err := p.logCollector.CollectMachineLog(ctx, p.GetClient(), m, path.Join(outputPath, m.GetName())) + err := p.logCollector.CollectMachineLog(ctx, p.GetClient(), m, path.Join(outputPath, "machines", m.GetName())) if err != nil { // NB. we are treating failures in collecting logs as a non blocking operation (best effort) fmt.Printf("Failed to get logs for machine %s, cluster %s/%s: %v\n", m.GetName(), namespace, name, err) } } + + machinePools, err := getMachinePoolsInCluster(ctx, p.GetClient(), namespace, name) + Expect(err).ToNot(HaveOccurred(), "Failed to get machine pools for the %s/%s cluster", namespace, name) + + for i := range machinePools.Items { + mp := &machinePools.Items[i] + err := p.logCollector.CollectMachinePoolLog(ctx, p.GetClient(), mp, path.Join(outputPath, "machine-pools", mp.GetName())) + if err != nil { + // NB. we are treating failures in collecting logs as a non blocking operation (best effort) + fmt.Printf("Failed to get logs for machine pool %s, cluster %s/%s: %v\n", mp.GetName(), namespace, name, err) + } + } } func getMachinesInCluster(ctx context.Context, c client.Client, namespace, name string) (*clusterv1.MachineList, error) { if name == "" { - return nil, nil + return nil, errors.New("cluster name should not be empty") } machineList := &clusterv1.MachineList{} labels := map[string]string{clusterv1.ClusterLabelName: name} - if err := c.List(ctx, machineList, client.InNamespace(namespace), client.MatchingLabels(labels)); err != nil { return nil, err } @@ -249,6 +263,20 @@ func getMachinesInCluster(ctx context.Context, c client.Client, namespace, name return machineList, nil } +func getMachinePoolsInCluster(ctx context.Context, c client.Client, namespace, name string) (*expv1.MachinePoolList, error) { + if name == "" { + return nil, errors.New("cluster name should not be empty") + } + + machinePoolList := &expv1.MachinePoolList{} + labels := map[string]string{clusterv1.ClusterLabelName: name} + if err := c.List(ctx, machinePoolList, client.InNamespace(namespace), client.MatchingLabels(labels)); err != nil { + return nil, err + } + + return machinePoolList, nil +} + func (p *clusterProxy) getKubeconfig(ctx context.Context, namespace string, name string) *api.Config { cl := p.GetClient() diff --git a/test/framework/docker_logcollector.go b/test/framework/docker_logcollector.go index e4d8bcceb5ab..5fd730e52f23 100644 --- a/test/framework/docker_logcollector.go +++ b/test/framework/docker_logcollector.go @@ -19,9 +19,11 @@ package framework import ( "context" "fmt" + kerrors "k8s.io/apimachinery/pkg/util/errors" "os" osExec "os/exec" "path/filepath" + expv1 "sigs.k8s.io/cluster-api/exp/api/v1alpha4" "strings" clusterv1 "sigs.k8s.io/cluster-api/api/v1alpha4" @@ -45,6 +47,22 @@ func machineContainerName(cluster, machine string) string { func (k DockerLogCollector) CollectMachineLog(ctx context.Context, managementClusterClient client.Client, m *clusterv1.Machine, outputPath string) error { containerName := machineContainerName(m.Spec.ClusterName, m.Name) + return k.collectLogsFromNode(outputPath, containerName) +} + +func (k DockerLogCollector) CollectMachinePoolLog(ctx context.Context, managementClusterClient client.Client, m *expv1.MachinePool, outputPath string) error { + var errs []error + for _, instance := range m.Status.NodeRefs { + containerName := machineContainerName(m.Spec.ClusterName, instance.Name) + if err := k.collectLogsFromNode(filepath.Join(outputPath, instance.Name), containerName); err != nil { + // collecting logs is best effort so we proceed to the next instance even if we encounter an error. + errs = append(errs, err) + } + } + return kerrors.NewAggregate(errs) +} + +func (k DockerLogCollector) collectLogsFromNode(outputPath string, containerName string) error { execToPathFn := func(outputFileName, command string, args ...string) func() error { return func() error { f, err := fileOnHost(filepath.Join(outputPath, outputFileName)) diff --git a/test/framework/machinepool_helpers.go b/test/framework/machinepool_helpers.go index 744e7e627341..0df4fe7d2446 100644 --- a/test/framework/machinepool_helpers.go +++ b/test/framework/machinepool_helpers.go @@ -20,7 +20,6 @@ import ( "context" "github.com/pkg/errors" corev1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "sigs.k8s.io/cluster-api/test/framework/internal/log" "sigs.k8s.io/cluster-api/util/patch" @@ -92,7 +91,7 @@ type DiscoveryAndWaitForMachinePoolsInput struct { Cluster *clusterv1.Cluster } -// DiscoveryAndWaitForMachinePools discovers the MachinePools existing in a cluster and waits for them to be ready (all the machine provisioned). +// DiscoveryAndWaitForMachinePools discovers the MachinePools existing in a cluster and waits for them to be ready (all the machines provisioned). func DiscoveryAndWaitForMachinePools(ctx context.Context, input DiscoveryAndWaitForMachinePoolsInput, intervals ...interface{}) []*clusterv1exp.MachinePool { Expect(ctx).NotTo(BeNil(), "ctx is required for DiscoveryAndWaitForMachinePools") Expect(input.Lister).ToNot(BeNil(), "Invalid argument. input.Lister can't be nil when calling DiscoveryAndWaitForMachinePools") @@ -131,25 +130,23 @@ func UpgradeMachinePoolAndWait(ctx context.Context, input UpgradeMachinePoolAndW mgmtClient := input.ClusterProxy.GetClient() for i := range input.MachinePools { mp := input.MachinePools[i] - log.Logf("Patching the new kubernetes version to Machine Pool %s/%s", mp.Namespace, mp.Name) + log.Logf("Patching the new Kubernetes version to Machine Pool %s/%s", mp.Namespace, mp.Name) patchHelper, err := patch.NewHelper(mp, mgmtClient) Expect(err).ToNot(HaveOccurred()) + oldVersion := mp.Spec.Template.Spec.Version mp.Spec.Template.Spec.Version = &input.UpgradeVersion Expect(patchHelper.Patch(ctx, mp)).To(Succeed()) - } - for i := range input.MachinePools { - mp := input.MachinePools[i] - oldVersion := mp.Spec.Template.Spec.Version log.Logf("Waiting for Kubernetes versions of machines in MachinePool %s/%s to be upgraded from %s to %s", mp.Namespace, mp.Name, *oldVersion, input.UpgradeVersion) WaitForMachinePoolInstancesToBeUpgraded(ctx, WaitForMachinePoolInstancesToBeUpgradedInput{ Getter: mgmtClient, + WorkloadClusterGetter: input.ClusterProxy.GetWorkloadCluster(ctx, input.Cluster.Namespace, input.Cluster.Name).GetClient(), Cluster: input.Cluster, MachineCount: int(*mp.Spec.Replicas), KubernetesUpgradeVersion: input.UpgradeVersion, - MachinePool: *mp, + MachinePool: mp, }, input.WaitForMachinePoolToBeUpgraded...) } } @@ -190,10 +187,11 @@ func ScaleMachinePoolAndWait(ctx context.Context, input ScaleMachinePoolAndWaitI // WaitForMachinePoolInstancesToBeUpgradedInput is the input for WaitForMachinePoolInstancesToBeUpgraded. type WaitForMachinePoolInstancesToBeUpgradedInput struct { Getter Getter + WorkloadClusterGetter Getter Cluster *clusterv1.Cluster KubernetesUpgradeVersion string MachineCount int - MachinePool clusterv1exp.MachinePool + MachinePool *clusterv1exp.MachinePool } // WaitForMachinePoolInstancesToBeUpgraded waits until all instances belonging to a MachinePool are upgraded to the correct kubernetes version. @@ -207,10 +205,17 @@ func WaitForMachinePoolInstancesToBeUpgraded(ctx context.Context, input WaitForM log.Logf("Ensuring all MachinePool Instances have upgraded kubernetes version %s", input.KubernetesUpgradeVersion) Eventually(func() (int, error) { - versions := GetMachinePoolInstanceVersions(ctx, GetMachinesPoolInstancesInput{ - Getter: input.Getter, - Namespace: input.Cluster.Namespace, - MachinePool: input.MachinePool, + nn := client.ObjectKey{ + Namespace: input.MachinePool.Namespace, + Name: input.MachinePool.Name, + } + if err := input.Getter.Get(ctx, nn, input.MachinePool); err != nil { + return 0, err + } + versions := getMachinePoolInstanceVersions(ctx, GetMachinesPoolInstancesInput{ + WorkloadClusterGetter: input.WorkloadClusterGetter, + Namespace: input.Cluster.Namespace, + MachinePool: input.MachinePool, }) matches := 0 @@ -230,41 +235,30 @@ func WaitForMachinePoolInstancesToBeUpgraded(ctx context.Context, input WaitForM // GetMachinesPoolInstancesInput is the input for GetMachinesPoolInstances. type GetMachinesPoolInstancesInput struct { - Getter Getter - Namespace string - MachinePool clusterv1exp.MachinePool + WorkloadClusterGetter Getter + Namespace string + MachinePool *clusterv1exp.MachinePool } -// GetMachinePoolInstanceVersions returns the. -func GetMachinePoolInstanceVersions(ctx context.Context, input GetMachinesPoolInstancesInput) []string { - Expect(ctx).NotTo(BeNil(), "ctx is required for GetMachinePoolInstanceVersions") - Expect(input.Namespace).ToNot(BeEmpty(), "Invalid argument. input.Namespace can't be empty when calling GetMachinePoolInstanceVersions") - Expect(input.MachinePool).ToNot(BeNil(), "Invalid argument. input.MachineDeployment can't be nil when calling GetMachinePoolInstanceVersions") - - obj := getUnstructuredRef(ctx, input.Getter, &input.MachinePool.Spec.Template.Spec.InfrastructureRef, input.Namespace) - instances, found, err := unstructured.NestedSlice(obj.Object, "status", "instances") - Expect(err).ToNot(HaveOccurred(), "failed to extract machines from unstructured") - if !found { - return nil - } +// getMachinePoolInstanceVersions returns the Kubernetes versions of the machine pool instances. +func getMachinePoolInstanceVersions(ctx context.Context, input GetMachinesPoolInstancesInput) []string { + Expect(ctx).NotTo(BeNil(), "ctx is required for getMachinePoolInstanceVersions") + Expect(input.WorkloadClusterGetter).ToNot(BeNil(), "Invalid argument. input.WorkloadClusterGetter can't be nil when calling getMachinePoolInstanceVersions") + Expect(input.Namespace).ToNot(BeEmpty(), "Invalid argument. input.Namespace can't be empty when calling getMachinePoolInstanceVersions") + Expect(input.MachinePool).ToNot(BeNil(), "Invalid argument. input.MachinePool can't be nil when calling getMachinePoolInstanceVersions") + instances := input.MachinePool.Status.NodeRefs versions := make([]string, len(instances)) for i, instance := range instances { - version, found, err := unstructured.NestedString(instance.(map[string]interface{}), "version") - Expect(err).ToNot(HaveOccurred(), "failed to extract versions from unstructured instance") - Expect(found).To(BeTrue(), "unable to find nested version string in unstructured instance") - versions[i] = version + node := &corev1.Node{} + err := input.WorkloadClusterGetter.Get(ctx, client.ObjectKey{Name: instance.Name}, node) + if err != nil { + versions[i] = "unknown" + } else { + versions[i] = node.Status.NodeInfo.KubeletVersion + } + log.Logf("Node %s version is %s", instance.Name, versions[i]) } return versions } - -func getUnstructuredRef(ctx context.Context, getter Getter, ref *corev1.ObjectReference, namespace string) *unstructured.Unstructured { - obj := new(unstructured.Unstructured) - obj.SetAPIVersion(ref.APIVersion) - obj.SetKind(ref.Kind) - obj.SetName(ref.Name) - key := client.ObjectKey{Name: obj.GetName(), Namespace: namespace} - Expect(getter.Get(ctx, key, obj)).ToNot(HaveOccurred(), "failed to retrieve %s object %q/%q", obj.GetKind(), key.Namespace, key.Name) - return obj -}