diff --git a/lib/vagrant/errors.rb b/lib/vagrant/errors.rb index ac3ff70f5..0a3accec6 100644 --- a/lib/vagrant/errors.rb +++ b/lib/vagrant/errors.rb @@ -560,6 +560,10 @@ module Vagrant error_key(:virtualbox_broken_version_040214) end + class VirtualBoxGuestPropertyNotFound < VagrantError + error_key(:virtualbox_guest_property_not_found) + end + class VirtualBoxInvalidVersion < VagrantError error_key(:virtualbox_invalid_version) end diff --git a/plugins/providers/virtualbox/action/prepare_nfs_settings.rb b/plugins/providers/virtualbox/action/prepare_nfs_settings.rb index 03755707e..9a1eec84c 100644 --- a/plugins/providers/virtualbox/action/prepare_nfs_settings.rb +++ b/plugins/providers/virtualbox/action/prepare_nfs_settings.rb @@ -2,41 +2,56 @@ module VagrantPlugins module ProviderVirtualBox module Action class PrepareNFSSettings + include Vagrant::Util::Retryable + def initialize(app,env) @app = app @logger = Log4r::Logger.new("vagrant::action::vm::nfs") end def call(env) + @machine = env[:machine] @app.call(env) - using_nfs = false - env[:machine].config.vm.synced_folders.each do |id, opts| - if opts[:type] == :nfs - using_nfs = true - break - end - end - - if using_nfs + if using_nfs? @logger.info("Using NFS, preparing NFS settings by reading host IP and machine IP") - env[:nfs_host_ip] = read_host_ip(env[:machine]) - env[:nfs_machine_ip] = read_machine_ip(env[:machine]) - - raise Vagrant::Errors::NFSNoHostonlyNetwork if !env[:nfs_machine_ip] + add_nfs_settings_to_env!(env) end end - # Returns the IP address of the first host only network adapter + # We're using NFS if we have any synced folder with NFS configured. If + # we are not using NFS we don't need to do the extra work to + # populate these fields in the environment. + def using_nfs? + @machine.config.vm.synced_folders.any? { |_, opts| opts[:type] == :nfs } + end + + # Extracts the proper host and guest IPs for NFS mounts and stores them + # in the environment for the SyncedFolder action to use them in + # mounting. # - # @param [Machine] machine - # @return [String] - def read_host_ip(machine) - machine.provider.driver.read_network_interfaces.each do |adapter, opts| + # The ! indicates that this method modifies its argument. + def add_nfs_settings_to_env!(env) + adapter, host_ip = find_host_only_adapter + machine_ip = nil + machine_ip = read_machine_ip(adapter) if adapter + + raise Vagrant::Errors::NFSNoHostonlyNetwork if !host_ip || !machine_ip + + env[:nfs_host_ip] = host_ip + env[:nfs_machine_ip] = machine_ip + end + + # Finds first host only network adapter and returns its adapter number + # and IP address + # + # @return [Integer, String] adapter number, ip address of found host-only adapter + def find_host_only_adapter + @machine.provider.driver.read_network_interfaces.each do |adapter, opts| if opts[:type] == :hostonly - machine.provider.driver.read_host_only_interfaces.each do |interface| + @machine.provider.driver.read_host_only_interfaces.each do |interface| if interface[:name] == opts[:hostonly] - return interface[:ip] + return adapter, interface[:ip] end end end @@ -45,23 +60,34 @@ module VagrantPlugins nil end - # Returns the IP address of the guest by looking at the first - # enabled host only network. + # Returns the IP address of the guest by looking at vbox guest property + # for the appropriate guest adapter. # - # @return [String] - def read_machine_ip(machine) - ips = [] - machine.config.vm.networks.each do |type, options| - if type == :private_network && options[:ip].is_a?(String) - ips << options[:ip] - end - end + # For DHCP interfaces, the guest property will not be present until the + # guest completes + # + # @param [Integer] adapter number to read IP for + # @return [String] ip address of adapter + def read_machine_ip(adapter) + # vbox guest properties are 0-indexed, while showvminfo network + # interfaces are 1-indexed. go figure. + guestproperty_adapter = adapter - 1 - if ips.empty? - return nil + # we need to wait for the guest's IP to show up as a guest property. + # retry thresholds are relatively high since we might need to wait + # for DHCP, but even static IPs can take a second or two to appear. + retryable(retry_options.merge(on: Vagrant::Errors::VirtualBoxGuestPropertyNotFound)) do + @machine.provider.driver.read_guest_ip(guestproperty_adapter) end + rescue Vagrant::Errors::VirtualBoxGuestPropertyNotFound + # this error is more specific with a better error message directing + # the user towards the fact that it's probably a reportable bug + raise Vagrant::Errors::NFSNoGuestIP + end - ips + # Separating these out so we can stub out the sleep in tests + def retry_options + {tries: 15, sleep: 1} end end end diff --git a/plugins/providers/virtualbox/driver/base.rb b/plugins/providers/virtualbox/driver/base.rb index 1e0795739..7a315a869 100644 --- a/plugins/providers/virtualbox/driver/base.rb +++ b/plugins/providers/virtualbox/driver/base.rb @@ -185,6 +185,14 @@ module VagrantPlugins def read_guest_additions_version end + # Returns the value of a guest property on the current VM. + # + # @param [String] property the name of the guest property to read + # @return [String] value of the guest property + # @raise [VirtualBoxGuestPropertyNotFound] if the guest property does not have a value + def read_guest_property(property) + end + # Returns a list of available host only interfaces. # # @return [Hash] @@ -292,7 +300,7 @@ module VagrantPlugins retryable(:on => Vagrant::Errors::VBoxManageError, :tries => tries, :sleep => 1) do # If there is an error with VBoxManage, this gets set to true errored = false - + # Execute the command r = raw(*command, &block) @@ -327,7 +335,7 @@ module VagrantPlugins errored = true end end - + # If there was an error running VBoxManage, show the error and the # output. if errored diff --git a/plugins/providers/virtualbox/driver/meta.rb b/plugins/providers/virtualbox/driver/meta.rb index 4a4304668..1da74d5d3 100644 --- a/plugins/providers/virtualbox/driver/meta.rb +++ b/plugins/providers/virtualbox/driver/meta.rb @@ -93,6 +93,8 @@ module VagrantPlugins :read_forwarded_ports, :read_bridged_interfaces, :read_guest_additions_version, + :read_guest_ip, + :read_guest_property, :read_host_only_interfaces, :read_mac_address, :read_mac_addresses, diff --git a/plugins/providers/virtualbox/driver/version_4_0.rb b/plugins/providers/virtualbox/driver/version_4_0.rb index e0ce73cac..92fc2f7c9 100644 --- a/plugins/providers/virtualbox/driver/version_4_0.rb +++ b/plugins/providers/virtualbox/driver/version_4_0.rb @@ -269,6 +269,19 @@ module VagrantPlugins return nil end + def read_guest_ip(adapter_number) + read_guest_property("/VirtualBox/GuestInfo/Net/#{adapter_number}/V4/IP") + end + + def read_guest_property(property) + output = execute("guestproperty", "get", @uuid, property) + if output =~ /^Value: (.+?)$/ + $1.to_s + else + raise Vagrant::Errors::VirtualBoxGuestPropertyNotFound, :guest_property => property + end + end + def read_host_only_interfaces dhcp = {} execute("list", "dhcpservers", :retryable => true).split("\n\n").each do |block| diff --git a/plugins/providers/virtualbox/driver/version_4_1.rb b/plugins/providers/virtualbox/driver/version_4_1.rb index d724fb280..fad2cf949 100644 --- a/plugins/providers/virtualbox/driver/version_4_1.rb +++ b/plugins/providers/virtualbox/driver/version_4_1.rb @@ -274,6 +274,19 @@ module VagrantPlugins return nil end + def read_guest_ip(adapter_number) + read_guest_property("/VirtualBox/GuestInfo/Net/#{adapter_number}/V4/IP") + end + + def read_guest_property(property) + output = execute("guestproperty", "get", @uuid, property) + if output =~ /^Value: (.+?)$/ + $1.to_s + else + raise Vagrant::Errors::VirtualBoxGuestPropertyNotFound, :guest_property => property + end + end + def read_host_only_interfaces dhcp = {} execute("list", "dhcpservers", :retryable => true).split("\n\n").each do |block| diff --git a/plugins/providers/virtualbox/driver/version_4_2.rb b/plugins/providers/virtualbox/driver/version_4_2.rb index 684c343fa..beb9575d9 100644 --- a/plugins/providers/virtualbox/driver/version_4_2.rb +++ b/plugins/providers/virtualbox/driver/version_4_2.rb @@ -298,6 +298,19 @@ module VagrantPlugins return nil end + def read_guest_ip(adapter_number) + read_guest_property("/VirtualBox/GuestInfo/Net/#{adapter_number}/V4/IP") + end + + def read_guest_property(property) + output = execute("guestproperty", "get", @uuid, property) + if output =~ /^Value: (.+?)$/ + $1.to_s + else + raise Vagrant::Errors::VirtualBoxGuestPropertyNotFound, :guest_property => property + end + end + def read_host_only_interfaces dhcp = {} execute("list", "dhcpservers", :retryable => true).split("\n\n").each do |block| diff --git a/plugins/providers/virtualbox/driver/version_4_3.rb b/plugins/providers/virtualbox/driver/version_4_3.rb index c57f85231..8e8001351 100644 --- a/plugins/providers/virtualbox/driver/version_4_3.rb +++ b/plugins/providers/virtualbox/driver/version_4_3.rb @@ -304,6 +304,19 @@ module VagrantPlugins return nil end + def read_guest_ip(adapter_number) + read_guest_property("/VirtualBox/GuestInfo/Net/#{adapter_number}/V4/IP") + end + + def read_guest_property(property) + output = execute("guestproperty", "get", @uuid, property) + if output =~ /^Value: (.+?)$/ + $1.to_s + else + raise Vagrant::Errors::VirtualBoxGuestPropertyNotFound, :guest_property => property + end + end + def read_host_only_interfaces dhcp = {} execute("list", "dhcpservers", :retryable => true).split("\n\n").each do |block| diff --git a/templates/locales/en.yml b/templates/locales/en.yml index 1c5562094..5fd4d0ceb 100644 --- a/templates/locales/en.yml +++ b/templates/locales/en.yml @@ -390,9 +390,9 @@ en: No host IP was given to the Vagrant core NFS helper. This is an internal error that should be reported as a bug. nfs_no_hostonly_network: |- - NFS requires a host-only network with a static IP to be created. - Please add a host-only network with a static IP to the machine - for NFS to work. + NFS requires a host-only network to be created. + Please add a host-only network to the machine (with either DHCP or a + static IP) for NFS to work. no_default_synced_folder_impl: |- No synced folder implementation is available for your synced folders! Please consult the documentation to learn why this may be the case. @@ -645,6 +645,10 @@ en: 4.2.14 contains a critical bug that causes it to not work with Vagrant. VirtualBox 4.2.16+ fixes this problem. Please upgrade VirtualBox. + virtualbox_guest_property_not_found: |- + Could not find a required VirtualBox guest property: + %{guest_property} + This is an internal error that should be reported as a bug. virtualbox_invalid_version: |- Vagrant has detected that you have a version of VirtualBox installed that is not supported. Please install one of the supported versions diff --git a/test/unit/base.rb b/test/unit/base.rb index dbbd92984..d2545b060 100644 --- a/test/unit/base.rb +++ b/test/unit/base.rb @@ -13,6 +13,7 @@ require "support/tempdir" require "unit/support/dummy_communicator" require "unit/support/dummy_provider" require "unit/support/shared/base_context" +require "unit/support/shared/virtualbox_context" # Do not buffer output $stdout.sync = true diff --git a/test/unit/plugins/providers/virtualbox/action/prepare_nfs_settings_test.rb b/test/unit/plugins/providers/virtualbox/action/prepare_nfs_settings_test.rb new file mode 100644 index 000000000..fdff5196a --- /dev/null +++ b/test/unit/plugins/providers/virtualbox/action/prepare_nfs_settings_test.rb @@ -0,0 +1,121 @@ +require_relative "../base" + +describe VagrantPlugins::ProviderVirtualBox::Action::PrepareNFSSettings do + include_context "virtualbox" + + let(:machine) { + environment = Vagrant::Environment.new + provider = :virtualbox + provider_cls, provider_options = Vagrant.plugin("2").manager.providers[provider] + provider_config = Vagrant.plugin("2").manager.provider_configs[provider] + + Vagrant::Machine.new( + 'test_machine', + provider, + provider_cls, + provider_config, + provider_options, + environment.config_global, + Pathname('data_dir'), + double('box'), + environment + ) + } + + let(:env) {{ machine: machine }} + let(:app) { lambda { |*args| }} + let(:driver) { env[:machine].provider.driver } + + subject { described_class.new(app, env) } + + it "calls the next action in the chain" do + called = false + app = lambda { |*args| called = true } + + action = described_class.new(app, env) + action.call(env) + + called.should == true + end + + describe "with an nfs synced folder" do + before do + env[:machine].config.vm.synced_folder("/host/path", "/guest/path", nfs: true) + env[:machine].config.finalize! + end + + it "sets nfs_host_ip and nfs_machine_ip properly" do + adapter_number = 2 + adapter_name = "vmnet2" + driver.stub(:read_network_interfaces).and_return( + adapter_number => {type: :hostonly, hostonly: adapter_name} + ) + driver.stub(:read_host_only_interfaces).and_return([ + {name: adapter_name, ip: "1.2.3.4"} + ]) + driver.should_receive(:read_guest_ip).with(adapter_number-1). + and_return("2.3.4.5") + + subject.call(env) + + env[:nfs_host_ip].should == "1.2.3.4" + env[:nfs_machine_ip].should == "2.3.4.5" + end + + it "raises an error when no host only adapter is configured" do + driver.stub(:read_network_interfaces) {{}} + + expect { subject.call(env) }. + to raise_error(Vagrant::Errors::NFSNoHostonlyNetwork) + end + + it "retries through guest property not found errors" do + adapter_number = 2 + adapter_name = "vmnet2" + driver.stub(:read_network_interfaces).and_return({ + adapter_number => {type: :hostonly, hostonly: adapter_name} + }) + driver.stub(:read_host_only_interfaces).and_return([ + {name: adapter_name, ip: "1.2.3.4"} + ]) + driver.should_receive(:read_guest_ip).with(adapter_number-1). + and_return("2.3.4.5") + + raise_then_return = [ + lambda { raise Vagrant::Errors::VirtualBoxGuestPropertyNotFound, :guest_property => 'stub' }, + lambda { "2.3.4.5" } + ] + driver.stub(:read_guest_ip) { raise_then_return.shift.call } + + # override sleep to 0 so test does not take seconds + retry_options = subject.retry_options + subject.stub(:retry_options).and_return(retry_options.merge(sleep: 0)) + + subject.call(env) + + env[:nfs_host_ip].should == "1.2.3.4" + env[:nfs_machine_ip].should == "2.3.4.5" + end + + it "raises an error informing the user of a bug when the guest IP cannot be found" do + adapter_number = 2 + adapter_name = "vmnet2" + driver.stub(:read_network_interfaces).and_return({ + adapter_number => {type: :hostonly, hostonly: adapter_name} + }) + driver.stub(:read_host_only_interfaces).and_return([ + {name: adapter_name, ip: "1.2.3.4"} + ]) + driver.stub(:read_guest_ip) { + raise Vagrant::Errors::VirtualBoxGuestPropertyNotFound, :guest_property => 'stub' + } + + # override sleep to 0 so test does not take seconds + retry_options = subject.retry_options + subject.stub(:retry_options).and_return(retry_options.merge(sleep: 0)) + + expect { subject.call(env) }. + to raise_error(Vagrant::Errors::NFSNoGuestIP) + end + end +end diff --git a/test/unit/plugins/providers/virtualbox/base.rb b/test/unit/plugins/providers/virtualbox/base.rb new file mode 100644 index 000000000..75016267e --- /dev/null +++ b/test/unit/plugins/providers/virtualbox/base.rb @@ -0,0 +1,4 @@ +# base test helper for virtualbox unit tests + +require_relative "../../../base" +require_relative "support/shared/virtualbox_driver_version_4_x_examples" diff --git a/test/unit/plugins/providers/virtualbox/driver/version_4_0_test.rb b/test/unit/plugins/providers/virtualbox/driver/version_4_0_test.rb new file mode 100644 index 000000000..f51f65215 --- /dev/null +++ b/test/unit/plugins/providers/virtualbox/driver/version_4_0_test.rb @@ -0,0 +1,9 @@ +require_relative "../base" + +describe VagrantPlugins::ProviderVirtualBox::Driver::Version_4_0 do + include_context "virtualbox" + let(:vbox_version) { "4.0.0" } + subject { VagrantPlugins::ProviderVirtualBox::Driver::Meta.new(uuid) } + + it_behaves_like "a version 4.x virtualbox driver" +end diff --git a/test/unit/plugins/providers/virtualbox/driver/version_4_1_test.rb b/test/unit/plugins/providers/virtualbox/driver/version_4_1_test.rb new file mode 100644 index 000000000..786c4f6f9 --- /dev/null +++ b/test/unit/plugins/providers/virtualbox/driver/version_4_1_test.rb @@ -0,0 +1,9 @@ +require_relative "../base" + +describe VagrantPlugins::ProviderVirtualBox::Driver::Version_4_1 do + include_context "virtualbox" + let(:vbox_version) { "4.1.0" } + subject { VagrantPlugins::ProviderVirtualBox::Driver::Meta.new(uuid) } + + it_behaves_like "a version 4.x virtualbox driver" +end diff --git a/test/unit/plugins/providers/virtualbox/driver/version_4_2_test.rb b/test/unit/plugins/providers/virtualbox/driver/version_4_2_test.rb new file mode 100644 index 000000000..84698d37f --- /dev/null +++ b/test/unit/plugins/providers/virtualbox/driver/version_4_2_test.rb @@ -0,0 +1,9 @@ +require_relative "../base" + +describe VagrantPlugins::ProviderVirtualBox::Driver::Version_4_2 do + include_context "virtualbox" + let(:vbox_version) { "4.2.0" } + subject { VagrantPlugins::ProviderVirtualBox::Driver::Meta.new(uuid) } + + it_behaves_like "a version 4.x virtualbox driver" +end diff --git a/test/unit/plugins/providers/virtualbox/driver/version_4_3_test.rb b/test/unit/plugins/providers/virtualbox/driver/version_4_3_test.rb new file mode 100644 index 000000000..3bb7b6c2e --- /dev/null +++ b/test/unit/plugins/providers/virtualbox/driver/version_4_3_test.rb @@ -0,0 +1,9 @@ +require_relative "../base" + +describe VagrantPlugins::ProviderVirtualBox::Driver::Version_4_3 do + include_context "virtualbox" + let(:vbox_version) { "4.3.0" } + subject { VagrantPlugins::ProviderVirtualBox::Driver::Meta.new(uuid) } + + it_behaves_like "a version 4.x virtualbox driver" +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 new file mode 100644 index 000000000..761e1bef7 --- /dev/null +++ b/test/unit/plugins/providers/virtualbox/support/shared/virtualbox_driver_version_4_x_examples.rb @@ -0,0 +1,42 @@ +shared_examples "a version 4.x virtualbox driver" do |options| + before do + raise ArgumentError, "Need virtualbox context to use these shared examples." if !(defined? vbox_context) + end + + describe "read_guest_property" do + it "reads the guest property of the machine referenced by the UUID" do + key = "/Foo/Bar" + + subprocess.should_receive(:execute). + with("VBoxManage", "guestproperty", "get", uuid, key, an_instance_of(Hash)). + and_return(subprocess_result(stdout: "Value: Baz\n")) + + subject.read_guest_property(key).should == "Baz" + end + + it "raises a virtualBoxGuestPropertyNotFound exception when the value is not set" do + key = "/Not/There" + + subprocess.should_receive(:execute). + with("VBoxManage", "guestproperty", "get", uuid, key, an_instance_of(Hash)). + and_return(subprocess_result(stdout: "No value set!")) + + expect { subject.read_guest_property(key) }. + to raise_error Vagrant::Errors::VirtualBoxGuestPropertyNotFound + end + end + + describe "read_guest_ip" do + it "reads the guest property for the provided adapter number" do + key = "/VirtualBox/GuestInfo/Net/1/V4/IP" + + subprocess.should_receive(:execute). + with("VBoxManage", "guestproperty", "get", uuid, key, an_instance_of(Hash)). + and_return(subprocess_result(stdout: "Value: 127.1.2.3")) + + value = subject.read_guest_ip(1) + + value.should == "127.1.2.3" + end + end +end diff --git a/test/unit/support/shared/virtualbox_context.rb b/test/unit/support/shared/virtualbox_context.rb new file mode 100644 index 000000000..6a13f58bc --- /dev/null +++ b/test/unit/support/shared/virtualbox_context.rb @@ -0,0 +1,30 @@ +shared_context "virtualbox" do + let(:vbox_context) { true } + let(:uuid) { "1234-abcd-5678-efgh" } + let(:vbox_version) { "4.3.4" } + let(:subprocess) { double("Vagrant::Util::Subprocess") } + + # this is a helper that returns a duck type suitable from a system command + # execution; allows setting exit_code, stdout, and stderr in stubs. + def subprocess_result(options={}) + defaults = {exit_code: 0, stdout: "", stderr: ""} + double("subprocess_result", defaults.merge(options)) + end + + before do + # we don't want unit tests to ever run commands on the system; so we wire + # in a double to ensure any unexpected messages raise exceptions + stub_const("Vagrant::Util::Subprocess", subprocess) + + # drivers will blow up on instantiation if they cannot determine the + # virtualbox version, so wire this stub in automatically + subprocess.stub(:execute). + with("VBoxManage", "--version", an_instance_of(Hash)). + and_return(subprocess_result(stdout: vbox_version)) + + # drivers also call vm_exists? during init; + subprocess.stub(:execute). + with("VBoxManage", "showvminfo", kind_of(String), kind_of(Hash)). + and_return(subprocess_result(exit_code: 0)) + end +end