providers/virtualbox: cleanup default vbox dhcp server

fixes #3083

Detect the presence of the default DHCP server that comes in a fresh
VirtualBox install and clean it up to prevent it from colliding with
Vagrant-managed network config.

In order to accomplish this, we:

 - add a `remove_dhcp_server` call to the virtualbox driver
 - fix dhcp options parsing to allow `:dhcp_{ip,lower,upper}`
   configuration options to make it through (so a user can override the
   removal behavior with some explicit configuration)
 - add the full `:network_name` to the details returned from
   `:read_dhcp_servers`, so we can have a durable value to pass to
   `:remove_dhcp_server`

Note that we do have to eat one more `VBoxManage list dhcpservers` for
each network interface to support this, but this seemed like a nominal
cost
This commit is contained in:
Paul Hinze 2014-11-29 14:24:39 -06:00
parent 24b6f21d1d
commit 25ff636ee2
10 changed files with 177 additions and 51 deletions

View File

@ -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<String>] interface hash as returned from read_host_only_interfaces
# @param [Hash<String>] 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<String>] dhcp_server as found by read_dhcp_servers
# @param [Hash<String>] 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<String>] 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

View File

@ -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.

View File

@ -108,6 +108,7 @@ module VagrantPlugins
:read_state,
:read_used_ports,
:read_vms,
:remove_dhcp_server,
:resume,
:set_mac_address,
:set_name,

View File

@ -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

View File

@ -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

View File

@ -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

View File

@ -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

View File

@ -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!

View File

@ -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

View File

@ -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