Merge pull request #2929 from mitchellh/f-provisioner-id

Provisioner Inheritance/Overriding

This allows provisioners to be overridden by sub-scopes (machine configs, provider overrides), allowing common provisioning to be put into the global scope and selectively overriding certain configurations. This allows for getting rid of a lot of duplication without having to know a lot of Ruby.

Completely documenting in the pull request (in website/docs commits), so read there.

plugins/kernel_v2/config/vm.rb

@@ -65,7 +65,6 @@ module VagrantPlugins
           other_networks = other.instance_variable_get(:@__networks)
 
           result.instance_variable_set(:@__networks, @__networks.merge(other_networks))
-          result.instance_variable_set(:@provisioners, @provisioners + other.provisioners)
 
           # Merge defined VMs by first merging the defined VM keys,
           # preserving the order in which they were defined.
@@ -107,6 +106,32 @@ module VagrantPlugins
             new_overrides[key] += blocks
           end
 
+          # Merge provisioners. First we deal with overrides and making
+          # sure the ordering is good there. Then we merge them.
+          new_provs   = []
+          other_provs = other.provisioners.dup
+          @provisioners.each do |p|
+            if p.id
+              other_p = other_provs.find { |o| p.id == o.id }
+              if other_p
+                # There is an override. Take it.
+                other_p.config = p.config.merge(other_p.config)
+                next if !other_p.preserve_order
+
+                # We're preserving order, delete from other
+                p = other_p
+                other_provs.delete(other_p)
+              end
+            end
+
+            # There is an override, merge it into the
+            new_provs << p.dup
+          end
+          other_provs.each do |p|
+            new_provs << p.dup
+          end
+          result.instance_variable_set(:@provisioners, new_provs)
+
           # Merge synced folders.
           other_folders = other.instance_variable_get(:@__synced_folders)
           new_folders = {}
@@ -221,8 +246,22 @@ module VagrantPlugins
         end
       end
 
-      def provision(name, options=nil, &block)
+      def provision(name, **options, &block)
-        @provisioners << VagrantConfigProvisioner.new(name.to_sym, options, &block)
+        options[:id] = options[:id].to_s if options[:id]
+
+        prov = nil
+        if options[:id]
+          prov = @provisioners.find { |p| p.id == options[:id] }
+        end
+
+        if !prov
+          prov = VagrantConfigProvisioner.new(options[:id], name.to_sym)
+          @provisioners << prov
+        end
+
+        prov.preserve_order = !!options[:preserve_order]
+        prov.add_config(options, &block)
+        nil
       end
 
       def defined_vms
@@ -321,6 +360,11 @@ module VagrantPlugins
           @__compiled_provider_configs[name]   = config
         end
 
+        # Finaliez all the provisioners
+        @provisioners.each do |p|
+          p.config.finalize! if !p.invalid?
+        end
+
         @__synced_folders.each do |id, options|
           if options[:nfs]
             options[:type] = :nfs

plugins/kernel_v2/config/vm_provisioner.rb

@@ -4,6 +4,11 @@ module VagrantPlugins
   module Kernel_V2
     # Represents a single configured provisioner for a VM.
     class VagrantConfigProvisioner
+      # Unique ID name for this provisioner
+      #
+      # @return [String]
+      attr_reader :id
+
       # The name of the provisioner that should be registered
       # as a plugin.
       #
@@ -13,15 +18,23 @@ module VagrantPlugins
       # The configuration associated with the provisioner, if there is any.
       #
       # @return [Object]
-      attr_reader :config
+      attr_accessor :config
 
-      def initialize(name, options=nil, &block)
+      # Whether or not to preserve the order when merging this with a
+      # parent scope.
+      #
+      # @return [Boolean]
+      attr_accessor :preserve_order
+
+      def initialize(id, name)
         @logger = Log4r::Logger.new("vagrant::config::vm::provisioner")
         @logger.debug("Provisioner defined: #{name}")
 
         @config  = nil
+        @id      = id
         @invalid = false
         @name    = name
+        @preserve_order = false
 
         # Attempt to find the provisioner...
         if !Vagrant.plugin("2").manager.provisioners[name]
@@ -31,15 +44,32 @@ module VagrantPlugins
 
         # Attempt to find the configuration class for this provider
         # if it exists and load the configuration.
