From 49ae77b2b833908b42de26746af456a86f30de00 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Thu, 4 Apr 2019 08:55:32 -0700 Subject: [PATCH 01/29] Update doc string for :run option --- plugins/kernel_v2/config/vm_provisioner.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/kernel_v2/config/vm_provisioner.rb b/plugins/kernel_v2/config/vm_provisioner.rb index 59833c850..e7d0c096f 100644 --- a/plugins/kernel_v2/config/vm_provisioner.rb +++ b/plugins/kernel_v2/config/vm_provisioner.rb @@ -29,7 +29,7 @@ module VagrantPlugins # @return [Object] attr_accessor :config - # When to run this provisioner. Either "once" or "always" + # When to run this provisioner. Either "once", "always", or "never" # # @return [String] attr_accessor :run From b82b33d2047c0abc3b1e85875dafd6c5e173bcaa Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Fri, 26 Jul 2019 09:37:56 -0700 Subject: [PATCH 02/29] Add new before/after options for the base Provisioner class This commit adds two new options: before, after. These string options refer to other named Provisioners. --- plugins/kernel_v2/config/vm_provisioner.rb | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/plugins/kernel_v2/config/vm_provisioner.rb b/plugins/kernel_v2/config/vm_provisioner.rb index e7d0c096f..d7ca43a27 100644 --- a/plugins/kernel_v2/config/vm_provisioner.rb +++ b/plugins/kernel_v2/config/vm_provisioner.rb @@ -40,6 +40,16 @@ module VagrantPlugins # @return [Boolean] attr_accessor :preserve_order + # The name of a provisioner to run before it has started + # + # @return [String] + attr_accessor :before + + # The name of a provisioner to run after it is finished + # + # @return [String] + attr_accessor :after + def initialize(name, type) @logger = Log4r::Logger.new("vagrant::config::vm::provisioner") @logger.debug("Provisioner defined: #{name}") @@ -51,6 +61,8 @@ module VagrantPlugins @preserve_order = false @run = nil @type = type + @before = nil + @after = nil # Attempt to find the provisioner... if !Vagrant.plugin("2").manager.provisioners[type] From e05437ddf264658720230fd80fd45d1131d92b52 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Fri, 26 Jul 2019 10:42:32 -0700 Subject: [PATCH 03/29] Add validation method and todo --- plugins/kernel_v2/config/vm_provisioner.rb | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/plugins/kernel_v2/config/vm_provisioner.rb b/plugins/kernel_v2/config/vm_provisioner.rb index d7ca43a27..a6f98d9cf 100644 --- a/plugins/kernel_v2/config/vm_provisioner.rb +++ b/plugins/kernel_v2/config/vm_provisioner.rb @@ -4,6 +4,9 @@ module VagrantPlugins module Kernel_V2 # Represents a single configured provisioner for a VM. class VagrantConfigProvisioner + # Defaults + VALID_BEFORE_AFTER_TYPES = [:each, :all].freeze + # Unique name for this provisioner # # @return [String] @@ -102,6 +105,25 @@ module VagrantPlugins @config.finalize! end + # TODO: This isn't being called + # + # @return [Array] array of strings of error messages from config option validation + def validate(machine) + errors = _detected_errors + + if @before + if @before.is_a?(Symbol) && !VALID_BEFORE_AFTER_TYPES.include?(@before) + errors << "Before symbol is not a valid symbol" + end + end + + if @after + if @after.is_a?(Symbol) && !VALID_BEFORE_AFTER_TYPES.include?(@after) + errors << "After symbol is not a valid symbol" + end + end + end + # Returns whether the provisioner used was invalid or not. A provisioner # is invalid if it can't be found. # From 28c0f6085c85db35323ac1e92a2d21d4404cecb9 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Tue, 6 Aug 2019 16:18:58 -0700 Subject: [PATCH 04/29] Attempt to validate top scope provisioner options --- plugins/kernel_v2/config/vm.rb | 7 +++++++ plugins/kernel_v2/config/vm_provisioner.rb | 8 ++++---- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/plugins/kernel_v2/config/vm.rb b/plugins/kernel_v2/config/vm.rb index fc8b5d5b2..2cf4a9c75 100644 --- a/plugins/kernel_v2/config/vm.rb +++ b/plugins/kernel_v2/config/vm.rb @@ -760,6 +760,13 @@ module VagrantPlugins next end + require 'pry' + binding.pry + #provisioner_errors = vm_provisioner.validate(machine) + #if provisioner_errors + # errors = Vagrant::Config::V2::Util.merge_errors(errors, provisioner_errors) + #end + if vm_provisioner.config provisioner_errors = vm_provisioner.config.validate(machine) if provisioner_errors diff --git a/plugins/kernel_v2/config/vm_provisioner.rb b/plugins/kernel_v2/config/vm_provisioner.rb index a6f98d9cf..4f49bed80 100644 --- a/plugins/kernel_v2/config/vm_provisioner.rb +++ b/plugins/kernel_v2/config/vm_provisioner.rb @@ -3,7 +3,7 @@ require 'log4r' module VagrantPlugins module Kernel_V2 # Represents a single configured provisioner for a VM. - class VagrantConfigProvisioner + class VagrantConfigProvisioner < Vagrant.plugin("2", :config) # Defaults VALID_BEFORE_AFTER_TYPES = [:each, :all].freeze @@ -53,7 +53,7 @@ module VagrantPlugins # @return [String] attr_accessor :after - def initialize(name, type) + def initialize(name, type, before=nil, after=nil) @logger = Log4r::Logger.new("vagrant::config::vm::provisioner") @logger.debug("Provisioner defined: #{name}") @@ -64,8 +64,8 @@ module VagrantPlugins @preserve_order = false @run = nil @type = type - @before = nil - @after = nil + @before = before #these aren't being properly set + @after = after # Attempt to find the provisioner... if !Vagrant.plugin("2").manager.provisioners[type] From 66aac2347005a44b9239b44e68f03d889c441685 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Mon, 19 Aug 2019 11:23:06 -0700 Subject: [PATCH 05/29] Properly set and validate before/after keys for base provisioner class --- plugins/kernel_v2/config/vm.rb | 19 ++++++++++++------- plugins/kernel_v2/config/vm_provisioner.rb | 12 ++++++++---- templates/locales/en.yml | 5 +++++ 3 files changed, 25 insertions(+), 11 deletions(-) diff --git a/plugins/kernel_v2/config/vm.rb b/plugins/kernel_v2/config/vm.rb index 2cf4a9c75..8379f09d8 100644 --- a/plugins/kernel_v2/config/vm.rb +++ b/plugins/kernel_v2/config/vm.rb @@ -331,7 +331,14 @@ module VagrantPlugins end if !prov - prov = VagrantConfigProvisioner.new(name, type.to_sym) + if options.key?(:before) + before = options.delete(:before) + end + if options.key?(:after) + after = options.delete(:after) + end + + prov = VagrantConfigProvisioner.new(name, type.to_sym, before, after) @provisioners << prov end @@ -760,12 +767,10 @@ module VagrantPlugins next end - require 'pry' - binding.pry - #provisioner_errors = vm_provisioner.validate(machine) - #if provisioner_errors - # errors = Vagrant::Config::V2::Util.merge_errors(errors, provisioner_errors) - #end + provisioner_errors = vm_provisioner.validate(machine) + if provisioner_errors + errors = Vagrant::Config::V2::Util.merge_errors(errors, provisioner_errors) + end if vm_provisioner.config provisioner_errors = vm_provisioner.config.validate(machine) diff --git a/plugins/kernel_v2/config/vm_provisioner.rb b/plugins/kernel_v2/config/vm_provisioner.rb index 4f49bed80..4c3c4a04f 100644 --- a/plugins/kernel_v2/config/vm_provisioner.rb +++ b/plugins/kernel_v2/config/vm_provisioner.rb @@ -105,23 +105,27 @@ module VagrantPlugins @config.finalize! end - # TODO: This isn't being called - # # @return [Array] array of strings of error messages from config option validation def validate(machine) errors = _detected_errors if @before if @before.is_a?(Symbol) && !VALID_BEFORE_AFTER_TYPES.include?(@before) - errors << "Before symbol is not a valid symbol" + errors << I18n.t("vagrant.provisioners.base.invalid_alias_value", opt: "before", alias: VALID_BEFORE_AFTER_TYPES.join(", ")) + elsif !@before.is_a?(String) && !VALID_BEFORE_AFTER_TYPES.include?(@before) + errors << I18n.t("vagrant.provisioners.base.wrong_type", opt: "before") end end if @after if @after.is_a?(Symbol) && !VALID_BEFORE_AFTER_TYPES.include?(@after) - errors << "After symbol is not a valid symbol" + errors << I18n.t("vagrant.provisioners.base.invalid_alias_value", opt: "after", alias: VALID_BEFORE_AFTER_TYPES.join(", ")) + elsif !@after.is_a?(String) && !VALID_BEFORE_AFTER_TYPES.include?(@after) + errors << I18n.t("vagrant.provisioners.base.wrong_type", opt: "after") end end + + {"provisioner" => errors} end # Returns whether the provisioner used was invalid or not. A provisioner diff --git a/templates/locales/en.yml b/templates/locales/en.yml index 2b63dd383..766f36984 100644 --- a/templates/locales/en.yml +++ b/templates/locales/en.yml @@ -2486,6 +2486,11 @@ en: VirtualBox has successfully been installed! provisioners: + base: + invalid_alias_value: |- + Provisioner option `%{opt}` is not set as a valid type. Must be a string, or one of the alias shortcuts: %{alias} + wrong_type: |- + Provisioner option `%{opt}` is not set as a valid type. Must be a string. chef: chef_not_detected: |- The chef binary (either `chef-solo` or `chef-client`) was not found on From 4933610398518312a6315b918a53704a87349cdc Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Mon, 19 Aug 2019 11:33:30 -0700 Subject: [PATCH 06/29] Update rubydoc for before/after return types --- plugins/kernel_v2/config/vm_provisioner.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/plugins/kernel_v2/config/vm_provisioner.rb b/plugins/kernel_v2/config/vm_provisioner.rb index 4c3c4a04f..0c81df22f 100644 --- a/plugins/kernel_v2/config/vm_provisioner.rb +++ b/plugins/kernel_v2/config/vm_provisioner.rb @@ -45,12 +45,12 @@ module VagrantPlugins # The name of a provisioner to run before it has started # - # @return [String] + # @return [String, Symbol] attr_accessor :before # The name of a provisioner to run after it is finished # - # @return [String] + # @return [String, Symbol] attr_accessor :after def initialize(name, type, before=nil, after=nil) From 8ecd32de53b9fb789a676e71dff1427dfb04a5cf Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Mon, 19 Aug 2019 11:36:54 -0700 Subject: [PATCH 07/29] Remove comment --- plugins/kernel_v2/config/vm_provisioner.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/kernel_v2/config/vm_provisioner.rb b/plugins/kernel_v2/config/vm_provisioner.rb index 0c81df22f..545da783d 100644 --- a/plugins/kernel_v2/config/vm_provisioner.rb +++ b/plugins/kernel_v2/config/vm_provisioner.rb @@ -64,7 +64,7 @@ module VagrantPlugins @preserve_order = false @run = nil @type = type - @before = before #these aren't being properly set + @before = before @after = after # Attempt to find the provisioner... From d15bac7fb7ee831e2c512da9757026a47d1f5e2b Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Mon, 19 Aug 2019 14:48:06 -0700 Subject: [PATCH 08/29] Validate that before/after provisioner exists in machines config --- plugins/kernel_v2/config/vm.rb | 3 +- plugins/kernel_v2/config/vm_provisioner.rb | 38 +++++++++++++++++----- templates/locales/en.yml | 2 ++ 3 files changed, 33 insertions(+), 10 deletions(-) diff --git a/plugins/kernel_v2/config/vm.rb b/plugins/kernel_v2/config/vm.rb index 8379f09d8..a0dc0c8b2 100644 --- a/plugins/kernel_v2/config/vm.rb +++ b/plugins/kernel_v2/config/vm.rb @@ -767,7 +767,8 @@ module VagrantPlugins next end - provisioner_errors = vm_provisioner.validate(machine) + provisioner_names = @provisioners.map { |i| i.name if i.name != vm_provisioner.name }.reject(&:blank?) + provisioner_errors = vm_provisioner.validate(machine, provisioner_names) if provisioner_errors errors = Vagrant::Config::V2::Util.merge_errors(errors, provisioner_errors) end diff --git a/plugins/kernel_v2/config/vm_provisioner.rb b/plugins/kernel_v2/config/vm_provisioner.rb index 545da783d..1dcdeedc1 100644 --- a/plugins/kernel_v2/config/vm_provisioner.rb +++ b/plugins/kernel_v2/config/vm_provisioner.rb @@ -105,23 +105,43 @@ module VagrantPlugins @config.finalize! end + # @param [Vagrant::Machine] machine - machine to validate against + # @param [Array] provisioner_names - Names of provisioners for a given machine # @return [Array] array of strings of error messages from config option validation - def validate(machine) + def validate(machine, provisioner_names) errors = _detected_errors if @before - if @before.is_a?(Symbol) && !VALID_BEFORE_AFTER_TYPES.include?(@before) - errors << I18n.t("vagrant.provisioners.base.invalid_alias_value", opt: "before", alias: VALID_BEFORE_AFTER_TYPES.join(", ")) - elsif !@before.is_a?(String) && !VALID_BEFORE_AFTER_TYPES.include?(@before) - errors << I18n.t("vagrant.provisioners.base.wrong_type", opt: "before") + if !VALID_BEFORE_AFTER_TYPES.include?(@before) + if @before.is_a?(Symbol) && !VALID_BEFORE_AFTER_TYPES.include?(@before) + errors << I18n.t("vagrant.provisioners.base.invalid_alias_value", opt: "before", alias: VALID_BEFORE_AFTER_TYPES.join(", ")) + elsif !@before.is_a?(String) && !VALID_BEFORE_AFTER_TYPES.include?(@before) + errors << I18n.t("vagrant.provisioners.base.wrong_type", opt: "before") + end + + if !provisioner_names.include?(@before.to_sym) + errors << I18n.t("vagrant.provisioners.base.missing_provisioner_name", + name: @before, + machine_name: machine.name, + action: "before") + end end end if @after - if @after.is_a?(Symbol) && !VALID_BEFORE_AFTER_TYPES.include?(@after) - errors << I18n.t("vagrant.provisioners.base.invalid_alias_value", opt: "after", alias: VALID_BEFORE_AFTER_TYPES.join(", ")) - elsif !@after.is_a?(String) && !VALID_BEFORE_AFTER_TYPES.include?(@after) - errors << I18n.t("vagrant.provisioners.base.wrong_type", opt: "after") + if !VALID_BEFORE_AFTER_TYPES.include?(@after) + if @after.is_a?(Symbol) + errors << I18n.t("vagrant.provisioners.base.invalid_alias_value", opt: "after", alias: VALID_BEFORE_AFTER_TYPES.join(", ")) + elsif !@after.is_a?(String) + errors << I18n.t("vagrant.provisioners.base.wrong_type", opt: "after") + end + + if !provisioner_names.include?(@after.to_sym) + errors << I18n.t("vagrant.provisioners.base.missing_provisioner_name", + name: @after, + machine_name: machine.name, + action: "after") + end end end diff --git a/templates/locales/en.yml b/templates/locales/en.yml index 766f36984..3590cdb9b 100644 --- a/templates/locales/en.yml +++ b/templates/locales/en.yml @@ -2489,6 +2489,8 @@ en: base: invalid_alias_value: |- Provisioner option `%{opt}` is not set as a valid type. Must be a string, or one of the alias shortcuts: %{alias} + missing_provisioner_name: |- + Could not find provisioner name `%{name}` defined for machine `%{machine_name}` to run provisioner `%{action}`. wrong_type: |- Provisioner option `%{opt}` is not set as a valid type. Must be a string. chef: From 6db03f2aed59f8112bba86bea83f3b448a0d0586 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Tue, 20 Aug 2019 11:30:42 -0700 Subject: [PATCH 09/29] Check if rejected entries are nil, not blank --- plugins/kernel_v2/config/vm.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/kernel_v2/config/vm.rb b/plugins/kernel_v2/config/vm.rb index a0dc0c8b2..23a5a4352 100644 --- a/plugins/kernel_v2/config/vm.rb +++ b/plugins/kernel_v2/config/vm.rb @@ -767,7 +767,7 @@ module VagrantPlugins next end - provisioner_names = @provisioners.map { |i| i.name if i.name != vm_provisioner.name }.reject(&:blank?) + provisioner_names = @provisioners.map { |i| i.name if i.name != vm_provisioner.name }.reject(&:nil?) provisioner_errors = vm_provisioner.validate(machine, provisioner_names) if provisioner_errors errors = Vagrant::Config::V2::Util.merge_errors(errors, provisioner_errors) From 6777493c461a51a2e1d6d48055eff389772a84b9 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Tue, 20 Aug 2019 11:31:00 -0700 Subject: [PATCH 10/29] Include before/after options in final provision hash --- lib/vagrant/action/builtin/mixin_provisioners.rb | 9 +++++++++ plugins/kernel_v2/config/vm_provisioner.rb | 2 ++ 2 files changed, 11 insertions(+) diff --git a/lib/vagrant/action/builtin/mixin_provisioners.rb b/lib/vagrant/action/builtin/mixin_provisioners.rb index 3d712c954..5c17556ad 100644 --- a/lib/vagrant/action/builtin/mixin_provisioners.rb +++ b/lib/vagrant/action/builtin/mixin_provisioners.rb @@ -29,6 +29,8 @@ module Vagrant options = { name: provisioner.name, run: provisioner.run, + before: provisioner.before, + after: provisioner.after, } # Return the result @@ -38,6 +40,13 @@ module Vagrant return @_provisioner_instances.compact end + # Sorts provisioners based on order specified with before/after options + # + # @return [Array] + def sort_provisioner_instances(pvs) + return pvs + end + # This will return a mapping of a provisioner instance to its # type. def provisioner_type_map(env) diff --git a/plugins/kernel_v2/config/vm_provisioner.rb b/plugins/kernel_v2/config/vm_provisioner.rb index 1dcdeedc1..309281d27 100644 --- a/plugins/kernel_v2/config/vm_provisioner.rb +++ b/plugins/kernel_v2/config/vm_provisioner.rb @@ -105,6 +105,8 @@ module VagrantPlugins @config.finalize! end + # Validates the before/after options + # # @param [Vagrant::Machine] machine - machine to validate against # @param [Array] provisioner_names - Names of provisioners for a given machine # @return [Array] array of strings of error messages from config option validation From be5964a1db53c52b907cae40cf301054abfdaf74 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Wed, 21 Aug 2019 14:44:16 -0700 Subject: [PATCH 11/29] Add basic provisioner sorting --- .../action/builtin/mixin_provisioners.rb | 72 ++++++++++++++++++- 1 file changed, 71 insertions(+), 1 deletion(-) diff --git a/lib/vagrant/action/builtin/mixin_provisioners.rb b/lib/vagrant/action/builtin/mixin_provisioners.rb index 5c17556ad..af7c3ddcb 100644 --- a/lib/vagrant/action/builtin/mixin_provisioners.rb +++ b/lib/vagrant/action/builtin/mixin_provisioners.rb @@ -37,14 +37,84 @@ module Vagrant [result, options] end + @_provisioner_instances = sort_provisioner_instances(@_provisioner_instances) return @_provisioner_instances.compact end # Sorts provisioners based on order specified with before/after options # + # TODO: make sure all defined provisioners work here (i.e. even thoughs defined in separate but loaded Vagrantfile) + # # @return [Array] def sort_provisioner_instances(pvs) - return pvs + final_provs = [] + root_provs = [] + dep_provs = [] + each_provs = [] + all_provs = [] + + # extract root provisioners + root_provs = pvs.map { |p,o| [p,o] if o[:before].nil? && o[:after].nil? }.reject(&:nil?) + # extract dependency provisioners + dep_provs = pvs.map { |p,o| [p,o] if (!o[:before].nil? && !o[:before].is_a?(Symbol)) || (!o[:after].nil? && !o[:after].is_a?(Symbol)) }.reject(&:nil?) + # extract each provisioners + each_provs = pvs.map { |p,o| [p,o] if o[:before] == :each || o[:after] == :each }.reject(&:nil?) + # extract all provisioners + all_provs = pvs.map { |p,o| [p,o] if o[:before] == :all || o[:after] == :all }.reject(&:nil?) + + if root_provs.size == pvs.size + # no dependencies found + return pvs + end + + # TODO: Log here, that provisioner order is being changed + + # insert provisioners in order + final_provs = root_provs + dep_provs.each do |p,options| + if options[:before] + idx = final_provs.each_with_index.map { |(p,o), i| i if o[:name].to_s == options[:before] }.reject(&:nil?).first + idx -= 1 unless idx == 0 + final_provs.insert(idx, [p, options]) + elsif options[:after] + idx = final_provs.each_with_index.map { |(p,o), i| i if o[:name].to_s == options[:after] }.reject(&:nil?).first + idx += 1 + final_provs.insert(idx, [p, options]) + end + end + + # add each to final array + tmp_final_provs = final_provs.dup + extra_index = 0 + each_provs.reverse_each do |p,options| + final_provs.each_with_index.map do |(prv,o), i| + if options[:before] + idx = i-1 unless idx == 0 + idx += extra_index + extra_index += 1 + tmp_final_provs.insert(idx, [p,options]) + elsif options[:after] + idx = i+1 + idx += extra_index + extra_index += 1 + tmp_final_provs.insert(idx, [p,options]) + end + end + end + final_provs = tmp_final_provs + + # add all to final array + tmp_final_provs = final_provs.dup + all_provs.reverse_each do |p,options| + if options[:before] + tmp_final_provs.insert(0, [p,options]) + elsif options[:after] + tmp_final_provs.push([p,options]) + end + end + final_provs = tmp_final_provs + + return final_provs end # This will return a mapping of a provisioner instance to its From ef82c0a5bca529e8eb555c6bbcd167e3853046b0 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Thu, 22 Aug 2019 12:01:19 -0700 Subject: [PATCH 12/29] Move return earlier --- lib/vagrant/action/builtin/mixin_provisioners.rb | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/lib/vagrant/action/builtin/mixin_provisioners.rb b/lib/vagrant/action/builtin/mixin_provisioners.rb index af7c3ddcb..d3b0d20ef 100644 --- a/lib/vagrant/action/builtin/mixin_provisioners.rb +++ b/lib/vagrant/action/builtin/mixin_provisioners.rb @@ -55,6 +55,12 @@ module Vagrant # extract root provisioners root_provs = pvs.map { |p,o| [p,o] if o[:before].nil? && o[:after].nil? }.reject(&:nil?) + + if root_provs.size == pvs.size + # no dependencies found + return pvs + end + # extract dependency provisioners dep_provs = pvs.map { |p,o| [p,o] if (!o[:before].nil? && !o[:before].is_a?(Symbol)) || (!o[:after].nil? && !o[:after].is_a?(Symbol)) }.reject(&:nil?) # extract each provisioners @@ -62,11 +68,6 @@ module Vagrant # extract all provisioners all_provs = pvs.map { |p,o| [p,o] if o[:before] == :all || o[:after] == :all }.reject(&:nil?) - if root_provs.size == pvs.size - # no dependencies found - return pvs - end - # TODO: Log here, that provisioner order is being changed # insert provisioners in order From 28ef368881a64f1c87da697cbcfadd6aa44bee2c Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Mon, 26 Aug 2019 12:03:36 -0700 Subject: [PATCH 13/29] Begin to add tests for mixin provisioner --- .../action/builtin/mixin_provisioners.rb | 28 ++- .../action/builtin/mixin_provisioners_test.rb | 203 ++++++++++++++++++ 2 files changed, 220 insertions(+), 11 deletions(-) create mode 100644 test/unit/vagrant/action/builtin/mixin_provisioners_test.rb diff --git a/lib/vagrant/action/builtin/mixin_provisioners.rb b/lib/vagrant/action/builtin/mixin_provisioners.rb index d3b0d20ef..f5a4e12a6 100644 --- a/lib/vagrant/action/builtin/mixin_provisioners.rb +++ b/lib/vagrant/action/builtin/mixin_provisioners.rb @@ -41,6 +41,8 @@ module Vagrant return @_provisioner_instances.compact end + private + # Sorts provisioners based on order specified with before/after options # # TODO: make sure all defined provisioners work here (i.e. even thoughs defined in separate but loaded Vagrantfile) @@ -49,10 +51,6 @@ module Vagrant def sort_provisioner_instances(pvs) final_provs = [] root_provs = [] - dep_provs = [] - each_provs = [] - all_provs = [] - # extract root provisioners root_provs = pvs.map { |p,o| [p,o] if o[:before].nil? && o[:after].nil? }.reject(&:nil?) @@ -61,6 +59,10 @@ module Vagrant return pvs end + dep_provs = [] + each_provs = [] + all_provs = [] + # extract dependency provisioners dep_provs = pvs.map { |p,o| [p,o] if (!o[:before].nil? && !o[:before].is_a?(Symbol)) || (!o[:after].nil? && !o[:after].is_a?(Symbol)) }.reject(&:nil?) # extract each provisioners @@ -73,6 +75,7 @@ module Vagrant # insert provisioners in order final_provs = root_provs dep_provs.each do |p,options| + idx = 0 if options[:before] idx = final_provs.each_with_index.map { |(p,o), i| i if o[:name].to_s == options[:before] }.reject(&:nil?).first idx -= 1 unless idx == 0 @@ -84,20 +87,23 @@ module Vagrant end end + # Add :each and :all provisioners in reverse to preserve order in Vagrantfile + # add each to final array + # TODO: Account for additional tmp_final size after insert for BOTH before/after cases (if both shortcuts are used) tmp_final_provs = final_provs.dup - extra_index = 0 + before_extra_index = 1 # offset for final array size + after_extra_index = 0 each_provs.reverse_each do |p,options| + idx = 0 final_provs.each_with_index.map do |(prv,o), i| if options[:before] - idx = i-1 unless idx == 0 - idx += extra_index - extra_index += 1 + idx = (i+before_extra_index)-1 unless i == 0 + before_extra_index += 1 tmp_final_provs.insert(idx, [p,options]) elsif options[:after] - idx = i+1 - idx += extra_index - extra_index += 1 + idx = (i+after_extra_index)+1 + after_extra_index += 1 tmp_final_provs.insert(idx, [p,options]) end end diff --git a/test/unit/vagrant/action/builtin/mixin_provisioners_test.rb b/test/unit/vagrant/action/builtin/mixin_provisioners_test.rb new file mode 100644 index 000000000..3c591a288 --- /dev/null +++ b/test/unit/vagrant/action/builtin/mixin_provisioners_test.rb @@ -0,0 +1,203 @@ +require File.expand_path("../../../../base", __FILE__) +require Vagrant.source_root.join("plugins/kernel_v2/config/vm") + +require "vagrant/action/builtin/mixin_provisioners" + +describe Vagrant::Action::Builtin::MixinProvisioners do + include_context "unit" + + let(:sandbox) { isolated_environment } + let(:iso_env) do + # We have to create a Vagrantfile so there is a root path + sandbox.vagrantfile("") + sandbox.create_vagrant_env + end + + let(:provisioner_config){ {} } + let(:provisioner_one) do + prov = VagrantPlugins::Kernel_V2::VagrantConfigProvisioner.new("spec-test", :shell) + prov.config = provisioner_config + prov + end + let(:provisioner_two) do + prov = VagrantPlugins::Kernel_V2::VagrantConfigProvisioner.new("spec-test", :shell) + prov.config = provisioner_config + prov + end + + let(:provisioner_instances) { [provisioner_one,provisioner_two] } + + let(:ui) { double("ui") } + let(:vm) { double("vm", provisioners: provisioner_instances) } + let(:config) { double("config", vm: vm) } + let(:machine) { double("machine", ui: ui, config: config) } + + let(:env) {{ machine: machine, ui: machine.ui, root_path: Pathname.new(".") }} + + subject do + Class.new do + extend Vagrant::Action::Builtin::MixinProvisioners + end + end + + after do + sandbox.close + end + + describe "#provisioner_instances" do + 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.first).to be_a(VagrantPlugins::Shell::Provisioner) + shell_two = result[1] + expect(shell_two.first).to be_a(VagrantPlugins::Shell::Provisioner) + end + end + + context "#sort_provisioner_instances" do + describe "with no dependency provisioners" do + it "returns the original array" do + result = subject.provisioner_instances(env) + expect(result.size).to eq(provisioner_instances.size) + shell_one = result.first + expect(shell_one.first).to be_a(VagrantPlugins::Shell::Provisioner) + shell_two = result[1] + expect(shell_two.first).to be_a(VagrantPlugins::Shell::Provisioner) + end + end + + describe "with before and after dependency provisioners" do + let(:provisioner_config){ {} } + let(:provisioner_root) do + prov = VagrantPlugins::Kernel_V2::VagrantConfigProvisioner.new("root-test", :shell) + prov.config = provisioner_config + prov + end + let(:provisioner_before) do + prov = VagrantPlugins::Kernel_V2::VagrantConfigProvisioner.new("before-test", :shell) + prov.config = provisioner_config + prov.before = "root-test" + prov + end + let(:provisioner_after) do + prov = VagrantPlugins::Kernel_V2::VagrantConfigProvisioner.new("after-test", :shell) + prov.config = provisioner_config + prov.after = "root-test" + prov + end + let(:provisioner_instances) { [provisioner_root,provisioner_before,provisioner_after] } + + 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[1].last[:name]).to eq("root-test") + expect(result[2].last[:name]).to eq("after-test") + end + end + + describe "with before :each dependency provisioners" do + let(:provisioner_config){ {} } + let(:provisioner_root) do + prov = VagrantPlugins::Kernel_V2::VagrantConfigProvisioner.new("root-test", :shell) + prov.config = provisioner_config + prov + end + let(:provisioner_root2) do + prov = VagrantPlugins::Kernel_V2::VagrantConfigProvisioner.new("root2-test", :shell) + prov.config = provisioner_config + prov + end + let(:provisioner_before) do + prov = VagrantPlugins::Kernel_V2::VagrantConfigProvisioner.new("before-test", :shell) + prov.config = provisioner_config + prov.before = :each + prov + end + + let(:provisioner_instances) { [provisioner_root,provisioner_root2,provisioner_before] } + + 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[1].last[:name]).to eq("root-test") + expect(result[2].last[:name]).to eq("before-test") + expect(result[3].last[:name]).to eq("root2-test") + end + end + + describe "with after :each dependency provisioners" do + let(:provisioner_config){ {} } + let(:provisioner_root) do + prov = VagrantPlugins::Kernel_V2::VagrantConfigProvisioner.new("root-test", :shell) + prov.config = provisioner_config + prov + end + let(:provisioner_root2) do + prov = VagrantPlugins::Kernel_V2::VagrantConfigProvisioner.new("root2-test", :shell) + prov.config = provisioner_config + prov + end + let(:provisioner_after) do + prov = VagrantPlugins::Kernel_V2::VagrantConfigProvisioner.new("after-test", :shell) + prov.config = provisioner_config + prov.after = :each + prov + end + + let(:provisioner_instances) { [provisioner_root,provisioner_root2,provisioner_after] } + + 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[1].last[:name]).to eq("after-test") + expect(result[2].last[:name]).to eq("root2-test") + expect(result[3].last[:name]).to eq("after-test") + end + end + + describe "with before and after :each dependency provisioners" do + let(:provisioner_config){ {} } + let(:provisioner_root) do + prov = VagrantPlugins::Kernel_V2::VagrantConfigProvisioner.new("root-test", :shell) + prov.config = provisioner_config + prov + end + let(:provisioner_root2) do + prov = VagrantPlugins::Kernel_V2::VagrantConfigProvisioner.new("root2-test", :shell) + prov.config = provisioner_config + prov + end + let(:provisioner_after) do + prov = VagrantPlugins::Kernel_V2::VagrantConfigProvisioner.new("after-test", :shell) + prov.config = provisioner_config + prov.after = :each + prov + end + let(:provisioner_before) do + prov = VagrantPlugins::Kernel_V2::VagrantConfigProvisioner.new("before-test", :shell) + prov.config = provisioner_config + prov.before = :each + prov + end + + let(:provisioner_instances) { [provisioner_root,provisioner_root2,provisioner_before,provisioner_after] } + + it "puts the each shortcut provisioners in place" do + result = subject.provisioner_instances(env) + result.each do |p,o| + puts o[:name] + end + + expect(result[0].last[:name]).to eq("before-test") + expect(result[1].last[:name]).to eq("root-test") + expect(result[2].last[:name]).to eq("after-test") + expect(result[3].last[:name]).to eq("before-test") + expect(result[4].last[:name]).to eq("root2-test") + expect(result[5].last[:name]).to eq("after-test") + end + end + end +end From c189e4d255f4ef93ada8e6b4d2b2014c13c6c7b8 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Mon, 26 Aug 2019 16:05:02 -0700 Subject: [PATCH 14/29] Redo how each provisioners are sorted --- .../action/builtin/mixin_provisioners.rb | 30 ++++++++++++------- .../action/builtin/mixin_provisioners_test.rb | 1 + 2 files changed, 21 insertions(+), 10 deletions(-) diff --git a/lib/vagrant/action/builtin/mixin_provisioners.rb b/lib/vagrant/action/builtin/mixin_provisioners.rb index f5a4e12a6..98d510e61 100644 --- a/lib/vagrant/action/builtin/mixin_provisioners.rb +++ b/lib/vagrant/action/builtin/mixin_provisioners.rb @@ -90,20 +90,23 @@ module Vagrant # Add :each and :all provisioners in reverse to preserve order in Vagrantfile # add each to final array - # TODO: Account for additional tmp_final size after insert for BOTH before/after cases (if both shortcuts are used) + # TODO: This doesn't work if before each provisioners are defined before after provisioners + # + # Maybe do before and after individually instead?? tmp_final_provs = final_provs.dup - before_extra_index = 1 # offset for final array size - after_extra_index = 0 - each_provs.reverse_each do |p,options| - idx = 0 - final_provs.each_with_index.map do |(prv,o), i| + index_offset = 0 + final_provs.each_with_index.map do |(prv,o), i| + each_provs.reverse_each do |p, options| + idx = 0 if options[:before] - idx = (i+before_extra_index)-1 unless i == 0 - before_extra_index += 1 + idx = (i+index_offset+1)-1 unless i == 0 + puts "b: #{idx}" + index_offset += 1 tmp_final_provs.insert(idx, [p,options]) elsif options[:after] - idx = (i+after_extra_index)+1 - after_extra_index += 1 + idx = (i+index_offset)+1 + index_offset += 1 + puts "a: #{idx}" tmp_final_provs.insert(idx, [p,options]) end end @@ -133,6 +136,13 @@ module Vagrant # Return the type map @_provisioner_types end + + # @private + # Reset the cached values for platform. This is not considered a public + # API and should only be used for testing. + def self.reset! + instance_variables.each(&method(:remove_instance_variable)) + end end end end diff --git a/test/unit/vagrant/action/builtin/mixin_provisioners_test.rb b/test/unit/vagrant/action/builtin/mixin_provisioners_test.rb index 3c591a288..9b4934060 100644 --- a/test/unit/vagrant/action/builtin/mixin_provisioners_test.rb +++ b/test/unit/vagrant/action/builtin/mixin_provisioners_test.rb @@ -42,6 +42,7 @@ describe Vagrant::Action::Builtin::MixinProvisioners do after do sandbox.close + described_class.reset! end describe "#provisioner_instances" do From 271d427c57d9abd373c8c9f444b18f492f55e6d1 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Tue, 27 Aug 2019 11:15:44 -0700 Subject: [PATCH 15/29] Fix bug in :each provisioner sorting Ensure each provisioners are properly inserted into the final provisioner array --- .../action/builtin/mixin_provisioners.rb | 28 +++++++------------ .../action/builtin/mixin_provisioners_test.rb | 3 -- 2 files changed, 10 insertions(+), 21 deletions(-) diff --git a/lib/vagrant/action/builtin/mixin_provisioners.rb b/lib/vagrant/action/builtin/mixin_provisioners.rb index 98d510e61..4f1c318cb 100644 --- a/lib/vagrant/action/builtin/mixin_provisioners.rb +++ b/lib/vagrant/action/builtin/mixin_provisioners.rb @@ -45,8 +45,6 @@ module Vagrant # Sorts provisioners based on order specified with before/after options # - # TODO: make sure all defined provisioners work here (i.e. even thoughs defined in separate but loaded Vagrantfile) - # # @return [Array] def sort_provisioner_instances(pvs) final_provs = [] @@ -70,8 +68,6 @@ module Vagrant # extract all provisioners all_provs = pvs.map { |p,o| [p,o] if o[:before] == :all || o[:after] == :all }.reject(&:nil?) - # TODO: Log here, that provisioner order is being changed - # insert provisioners in order final_provs = root_provs dep_provs.each do |p,options| @@ -90,26 +86,22 @@ module Vagrant # Add :each and :all provisioners in reverse to preserve order in Vagrantfile # add each to final array - # TODO: This doesn't work if before each provisioners are defined before after provisioners - # - # Maybe do before and after individually instead?? - tmp_final_provs = final_provs.dup - index_offset = 0 + tmp_final_provs = [] final_provs.each_with_index.map do |(prv,o), i| + tmp_before = [] + tmp_after = [] + each_provs.reverse_each do |p, options| - idx = 0 if options[:before] - idx = (i+index_offset+1)-1 unless i == 0 - puts "b: #{idx}" - index_offset += 1 - tmp_final_provs.insert(idx, [p,options]) + tmp_before << [p,options] elsif options[:after] - idx = (i+index_offset)+1 - index_offset += 1 - puts "a: #{idx}" - tmp_final_provs.insert(idx, [p,options]) + tmp_after << [p,options] end end + + tmp_final_provs += tmp_before unless tmp_before.empty? + tmp_final_provs += [[prv,o]] + tmp_final_provs += tmp_after unless tmp_after.empty? end final_provs = tmp_final_provs diff --git a/test/unit/vagrant/action/builtin/mixin_provisioners_test.rb b/test/unit/vagrant/action/builtin/mixin_provisioners_test.rb index 9b4934060..708247787 100644 --- a/test/unit/vagrant/action/builtin/mixin_provisioners_test.rb +++ b/test/unit/vagrant/action/builtin/mixin_provisioners_test.rb @@ -188,9 +188,6 @@ describe Vagrant::Action::Builtin::MixinProvisioners do it "puts the each shortcut provisioners in place" do result = subject.provisioner_instances(env) - result.each do |p,o| - puts o[:name] - end expect(result[0].last[:name]).to eq("before-test") expect(result[1].last[:name]).to eq("root-test") From e56d2581ee67bfe47a0b7e8e8651e5b25ce6ffb3 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Tue, 27 Aug 2019 14:36:36 -0700 Subject: [PATCH 16/29] Add before/after all tests --- .../action/builtin/mixin_provisioners_test.rb | 37 +++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/test/unit/vagrant/action/builtin/mixin_provisioners_test.rb b/test/unit/vagrant/action/builtin/mixin_provisioners_test.rb index 708247787..63cb4def3 100644 --- a/test/unit/vagrant/action/builtin/mixin_provisioners_test.rb +++ b/test/unit/vagrant/action/builtin/mixin_provisioners_test.rb @@ -197,5 +197,42 @@ describe Vagrant::Action::Builtin::MixinProvisioners do expect(result[5].last[:name]).to eq("after-test") end end + + describe "with before and after :all dependency provisioners" do + let(:provisioner_config){ {} } + let(:provisioner_root) do + prov = VagrantPlugins::Kernel_V2::VagrantConfigProvisioner.new("root-test", :shell) + prov.config = provisioner_config + prov + end + let(:provisioner_root2) do + prov = VagrantPlugins::Kernel_V2::VagrantConfigProvisioner.new("root2-test", :shell) + prov.config = provisioner_config + prov + end + let(:provisioner_after) do + prov = VagrantPlugins::Kernel_V2::VagrantConfigProvisioner.new("after-test", :shell) + prov.config = provisioner_config + prov.after = :all + prov + end + let(:provisioner_before) do + prov = VagrantPlugins::Kernel_V2::VagrantConfigProvisioner.new("before-test", :shell) + prov.config = provisioner_config + prov.before = :all + prov + end + + let(:provisioner_instances) { [provisioner_root,provisioner_root2,provisioner_before,provisioner_after] } + + 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[1].last[:name]).to eq("root-test") + expect(result[2].last[:name]).to eq("root2-test") + expect(result[3].last[:name]).to eq("after-test") + end + end end end From 2677a721fba74e0683ececd689cf84114fdce7c0 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Tue, 27 Aug 2019 15:11:14 -0700 Subject: [PATCH 17/29] Update provisioner basic usage with dependency provisioners --- .../docs/provisioning/basic_usage.html.md | 107 ++++++++++++++++++ 1 file changed, 107 insertions(+) diff --git a/website/source/docs/provisioning/basic_usage.html.md b/website/source/docs/provisioning/basic_usage.html.md index b4d3dfe44..79ebf9226 100644 --- a/website/source/docs/provisioning/basic_usage.html.md +++ b/website/source/docs/provisioning/basic_usage.html.md @@ -14,6 +14,27 @@ While Vagrant offers multiple options for how you are able to provision your machine, there is a standard usage pattern as well as some important points common to all provisioners that are important to know. +## Options + +Every Vagrant provisioner accepts a few base options. The only required +option is what type a provisioner is: + + +* `name` (string) - The name of the provisioner. Note: if no `type` option is given, + this option _must_ be the type of provisioner it is. If you wish to give it a + different name you must also set the `type` option to define the kind of provisioner. +* `type` (string) - The class of provisioner to configure. (i.e. `"shell"` or `"file"`) +* `before` (string or symbol) - The exact name of an already defined provisioner + that _this_ provisioner should run before. If defined as a symbol, its only valid + values are `:each` or `:all`, which makes the provisioner run before each and + every root provisioner, or before all provisioners respectively. +* `after` (string or symbol) - The exact name of an already defined provisioner + that _this_ provisioner should run after. If defined as a symbol, its only valid + values are `:each` or `:all`, which makes the provisioner run after each and + every root provisioner, or before all provisioners respectively. + +More information about `before` and `after` options can be read [below](#dependency-provisioners). + ## Configuration First, every provisioner is configured within your @@ -226,3 +247,89 @@ Vagrant.configure("2") do |config| inline: "echo SECOND!" end ``` + +## Dependency Provisioners + +If a provisioner has been configured using the `before` or `after` options, it +is considered a _Dependency Provisioner_. This means it has been configured to +run before or after a _Root Provisioner_, which does not have the `before` or +`after` options configured. Dependency provisioners also have two valid shortcuts: +`:each` and `:all`. + +An example of these dependency provisioners can be seen below: + +```ruby +Vagrant.configure("2") do |config| + config.vm.provision "C", after: "B", type: "shell", inline:<<-SHELL + echo 'C' + SHELL + config.vm.provision "B", type: "shell", inline:<<-SHELL + echo 'B' + SHELL + config.vm.provision "D", type: "shell", inline:<<-SHELL + echo 'D' + SHELL + config.vm.provision "A", before: "B", type: "shell", inline:<<-SHELL + echo 'A' + SHELL + config.vm.provision "Separate After", after: :each, type: "shell", inline:<<-SHELL + echo '==============================' + SHELL + config.vm.provision "Separate Before", before: :each, type: "shell", inline:<<-SHELL + echo '++++++++++++++++++++++++++++++' + SHELL + config.vm.provision "Hello", before: :all, type: "shell", inline:<<-SHELL + echo 'HERE WE GO!!' + SHELL + config.vm.provision "Goodbye", after: :all, type: "shell", inline:<<-SHELL + echo 'The end' + SHELL +end +``` + +The result of running `vagrant provision` with a guest configured above: + +``` +==> default: Running provisioner: Hello (shell)... + default: Running: inline script + default: HERE WE GO!! +==> default: Running provisioner: Separate Before (shell)... + default: Running: inline script + default: ++++++++++++++++++++++++++++++ +==> default: Running provisioner: A (shell)... + default: Running: inline script + default: A +==> default: Running provisioner: Separate After (shell)... + default: Running: inline script + default: ============================== +==> default: Running provisioner: Separate Before (shell)... + default: Running: inline script + default: ++++++++++++++++++++++++++++++ +==> default: Running provisioner: B (shell)... + default: Running: inline script + default: B +==> default: Running provisioner: Separate After (shell)... + default: Running: inline script + default: ============================== +==> default: Running provisioner: Separate Before (shell)... + default: Running: inline script + default: ++++++++++++++++++++++++++++++ +==> default: Running provisioner: C (shell)... + default: Running: inline script + default: C +==> default: Running provisioner: Separate After (shell)... + default: Running: inline script + default: ============================== +==> default: Running provisioner: Separate Before (shell)... + default: Running: inline script + default: ++++++++++++++++++++++++++++++ +==> default: Running provisioner: D (shell)... + default: Running: inline script + default: D +==> default: Running provisioner: Separate After (shell)... + default: Running: inline script + default: ============================== +==> default: Running provisioner: Goodbye (shell)... + default: Running: inline script + default: The end +``` From 5400af7a4aedeeb99f335625e5624cd925a2340d Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Tue, 27 Aug 2019 15:37:39 -0700 Subject: [PATCH 18/29] Add note about dependency provisioner dependencies --- website/source/docs/provisioning/basic_usage.html.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/website/source/docs/provisioning/basic_usage.html.md b/website/source/docs/provisioning/basic_usage.html.md index 79ebf9226..eecd4a2b8 100644 --- a/website/source/docs/provisioning/basic_usage.html.md +++ b/website/source/docs/provisioning/basic_usage.html.md @@ -256,6 +256,13 @@ run before or after a _Root Provisioner_, which does not have the `before` or `after` options configured. Dependency provisioners also have two valid shortcuts: `:each` and `:all`. +**TODO: Add good real example here** + +**Note**: As of 2.2.6, dependency provisioners cannot rely on other dependency +provisioners and is considered a configuration state error in Vagrant. If you must +order dependency provisioners, you can still order them by the order they are defined +inside your Vagrantfile. + An example of these dependency provisioners can be seen below: ```ruby From c1f0bd638d9640987030bb1ee6ba3427c3a5e2aa Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Wed, 28 Aug 2019 14:11:25 -0700 Subject: [PATCH 19/29] Insert _at_ index, rather than before Inserting at the right index places new item *before* the index, so no need to decrement and then insert, since `insert` takes care of shifting down elements of the array. --- lib/vagrant/action/builtin/mixin_provisioners.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/vagrant/action/builtin/mixin_provisioners.rb b/lib/vagrant/action/builtin/mixin_provisioners.rb index 4f1c318cb..a5ccd93ac 100644 --- a/lib/vagrant/action/builtin/mixin_provisioners.rb +++ b/lib/vagrant/action/builtin/mixin_provisioners.rb @@ -74,7 +74,6 @@ module Vagrant idx = 0 if options[:before] idx = final_provs.each_with_index.map { |(p,o), i| i if o[:name].to_s == options[:before] }.reject(&:nil?).first - idx -= 1 unless idx == 0 final_provs.insert(idx, [p, options]) elsif options[:after] idx = final_provs.each_with_index.map { |(p,o), i| i if o[:name].to_s == options[:after] }.reject(&:nil?).first From 8c39d9bfed67930b4fdfefc440b32bf1d6ca6393 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Wed, 28 Aug 2019 14:30:32 -0700 Subject: [PATCH 20/29] Add dependency provisioner name to error message --- plugins/kernel_v2/config/vm_provisioner.rb | 6 ++++-- templates/locales/en.yml | 2 +- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/plugins/kernel_v2/config/vm_provisioner.rb b/plugins/kernel_v2/config/vm_provisioner.rb index 309281d27..9ba6f639d 100644 --- a/plugins/kernel_v2/config/vm_provisioner.rb +++ b/plugins/kernel_v2/config/vm_provisioner.rb @@ -125,7 +125,8 @@ module VagrantPlugins errors << I18n.t("vagrant.provisioners.base.missing_provisioner_name", name: @before, machine_name: machine.name, - action: "before") + action: "before", + provisioner_name: @name) end end end @@ -142,7 +143,8 @@ module VagrantPlugins errors << I18n.t("vagrant.provisioners.base.missing_provisioner_name", name: @after, machine_name: machine.name, - action: "after") + action: "after", + provisioner_name: @name) end end end diff --git a/templates/locales/en.yml b/templates/locales/en.yml index 3590cdb9b..a28d86634 100644 --- a/templates/locales/en.yml +++ b/templates/locales/en.yml @@ -2490,7 +2490,7 @@ en: invalid_alias_value: |- Provisioner option `%{opt}` is not set as a valid type. Must be a string, or one of the alias shortcuts: %{alias} missing_provisioner_name: |- - Could not find provisioner name `%{name}` defined for machine `%{machine_name}` to run provisioner `%{action}`. + Could not find provisioner name `%{name}` defined for machine `%{machine_name}` to run provisioner `%{action}` "%{provisioner_name}". wrong_type: |- Provisioner option `%{opt}` is not set as a valid type. Must be a string. chef: From 160ee09da2a4b28bd2c6fbaba6c9f0bd0ad7c0ba Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Wed, 28 Aug 2019 14:32:51 -0700 Subject: [PATCH 21/29] Cleanup sorting method for provisioner sorting --- lib/vagrant/action/builtin/mixin_provisioners.rb | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/lib/vagrant/action/builtin/mixin_provisioners.rb b/lib/vagrant/action/builtin/mixin_provisioners.rb index a5ccd93ac..35ca0aebb 100644 --- a/lib/vagrant/action/builtin/mixin_provisioners.rb +++ b/lib/vagrant/action/builtin/mixin_provisioners.rb @@ -57,6 +57,7 @@ module Vagrant return pvs end + # ensure placeholder variables are Arrays dep_provs = [] each_provs = [] all_provs = [] @@ -83,8 +84,6 @@ module Vagrant end # Add :each and :all provisioners in reverse to preserve order in Vagrantfile - - # add each to final array tmp_final_provs = [] final_provs.each_with_index.map do |(prv,o), i| tmp_before = [] @@ -104,16 +103,14 @@ module Vagrant end final_provs = tmp_final_provs - # add all to final array - tmp_final_provs = final_provs.dup + # Add all to final array all_provs.reverse_each do |p,options| if options[:before] - tmp_final_provs.insert(0, [p,options]) + final_provs.insert(0, [p,options]) elsif options[:after] - tmp_final_provs.push([p,options]) + final_provs.push([p,options]) end end - final_provs = tmp_final_provs return final_provs end From 963319d93835a53ea9e3ec8ab9bdacd080b74bbf Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Wed, 28 Aug 2019 14:39:42 -0700 Subject: [PATCH 22/29] Flip around action to be after dependency provisioner name --- templates/locales/en.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/templates/locales/en.yml b/templates/locales/en.yml index a28d86634..f850324c6 100644 --- a/templates/locales/en.yml +++ b/templates/locales/en.yml @@ -2490,7 +2490,7 @@ en: invalid_alias_value: |- Provisioner option `%{opt}` is not set as a valid type. Must be a string, or one of the alias shortcuts: %{alias} missing_provisioner_name: |- - Could not find provisioner name `%{name}` defined for machine `%{machine_name}` to run provisioner `%{action}` "%{provisioner_name}". + Could not find provisioner name `%{name}` defined for machine `%{machine_name}` to run provisioner "%{provisioner_name}" `%{action}`. wrong_type: |- Provisioner option `%{opt}` is not set as a valid type. Must be a string. chef: From c192651e90a43f54f3820180056ead33d8453bf5 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Wed, 28 Aug 2019 15:09:38 -0700 Subject: [PATCH 23/29] Make dependency provisioners experimental --- plugins/kernel_v2/config/vm.rb | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/plugins/kernel_v2/config/vm.rb b/plugins/kernel_v2/config/vm.rb index 23a5a4352..25bc74762 100644 --- a/plugins/kernel_v2/config/vm.rb +++ b/plugins/kernel_v2/config/vm.rb @@ -7,6 +7,7 @@ require "vagrant/action/builtin/mixin_synced_folders" require "vagrant/config/v2/util" require "vagrant/util/platform" require "vagrant/util/presence" +require "vagrant/util/experimental" require File.expand_path("../vm_provisioner", __FILE__) require File.expand_path("../vm_subvm", __FILE__) @@ -338,7 +339,11 @@ module VagrantPlugins after = options.delete(:after) end - prov = VagrantConfigProvisioner.new(name, type.to_sym, before, after) + if Vagrant::Util::Experimental.feature_enabled?("dependency_provisioners") + prov = VagrantConfigProvisioner.new(name, type.to_sym, before, after) + else + prov = VagrantConfigProvisioner.new(name, type.to_sym) + end @provisioners << prov end From f608c324e5556fa932593a65918771b3c570dab6 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Wed, 28 Aug 2019 15:09:48 -0700 Subject: [PATCH 24/29] Update dependency provisioner docs --- website/source/docs/experimental/index.html.md | 7 +++++++ .../source/docs/provisioning/basic_usage.html.md | 16 ++++++++++++++-- 2 files changed, 21 insertions(+), 2 deletions(-) diff --git a/website/source/docs/experimental/index.html.md b/website/source/docs/experimental/index.html.md index 5d0c7d34b..5de2365d7 100644 --- a/website/source/docs/experimental/index.html.md +++ b/website/source/docs/experimental/index.html.md @@ -47,3 +47,10 @@ This is a list of all the valid experimental features that Vagrant recognizes: Enabling this feature allows triggers to recognize and execute `:type` triggers. More information about how these should be used can be found on the [trigger documentation page](/docs/triggers/configuration.html#trigger-types) + +### `dependency_provisioners` + +Enabling this feature allows all provisioners to specify `before` and `after` +options. These options allow provisioners to be configured to run before or after +any given "root" provisioner. more information about these options can be found +on the [base provisioner documentation page](/docs/provisioning/basic_usage.html) diff --git a/website/source/docs/provisioning/basic_usage.html.md b/website/source/docs/provisioning/basic_usage.html.md index eecd4a2b8..724e8805f 100644 --- a/website/source/docs/provisioning/basic_usage.html.md +++ b/website/source/docs/provisioning/basic_usage.html.md @@ -28,12 +28,16 @@ option is what type a provisioner is: that _this_ provisioner should run before. If defined as a symbol, its only valid values are `:each` or `:all`, which makes the provisioner run before each and every root provisioner, or before all provisioners respectively. + __Note__: This option is currently experimental, so it needs to be explicitly + enabled to work. More info can be found [here](/docs/experimental/index.html). * `after` (string or symbol) - The exact name of an already defined provisioner that _this_ provisioner should run after. If defined as a symbol, its only valid values are `:each` or `:all`, which makes the provisioner run after each and every root provisioner, or before all provisioners respectively. + __Note__: This option is currently experimental, so it needs to be explicitly + enabled to work. More info can be found [here](/docs/experimental/index.html). -More information about `before` and `after` options can be read [below](#dependency-provisioners). +More information about how to use `before` and `after` options can be read [below](#dependency-provisioners). ## Configuration @@ -250,10 +254,18 @@ end ## Dependency Provisioners +
+ Warning: Advanced Topic! Dependency provisioners are + an advanced topic. If you are just getting started with Vagrant, you can + safely skip this. +
+ If a provisioner has been configured using the `before` or `after` options, it is considered a _Dependency Provisioner_. This means it has been configured to run before or after a _Root Provisioner_, which does not have the `before` or -`after` options configured. Dependency provisioners also have two valid shortcuts: +`after` options configured. + +Dependency provisioners also have two valid shortcuts: `:each` and `:all`. **TODO: Add good real example here** From fc8bf6aed4f0868a7d04c446ce7f29f60e581db6 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Wed, 28 Aug 2019 15:52:38 -0700 Subject: [PATCH 25/29] Ensure a dependency provisioner isnt configured to rely on another dependency provisioner --- plugins/kernel_v2/config/vm.rb | 3 +-- plugins/kernel_v2/config/vm_provisioner.rb | 21 +++++++++++++++++++-- templates/locales/en.yml | 2 ++ 3 files changed, 22 insertions(+), 4 deletions(-) diff --git a/plugins/kernel_v2/config/vm.rb b/plugins/kernel_v2/config/vm.rb index 25bc74762..e7196a8ea 100644 --- a/plugins/kernel_v2/config/vm.rb +++ b/plugins/kernel_v2/config/vm.rb @@ -772,8 +772,7 @@ module VagrantPlugins next end - provisioner_names = @provisioners.map { |i| i.name if i.name != vm_provisioner.name }.reject(&:nil?) - provisioner_errors = vm_provisioner.validate(machine, provisioner_names) + provisioner_errors = vm_provisioner.validate(machine, @provisioners) if provisioner_errors errors = Vagrant::Config::V2::Util.merge_errors(errors, provisioner_errors) end diff --git a/plugins/kernel_v2/config/vm_provisioner.rb b/plugins/kernel_v2/config/vm_provisioner.rb index 9ba6f639d..61f16ba4b 100644 --- a/plugins/kernel_v2/config/vm_provisioner.rb +++ b/plugins/kernel_v2/config/vm_provisioner.rb @@ -108,11 +108,13 @@ module VagrantPlugins # Validates the before/after options # # @param [Vagrant::Machine] machine - machine to validate against - # @param [Array] provisioner_names - Names of provisioners for a given machine + # @param [Array] provisioners - Array of defined provisioners for the guest machine # @return [Array] array of strings of error messages from config option validation - def validate(machine, provisioner_names) + def validate(machine, provisioners) errors = _detected_errors + provisioner_names = provisioners.map { |i| i.name if i.name != name }.reject(&:nil?) + if @before if !VALID_BEFORE_AFTER_TYPES.include?(@before) if @before.is_a?(Symbol) && !VALID_BEFORE_AFTER_TYPES.include?(@before) @@ -128,6 +130,14 @@ module VagrantPlugins action: "before", provisioner_name: @name) end + + dep_prov = provisioners.map { |i| i if i.name.to_s == @before && (i.before || i.after) }.reject(&:nil?) + + if !dep_prov.empty? + errors << I18n.t("vagrant.provisioners.base.dependency_provisioner_dependency", + name: @name, + dep_name: dep_prov.first.name.to_s) + end end end @@ -146,6 +156,13 @@ module VagrantPlugins action: "after", provisioner_name: @name) end + + dep_prov = provisioners.map { |i| i if i.name.to_s == @after && (i.before || i.after) }.reject(&:nil?) + if !dep_prov.empty? + errors << I18n.t("vagrant.provisioners.base.dependency_provisioner_dependency", + name: @name, + dep_name: dep_prov.first.name.to_s) + end end end diff --git a/templates/locales/en.yml b/templates/locales/en.yml index f850324c6..5629125d3 100644 --- a/templates/locales/en.yml +++ b/templates/locales/en.yml @@ -2487,6 +2487,8 @@ en: provisioners: base: + dependency_provisioner_dependency: |- + Dependency provisioner "%{name}" relies on another dependency provisioner "%{dep_name}". This is currently not supported. invalid_alias_value: |- Provisioner option `%{opt}` is not set as a valid type. Must be a string, or one of the alias shortcuts: %{alias} missing_provisioner_name: |- From 258ce7733c5b5795b3d96fa9c7cfdb444eb7fa30 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Wed, 28 Aug 2019 16:09:02 -0700 Subject: [PATCH 26/29] Remove todo note from docs --- website/source/docs/provisioning/basic_usage.html.md | 2 -- 1 file changed, 2 deletions(-) diff --git a/website/source/docs/provisioning/basic_usage.html.md b/website/source/docs/provisioning/basic_usage.html.md index 724e8805f..c514b7663 100644 --- a/website/source/docs/provisioning/basic_usage.html.md +++ b/website/source/docs/provisioning/basic_usage.html.md @@ -268,8 +268,6 @@ run before or after a _Root Provisioner_, which does not have the `before` or Dependency provisioners also have two valid shortcuts: `:each` and `:all`. -**TODO: Add good real example here** - **Note**: As of 2.2.6, dependency provisioners cannot rely on other dependency provisioners and is considered a configuration state error in Vagrant. If you must order dependency provisioners, you can still order them by the order they are defined From 07bcfc6077fbee7a17cbbc75f9db727f0aa6e91b Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Wed, 28 Aug 2019 16:15:33 -0700 Subject: [PATCH 27/29] Add error if both before and after options are set --- plugins/kernel_v2/config/vm_provisioner.rb | 4 ++++ templates/locales/en.yml | 2 ++ 2 files changed, 6 insertions(+) diff --git a/plugins/kernel_v2/config/vm_provisioner.rb b/plugins/kernel_v2/config/vm_provisioner.rb index 61f16ba4b..a8e106f7a 100644 --- a/plugins/kernel_v2/config/vm_provisioner.rb +++ b/plugins/kernel_v2/config/vm_provisioner.rb @@ -115,6 +115,10 @@ module VagrantPlugins provisioner_names = provisioners.map { |i| i.name if i.name != name }.reject(&:nil?) + if @before && @after + errors << I18n.t("vagrant.provisioners.base.both_before_after_set") + end + if @before if !VALID_BEFORE_AFTER_TYPES.include?(@before) if @before.is_a?(Symbol) && !VALID_BEFORE_AFTER_TYPES.include?(@before) diff --git a/templates/locales/en.yml b/templates/locales/en.yml index 5629125d3..49d99187d 100644 --- a/templates/locales/en.yml +++ b/templates/locales/en.yml @@ -2487,6 +2487,8 @@ en: provisioners: base: + both_before_after_set: |- + Dependency provisioners cannot currently set both `before` and `after` options. dependency_provisioner_dependency: |- Dependency provisioner "%{name}" relies on another dependency provisioner "%{dep_name}". This is currently not supported. invalid_alias_value: |- From 7b0dc8d528002d6aee73485822f0668b3c9c8389 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Thu, 29 Aug 2019 13:50:22 -0700 Subject: [PATCH 28/29] Update provisioner enhancements from pull request feedback --- .../action/builtin/mixin_provisioners.rb | 14 +++++++------- plugins/kernel_v2/config/vm.rb | 3 ++- plugins/kernel_v2/config/vm_provisioner.rb | 17 +++++++++-------- .../docs/provisioning/basic_usage.html.md | 16 ++++++++++++++++ 4 files changed, 34 insertions(+), 16 deletions(-) diff --git a/lib/vagrant/action/builtin/mixin_provisioners.rb b/lib/vagrant/action/builtin/mixin_provisioners.rb index 35ca0aebb..ac5c89cab 100644 --- a/lib/vagrant/action/builtin/mixin_provisioners.rb +++ b/lib/vagrant/action/builtin/mixin_provisioners.rb @@ -50,7 +50,7 @@ module Vagrant final_provs = [] root_provs = [] # extract root provisioners - root_provs = pvs.map { |p,o| [p,o] if o[:before].nil? && o[:after].nil? }.reject(&:nil?) + root_provs = pvs.find_all { |_, o| o[:before].nil? && o[:after].nil? } if root_provs.size == pvs.size # no dependencies found @@ -63,21 +63,21 @@ module Vagrant all_provs = [] # extract dependency provisioners - dep_provs = pvs.map { |p,o| [p,o] if (!o[:before].nil? && !o[:before].is_a?(Symbol)) || (!o[:after].nil? && !o[:after].is_a?(Symbol)) }.reject(&:nil?) + dep_provs = pvs.find_all { |_, o| o[:before].is_a?(String) || o[:after].is_a?(String) } # extract each provisioners - each_provs = pvs.map { |p,o| [p,o] if o[:before] == :each || o[:after] == :each }.reject(&:nil?) + each_provs = pvs.find_all { |_,o| o[:before] == :each || o[:after] == :each } # extract all provisioners - all_provs = pvs.map { |p,o| [p,o] if o[:before] == :all || o[:after] == :all }.reject(&:nil?) + all_provs = pvs.find_all { |_,o| o[:before] == :all || o[:after] == :all } # insert provisioners in order final_provs = root_provs dep_provs.each do |p,options| idx = 0 if options[:before] - idx = final_provs.each_with_index.map { |(p,o), i| i if o[:name].to_s == options[:before] }.reject(&:nil?).first + idx = final_provs.index { |_, o| o[:name].to_s == options[:before] } final_provs.insert(idx, [p, options]) elsif options[:after] - idx = final_provs.each_with_index.map { |(p,o), i| i if o[:name].to_s == options[:after] }.reject(&:nil?).first + idx = final_provs.index { |_, o| o[:name].to_s == options[:after] } idx += 1 final_provs.insert(idx, [p, options]) end @@ -85,7 +85,7 @@ module Vagrant # Add :each and :all provisioners in reverse to preserve order in Vagrantfile tmp_final_provs = [] - final_provs.each_with_index.map do |(prv,o), i| + final_provs.each_with_index do |(prv,o), i| tmp_before = [] tmp_after = [] diff --git a/plugins/kernel_v2/config/vm.rb b/plugins/kernel_v2/config/vm.rb index e7196a8ea..3169b0552 100644 --- a/plugins/kernel_v2/config/vm.rb +++ b/plugins/kernel_v2/config/vm.rb @@ -340,7 +340,8 @@ module VagrantPlugins end if Vagrant::Util::Experimental.feature_enabled?("dependency_provisioners") - prov = VagrantConfigProvisioner.new(name, type.to_sym, before, after) + opts = {before: before, after: after} + prov = VagrantConfigProvisioner.new(name, type.to_sym, opts) else prov = VagrantConfigProvisioner.new(name, type.to_sym) end diff --git a/plugins/kernel_v2/config/vm_provisioner.rb b/plugins/kernel_v2/config/vm_provisioner.rb index a8e106f7a..da57d1808 100644 --- a/plugins/kernel_v2/config/vm_provisioner.rb +++ b/plugins/kernel_v2/config/vm_provisioner.rb @@ -53,7 +53,7 @@ module VagrantPlugins # @return [String, Symbol] attr_accessor :after - def initialize(name, type, before=nil, after=nil) + def initialize(name, type, **options) @logger = Log4r::Logger.new("vagrant::config::vm::provisioner") @logger.debug("Provisioner defined: #{name}") @@ -64,8 +64,8 @@ module VagrantPlugins @preserve_order = false @run = nil @type = type - @before = before - @after = after + @before = options[:before] + @after = options[:after] # Attempt to find the provisioner... if !Vagrant.plugin("2").manager.provisioners[type] @@ -113,7 +113,7 @@ module VagrantPlugins def validate(machine, provisioners) errors = _detected_errors - provisioner_names = provisioners.map { |i| i.name if i.name != name }.reject(&:nil?) + provisioner_names = provisioners.map { |i| i.name.to_s if i.name != name }.reject(&:nil?) if @before && @after errors << I18n.t("vagrant.provisioners.base.both_before_after_set") @@ -127,7 +127,7 @@ module VagrantPlugins errors << I18n.t("vagrant.provisioners.base.wrong_type", opt: "before") end - if !provisioner_names.include?(@before.to_sym) + if !provisioner_names.include?(@before) errors << I18n.t("vagrant.provisioners.base.missing_provisioner_name", name: @before, machine_name: machine.name, @@ -135,7 +135,7 @@ module VagrantPlugins provisioner_name: @name) end - dep_prov = provisioners.map { |i| i if i.name.to_s == @before && (i.before || i.after) }.reject(&:nil?) + dep_prov = provisioners.find_all { |i| i.name.to_s == @before && (i.before || i.after) } if !dep_prov.empty? errors << I18n.t("vagrant.provisioners.base.dependency_provisioner_dependency", @@ -153,7 +153,7 @@ module VagrantPlugins errors << I18n.t("vagrant.provisioners.base.wrong_type", opt: "after") end - if !provisioner_names.include?(@after.to_sym) + if !provisioner_names.include?(@after) errors << I18n.t("vagrant.provisioners.base.missing_provisioner_name", name: @after, machine_name: machine.name, @@ -161,7 +161,8 @@ module VagrantPlugins provisioner_name: @name) end - dep_prov = provisioners.map { |i| i if i.name.to_s == @after && (i.before || i.after) }.reject(&:nil?) + dep_prov = provisioners.find_all { |i| i.name.to_s == @after && (i.before || i.after) } + if !dep_prov.empty? errors << I18n.t("vagrant.provisioners.base.dependency_provisioner_dependency", name: @name, diff --git a/website/source/docs/provisioning/basic_usage.html.md b/website/source/docs/provisioning/basic_usage.html.md index c514b7663..43c20cb55 100644 --- a/website/source/docs/provisioning/basic_usage.html.md +++ b/website/source/docs/provisioning/basic_usage.html.md @@ -260,6 +260,22 @@ end safely skip this. +
+ Warning! This feature is still experimental and may break or + change in between releases. Use at your own risk. + + This feature currently reqiures the experimental flag to be used. To explicitly enable this feature, you can set the experimental flag to: + + ``` + VAGRANT_EXPERIMENTAL="dependency_provisioners" + ``` + + Please note that `VAGRANT_EXPERIMENTAL` is an environment variable. For more + information about this flag visit the [Experimental docs page](/docs/experimental/) + for more info. Without this flag enabled, provisioners with the `before` and + `after` option will be ignored. +
+ If a provisioner has been configured using the `before` or `after` options, it is considered a _Dependency Provisioner_. This means it has been configured to run before or after a _Root Provisioner_, which does not have the `before` or From 99b58675599b7ca8a0a6a685419ea3387c7d41c0 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Thu, 5 Sep 2019 15:39:24 -0700 Subject: [PATCH 29/29] Update reject to compact --- plugins/kernel_v2/config/vm_provisioner.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/kernel_v2/config/vm_provisioner.rb b/plugins/kernel_v2/config/vm_provisioner.rb index da57d1808..d0c545ba9 100644 --- a/plugins/kernel_v2/config/vm_provisioner.rb +++ b/plugins/kernel_v2/config/vm_provisioner.rb @@ -113,7 +113,7 @@ module VagrantPlugins def validate(machine, provisioners) errors = _detected_errors - provisioner_names = provisioners.map { |i| i.name.to_s if i.name != name }.reject(&:nil?) + provisioner_names = provisioners.map { |i| i.name.to_s if i.name != name }.compact if @before && @after errors << I18n.t("vagrant.provisioners.base.both_before_after_set")