Add tests to virtualbox driver

Signed-off-by: David Gageot <david@gageot.net>

drivers/virtualbox/disk.go

@@ -11,6 +11,16 @@ type VirtualDisk struct {
 	Path string
 }
 
+func (d *Driver) getVMDiskInfo() (*VirtualDisk, error) {
+	out, err := d.vbmOut("showvminfo", d.MachineName, "--machinereadable")
+	if err != nil {
+		return nil, err
+	}
+
+	r := strings.NewReader(out)
+	return parseDiskInfo(r)
+}
+
 func parseDiskInfo(r io.Reader) (*VirtualDisk, error) {
 	s := bufio.NewScanner(r)
 	disk := &VirtualDisk{}
@@ -36,12 +46,3 @@ func parseDiskInfo(r io.Reader) (*VirtualDisk, error) {
 	}
 	return disk, nil
 }
-
-func getVMDiskInfo(name string) (*VirtualDisk, error) {
-	out, err := vbmOut("showvminfo", name, "--machinereadable")
-	if err != nil {
-		return nil, err
-	}
-	r := strings.NewReader(out)
-	return parseDiskInfo(r)
-}

drivers/virtualbox/network.go

@@ -31,7 +31,21 @@ type hostOnlyNetwork struct {
 	NetworkName string // referenced in DHCP.NetworkName
 }
 