-        config_class = Vagrant.plugin("2").manager.provisioner_configs[@name]
+        @config_class = Vagrant.plugin("2").manager.
-        if !config_class
+          provisioner_configs[@name]
-          @logger.info("Provisioner config for '#{@name}' not found. Ignoring config.")
+        if !@config_class
-          return
+          @logger.info(
+            "Provisioner config for '#{@name}' not found. Ignoring config.")
+        end
       end
 
-        @config = config_class.new
+      def initialize_copy(orig)
-        @config.set_options(options) if options
+        super
-        block.call(@config) if block
+        @config = @config.dup if @config
+      end
+
+      def add_config(**options, &block)
+        return if invalid?
+
+        current = @config_class.new
+        current.set_options(options) if options
+        current.call(@config) if block
+        current = @config.merge(current) if @config
+        @config = current
+      end
+
+      def finalize!
+        return if invalid?
+
         @config.finalize!
       end
 

plugins/provisioners/docker/config.rb

@@ -3,14 +3,25 @@ require 'set'
 module VagrantPlugins
   module Docker
     class Config < Vagrant.plugin("2", :config)
-      attr_reader :build_images, :images, :containers, :build_options
+      attr_reader :images
       attr_accessor :version
 
       def initialize
         @images         = Set.new
-        @containers     = Hash.new
         @version        = UNSET_VALUE
-        @build_images   = []
+
+        @__build_images   = []
+        @__containers     = Hash.new { |h, k| h[k] = {} }
+      end
+
+      # Accessor for internal state.
+      def build_images
+        @__build_images
+      end
+
+      # Accessor for the internal state.
+      def containers
+        @__containers
       end
 
       # Defines an image to build using `docker build` within the machine.
@@ -18,7 +29,7 @@ module VagrantPlugins
       # @param [String] path Path to the Dockerfile to pass to
       #   `docker build`.
       def build_image(path, **opts)
-        @build_images << [path, opts]
+        @__build_images << [path, opts]
       end
 
       def images=(images)
@@ -30,22 +41,36 @@ module VagrantPlugins
       end
 
       def run(name, **options)
-        params = options.dup
+        @__containers[name.to_s] = options.dup
-        params[:image] ||= name
-        params[:daemonize] = true if !params.has_key?(:daemonize)
-
-        # TODO: Validate provided parameters before assignment
-        @containers[name.to_s] = params
-      end
-
-      def finalize!
-        @version = "latest" if @version == UNSET_VALUE
-        @version = @version.to_sym
       end
 
       def merge(other)
         super.tap do |result|
           result.pull_images(*(other.images + self.images))
+
+          build_images = @__build_images.dup
+          build_images += other.build_images
+          result.instance_variable_set(:@__build_images, build_images)
+
+          containers = {}
+          @__containers.each do |name, params|
+            containers[name] = params.dup
+          end
+          other.containers.each do |name, params|
+            containers[name] = @__containers[name].merge(params)
+          end
+
+          result.instance_variable_set(:@__containers, containers)
+        end
+      end
+
+      def finalize!
+        @version = "latest" if @version == UNSET_VALUE
+        @version = @version.to_sym
+
+        @__containers.each do |name, params|
+          params[:image] ||= name
+          params[:daemonize] = true if !params.has_key?(:daemonize)
         end
       end
     end

plugins/provisioners/puppet/config/puppet.rb

@@ -42,6 +42,12 @@ module VagrantPlugins
           end
         end
 
+        def merge(other)
+          super.tap do |result|
+            result.facter  = @facter.merge(other.facter)
+          end
+        end
+
         def finalize!
           super
 

plugins/provisioners/puppet/config/puppet_server.rb

@@ -20,6 +20,12 @@ module VagrantPlugins
           @puppet_server           = UNSET_VALUE
         end
 
+        def merge(other)
+          super.tap do |result|
+            result.facter  = @facter.merge(other.facter)
+          end
+        end
+
         def finalize!
           super
 

test/unit/plugins/kernel_v2/config/vm_test.rb

