Merge pull request #2560 from phinze/vbox-nfs-dhcp-support

providers/virtualbox: support DHCP interfaces for NFS

lib/vagrant/errors.rb

@@ -568,6 +568,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]
-            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

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

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

@@ -402,9 +402,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.
@@ -657,6 +657,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