From ad758bf69a267c8586cee446cf599de2d7cead75 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 23 Oct 2014 15:32:54 -0700 Subject: [PATCH] core: prefer providers in the Vagrantfile [GH-3812] --- CHANGELOG.md | 5 +++ lib/vagrant/environment.rb | 19 +++++++++- plugins/kernel_v2/config/vm.rb | 15 ++++++++ test/unit/plugins/kernel_v2/config/vm_test.rb | 35 +++++++++++++++++++ test/unit/vagrant/environment_test.rb | 28 +++++++++++++++ 5 files changed, 101 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2fbac3d5d..4be1e0edf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,10 @@ ## 1.7.0 (unreleased) +FEATURES: + + - Default provider logic improved: Providers in `config.vm.provider` blocks + in your Vagrantfile now have higher priority than plugins. [GH-3812] + IMPROVEMENTS: - core: `has_plugin?` function now takes a second argument which is a diff --git a/lib/vagrant/environment.rb b/lib/vagrant/environment.rb index 571ae75ab..c4665a718 100644 --- a/lib/vagrant/environment.rb +++ b/lib/vagrant/environment.rb @@ -308,6 +308,13 @@ module Vagrant # that (the default behavior) return default if default && opts[:force_default] + # Get the list of providers within our configuration and assign + # a priority to each in the order they exist so that we try these first. + config = {} + vagrantfile.config.vm.__providers.each_with_index do |key, idx| + config[key] = idx + end + ordered = [] Vagrant.plugin("2").manager.providers.each do |key, data| impl = data[0] @@ -319,7 +326,17 @@ module Vagrant # Skip providers that can't be defaulted next if popts.has_key?(:defaultable) && !popts[:defaultable] - ordered << [popts[:priority], key, impl, popts] + # The priority is higher if it is in our config. Otherwise, it is + # the priority it set PLUS the length of the config to make sure it + # is never higher than the configuration keys. + priority = config[key] + priority ||= popts[:priority] + config.length + + # If we have config, we multiply by -1 so that the ordering is + # such that the smallest numbers win. + priority *= -1 if !config.empty? + + ordered << [priority, key, impl, popts] end # Order the providers by priority. Higher values are tried first. diff --git a/plugins/kernel_v2/config/vm.rb b/plugins/kernel_v2/config/vm.rb index ccebfdede..9ea27f788 100644 --- a/plugins/kernel_v2/config/vm.rb +++ b/plugins/kernel_v2/config/vm.rb @@ -63,6 +63,7 @@ module VagrantPlugins @__finalized = false @__networks = {} @__providers = {} + @__provider_order = [] @__provider_overrides = {} @__synced_folders = {} end @@ -112,6 +113,12 @@ module VagrantPlugins new_providers[key] += blocks end + # Merge the provider ordering. Anything defined in our CURRENT + # scope is before anything else. + other_order = other.instance_variable_get(:@__provider_order) + new_order = @__provider_order.dup + new_order = (new_order + other_order).uniq + # Merge the provider overrides by appending them... other_overrides = other.instance_variable_get(:@__provider_overrides) new_overrides = @__provider_overrides.dup @@ -162,6 +169,7 @@ module VagrantPlugins result.instance_variable_set(:@__defined_vm_keys, new_defined_vm_keys) result.instance_variable_set(:@__defined_vms, new_defined_vms) result.instance_variable_set(:@__providers, new_providers) + result.instance_variable_set(:@__provider_order, new_order) result.instance_variable_set(:@__provider_overrides, new_overrides) result.instance_variable_set(:@__synced_folders, new_folders) end @@ -255,6 +263,9 @@ module VagrantPlugins @__providers[name] ||= [] @__provider_overrides[name] ||= [] + # Add the provider to the ordering list + @__provider_order << name + if block_given? @__providers[name] << block if block_given? @@ -676,6 +687,10 @@ module VagrantPlugins errors end + + def __providers + @__provider_order + end end end end diff --git a/test/unit/plugins/kernel_v2/config/vm_test.rb b/test/unit/plugins/kernel_v2/config/vm_test.rb index bbe2a5979..fb4658920 100644 --- a/test/unit/plugins/kernel_v2/config/vm_test.rb +++ b/test/unit/plugins/kernel_v2/config/vm_test.rb @@ -262,6 +262,41 @@ describe VagrantPlugins::Kernel_V2::VMConfig do end end + describe "#provider and #__providers" do + it "returns the providers in order" do + subject.provider "foo" + subject.provider "bar" + subject.finalize! + + expect(subject.__providers).to eq([:foo, :bar]) + end + + describe "merging" do + it "prioritizes new orders in later configs" do + subject.provider "foo" + + other = described_class.new + other.provider "bar" + + merged = subject.merge(other) + + expect(merged.__providers).to eq([:foo, :bar]) + end + + it "prioritizes duplicates in new orders in later configs" do + subject.provider "foo" + + other = described_class.new + other.provider "bar" + other.provider "foo" + + merged = subject.merge(other) + + expect(merged.__providers).to eq([:foo, :bar]) + end + end + end + describe "#provider and #get_provider_config" do it "compiles the configurations for a provider" do subject.provider "virtualbox" do |vb| diff --git a/test/unit/vagrant/environment_test.rb b/test/unit/vagrant/environment_test.rb index a447ea9f1..0d8200762 100644 --- a/test/unit/vagrant/environment_test.rb +++ b/test/unit/vagrant/environment_test.rb @@ -807,6 +807,34 @@ VF expect(subject.default_provider).to eq(:virtualbox) end end + + it "is the provider in the Vagrantfile that is usable" do + subject.vagrantfile.config.vm.provider "foo" + subject.vagrantfile.config.vm.finalize! + + plugin_providers[:foo] = [provider_usable_class(true), { priority: 5 }] + plugin_providers[:bar] = [provider_usable_class(true), { priority: 7 }] + plugin_providers[:baz] = [provider_usable_class(true), { priority: 2 }] + plugin_providers[:boom] = [provider_usable_class(true), { priority: 3 }] + + with_temp_env("VAGRANT_DEFAULT_PROVIDER" => nil) do + expect(subject.default_provider).to eq(:foo) + end + end + + it "is the provider in the Vagrantfile that is usable" do + subject.vagrantfile.config.vm.provider "foo" + subject.vagrantfile.config.vm.finalize! + + plugin_providers[:foo] = [provider_usable_class(false), { priority: 5 }] + plugin_providers[:bar] = [provider_usable_class(true), { priority: 7 }] + plugin_providers[:baz] = [provider_usable_class(true), { priority: 2 }] + plugin_providers[:boom] = [provider_usable_class(true), { priority: 3 }] + + with_temp_env("VAGRANT_DEFAULT_PROVIDER" => nil) do + expect(subject.default_provider).to eq(:bar) + end + end end describe "local data path" do