@@ -0,0 +1,101 @@
+require File.expand_path("../../../../base", __FILE__)
+
+require Vagrant.source_root.join("plugins/kernel_v2/config/vm")
+
+describe VagrantPlugins::Kernel_V2::VMConfig do
+  subject { described_class.new }
+
+  describe "#provision" do
+    it "stores the provisioners" do
+      subject.provision("shell", inline: "foo")
+      subject.provision("shell", inline: "bar")
+      subject.finalize!
+
+      r = subject.provisioners
+      expect(r.length).to eql(2)
+      expect(r[0].config.inline).to eql("foo")
+      expect(r[1].config.inline).to eql("bar")
+    end
+
+    it "allows provisioner settings to be overriden" do
+      subject.provision("shell", path: "foo", inline: "foo", id: "s")
+      subject.provision("shell", inline: "bar", id: "s")
+      subject.finalize!
+
+      r = subject.provisioners
+      expect(r.length).to eql(1)
+      expect(r[0].config.inline).to eql("bar")
+      expect(r[0].config.path).to eql("foo")
+    end
+
+    it "marks as invalid if a bad name" do
+      subject.provision("nope", inline: "foo")
+      subject.finalize!
+
+      r = subject.provisioners
+      expect(r.length).to eql(1)
+      expect(r[0]).to be_invalid
+    end
+
+    describe "merging" do
+      it "copies the configs" do
+        subject.provision("shell", inline: "foo")
+        subject_provs = subject.provisioners
+
+        other = described_class.new
+        other.provision("shell", inline: "bar")
+
+        merged = subject.merge(other)
+        merged_provs = merged.provisioners
+
+        expect(merged_provs.length).to eql(2)
+        expect(merged_provs[0].config.inline).
+          to eq(subject_provs[0].config.inline)
+        expect(merged_provs[0].config.object_id).
+          to_not eq(subject_provs[0].config.object_id)
+      end
+
+      it "uses the proper order when merging overrides" do
+        subject.provision("shell", inline: "foo", id: "original")
+        subject.provision("shell", inline: "other", id: "other")
+
+        other = described_class.new
+        other.provision("shell", inline: "bar")
+        other.provision("shell", inline: "foo-overload", id: "original")
+
+        merged = subject.merge(other)
+        merged_provs = merged.provisioners
+
+        expect(merged_provs.length).to eql(3)
+        expect(merged_provs[0].config.inline).
+          to eq("other")
+        expect(merged_provs[1].config.inline).
+          to eq("bar")
+        expect(merged_provs[2].config.inline).
+          to eq("foo-overload")
+      end
+
+      it "can preserve order for overrides" do
+        subject.provision("shell", inline: "foo", id: "original")
+        subject.provision("shell", inline: "other", id: "other")
+
+        other = described_class.new
+        other.provision("shell", inline: "bar")
+        other.provision(
+          "shell", inline: "foo-overload", id: "original",
+          preserve_order: true)
+
+        merged = subject.merge(other)
+        merged_provs = merged.provisioners
+
+        expect(merged_provs.length).to eql(3)
+        expect(merged_provs[0].config.inline).
+          to eq("foo-overload")
+        expect(merged_provs[1].config.inline).
+          to eq("other")
+        expect(merged_provs[2].config.inline).
+          to eq("bar")
+      end
+    end
+  end
+end

test/unit/plugins/provisioners/docker/config_test.rb

@@ -31,6 +31,56 @@ describe VagrantPlugins::Docker::Config do
     end
   end
 
+  describe "#merge" do
+    it "has all images to pull" do
+      subject.pull_images("1")
+
+      other = described_class.new
+      other.pull_images("2", "3")
+
+      result = subject.merge(other)
+      expect(result.images.to_a.sort).to eq(
+        ["1", "2", "3"])
+    end
+
+    it "has all the containers to run" do
+      subject.run("foo", image: "bar", daemonize: false)
+      subject.run("bar")
+
+      other = described_class.new
+      other.run("foo", image: "foo")
+
+      result = subject.merge(other)
+      result.finalize!
+
+      cs     = result.containers
+      expect(cs.length).to eq(2)
+      expect(cs["foo"]).to eq({
+        image: "foo",
+        daemonize: false,
+      })
+      expect(cs["bar"]).to eq({
+        image: "bar",
+        daemonize: true,
+      })
+    end
+
+    it "has all the containers to build" do
+      subject.build_image("foo")
+
+      other = described_class.new
+      other.build_image("bar")
+
+      result = subject.merge(other)
+      result.finalize!
+
+      images = result.build_images
+      expect(images.length).to eq(2)
+      expect(images[0]).to eq(["foo", {}])
+      expect(images[1]).to eq(["bar", {}])
+    end
+  end
+
   describe "#pull_images" do
     it "adds images to the list of images to build" do
       subject.pull_images("1")

website/docs/source/v2/provisioning/basic_usage.html.md

