diff --git a/drivers/virtualbox/network.go b/drivers/virtualbox/network.go index f1dfca89ec..939e9ec8e8 100644 --- a/drivers/virtualbox/network.go +++ b/drivers/virtualbox/network.go @@ -10,6 +10,10 @@ import ( "strings" ) +const ( + buggyNetmask = "0f000000" +) + var ( reHostonlyInterfaceCreated = regexp.MustCompile(`Interface '(.+)' was successfully created`) ) @@ -121,28 +125,30 @@ func listHostOnlyNetworks() (map[string]*hostOnlyNetwork, error) { return m, nil } -func getHostOnlyNetwork(hostIP net.IP, netmask net.IPMask) (*hostOnlyNetwork, error) { - nets, err := listHostOnlyNetworks() - if err != nil { - return nil, err - } +func getHostOnlyNetwork(nets map[string]*hostOnlyNetwork, hostIP net.IP, netmask net.IPMask) (*hostOnlyNetwork, error) { for _, n := range nets { - //handle known issue where vbox throws us a bad netmask - //if the value returned is "0f000000" it's the newly - //created adpater and should be the right one + // 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() == "0f000000") { + (netmask.String() == n.IPv4.Mask.String() || n.IPv4.Mask.String() == buggyNetmask) { return n, nil } } - return nil, nil + return nil, errors.New("Valid host only network not found") } func getOrCreateHostOnlyNetwork(hostIP net.IP, netmask net.IPMask, dhcpIP net.IP, dhcpUpperIP net.IP, dhcpLowerIP net.IP) (*hostOnlyNetwork, error) { - hostOnlyNet, err := getHostOnlyNetwork(hostIP, netmask) - if err != nil || hostOnlyNet != nil { + nets, err := listHostOnlyNetworks() + if err != nil { + return nil, err + } + + hostOnlyNet, err := getHostOnlyNetwork(nets, hostIP, netmask) + if err != nil { return hostOnlyNet, err } + // No existing host-only interface found. Create a new one. hostOnlyNet, err = createHostonlyNet() if err != nil { diff --git a/drivers/virtualbox/virtualbox_test.go b/drivers/virtualbox/virtualbox_test.go index 5df5e5f4cb..c640799d0b 100644 --- a/drivers/virtualbox/virtualbox_test.go +++ b/drivers/virtualbox/virtualbox_test.go @@ -2,6 +2,7 @@ package virtualbox import ( "net" + "reflect" "testing" ) @@ -29,3 +30,91 @@ 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) + } +}