From a801c2a0390064b9f3c0886949b5c1e937972f1b Mon Sep 17 00:00:00 2001 From: Christian Schlotter Date: Thu, 16 May 2024 17:29:35 +0200 Subject: [PATCH] test: fix collector for machines not having an IP in status --- .../main/ignition/kustomization.yaml | 7 ++ .../main/ignition/patch-user-kcp.yaml | 3 + .../main/ignition/patch-user-md.yaml | 3 + test/e2e/e2e_suite_test.go | 5 +- test/framework/log/collector.go | 89 ++++++++++++++++--- 5 files changed, 92 insertions(+), 15 deletions(-) create mode 100644 test/e2e/data/infrastructure-vsphere-govmomi/main/ignition/patch-user-kcp.yaml create mode 100644 test/e2e/data/infrastructure-vsphere-govmomi/main/ignition/patch-user-md.yaml diff --git a/test/e2e/data/infrastructure-vsphere-govmomi/main/ignition/kustomization.yaml b/test/e2e/data/infrastructure-vsphere-govmomi/main/ignition/kustomization.yaml index 6412c24716..f0e074027b 100644 --- a/test/e2e/data/infrastructure-vsphere-govmomi/main/ignition/kustomization.yaml +++ b/test/e2e/data/infrastructure-vsphere-govmomi/main/ignition/kustomization.yaml @@ -14,3 +14,10 @@ patches: - target: kind: VSphereMachineTemplate path: ../commons/remove-storage-policy.yaml + # Replace ssh user to match expected user by the e2e machine collector + - target: + kind: KubeadmControlPlane + path: patch-user-kcp.yaml + - target: + kind: KubeadmConfigTemplate + path: patch-user-md.yaml diff --git a/test/e2e/data/infrastructure-vsphere-govmomi/main/ignition/patch-user-kcp.yaml b/test/e2e/data/infrastructure-vsphere-govmomi/main/ignition/patch-user-kcp.yaml new file mode 100644 index 0000000000..e887055f54 --- /dev/null +++ b/test/e2e/data/infrastructure-vsphere-govmomi/main/ignition/patch-user-kcp.yaml @@ -0,0 +1,3 @@ +- op: add + path: /spec/kubeadmConfigSpec/users/0/name + value: "capv" diff --git a/test/e2e/data/infrastructure-vsphere-govmomi/main/ignition/patch-user-md.yaml b/test/e2e/data/infrastructure-vsphere-govmomi/main/ignition/patch-user-md.yaml new file mode 100644 index 0000000000..b737eb7032 --- /dev/null +++ b/test/e2e/data/infrastructure-vsphere-govmomi/main/ignition/patch-user-md.yaml @@ -0,0 +1,3 @@ +- op: add + path: /spec/template/spec/users/0/name + value: "capv" diff --git a/test/e2e/e2e_suite_test.go b/test/e2e/e2e_suite_test.go index 537e9d4d7a..e7c0dfb54b 100644 --- a/test/e2e/e2e_suite_test.go +++ b/test/e2e/e2e_suite_test.go @@ -266,7 +266,10 @@ var _ = SynchronizedBeforeSuite(func() []byte { // vspherelog.MachineLogCollector tries to ssh to the machines to collect logs. // This does not work when using vcsim because there are no real machines running ssh. if testTarget != VCSimTestTarget { - clusterProxyOptions = append(clusterProxyOptions, framework.WithMachineLogCollector(vspherelog.MachineLogCollector{})) + clusterProxyOptions = append(clusterProxyOptions, framework.WithMachineLogCollector(&vspherelog.MachineLogCollector{ + Client: vsphereClient, + Finder: vsphereFinder, + })) } bootstrapClusterProxy = framework.NewClusterProxy("bootstrap", kubeconfigPath, initScheme(), clusterProxyOptions...) diff --git a/test/framework/log/collector.go b/test/framework/log/collector.go index ce58a8bc72..c28f9366a4 100644 --- a/test/framework/log/collector.go +++ b/test/framework/log/collector.go @@ -22,12 +22,18 @@ import ( "context" "fmt" "io" + "net" "os" "path/filepath" "strings" "github.com/pkg/errors" + "github.com/vmware/govmomi" + "github.com/vmware/govmomi/find" + "github.com/vmware/govmomi/vim25/mo" "golang.org/x/crypto/ssh" + kerrors "k8s.io/apimachinery/pkg/util/errors" + "k8s.io/klog/v2" clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" expv1 "sigs.k8s.io/cluster-api/exp/api/v1beta1" "sigs.k8s.io/controller-runtime/pkg/client" @@ -39,20 +45,19 @@ const ( VSpherePrivateKeyFilePath = "VSPHERE_SSH_PRIVATE_KEY" ) -type MachineLogCollector struct{} +type MachineLogCollector struct { + Client *govmomi.Client + Finder *find.Finder +} -func (collector MachineLogCollector) CollectMachinePoolLog(_ context.Context, _ client.Client, _ *expv1.MachinePool, _ string) error { +func (c *MachineLogCollector) CollectMachinePoolLog(_ context.Context, _ client.Client, _ *expv1.MachinePool, _ string) error { return nil } -func (collector MachineLogCollector) CollectMachineLog(_ context.Context, _ client.Client, m *clusterv1.Machine, outputPath string) error { - var hostIPAddr string - for _, address := range m.Status.Addresses { - if address.Type != clusterv1.MachineExternalIP { - continue - } - hostIPAddr = address.Address - break +func (c *MachineLogCollector) CollectMachineLog(ctx context.Context, _ client.Client, m *clusterv1.Machine, outputPath string) error { + machineIPAddresses, err := c.machineIPAddresses(ctx, m) + if err != nil { + return err } captureLogs := func(hostFileName, command string, args ...string) func() error { @@ -62,7 +67,20 @@ func (collector MachineLogCollector) CollectMachineLog(_ context.Context, _ clie return err } defer f.Close() - return executeRemoteCommand(f, hostIPAddr, command, args...) + var errs []error + // Try with all available IPs unless it succeeded. + for _, machineIPAddress := range machineIPAddresses { + if err := executeRemoteCommand(f, machineIPAddress, command, args...); err != nil { + errs = append(errs, err) + continue + } + return nil + } + + if err := kerrors.NewAggregate(errs); err != nil { + return errors.Wrapf(err, "Failed to run command %s for machine %s on ips [%s]", command, klog.KObj(m), strings.Join(machineIPAddresses, ", ")) + } + return nil } } @@ -78,10 +96,53 @@ func (collector MachineLogCollector) CollectMachineLog(_ context.Context, _ clie }) } -func (collector MachineLogCollector) CollectInfrastructureLogs(_ context.Context, _ client.Client, _ *clusterv1.Cluster, _ string) error { +func (c *MachineLogCollector) CollectInfrastructureLogs(_ context.Context, _ client.Client, _ *clusterv1.Cluster, _ string) error { return nil } +func (c *MachineLogCollector) machineIPAddresses(ctx context.Context, m *clusterv1.Machine) ([]string, error) { + for _, address := range m.Status.Addresses { + if address.Type != clusterv1.MachineExternalIP { + continue + } + return []string{address.Address}, nil + } + + vmObj, err := c.Finder.VirtualMachine(ctx, m.GetName()) + if err != nil { + return nil, err + } + + var vm mo.VirtualMachine + + if err := c.Client.RetrieveOne(ctx, vmObj.Reference(), []string{"guest.net"}, &vm); err != nil { + // We cannot get the properties e.g. when the vm already got deleted or is getting deleted. + return nil, errors.Errorf("Error retrieving properties for machine %s", klog.KObj(m)) + } + + addresses := []string{} + + // Return all IPs so we can try each of them until one succeeded. + for _, nic := range vm.Guest.Net { + if nic.IpConfig == nil { + continue + } + for _, ip := range nic.IpConfig.IpAddress { + netIP := net.ParseIP(ip.IpAddress) + ipv4 := netIP.To4() + if ipv4 != nil { + addresses = append(addresses, ip.IpAddress) + } + } + } + + if len(addresses) == 0 { + return nil, errors.Errorf("Unable to find IP Addresses for Machine %s", klog.KObj(m)) + } + + return addresses, nil +} + func createOutputFile(path string) (*os.File, error) { if err := os.MkdirAll(filepath.Dir(path), os.ModePerm); err != nil { return nil, err @@ -133,7 +194,7 @@ func newSSHConfig() (*ssh.ClientConfig, error) { signer, err := ssh.ParsePrivateKey(sshPrivateKeyContent) if err != nil { - return nil, errors.Wrapf(err, fmt.Sprintf("error parsing private key: %s", sshPrivateKeyContent)) + return nil, errors.Wrapf(err, fmt.Sprintf("Error parsing private key: %s", sshPrivateKeyContent)) } config := &ssh.ClientConfig{ @@ -150,7 +211,7 @@ func newSSHConfig() (*ssh.ClientConfig, error) { func readPrivateKey() ([]byte, error) { privateKeyFilePath := os.Getenv(VSpherePrivateKeyFilePath) if privateKeyFilePath == "" { - return nil, errors.Errorf("private key information missing. Please set %s environment variable", VSpherePrivateKeyFilePath) + return nil, errors.Errorf("Private key information missing. Please set %s environment variable", VSpherePrivateKeyFilePath) } return os.ReadFile(filepath.Clean(privateKeyFilePath))