diff --git a/drivers/virtualbox/disk.go b/drivers/virtualbox/disk.go index d99f1f395c..c1a207e402 100644 --- a/drivers/virtualbox/disk.go +++ b/drivers/virtualbox/disk.go @@ -2,7 +2,6 @@ package virtualbox import ( "bufio" - "io" "strings" ) @@ -11,28 +10,31 @@ type VirtualDisk struct { Path string } -func (d *Driver) getVMDiskInfo(name string) (*VirtualDisk, error) { - out, err := d.vbmOut("showvminfo", name, "--machinereadable") +func getVMDiskInfo(name string, vbox VBoxManager) (*VirtualDisk, error) { + out, err := vbox.vbmOut("showvminfo", name, "--machinereadable") if err != nil { return nil, err } - r := strings.NewReader(out) - return parseDiskInfo(r) + return parseDiskInfo(out) } -func parseDiskInfo(r io.Reader) (*VirtualDisk, error) { - s := bufio.NewScanner(r) +func parseDiskInfo(out string) (*VirtualDisk, error) { disk := &VirtualDisk{} + + r := strings.NewReader(out) + s := bufio.NewScanner(r) for s.Scan() { line := s.Text() if line == "" { continue } + res := reEqualQuoteLine.FindStringSubmatch(line) if res == nil { continue } + key, val := res[1], res[2] switch key { case "SATA-1-0": @@ -44,5 +46,6 @@ func parseDiskInfo(r io.Reader) (*VirtualDisk, error) { if err := s.Err(); err != nil { return nil, err } + return disk, nil } diff --git a/drivers/virtualbox/disk_test.go b/drivers/virtualbox/disk_test.go index b6b80cfbe7..bd43e3d936 100644 --- a/drivers/virtualbox/disk_test.go +++ b/drivers/virtualbox/disk_test.go @@ -1,12 +1,14 @@ package virtualbox import ( - "strings" "testing" + + "github.com/docker/machine/drivers/vmwarevsphere/errors" + "github.com/stretchr/testify/assert" ) -var ( - testDiskInfoText = ` +const ( + validDiskInfoText = ` storagecontrollerbootable0="on" "SATA-0-0"="/home/ehazlett/.boot2docker/boot2docker.iso" "SATA-IsEjected"="off" @@ -18,19 +20,39 @@ nic1="nat" ) func TestVMDiskInfo(t *testing.T) { - r := strings.NewReader(testDiskInfoText) - disk, err := parseDiskInfo(r) - if err != nil { - t.Fatal(err) + vbox := &VBoxManagerMock{ + args: "showvminfo default --machinereadable", + stdOut: validDiskInfoText, } - diskPath := "/home/ehazlett/vm/test/disk.vmdk" - diskUUID := "12345-abcdefg" - if disk.Path != diskPath { - t.Fatalf("expected disk path %s", diskPath) - } + disk, err := getVMDiskInfo("default", vbox) - if disk.UUID != diskUUID { - t.Fatalf("expected disk uuid %s", diskUUID) - } + assert.Equal(t, "/home/ehazlett/vm/test/disk.vmdk", disk.Path) + assert.Equal(t, "12345-abcdefg", disk.UUID) + assert.NoError(t, err) +} + +func TestVMDiskInfoError(t *testing.T) { + vbox := &VBoxManagerMock{ + args: "showvminfo default --machinereadable", + err: errors.New("BUG"), + } + + disk, err := getVMDiskInfo("default", vbox) + + assert.Nil(t, disk) + assert.EqualError(t, err, "BUG") +} + +func TestVMDiskInfoInvalidOutput(t *testing.T) { + vbox := &VBoxManagerMock{ + args: "showvminfo default --machinereadable", + stdOut: "INVALID", + } + + disk, err := getVMDiskInfo("default", vbox) + + assert.Empty(t, disk.Path) + assert.Empty(t, disk.UUID) + assert.NoError(t, err) } diff --git a/drivers/virtualbox/network.go b/drivers/virtualbox/network.go index 143409bdca..f3efe9283b 100644 --- a/drivers/virtualbox/network.go +++ b/drivers/virtualbox/network.go @@ -31,75 +31,65 @@ type hostOnlyNetwork struct { NetworkName string // referenced in DHCP.NetworkName } -// 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 { +func (n *hostOnlyNetwork) Save(vbox VBoxManager) 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 { + if err := vbox.vbm("hostonlyif", "ipconfig", n.Name, "--ip", n.IPv4.IP.String(), "--netmask", net.IP(n.IPv4.Mask).String()); err != nil { return err } } if n.IPv6.IP != nil && n.IPv6.Mask != nil { prefixLen, _ := n.IPv6.Mask.Size() - if err := vbm("hostonlyif", "ipconfig", n.Name, "--ipv6", n.IPv6.IP.String(), "--netmasklengthv6", fmt.Sprintf("%d", prefixLen)); err != nil { + if err := vbox.vbm("hostonlyif", "ipconfig", n.Name, "--ipv6", n.IPv6.IP.String(), "--netmasklengthv6", fmt.Sprintf("%d", prefixLen)); err != nil { return err } } if n.DHCP { - vbm("hostonlyif", "ipconfig", n.Name, "--dhcp") // not implemented as of VirtualBox 4.3 + vbox.vbm("hostonlyif", "ipconfig", n.Name, "--dhcp") // not implemented as of VirtualBox 4.3 } return nil } // createHostonlyNet creates a new host-only network. -func createHostonlyNet() (*hostOnlyNetwork, error) { - out, err := vbmOut("hostonlyif", "create") +func createHostonlyNet(vbox VBoxManager) (*hostOnlyNetwork, error) { + out, err := vbox.vbmOut("hostonlyif", "create") if err != nil { return nil, err } + res := reHostonlyInterfaceCreated.FindStringSubmatch(string(out)) if res == nil { return nil, errors.New("failed to create hostonly interface") } + return &hostOnlyNetwork{Name: res[1]}, nil } // listHostOnlyNetworks gets all host-only networks in a map keyed by HostonlyNet.NetworkName. -func listHostOnlyNetworks() (map[string]*hostOnlyNetwork, error) { - out, err := vbmOut("list", "hostonlyifs") +func listHostOnlyNetworks(vbox VBoxManager) (map[string]*hostOnlyNetwork, error) { + out, err := vbox.vbmOut("list", "hostonlyifs") if err != nil { return nil, err } - s := bufio.NewScanner(strings.NewReader(out)) + m := map[string]*hostOnlyNetwork{} n := &hostOnlyNetwork{} + + s := bufio.NewScanner(strings.NewReader(out)) for s.Scan() { line := s.Text() if line == "" { - m[n.NetworkName] = n - n = &hostOnlyNetwork{} continue } + res := reColonLine.FindStringSubmatch(line) if res == nil { continue } + switch key, val := res[1], res[2]; key { case "Name": n.Name = val @@ -131,8 +121,11 @@ func listHostOnlyNetworks() (map[string]*hostOnlyNetwork, error) { n.Status = val case "VBoxNetworkName": n.NetworkName = val + m[val] = n + n = &hostOnlyNetwork{} } } + if err := s.Err(); err != nil { return nil, err } @@ -153,35 +146,37 @@ func getHostOnlyNetwork(nets map[string]*hostOnlyNetwork, hostIP net.IP, netmask return nil } -func getOrCreateHostOnlyNetwork(hostIP net.IP, netmask net.IPMask, dhcpIP net.IP, dhcpUpperIP net.IP, dhcpLowerIP net.IP) (*hostOnlyNetwork, error) { - nets, err := listHostOnlyNetworks() +func getOrCreateHostOnlyNetwork(hostIP net.IP, netmask net.IPMask, dhcpIP net.IP, dhcpLowerIP net.IP, dhcpUpperIP net.IP, vbox VBoxManager) (*hostOnlyNetwork, error) { + nets, err := listHostOnlyNetworks(vbox) if err != nil { return nil, err } hostOnlyNet := getHostOnlyNetwork(nets, hostIP, netmask) + if hostOnlyNet != nil { + return hostOnlyNet, nil + } - if hostOnlyNet == nil { - // No existing host-only interface found. Create a new one. - hostOnlyNet, err = createHostonlyNet() - if err != nil { - return nil, err - } - hostOnlyNet.IPv4.IP = hostIP - hostOnlyNet.IPv4.Mask = netmask - if err := hostOnlyNet.Save(); err != nil { - return nil, err - } + // No existing host-only interface found. Create a new one. + hostOnlyNet, err = createHostonlyNet(vbox) + if err != nil { + return nil, err + } - dhcp := dhcpServer{} - dhcp.IPv4.IP = dhcpIP - dhcp.IPv4.Mask = netmask - dhcp.LowerIP = dhcpUpperIP - dhcp.UpperIP = dhcpLowerIP - dhcp.Enabled = true - if err := addHostonlyDHCP(hostOnlyNet.Name, dhcp); err != nil { - return nil, err - } + hostOnlyNet.IPv4.IP = hostIP + hostOnlyNet.IPv4.Mask = netmask + if err := hostOnlyNet.Save(vbox); err != nil { + return nil, err + } + + dhcp := dhcpServer{} + dhcp.IPv4.IP = dhcpIP + dhcp.IPv4.Mask = netmask + dhcp.LowerIP = dhcpLowerIP + dhcp.UpperIP = dhcpUpperIP + dhcp.Enabled = true + if err := addHostonlyDHCP(hostOnlyNet.Name, dhcp, vbox); err != nil { + return nil, err } return hostOnlyNet, nil @@ -196,12 +191,12 @@ type dhcpServer struct { Enabled bool } -func addDHCPServer(kind, name string, d dhcpServer) error { +func addDHCPServer(kind, name string, d dhcpServer, vbox VBoxManager) error { command := "modify" // On some platforms (OSX), creating a hostonlyinterface adds a default dhcpserver // While on others (Windows?) it does not. - dhcps, err := getDHCPServers() + dhcps, err := getDHCPServers(vbox) if err != nil { return err } @@ -222,17 +217,18 @@ func addDHCPServer(kind, name string, d dhcpServer) error { } else { args = append(args, "--disable") } - return vbm(args...) + + return vbox.vbm(args...) } // addHostonlyDHCP adds a DHCP server to a host-only network. -func addHostonlyDHCP(ifname string, d dhcpServer) error { - return addDHCPServer("--netname", "HostInterfaceNetworking-"+ifname, d) +func addHostonlyDHCP(ifname string, d dhcpServer, vbox VBoxManager) error { + return addDHCPServer("--netname", "HostInterfaceNetworking-"+ifname, d, vbox) } // getDHCPServers gets all DHCP server settings in a map keyed by DHCP.NetworkName. -func getDHCPServers() (map[string]*dhcpServer, error) { - out, err := vbmOut("list", "dhcpservers") +func getDHCPServers(vbox VBoxManager) (map[string]*dhcpServer, error) { + out, err := vbox.vbmOut("list", "dhcpservers") if err != nil { return nil, err } diff --git a/drivers/virtualbox/network_test.go b/drivers/virtualbox/network_test.go index 4ca5ea8c09..a2b2c6aeb6 100644 --- a/drivers/virtualbox/network_test.go +++ b/drivers/virtualbox/network_test.go @@ -4,8 +4,49 @@ import ( "net" "reflect" "testing" + + "github.com/stretchr/testify/assert" ) +const stdOutOneHostOnlyNetwork = `Name: vboxnet0 +GUID: 786f6276-656e-4074-8000-0a0027000000 +DHCP: Disabled +IPAddress: 192.168.99.1 +NetworkMask: 255.255.255.0 +IPV6Address: +IPV6NetworkMaskPrefixLength: 0 +HardwareAddress: 0a:00:27:00:00:00 +MediumType: Ethernet +Status: Up +VBoxNetworkName: HostInterfaceNetworking-vboxnet0 + +` +const stdOutTwoHostOnlyNetwork = `Name: vboxnet0 +GUID: 786f6276-656e-4074-8000-0a0027000000 +DHCP: Disabled +IPAddress: 192.168.99.1 +NetworkMask: 255.255.255.0 +IPV6Address: +IPV6NetworkMaskPrefixLength: 0 +HardwareAddress: 0a:00:27:00:00:00 +MediumType: Ethernet +Status: Up +VBoxNetworkName: HostInterfaceNetworking-vboxnet0 + +Name: vboxnet1 +GUID: 786f6276-656e-4174-8000-0a0027000001 +DHCP: Disabled +IPAddress: 192.168.99.1 +NetworkMask: 255.255.255.0 +IPV6Address: +IPV6NetworkMaskPrefixLength: 0 +HardwareAddress: 0a:00:27:00:00:01 +MediumType: Ethernet +Status: Up +VBoxNetworkName: HostInterfaceNetworking-vboxnet1 + +` + // Tests that when we have a host only network which matches our expectations, // it gets returned correctly. func TestGetHostOnlyNetworkHappy(t *testing.T) { @@ -82,3 +123,91 @@ func TestGetHostOnlyNetworkWindows10Bug(t *testing.T) { t.Fatalf("Expected result of calling getHostOnlyNetwork to be the same as expected but it was not:\nexpected: %+v\nactual: %+v\n", expectedHostOnlyNetwork, n) } } + +func TestListHostOnlyNetworks(t *testing.T) { + vbox := &VBoxManagerMock{ + args: "list hostonlyifs", + stdOut: stdOutOneHostOnlyNetwork, + } + + nets, err := listHostOnlyNetworks(vbox) + + assert.Equal(t, 1, len(nets)) + assert.NoError(t, err) + + net, present := nets["HostInterfaceNetworking-vboxnet0"] + + assert.True(t, present) + assert.Equal(t, "vboxnet0", net.Name) + assert.Equal(t, "786f6276-656e-4074-8000-0a0027000000", net.GUID) + assert.False(t, net.DHCP) + assert.Equal(t, "192.168.99.1", net.IPv4.IP.String()) + assert.Equal(t, "ffffff00", net.IPv4.Mask.String()) + assert.Empty(t, net.IPv6.IP) + assert.Equal(t, "0a:00:27:00:00:00", net.HwAddr.String()) + assert.Equal(t, "Ethernet", net.Medium) + assert.Equal(t, "Up", net.Status) + assert.Equal(t, "HostInterfaceNetworking-vboxnet0", net.NetworkName) +} + +func TestListTwoHostOnlyNetworks(t *testing.T) { + vbox := &VBoxManagerMock{ + args: "list hostonlyifs", + stdOut: stdOutTwoHostOnlyNetwork, + } + + nets, err := listHostOnlyNetworks(vbox) + + assert.Equal(t, 2, len(nets)) + assert.NoError(t, err) + + net, present := nets["HostInterfaceNetworking-vboxnet1"] + + assert.True(t, present) + assert.Equal(t, "vboxnet1", net.Name) + assert.Equal(t, "786f6276-656e-4174-8000-0a0027000001", net.GUID) + assert.False(t, net.DHCP) + assert.Equal(t, "192.168.99.1", net.IPv4.IP.String()) + assert.Equal(t, "ffffff00", net.IPv4.Mask.String()) + assert.Empty(t, net.IPv6.IP) + assert.Equal(t, "0a:00:27:00:00:01", net.HwAddr.String()) + assert.Equal(t, "Ethernet", net.Medium) + assert.Equal(t, "Up", net.Status) + assert.Equal(t, "HostInterfaceNetworking-vboxnet1", net.NetworkName) +} + +func TestListHostOnlyNetworksDontRelyOnEmptyLinesForParsing(t *testing.T) { + vbox := &VBoxManagerMock{ + args: "list hostonlyifs", + stdOut: `Name: vboxnet0 +VBoxNetworkName: HostInterfaceNetworking-vboxnet0 +Name: vboxnet1 +VBoxNetworkName: HostInterfaceNetworking-vboxnet1`, + } + + nets, err := listHostOnlyNetworks(vbox) + + assert.Equal(t, 2, len(nets)) + assert.NoError(t, err) + + net, present := nets["HostInterfaceNetworking-vboxnet1"] + assert.True(t, present) + assert.Equal(t, "vboxnet1", net.Name) + + net, present = nets["HostInterfaceNetworking-vboxnet0"] + assert.True(t, present) + assert.Equal(t, "vboxnet0", net.Name) +} + +func TestGetHostOnlyNetwork(t *testing.T) { + vbox := &VBoxManagerMock{ + args: "list hostonlyifs", + stdOut: stdOutOneHostOnlyNetwork, + } + + net, err := getOrCreateHostOnlyNetwork(net.ParseIP("192.168.99.1"), parseIPv4Mask("255.255.255.0"), nil, nil, nil, vbox) + + assert.NotNil(t, net) + assert.Equal(t, "HostInterfaceNetworking-vboxnet0", net.NetworkName) + assert.NoError(t, err) +} diff --git a/drivers/virtualbox/virtualbox.go b/drivers/virtualbox/virtualbox.go index 1b481b87dc..d6d67154dc 100644 --- a/drivers/virtualbox/virtualbox.go +++ b/drivers/virtualbox/virtualbox.go @@ -239,7 +239,7 @@ func (d *Driver) Create() error { // make sure vm is stopped _ = d.vbm("controlvm", name, "poweroff") - diskInfo, err := d.getVMDiskInfo(name) + diskInfo, err := getVMDiskInfo(name, d.VBoxManager) if err != nil { return err } @@ -253,7 +253,7 @@ func (d *Driver) Create() error { } log.Debugf("Importing VM settings...") - vmInfo, err := d.getVMInfo(name) + vmInfo, err := getVMInfo(name, d.VBoxManager) if err != nil { return err } @@ -698,6 +698,7 @@ func (d *Driver) setupHostOnlyNetwork(machineName string) error { dhcpAddr, lowerDHCPIP, upperDHCPIP, + d.VBoxManager, ) if err != nil { return err diff --git a/drivers/virtualbox/virtualbox_test.go b/drivers/virtualbox/virtualbox_test.go index d580b7b49d..d9eee6d100 100644 --- a/drivers/virtualbox/virtualbox_test.go +++ b/drivers/virtualbox/virtualbox_test.go @@ -31,13 +31,22 @@ func TestDefaultSSHUsername(t *testing.T) { } type VBoxManagerMock struct { - VBoxCmdManager args string stdOut string stdErr string err error } +func (v *VBoxManagerMock) vbm(args ...string) error { + _, _, err := v.vbmOutErr(args...) + return err +} + +func (v *VBoxManagerMock) vbmOut(args ...string) (string, error) { + stdout, _, err := v.vbmOutErr(args...) + return stdout, err +} + func (v *VBoxManagerMock) vbmOutErr(args ...string) (string, string, error) { if strings.Join(args, " ") == v.args { return v.stdOut, v.stdErr, v.err diff --git a/drivers/virtualbox/vm.go b/drivers/virtualbox/vm.go index 01472cff5e..e72857c5bf 100644 --- a/drivers/virtualbox/vm.go +++ b/drivers/virtualbox/vm.go @@ -12,8 +12,8 @@ type VM struct { Memory int } -func (d *Driver) getVMInfo(name string) (*VM, error) { - out, err := d.vbmOut("showvminfo", name, "--machinereadable") +func getVMInfo(name string, vbox VBoxManager) (*VM, error) { + out, err := vbox.vbmOut("showvminfo", name, "--machinereadable") if err != nil { return nil, err }