From 3b85723982b111ec0b3a7160b25af85dc4de6851 Mon Sep 17 00:00:00 2001 From: Nathan LeClaire Date: Tue, 22 Sep 2015 19:58:19 -0700 Subject: [PATCH] Fix failing case creating host only interface Signed-off-by: Nathan LeClaire --- drivers/virtualbox/network.go | 50 +++++++-------- drivers/virtualbox/network_test.go | 83 +++++++++++++++++++++++++ drivers/virtualbox/virtualbox_test.go | 89 --------------------------- 3 files changed, 108 insertions(+), 114 deletions(-) diff --git a/drivers/virtualbox/network.go b/drivers/virtualbox/network.go index 939e9ec8e8..4524e118ff 100644 --- a/drivers/virtualbox/network.go +++ b/drivers/virtualbox/network.go @@ -125,17 +125,18 @@ func listHostOnlyNetworks() (map[string]*hostOnlyNetwork, error) { return m, nil } -func getHostOnlyNetwork(nets map[string]*hostOnlyNetwork, hostIP net.IP, netmask net.IPMask) (*hostOnlyNetwork, error) { +func getHostOnlyNetwork(nets map[string]*hostOnlyNetwork, hostIP net.IP, netmask net.IPMask) *hostOnlyNetwork { for _, n := range nets { // Second part of this conditional handles a race where // VirtualBox returns us the incorrect netmask value for the // newly created interface. if hostIP.Equal(n.IPv4.IP) && (netmask.String() == n.IPv4.Mask.String() || n.IPv4.Mask.String() == buggyNetmask) { - return n, nil + return n } } - return nil, errors.New("Valid host only network not found") + + return nil } func getOrCreateHostOnlyNetwork(hostIP net.IP, netmask net.IPMask, dhcpIP net.IP, dhcpUpperIP net.IP, dhcpLowerIP net.IP) (*hostOnlyNetwork, error) { @@ -144,30 +145,29 @@ func getOrCreateHostOnlyNetwork(hostIP net.IP, netmask net.IPMask, dhcpIP net.IP return nil, err } - hostOnlyNet, err := getHostOnlyNetwork(nets, hostIP, netmask) - if err != nil { - return hostOnlyNet, err - } + hostOnlyNet := getHostOnlyNetwork(nets, hostIP, netmask) - // 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 - } + 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 + } - 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 + 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 + } } return hostOnlyNet, nil diff --git a/drivers/virtualbox/network_test.go b/drivers/virtualbox/network_test.go index 50e40b533b..4ca5ea8c09 100644 --- a/drivers/virtualbox/network_test.go +++ b/drivers/virtualbox/network_test.go @@ -1 +1,84 @@ package virtualbox + +import ( + "net" + "reflect" + "testing" +) + +// Tests that when we have a host only network which matches our expectations, +// it gets returned correctly. +func TestGetHostOnlyNetworkHappy(t *testing.T) { + cidr := "192.168.99.0/24" + ip, ipnet, err := net.ParseCIDR(cidr) + if err != nil { + t.Fatalf("Error parsing cidr: %s", err) + } + expectedHostOnlyNetwork := &hostOnlyNetwork{ + IPv4: *ipnet, + } + vboxNets := map[string]*hostOnlyNetwork{ + "HostInterfaceNetworking-vboxnet0": expectedHostOnlyNetwork, + } + + n := getHostOnlyNetwork(vboxNets, ip, ipnet.Mask) + if !reflect.DeepEqual(n, expectedHostOnlyNetwork) { + t.Fatalf("Expected result of calling getHostOnlyNetwork to be the same as expected but it was not:\nexpected: %+v\nactual: %+v\n", expectedHostOnlyNetwork, n) + } +} + +// Tests that we are able to properly detect when a host only network which +// matches our expectations can not be found. +func TestGetHostOnlyNetworkNotFound(t *testing.T) { + // Note that this has a different ip is different from "ip" below. + cidr := "192.168.99.0/24" + ip, ipnet, err := net.ParseCIDR(cidr) + if err != nil { + t.Fatalf("Error parsing cidr: %s", err) + } + + ip = net.ParseIP("192.168.59.0").To4() + + // Suppose a vbox net is created, but it doesn't align with our + // expectation. + vboxNet := &hostOnlyNetwork{ + IPv4: *ipnet, + } + vboxNets := map[string]*hostOnlyNetwork{ + "HostInterfaceNetworking-vboxnet0": vboxNet, + } + + n := getHostOnlyNetwork(vboxNets, ip, ipnet.Mask) + if n != nil { + t.Fatalf("Expected vbox net to be nil but it has a value: %+v\n", n) + } +} + +// Tests a special case where Virtualbox creates the host only network +// successfully but mis-reports the netmask. +func TestGetHostOnlyNetworkWindows10Bug(t *testing.T) { + cidr := "192.168.99.0/24" + ip, ipnet, err := net.ParseCIDR(cidr) + if err != nil { + t.Fatalf("Error parsing cidr: %s", err) + } + + // This is a faulty netmask: a VirtualBox bug causes it to be + // misreported. + ipnet.Mask = net.IPMask(net.ParseIP("15.0.0.0").To4()) + + expectedHostOnlyNetwork := &hostOnlyNetwork{ + IPv4: *ipnet, + } + + vboxNets := map[string]*hostOnlyNetwork{ + "HostInterfaceNetworking-vboxnet0": expectedHostOnlyNetwork, + } + + // The Mask that we are passing in will be the "legitimate" mask, so it + // must differ from the magic buggy mask. + n := getHostOnlyNetwork(vboxNets, ip, net.IPMask(net.ParseIP("255.255.255.0").To4())) + if !reflect.DeepEqual(n, expectedHostOnlyNetwork) { + t.Fatalf("Expected result of calling getHostOnlyNetwork to be the same as expected but it was not:\nexpected: %+v\nactual: %+v\n", expectedHostOnlyNetwork, n) + } +} diff --git a/drivers/virtualbox/virtualbox_test.go b/drivers/virtualbox/virtualbox_test.go index c640799d0b..5df5e5f4cb 100644 --- a/drivers/virtualbox/virtualbox_test.go +++ b/drivers/virtualbox/virtualbox_test.go @@ -2,7 +2,6 @@ package virtualbox import ( "net" - "reflect" "testing" ) @@ -30,91 +29,3 @@ func TestGetRandomIPinSubnet(t *testing.T) { t.Fatalf("expected third octet of %d; received %d", testIP[2], newIP[2]) } } - -// Tests that when we have a host only network which matches our expectations, -// it gets returned correctly. -func TestGetHostOnlyNetworkHappy(t *testing.T) { - cidr := "192.168.99.0/24" - ip, ipnet, err := net.ParseCIDR(cidr) - if err != nil { - t.Fatalf("Error parsing cidr: %s", err) - } - expectedHostOnlyNetwork := &hostOnlyNetwork{ - IPv4: *ipnet, - } - vboxNets := map[string]*hostOnlyNetwork{ - "HostInterfaceNetworking-vboxnet0": expectedHostOnlyNetwork, - } - - n, err := getHostOnlyNetwork(vboxNets, ip, ipnet.Mask) - if err != nil { - t.Fatalf("Was not expecting an error calling getHostOnlyNetwork: %s", err) - } - - if !reflect.DeepEqual(n, expectedHostOnlyNetwork) { - t.Fatalf("Expected result of calling getHostOnlyNetwork to be the same as expected but it was not:\nexpected: %+v\nactual: %+v\n", expectedHostOnlyNetwork, n) - } -} - -// Tests that we are able to properly detect when a host only network which -// matches our expectations can not be found. -func TestGetHostOnlyNetworkNotFound(t *testing.T) { - // Note that this has a different ip is different from "ip" below. - cidr := "192.168.99.0/24" - ip, ipnet, err := net.ParseCIDR(cidr) - if err != nil { - t.Fatalf("Error parsing cidr: %s", err) - } - - ip = net.ParseIP("192.168.59.0").To4() - - // Suppose a vbox net is created, but it doesn't align with our - // expectation. - vboxNet := &hostOnlyNetwork{ - IPv4: *ipnet, - } - vboxNets := map[string]*hostOnlyNetwork{ - "HostInterfaceNetworking-vboxnet0": vboxNet, - } - - n, err := getHostOnlyNetwork(vboxNets, ip, ipnet.Mask) - if err == nil { - t.Fatalf("Expected to get an error but instead got no error") - } - if n != nil { - t.Fatalf("Expected vbox net to be nil but it has a value: %+v\n", n) - } -} - -// Tests a special case where Virtualbox creates the host only network -// successfully but mis-reports the netmask. -func TestGetHostOnlyNetworkWindows10Bug(t *testing.T) { - cidr := "192.168.99.0/24" - ip, ipnet, err := net.ParseCIDR(cidr) - if err != nil { - t.Fatalf("Error parsing cidr: %s", err) - } - - // This is a faulty netmask: a VirtualBox bug causes it to be - // misreported. - ipnet.Mask = net.IPMask(net.ParseIP("15.0.0.0").To4()) - - expectedHostOnlyNetwork := &hostOnlyNetwork{ - IPv4: *ipnet, - } - - vboxNets := map[string]*hostOnlyNetwork{ - "HostInterfaceNetworking-vboxnet0": expectedHostOnlyNetwork, - } - - // The Mask that we are passing in will be the "legitimate" mask, so it - // must differ from the magic buggy mask. - n, err := getHostOnlyNetwork(vboxNets, ip, net.IPMask(net.ParseIP("255.255.255.0").To4())) - if err != nil { - t.Fatalf("Was not expecting an error calling getHostOnlyNetwork: %s", err) - } - - if !reflect.DeepEqual(n, expectedHostOnlyNetwork) { - t.Fatalf("Expected result of calling getHostOnlyNetwork to be the same as expected but it was not:\nexpected: %+v\nactual: %+v\n", expectedHostOnlyNetwork, n) - } -}