core: prefer providers in the Vagrantfile [GH-3812]

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

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.

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

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|

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