Merge pull request #11295 from briancain/fixup/provision-with-name-shell

Fixes #11287: Set top level provisioner name if set in provisioner config

lib/vagrant/action/builtin/mixin_provisioners.rb

@@ -25,9 +25,26 @@ module Vagrant
             # Store in the type map so that --provision-with works properly
             @_provisioner_types[result] = provisioner.type
 
+            # Set top level provisioner name to provisioner configs name if top level name not set.
+            # This is mostly for handling the shell provisioner, if a user has set its name like:
+            #
+            #   config.vm.provision "shell", name: "my_provisioner"
+            #
+            # Where `name` is a shell config option, not a top level provisioner class option
+            #
+            # Note: `name` is set to a symbol, since it is converted to one via #Config::VM.provision
+            provisioner_name = provisioner.name
+            if !provisioner_name
+              if provisioner.config.name
+                provisioner_name = provisioner.config.name.to_sym
+              end
+            else
+              provisioner_name = provisioner_name.to_sym
+            end
+
             # Build up the options
             options = {
-              name: provisioner.name,
+              name: provisioner_name,
               run:  provisioner.run,
               before:  provisioner.before,
               after:  provisioner.after,

plugins/kernel_v2/config/vm_provisioner.rb

@@ -9,7 +9,10 @@ module VagrantPlugins
 
       # Unique name for this provisioner
       #
-      # @return [String]
+      # Accepts a string, but is ultimately forced into a symbol in the top level method inside
+      # #Config::VM.provision method while being parsed from a Vagrantfile
+      #
+      # @return [Symbol]
       attr_reader :name
 
       # Internal unique name for this provisioner

test/unit/vagrant/action/builtin/mixin_provisioners_test.rb

@@ -13,7 +13,7 @@ describe Vagrant::Action::Builtin::MixinProvisioners do
     sandbox.create_vagrant_env
   end
 
-  let(:provisioner_config){ {} }
+  let(:provisioner_config){ double("provisioner_config", name: nil) }
   let(:provisioner_one) do
     prov = VagrantPlugins::Kernel_V2::VagrantConfigProvisioner.new("spec-test", :shell)
     prov.config = provisioner_config
@@ -24,8 +24,14 @@ describe Vagrant::Action::Builtin::MixinProvisioners do
     prov.config = provisioner_config
     prov
   end
+  let(:provisioner_three) do
+    prov = VagrantPlugins::Kernel_V2::VagrantConfigProvisioner.new(nil, :shell)
+    provisioner_config = double("provisioner_config", name: "my_shell")
+    prov.config = provisioner_config
+    prov
+  end
 
-  let(:provisioner_instances) { [provisioner_one,provisioner_two] }
+  let(:provisioner_instances) { [provisioner_one,provisioner_two,provisioner_three] }
 
   let(:ui) { double("ui") }
   let(:vm) { double("vm", provisioners: provisioner_instances) }
@@ -54,6 +60,17 @@ describe Vagrant::Action::Builtin::MixinProvisioners do
       shell_two = result[1]
       expect(shell_two.first).to be_a(VagrantPlugins::Shell::Provisioner)
     end
+
+    it "returns all the instances of configured provisioners" do
+      result = subject.provisioner_instances(env)
+      expect(result.size).to eq(provisioner_instances.size)
+      shell_one = result.first
+      expect(shell_one[1][:name]).to eq(:"spec-test")
+      shell_two = result[1]
+      expect(shell_two[1][:name]).to eq(:"spec-test")
+      shell_three = result[2]
+      expect(shell_three[1][:name]).to eq(:"my_shell")
+    end
   end
 
   context "#sort_provisioner_instances" do
@@ -91,9 +108,9 @@ describe Vagrant::Action::Builtin::MixinProvisioners do
 
       it "returns the array in the correct order" do
         result = subject.provisioner_instances(env)
-        expect(result[0].last[:name]).to eq("before-test")
+        expect(result[0].last[:name]).to eq(:"before-test")
-        expect(result[1].last[:name]).to eq("root-test")
+        expect(result[1].last[:name]).to eq(:"root-test")
-        expect(result[2].last[:name]).to eq("after-test")
+        expect(result[2].last[:name]).to eq(:"after-test")
       end
     end
 
@@ -121,10 +138,10 @@ describe Vagrant::Action::Builtin::MixinProvisioners do
       it "puts the each shortcut provisioners in place" do
         result = subject.provisioner_instances(env)
 
-        expect(result[0].last[:name]).to eq("before-test")
+        expect(result[0].last[:name]).to eq(:"before-test")
-        expect(result[1].last[:name]).to eq("root-test")
+        expect(result[1].last[:name]).to eq(:"root-test")
-        expect(result[2].last[:name]).to eq("before-test")
+        expect(result[2].last[:name]).to eq(:"before-test")
-        expect(result[3].last[:name]).to eq("root2-test")
+        expect(result[3].last[:name]).to eq(:"root2-test")
       end
     end
 
@@ -152,10 +169,10 @@ describe Vagrant::Action::Builtin::MixinProvisioners do
       it "puts the each shortcut provisioners in place" do
         result = subject.provisioner_instances(env)
 
-        expect(result[0].last[:name]).to eq("root-test")
+        expect(result[0].last[:name]).to eq(:"root-test")
-        expect(result[1].last[:name]).to eq("after-test")
+        expect(result[1].last[:name]).to eq(:"after-test")
-        expect(result[2].last[:name]).to eq("root2-test")
+        expect(result[2].last[:name]).to eq(:"root2-test")
-        expect(result[3].last[:name]).to eq("after-test")
+        expect(result[3].last[:name]).to eq(:"after-test")
       end
     end
 
@@ -189,12 +206,12 @@ describe Vagrant::Action::Builtin::MixinProvisioners do
       it "puts the each shortcut provisioners in place" do
         result = subject.provisioner_instances(env)
 
-        expect(result[0].last[:name]).to eq("before-test")
+        expect(result[0].last[:name]).to eq(:"before-test")
-        expect(result[1].last[:name]).to eq("root-test")
+        expect(result[1].last[:name]).to eq(:"root-test")
-        expect(result[2].last[:name]).to eq("after-test")
+        expect(result[2].last[:name]).to eq(:"after-test")
-        expect(result[3].last[:name]).to eq("before-test")
+        expect(result[3].last[:name]).to eq(:"before-test")
-        expect(result[4].last[:name]).to eq("root2-test")
+        expect(result[4].last[:name]).to eq(:"root2-test")
-        expect(result[5].last[:name]).to eq("after-test")
+        expect(result[5].last[:name]).to eq(:"after-test")
       end
     end
 
@@ -228,10 +245,10 @@ describe Vagrant::Action::Builtin::MixinProvisioners do
       it "puts the each shortcut provisioners in place" do
         result = subject.provisioner_instances(env)
 
-        expect(result[0].last[:name]).to eq("before-test")
+        expect(result[0].last[:name]).to eq(:"before-test")
-        expect(result[1].last[:name]).to eq("root-test")
+        expect(result[1].last[:name]).to eq(:"root-test")
-        expect(result[2].last[:name]).to eq("root2-test")
+        expect(result[2].last[:name]).to eq(:"root2-test")
-        expect(result[3].last[:name]).to eq("after-test")
+        expect(result[3].last[:name]).to eq(:"after-test")
       end
     end
   end

test/unit/vagrant/action/builtin/provision_test.rb

@@ -71,7 +71,7 @@ describe Vagrant::Action::Builtin::Provision do
         prov.config = provisioner_config
         prov
       end
-      let(:provisioner_config){ {} }
+      let(:provisioner_config){ double("provisioner_config", name: "spec-test") }
 
       before{ expect(vm_config).to receive(:provisioners).and_return([provisioner]) }
 
@@ -107,13 +107,13 @@ describe Vagrant::Action::Builtin::Provision do
         end
 
         it "should not run if provision types are set and provisioner is not included" do
-          env[:provision_types] = ["other-provisioner", "other-test"]
+          env[:provision_types] = [:"other-provisioner", :"other-test"]
           expect(hook).not_to receive(:call).with(:provisioner_run, anything)
           instance.call(env)
         end
 
         it "should run if provision types are set and include provisioner name" do
-          env[:provision_types] = ["spec-test"]
+          env[:provision_types] = [:"spec-test"]
           expect(hook).to receive(:call).with(:provisioner_run, anything)
           instance.call(env)
         end
@@ -142,13 +142,13 @@ describe Vagrant::Action::Builtin::Provision do
         end
 
         it "should not run if provision types are set and provisioner is not included" do
-          env[:provision_types] = ["other-provisioner", "other-test"]
+          env[:provision_types] = [:"other-provisioner", :"other-test"]
           expect(hook).not_to receive(:call).with(:provisioner_run, anything)
           instance.call(env)
         end
 
         it "should run if provision types are set and include provisioner name" do
-          env[:provision_types] = ["spec-test"]
+          env[:provision_types] = [:"spec-test"]
           expect(hook).to receive(:call).with(:provisioner_run, anything)
           instance.call(env)
         end
@@ -177,13 +177,13 @@ describe Vagrant::Action::Builtin::Provision do
         end
 
         it "should not run if provision types are set and provisioner is not included" do
-          env[:provision_types] = ["other-provisioner", "other-test"]
+          env[:provision_types] = [:"other-provisioner", :"other-test"]
           expect(hook).not_to receive(:call).with(:provisioner_run, anything)
           instance.call(env)
         end
 
         it "should run if provision types are set and include provisioner name" do
-          env[:provision_types] = ["spec-test"]
+          env[:provision_types] = [:"spec-test"]
           expect(hook).to receive(:call).with(:provisioner_run, anything)
           instance.call(env)
         end
@@ -192,7 +192,7 @@ describe Vagrant::Action::Builtin::Provision do
           File.open(File.join(data_dir.to_s, "action_provision"), "w") do |file|
             file.write("1.5:machine-id")
           end
-          env[:provision_types] = ["spec-test"]
+          env[:provision_types] = [:"spec-test"]
           expect(hook).to receive(:call).with(:provisioner_run, anything)
           instance.call(env)
         end