From 99346cc516b4fb15fa8c049fc00ad9cf925deaed Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Wed, 5 Feb 2014 19:05:50 -0800 Subject: [PATCH 01/12] core: add Vagrantfile class, tests --- lib/vagrant/vagrantfile.rb | 157 ++++++++++++++++++ test/unit/vagrant/vagrantfile_test.rb | 230 ++++++++++++++++++++++++++ 2 files changed, 387 insertions(+) create mode 100644 lib/vagrant/vagrantfile.rb create mode 100644 test/unit/vagrant/vagrantfile_test.rb diff --git a/lib/vagrant/vagrantfile.rb b/lib/vagrant/vagrantfile.rb new file mode 100644 index 000000000..0857dc807 --- /dev/null +++ b/lib/vagrant/vagrantfile.rb @@ -0,0 +1,157 @@ +module Vagrant + # This class provides a way to load and access the contents + # of a Vagrantfile. + # + # This class doesn't actually load Vagrantfiles, parse them, + # merge them, etc. That is the job of {Config::Loader}. This + # class, on the other hand, has higher-level operations on + # a loaded Vagrantfile such as looking up the defined machines, + # loading the configuration of a specific machine/provider combo, + # etc. + class Vagrantfile + # Initializes by loading a Vagrantfile. + # + # @param [Config::Loader] loader Configuration loader that should + # already be configured with the proper Vagrantflie locations. + # This usually comes from {Vagrant::Environment} + # @param [Array] keys The Vagrantfiles to load and the + # order to load them in (keys within the loader). + def initialize(loader, keys) + @keys = keys + @loader = loader + @config, _ = loader.load(keys) + end + + # Returns the configuration for a single machine. + # + # When loading a box Vagrantfile, it will be prepended to the + # key order specified when initializing this class. Sub-machine + # and provider-specific overrides are appended at the end. The + # actual order is: + # + # - box + # - keys specified for #initialize + # - sub-machine + # - provider + # + # @param [Symbol] name Name of the machine. + # @param [Symbol] provider The provider the machine should + # be backed by (required for provider overrides). + # @param [BoxCollection] boxes BoxCollection to look up the + # box Vagrantfile. + def machine_config(name, provider, boxes) + keys = @keys.dup + + sub_machine = @config.vm.defined_vms[name] + if !sub_machine + raise Errors::MachineNotFound, + :name => name, :provider => provider + end + + provider_plugin = Vagrant.plugin("2").manager.providers[provider] + if !provider_plugin + raise Errors::ProviderNotFound, + :machine => name, :provider => provider + end + + box_formats = provider_plugin[1][:box_format] || provider + + # Add the sub-machine configuration to the loader and keys + vm_config_key = "#{object_id}_machine_#{name}" + @loader.set(vm_config_key, sub_machine.config_procs) + keys << vm_config_key + + # Load once so that we can get the proper box value + config, config_warnings, config_errors = @loader.load(keys) + + # Track the original box so we know if we changed + original_box = config.vm.box + + # The proc below loads the box and provider overrides. This is + # in a proc because it may have to recurse if the provider override + # changes the box. + load_box_proc = lambda do + local_keys = keys.dup + + # Load the box Vagrantfile, if there is one + if config.vm.box + box = boxes.find(config.vm.box, box_formats) + if box + box_vagrantfile = find_vagrantfile(box.directory) + if box_vagrantfile + box_config_key = + "#{boxes.object_id}_#{box.name}_#{box.provider}".to_sym + @loader.set(box_config_key, box_vagrantfile) + local_keys.unshift(box_config_key) + config, config_warnings, config_errors = @loader.load(local_keys) + end + end + end + + # Load provider overrides + provider_overrides = config.vm.get_provider_overrides(provider) + if !provider_overrides.empty? + config_key = + "#{object_id}_vm_#{name}_#{config.vm.box}_#{provider}".to_sym + @loader.set(config_key, provider_overrides) + local_keys << config_key + config, config_warnings, config_errors = @loader.load(local_keys) + end + + # If the box changed, then we need to reload + if original_box != config.vm.box + # TODO: infinite loop protection? + + original_box = config.vm.box + load_box_proc.call + end + end + + # Load the box and provider overrides + load_box_proc.call + + return config, config_warnings, config_errors + end + + # Returns a list of the machines that are defined within this + # Vagrantfile. + # + # @return [Array] + def machine_names + @config.vm.defined_vm_keys.dup + end + + # Returns the name of the machine that is designated as the + # "primary." + # + # In the case of a single-machine environment, this is just the + # single machine name. In the case of a multi-machine environment, + # then this is the machine that is marked as primary, or nil if + # no primary machine was specified. + # + # @return [Symbol] + def primary_machine_name + # If it is a single machine environment, then return the name + return machine_names.first if machine_names.length == 1 + + # If it is a multi-machine environment, then return the primary + @config.vm.defined_vms.each do |name, subvm| + return name if subvm.options[:primary] + end + + # If no primary was specified, nil it is + nil + end + + protected + + def find_vagrantfile(search_path) + ["Vagrantfile", "vagrantfile"].each do |vagrantfile| + current_path = search_path.join(vagrantfile) + return current_path if current_path.file? + end + + nil + end + end +end diff --git a/test/unit/vagrant/vagrantfile_test.rb b/test/unit/vagrant/vagrantfile_test.rb new file mode 100644 index 000000000..04885a0ae --- /dev/null +++ b/test/unit/vagrant/vagrantfile_test.rb @@ -0,0 +1,230 @@ +require File.expand_path("../../base", __FILE__) + +require "vagrant/vagrantfile" + +describe Vagrant::Vagrantfile do + include_context "unit" + + let(:keys) { [] } + let(:loader) { + Vagrant::Config::Loader.new( + Vagrant::Config::VERSIONS, Vagrant::Config::VERSIONS_ORDER) + } + + subject { described_class.new(loader, keys) } + + describe "#machine_config" do + let(:iso_env) { isolated_environment } + let(:boxes) { Vagrant::BoxCollection.new(iso_env.boxes_dir) } + + before do + keys << :test + end + + def configure(&block) + loader.set(:test, [["2", block]]) + end + + # A helper to register a provider for use in tests. + def register_provider(name, config_class=nil, options=nil) + provider_cls = Class.new(Vagrant.plugin("2", :provider)) + + register_plugin("2") do |p| + p.provider(name, options) { provider_cls } + + if config_class + p.config(name, :provider) { config_class } + end + end + + provider_cls + end + + it "should return a basic configured machine" do + register_provider("foo") + + configure do |config| + config.vm.box = "foo" + end + + config, _ = subject.machine_config(:default, :foo, boxes) + expect(config.vm.box).to eq("foo") + end + + it "configures with sub-machine config" do + register_provider("foo") + + configure do |config| + config.ssh.port = "1" + config.vm.box = "base" + + config.vm.define "foo" do |f| + f.ssh.port = 100 + end + end + + config, _ = subject.machine_config(:foo, :foo, boxes) + expect(config.vm.box).to eq("base") + expect(config.ssh.port).to eq(100) + end + + it "configures with box configuration if it exists" do + register_provider("foo") + + configure do |config| + config.vm.box = "base" + end + + iso_env.box2("base", :foo, vagrantfile: <<-VF) + Vagrant.configure("2") do |config| + config.ssh.port = 123 + end + VF + + config, _ = subject.machine_config(:default, :foo, boxes) + expect(config.vm.box).to eq("base") + expect(config.ssh.port).to eq(123) + end + + it "configures with box config of other supported formats" do + register_provider("foo", nil, box_format: "bar") + + configure do |config| + config.vm.box = "base" + end + + iso_env.box2("base", :bar, vagrantfile: <<-VF) + Vagrant.configure("2") do |config| + config.ssh.port = 123 + end + VF + + config, _ = subject.machine_config(:default, :foo, boxes) + expect(config.vm.box).to eq("base") + expect(config.ssh.port).to eq(123) + end + + it "loads provider overrides if set" do + register_provider("foo") + register_provider("bar") + + configure do |config| + config.ssh.port = 1 + config.vm.box = "base" + + config.vm.provider "foo" do |_, c| + c.ssh.port = 100 + end + end + + # Test with the override + config, _ = subject.machine_config(:default, :foo, boxes) + expect(config.vm.box).to eq("base") + expect(config.ssh.port).to eq(100) + + # Test without the override + config, _ = subject.machine_config(:default, :bar, boxes) + expect(config.vm.box).to eq("base") + expect(config.ssh.port).to eq(1) + end + + it "loads the proper box if in a provider override" do + register_provider("foo") + + configure do |config| + config.vm.box = "base" + + config.vm.provider "foo" do |_, c| + c.vm.box = "foobox" + end + end + + iso_env.box2("base", :foo, vagrantfile: <<-VF) + Vagrant.configure("2") do |config| + config.ssh.port = 123 + end + VF + + iso_env.box2("foobox", :foo, vagrantfile: <<-VF) + Vagrant.configure("2") do |config| + config.ssh.port = 234 + end + VF + + config, _ = subject.machine_config(:default, :foo, boxes) + expect(config.vm.box).to eq("foobox") + expect(config.ssh.port).to eq(234) + end + + it "raises an error if the machine is not found" do + expect { subject.machine_config(:foo, :foo, boxes) }. + to raise_error(Vagrant::Errors::MachineNotFound) + end + + it "raises an error if the provider is not found" do + expect { subject.machine_config(:default, :foo, boxes) }. + to raise_error(Vagrant::Errors::ProviderNotFound) + end + end + + describe "#machine_names" do + before do + keys << :test + end + + def configure(&block) + loader.set(:test, [["2", block]]) + end + + it "returns the default name when single-VM" do + configure { |config| } + + expect(subject.machine_names).to eq([:default]) + end + + it "returns all of the names in a multi-VM" do + configure do |config| + config.vm.define "foo" + config.vm.define "bar" + end + + expect(subject.machine_names).to eq( + [:foo, :bar]) + end + end + + describe "#primary_machine_name" do + before do + keys << :test + end + + def configure(&block) + loader.set(:test, [["2", block]]) + end + + it "returns the default name when single-VM" do + configure { |config| } + + expect(subject.primary_machine_name).to eq(:default) + end + + it "returns the designated machine in multi-VM" do + configure do |config| + config.vm.define "foo" + config.vm.define "bar", primary: true + config.vm.define "baz" + end + + expect(subject.primary_machine_name).to eq(:bar) + end + + it "returns nil if no designation in multi-VM" do + configure do |config| + config.vm.define "foo" + config.vm.define "baz" + end + + expect(subject.primary_machine_name).to be_nil + end + end +end From e3a67f7ab641ef4290f848a38bf596abdd0942d3 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Wed, 5 Feb 2014 19:16:27 -0800 Subject: [PATCH 02/12] core: Environment uses the new Vagrantflie class --- lib/vagrant/environment.rb | 107 ++++---------------------- lib/vagrant/vagrantfile.rb | 3 +- test/unit/vagrant/vagrantfile_test.rb | 11 ++- 3 files changed, 25 insertions(+), 96 deletions(-) diff --git a/lib/vagrant/environment.rb b/lib/vagrant/environment.rb index 38ced5377..72d218898 100644 --- a/lib/vagrant/environment.rb +++ b/lib/vagrant/environment.rb @@ -8,6 +8,7 @@ require 'log4r' require 'vagrant/util/file_mode' require 'vagrant/util/platform' +require "vagrant/vagrantfile" module Vagrant # Represents a single Vagrant environment. A "Vagrant environment" is @@ -313,11 +314,6 @@ module Vagrant end @logger.info("Uncached load of machine.") - sub_vm = config_global.vm.defined_vms[name] - if !sub_vm - raise Errors::MachineNotFound, :name => name, :provider => provider - end - provider_plugin = Vagrant.plugin("2").manager.providers[provider] if !provider_plugin raise Errors::ProviderNotFound, :machine => name, :provider => provider @@ -327,87 +323,16 @@ module Vagrant provider_cls = provider_plugin[0] provider_options = provider_plugin[1] - # Build the machine configuration. This requires two passes: The first pass - # loads in the machine sub-configuration. Since this can potentially - # define a new box to base the machine from, we then make a second pass - # with the box Vagrantfile (if it has one). - vm_config_key = "vm_#{name}".to_sym - @config_loader.set(vm_config_key, sub_vm.config_procs) - config, config_warnings, config_errors = \ - @config_loader.load([:home, :root, vm_config_key]) - - # Determine the possible box formats for any boxes and find the box - box_formats = provider_options[:box_format] || provider - box = nil - - # Set this variable in order to keep track of if the box changes - # too many times. - original_box = config.vm.box - box_changed = false - - load_box_and_overrides = lambda do - box = nil - if config.vm.box - box = boxes.find( - config.vm.box, box_formats, config.vm.box_version) - end - - # If a box was found, then we attempt to load the Vagrantfile for - # that box. We don't require a box since we allow providers to download - # boxes and so on. - if box - box_vagrantfile = find_vagrantfile(box.directory) - if box_vagrantfile - # The box has a custom Vagrantfile, so we load that into the config - # as well. - @logger.info("Box exists with Vagrantfile. Reloading machine config.") - box_config_key = "box_#{box.name}_#{box.provider}".to_sym - @config_loader.set(box_config_key, box_vagrantfile) - config, config_warnings, config_errors = \ - @config_loader.load([box_config_key, :home, :root, vm_config_key]) - end - end - - # If there are provider overrides for the machine, then we run - # those as well. - provider_overrides = config.vm.get_provider_overrides(provider) - if provider_overrides.length > 0 - @logger.info("Applying #{provider_overrides.length} provider overrides. Reloading config.") - provider_override_key = "vm_#{name}_#{config.vm.box}_#{provider}".to_sym - @config_loader.set(provider_override_key, provider_overrides) - config, config_warnings, config_errors = \ - @config_loader.load([box_config_key, :home, :root, vm_config_key, provider_override_key]) - end - - if config.vm.box && original_box != config.vm.box - if box_changed - # We already changed boxes once, so report an error that a - # box is attempting to change boxes again. - raise Errors::BoxConfigChangingBox - end - - # The box changed, probably due to the provider override. Let's - # run the configuration one more time with the new box. - @logger.info("Box changed to: #{config.vm.box}. Reloading configurations.") - original_box = config.vm.box - box_changed = true - - # Recurse so that we reload all the configurations - load_box_and_overrides.call - end - end - - # Load the box and overrides configuration - load_box_and_overrides.call - - # Get the provider configuration from the final loaded configuration - provider_config = config.vm.get_provider_config(provider) - # Determine the machine data directory and pass it to the machine. # XXX: Permissions error here. - machine_data_path = @local_data_path.join("machines/#{name}/#{provider}") + machine_data_path = @local_data_path.join( + "machines/#{name}/#{provider}") FileUtils.mkdir_p(machine_data_path) + # Load the actual configuration for the machine + config, config_warnings, config_errors, box = + vagrantfile.machine_config(name, provider, boxes) + # If there were warnings or errors we want to output them if !config_warnings.empty? || !config_errors.empty? # The color of the output depends on whether we have warnings @@ -425,6 +350,9 @@ module Vagrant raise Errors::ConfigUpgradeErrors if !config_errors.empty? end + # Get the provider configuration from the final loaded configuration + provider_config = config.vm.get_provider_config(provider) + # Create the machine and cache it for future calls. This will also # return the machine from this method. @machines[cache_key] = Machine.new(name, provider, provider_cls, provider_config, @@ -437,7 +365,7 @@ module Vagrant # # @return [Array] Configured machine names. def machine_names - config_global.vm.defined_vm_keys.dup + vagrantfile.machine_names end # This returns the name of the machine that is the "primary." In the @@ -447,16 +375,11 @@ module Vagrant # # @return [Symbol] def primary_machine_name - # If it is a single machine environment, then return the name - return machine_names.first if machine_names.length == 1 + vagrantfile.primary_machine_name + end - # If it is a multi-machine environment, then return the primary - config_global.vm.defined_vms.each do |name, subvm| - return name if subvm.options[:primary] - end - - # If no primary was specified, nil it is - nil + def vagrantfile + @vagrantfile ||= Vagrantfile.new(@config_loader, [:home, :root]) end # Unload the environment, running completion hooks. The environment diff --git a/lib/vagrant/vagrantfile.rb b/lib/vagrant/vagrantfile.rb index 0857dc807..afaf33da6 100644 --- a/lib/vagrant/vagrantfile.rb +++ b/lib/vagrant/vagrantfile.rb @@ -65,6 +65,7 @@ module Vagrant config, config_warnings, config_errors = @loader.load(keys) # Track the original box so we know if we changed + box = nil original_box = config.vm.box # The proc below loads the box and provider overrides. This is @@ -110,7 +111,7 @@ module Vagrant # Load the box and provider overrides load_box_proc.call - return config, config_warnings, config_errors + return config, config_warnings, config_errors, box end # Returns a list of the machines that are defined within this diff --git a/test/unit/vagrant/vagrantfile_test.rb b/test/unit/vagrant/vagrantfile_test.rb index 04885a0ae..5531605d5 100644 --- a/test/unit/vagrant/vagrantfile_test.rb +++ b/test/unit/vagrant/vagrantfile_test.rb @@ -47,8 +47,9 @@ describe Vagrant::Vagrantfile do config.vm.box = "foo" end - config, _ = subject.machine_config(:default, :foo, boxes) + config, _, _, box = subject.machine_config(:default, :foo, boxes) expect(config.vm.box).to eq("foo") + expect(box).to be_nil end it "configures with sub-machine config" do @@ -81,9 +82,11 @@ describe Vagrant::Vagrantfile do end VF - config, _ = subject.machine_config(:default, :foo, boxes) + config, _, _, box = subject.machine_config(:default, :foo, boxes) expect(config.vm.box).to eq("base") expect(config.ssh.port).to eq(123) + expect(box).to_not be_nil + expect(box.name).to eq("base") end it "configures with box config of other supported formats" do @@ -151,9 +154,11 @@ describe Vagrant::Vagrantfile do end VF - config, _ = subject.machine_config(:default, :foo, boxes) + config, _, _, box = subject.machine_config(:default, :foo, boxes) expect(config.vm.box).to eq("foobox") expect(config.ssh.port).to eq(234) + expect(box).to_not be_nil + expect(box.name).to eq("foobox") end it "raises an error if the machine is not found" do From 7549704c5b7d0a76db7df3224a3c659bc0f91745 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Wed, 5 Feb 2014 19:24:47 -0800 Subject: [PATCH 03/12] core: Environment#config_global is gone! --- lib/vagrant/action/general/package.rb | 3 +- lib/vagrant/environment.rb | 41 +++++++++------------------ lib/vagrant/vagrantfile.rb | 4 +++ test/unit/vagrant/vagrantfile_test.rb | 18 ++++++++++++ 4 files changed, 37 insertions(+), 29 deletions(-) diff --git a/lib/vagrant/action/general/package.rb b/lib/vagrant/action/general/package.rb index 996368d3a..f112e9ede 100644 --- a/lib/vagrant/action/general/package.rb +++ b/lib/vagrant/action/general/package.rb @@ -23,7 +23,8 @@ module Vagrant @app = app env["package.files"] ||= {} - env["package.output"] ||= env[:global_config].package.name + env["package.output"] ||= env[:machine].package.name + env["package.output"] ||= "package.box" end def call(env) diff --git a/lib/vagrant/environment.rb b/lib/vagrant/environment.rb index 72d218898..07a144445 100644 --- a/lib/vagrant/environment.rb +++ b/lib/vagrant/environment.rb @@ -236,39 +236,25 @@ module Vagrant @_boxes ||= BoxCollection.new(boxes_path, temp_dir_root: tmp_path) end - # This is the global config, comprised of loading configuration from - # the default, home, and root Vagrantfiles. This configuration is only - # really useful for reading the list of virtual machines, since each - # individual VM can override _most_ settings. + # Returns the {Config::Loader} that can be used to load Vagrantflies + # given the settings of this environment. # - # This is lazy-loaded upon first use. - # - # @return [Object] - def config_global - return @config_global if @config_global - - @logger.info("Initializing config...") + # @return [Config::Loader] + def config_loader + return @config_loader if @config_loader home_vagrantfile = nil root_vagrantfile = nil home_vagrantfile = find_vagrantfile(home_path) if home_path - root_vagrantfile = find_vagrantfile(root_path, @vagrantfile_name) if root_path + if root_path + root_vagrantfile = find_vagrantfile(root_path, @vagrantfile_name) + end - # Create the configuration loader and set the sources that are global. - # We use this to load the configuration, and the list of machines we are - # managing. Then, the actual individual configuration is loaded for - # each {#machine} call. - @config_loader = Config::Loader.new(Config::VERSIONS, Config::VERSIONS_ORDER) + @config_loader = Config::Loader.new( + Config::VERSIONS, Config::VERSIONS_ORDER) @config_loader.set(:home, home_vagrantfile) if home_vagrantfile @config_loader.set(:root, root_vagrantfile) if root_vagrantfile - - # Make the initial call to get the "global" config. This is mostly - # only useful to get the list of machines that we are managing. - # Because of this, we ignore any warnings or errors. - @config_global, _ = @config_loader.load([:home, :root]) - - # Return the config - @config_global + @config_loader end # This defines a hook point where plugin action hooks that are registered @@ -379,7 +365,7 @@ module Vagrant end def vagrantfile - @vagrantfile ||= Vagrantfile.new(@config_loader, [:home, :root]) + @vagrantfile ||= Vagrantfile.new(config_loader, [:home, :root]) end # Unload the environment, running completion hooks. The environment @@ -413,7 +399,7 @@ module Vagrant # that shouldn't be valid anymore, but we respect it here by assuming # its old behavior. No need to deprecate this because I thin it is # fairly harmless. - host_klass = config_global.vagrant.host + host_klass = vagrantfile.config.vagrant.host host_klass = nil if host_klass == :detect begin @@ -447,7 +433,6 @@ module Vagrant { :action_runner => action_runner, :box_collection => boxes, - :global_config => config_global, :hook => method(:hook), :host => host, :gems_path => gems_path, diff --git a/lib/vagrant/vagrantfile.rb b/lib/vagrant/vagrantfile.rb index afaf33da6..f6a660efc 100644 --- a/lib/vagrant/vagrantfile.rb +++ b/lib/vagrant/vagrantfile.rb @@ -9,6 +9,10 @@ module Vagrant # loading the configuration of a specific machine/provider combo, # etc. class Vagrantfile + # This is the configuration loaded as-is given the loader and + # keys to #initialize. + attr_reader :config + # Initializes by loading a Vagrantfile. # # @param [Config::Loader] loader Configuration loader that should diff --git a/test/unit/vagrant/vagrantfile_test.rb b/test/unit/vagrant/vagrantfile_test.rb index 5531605d5..623b2c208 100644 --- a/test/unit/vagrant/vagrantfile_test.rb +++ b/test/unit/vagrant/vagrantfile_test.rb @@ -13,6 +13,24 @@ describe Vagrant::Vagrantfile do subject { described_class.new(loader, keys) } + describe "#config" do + before do + keys << :test + end + + def configure(&block) + loader.set(:test, [["2", block]]) + end + + it "exposes the global configuration" do + configure do |config| + config.vm.box = "what" + end + + expect(subject.config.vm.box).to eq("what") + end + end + describe "#machine_config" do let(:iso_env) { isolated_environment } let(:boxes) { Vagrant::BoxCollection.new(iso_env.boxes_dir) } From e2403c093d4f230ec5fa36bb2463e221772369a5 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 6 Feb 2014 20:17:24 -0800 Subject: [PATCH 04/12] core: Vagrantfile class now adheres to box_version --- lib/vagrant/vagrantfile.rb | 2 +- test/unit/vagrant/vagrantfile_test.rb | 36 ++++++++++++++++++++++++--- 2 files changed, 33 insertions(+), 5 deletions(-) diff --git a/lib/vagrant/vagrantfile.rb b/lib/vagrant/vagrantfile.rb index f6a660efc..52b5daf85 100644 --- a/lib/vagrant/vagrantfile.rb +++ b/lib/vagrant/vagrantfile.rb @@ -80,7 +80,7 @@ module Vagrant # Load the box Vagrantfile, if there is one if config.vm.box - box = boxes.find(config.vm.box, box_formats) + box = boxes.find(config.vm.box, box_formats, config.vm.box_version) if box box_vagrantfile = find_vagrantfile(box.directory) if box_vagrantfile diff --git a/test/unit/vagrant/vagrantfile_test.rb b/test/unit/vagrant/vagrantfile_test.rb index 623b2c208..3aa947046 100644 --- a/test/unit/vagrant/vagrantfile_test.rb +++ b/test/unit/vagrant/vagrantfile_test.rb @@ -94,7 +94,7 @@ describe Vagrant::Vagrantfile do config.vm.box = "base" end - iso_env.box2("base", :foo, vagrantfile: <<-VF) + iso_env.box3("base", "1.0", :foo, vagrantfile: <<-VF) Vagrant.configure("2") do |config| config.ssh.port = 123 end @@ -107,6 +107,34 @@ describe Vagrant::Vagrantfile do expect(box.name).to eq("base") end + it "configures with the proper box version" do + register_provider("foo") + + configure do |config| + config.vm.box = "base" + config.vm.box_version = "~> 1.2" + end + + iso_env.box3("base", "1.0", :foo, vagrantfile: <<-VF) + Vagrant.configure("2") do |config| + config.ssh.port = 123 + end + VF + + iso_env.box3("base", "1.3", :foo, vagrantfile: <<-VF) + Vagrant.configure("2") do |config| + config.ssh.port = 245 + end + VF + + config, _, _, box = subject.machine_config(:default, :foo, boxes) + expect(config.vm.box).to eq("base") + expect(config.ssh.port).to eq(245) + expect(box).to_not be_nil + expect(box.name).to eq("base") + expect(box.version).to eq("1.3") + end + it "configures with box config of other supported formats" do register_provider("foo", nil, box_format: "bar") @@ -114,7 +142,7 @@ describe Vagrant::Vagrantfile do config.vm.box = "base" end - iso_env.box2("base", :bar, vagrantfile: <<-VF) + iso_env.box3("base", "1.0", :bar, vagrantfile: <<-VF) Vagrant.configure("2") do |config| config.ssh.port = 123 end @@ -160,13 +188,13 @@ describe Vagrant::Vagrantfile do end end - iso_env.box2("base", :foo, vagrantfile: <<-VF) + iso_env.box3("base", "1.0", :foo, vagrantfile: <<-VF) Vagrant.configure("2") do |config| config.ssh.port = 123 end VF - iso_env.box2("foobox", :foo, vagrantfile: <<-VF) + iso_env.box3("foobox", "1.0", :foo, vagrantfile: <<-VF) Vagrant.configure("2") do |config| config.ssh.port = 234 end From cdf1d7f3182b76e2a36e60234b09a24977179b83 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 6 Feb 2014 20:22:15 -0800 Subject: [PATCH 05/12] core: get rid of all uses of config_global --- plugins/commands/box/command/repackage.rb | 2 +- plugins/commands/package/command.rb | 2 +- .../virtualbox/action/prepare_nfs_valid_ids_test.rb | 2 +- test/unit/vagrant/environment_test.rb | 6 +++--- test/unit/vagrant/machine_test.rb | 2 +- 5 files changed, 7 insertions(+), 7 deletions(-) diff --git a/plugins/commands/box/command/repackage.rb b/plugins/commands/box/command/repackage.rb index fa986119a..ffff2b0ff 100644 --- a/plugins/commands/box/command/repackage.rb +++ b/plugins/commands/box/command/repackage.rb @@ -31,7 +31,7 @@ module VagrantPlugins raise Vagrant::Errors::BoxNotFound, :name => box_name, :provider => box_provider if !box # Repackage the box - output_name = @env.config_global.package.name || "package.box" + output_name = @env.vagrantfile.config.package.name || "package.box" output_path = Pathname.new(File.expand_path(output_name, FileUtils.pwd)) box.repackage(output_path) diff --git a/plugins/commands/package/command.rb b/plugins/commands/package/command.rb index 255c6f95e..36e3f8cca 100644 --- a/plugins/commands/package/command.rb +++ b/plugins/commands/package/command.rb @@ -59,7 +59,7 @@ module VagrantPlugins vm = Vagrant::Machine.new( options[:base], :virtualbox, provider[0], nil, provider[1], - @env.config_global, + @env.vagrantfile.config, nil, nil, @env, true) @logger.debug("Packaging base VM: #{vm.name}") 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 index 374f07132..bd617381b 100644 --- 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 @@ -15,7 +15,7 @@ describe VagrantPlugins::ProviderVirtualBox::Action::PrepareNFSValidIds do provider_cls, provider_config, provider_options, - environment.config_global, + environment.vagrantfile.config, Pathname('data_dir'), double('box'), environment diff --git a/test/unit/vagrant/environment_test.rb b/test/unit/vagrant/environment_test.rb index 2e7758dcd..4478b5abf 100644 --- a/test/unit/vagrant/environment_test.rb +++ b/test/unit/vagrant/environment_test.rb @@ -833,7 +833,7 @@ VF end env = environment.create_vagrant_env - env.config_global.ssh.port.should == 200 + env.vagrantfile.config.ssh.port.should == 200 end it "should load from a custom Vagrantfile" do @@ -846,7 +846,7 @@ VF end env = environment.create_vagrant_env(:vagrantfile_name => "non_standard_name") - env.config_global.ssh.port.should == 200 + env.vagrantfile.config.ssh.port.should == 200 end it "should load from a custom Vagrantfile specified by env var" do @@ -862,7 +862,7 @@ VF environment.create_vagrant_env end - env.config_global.ssh.port.should == 400 + env.vagrantfile.config.ssh.port.should == 400 end end diff --git a/test/unit/vagrant/machine_test.rb b/test/unit/vagrant/machine_test.rb index 0be5455be..bcb3f8a0c 100644 --- a/test/unit/vagrant/machine_test.rb +++ b/test/unit/vagrant/machine_test.rb @@ -21,7 +21,7 @@ describe Vagrant::Machine do let(:provider_name) { :test } let(:provider_options) { {} } let(:box) { Object.new } - let(:config) { env.config_global } + let(:config) { env.vagrantfile.config } let(:data_dir) { Pathname.new(Dir.mktmpdir("vagrant")) } let(:env) do # We need to create a Vagrantfile so that this test environment From d40dc919d891938dbcd99a4f37702a041242af24 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 6 Feb 2014 20:50:51 -0800 Subject: [PATCH 06/12] core: plugin tests no longer obliterate manager --- test/unit/vagrant/plugin/v2/plugin_test.rb | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/test/unit/vagrant/plugin/v2/plugin_test.rb b/test/unit/vagrant/plugin/v2/plugin_test.rb index 226a0f875..3341bf87d 100644 --- a/test/unit/vagrant/plugin/v2/plugin_test.rb +++ b/test/unit/vagrant/plugin/v2/plugin_test.rb @@ -1,10 +1,8 @@ require File.expand_path("../../../../base", __FILE__) describe Vagrant::Plugin::V2::Plugin do - after(:each) do - # We want to make sure that the registered plugins remains empty - # after each test. - described_class.manager.reset! + before do + described_class.stub(manager: Vagrant::Plugin::V2::Manager.new) end it "should be able to set and get the name" do From 506e1a433f07172088883ecfb5ea137f5f661e22 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 6 Feb 2014 21:04:59 -0800 Subject: [PATCH 07/12] core: Vagrantfile#machine_config returns more info now --- lib/vagrant/environment.rb | 19 +++++++--------- lib/vagrant/vagrantfile.rb | 13 +++++++++-- test/unit/vagrant/vagrantfile_test.rb | 31 +++++++++++++++++++-------- 3 files changed, 41 insertions(+), 22 deletions(-) diff --git a/lib/vagrant/environment.rb b/lib/vagrant/environment.rb index 07a144445..d8330b955 100644 --- a/lib/vagrant/environment.rb +++ b/lib/vagrant/environment.rb @@ -300,14 +300,15 @@ module Vagrant end @logger.info("Uncached load of machine.") - provider_plugin = Vagrant.plugin("2").manager.providers[provider] - if !provider_plugin - raise Errors::ProviderNotFound, :machine => name, :provider => provider - end - # Extra the provider class and options from the plugin data - provider_cls = provider_plugin[0] - provider_options = provider_plugin[1] + # Load the actual configuration for the machine + results = vagrantfile.machine_config(name, provider, boxes) + box = results[:box] + config = results[:config] + config_errors = results[:config_errors] + config_warnings = results[:config_warnings] + provider_cls = results[:provider_cls] + provider_options = results[:provider_options] # Determine the machine data directory and pass it to the machine. # XXX: Permissions error here. @@ -315,10 +316,6 @@ module Vagrant "machines/#{name}/#{provider}") FileUtils.mkdir_p(machine_data_path) - # Load the actual configuration for the machine - config, config_warnings, config_errors, box = - vagrantfile.machine_config(name, provider, boxes) - # If there were warnings or errors we want to output them if !config_warnings.empty? || !config_errors.empty? # The color of the output depends on whether we have warnings diff --git a/lib/vagrant/vagrantfile.rb b/lib/vagrant/vagrantfile.rb index 52b5daf85..68348abb0 100644 --- a/lib/vagrant/vagrantfile.rb +++ b/lib/vagrant/vagrantfile.rb @@ -58,7 +58,9 @@ module Vagrant :machine => name, :provider => provider end - box_formats = provider_plugin[1][:box_format] || provider + provider_cls = provider_plugin[0] + provider_options = provider_plugin[1] + box_formats = provider_options[:box_format] || provider # Add the sub-machine configuration to the loader and keys vm_config_key = "#{object_id}_machine_#{name}" @@ -115,7 +117,14 @@ module Vagrant # Load the box and provider overrides load_box_proc.call - return config, config_warnings, config_errors, box + return { + box: box, + provider_cls: provider_cls, + provider_options: provider_options, + config: config, + config_warnings: config_warnings, + config_errors: config_errors, + } end # Returns a list of the machines that are defined within this diff --git a/test/unit/vagrant/vagrantfile_test.rb b/test/unit/vagrant/vagrantfile_test.rb index 3aa947046..22b6bf9b9 100644 --- a/test/unit/vagrant/vagrantfile_test.rb +++ b/test/unit/vagrant/vagrantfile_test.rb @@ -59,15 +59,18 @@ describe Vagrant::Vagrantfile do end it "should return a basic configured machine" do - register_provider("foo") + provider_cls = register_provider("foo") configure do |config| config.vm.box = "foo" end - config, _, _, box = subject.machine_config(:default, :foo, boxes) + results = subject.machine_config(:default, :foo, boxes) + box = results[:box] + config = results[:config] expect(config.vm.box).to eq("foo") expect(box).to be_nil + expect(results[:provider_cls]).to equal(provider_cls) end it "configures with sub-machine config" do @@ -82,7 +85,8 @@ describe Vagrant::Vagrantfile do end end - config, _ = subject.machine_config(:foo, :foo, boxes) + results = subject.machine_config(:foo, :foo, boxes) + config = results[:config] expect(config.vm.box).to eq("base") expect(config.ssh.port).to eq(100) end @@ -100,7 +104,9 @@ describe Vagrant::Vagrantfile do end VF - config, _, _, box = subject.machine_config(:default, :foo, boxes) + results = subject.machine_config(:default, :foo, boxes) + box = results[:box] + config = results[:config] expect(config.vm.box).to eq("base") expect(config.ssh.port).to eq(123) expect(box).to_not be_nil @@ -127,7 +133,9 @@ describe Vagrant::Vagrantfile do end VF - config, _, _, box = subject.machine_config(:default, :foo, boxes) + results = subject.machine_config(:default, :foo, boxes) + box = results[:box] + config = results[:config] expect(config.vm.box).to eq("base") expect(config.ssh.port).to eq(245) expect(box).to_not be_nil @@ -148,7 +156,8 @@ describe Vagrant::Vagrantfile do end VF - config, _ = subject.machine_config(:default, :foo, boxes) + results = subject.machine_config(:default, :foo, boxes) + config = results[:config] expect(config.vm.box).to eq("base") expect(config.ssh.port).to eq(123) end @@ -167,12 +176,14 @@ describe Vagrant::Vagrantfile do end # Test with the override - config, _ = subject.machine_config(:default, :foo, boxes) + results = subject.machine_config(:default, :foo, boxes) + config = results[:config] expect(config.vm.box).to eq("base") expect(config.ssh.port).to eq(100) # Test without the override - config, _ = subject.machine_config(:default, :bar, boxes) + results = subject.machine_config(:default, :bar, boxes) + config = results[:config] expect(config.vm.box).to eq("base") expect(config.ssh.port).to eq(1) end @@ -200,7 +211,9 @@ describe Vagrant::Vagrantfile do end VF - config, _, _, box = subject.machine_config(:default, :foo, boxes) + results = subject.machine_config(:default, :foo, boxes) + config = results[:config] + box = results[:box] expect(config.vm.box).to eq("foobox") expect(config.ssh.port).to eq(234) expect(box).to_not be_nil From 3533256cd71a61ee5ad2f325d4319bb16665fbb3 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 6 Feb 2014 21:31:11 -0800 Subject: [PATCH 08/12] core: Vagrantfile can create machines, Environment#machine uses that --- lib/vagrant/environment.rb | 33 +------- lib/vagrant/machine.rb | 2 +- lib/vagrant/vagrantfile.rb | 62 +++++++++++++++ test/unit/vagrant/vagrantfile_test.rb | 110 ++++++++++++++++---------- 4 files changed, 134 insertions(+), 73 deletions(-) diff --git a/lib/vagrant/environment.rb b/lib/vagrant/environment.rb index d8330b955..b8eae97fe 100644 --- a/lib/vagrant/environment.rb +++ b/lib/vagrant/environment.rb @@ -301,45 +301,16 @@ module Vagrant @logger.info("Uncached load of machine.") - # Load the actual configuration for the machine - results = vagrantfile.machine_config(name, provider, boxes) - box = results[:box] - config = results[:config] - config_errors = results[:config_errors] - config_warnings = results[:config_warnings] - provider_cls = results[:provider_cls] - provider_options = results[:provider_options] - # Determine the machine data directory and pass it to the machine. # XXX: Permissions error here. machine_data_path = @local_data_path.join( "machines/#{name}/#{provider}") FileUtils.mkdir_p(machine_data_path) - # If there were warnings or errors we want to output them - if !config_warnings.empty? || !config_errors.empty? - # The color of the output depends on whether we have warnings - # or errors... - level = config_errors.empty? ? :warn : :error - output = Util::TemplateRenderer.render( - "config/messages", - :warnings => config_warnings, - :errors => config_errors).chomp - @ui.send(level, I18n.t("vagrant.general.config_upgrade_messages", - name: name, - :output => output)) - - # If we had errors, then we bail - raise Errors::ConfigUpgradeErrors if !config_errors.empty? - end - - # Get the provider configuration from the final loaded configuration - provider_config = config.vm.get_provider_config(provider) - # Create the machine and cache it for future calls. This will also # return the machine from this method. - @machines[cache_key] = Machine.new(name, provider, provider_cls, provider_config, - provider_options, config, machine_data_path, box, self) + @machines[cache_key] = vagrantfile.machine( + name, provider, boxes, machine_data_path, self) end # This returns a list of the configured machines for this environment. diff --git a/lib/vagrant/machine.rb b/lib/vagrant/machine.rb index e5c9651fb..dcfbd821f 100644 --- a/lib/vagrant/machine.rb +++ b/lib/vagrant/machine.rb @@ -34,7 +34,7 @@ module Vagrant # Name of the machine. This is assigned by the Vagrantfile. # - # @return [String] + # @return [Symbol] attr_reader :name # The provider backing this machine. diff --git a/lib/vagrant/vagrantfile.rb b/lib/vagrant/vagrantfile.rb index 68348abb0..af4d0fa9a 100644 --- a/lib/vagrant/vagrantfile.rb +++ b/lib/vagrant/vagrantfile.rb @@ -1,3 +1,5 @@ +require "vagrant/util/template_renderer" + module Vagrant # This class provides a way to load and access the contents # of a Vagrantfile. @@ -26,6 +28,54 @@ module Vagrant @config, _ = loader.load(keys) end + # Returns a {Machine} for the given name and provider that + # is represented by this Vagrantfile. + # + # @param [Symbol] name Name of the machine. + # @param [Symbol] provider The provider the machine should + # be backed by (required for provider overrides). + # @param [BoxCollection] boxes BoxCollection to look up the + # box Vagrantfile. + # @param [Pathname] data_path Path where local machine data + # can be stored. + # @param [Environment] env The environment running this machine + # @return [Machine] + def machine(name, provider, boxes, data_path, env) + # Load the configuration for the machine + results = machine_config(name, provider, boxes) + box = results[:box] + config = results[:config] + config_errors = results[:config_errors] + config_warnings = results[:config_warnings] + provider_cls = results[:provider_cls] + provider_options = results[:provider_options] + + # If there were warnings or errors we want to output them + if !config_warnings.empty? || !config_errors.empty? + # The color of the output depends on whether we have warnings + # or errors... + level = config_errors.empty? ? :warn : :error + output = Util::TemplateRenderer.render( + "config/messages", + :warnings => config_warnings, + :errors => config_errors).chomp + env.ui.send(level, I18n.t("vagrant.general.config_upgrade_messages", + name: name, + output: output)) + + # If we had errors, then we bail + raise Errors::ConfigUpgradeErrors if !config_errors.empty? + end + + # Get the provider configuration from the final loaded configuration + provider_config = config.vm.get_provider_config(provider) + + # Create the machine and cache it for future calls. This will also + # return the machine from this method. + return Machine.new(name, provider, provider_cls, provider_config, + provider_options, config, data_path, box, env) + end + # Returns the configuration for a single machine. # # When loading a box Vagrantfile, it will be prepended to the @@ -38,11 +88,23 @@ module Vagrant # - sub-machine # - provider # + # The return value is a hash with the following keys (symbols) + # and values: + # + # - box: the {Box} backing the machine + # - config: the actual configuration + # - config_errors: list of errors, if any + # - config_warnings: list of warnings, if any + # - provider_cls: class of the provider backing the machine + # - provider_options: options for the provider + # # @param [Symbol] name Name of the machine. # @param [Symbol] provider The provider the machine should # be backed by (required for provider overrides). # @param [BoxCollection] boxes BoxCollection to look up the # box Vagrantfile. + # @return [Hash] Various configuration parameters for a + # machine. See the main documentation body for more info. def machine_config(name, provider, boxes) keys = @keys.dup diff --git a/test/unit/vagrant/vagrantfile_test.rb b/test/unit/vagrant/vagrantfile_test.rb index 22b6bf9b9..1f727afce 100644 --- a/test/unit/vagrant/vagrantfile_test.rb +++ b/test/unit/vagrant/vagrantfile_test.rb @@ -1,5 +1,8 @@ require File.expand_path("../../base", __FILE__) +require "pathname" +require "tmpdir" + require "vagrant/vagrantfile" describe Vagrant::Vagrantfile do @@ -13,15 +16,30 @@ describe Vagrant::Vagrantfile do subject { described_class.new(loader, keys) } + before do + keys << :test + end + + def configure(&block) + loader.set(:test, [["2", block]]) + end + + # A helper to register a provider for use in tests. + def register_provider(name, config_class=nil, options=nil) + provider_cls = Class.new(Vagrant.plugin("2", :provider)) + + register_plugin("2") do |p| + p.provider(name, options) { provider_cls } + + if config_class + p.config(name, :provider) { config_class } + end + end + + provider_cls + end + describe "#config" do - before do - keys << :test - end - - def configure(&block) - loader.set(:test, [["2", block]]) - end - it "exposes the global configuration" do configure do |config| config.vm.box = "what" @@ -31,33 +49,59 @@ describe Vagrant::Vagrantfile do end end - describe "#machine_config" do - let(:iso_env) { isolated_environment } + describe "#machine" do let(:boxes) { Vagrant::BoxCollection.new(iso_env.boxes_dir) } + let(:data_path) { Pathname.new(Dir.mktmpdir) } + let(:env) { iso_env.create_vagrant_env } + let(:iso_env) { isolated_environment } + + subject { super().machine(:default, :foo, boxes, data_path, env) } before do - keys << :test - end + @foo_config_cls = Class.new(Vagrant.plugin("2", "config")) do + attr_accessor :value + end - def configure(&block) - loader.set(:test, [["2", block]]) - end + @provider_cls = register_provider("foo", @foo_config_cls) - # A helper to register a provider for use in tests. - def register_provider(name, config_class=nil, options=nil) - provider_cls = Class.new(Vagrant.plugin("2", :provider)) - - register_plugin("2") do |p| - p.provider(name, options) { provider_cls } - - if config_class - p.config(name, :provider) { config_class } + configure do |config| + config.vm.box = "foo" + config.vm.provider "foo" do |p| + p.value = "rawr" end end - provider_cls + iso_env.box3("foo", "1.0", :foo, vagrantfile: <<-VF) + Vagrant.configure("2") do |config| + config.ssh.port = 123 + end + VF end + its(:data_dir) { should eq(data_path) } + its(:env) { should equal(env) } + its(:name) { should eq(:default) } + its(:provider) { should be_kind_of(@provider_cls) } + its(:provider_name) { should eq(:foo) } + + it "has the proper box" do + expect(subject.box.name).to eq("foo") + end + + it "has the valid configuration" do + expect(subject.config.vm.box).to eq("foo") + end + + it "loads the provider-specific configuration" do + expect(subject.provider_config).to be_kind_of(@foo_config_cls) + expect(subject.provider_config.value).to eq("rawr") + end + end + + describe "#machine_config" do + let(:iso_env) { isolated_environment } + let(:boxes) { Vagrant::BoxCollection.new(iso_env.boxes_dir) } + it "should return a basic configured machine" do provider_cls = register_provider("foo") @@ -232,14 +276,6 @@ describe Vagrant::Vagrantfile do end describe "#machine_names" do - before do - keys << :test - end - - def configure(&block) - loader.set(:test, [["2", block]]) - end - it "returns the default name when single-VM" do configure { |config| } @@ -258,14 +294,6 @@ describe Vagrant::Vagrantfile do end describe "#primary_machine_name" do - before do - keys << :test - end - - def configure(&block) - loader.set(:test, [["2", block]]) - end - it "returns the default name when single-VM" do configure { |config| } From 8af67cf24129533fc492e433dafd75756f9f82d8 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 6 Feb 2014 21:33:28 -0800 Subject: [PATCH 09/12] core: update docs for Vagrant::Environment --- lib/vagrant/environment.rb | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/lib/vagrant/environment.rb b/lib/vagrant/environment.rb index b8eae97fe..0ea1155e6 100644 --- a/lib/vagrant/environment.rb +++ b/lib/vagrant/environment.rb @@ -11,9 +11,10 @@ require 'vagrant/util/platform' require "vagrant/vagrantfile" module Vagrant - # Represents a single Vagrant environment. A "Vagrant environment" is - # defined as basically a folder with a "Vagrantfile." This class allows - # access to the VMs, CLI, etc. all in the scope of this environment. + # A "Vagrant environment" represents a configuration of how Vagrant + # should behave: data directories, working directory, UI output, + # etc. In day-to-day usage, every `vagrant` invocation typically + # leads to a single Vagrant environment. class Environment # This is the current version that this version of Vagrant is # compatible with in the home directory. From 66f722ef68fb970f325b87018344e95fbabcaf68 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 6 Feb 2014 21:36:57 -0800 Subject: [PATCH 10/12] core: Alphabetize functions and comment better in Environment --- lib/vagrant/environment.rb | 234 +++++++++++++++++++------------------ 1 file changed, 122 insertions(+), 112 deletions(-) diff --git a/lib/vagrant/environment.rb b/lib/vagrant/environment.rb index 0ea1155e6..d4adac195 100644 --- a/lib/vagrant/environment.rb +++ b/lib/vagrant/environment.rb @@ -152,9 +152,24 @@ module Vagrant "#<#{self.class}: #{@cwd}>" end - #--------------------------------------------------------------- - # Helpers - #--------------------------------------------------------------- + # Action runner for executing actions in the context of this environment. + # + # @return [Action::Runner] + def action_runner + @action_runner ||= Action::Runner.new do + { + :action_runner => action_runner, + :box_collection => boxes, + :hook => method(:hook), + :host => host, + :gems_path => gems_path, + :home_path => home_path, + :root_path => root_path, + :tmp_path => tmp_path, + :ui => @ui + } + end + end # Returns a list of machines that this environment is currently # managing that physically have been created. @@ -220,6 +235,15 @@ module Vagrant end end + # Makes a call to the CLI with the given arguments as if they + # came from the real command line (sometimes they do!). An example: + # + # env.cli("package", "--vagrantfile", "Vagrantfile") + # + def cli(*args) + CLI.new(args.flatten, self).execute + end + # This returns the provider name for the default provider for this # environment. The provider returned is currently hardcoded to "virtualbox" # but one day should be a detected valid, best-case provider for this @@ -273,6 +297,75 @@ module Vagrant opts.delete(:runner).run(opts.delete(:callable), opts) end + # Returns the host object associated with this environment. + # + # @return [Class] + def host + return @host if defined?(@host) + + # Determine the host class to use. ":detect" is an old Vagrant config + # that shouldn't be valid anymore, but we respect it here by assuming + # its old behavior. No need to deprecate this because I thin it is + # fairly harmless. + host_klass = vagrantfile.config.vagrant.host + host_klass = nil if host_klass == :detect + + begin + @host = Host.new( + host_klass, + Vagrant.plugin("2").manager.hosts, + Vagrant.plugin("2").manager.host_capabilities, + self) + rescue Errors::CapabilityHostNotDetected + # If the auto-detect failed, then we create a brand new host + # with no capabilities and use that. This should almost never happen + # since Vagrant works on most host OS's now, so this is a "slow path" + klass = Class.new(Vagrant.plugin("2", :host)) do + def detect?(env); true; end + end + + hosts = { generic: [klass, nil] } + host_caps = {} + + @host = Host.new(:generic, hosts, host_caps, self) + rescue Errors::CapabilityHostExplicitNotDetected => e + raise Errors::HostExplicitNotDetected, e.extra_data + end + end + + # This returns the path which Vagrant uses to determine the location + # of the file lock. This is specific to each operating system. + def lock_path + @lock_path || tmp_path.join("vagrant.lock") + end + + # This locks Vagrant for the duration of the block passed to this + # method. During this time, any other environment which attempts + # to lock which points to the same lock file will fail. + def lock + # This allows multiple locks in the same process to be nested + return yield if @lock_acquired + + File.open(lock_path, "w+") do |f| + # The file locking fails only if it returns "false." If it + # succeeds it returns a 0, so we must explicitly check for + # the proper error case. + raise Errors::EnvironmentLockedError if f.flock(File::LOCK_EX | File::LOCK_NB) === false + + begin + # Mark that we have a lock + @lock_acquired = true + + yield + ensure + # We need to make sure that no matter what this is always + # reset to false so we don't think we have a lock when we + # actually don't. + @lock_acquired = false + end + end + end + # This returns a machine with the proper provider for this environment. # The machine named by `name` must be in this environment. # @@ -333,86 +426,6 @@ module Vagrant vagrantfile.primary_machine_name end - def vagrantfile - @vagrantfile ||= Vagrantfile.new(config_loader, [:home, :root]) - end - - # Unload the environment, running completion hooks. The environment - # should not be used after this (but CAN be, technically). It is - # recommended to always immediately set the variable to `nil` after - # running this so you can't accidentally run any more methods. Example: - # - # env.unload - # env = nil - # - def unload - hook(:environment_unload) - end - - # Makes a call to the CLI with the given arguments as if they - # came from the real command line (sometimes they do!). An example: - # - # env.cli("package", "--vagrantfile", "Vagrantfile") - # - def cli(*args) - CLI.new(args.flatten, self).execute - end - - # Returns the host object associated with this environment. - # - # @return [Class] - def host - return @host if defined?(@host) - - # Determine the host class to use. ":detect" is an old Vagrant config - # that shouldn't be valid anymore, but we respect it here by assuming - # its old behavior. No need to deprecate this because I thin it is - # fairly harmless. - host_klass = vagrantfile.config.vagrant.host - host_klass = nil if host_klass == :detect - - begin - @host = Host.new( - host_klass, - Vagrant.plugin("2").manager.hosts, - Vagrant.plugin("2").manager.host_capabilities, - self) - rescue Errors::CapabilityHostNotDetected - # If the auto-detect failed, then we create a brand new host - # with no capabilities and use that. This should almost never happen - # since Vagrant works on most host OS's now, so this is a "slow path" - klass = Class.new(Vagrant.plugin("2", :host)) do - def detect?(env); true; end - end - - hosts = { generic: [klass, nil] } - host_caps = {} - - @host = Host.new(:generic, hosts, host_caps, self) - rescue Errors::CapabilityHostExplicitNotDetected => e - raise Errors::HostExplicitNotDetected, e.extra_data - end - end - - # Action runner for executing actions in the context of this environment. - # - # @return [Action::Runner] - def action_runner - @action_runner ||= Action::Runner.new do - { - :action_runner => action_runner, - :box_collection => boxes, - :hook => method(:hook), - :host => host, - :gems_path => gems_path, - :home_path => home_path, - :root_path => root_path, - :tmp_path => tmp_path, - :ui => @ui - } - end - end - # The root path is the path where the top-most (loaded last) # Vagrantfile resides. It can be considered the project root for # this environment. @@ -433,37 +446,34 @@ module Vagrant @root_path = root_finder.call(cwd) end - # This returns the path which Vagrant uses to determine the location - # of the file lock. This is specific to each operating system. - def lock_path - @lock_path || tmp_path.join("vagrant.lock") + # Unload the environment, running completion hooks. The environment + # should not be used after this (but CAN be, technically). It is + # recommended to always immediately set the variable to `nil` after + # running this so you can't accidentally run any more methods. Example: + # + # env.unload + # env = nil + # + def unload + hook(:environment_unload) end - # This locks Vagrant for the duration of the block passed to this - # method. During this time, any other environment which attempts - # to lock which points to the same lock file will fail. - def lock - # This allows multiple locks in the same process to be nested - return yield if @lock_acquired - - File.open(lock_path, "w+") do |f| - # The file locking fails only if it returns "false." If it - # succeeds it returns a 0, so we must explicitly check for - # the proper error case. - raise Errors::EnvironmentLockedError if f.flock(File::LOCK_EX | File::LOCK_NB) === false - - begin - # Mark that we have a lock - @lock_acquired = true - - yield - ensure - # We need to make sure that no matter what this is always - # reset to false so we don't think we have a lock when we - # actually don't. - @lock_acquired = false - end - end + # Represents the default Vagrantfile, or the Vagrantfile that is + # in the working directory or a parent of the working directory + # of this environment. + # + # The existence of this function is primarily a convenience. There + # is nothing stopping you from instantiating your own {Vagrantfile} + # and loading machines in any way you see fit. Typical behavior of + # Vagrant, however, loads this Vagrantfile. + # + # This Vagrantfile is comprised of two major sources: the Vagrantfile + # in the user's home directory as well as the "root" Vagrantfile or + # the Vagrantfile in the working directory (or parent). + # + # @return [Vagrantfile] + def vagrantfile + @vagrantfile ||= Vagrantfile.new(config_loader, [:home, :root]) end #--------------------------------------------------------------- From 8e9ceeaf5c0a87eefbf991c3145008aeeb50c6c0 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Fri, 7 Feb 2014 09:16:55 -0800 Subject: [PATCH 11/12] core: Machine has reference to Vagrantfile --- lib/vagrant/machine.rb | 8 ++++- lib/vagrant/vagrantfile.rb | 2 +- plugins/commands/package/command.rb | 2 +- .../action/prepare_nfs_valid_ids_test.rb | 31 ++++++++----------- test/unit/vagrant/vagrantfile_test.rb | 4 ++- 5 files changed, 25 insertions(+), 22 deletions(-) diff --git a/lib/vagrant/machine.rb b/lib/vagrant/machine.rb index dcfbd821f..e0728aafb 100644 --- a/lib/vagrant/machine.rb +++ b/lib/vagrant/machine.rb @@ -62,6 +62,11 @@ module Vagrant # @return [UI] attr_reader :ui + # The Vagrantfile that this machine is attached to. + # + # @return [Vagrantfile] + attr_reader :vagrantfile + # Initialize a new machine. # # @param [String] name Name of the virtual machine. @@ -77,7 +82,7 @@ module Vagrant # @param [Box] box The box that is backing this virtual machine. # @param [Environment] env The environment that this machine is a # part of. - def initialize(name, provider_name, provider_cls, provider_config, provider_options, config, data_dir, box, env, base=false) + def initialize(name, provider_name, provider_cls, provider_config, provider_options, config, data_dir, box, env, vagrantfile, base=false) @logger = Log4r::Logger.new("vagrant::machine") @logger.info("Initializing machine: #{name}") @logger.info(" - Provider: #{provider_cls}") @@ -88,6 +93,7 @@ module Vagrant @config = config @data_dir = data_dir @env = env + @vagrantfile = vagrantfile @guest = Guest.new( self, Vagrant.plugin("2").manager.guests, diff --git a/lib/vagrant/vagrantfile.rb b/lib/vagrant/vagrantfile.rb index af4d0fa9a..fde3769cf 100644 --- a/lib/vagrant/vagrantfile.rb +++ b/lib/vagrant/vagrantfile.rb @@ -73,7 +73,7 @@ module Vagrant # Create the machine and cache it for future calls. This will also # return the machine from this method. return Machine.new(name, provider, provider_cls, provider_config, - provider_options, config, data_path, box, env) + provider_options, config, data_path, box, env, self) end # Returns the configuration for a single machine. diff --git a/plugins/commands/package/command.rb b/plugins/commands/package/command.rb index 36e3f8cca..92541be60 100644 --- a/plugins/commands/package/command.rb +++ b/plugins/commands/package/command.rb @@ -61,7 +61,7 @@ module VagrantPlugins :virtualbox, provider[0], nil, provider[1], @env.vagrantfile.config, nil, nil, - @env, true) + @env, @env.vagrantfile, true) @logger.debug("Packaging base VM: #{vm.name}") package_vm(vm, options) end 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 index bd617381b..23a2328a6 100644 --- 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 @@ -1,30 +1,25 @@ require_relative "../base" describe VagrantPlugins::ProviderVirtualBox::Action::PrepareNFSValidIds do + include_context "unit" 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] + let(:iso_env) do + # We have to create a Vagrantfile so there is a root path + env = isolated_environment + env.vagrantfile("") + env.create_vagrant_env + end - Vagrant::Machine.new( - 'test_machine', - provider, - provider_cls, - provider_config, - provider_options, - environment.vagrantfile.config, - Pathname('data_dir'), - double('box'), - environment - ) - } + let(:machine) do + iso_env.machine(iso_env.machine_names[0], :dummy).tap do |m| + m.provider.stub(driver: driver) + end + end let(:env) {{ machine: machine }} let(:app) { lambda { |*args| }} - let(:driver) { env[:machine].provider.driver } + let(:driver) { double("driver") } subject { described_class.new(app, env) } diff --git a/test/unit/vagrant/vagrantfile_test.rb b/test/unit/vagrant/vagrantfile_test.rb index 1f727afce..7fbaa085e 100644 --- a/test/unit/vagrant/vagrantfile_test.rb +++ b/test/unit/vagrant/vagrantfile_test.rb @@ -54,8 +54,9 @@ describe Vagrant::Vagrantfile do let(:data_path) { Pathname.new(Dir.mktmpdir) } let(:env) { iso_env.create_vagrant_env } let(:iso_env) { isolated_environment } + let(:vagrantfile) { described_class.new(loader, keys) } - subject { super().machine(:default, :foo, boxes, data_path, env) } + subject { vagrantfile.machine(:default, :foo, boxes, data_path, env) } before do @foo_config_cls = Class.new(Vagrant.plugin("2", "config")) do @@ -83,6 +84,7 @@ describe Vagrant::Vagrantfile do its(:name) { should eq(:default) } its(:provider) { should be_kind_of(@provider_cls) } its(:provider_name) { should eq(:foo) } + its(:vagrantfile) { should equal(vagrantfile) } it "has the proper box" do expect(subject.box.name).to eq("foo") From 40cbfb95e31449f59886e73b8a0b3b2c12b4cb88 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Fri, 7 Feb 2014 09:24:09 -0800 Subject: [PATCH 12/12] core: fix Machine tests --- test/unit/vagrant/machine_test.rb | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/test/unit/vagrant/machine_test.rb b/test/unit/vagrant/machine_test.rb index bcb3f8a0c..8e48dd511 100644 --- a/test/unit/vagrant/machine_test.rb +++ b/test/unit/vagrant/machine_test.rb @@ -41,7 +41,8 @@ describe Vagrant::Machine do # Returns a new instance with the test data def new_instance described_class.new(name, provider_name, provider_cls, provider_config, - provider_options, config, data_dir, box, env) + provider_options, config, data_dir, box, + env, env.vagrantfile) end describe "initialization" do @@ -77,7 +78,8 @@ describe Vagrant::Machine do # Initialize a new machine and verify that we properly receive # the machine we expect. instance = described_class.new(name, provider_name, provider_cls, provider_config, - provider_options, config, data_dir, box, env) + provider_options, config, data_dir, box, + env, env.vagrantfile) received_machine.should eql(instance) end @@ -110,6 +112,12 @@ describe Vagrant::Machine do end end + it "should have the vagrantfile" do + provider_init_test do |machine| + expect(machine.vagrantfile).to equal(env.vagrantfile) + end + end + it "should have access to the ID" do # Stub this because #id= calls it. provider.stub(:machine_id_changed)