From 16c9e88b1cadbcece9261138b1cec498db8345a1 Mon Sep 17 00:00:00 2001 From: sophia Date: Tue, 5 May 2020 10:53:51 -0400 Subject: [PATCH 1/7] Add communicator_require option to provisioners --- plugins/kernel_v2/config/vm.rb | 2 ++ plugins/kernel_v2/config/vm_provisioner.rb | 7 +++++++ 2 files changed, 9 insertions(+) diff --git a/plugins/kernel_v2/config/vm.rb b/plugins/kernel_v2/config/vm.rb index b167ab270..ce01deb3f 100644 --- a/plugins/kernel_v2/config/vm.rb +++ b/plugins/kernel_v2/config/vm.rb @@ -381,6 +381,8 @@ module VagrantPlugins prov.preserve_order = !!options.delete(:preserve_order) if \ options.key?(:preserve_order) prov.run = options.delete(:run) if options.key?(:run) + prov.communicator_required = options.delete(:communicator_required) if options.key?(:communicator_required) + prov.add_config(options, &block) nil end diff --git a/plugins/kernel_v2/config/vm_provisioner.rb b/plugins/kernel_v2/config/vm_provisioner.rb index 97f838e6e..cdd0646d3 100644 --- a/plugins/kernel_v2/config/vm_provisioner.rb +++ b/plugins/kernel_v2/config/vm_provisioner.rb @@ -56,6 +56,12 @@ module VagrantPlugins # @return [String, Symbol] attr_accessor :after + # Boolean, when true signifies that some communicator must + # be available in order for the provisioner to run. + # + # @return [Boolean] + attr_accessor :communicator_required + def initialize(name, type, **options) @logger = Log4r::Logger.new("vagrant::config::vm::provisioner") @logger.debug("Provisioner defined: #{name}") @@ -69,6 +75,7 @@ module VagrantPlugins @type = type @before = options[:before] @after = options[:after] + @communicator_required = options[:communicator_required] || true # Attempt to find the provisioner... if !Vagrant.plugin("2").manager.provisioners[type] From 1a3e6c8ed5e3558a9a4b6696cfb22a9368f45234 Mon Sep 17 00:00:00 2001 From: sophia Date: Tue, 5 May 2020 12:45:22 -0400 Subject: [PATCH 2/7] Allow provisioner to take list of provisioners to run --- lib/vagrant/action/builtin/mixin_provisioners.rb | 2 ++ lib/vagrant/action/builtin/provision.rb | 11 +++++++++-- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/lib/vagrant/action/builtin/mixin_provisioners.rb b/lib/vagrant/action/builtin/mixin_provisioners.rb index 32e7fa7d5..42e7bf059 100644 --- a/lib/vagrant/action/builtin/mixin_provisioners.rb +++ b/lib/vagrant/action/builtin/mixin_provisioners.rb @@ -49,6 +49,8 @@ module Vagrant run: provisioner.run, before: provisioner.before, after: provisioner.after, + communicator_required: provisioner.communicator_required, + id: provisioner.id } # Return the result diff --git a/lib/vagrant/action/builtin/provision.rb b/lib/vagrant/action/builtin/provision.rb index 6c26121b7..168d15b2d 100644 --- a/lib/vagrant/action/builtin/provision.rb +++ b/lib/vagrant/action/builtin/provision.rb @@ -14,9 +14,10 @@ module Vagrant class Provision include MixinProvisioners - def initialize(app, env) + def initialize(app, env, ids=nil) @app = app @logger = Log4r::Logger.new("vagrant::action::builtin::provision") + @provisioner_ids = ids end def call(env) @@ -100,7 +101,13 @@ module Vagrant end type_map = provisioner_type_map(env) - provisioner_instances(env).each do |p, options| + if @provisioner_ids + provisioners = provisioner_instances(env).map { |p, opts| [p, opts] if @provisioner_ids.include?(opts[:id]) }.compact + else + provisioners = provisioner_instances(env) + end + + provisioners.each do |p, options| type_name = type_map[p] if options[:run] == :never From 33576b584602bb532f4e7631e8bbdfce0d61194a Mon Sep 17 00:00:00 2001 From: sophia Date: Tue, 5 May 2020 13:19:14 -0400 Subject: [PATCH 3/7] Docker provider to run provisioners if available --- plugins/providers/docker/action.rb | 15 ++++----- .../docker/action/has_provisioner.rb | 32 +++++++++++++++++++ 2 files changed, 39 insertions(+), 8 deletions(-) create mode 100644 plugins/providers/docker/action/has_provisioner.rb diff --git a/plugins/providers/docker/action.rb b/plugins/providers/docker/action.rb index f0a2d7a05..eda05cffb 100644 --- a/plugins/providers/docker/action.rb +++ b/plugins/providers/docker/action.rb @@ -216,14 +216,12 @@ module VagrantPlugins Vagrant::Action::Builder.new.tap do |b| b.use Call, IsState, :running do |env, b2| if env[:machine_action] != :run_command - b2.use Call, HasSSH do |env2, b3| - if env2[:result] - b3.use Provision - else - b3.use Message, - I18n.t("docker_provider.messages.provision_no_ssh"), - post: true - end + b2.use Call, HasProvisioner do |env2, b3| + ids = env2[:run].map { |r| r.id } + name_type = env2[:skip].map { |r| [r.name, r.type] } + b3.use Provision, ids + # TODO: fix message + b3.use Message, "skipping provisioners: #{name_type}" end end @@ -304,6 +302,7 @@ module VagrantPlugins autoload :DestroyNetwork, action_root.join("destroy_network") autoload :ForwardedPorts, action_root.join("forwarded_ports") autoload :HasSSH, action_root.join("has_ssh") + autoload :HasProvisioner, action_root.join("has_provisioner") autoload :HostMachine, action_root.join("host_machine") autoload :HostMachineBuildDir, action_root.join("host_machine_build_dir") autoload :HostMachinePortChecker, action_root.join("host_machine_port_checker") diff --git a/plugins/providers/docker/action/has_provisioner.rb b/plugins/providers/docker/action/has_provisioner.rb new file mode 100644 index 000000000..365a84ab4 --- /dev/null +++ b/plugins/providers/docker/action/has_provisioner.rb @@ -0,0 +1,32 @@ +module VagrantPlugins + module DockerProvider + module Action + # This middleware is used with Call to test if this machine + # has available provisioners + class HasProvisioner + def initialize(app, env) + @app = app + end + + def call(env) + env[:run] = [] + env[:skip] = [] + has_ssh = env[:machine].provider_config.has_ssh + if has_ssh + env[:run] = env[:machine].config.vm.provisioners + else + env[:machine].config.vm.provisioners.each do |p| + if p.communicator_required + env[:skip].push(p) + else + env[:run].push(p) + end + end + end + + @app.call(env) + end + end + end + end +end From 9b6c562016baf3cf5a997dad1590b5a67002a16f Mon Sep 17 00:00:00 2001 From: sophia Date: Tue, 5 May 2020 18:37:18 -0400 Subject: [PATCH 4/7] Set run config for provisioner --- .../action/builtin/mixin_provisioners.rb | 1 - lib/vagrant/action/builtin/provision.rb | 11 ++------ plugins/providers/docker/action.rb | 27 ++++++++++--------- .../docker/action/has_provisioner.rb | 12 +++------ templates/locales/providers_docker.yml | 4 +++ 5 files changed, 24 insertions(+), 31 deletions(-) diff --git a/lib/vagrant/action/builtin/mixin_provisioners.rb b/lib/vagrant/action/builtin/mixin_provisioners.rb index 42e7bf059..07f14eb73 100644 --- a/lib/vagrant/action/builtin/mixin_provisioners.rb +++ b/lib/vagrant/action/builtin/mixin_provisioners.rb @@ -50,7 +50,6 @@ module Vagrant before: provisioner.before, after: provisioner.after, communicator_required: provisioner.communicator_required, - id: provisioner.id } # Return the result diff --git a/lib/vagrant/action/builtin/provision.rb b/lib/vagrant/action/builtin/provision.rb index 168d15b2d..6c26121b7 100644 --- a/lib/vagrant/action/builtin/provision.rb +++ b/lib/vagrant/action/builtin/provision.rb @@ -14,10 +14,9 @@ module Vagrant class Provision include MixinProvisioners - def initialize(app, env, ids=nil) + def initialize(app, env) @app = app @logger = Log4r::Logger.new("vagrant::action::builtin::provision") - @provisioner_ids = ids end def call(env) @@ -101,13 +100,7 @@ module Vagrant end type_map = provisioner_type_map(env) - if @provisioner_ids - provisioners = provisioner_instances(env).map { |p, opts| [p, opts] if @provisioner_ids.include?(opts[:id]) }.compact - else - provisioners = provisioner_instances(env) - end - - provisioners.each do |p, options| + provisioner_instances(env).each do |p, options| type_name = type_map[p] if options[:run] == :never diff --git a/plugins/providers/docker/action.rb b/plugins/providers/docker/action.rb index eda05cffb..f9180afd8 100644 --- a/plugins/providers/docker/action.rb +++ b/plugins/providers/docker/action.rb @@ -66,14 +66,11 @@ module VagrantPlugins next end - b3.use Call, HasSSH do |env3, b4| - if env3[:result] - b4.use Provision - else - b4.use Message, - I18n.t("docker_provider.messages.provision_no_ssh"), - post: true + b3.use Call, HasProvisioner do |env3, b4| + if env3[:skip].length > 0 + b4.use Message, I18n.t("docker_provider.messages.not_provisioning", provisioiners: check_skipped_provisioners(env3[:skip])) end + b4.use Provision end end end @@ -217,11 +214,10 @@ module VagrantPlugins b.use Call, IsState, :running do |env, b2| if env[:machine_action] != :run_command b2.use Call, HasProvisioner do |env2, b3| - ids = env2[:run].map { |r| r.id } - name_type = env2[:skip].map { |r| [r.name, r.type] } - b3.use Provision, ids - # TODO: fix message - b3.use Message, "skipping provisioners: #{name_type}" + if env2[:skip].length > 0 + b3.use Message, I18n.t("docker_provider.messages.not_provisioning", provisioiners: check_skipped_provisioners(env2[:skip])) + end + b3.use Provision end end @@ -323,6 +319,13 @@ module VagrantPlugins autoload :Start, action_root.join("start") autoload :Stop, action_root.join("stop") autoload :WaitForRunning, action_root.join("wait_for_running") + + private + + def self.check_skipped_provisioners(provisioners) + skipped_provisioners = provisioners.map { |p| "#{p.name || 'no name'}, type: #{p.type}" } + skipped_provisioners.join("\n- ") + end end end end diff --git a/plugins/providers/docker/action/has_provisioner.rb b/plugins/providers/docker/action/has_provisioner.rb index 365a84ab4..fdf324ce5 100644 --- a/plugins/providers/docker/action/has_provisioner.rb +++ b/plugins/providers/docker/action/has_provisioner.rb @@ -9,21 +9,15 @@ module VagrantPlugins end def call(env) - env[:run] = [] env[:skip] = [] - has_ssh = env[:machine].provider_config.has_ssh - if has_ssh - env[:run] = env[:machine].config.vm.provisioners - else + if !env[:machine].provider_config.has_ssh env[:machine].config.vm.provisioners.each do |p| if p.communicator_required - env[:skip].push(p) - else - env[:run].push(p) + env[:skip].push(p) + p.run = :never end end end - @app.call(env) end end diff --git a/templates/locales/providers_docker.yml b/templates/locales/providers_docker.yml index 0760b5567..88eb3448e 100644 --- a/templates/locales/providers_docker.yml +++ b/templates/locales/providers_docker.yml @@ -133,6 +133,10 @@ en: Stopping container... container_ready: |- Container started and ready for use! + not_provisioning: |- + The following provisioners require a communicator, though none is available (this container does not supprt SSH). + Not running the following provisioners: + - %{provisioiners} status: host_state_unknown: |- From 64f5a9e57f8dd3210ab876a63ab1624c2156016a Mon Sep 17 00:00:00 2001 From: sophia Date: Thu, 7 May 2020 15:20:23 -0400 Subject: [PATCH 5/7] Validate communicator type --- plugins/kernel_v2/config/vm_provisioner.rb | 8 ++++++-- templates/locales/en.yml | 2 +- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/plugins/kernel_v2/config/vm_provisioner.rb b/plugins/kernel_v2/config/vm_provisioner.rb index cdd0646d3..6a3042ad9 100644 --- a/plugins/kernel_v2/config/vm_provisioner.rb +++ b/plugins/kernel_v2/config/vm_provisioner.rb @@ -134,7 +134,7 @@ module VagrantPlugins 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") + errors << I18n.t("vagrant.provisioners.base.wrong_type", opt: "before", type: "string") end if !provisioner_names.include?(@before) @@ -160,7 +160,7 @@ module VagrantPlugins 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") + errors << I18n.t("vagrant.provisioners.base.wrong_type", opt: "after", type: "string") end if !provisioner_names.include?(@after) @@ -181,6 +181,10 @@ module VagrantPlugins end end + if !(@communicator_required.is_a?(TrueClass) || @communicator_required.is_a?(FalseClass)) + errors << I18n.t("vagrant.provisioners.base.wrong_type", opt: "communicator_required", type: "boolean") + end + {"provisioner" => errors} end diff --git a/templates/locales/en.yml b/templates/locales/en.yml index a6f8feea9..9f45f9cf7 100644 --- a/templates/locales/en.yml +++ b/templates/locales/en.yml @@ -2609,7 +2609,7 @@ en: missing_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. + Provisioner option `%{opt}` is not set as a valid type. Must be a %{type}. chef: chef_not_detected: |- The chef binary (either `chef-solo` or `chef-client`) was not found on From 672859e2967615b680334c8faf94f9b191ada62b Mon Sep 17 00:00:00 2001 From: sophia Date: Thu, 7 May 2020 16:23:58 -0400 Subject: [PATCH 6/7] Add tests and docs --- plugins/kernel_v2/config/vm_provisioner.rb | 8 +-- templates/locales/providers_docker.yml | 4 +- test/unit/plugins/kernel_v2/config/vm_test.rb | 5 +- .../docker/action/has_provisioner_test.rb | 62 +++++++++++++++++++ .../pages/docs/provisioning/basic_usage.mdx | 5 ++ 5 files changed, 77 insertions(+), 7 deletions(-) create mode 100644 test/unit/plugins/providers/docker/action/has_provisioner_test.rb diff --git a/plugins/kernel_v2/config/vm_provisioner.rb b/plugins/kernel_v2/config/vm_provisioner.rb index 6a3042ad9..b49280261 100644 --- a/plugins/kernel_v2/config/vm_provisioner.rb +++ b/plugins/kernel_v2/config/vm_provisioner.rb @@ -125,6 +125,10 @@ module VagrantPlugins provisioner_names = provisioners.map { |i| i.name.to_s if i.name != name }.compact + if ![TrueClass, FalseClass].include?(@communicator_required.class) + errors << I18n.t("vagrant.provisioners.base.wrong_type", opt: "communicator_required", type: "boolean") + end + if @before && @after errors << I18n.t("vagrant.provisioners.base.both_before_after_set") end @@ -181,10 +185,6 @@ module VagrantPlugins end end - if !(@communicator_required.is_a?(TrueClass) || @communicator_required.is_a?(FalseClass)) - errors << I18n.t("vagrant.provisioners.base.wrong_type", opt: "communicator_required", type: "boolean") - end - {"provisioner" => errors} end diff --git a/templates/locales/providers_docker.yml b/templates/locales/providers_docker.yml index 88eb3448e..b4709dd2d 100644 --- a/templates/locales/providers_docker.yml +++ b/templates/locales/providers_docker.yml @@ -134,9 +134,9 @@ en: container_ready: |- Container started and ready for use! not_provisioning: |- - The following provisioners require a communicator, though none is available (this container does not supprt SSH). + The following provisioners require a communicator, though none is available (this container does not support SSH). Not running the following provisioners: - - %{provisioiners} + - %{provisioners} status: host_state_unknown: |- diff --git a/test/unit/plugins/kernel_v2/config/vm_test.rb b/test/unit/plugins/kernel_v2/config/vm_test.rb index c170c873f..7b90dc938 100644 --- a/test/unit/plugins/kernel_v2/config/vm_test.rb +++ b/test/unit/plugins/kernel_v2/config/vm_test.rb @@ -399,15 +399,18 @@ describe VagrantPlugins::Kernel_V2::VMConfig do it "stores the provisioners" do subject.provision("shell", inline: "foo") subject.provision("shell", inline: "bar", run: "always") { |s| s.path = "baz" } + subject.provision("shell", inline: "foo", communicator_required: false) subject.finalize! r = subject.provisioners - expect(r.length).to eql(2) + expect(r.length).to eql(3) expect(r[0].run).to be_nil expect(r[0].config.inline).to eql("foo") expect(r[1].config.inline).to eql("bar") expect(r[1].config.path).to eql("baz") expect(r[1].run).to eql(:always) + expect(r[1].communicator_required).to eql(true) + expect(r[2].communicator_required).to eql(false) end it "allows provisioner settings to be overridden" do diff --git a/test/unit/plugins/providers/docker/action/has_provisioner_test.rb b/test/unit/plugins/providers/docker/action/has_provisioner_test.rb new file mode 100644 index 000000000..dd8aa1360 --- /dev/null +++ b/test/unit/plugins/providers/docker/action/has_provisioner_test.rb @@ -0,0 +1,62 @@ +require_relative "../../../../base" +require_relative "../../../../../../plugins/providers/docker/action/has_provisioner" + + +describe VagrantPlugins::DockerProvider::Action::HasProvisioner 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_one) { double("provisioner_one") } + let(:provisioner_two) { double("provisioner_two") } + let(:provisioners) { [provisioner_one, provisioner_two] } + + let(:machine) do + iso_env.machine(iso_env.machine_names[0], :docker).tap do |m| + allow(m).to receive_message_chain(:config, :vm, :provisioners).and_return(provisioners) + end + end + + let(:env) {{ machine: machine, ui: machine.ui, root_path: Pathname.new(".") }} + let(:app) { lambda { |*args| }} + + subject { described_class.new(app, env) } + + after do + sandbox.close + end + + describe "#call" do + + before do + allow(provisioner_one).to receive(:communicator_required).and_return(true) + allow(provisioner_two).to receive(:communicator_required).and_return(false) + end + + it "does not skip any provisioners if provider has ssh" do + env[:machine].provider_config.has_ssh = true + expect(provisioner_one).to_not receive(:communicator_required) + expect(provisioner_two).to_not receive(:communicator_required) + + subject.call(env) + expect(env[:skip]).to eq([]) + end + + it "skips provisioners that require a communicator if provider does not have ssh" do + env[:machine].provider_config.has_ssh = false + expect(provisioner_one).to receive(:communicator_required) + expect(provisioner_two).to receive(:communicator_required) + expect(provisioner_one).to receive(:run=).with(:never) + + subject.call(env) + expect(env[:skip]).to eq([provisioner_one]) + end + + end +end diff --git a/website/pages/docs/provisioning/basic_usage.mdx b/website/pages/docs/provisioning/basic_usage.mdx index 8da7f6c06..82b94006d 100644 --- a/website/pages/docs/provisioning/basic_usage.mdx +++ b/website/pages/docs/provisioning/basic_usage.mdx @@ -35,6 +35,11 @@ option is what type a provisioner is: 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). +* `communicator_required` (boolean) - Specifies the machine must be accessible by + Vagrant in order to run the provisioner. If set to true, the provisioner will + only run if Vagrant can establish communication with the guest. If set to false + the provisioner will run regardless of Vagrant's ability to communicate with the + guest. Defaults to true. More information about how to use `before` and `after` options can be read [below](#dependency-provisioners). From 66a3b58c088420b3a9e742e47f5a9e5847f2dd03 Mon Sep 17 00:00:00 2001 From: sophia Date: Mon, 18 May 2020 14:10:42 -0400 Subject: [PATCH 7/7] Add provider capaiblity :has_communicator --- lib/vagrant/action.rb | 1 + lib/vagrant/action/builtin/has_provisioner.rb | 36 ++++++++++ plugins/kernel_v2/config/vm_provisioner.rb | 2 +- plugins/providers/docker/action.rb | 14 ---- .../docker/action/has_provisioner.rb | 26 -------- .../providers/docker/cap/has_communicator.rb | 11 ++++ plugins/providers/docker/plugin.rb | 5 ++ .../docker/action/has_provisioner_test.rb | 62 ----------------- .../action/builtin/has_provisioner_test.rb | 66 +++++++++++++++++++ 9 files changed, 120 insertions(+), 103 deletions(-) create mode 100644 lib/vagrant/action/builtin/has_provisioner.rb delete mode 100644 plugins/providers/docker/action/has_provisioner.rb create mode 100644 plugins/providers/docker/cap/has_communicator.rb delete mode 100644 test/unit/plugins/providers/docker/action/has_provisioner_test.rb create mode 100644 test/unit/vagrant/action/builtin/has_provisioner_test.rb diff --git a/lib/vagrant/action.rb b/lib/vagrant/action.rb index 31a910c04..77bc47cd9 100644 --- a/lib/vagrant/action.rb +++ b/lib/vagrant/action.rb @@ -24,6 +24,7 @@ module Vagrant autoload :HandleBox, "vagrant/action/builtin/handle_box" autoload :HandleBoxUrl, "vagrant/action/builtin/handle_box_url" autoload :HandleForwardedPortCollisions, "vagrant/action/builtin/handle_forwarded_port_collisions" + autoload :HasProvisioner, "vagrant/action/builtin/has_provisioner" autoload :IsEnvSet, "vagrant/action/builtin/is_env_set" autoload :IsState, "vagrant/action/builtin/is_state" autoload :Lock, "vagrant/action/builtin/lock" diff --git a/lib/vagrant/action/builtin/has_provisioner.rb b/lib/vagrant/action/builtin/has_provisioner.rb new file mode 100644 index 000000000..c58abe9b4 --- /dev/null +++ b/lib/vagrant/action/builtin/has_provisioner.rb @@ -0,0 +1,36 @@ +module Vagrant + module Action + module Builtin + # This middleware is used with Call to test if this machine + # has available provisioners + class HasProvisioner + def initialize(app, env) + @app = app + @logger = Log4r::Logger.new("vagrant::action::builtin::has_provisioner") + end + + def call(env) + machine = env[:machine] + + if machine.provider.capability?(:has_communicator) + has_communicator = machine.provider.capability(:has_communicator) + else + has_communicator = true + end + + env[:skip] = [] + if !has_communicator + machine.config.vm.provisioners.each do |p| + if p.communicator_required + env[:skip].push(p) + @logger.info("Skipping running provisioner #{p.name || 'no name'}, type: #{p.type}") + p.run = :never + end + end + end + @app.call(env) + end + end + end + end +end diff --git a/plugins/kernel_v2/config/vm_provisioner.rb b/plugins/kernel_v2/config/vm_provisioner.rb index b49280261..a99e143d0 100644 --- a/plugins/kernel_v2/config/vm_provisioner.rb +++ b/plugins/kernel_v2/config/vm_provisioner.rb @@ -75,7 +75,7 @@ module VagrantPlugins @type = type @before = options[:before] @after = options[:after] - @communicator_required = options[:communicator_required] || true + @communicator_required = options.fetch(:communicator_required, true) # Attempt to find the provisioner... if !Vagrant.plugin("2").manager.provisioners[type] diff --git a/plugins/providers/docker/action.rb b/plugins/providers/docker/action.rb index f9180afd8..2705439f4 100644 --- a/plugins/providers/docker/action.rb +++ b/plugins/providers/docker/action.rb @@ -67,9 +67,6 @@ module VagrantPlugins end b3.use Call, HasProvisioner do |env3, b4| - if env3[:skip].length > 0 - b4.use Message, I18n.t("docker_provider.messages.not_provisioning", provisioiners: check_skipped_provisioners(env3[:skip])) - end b4.use Provision end end @@ -214,9 +211,6 @@ module VagrantPlugins b.use Call, IsState, :running do |env, b2| if env[:machine_action] != :run_command b2.use Call, HasProvisioner do |env2, b3| - if env2[:skip].length > 0 - b3.use Message, I18n.t("docker_provider.messages.not_provisioning", provisioiners: check_skipped_provisioners(env2[:skip])) - end b3.use Provision end end @@ -298,7 +292,6 @@ module VagrantPlugins autoload :DestroyNetwork, action_root.join("destroy_network") autoload :ForwardedPorts, action_root.join("forwarded_ports") autoload :HasSSH, action_root.join("has_ssh") - autoload :HasProvisioner, action_root.join("has_provisioner") autoload :HostMachine, action_root.join("host_machine") autoload :HostMachineBuildDir, action_root.join("host_machine_build_dir") autoload :HostMachinePortChecker, action_root.join("host_machine_port_checker") @@ -319,13 +312,6 @@ module VagrantPlugins autoload :Start, action_root.join("start") autoload :Stop, action_root.join("stop") autoload :WaitForRunning, action_root.join("wait_for_running") - - private - - def self.check_skipped_provisioners(provisioners) - skipped_provisioners = provisioners.map { |p| "#{p.name || 'no name'}, type: #{p.type}" } - skipped_provisioners.join("\n- ") - end end end end diff --git a/plugins/providers/docker/action/has_provisioner.rb b/plugins/providers/docker/action/has_provisioner.rb deleted file mode 100644 index fdf324ce5..000000000 --- a/plugins/providers/docker/action/has_provisioner.rb +++ /dev/null @@ -1,26 +0,0 @@ -module VagrantPlugins - module DockerProvider - module Action - # This middleware is used with Call to test if this machine - # has available provisioners - class HasProvisioner - def initialize(app, env) - @app = app - end - - def call(env) - env[:skip] = [] - if !env[:machine].provider_config.has_ssh - env[:machine].config.vm.provisioners.each do |p| - if p.communicator_required - env[:skip].push(p) - p.run = :never - end - end - end - @app.call(env) - end - end - end - end -end diff --git a/plugins/providers/docker/cap/has_communicator.rb b/plugins/providers/docker/cap/has_communicator.rb new file mode 100644 index 000000000..42ef6c763 --- /dev/null +++ b/plugins/providers/docker/cap/has_communicator.rb @@ -0,0 +1,11 @@ +module VagrantPlugins + module DockerProvider + module Cap + module HasCommunicator + def self.has_communicator(machine) + return machine.provider_config.has_ssh + end + end + end + end +end diff --git a/plugins/providers/docker/plugin.rb b/plugins/providers/docker/plugin.rb index d5f7c7bd4..b297016ec 100644 --- a/plugins/providers/docker/plugin.rb +++ b/plugins/providers/docker/plugin.rb @@ -67,6 +67,11 @@ module VagrantPlugins Cap::ProxyMachine end + provider_capability("docker", "has_communicator") do + require_relative "cap/has_communicator" + Cap::HasCommunicator + end + protected def self.init! diff --git a/test/unit/plugins/providers/docker/action/has_provisioner_test.rb b/test/unit/plugins/providers/docker/action/has_provisioner_test.rb deleted file mode 100644 index dd8aa1360..000000000 --- a/test/unit/plugins/providers/docker/action/has_provisioner_test.rb +++ /dev/null @@ -1,62 +0,0 @@ -require_relative "../../../../base" -require_relative "../../../../../../plugins/providers/docker/action/has_provisioner" - - -describe VagrantPlugins::DockerProvider::Action::HasProvisioner 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_one) { double("provisioner_one") } - let(:provisioner_two) { double("provisioner_two") } - let(:provisioners) { [provisioner_one, provisioner_two] } - - let(:machine) do - iso_env.machine(iso_env.machine_names[0], :docker).tap do |m| - allow(m).to receive_message_chain(:config, :vm, :provisioners).and_return(provisioners) - end - end - - let(:env) {{ machine: machine, ui: machine.ui, root_path: Pathname.new(".") }} - let(:app) { lambda { |*args| }} - - subject { described_class.new(app, env) } - - after do - sandbox.close - end - - describe "#call" do - - before do - allow(provisioner_one).to receive(:communicator_required).and_return(true) - allow(provisioner_two).to receive(:communicator_required).and_return(false) - end - - it "does not skip any provisioners if provider has ssh" do - env[:machine].provider_config.has_ssh = true - expect(provisioner_one).to_not receive(:communicator_required) - expect(provisioner_two).to_not receive(:communicator_required) - - subject.call(env) - expect(env[:skip]).to eq([]) - end - - it "skips provisioners that require a communicator if provider does not have ssh" do - env[:machine].provider_config.has_ssh = false - expect(provisioner_one).to receive(:communicator_required) - expect(provisioner_two).to receive(:communicator_required) - expect(provisioner_one).to receive(:run=).with(:never) - - subject.call(env) - expect(env[:skip]).to eq([provisioner_one]) - end - - end -end diff --git a/test/unit/vagrant/action/builtin/has_provisioner_test.rb b/test/unit/vagrant/action/builtin/has_provisioner_test.rb new file mode 100644 index 000000000..474b8d60b --- /dev/null +++ b/test/unit/vagrant/action/builtin/has_provisioner_test.rb @@ -0,0 +1,66 @@ +require File.expand_path("../../../../base", __FILE__) + + +describe Vagrant::Action::Builtin::HasProvisioner do + include_context "unit" + + let(:provisioner_one) { double("provisioner_one") } + let(:provisioner_two) { double("provisioner_two") } + let(:provisioners) { [provisioner_one, provisioner_two] } + let(:machine) { double("machine") } + let(:ui) {double("ui") } + let(:env) {{ machine: machine, ui: ui, root_path: Pathname.new(".") }} + let(:app) { lambda { |*args| }} + + subject { described_class.new(app, env) } + + describe "#call" do + before do + allow(provisioner_one).to receive(:communicator_required).and_return(true) + allow(provisioner_one).to receive(:name) + allow(provisioner_one).to receive(:type) + allow(provisioner_two).to receive(:communicator_required).and_return(false) + allow(provisioner_two).to receive(:name) + allow(provisioner_two).to receive(:type) + allow(machine).to receive_message_chain(:config, :vm, :provisioners).and_return(provisioners) + end + + context "provider has capability :has_communicator" do + before do + allow(machine).to receive_message_chain(:provider, :capability?).with(:has_communicator).and_return(true) + end + + it "does not skip any provisioners if provider has ssh" do + allow(machine).to receive_message_chain(:provider, :capability).with(:has_communicator).and_return(true) + expect(provisioner_one).to_not receive(:communicator_required) + expect(provisioner_two).to_not receive(:communicator_required) + + subject.call(env) + expect(env[:skip]).to eq([]) + end + + it "skips provisioners that require a communicator if provider does not have ssh" do + allow(machine).to receive_message_chain(:provider, :capability).with(:has_communicator).and_return(false) + expect(provisioner_one).to receive(:communicator_required) + expect(provisioner_two).to receive(:communicator_required) + expect(provisioner_one).to receive(:run=).with(:never) + + subject.call(env) + expect(env[:skip]).to eq([provisioner_one]) + end + end + + context "provider does not have capability :has_communicator" do + before do + allow(machine).to receive_message_chain(:provider, :capability?).with(:has_communicator).and_return(false) + end + + it "does not skip any provisioners" do + expect(provisioner_one).to_not receive(:communicator_required) + expect(provisioner_two).to_not receive(:communicator_required) + subject.call(env) + expect(env[:skip]).to eq([]) + end + end + end +end