-// Config changes the configuration of the host-only network.
+// TODO: use VBoxManager.vbm() instead
+func vbm(args ...string) error {
+	vBoxManager := &VBoxCmdManager{}
+	_, _, err := vBoxManager.vbmOutErr(args...)
+	return err
+}
+
+// TODO: use VBoxManager.vbmOut() instead
+func vbmOut(args ...string) (string, error) {
+	vBoxManager := &VBoxCmdManager{}
+	stdout, _, err := vBoxManager.vbmOutErr(args...)
+	return stdout, err
+}
+
+// Save changes the configuration of the host-only network.
 func (n *hostOnlyNetwork) Save() error {
 	if n.IPv4.IP != nil && n.IPv4.Mask != nil {
 		if err := vbm("hostonlyif", "ipconfig", n.Name, "--ip", n.IPv4.IP.String(), "--netmask", net.IP(n.IPv4.Mask).String()); err != nil {
@@ -66,7 +80,7 @@ func createHostonlyNet() (*hostOnlyNetwork, error) {
 	return &hostOnlyNetwork{Name: res[1]}, nil
 }
 
-// HostonlyNets gets all host-only networks in a  map keyed by HostonlyNet.NetworkName.
+// listHostOnlyNetworks gets all host-only networks in a  map keyed by HostonlyNet.NetworkName.
 func listHostOnlyNetworks() (map[string]*hostOnlyNetwork, error) {
 	out, err := vbmOut("list", "hostonlyifs")
 	if err != nil {
@@ -211,17 +225,17 @@ func addDHCPServer(kind, name string, d dhcpServer) error {
 	return vbm(args...)
 }
 
-// AddInternalDHCP adds a DHCP server to an internal network.
+// addInternalDHCP adds a DHCP server to an internal network.
 func addInternalDHCP(netname string, d dhcpServer) error {
 	return addDHCPServer("--netname", netname, d)
 }
 
-// AddHostonlyDHCP adds a DHCP server to a host-only network.
+// addHostonlyDHCP adds a DHCP server to a host-only network.
 func addHostonlyDHCP(ifname string, d dhcpServer) error {
 	return addDHCPServer("--netname", "HostInterfaceNetworking-"+ifname, d)
 }
 
-// DHCPs gets all DHCP server settings in a map keyed by DHCP.NetworkName.
+// getDHCPServers gets all DHCP server settings in a map keyed by DHCP.NetworkName.
 func getDHCPServers() (map[string]*dhcpServer, error) {
 	out, err := vbmOut("list", "dhcpservers")
 	if err != nil {

drivers/virtualbox/vbm.go

@@ -15,58 +15,40 @@ import (
 )
 
 var (
-	reVMNameUUID      = regexp.MustCompile(`"(.+)" {([0-9a-f-]+)}`)
-	reVMInfoLine      = regexp.MustCompile(`(?:"(.+)"|(.+))=(?:"(.*)"|(.*))`)
 	reColonLine       = regexp.MustCompile(`(.+):\s+(.*)`)
 	reEqualLine       = regexp.MustCompile(`(.+)=(.*)`)
 	reEqualQuoteLine  = regexp.MustCompile(`"(.+)"="(.*)"`)
 	reMachineNotFound = regexp.MustCompile(`Could not find a registered machine named '(.+)'`)
-)
 
-var (
-	ErrMachineExist    = errors.New("machine already exists")
 	ErrMachineNotExist = errors.New("machine does not exist")
 	ErrVBMNotFound     = errors.New("VBoxManage not found")
-	vboxManageCmd      = setVBoxManageCmd()
+
+	vboxManageCmd = detectVBoxManageCmd()
 )
 
-// detect the VBoxManage cmd's path if needed
+// VBoxManager defines the interface to communicate to VirtualBox.
-func setVBoxManageCmd() string {
+type VBoxManager interface {
-	cmd := "VBoxManage"
+	vbm(args ...string) error
-	if path, err := exec.LookPath(cmd); err == nil {
+
-		return path
+	vbmOut(args ...string) (string, error)
-	}
+
-	if runtime.GOOS == "windows" {
+	vbmOutErr(args ...string) (string, string, error)
-		if p := os.Getenv("VBOX_INSTALL_PATH"); p != "" {
-			if path, err := exec.LookPath(filepath.Join(p, cmd)); err == nil {
-				return path
-			}
-		}
-		if p := os.Getenv("VBOX_MSI_INSTALL_PATH"); p != "" {
-			if path, err := exec.LookPath(filepath.Join(p, cmd)); err == nil {
-				return path
-			}
-		}
-		// look at HKEY_LOCAL_MACHINE\SOFTWARE\Oracle\VirtualBox\InstallDir
-		p := "C:\\Program Files\\Oracle\\VirtualBox"
-		if path, err := exec.LookPath(filepath.Join(p, cmd)); err == nil {
-			return path
-		}
-	}
-	return cmd
 }
 
-func vbm(args ...string) error {
+// VBoxCmdManager communicates with VirtualBox through the commandline using `VBoxManage`.
-	_, _, err := vbmOutErr(args...)
+type VBoxCmdManager struct{}
+
+func (v *VBoxCmdManager) vbm(args ...string) error {
+	_, _, err := v.vbmOutErr(args...)
 	return err
 }
 
-func vbmOut(args ...string) (string, error) {
+func (v *VBoxCmdManager) vbmOut(args ...string) (string, error) {
-	stdout, _, err := vbmOutErr(args...)
+	stdout, _, err := v.vbmOutErr(args...)
 	return stdout, err
 }
 
-func vbmOutErr(args ...string) (string, string, error) {
+func (v *VBoxCmdManager) vbmOutErr(args ...string) (string, string, error) {
 	cmd := exec.Command(vboxManageCmd, args...)
 	log.Debugf("COMMAND: %v %v", vboxManageCmd, strings.Join(args, " "))
 	var stdout bytes.Buffer
@@ -92,3 +74,31 @@ func vbmOutErr(args ...string) (string, string, error) {
 	}
 	return stdout.String(), stderrStr, err
 }
+
+// detectVBoxManageCmd detects the VBoxManage cmd's path if needed
+func detectVBoxManageCmd() string {
+	cmd := "VBoxManage"
+	if path, err := exec.LookPath(cmd); err == nil {
+		return path
+	}
+
+	if runtime.GOOS == "windows" {
+		if p := os.Getenv("VBOX_INSTALL_PATH"); p != "" {
+			if path, err := exec.LookPath(filepath.Join(p, cmd)); err == nil {
+				return path
+			}
+		}
+		if p := os.Getenv("VBOX_MSI_INSTALL_PATH"); p != "" {
+			if path, err := exec.LookPath(filepath.Join(p, cmd)); err == nil {
+				return path
+			}
+		}
+		// look at HKEY_LOCAL_MACHINE\SOFTWARE\Oracle\VirtualBox\InstallDir
+		p := "C:\\Program Files\\Oracle\\VirtualBox"
+		if path, err := exec.LookPath(filepath.Join(p, cmd)); err == nil {
+			return path
+		}
+	}
+
+	return cmd
+}

drivers/virtualbox/virtualbox.go

@@ -48,6 +48,7 @@ var (
 )
 
 type Driver struct {
+	VBoxManager
 	*drivers.BaseDriver
 	CPU                 int
 	Memory              int
@@ -60,8 +61,10 @@ type Driver struct {
 	NoShare             bool
 }
 
+// NewDriver creates a new VirtualBox driver with default settings.
 func NewDriver(hostName, storePath string) *Driver {
 	return &Driver{
+		VBoxManager: &VBoxCmdManager{},
 		BaseDriver: &drivers.BaseDriver{
 			MachineName: hostName,
 			StorePath:   storePath,
@@ -75,6 +78,8 @@ func NewDriver(hostName, storePath string) *Driver {
 	}
 }
 
+// GetCreateFlags registers the flags this driver adds to
+// "docker hosts create"
 func (d *Driver) GetCreateFlags() []mcnflag.Flag {
 	return []mcnflag.Flag{
 		mcnflag.IntFlag{
@@ -176,9 +181,10 @@ func (d *Driver) SetConfigFromFlags(flags drivers.DriverOptions) error {
 	return nil
 }
 
+// PreCreateCheck checks that VBoxManage exists and works
 func (d *Driver) PreCreateCheck() error {
 	// Check that VBoxManage exists and works
-	return vbm()
+	return d.vbm()
 }
 
 // IsVTXDisabled checks if VT-X is disabled in the BIOS. If it is, the vm will fail to start.
@@ -250,9 +256,9 @@ func (d *Driver) Create() error {
 		name := d.Boot2DockerImportVM
 
 		// make sure vm is stopped
-		_ = vbm("controlvm", name, "poweroff")
+		_ = d.vbm("controlvm", name, "poweroff")
 
-		diskInfo, err := getVMDiskInfo(name)
+		diskInfo, err := d.getVMDiskInfo()
 		if err != nil {
 			return err
 		}
@@ -261,12 +267,12 @@ func (d *Driver) Create() error {
 			return err
 		}
 
-		if err := vbm("clonehd", diskInfo.Path, d.diskPath()); err != nil {
+		if err := d.vbm("clonehd", diskInfo.Path, d.diskPath()); err != nil {
 			return err
 		}
 
 		log.Debugf("Importing VM settings...")
-		vmInfo, err := getVMInfo(name)
+		vmInfo, err := d.getVMInfo()
 		if err != nil {
 			return err
 		}
@@ -291,7 +297,7 @@ func (d *Driver) Create() error {
 		}
 	}
 
-	if err := vbm("createvm",
+	if err := d.vbm("createvm",
 		"--basefolder", d.ResolveStorePath("."),
 		"--name", d.MachineName,
 		"--register"); err != nil {
@@ -309,7 +315,7 @@ func (d *Driver) Create() error {
 		cpus = 32
 	}
 
-	if err := vbm("modifyvm", d.MachineName,
+	if err := d.vbm("modifyvm", d.MachineName,
 		"--firmware", "bios",
 		"--bioslogofadein", "off",
 		"--bioslogofadeout", "off",
@@ -335,7 +341,7 @@ func (d *Driver) Create() error {
 		return err
 	}
 
-	if err := vbm("modifyvm", d.MachineName,
+	if err := d.vbm("modifyvm", d.MachineName,
 		"--nic1", "nat",
 		"--nictype1", "82540EM",
 		"--cableconnected1", "on"); err != nil {
@@ -346,14 +352,14 @@ func (d *Driver) Create() error {
 		return err
 	}
 
-	if err := vbm("storagectl", d.MachineName,
+	if err := d.vbm("storagectl", d.MachineName,
 		"--name", "SATA",
 		"--add", "sata",
 		"--hostiocache", "on"); err != nil {
 		return err
 	}
 
-	if err := vbm("storageattach", d.MachineName,
+	if err := d.vbm("storageattach", d.MachineName,
 		"--storagectl", "SATA",
 		"--port", "0",
 		"--device", "0",
@@ -362,7 +368,7 @@ func (d *Driver) Create() error {
 		return err
 	}
 
-	if err := vbm("storageattach", d.MachineName,
+	if err := d.vbm("storageattach", d.MachineName,
 		"--storagectl", "SATA",
 		"--port", "1",
 		"--device", "0",
@@ -372,10 +378,10 @@ func (d *Driver) Create() error {
 	}
 
 	// let VBoxService do nice magic automounting (when it's used)
-	if err := vbm("guestproperty", "set", d.MachineName, "/VirtualBox/GuestAdd/SharedFolders/MountPrefix", "/"); err != nil {
+	if err := d.vbm("guestproperty", "set", d.MachineName, "/VirtualBox/GuestAdd/SharedFolders/MountPrefix", "/"); err != nil {
 		return err
 	}
-	if err := vbm("guestproperty", "set", d.MachineName, "/VirtualBox/GuestAdd/SharedFolders/MountDir", "/"); err != nil {
+	if err := d.vbm("guestproperty", "set", d.MachineName, "/VirtualBox/GuestAdd/SharedFolders/MountDir", "/"); err != nil {
 		return err
 	}
 
@@ -403,12 +409,12 @@ func (d *Driver) Create() error {
 			}
 
 			// woo, shareDir exists!  let's carry on!
-			if err := vbm("sharedfolder", "add", d.MachineName, "--name", shareName, "--hostpath", shareDir, "--automount"); err != nil {
+			if err := d.vbm("sharedfolder", "add", d.MachineName, "--name", shareName, "--hostpath", shareDir, "--automount"); err != nil {
 				return err
 			}
 
 			// enable symlinks
-			if err := vbm("setextradata", d.MachineName, "VBoxInternal2/SharedFoldersEnableSymlinksCreate/"+shareName, "1"); err != nil {
+			if err := d.vbm("setextradata", d.MachineName, "VBoxInternal2/SharedFoldersEnableSymlinksCreate/"+shareName, "1"); err != nil {
 				return err
 			}
 		}
@@ -425,12 +431,13 @@ func (d *Driver) hostOnlyIpAvailable() bool {
 		log.Debugf("ERROR getting IP: %s", err)
 		return false
 	}
-	if ip != "" {
+	if ip == "" {
-		log.Debugf("IP is %s", ip)
-		return true
-	}
 		log.Debug("Strangely, there was no error attempting to get the IP, but it was still empty.")
 		return false
+	}
+
+	log.Debugf("IP is %s", ip)
+	return true
 }
 
 func (d *Driver) Start() error {
@@ -448,16 +455,16 @@ func (d *Driver) Start() error {
 
 	switch s {
 	case state.Stopped, state.Saved:
-		d.SSHPort, err = setPortForwarding(d.MachineName, 1, "ssh", "tcp", 22, d.SSHPort)
+		d.SSHPort, err = setPortForwarding(d, 1, "ssh", "tcp", 22, d.SSHPort)
 		if err != nil {
 			return err
 		}
-		if err := vbm("startvm", d.MachineName, "--type", "headless"); err != nil {
+		if err := d.vbm("startvm", d.MachineName, "--type", "headless"); err != nil {
 			return err
 		}
 		log.Infof("Starting VM...")
 	case state.Paused:
-		if err := vbm("controlvm", d.MachineName, "resume", "--type", "headless"); err != nil {
+		if err := d.vbm("controlvm", d.MachineName, "resume", "--type", "headless"); err != nil {
 			return err
 		}
 		log.Infof("Resuming VM ...")
@@ -491,7 +498,7 @@ func (d *Driver) Start() error {
 }
 
 func (d *Driver) Stop() error {
-	if err := vbm("controlvm", d.MachineName, "acpipowerbutton"); err != nil {
+	if err := d.vbm("controlvm", d.MachineName, "acpipowerbutton"); err != nil {
 		return err
 	}
 	for {
@@ -527,7 +534,7 @@ func (d *Driver) Remove() error {
 	}
 	// vbox will not release it's lock immediately after the stop
 	time.Sleep(1 * time.Second)
-	return vbm("unregistervm", "--delete", d.MachineName)
+	return d.vbm("unregistervm", "--delete", d.MachineName)
 }
 
 func (d *Driver) Restart() error {
@@ -545,11 +552,11 @@ func (d *Driver) Restart() error {
 }
 
 func (d *Driver) Kill() error {
-	return vbm("controlvm", d.MachineName, "poweroff")
+	return d.vbm("controlvm", d.MachineName, "poweroff")
 }
 
 func (d *Driver) GetState() (state.State, error) {
-	stdout, stderr, err := vbmOutErr("showvminfo", d.MachineName,
+	stdout, stderr, err := d.vbmOutErr("showvminfo", d.MachineName,
 		"--machinereadable")
 	if err != nil {
 		if reMachineNotFound.FindString(stderr) != "" {
@@ -694,21 +701,16 @@ func (d *Driver) setupHostOnlyNetwork(machineName string) error {
 		lowerDHCPIP,
 		upperDHCPIP,
 	)
-
 	if err != nil {
 		return err
 	}
 
-	if err := vbm("modifyvm", machineName,
+	return d.vbm("modifyvm", machineName,
 		"--nic2", "hostonly",
 		"--nictype2", d.HostOnlyNicType,
 		"--nicpromisc2", d.HostOnlyPromiscMode,
 		"--hostonlyadapter2", hostOnlyNetwork.Name,
-		"--cableconnected2", "on"); err != nil {
+		"--cableconnected2", "on")
-		return err
-	}
-
-	return nil
 }
 
 // createDiskImage makes a disk image at dest with the given size in MB. If r is
@@ -800,7 +802,7 @@ func getAvailableTCPPort(port int) (int, error) {
 }
 
 // Setup a NAT port forwarding entry.
-func setPortForwarding(machine string, interfaceNum int, mapName, protocol string, guestPort, desiredHostPort int) (int, error) {
+func setPortForwarding(d *Driver, interfaceNum int, mapName, protocol string, guestPort, desiredHostPort int) (int, error) {
 	actualHostPort, err := getAvailableTCPPort(desiredHostPort)
 	if err != nil {
 		return -1, err
@@ -810,8 +812,8 @@ func setPortForwarding(machine string, interfaceNum int, mapName, protocol strin
 			guestPort, mapName, desiredHostPort, actualHostPort)
 	}
 	cmd := fmt.Sprintf("--natpf%d", interfaceNum)
-	vbm("modifyvm", machine, cmd, "delete", mapName)
+	d.vbm("modifyvm", d.MachineName, cmd, "delete", mapName)
-	if err := vbm("modifyvm", machine,
+	if err := d.vbm("modifyvm", d.MachineName,
 		cmd, fmt.Sprintf("%s,%s,127.0.0.1,%d,,%d", mapName, protocol, actualHostPort, guestPort)); err != nil {
 		return -1, err
 	}

drivers/virtualbox/virtualbox_test.go

@@ -1,10 +1,102 @@
 package virtualbox
 
 import (
+	"errors"
 	"net"
+	"strings"
 	"testing"
+
+	"github.com/docker/machine/libmachine/state"
+	"github.com/stretchr/testify/assert"
 )
 
+func TestDriverName(t *testing.T) {
+	driverName := newTestDriver("default").DriverName()
+
+	assert.Equal(t, "virtualbox", driverName)
+}
+
+func TestSSHHostname(t *testing.T) {
+	hostname, err := newTestDriver("default").GetSSHHostname()
+
+	assert.Equal(t, "127.0.0.1", hostname)
+	assert.NoError(t, err)
+}
+
+func TestDefaultSSHUsername(t *testing.T) {
+	username := newTestDriver("default").GetSSHUsername()
+
+	assert.Equal(t, "docker", username)
+}
+
+type VBoxManagerMock struct {
+	VBoxCmdManager
+	args   string
+	stdOut string
+	stdErr string
+	err    error
+}
+
+func (v *VBoxManagerMock) vbmOutErr(args ...string) (string, string, error) {
+	if strings.Join(args, " ") == v.args {
+		return v.stdOut, v.stdErr, v.err
+	}
+	return "", "", errors.New("Invalid args")
+}
+
+func TestState(t *testing.T) {
+	var tests = []struct {
+		stdOut string
+		state  state.State
+	}{
+		{`VMState="running"`, state.Running},
+		{`VMState="paused"`, state.Paused},
+		{`VMState="saved"`, state.Saved},
+		{`VMState="poweroff"`, state.Stopped},
+		{`VMState="aborted"`, state.Stopped},
+		{`VMState="whatever"`, state.None},
+		{`VMState=`, state.None},
+	}
+
+	for _, expected := range tests {
+		driver := newTestDriver("default")
+		driver.VBoxManager = &VBoxManagerMock{
+			args:   "showvminfo default --machinereadable",
+			stdOut: expected.stdOut,
+		}
+
+		machineState, err := driver.GetState()
+
+		assert.NoError(t, err)
+		assert.Equal(t, expected.state, machineState)
+	}
+}
+
+func TestStateErrors(t *testing.T) {
+	var tests = []struct {
+		stdErr   string
+		err      error
+		finalErr error
+	}{
+		{"Could not find a registered machine named 'unknown'", errors.New("Bug"), errors.New("machine does not exist")},
+		{"", errors.New("Unexpected error"), errors.New("Unexpected error")},
+	}
+
+	for _, expected := range tests {
+		driver := newTestDriver("default")
+		driver.VBoxManager = &VBoxManagerMock{
+			args:   "showvminfo default --machinereadable",
+			stdErr: expected.stdErr,
+			err:    expected.err,
+		}
+
+		machineState, err := driver.GetState()
+
+		assert.Equal(t, err, expected.finalErr)
+		assert.Equal(t, state.Error, machineState)
+	}
+}
+
 func TestGetRandomIPinSubnet(t *testing.T) {
 	// test IP 1.2.3.4
 	testIP := net.IPv4(byte(1), byte(2), byte(3), byte(4))
@@ -29,3 +121,37 @@ func TestGetRandomIPinSubnet(t *testing.T) {
 		t.Fatalf("expected third octet of %d; received %d", testIP[2], newIP[2])
 	}
 }
+
+func TestGetIPErrors(t *testing.T) {
+	var tests = []struct {
+		stdOut   string
+		err      error
+		finalErr error
+	}{
+		{`VMState="poweroff"`, nil, errors.New("Host is not running")},
+		{"", errors.New("Unable to get state"), errors.New("Unable to get state")},
+	}
+
+	for _, expected := range tests {
+		driver := newTestDriver("default")
+		driver.VBoxManager = &VBoxManagerMock{
+			args:   "showvminfo default --machinereadable",
+			stdOut: expected.stdOut,
+			err:    expected.err,
+		}
+
+		ip, err := driver.GetIP()
+
+		assert.Empty(t, ip)
+		assert.Equal(t, err, expected.finalErr)
+
+		url, err := driver.GetURL()
+
+		assert.Empty(t, url)
+		assert.Equal(t, err, expected.finalErr)
+	}
+}
+
+func newTestDriver(name string) *Driver {
+	return NewDriver(name, "")
+}

drivers/virtualbox/vm.go

@@ -12,6 +12,16 @@ type VirtualBoxVM struct {
 	Memory int
 }
 
+func (d *Driver) getVMInfo() (*VirtualBoxVM, error) {
+	out, err := d.vbmOut("showvminfo", d.MachineName, "--machinereadable")
+	if err != nil {
+		return nil, err
+	}
+
+	r := strings.NewReader(out)
+	return parseVMInfo(r)
+}
+
 func parseVMInfo(r io.Reader) (*VirtualBoxVM, error) {
 	s := bufio.NewScanner(r)
 	vm := &VirtualBoxVM{}
@@ -44,12 +54,3 @@ func parseVMInfo(r io.Reader) (*VirtualBoxVM, error) {
 	}
 	return vm, nil
 }
-
-func getVMInfo(name string) (*VirtualBoxVM, error) {
-	out, err := vbmOut("showvminfo", name, "--machinereadable")
-	if err != nil {
-		return nil, err
-	}
-	r := strings.NewReader(out)
-	return parseVMInfo(r)
-}