Merge pull request #10005 from briancain/add-exit-code-checks-for-run-scripts

Catch and allow for non-standard exit codes

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

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,

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
 

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: |-

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

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.