diff --git a/plugins/providers/virtualbox/action/network.rb b/plugins/providers/virtualbox/action/network.rb index ee8966ba6..22eb08cb1 100644 --- a/plugins/providers/virtualbox/action/network.rb +++ b/plugins/providers/virtualbox/action/network.rb @@ -285,16 +285,16 @@ module VagrantPlugins # with the final octet + 2. So "172.28.0.0" turns into "172.28.0.2" dhcp_ip = ip_parts.dup dhcp_ip[3] += 2 - dhcp_options[:dhcp_ip] ||= dhcp_ip.join(".") + dhcp_options[:dhcp_ip] = options[:dhcp_ip] || dhcp_ip.join(".") # Calculate the lower and upper bound for the DHCP server dhcp_lower = ip_parts.dup dhcp_lower[3] += 3 - dhcp_options[:dhcp_lower] ||= dhcp_lower.join(".") + dhcp_options[:dhcp_lower] = options[:dhcp_lower] || dhcp_lower.join(".") dhcp_upper = ip_parts.dup dhcp_upper[3] = 254 - dhcp_options[:dhcp_upper] ||= dhcp_upper.join(".") + dhcp_options[:dhcp_upper] = options[:dhcp_upper] || dhcp_upper.join(".") end return { @@ -327,22 +327,7 @@ module VagrantPlugins end if config[:type] == :dhcp - # Check that if there is a DHCP server attached on our interface, - # then it is identical. Otherwise, we can't set it. - existing_dhcp_server = find_matching_dhcp_server(interface) - if existing_dhcp_server - valid = existing_dhcp_server[:ip] == config[:dhcp_ip] && - existing_dhcp_server[:lower] == config[:dhcp_lower] && - existing_dhcp_server[:upper] == config[:dhcp_upper] - - raise Vagrant::Errors::NetworkDHCPAlreadyAttached if !valid - - @logger.debug("DHCP server already properly configured") - else - # Configure the DHCP server for the network. - @logger.debug("Creating a DHCP server...") - @env[:machine].provider.driver.create_dhcp_server(interface[:name], config) - end + create_dhcp_server_if_necessary(interface, config) end return { @@ -473,14 +458,68 @@ module VagrantPlugins nil end - def find_matching_dhcp_server(interface) - dhcp_servers.detect do |dhcp_server| - interface[:name] && interface[:name] == dhcp_server[:network] + #----------------------------------------------------------------- + # DHCP Server Helper Functions + #----------------------------------------------------------------- + + DEFAULT_DHCP_SERVER_FROM_VBOX_INSTALL = { + network_name: 'HostInterfaceNetworking-vboxnet0', + network: 'vboxnet0', + ip: '192.168.56.100', + netmask: '255.255.255.0', + lower: '192.168.56.101', + upper: '192.168.56.254' + }.freeze + + # + # When a host-only network of type: :dhcp is configured, + # this handles the potential creation of a vbox dhcpserver to manage + # it. + # + # @param [Hash] interface hash as returned from read_host_only_interfaces + # @param [Hash] config hash as returned from hostonly_config + def create_dhcp_server_if_necessary(interface, config) + existing_dhcp_server = find_matching_dhcp_server(interface) + + if existing_dhcp_server + if dhcp_server_matches_config?(existing_dhcp_server, config) + @logger.debug("DHCP server already properly configured") + return + elsif existing_dhcp_server == DEFAULT_DHCP_SERVER_FROM_VBOX_INSTALL + @env[:ui].info I18n.t("vagrant.actions.vm.network.cleanup_vbox_default_dhcp") + @env[:machine].provider.driver.remove_dhcp_server(existing_dhcp_server[:network_name]) + else + # We have an invalid DHCP server that we're not able to + # automatically clean up, so we need to give up and tell the user + # to sort out their own vbox dhcpservers and hostonlyifs + raise Vagrant::Errors::NetworkDHCPAlreadyAttached + end end + + @logger.debug("Creating a DHCP server...") + @env[:machine].provider.driver.create_dhcp_server(interface[:name], config) end - def dhcp_servers - @dhcp_servers ||= @env[:machine].provider.driver.read_dhcp_servers + # Detect when an existing DHCP server matches precisely the + # requested config for a hostonly interface. + # + # @param [Hash] dhcp_server as found by read_dhcp_servers + # @param [Hash] config as returned from hostonly_config + # @return [Boolean] + def dhcp_server_matches_config?(dhcp_server, config) + dhcp_server[:ip] == config[:dhcp_ip] && + dhcp_server[:lower] == config[:dhcp_lower] && + dhcp_server[:upper] == config[:dhcp_upper] + end + + # Returns the existing dhcp server, if any, that is attached to the + # specified interface. + # + # @return [Hash] dhcp_server or nil if not found + def find_matching_dhcp_server(interface) + @env[:machine].provider.driver.read_dhcp_servers.detect do |dhcp_server| + interface[:name] && interface[:name] == dhcp_server[:network] + end end end end diff --git a/plugins/providers/virtualbox/driver/base.rb b/plugins/providers/virtualbox/driver/base.rb index 262a39df2..3002418f7 100644 --- a/plugins/providers/virtualbox/driver/base.rb +++ b/plugins/providers/virtualbox/driver/base.rb @@ -263,6 +263,13 @@ module VagrantPlugins def read_vms end + # Removes the DHCP server identified by the provided network name. + # + # @param [String] network_name The the full network name associated + # with the DHCP server to be removed, e.g. "HostInterfaceNetworking-vboxnet0" + def remove_dhcp_server(network_name) + end + # Sets the MAC address of the first network adapter. # # @param [String] mac MAC address without any spaces/hyphens. diff --git a/plugins/providers/virtualbox/driver/meta.rb b/plugins/providers/virtualbox/driver/meta.rb index e830b724d..3a3cfd396 100644 --- a/plugins/providers/virtualbox/driver/meta.rb +++ b/plugins/providers/virtualbox/driver/meta.rb @@ -108,6 +108,7 @@ module VagrantPlugins :read_state, :read_used_ports, :read_vms, + :remove_dhcp_server, :resume, :set_mac_address, :set_name, diff --git a/plugins/providers/virtualbox/driver/version_4_0.rb b/plugins/providers/virtualbox/driver/version_4_0.rb index 92424ee25..04070b559 100644 --- a/plugins/providers/virtualbox/driver/version_4_0.rb +++ b/plugins/providers/virtualbox/driver/version_4_0.rb @@ -261,7 +261,8 @@ module VagrantPlugins block.split("\n").each do |line| if network = line[/^NetworkName:\s+HostInterfaceNetworking-(.+?)$/, 1] - info[:network] = network + info[:network] = network + info[:network_name] = "HostInterfaceNetworking-#{network}" elsif ip = line[/^IP:\s+(.+?)$/, 1] info[:ip] = ip elsif lower = line[/^lowerIPAddress:\s+(.+?)$/, 1] @@ -426,6 +427,10 @@ module VagrantPlugins results end + def remove_dhcp_server(network_name) + execute("dhcpserver", "remove", "--netname", network_name) + end + def set_mac_address(mac) execute("modifyvm", @uuid, "--macaddress1", mac) end diff --git a/plugins/providers/virtualbox/driver/version_4_1.rb b/plugins/providers/virtualbox/driver/version_4_1.rb index 295473526..b3fd44978 100644 --- a/plugins/providers/virtualbox/driver/version_4_1.rb +++ b/plugins/providers/virtualbox/driver/version_4_1.rb @@ -266,7 +266,8 @@ module VagrantPlugins block.split("\n").each do |line| if network = line[/^NetworkName:\s+HostInterfaceNetworking-(.+?)$/, 1] - info[:network] = network + info[:network] = network + info[:network_name] = "HostInterfaceNetworking-#{network}" elsif ip = line[/^IP:\s+(.+?)$/, 1] info[:ip] = ip elsif lower = line[/^lowerIPAddress:\s+(.+?)$/, 1] @@ -431,6 +432,10 @@ module VagrantPlugins results end + def remove_dhcp_server(network_name) + execute("dhcpserver", "remove", "--netname", network_name) + end + def set_mac_address(mac) execute("modifyvm", @uuid, "--macaddress1", mac) end diff --git a/plugins/providers/virtualbox/driver/version_4_2.rb b/plugins/providers/virtualbox/driver/version_4_2.rb index 9107cc8ac..765855e77 100644 --- a/plugins/providers/virtualbox/driver/version_4_2.rb +++ b/plugins/providers/virtualbox/driver/version_4_2.rb @@ -288,14 +288,15 @@ module VagrantPlugins info = {} block.split("\n").each do |line| - if line =~ /^NetworkName:\s+HostInterfaceNetworking-(.+?)$/ - info[:network] = $1.to_s - elsif line =~ /^IP:\s+(.+?)$/ - info[:ip] = $1.to_s - elsif line =~ /^lowerIPAddress:\s+(.+?)$/ - info[:lower] = $1.to_s - elsif line =~ /^upperIPAddress:\s+(.+?)$/ - info[:upper] = $1.to_s + if network = line[/^NetworkName:\s+HostInterfaceNetworking-(.+?)$/, 1] + info[:network] = network + info[:network_name] = "HostInterfaceNetworking-#{network}" + elsif ip = line[/^IP:\s+(.+?)$/, 1] + info[:ip] = ip + elsif lower = line[/^lowerIPAddress:\s+(.+?)$/, 1] + info[:lower] = lower + elsif upper = line[/^upperIPAddress:\s+(.+?)$/, 1] + info[:upper] = upper end end @@ -462,6 +463,10 @@ module VagrantPlugins results end + def remove_dhcp_server(network_name) + execute("dhcpserver", "remove", "--netname", network_name) + end + def set_mac_address(mac) execute("modifyvm", @uuid, "--macaddress1", mac) end diff --git a/plugins/providers/virtualbox/driver/version_4_3.rb b/plugins/providers/virtualbox/driver/version_4_3.rb index 288bffef8..ad3cc4cd9 100644 --- a/plugins/providers/virtualbox/driver/version_4_3.rb +++ b/plugins/providers/virtualbox/driver/version_4_3.rb @@ -297,14 +297,15 @@ module VagrantPlugins info = {} block.split("\n").each do |line| - if line =~ /^NetworkName:\s+HostInterfaceNetworking-(.+?)$/ - info[:network] = $1.to_s - elsif line =~ /^IP:\s+(.+?)$/ - info[:ip] = $1.to_s - elsif line =~ /^lowerIPAddress:\s+(.+?)$/ - info[:lower] = $1.to_s - elsif line =~ /^upperIPAddress:\s+(.+?)$/ - info[:upper] = $1.to_s + if network = line[/^NetworkName:\s+HostInterfaceNetworking-(.+?)$/, 1] + info[:network] = network + info[:network_name] = "HostInterfaceNetworking-#{network}" + elsif ip = line[/^IP:\s+(.+?)$/, 1] + info[:ip] = ip + elsif lower = line[/^lowerIPAddress:\s+(.+?)$/, 1] + info[:lower] = lower + elsif upper = line[/^upperIPAddress:\s+(.+?)$/, 1] + info[:upper] = upper end end @@ -472,6 +473,10 @@ module VagrantPlugins results end + def remove_dhcp_server(network_name) + execute("dhcpserver", "remove", "--netname", network_name) + end + def set_mac_address(mac) execute("modifyvm", @uuid, "--macaddress1", mac) end diff --git a/templates/locales/en.yml b/templates/locales/en.yml index 81365587d..72124f7e9 100644 --- a/templates/locales/en.yml +++ b/templates/locales/en.yml @@ -1652,6 +1652,8 @@ en: using the other host only network. preparing: |- Preparing network interfaces based on configuration... + cleanup_vbox_default_dhcp: |- + Found default DHCP server from initial VirtualBox install. Cleaning it up... host_only_network: collides: |- The specified host network collides with a non-hostonly network! diff --git a/test/unit/plugins/providers/virtualbox/action/network_test.rb b/test/unit/plugins/providers/virtualbox/action/network_test.rb index fa065a1a8..82dc14865 100644 --- a/test/unit/plugins/providers/virtualbox/action/network_test.rb +++ b/test/unit/plugins/providers/virtualbox/action/network_test.rb @@ -47,9 +47,10 @@ describe VagrantPlugins::ProviderVirtualBox::Action::Network do let(:hostonlyifs) { [] } let(:dhcpservers) { [] } let(:guest) { double("guest") } + let(:network_args) {{ type: :dhcp }} before do - machine.config.vm.network 'private_network', type: :dhcp + machine.config.vm.network 'private_network', network_args allow(driver).to receive(:read_bridged_interfaces) { bridgedifs } allow(driver).to receive(:read_host_only_interfaces) { hostonlyifs } allow(driver).to receive(:read_dhcp_servers) { dhcpservers } @@ -57,7 +58,7 @@ describe VagrantPlugins::ProviderVirtualBox::Action::Network do end it "creates a host only interface and a dhcp server using default ips, then tells the guest to configure the network after boot" do - allow(driver).to receive(:create_host_only_network) {{ name: 'HostInterfaceNetworking-vboxnet0' }} + allow(driver).to receive(:create_host_only_network) {{ name: 'vboxnet0' }} allow(driver).to receive(:create_dhcp_server) allow(guest).to receive(:capability) @@ -68,7 +69,7 @@ describe VagrantPlugins::ProviderVirtualBox::Action::Network do netmask: '255.255.255.0', }) - expect(driver).to have_received(:create_dhcp_server).with('HostInterfaceNetworking-vboxnet0', { + expect(driver).to have_received(:create_dhcp_server).with('vboxnet0', { adapter_ip: "172.28.128.1", auto_config: true, ip: "172.28.128.1", @@ -91,5 +92,50 @@ describe VagrantPlugins::ProviderVirtualBox::Action::Network do interface: nil }]) end + + context "when the default vbox dhcpserver is present from a fresh vbox install (see issue #3803)" do + let(:dhcpservers) {[ + { + network_name: 'HostInterfaceNetworking-vboxnet0', + network: 'vboxnet0', + ip: '192.168.56.100', + netmask: '255.255.255.0', + lower: '192.168.56.101', + upper: '192.168.56.254' + } + ]} + + it "removes the invalid dhcpserver so it won't collide with any host only interface" do + allow(driver).to receive(:remove_dhcp_server) + allow(driver).to receive(:create_host_only_network) {{ name: 'vboxnet0' }} + allow(driver).to receive(:create_dhcp_server) + allow(guest).to receive(:capability) + + subject.call(env) + + expect(driver).to have_received(:remove_dhcp_server).with('HostInterfaceNetworking-vboxnet0') + end + + context "but the user has intentionally configured their network just that way" do + let (:network_args) {{ + type: :dhcp, + adapter_ip: '192.168.56.1', + dhcp_ip: '192.168.56.100', + dhcp_lower: '192.168.56.101', + dhcp_upper: '192.168.56.254' + }} + + it "does not attempt to remove the dhcpserver" do + allow(driver).to receive(:remove_dhcp_server) + allow(driver).to receive(:create_host_only_network) {{ name: 'vboxnet0' }} + allow(driver).to receive(:create_dhcp_server) + allow(guest).to receive(:capability) + + subject.call(env) + + expect(driver).not_to have_received(:remove_dhcp_server).with('HostInterfaceNetworking-vboxnet0') + end + end + end end end diff --git a/test/unit/plugins/providers/virtualbox/support/shared/virtualbox_driver_version_4_x_examples.rb b/test/unit/plugins/providers/virtualbox/support/shared/virtualbox_driver_version_4_x_examples.rb index 68c5ce5f0..513f065a7 100644 --- a/test/unit/plugins/providers/virtualbox/support/shared/virtualbox_driver_version_4_x_examples.rb +++ b/test/unit/plugins/providers/virtualbox/support/shared/virtualbox_driver_version_4_x_examples.rb @@ -34,10 +34,11 @@ shared_examples "a version 4.x virtualbox driver" do |options| it "returns a list with one entry describing that server" do expect(subject.read_dhcp_servers).to eq([{ - network: 'vboxnet0', - ip: '172.28.128.2', - lower: '172.28.128.3', - upper: '172.28.128.254', + network_name: 'HostInterfaceNetworking-vboxnet0', + network: 'vboxnet0', + ip: '172.28.128.2', + lower: '172.28.128.3', + upper: '172.28.128.254', }]) end end @@ -64,8 +65,8 @@ shared_examples "a version 4.x virtualbox driver" do |options| it "returns a list with one entry for each server" do expect(subject.read_dhcp_servers).to eq([ - {network: 'vboxnet0', ip: '172.28.128.2', lower: '172.28.128.3', upper: '172.28.128.254'}, - {network: 'vboxnet1', ip: '10.0.0.2', lower: '10.0.0.3', upper: '10.0.0.254'}, + {network_name: 'HostInterfaceNetworking-vboxnet0', network: 'vboxnet0', ip: '172.28.128.2', lower: '172.28.128.3', upper: '172.28.128.254'}, + {network_name: 'HostInterfaceNetworking-vboxnet1', network: 'vboxnet1', ip: '10.0.0.2', lower: '10.0.0.3', upper: '10.0.0.254'}, ]) end end @@ -200,4 +201,14 @@ shared_examples "a version 4.x virtualbox driver" do |options| end end end + + describe "remove_dhcp_server" do + it "removes the dhcp server with the specified network name" do + expect(subprocess).to receive(:execute). + with("VBoxManage", "dhcpserver", "remove", "--netname", "HostInterfaceNetworking-vboxnet0", an_instance_of(Hash)). + and_return(subprocess_result(stdout: '')) + + subject.remove_dhcp_server("HostInterfaceNetworking-vboxnet0") + end + end end