From 53c5198a5a561babd5af50df73d70c3bb23e35c0 Mon Sep 17 00:00:00 2001 From: David Gageot Date: Thu, 19 Nov 2015 19:39:02 +0100 Subject: [PATCH 1/4] 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 } From e812df723c081ec9d8e361376e6daecce5c206d7 Mon Sep 17 00:00:00 2001 From: David Gageot Date: Fri, 20 Nov 2015 13:38:35 +0100 Subject: [PATCH 2/4] Add somes tests to virtualbox driver Signed-off-by: David Gageot --- drivers/virtualbox/disk_test.go | 6 +- drivers/virtualbox/network.go | 6 +- drivers/virtualbox/network_test.go | 90 ++++++++++++++++++++++++++++++ 3 files changed, 98 insertions(+), 4 deletions(-) diff --git a/drivers/virtualbox/disk_test.go b/drivers/virtualbox/disk_test.go index 169377b636..bd43e3d936 100644 --- a/drivers/virtualbox/disk_test.go +++ b/drivers/virtualbox/disk_test.go @@ -7,8 +7,8 @@ import ( "github.com/stretchr/testify/assert" ) -var ( - testDiskInfoText = ` +const ( + validDiskInfoText = ` storagecontrollerbootable0="on" "SATA-0-0"="/home/ehazlett/.boot2docker/boot2docker.iso" "SATA-IsEjected"="off" @@ -22,7 +22,7 @@ nic1="nat" func TestVMDiskInfo(t *testing.T) { vbox := &VBoxManagerMock{ args: "showvminfo default --machinereadable", - stdOut: testDiskInfoText, + stdOut: validDiskInfoText, } disk, err := getVMDiskInfo("default", vbox) diff --git a/drivers/virtualbox/network.go b/drivers/virtualbox/network.go index 0e6658f134..7aec22aba9 100644 --- a/drivers/virtualbox/network.go +++ b/drivers/virtualbox/network.go @@ -75,9 +75,10 @@ func listHostOnlyNetworks(vbox VBoxManager) (map[string]*hostOnlyNetwork, error) 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 == "" { @@ -85,10 +86,12 @@ func listHostOnlyNetworks(vbox VBoxManager) (map[string]*hostOnlyNetwork, error) n = &hostOnlyNetwork{} continue } + res := reColonLine.FindStringSubmatch(line) if res == nil { continue } + switch key, val := res[1], res[2]; key { case "Name": n.Name = val @@ -122,6 +125,7 @@ func listHostOnlyNetworks(vbox VBoxManager) (map[string]*hostOnlyNetwork, error) n.NetworkName = val } } + if err := s.Err(); err != nil { return nil, err } diff --git a/drivers/virtualbox/network_test.go b/drivers/virtualbox/network_test.go index 4ca5ea8c09..18312195ce 100644 --- a/drivers/virtualbox/network_test.go +++ b/drivers/virtualbox/network_test.go @@ -4,6 +4,8 @@ import ( "net" "reflect" "testing" + + "github.com/stretchr/testify/assert" ) // Tests that when we have a host only network which matches our expectations, @@ -82,3 +84,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: `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 + +`, + } + + 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: `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 + +`, + } + + 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) +} From bbe59b8020fb6d0ce5d0d3830dd451565c3ee6c4 Mon Sep 17 00:00:00 2001 From: David Gageot Date: Fri, 20 Nov 2015 13:41:18 +0100 Subject: [PATCH 3/4] Don't rely on empty lines It could lead to creating multiple host only interfaces Signed-off-by: David Gageot --- drivers/virtualbox/network.go | 4 ++-- drivers/virtualbox/network_test.go | 23 +++++++++++++++++++++++ 2 files changed, 25 insertions(+), 2 deletions(-) diff --git a/drivers/virtualbox/network.go b/drivers/virtualbox/network.go index 7aec22aba9..5b9dd44e64 100644 --- a/drivers/virtualbox/network.go +++ b/drivers/virtualbox/network.go @@ -82,8 +82,6 @@ func listHostOnlyNetworks(vbox VBoxManager) (map[string]*hostOnlyNetwork, error) for s.Scan() { line := s.Text() if line == "" { - m[n.NetworkName] = n - n = &hostOnlyNetwork{} continue } @@ -123,6 +121,8 @@ func listHostOnlyNetworks(vbox VBoxManager) (map[string]*hostOnlyNetwork, error) n.Status = val case "VBoxNetworkName": n.NetworkName = val + m[val] = n + n = &hostOnlyNetwork{} } } diff --git a/drivers/virtualbox/network_test.go b/drivers/virtualbox/network_test.go index 18312195ce..f805f74b29 100644 --- a/drivers/virtualbox/network_test.go +++ b/drivers/virtualbox/network_test.go @@ -172,3 +172,26 @@ VBoxNetworkName: HostInterfaceNetworking-vboxnet1 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) +} From 308f9d025c04e80a9d3955950faef98613967fd7 Mon Sep 17 00:00:00 2001 From: David Gageot Date: Fri, 20 Nov 2015 14:11:44 +0100 Subject: [PATCH 4/4] Add more tests to virtualbox driver Signed-off-by: David Gageot --- drivers/virtualbox/network.go | 44 +++++++------- drivers/virtualbox/network_test.go | 96 +++++++++++++++++------------- 2 files changed, 79 insertions(+), 61 deletions(-) diff --git a/drivers/virtualbox/network.go b/drivers/virtualbox/network.go index 5b9dd44e64..f3efe9283b 100644 --- a/drivers/virtualbox/network.go +++ b/drivers/virtualbox/network.go @@ -146,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, vbox VBoxManager) (*hostOnlyNetwork, error) { +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(vbox) - if err != nil { - return nil, err - } - hostOnlyNet.IPv4.IP = hostIP - hostOnlyNet.IPv4.Mask = netmask - if err := hostOnlyNet.Save(vbox); 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, vbox); 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 diff --git a/drivers/virtualbox/network_test.go b/drivers/virtualbox/network_test.go index f805f74b29..a2b2c6aeb6 100644 --- a/drivers/virtualbox/network_test.go +++ b/drivers/virtualbox/network_test.go @@ -8,6 +8,45 @@ import ( "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) { @@ -87,20 +126,8 @@ func TestGetHostOnlyNetworkWindows10Bug(t *testing.T) { func TestListHostOnlyNetworks(t *testing.T) { vbox := &VBoxManagerMock{ - args: "list hostonlyifs", - stdOut: `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 - -`, + args: "list hostonlyifs", + stdOut: stdOutOneHostOnlyNetwork, } nets, err := listHostOnlyNetworks(vbox) @@ -125,32 +152,8 @@ VBoxNetworkName: HostInterfaceNetworking-vboxnet0 func TestListTwoHostOnlyNetworks(t *testing.T) { vbox := &VBoxManagerMock{ - args: "list hostonlyifs", - stdOut: `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 - -`, + args: "list hostonlyifs", + stdOut: stdOutTwoHostOnlyNetwork, } nets, err := listHostOnlyNetworks(vbox) @@ -195,3 +198,16 @@ VBoxNetworkName: HostInterfaceNetworking-vboxnet1`, 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) +}