@@ -23,7 +23,7 @@ Vagrant.configure("2") do |config|
 end
 ```
 
-Every provisioner has an identifier, such as `"shell"`, used as the first
+Every provisioner has a type, such as `"shell"`, used as the first
 parameter to the provisioning configuration. Following that is basic key/value
 for configuring that specific provisioner. Instead of basic key/value, you
 can also use a Ruby block for a syntax that is more like variable assignment.
@@ -44,14 +44,6 @@ it can greatly improve readability. Additionally, some provisioners, like
 the Chef provisioner, have special methods that can be called within that
 block to ease configuration that can't be done with the key/value approach.
 
-## Multiple Provisioners
-
-Multiple `config.vm.provision` methods can be used to define multiple
-provisioners. These provisioners will be run in the order they're defined.
-This is useful for a variety of reasons, but most commonly it is used so
-that a shell script can bootstrap some of the system so that another provisioner
-can take over later.
-
 ## Running Provisioners
 
 Provisioners are run in three cases: the initial `vagrant up`, `vagrant
@@ -65,3 +57,93 @@ The `--provision-with` flag can be used if you only want to run a
 specific provisioner if you have multiple provisioners specified. For
 example, if you have a shell and Puppet provisioner and only want to
 run the shell one, you can do `vagrant provision --provision-with shell`.
+
+## Multiple Provisioners
+
+Multiple `config.vm.provision` methods can be used to define multiple
+provisioners. These provisioners will be run in the order they're defined.
+This is useful for a variety of reasons, but most commonly it is used so
+that a shell script can bootstrap some of the system so that another provisioner
+can take over later.
+
+If you define provisioners at multiple "scope" levels (such as globally
+in the configuration block, then in a
+[multi-machine](/v2/multi-machine/index.html) definition, then maybe
+in a [provider-specific override](/v2/providers/configuration.html)),
+then the outer scopes will always run _before_ any inner scopes. For
+example, in the Vagrantfile below:
+
+```ruby
+Vagrant.configure("2") do |config|
+  config.vm.provision "shell", inline: "echo foo"
+
+  config.vm.define "web" do |web|
+    web.vm.provision "shell", inline: "echo bar"
+  end
+
+  config.vm.provision "shell", inline: "echo baz"
+end
+```
+
+The ordering of the provisioners will be to echo "foo", "baz", then
+"bar" (note the second one might not be what you expect!). Remember:
+ordering is _outside in_.
+
+## Overriding Provisioner Settings
+
+<div class="alert alert-block alert-warn">
+<p>
+<strong>Warning: Advanced Topic!</strong> Provisioner overriding is
+an advanced topic that really only becomes useful if you're already
+using multi-machine and/or provider overrides. If you're just getting
+started with Vagrant, you can safely skip this.
+</p>
+</div>
+
+When using features such as [multi-machine](/v2/multi-machine/index.html)
+or [provider-specific overrides](/v2/providers/configuration.html),
+you may want to define common provisioners in the global configuration
+scope of a Vagrantfile, but override certain aspects of them internally.
+Vagrant allows you to do this, but has some details to consider.
+
+To override settings, you must assign an ID to your provisioner. Then
+it is only a matter of specifying the same ID to override:
+
+```ruby
+Vagrant.configure("2") do |config|
+  config.vm.provision "shell",
+    inline: "echo foo", id: "foo"
+
+  config.vm.define "web" do |web|
+    web.vm.provision "shell",
+      inline: "echo bar", id: "foo"
+  end
+end
+```
+
+In the above, only "bar" will be echoed, because the inline setting
+overloaded the outer provisioner. This overload is only effective
+within that scope: the "web" VM. If there were another VM defined,
+it would still echo "foo" unless it itself also overloaded the
+provisioner.
+
+**Be careful with ordering.** When overriding a provisioner in
+a sub-scope, the provisioner will run at _that point_. In the example
+below, the output would be "foo" then "bar":
+
+```ruby
+Vagrant.configure("2") do |config|
+  config.vm.provision "shell",
+    inline: "echo ORIGINAL!", id: "foo"
+
+  config.vm.define "web" do |web|
+    web.vm.provision "shell",
+      inline: "echo foo"
+    web.vm.provision "shell",
+      inline: "echo bar", id: "foo"
+  end
+end
+```
+
+If you want to preserve the original ordering, you can specify
+the `preserve_order: true` flag.