Added a fail_on_errors configuration option

README.rst

@@ -142,6 +142,10 @@ pillar_data
 verbose          (true/false)
     Prints bootstrap script output to screen
 
+fail_on_errors   (true/false)
+    Causes provisioning to cease if state.highstate fails and prints out the
+    relevant bits from the salt-call output.  Set to 'false' by default.
+
 
 Installation Notes
 ==================

lib/vagrant-salt.rb

@@ -7,6 +7,7 @@ module Salt
 
     lib_path = Pathname.new(File.expand_path("../vagrant-salt", __FILE__))
     autoload :Errors, lib_path.join("errors")
+    autoload :StateResultsParser, lib_path.join("state_results_parser")
 
     @source_root = Pathname.new(File.expand_path("../../", __FILE__))
 

lib/vagrant-salt/config.rb

@@ -17,6 +17,7 @@ class Config < Vagrant.plugin("2", :config)
       attr_accessor :accept_keys
       attr_accessor :bootstrap_script
       attr_accessor :verbose
+      attr_accessor :fail_on_errors
       attr_accessor :seed_master
       attr_reader   :pillar_data
 
@@ -41,6 +42,7 @@ def initialize
         @accept_keys = UNSET_VALUE
         @bootstrap_script = UNSET_VALUE
         @verbose = UNSET_VALUE
+        @fail_on_errors = UNSET_VALUE
         @seed_master = UNSET_VALUE
         @pillar_data = UNSET_VALUE
         @temp_config_dir = UNSET_VALUE
@@ -64,6 +66,7 @@ def finalize!
         @accept_keys        = nil if @accept_keys == UNSET_VALUE
         @bootstrap_script   = nil if @bootstrap_script == UNSET_VALUE
         @verbose            = nil if @verbose == UNSET_VALUE
+        @fail_on_errors     = nil if @fail_on_errors == UNSET_VALUE
         @seed_master        = nil if @seed_master == UNSET_VALUE
         @pillar_data        = {}  if @pillar_data == UNSET_VALUE
         @temp_config_dir    = nil if @temp_config_dir == UNSET_VALUE

lib/vagrant-salt/errors.rb

@@ -6,6 +6,14 @@ module Errors
       class SaltError < Vagrant::Errors::VagrantError
         error_namespace("salt")
       end
+
+      class SaltCallHighstateError < SaltError
+        error_key("highstate_failed")
+
+        def initialize(salt_call_output)
+          super :salt_call_output => salt_call_output
+        end
+      end
     end
   end
 end

lib/vagrant-salt/provisioner.rb

@@ -30,10 +30,10 @@ def seed_master
             next
           end
           sourcepath = expanded_path(keyfile).to_s
-          dest = '/tmp/seed-%s.pub' %name 
-          
-          @machine.communicate.upload(sourcepath, dest)   
-          @machine.communicate.sudo("mv /tmp/seed-%s.pub /etc/salt/pki/master/minions/%s" %[name, name])   
+          dest = '/tmp/seed-%s.pub' %name
+
+          @machine.communicate.upload(sourcepath, dest)
+          @machine.communicate.sudo("mv /tmp/seed-%s.pub /etc/salt/pki/master/minions/%s" %[name, name])
         end
       end
 
@@ -49,7 +49,7 @@ def keys(group='minions')
         end
         return out
       end
-      
+
       ## Utilities
       def expanded_path(rel_path)
         Pathname.new(rel_path).expand_path(@machine.env.root_path)
@@ -120,9 +120,9 @@ def bootstrap_options(install, configure, config_dir)
           @config.seed_master.each do |name, keyfile|
             sourcepath = expanded_path(keyfile).to_s
             dest = "#{seed_dir}/seed-#{name}.pub"
-            @machine.communicate.upload(sourcepath, dest)   
+            @machine.communicate.upload(sourcepath, dest)
           end
-          options = "#{options} -k #{seed_dir}" 
+          options = "#{options} -k #{seed_dir}"
         end
 
         if configure and !install
@@ -247,12 +247,12 @@ def run_bootstrap_script
           @machine.env.ui.info "Salt did not need installing or configuring."
         end
       end
-      
+
       # DEPRECATED
       def accept_keys
         if [email protected]("which salt-key")
           @machine.env.ui.info "Salt-key not installed!"
-          return 
+          return
         end
 
         key_staged = false
@@ -290,33 +290,51 @@ def accept_keys
         end
 
         if key_staged
-          @machine.env.ui.info "Adding %s key(s) for minion(s)" %numkeys           
+          @machine.env.ui.info "Adding %s key(s) for minion(s)" %numkeys
           @machine.communicate.sudo("salt-key -A")
         end
       end
-  
+
       def call_highstate
         if @config.run_highstate
           @machine.env.ui.info "Calling state.highstate... (this may take a while)"
           if @config.install_master
             @machine.communicate.sudo("salt '*' saltutil.sync_all")
             @machine.communicate.sudo("salt '*' state.highstate --verbose#{get_pillar}") do |type, data|
-              if @config.verbose
-                @machine.env.ui.info(data)
-              end
+              handle_highstate_output type, data
             end
           else
             @machine.communicate.sudo("salt-call saltutil.sync_all")
             @machine.communicate.sudo("salt-call state.highstate -l debug#{get_pillar}") do |type, data|
-              if @config.verbose
-                @machine.env.ui.info(data)
-              end
+              handle_highstate_output type, data
             end
           end
+
+          if @config.fail_on_errors
+            raise Salt::Errors::SaltCallHighstateError.new(state_results_parser.all_of_stdout) if state_results_parser.saw_no_states?
+            raise Salt::Errors::SaltCallHighstateError.new(state_results_parser.red_lines) if state_results_parser.saw_failed_states?
+          end
         else
           @machine.env.ui.info "run_highstate set to false. Not running state.highstate."
         end
       end
