From 53c5198a5a561babd5af50df73d70c3bb23e35c0 Mon Sep 17 00:00:00 2001 From: David Gageot Date: Thu, 19 Nov 2015 19:39:02 +0100 Subject: [PATCH] Use the passed VBoxManager This makes the code easier to test. Signed-off-by: David Gageot --- drivers/virtualbox/disk.go | 17 ++++---- drivers/virtualbox/disk_test.go | 48 ++++++++++++++++------ drivers/virtualbox/network.go | 58 +++++++++++---------------- drivers/virtualbox/virtualbox.go | 5 ++- drivers/virtualbox/virtualbox_test.go | 11 ++++- drivers/virtualbox/vm.go | 4 +- 6 files changed, 84 insertions(+), 59 deletions(-) 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..169377b636 100644 --- a/drivers/virtualbox/disk_test.go +++ b/drivers/virtualbox/disk_test.go @@ -1,8 +1,10 @@ package virtualbox import ( - "strings" "testing" + + "github.com/docker/machine/drivers/vmwarevsphere/errors" + "github.com/stretchr/testify/assert" ) var ( @@ -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: testDiskInfoText, } - 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..0e6658f134 100644 --- a/drivers/virtualbox/network.go +++ b/drivers/virtualbox/network.go @@ -31,61 +31,50 @@ 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{} @@ -153,8 +142,8 @@ 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, dhcpUpperIP net.IP, dhcpLowerIP net.IP, vbox VBoxManager) (*hostOnlyNetwork, error) { + nets, err := listHostOnlyNetworks(vbox) if err != nil { return nil, err } @@ -163,13 +152,13 @@ func getOrCreateHostOnlyNetwork(hostIP net.IP, netmask net.IPMask, dhcpIP net.IP if hostOnlyNet == nil { // No existing host-only interface found. Create a new one. - hostOnlyNet, err = createHostonlyNet() + hostOnlyNet, err = createHostonlyNet(vbox) if err != nil { return nil, err } hostOnlyNet.IPv4.IP = hostIP hostOnlyNet.IPv4.Mask = netmask - if err := hostOnlyNet.Save(); err != nil { + if err := hostOnlyNet.Save(vbox); err != nil { return nil, err } @@ -179,7 +168,7 @@ func getOrCreateHostOnlyNetwork(hostIP net.IP, netmask net.IPMask, dhcpIP net.IP dhcp.LowerIP = dhcpUpperIP dhcp.UpperIP = dhcpLowerIP dhcp.Enabled = true - if err := addHostonlyDHCP(hostOnlyNet.Name, dhcp); err != nil { + if err := addHostonlyDHCP(hostOnlyNet.Name, dhcp, vbox); err != nil { return nil, err } } @@ -196,12 +185,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 +211,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/virtualbox.go b/drivers/virtualbox/virtualbox.go index a47c457098..8701774a3a 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 } @@ -681,6 +681,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 }