From 52c3dcc70ec93a84433b1acb31d8fbe930faa421 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Mon, 9 Jul 2018 15:19:13 -0700 Subject: [PATCH] (#9997) Catch and allow for non-standard exit codes Prior to this commit, the run trigger option wouldn't catch for failures outside of the #Subprocess.execute raising exceptions. This commit fixes that by inspecting the exit code result of the subprocess and using the new `exit_codes` option to determine how to move forward with the trigger. --- lib/vagrant/errors.rb | 4 + lib/vagrant/plugin/v2/trigger.rb | 12 ++- plugins/kernel_v2/config/vm_trigger.rb | 19 ++++- templates/locales/en.yml | 5 ++ test/unit/vagrant/plugin/v2/trigger_test.rb | 79 ++++++++++++++++--- .../docs/triggers/configuration.html.md | 2 + 6 files changed, 105 insertions(+), 16 deletions(-) diff --git a/lib/vagrant/errors.rb b/lib/vagrant/errors.rb index 9b57c466f..7eef2d6d9 100644 --- a/lib/vagrant/errors.rb +++ b/lib/vagrant/errors.rb @@ -784,6 +784,10 @@ module Vagrant error_key(:synced_folder_unusable) end + class TriggersBadExitCodes < VagrantError + error_key(:triggers_bad_exit_codes) + end + class TriggersGuestNotRunning < VagrantError error_key(:triggers_guest_not_running) end diff --git a/lib/vagrant/plugin/v2/trigger.rb b/lib/vagrant/plugin/v2/trigger.rb index f869af76e..67f6643fe 100644 --- a/lib/vagrant/plugin/v2/trigger.rb +++ b/lib/vagrant/plugin/v2/trigger.rb @@ -125,11 +125,11 @@ module Vagrant end if trigger.run - run(trigger.run, trigger.on_error) + run(trigger.run, trigger.on_error, trigger.exit_codes) end if trigger.run_remote - run_remote(trigger.run_remote, trigger.on_error) + run_remote(trigger.run_remote, trigger.on_error, trigger.exit_codes) end end end @@ -151,7 +151,7 @@ module Vagrant # Runs a script on a guest # # @param [Provisioners::Shell::Config] config A Shell provisioner config - def run(config, on_error) + def run(config, on_error, exit_codes) if config.inline cmd = Shellwords.split(config.inline) @@ -188,6 +188,10 @@ module Vagrant @machine.ui.detail(data, options) end + if !exit_codes.include?(result.exit_code) + raise Errors::TriggersBadExitCodes, + code: result.exit_code + end rescue => e @machine.ui.error(I18n.t("vagrant.errors.triggers_run_fail")) @machine.ui.error(e.message) @@ -205,7 +209,7 @@ module Vagrant # Runs a script on the guest # # @param [ShellProvisioner/Config] config A Shell provisioner config - def run_remote(config, on_error) + def run_remote(config, on_error, exit_codes) unless @machine.state.id == :running if on_error == :halt raise Errors::TriggersGuestNotRunning, diff --git a/plugins/kernel_v2/config/vm_trigger.rb b/plugins/kernel_v2/config/vm_trigger.rb index 053d9ae74..4baa4a052 100644 --- a/plugins/kernel_v2/config/vm_trigger.rb +++ b/plugins/kernel_v2/config/vm_trigger.rb @@ -7,6 +7,7 @@ module VagrantPlugins class VagrantConfigTrigger < Vagrant.plugin("2", :config) # Defaults DEFAULT_ON_ERROR = :halt + DEFAULT_EXIT_CODE = 0 #------------------------------------------------------------------- # Config class for a given Trigger @@ -50,7 +51,6 @@ module VagrantPlugins # @return [Symbol, Array] attr_accessor :ignore - # If set, will only run trigger for guests that match keys for this parameter. # # @return [String, Regex, Array] @@ -66,6 +66,11 @@ module VagrantPlugins # @return [Hash] attr_accessor :run_remote + # If set, will not run trigger for the configured Vagrant commands. + # + # @return [Integer, Array] + attr_accessor :exit_codes + def initialize(command) @logger = Log4r::Logger.new("vagrant::config::vm::trigger::config") @@ -77,6 +82,7 @@ module VagrantPlugins @only_on = UNSET_VALUE @run = UNSET_VALUE @run_remote = UNSET_VALUE + @exit_codes = UNSET_VALUE # Internal options @id = SecureRandom.uuid @@ -96,6 +102,7 @@ module VagrantPlugins @run = nil if @run == UNSET_VALUE @run_remote = nil if @run_remote == UNSET_VALUE @only_on = nil if @only_on == UNSET_VALUE + @exit_codes = DEFAULT_EXIT_CODE if @exit_codes == UNSET_VALUE # these values are expected to always be an Array internally, # but can be set as a single String or Symbol @@ -111,6 +118,10 @@ module VagrantPlugins @ignore.map! { |i| i.to_sym } end + if @exit_codes + @exit_codes = Array(@exit_codes) + end + # Convert @run and @run_remote to be a "Shell provisioner" config if @run && @run.is_a?(Hash) # Powershell args and privileged for run commands is currently not supported @@ -190,6 +201,12 @@ module VagrantPlugins end end + if @exit_codes + if !@exit_codes.all? {|i| i.is_a?(Integer)} + errors << I18n.t("vagrant.config.triggers.exit_codes_bad_type", cmd: @command) + end + end + errors end diff --git a/templates/locales/en.yml b/templates/locales/en.yml index 24ce2391f..0fd99ccc8 100644 --- a/templates/locales/en.yml +++ b/templates/locales/en.yml @@ -1430,6 +1430,8 @@ en: Trigger run failed triggers_guest_not_running: |- Could not run remote script on %{machine_name} because its state is %{state} + triggers_bad_exit_codes: |- + A script exited with an unacceptable exit code %{code}. triggers_no_block_given: |- There was an error parsing the Vagrantfile: No config was given for the trigger(s) %{command}. @@ -1728,6 +1730,9 @@ en: on_error_bad_type: |- Invalid type set for `on_error` on trigger for command '%{cmd}'. `on_error` can only be `:halt` (default) or `:continue`. + exit_codes_bad_type: |- + Invalid type set for `exit_codes` on trigger for command '%{cmd}'. `exit_codes` can + only be a single integer or an array of integers. only_on_bad_type: |- Invalid type found for `only_on`. All values must be a `String` or `Regexp`. privileged_ignored: |- diff --git a/test/unit/vagrant/plugin/v2/trigger_test.rb b/test/unit/vagrant/plugin/v2/trigger_test.rb index 8c44a5316..1bf09ff44 100644 --- a/test/unit/vagrant/plugin/v2/trigger_test.rb +++ b/test/unit/vagrant/plugin/v2/trigger_test.rb @@ -127,6 +127,9 @@ describe Vagrant::Plugin::V2::Trigger do context "#run" do let(:trigger_run) { VagrantPlugins::Kernel_V2::TriggerConfig.new } let(:shell_block) { {info: "hi", run: {inline: "echo 'hi'", env: {"KEY"=>"VALUE"}}} } + let(:shell_block_exit_codes) { + {info: "hi", run: {inline: "echo 'hi'", env: {"KEY"=>"VALUE"}}, + exit_codes: [0,50]} } let(:path_block) { {warn: "bye", run: {path: "script.sh", env: {"KEY"=>"VALUE"}}, on_error: :continue} } @@ -145,8 +148,23 @@ describe Vagrant::Plugin::V2::Trigger do end end + let(:subprocess_result_failure) do + double("subprocess_result_failure").tap do |result| + allow(result).to receive(:exit_code).and_return(1) + allow(result).to receive(:stderr).and_return("") + end + end + + let(:subprocess_result_custom) do + double("subprocess_result_custom").tap do |result| + allow(result).to receive(:exit_code).and_return(50) + allow(result).to receive(:stderr).and_return("") + end + end + before do trigger_run.after(:up, shell_block) + trigger_run.after(:up, shell_block_exit_codes) trigger_run.before(:destroy, path_block) trigger_run.before(:destroy, path_block_ps1) trigger_run.finalize! @@ -160,10 +178,11 @@ describe Vagrant::Plugin::V2::Trigger do trigger = trigger_run.after_triggers.first shell_config = trigger.run on_error = trigger.on_error + exit_codes = trigger.exit_codes expect(Vagrant::Util::PowerShell).to receive(:execute_inline). with("echo", "hi", options) - subject.send(:run, shell_config, on_error) + subject.send(:run, shell_config, on_error, exit_codes) end it "executes an path script with powershell if windows" do @@ -175,10 +194,11 @@ describe Vagrant::Plugin::V2::Trigger do trigger = trigger_run.before_triggers[1] shell_config = trigger.run on_error = trigger.on_error + exit_codes = trigger.exit_codes expect(Vagrant::Util::PowerShell).to receive(:execute). with("/vagrant/home/script.ps1", options) - subject.send(:run, shell_config, on_error) + subject.send(:run, shell_config, on_error, exit_codes) end it "executes an inline script" do @@ -188,10 +208,11 @@ describe Vagrant::Plugin::V2::Trigger do trigger = trigger_run.after_triggers.first shell_config = trigger.run on_error = trigger.on_error + exit_codes = trigger.exit_codes expect(Vagrant::Util::Subprocess).to receive(:execute). with("echo", "hi", options) - subject.send(:run, shell_config, on_error) + subject.send(:run, shell_config, on_error, exit_codes) end it "executes an path script" do @@ -203,10 +224,11 @@ describe Vagrant::Plugin::V2::Trigger do trigger = trigger_run.before_triggers.first shell_config = trigger.run on_error = trigger.on_error + exit_codes = trigger.exit_codes expect(Vagrant::Util::Subprocess).to receive(:execute). with("/vagrant/home/script.sh", options) - subject.send(:run, shell_config, on_error) + subject.send(:run, shell_config, on_error, exit_codes) end it "continues on error" do @@ -218,10 +240,11 @@ describe Vagrant::Plugin::V2::Trigger do trigger = trigger_run.before_triggers.first shell_config = trigger.run on_error = trigger.on_error + exit_codes = trigger.exit_codes expect(Vagrant::Util::Subprocess).to receive(:execute). with("/vagrant/home/script.sh", options) - subject.send(:run, shell_config, on_error) + subject.send(:run, shell_config, on_error, exit_codes) end it "halts on error" do @@ -231,10 +254,39 @@ describe Vagrant::Plugin::V2::Trigger do trigger = trigger_run.after_triggers.first shell_config = trigger.run on_error = trigger.on_error + exit_codes = trigger.exit_codes expect(Vagrant::Util::Subprocess).to receive(:execute). with("echo", "hi", options) - expect { subject.send(:run, shell_config, on_error) }.to raise_error("Fail!") + expect { subject.send(:run, shell_config, on_error, exit_codes) }.to raise_error("Fail!") + end + + it "allows for acceptable exit codes" do + allow(Vagrant::Util::Subprocess).to receive(:execute). + and_return(subprocess_result_custom) + + trigger = trigger_run.after_triggers[1] + shell_config = trigger.run + on_error = trigger.on_error + exit_codes = trigger.exit_codes + + expect(Vagrant::Util::Subprocess).to receive(:execute). + with("echo", "hi", options) + subject.send(:run, shell_config, on_error, exit_codes) + end + + it "exits if given a bad exit code" do + allow(Vagrant::Util::Subprocess).to receive(:execute). + and_return(subprocess_result_custom) + + trigger = trigger_run.after_triggers.first + shell_config = trigger.run + on_error = trigger.on_error + exit_codes = trigger.exit_codes + + expect(Vagrant::Util::Subprocess).to receive(:execute). + with("echo", "hi", options) + expect { subject.send(:run, shell_config, on_error, exit_codes) }.to raise_error(Vagrant::Errors::TriggersBadExitCodes) end end @@ -258,8 +310,9 @@ describe Vagrant::Plugin::V2::Trigger do trigger = trigger_run.after_triggers.first shell_config = trigger.run_remote on_error = trigger.on_error + exit_codes = trigger.exit_codes - expect { subject.send(:run_remote, shell_config, on_error) }. + expect { subject.send(:run_remote, shell_config, on_error, exit_codes) }. to raise_error(Vagrant::Errors::TriggersGuestNotRunning) end @@ -272,8 +325,9 @@ describe Vagrant::Plugin::V2::Trigger do trigger = trigger_run.before_triggers.first shell_config = trigger.run_remote on_error = trigger.on_error + exit_codes = trigger.exit_codes - subject.send(:run_remote, shell_config, on_error) + subject.send(:run_remote, shell_config, on_error, exit_codes) end it "calls the provision function on the shell provisioner" do @@ -285,8 +339,9 @@ describe Vagrant::Plugin::V2::Trigger do trigger = trigger_run.after_triggers.first shell_config = trigger.run_remote on_error = trigger.on_error + exit_codes = trigger.exit_codes - subject.send(:run_remote, shell_config, on_error) + subject.send(:run_remote, shell_config, on_error, exit_codes) end it "continues on if provision fails" do @@ -298,8 +353,9 @@ describe Vagrant::Plugin::V2::Trigger do trigger = trigger_run.before_triggers.first shell_config = trigger.run_remote on_error = trigger.on_error + exit_codes = trigger.exit_codes - subject.send(:run_remote, shell_config, on_error) + subject.send(:run_remote, shell_config, on_error, exit_codes) end it "fails if it encounters an error" do @@ -311,8 +367,9 @@ describe Vagrant::Plugin::V2::Trigger do trigger = trigger_run.after_triggers.first shell_config = trigger.run_remote on_error = trigger.on_error + exit_codes = trigger.exit_codes - expect { subject.send(:run_remote, shell_config, on_error) }. + expect { subject.send(:run_remote, shell_config, on_error, exit_codes) }. to raise_error("Nope!") end end diff --git a/website/source/docs/triggers/configuration.html.md b/website/source/docs/triggers/configuration.html.md index bd1581220..c7528405b 100644 --- a/website/source/docs/triggers/configuration.html.md +++ b/website/source/docs/triggers/configuration.html.md @@ -42,3 +42,5 @@ The trigger class takes various options. + `path` * `warn` (string) - A warning message that will be printed at the beginning of a trigger. + +* `exit_codes` (integer, array) - A set of acceptable exit codes to continue on. Defaults to `0` if option is absent. For now only valid with the `run` option.