From c1141dded30fbe8a67e62a139ccfbe92215db6d2 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Fri, 13 Dec 2013 22:03:14 -0800 Subject: [PATCH] providers/virtualbox: don't clear /etc/exports on BSDs for NFS [GH-2645] --- CHANGELOG.md | 4 +- lib/vagrant/errors.rb | 4 ++ plugins/providers/virtualbox/action.rb | 4 ++ .../virtualbox/action/prepare_nfs_settings.rb | 7 +-- .../action/prepare_nfs_valid_ids.rb | 17 ++++++ plugins/synced_folders/nfs/synced_folder.rb | 16 +++--- templates/locales/en.yml | 4 ++ .../action/prepare_nfs_settings_test.rb | 41 ++++---------- .../action/prepare_nfs_valid_ids_test.rb | 53 +++++++++++++++++++ 9 files changed, 109 insertions(+), 41 deletions(-) create mode 100644 plugins/providers/virtualbox/action/prepare_nfs_valid_ids.rb create mode 100644 test/unit/plugins/providers/virtualbox/action/prepare_nfs_valid_ids_test.rb diff --git a/CHANGELOG.md b/CHANGELOG.md index c8f6ae01a..84ffee6bc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,7 +22,9 @@ BUG FIXES: [GH-2620] - guests/redhat: fix configure networks bringing down interfaces that don't exist. [GH-2614] - - provisioners/chef: fix node/client deletion when node_name is not + - providers/virtualbox: don't override NFS exports for all VMs when + coming up. [GH-2645] + - provisioners/chef: fix node/client deletion when node\_name is not set. [GH-2345] ## 1.4.0 (December 9, 2013) diff --git a/lib/vagrant/errors.rb b/lib/vagrant/errors.rb index 82c3d32c8..7a41a5e85 100644 --- a/lib/vagrant/errors.rb +++ b/lib/vagrant/errors.rb @@ -380,6 +380,10 @@ module Vagrant error_key(:nfs_no_hostonly_network) end + class NFSNoValidIds < VagrantError + error_key(:nfs_no_valid_ids) + end + class NoDefaultSyncedFolderImpl < VagrantError error_key(:no_default_synced_folder_impl) end diff --git a/plugins/providers/virtualbox/action.rb b/plugins/providers/virtualbox/action.rb index fb9a29654..535364a89 100644 --- a/plugins/providers/virtualbox/action.rb +++ b/plugins/providers/virtualbox/action.rb @@ -33,6 +33,7 @@ module VagrantPlugins autoload :Package, File.expand_path("../action/package", __FILE__) autoload :PackageVagrantfile, File.expand_path("../action/package_vagrantfile", __FILE__) autoload :PrepareNFSSettings, File.expand_path("../action/prepare_nfs_settings", __FILE__) + autoload :PrepareNFSValidIds, File.expand_path("../action/prepare_nfs_valid_ids", __FILE__) autoload :PrepareForwardedPortCollisionParams, File.expand_path("../action/prepare_forwarded_port_collision_params", __FILE__) autoload :Resume, File.expand_path("../action/resume", __FILE__) autoload :SaneDefaults, File.expand_path("../action/sane_defaults", __FILE__) @@ -56,6 +57,7 @@ module VagrantPlugins b.use EnvSet, :port_collision_repair => true b.use PrepareForwardedPortCollisionParams b.use HandleForwardedPortCollisions + b.use PrepareNFSValidIds b.use SyncedFolderCleanup b.use SyncedFolders b.use PrepareNFSSettings @@ -93,6 +95,7 @@ module VagrantPlugins b3.use CleanMachineFolder b3.use DestroyUnusedNetworkInterfaces b3.use ProvisionerCleanup + b3.use PrepareNFSValidIds b3.use SyncedFolderCleanup else b3.use MessageWillNotDestroy @@ -143,6 +146,7 @@ module VagrantPlugins b2.use CheckAccessible b2.use action_halt b2.use ClearForwardedPorts + b2.use PrepareNFSValidIds b2.use SyncedFolderCleanup b2.use Export b2.use PackageVagrantfile diff --git a/plugins/providers/virtualbox/action/prepare_nfs_settings.rb b/plugins/providers/virtualbox/action/prepare_nfs_settings.rb index 9a1eec84c..f6e9ba5cf 100644 --- a/plugins/providers/virtualbox/action/prepare_nfs_settings.rb +++ b/plugins/providers/virtualbox/action/prepare_nfs_settings.rb @@ -4,18 +4,19 @@ module VagrantPlugins class PrepareNFSSettings include Vagrant::Util::Retryable - def initialize(app,env) + def initialize(app, env) @app = app @logger = Log4r::Logger.new("vagrant::action::vm::nfs") end def call(env) @machine = env[:machine] + @app.call(env) if using_nfs? @logger.info("Using NFS, preparing NFS settings by reading host IP and machine IP") - add_nfs_settings_to_env!(env) + add_ips_to_env!(env) end end @@ -31,7 +32,7 @@ module VagrantPlugins # mounting. # # The ! indicates that this method modifies its argument. - def add_nfs_settings_to_env!(env) + def add_ips_to_env!(env) adapter, host_ip = find_host_only_adapter machine_ip = nil machine_ip = read_machine_ip(adapter) if adapter diff --git a/plugins/providers/virtualbox/action/prepare_nfs_valid_ids.rb b/plugins/providers/virtualbox/action/prepare_nfs_valid_ids.rb new file mode 100644 index 000000000..902e27485 --- /dev/null +++ b/plugins/providers/virtualbox/action/prepare_nfs_valid_ids.rb @@ -0,0 +1,17 @@ +module VagrantPlugins + module ProviderVirtualBox + module Action + class PrepareNFSValidIds + def initialize(app, env) + @app = app + @logger = Log4r::Logger.new("vagrant::action::vm::nfs") + end + + def call(env) + env[:nfs_valid_ids] = env[:machine].provider.driver.read_vms.values + @app.call(env) + end + end + end + end +end diff --git a/plugins/synced_folders/nfs/synced_folder.rb b/plugins/synced_folders/nfs/synced_folder.rb index d74fce9e1..4d98b12ac 100644 --- a/plugins/synced_folders/nfs/synced_folder.rb +++ b/plugins/synced_folders/nfs/synced_folder.rb @@ -1,6 +1,8 @@ require 'fileutils' require 'zlib' +require "log4r" + require "vagrant/util/platform" module VagrantPlugins @@ -13,6 +15,12 @@ module VagrantPlugins # will be mounted. # class SyncedFolder < Vagrant.plugin("2", :synced_folder) + def initialize(*args) + super + + @logger = Log4r::Logger.new("vagrant::synced_folders::nfs") + end + def usable?(machine) # NFS is always available true @@ -53,14 +61,10 @@ module VagrantPlugins def cleanup(machine, opts) ids = opts[:nfs_valid_ids] - if !ids - # Get the ID of all active machines. - ids = machine.env.active_machines.map do |name, provider| - machine.env.machine(name, provider).id - end - end + raise Vagrant::Errors::NFSNoValidIds if !ids # Prune any of the unused machines + @logger.info("NFS pruning. Valid IDs: #{ids.inspect}") machine.env.host.nfs_prune(ids) end diff --git a/templates/locales/en.yml b/templates/locales/en.yml index abf9a4da4..12981a861 100644 --- a/templates/locales/en.yml +++ b/templates/locales/en.yml @@ -443,6 +443,10 @@ en: 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. + nfs_no_valid_ids: |- + No valid IDs were given to the NFS synced folder implementation to + prune. This is an internal bug with Vagrant and an issue should be + filed. 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. 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 index fdff5196a..03223fa1a 100644 --- a/test/unit/plugins/providers/virtualbox/action/prepare_nfs_settings_test.rb +++ b/test/unit/plugins/providers/virtualbox/action/prepare_nfs_settings_test.rb @@ -38,24 +38,22 @@ describe VagrantPlugins::ProviderVirtualBox::Action::PrepareNFSSettings do called.should == true end - describe "with an nfs synced folder" do + context "with an nfs synced folder" do before do env[:machine].config.vm.synced_folder("/host/path", "/guest/path", nfs: true) env[:machine].config.finalize! + + # Stub out the stuff so it just works by default + driver.stub(read_network_interfaces: { + 2 => {type: :hostonly, hostonly: "vmnet2"}, + }) + driver.stub(read_host_only_interfaces: [ + {name: "vmnet2", ip: "1.2.3.4"}, + ]) + driver.stub(:read_guest_ip).with(1).and_return("2.3.4.5") 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" @@ -70,17 +68,6 @@ describe VagrantPlugins::ProviderVirtualBox::Action::PrepareNFSSettings do 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" } @@ -98,14 +85,6 @@ describe VagrantPlugins::ProviderVirtualBox::Action::PrepareNFSSettings do 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' } diff --git a/test/unit/plugins/providers/virtualbox/action/prepare_nfs_valid_ids_test.rb b/test/unit/plugins/providers/virtualbox/action/prepare_nfs_valid_ids_test.rb new file mode 100644 index 000000000..374f07132 --- /dev/null +++ b/test/unit/plugins/providers/virtualbox/action/prepare_nfs_valid_ids_test.rb @@ -0,0 +1,53 @@ +require_relative "../base" + +describe VagrantPlugins::ProviderVirtualBox::Action::PrepareNFSValidIds 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) } + + before do + driver.stub(read_vms: {}) + end + + 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 + + it "sets nfs_valid_ids" do + hash = {"foo" => "1", "bar" => "4"} + driver.stub(read_vms: hash) + + subject.call(env) + + expect(env[:nfs_valid_ids]).to eql(hash.values) + end +end