+
+      private
+      # Writes out the output from salt-call if configured to do so.
+      # Parses the output looking for errors, if configured to do so.
+      def handle_highstate_output(type, data)
+        if @config.verbose
+          @machine.env.ui.info(data)
+        end
+
+        if @config.fail_on_errors
+          state_results_parser.parse(type, data)
+        end
+      end
+
+      def state_results_parser
+        @state_results_parser ||= Salt::StateResultsParser.new
+      end
     end
   end
 end

lib/vagrant-salt/state_results_parser.rb

@@ -0,0 +1,146 @@
+module VagrantPlugins
+  module Salt
+    # Takes in any number of stdout/stderr lines taken from the output of
+    # 'salt-call state.highstate', via #parse.  Then the appropriate error lines
+    # are made available if there were any failures.
+    #
+    # This completely depends on the 'highstate' format of the output of the
+    # 'salt-call state.highstate' command not changing, so it could potentially be a bit
+    # brittle.  We could simply use the json or yaml format but that doesn't
+    # look as nice as the highstate format.
+    #
+    # Currently error handling has to be done this way because the
+    # return code of the salt-call is always 0.  There is an issue open for
+    # that, so there'll be a less brittle solution available in the future!
+    # https://github.com/saltstack/salt/issues/4176
+    #
+    # As of right now, the output of the salt-call command looks like this when
+    # there's an error:
+    #
+    # ----------
+    # State: - cmd
+    # Name:      /usr/local/bin/something
+    # Function:  run
+    #     Result:    False
+    #     Comment:   One or more requisite failed
+    #     Changes:
+    class StateResultsParser
+
+      # Takes in the type (:stdout/:stdin) and data from part of the output
+      # of the salt-call state.highstate.
+      def parse(type, data)
+        # We only care about stdout lines.
+        parse_stdout data if type == :stdout
+      end
+
+      # Returns true if there was no state output at all - usually caused by a mis-
+      # configuration in the sls files or something along those lines.
+      def saw_no_states?
+        !any_results?
+      end
+
+      # Returns true if there were any failed states.
+      def saw_failed_states?
+        !failed_state_output.empty?
+      end
+
+      # Returns the complete set of stdout lines from the salt-call.
+      def all_of_stdout
+        @all_of_stdout ||= ""
+      end
+
+      # Returns all of the 'red' lines from the output of the salt-call (i.e. all
+      # the failed states).
+      def red_lines
+        failed_state_output
+      end
+
+      private
+      # Helper class that knows what the salt-call output looks like.  It takes
+      # in one line of that output and can tell me useful stuff like the result
+      # from one of the states.
+      class Line
+
+        def initialize(line_string)
+          @line_string = line_string
+        end
+
+        def looks_like_a_header?
+          @line_string =~ /^-+\n$/
+        end
+
+        def is_a_state_result?
+          result != nil
+        end
+
+        def result
+          if @line_string =~ /^\s+Result:\s+(True|False)\n$/
+            $1 == "True" ? true : false
+          end
+        end
+
+        def to_s
+          @line_string
+        end
+      end
+
+      # Reads the output from salt-call state.highstate line by line and stores
+      # the parts that look like results.
+      def parse_stdout(data)
+        data.each_line do |line|
+          parse_line Line.new(line)
+        end
+        save_failed_state_output!
+      end
+
+      # Looks at the current line (as a Line object) and stores the output for
+      # any failed states.
+      def parse_line(line)
+        reset_current_state! if line.looks_like_a_header?
+
+        # Store each line from this state in case it's a failed state.
+        current_state << line.to_s
+        all_of_stdout << line.to_s
+
+        if line.is_a_state_result?
+          @any_results = true
+          current_state_is_unsuccessful! if line.result == false
+        end
+      end
+
+      def any_results?
+        @any_results == true
+      end
+
+      def failed_state_output
+        @failed_state_output ||= ""
+      end
+
+      # Appends the current_state string if there is one and if it represents
+      # an unsuccessful state.
+      def save_failed_state_output!
+        failed_state_output << current_state if current_state_is_unsuccessful?
+      end
+
+      # Resets the current_state string back to nothing.
+      def reset_current_state!
+        save_failed_state_output!
+        @current_state = nil
+        @current_state_is_unsuccessful = nil
+      end
+
+      def current_state_is_unsuccessful!
+        @current_state_is_unsuccessful = true
+      end
+
+      def current_state_is_unsuccessful?
+        @current_state_is_unsuccessful
+      end
+
+      def current_state
+        @current_state ||= ""
+      end
+
+    end
+  end
+end

templates/locales/en.yml

@@ -11,4 +11,6 @@ en:
     not_received_minion_key: |-
       Salt Master did not receive minion key.
     bootstrap_failed: |-
-      Bootstrap script failed, see /var/log/bootstrap-salt.log on VM.
+      Bootstrap script failed, see /var/log/bootstrap-salt.log on VM.
+    highstate_failed: |-
+      %{salt_call_output}

vagrant-salt.gemspec

@@ -15,8 +15,6 @@ Gem::Specification.new do |s|
   s.executables   = `git ls-files -- bin/*`.split("\n").map{ |f| File.basename(f) }
   s.require_paths = ["lib", "templates"]
 
-  s.add_runtime_dependency "vagrant"
-
   # get an array of submodule dirs by executing 'pwd' inside each submodule
   `git submodule --quiet foreach pwd`.split($\).each do |submodule_path|
     # for each submodule, change working directory to that submodule