providers/virtualbox: support DHCP interfaces for NFS

Previously, we required a host-only interface with a static IP for NFS
to work in VirtualBox, because we needed access to the guest's IP in
order to properly configure mount commands.

After boot, VirtualBox exposes the IP addresses of a guest's network
adapters via the "guestproperty" interface.

This adds support for reading VirtualBox guest properties to the
VirtualBox driver and utilizes that support to prepare NFS settings,
which removes the necessity for a static IP for NFS to work.

In this commit we also start building out scaffolding for unit testing
vbox actions and drivers.

Test plan:
 - Prepare a Vagrantfile with the following:
   * private network with type: :dhcp
   * synced folder with nfs: true
 - Boot a VM from this Vagrantfile using the virtualbox provider
 - Machine should boot successfully with working synced folder

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

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]
+        # 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
+
+          # 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
 
-          if ips.empty?
-            return nil
-          end
-
-          ips
+        # Separating these out so we can stub out the sleep in tests
+        def retry_options
+          {tries: 15, sleep: 1}
         end
       end
     end

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]

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,

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|

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|

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|

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|

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

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

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

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"

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

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

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

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

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

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