Merge commit 'davids-bugfixes/rest/fix-903'

2 files changed, 141 insertions, 10 deletions

diff --git a/lib/puppet/type/exec.rb b/lib/puppet/type/exec.rb
index 9aab4dbd7..5bb3158c4 100755
--- a/lib/puppet/type/exec.rb
+++ b/lib/puppet/type/exec.rb
@@ -113,15 +113,16 @@ module Puppet
                     self.fail "Command exceeded timeout" % value.inspect
                 end
 
-                loglevel = @resource[:loglevel]
-                if status.exitstatus.to_s != self.should.to_s
-                    self.fail("%s returned %s instead of %s" %
-                        [self.resource[:command], status.exitstatus, self.should.to_s])
-                end
-
                 if log = @resource[:logoutput]
-                    if log == :true
+                    case log 
+                    when :true 
                         log = @resource[:loglevel]
+                    when :on_failure
+                        if status.exitstatus.to_s != self.should.to_s
+                            log = @resource[:loglevel]
+                        else
+                            log = :false
+                        end
                     end
                     unless log == :false
                         @output.split(/\n/).each { |line|
@@ -130,6 +131,11 @@ module Puppet
                     end
                 end
 
+                if status.exitstatus.to_s != self.should.to_s
+                    self.fail("%s returned %s instead of %s" %
+                        [self.resource[:command], status.exitstatus, self.should.to_s])
+                end
+
                 return event
             end
         end
@@ -201,10 +207,11 @@ module Puppet
 
         newparam(:logoutput) do
             desc "Whether to log output.  Defaults to logging output at the
-                loglevel for the ``exec`` resource.  Values are **true**, *false*,
-                and any legal log level."
+                loglevel for the ``exec`` resource. Use *on_failure* to only
+                log the output when the command reports an error.  Values are
+                **true**, *false*, *on_failure*, and any legal log level."
 
-            values = [:true, :false]
+            values = [:true, :false, :on_failure]
             # And all of the log levels
             Puppet::Util::Log.eachlevel { |level| values << level }
             newvalues(*values)
diff --git a/spec/unit/ral/exec.rb b/spec/unit/ral/exec.rb
new file mode 100755
index 000000000..b465123f8
--- /dev/null
+++ b/spec/unit/ral/exec.rb
@@ -0,0 +1,124 @@
+#!/usr/bin/env ruby
+
+require File.dirname(__FILE__) + '/../../spec_helper'
+
+require 'puppet/type/exec'
+
+module ExecModuleTesting
+
+    def create_resource(command, output, exitstatus)
+        @user_name = 'some_user_name'
+        @group_name = 'some_group_name'
+        @execer = Puppet::Type.type(:exec).create(:name => command, :user => @user_name, :group => @group_name)
+
+        status = stub "process"
+        status.stubs(:exitstatus).returns(exitstatus)
+
+        Puppet::Util::SUIDManager.expects(:run_and_capture).with([command], @user_name, @group_name).returns([output, status])
+    end
+
+    def create_logging_resource(command, output, exitstatus, logoutput, loglevel)
+        create_resource(command, output, exitstatus)
+        @execer[:logoutput] = logoutput
+        @execer[:loglevel] = loglevel
+    end
+
+    def expect_output(output, loglevel)
+        output.split(/\n/).each do |line|
+            @execer.property(:returns).expects(loglevel).with(line)
+        end
+    end
+end
+
+describe Puppet::Type::Exec, " when execing" do
+    include ExecModuleTesting
+
+    it "should use the 'run_and_capture' method to exec" do
+        command = "/bin/true"
+        create_resource(command, "", 0)
+
+        @execer.refresh.should == :executed_command
+    end
+
+    it "should report a failure" do
+        command = "/bin/false"
+        create_resource(command, "", 1)
+
+        # no idea, why should raise_error doesn't work here
+        raised = false
+        begin
+            @execer.refresh.should raise_error(Puppet::Error)
+        rescue Puppet::Error
+            raised = true
+        end
+        unless raised 
+            raise "didn't receive Puppet::Error"
+        end
+    end
+
+    it "should log the output on success" do
+        #Puppet::Util::Log.newdestination :console
+        command = "/bin/false"
+        output = "output1\noutput2\n"
+        create_logging_resource(command, output, 0, true, :err)
+        expect_output(output, :err)
+        @execer.refresh
+    end
+
+    it "should log the output on failure" do
+        #Puppet::Util::Log.newdestination :console
+        command = "/bin/false"
+        output = "output1\noutput2\n"
+        create_logging_resource(command, output, 1, true, :err)
+        expect_output(output, :err)
+
+        # no idea, why should raise_error doesn't work here
+        raised = false
+        begin
+            @execer.refresh.should raise_error(Puppet::Error)
+        rescue Puppet::Error
+            raised = true
+        end
+        unless raised 
+            raise "didn't receive Puppet::Error"
+        end
+    end
+
+end
+
+
+describe Puppet::Type::Exec, " when logoutput=>on_failure is set," do
+    include ExecModuleTesting
+
+    it "should log the output on failure" do
+        #Puppet::Util::Log.newdestination :console
+        command = "/bin/false"
+        output = "output1\noutput2\n"
+        create_logging_resource(command, output, 1, :on_failure, :err)
+        expect_output(output, :err)
+
+        # no idea, why should raise_error doesn't work here
+        raised = false
+        begin
+            @execer.refresh.should raise_error(Puppet::Error)
+        rescue Puppet::Error
+            raised = true
+        end
+        unless raised 
+            raise "didn't receive Puppet::Error"
+        end
+    end
+
+    it "shouldn't log the output on success" do
+        #Puppet::Util::Log.newdestination :console
+        command = "/bin/true"
+        output = "output1\noutput2\n"
+        create_logging_resource(command, output, 0, :on_failure, :err)
+        @execer.property(:returns).stubs(:err) do 
+            fail "logging, where non was expected"
+        end
+        @execer.refresh
+    end
+
+end
+