Make e2e tests more robust

test/vm_helpers.go

@@ -3,9 +3,11 @@ package test
 import (
 	"fmt"
 	"net"
+	"os"
 	"os/exec"
 	"path/filepath"
 	"strconv"
+	"strings"
 	"testing"
 	"time"
 
@@ -17,79 +19,95 @@ import (
 	"golang.org/x/crypto/ssh"
 )
 
-func sshOverTCP(macAddress string, sshPort int, sshConfig *ssh.ClientConfig) (*ssh.Client, error) {
+func retryIPFromMAC(errCh chan error, macAddress string) (string, error) {
 	var (
 		err error
 		ip  string
 	)
 
-	for i := 0; i < 100; i++ {
-		ip, err = vfkithelpers.GetIPAddressByMACAddress(macAddress)
-		if err == nil {
-			break
-		}
-
-		time.Sleep(time.Second)
-	}
-	if err != nil {
-		return nil, err
-	}
-	log.Infof("found IP address %s for MAC %s", ip, macAddress)
-
-	var sshClient *ssh.Client
-	for i := 0; i < 10; i++ {
-		sshClient, err = ssh.Dial("tcp", net.JoinHostPort(ip, strconv.Itoa(sshPort)), sshConfig)
-		if err == nil {
-			break
+	for {
+		select {
+		case err := <-errCh:
+			return "", err
+		case <-time.After(1 * time.Second):
+			ip, err = vfkithelpers.GetIPAddressByMACAddress(macAddress)
+			if err == nil {
+				log.Infof("found IP address %s for MAC %s", ip, macAddress)
+				return ip, nil
+			}
+		case <-time.After(10 * time.Second):
+			return "", fmt.Errorf("timeout getting IP from MAC: %w", err)
 		}
-		time.Sleep(time.Second)
 	}
-	log.Infof("established SSH connection to %s:%d over TCP", ip, sshPort)
-	return sshClient, err
 }
 
-func sshOverVsock(unixSocket string, sshConfig *ssh.ClientConfig) (*ssh.Client, error) {
+func retrySSHDial(errCh chan error, scheme string, address string, sshConfig *ssh.ClientConfig) (*ssh.Client, error) {
 	var (
 		sshClient *ssh.Client
 		err       error
 	)
-	for i := 0; i < 100; i++ {
-		sshClient, err = ssh.Dial("unix", unixSocket, sshConfig)
-		if err == nil {
-			break
+	for {
+		select {
+		case err := <-errCh:
+			return nil, err
+		case <-time.After(1 * time.Second):
+			log.Debugf("trying ssh dial")
+			sshClient, err = ssh.Dial(scheme, address, sshConfig)
+			if err == nil {
+				log.Infof("established SSH connection to %s over %s", address, scheme)
+				return sshClient, nil
+			}
+			log.Debugf("ssh failed: %v", err)
+		case <-time.After(10 * time.Second):
+			return nil, fmt.Errorf("timeout waiting for SSH: %w", err)
 		}
-		time.Sleep(time.Second)
 	}
-	log.Infof("established SSH connection over Unix socket %s", unixSocket)
-	return sshClient, err
 }
 
 type vfkitRunner struct {
 	*exec.Cmd
+	errCh              chan error
 	gracefullyShutdown bool
 }
 
 func startVfkit(t *testing.T, vm *config.VirtualMachine) *vfkitRunner {
 	const vfkitRelativePath = "../out/vfkit"
 
+	logFilePath := filepath.Join(t.TempDir(), fmt.Sprintf("%s.log", strings.ReplaceAll(t.Name(), "/", "")))
+	logFile, err := os.Create(logFilePath)
+	require.NoError(t, err)
+	log.Infof("vfkit log file: %s", logFilePath)
+
 	binaryPath, err := exec.LookPath(vfkitRelativePath)
 	require.NoError(t, err)
 
 	log.Infof("starting %s", binaryPath)
 	vfkitCmd, err := vm.Cmd(binaryPath)
 	require.NoError(t, err)
+	vfkitCmd.Stdout = logFile
+	vfkitCmd.Stderr = logFile
 
 	err = vfkitCmd.Start()
 	require.NoError(t, err)
+	errCh := make(chan error)
+	go func() {
+		err := vfkitCmd.Wait()
+		if err != nil {
+			log.Infof("vfkitCmd.Wait() returned %v", err)
+		}
+		errCh <- err
+		close(errCh)
+	}()
 
 	return &vfkitRunner{
 		vfkitCmd,
+		errCh,
 		false,
 	}
 }
 
 func (cmd *vfkitRunner) Wait(t *testing.T) {
-	err := cmd.Cmd.Wait()
+	err := <-cmd.errCh
 	require.NoError(t, err)
 	cmd.gracefullyShutdown = true
 }
@@ -202,10 +220,12 @@ func (vm *testVM) WaitForSSH(t *testing.T) {
 	)
 	switch vm.sshNetwork {
 	case "tcp":
-		sshClient, err = sshOverTCP(vm.macAddress, vm.port, vm.provider.SSHConfig())
+		ip, err := retryIPFromMAC(vm.vfkitCmd.errCh, vm.macAddress)
+		require.NoError(t, err)
+		sshClient, err = retrySSHDial(vm.vfkitCmd.errCh, "tcp", net.JoinHostPort(ip, strconv.Itoa(vm.port)), vm.provider.SSHConfig())
 		require.NoError(t, err)
 	case "vsock":
-		sshClient, err = sshOverVsock(vm.vsockPath, vm.provider.SSHConfig())
+		sshClient, err = retrySSHDial(vm.vfkitCmd.errCh, "unix", vm.vsockPath, vm.provider.SSHConfig())
 		require.NoError(t, err)
 	default:
 		t.FailNow()

test/vm_test.go

@@ -16,6 +16,29 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
+func TestFailedVfkitStart(t *testing.T) {
+	puipuiProvider := NewPuipuiProvider()
+	log.Info("fetching os image")
+	err := puipuiProvider.Fetch(t.TempDir())
+	require.NoError(t, err)
+
+	vm := NewTestVM(t, puipuiProvider)
+	defer vm.Close(t)
+	require.NotNil(t, vm)
+
+	vm.AddSSH(t, "vsock")
+
+	dev, err := config.NVMExpressControllerNew("/a/b")
+	require.NoError(t, err)
+	vm.AddDevice(t, dev)
+
+	vm.Start(t)
+
+	log.Infof("waiting for SSH")
+	_, err = retrySSHDial(vm.vfkitCmd.errCh, "unix", vm.vsockPath, vm.provider.SSHConfig())
+	require.Error(t, err)
+}
+
 func testSSHAccess(t *testing.T, vm *testVM, network string) {
 	log.Infof("testing SSH access over %s", network)
 	vm.AddSSH(t, network)
@@ -228,6 +251,25 @@ var pciidMacOS13Tests = map[string]pciidTest{
 	},
 }
 
+var pciidMacOS14Tests = map[string]pciidTest{
+	"nvm-express": {
+		vendorID: 0x106b, // Apple
+		deviceID: 0x1a09,
+		createDev: func(t *testing.T) (config.VirtioDevice, error) {
+			diskimg := filepath.Join(t.TempDir(), "nvmexpress.img")
+			f, err := os.Create(diskimg)
+			require.NoError(t, err)
+			require.NoError(t, f.Close())
+			return config.NVMExpressControllerNew(diskimg)
+		},
+	},
+}
+
+var pciidVersionedTests = map[int]map[string]pciidTest{
+	13: pciidMacOS13Tests,
+	14: pciidMacOS14Tests,
+}
+
 func testPCIId(t *testing.T, test pciidTest, provider OsProvider) {
 	vm := NewTestVM(t, provider)
 	defer vm.Close(t)
@@ -257,14 +299,16 @@ func TestPCIIds(t *testing.T) {
 		})
 	}
 
-	if err := macOSAvailable(13); err == nil {
-		for name, test := range pciidMacOS13Tests {
-			t.Run(name, func(t *testing.T) {
-				testPCIId(t, test, puipuiProvider)
-			})
+	for macosVersion, tests := range pciidVersionedTests {
+		if err := macOSAvailable(float64(macosVersion)); err == nil {
+			for name, test := range tests {
+				t.Run(name, func(t *testing.T) {
+					testPCIId(t, test, puipuiProvider)
+				})
+			}
+		} else {
+			t.Logf("Skipping macOS %d tests", macosVersion)
 		}
-	} else {
-		t.Log("Skipping macOS 13 tests")
 	}
 }