* Add 'tries' and 'try_sleep' parameters
* Fix bug where returns is specified as an array and logoutput on
* failure.
* unit tests for both cases above.

Signed-off-by: Nigel Kersten <[email protected]>
---
 lib/puppet/type/exec.rb |   62 +++++++++++++++++++++++++++++++++++++++++++---
 spec/unit/type/exec.rb  |   42 +++++++++++++++++++++++++++++--
 2 files changed, 97 insertions(+), 7 deletions(-)

diff --git a/lib/puppet/type/exec.rb b/lib/puppet/type/exec.rb
index 47d85da..ff6317a 100755
--- a/lib/puppet/type/exec.rb
+++ b/lib/puppet/type/exec.rb
@@ -111,9 +111,20 @@ module Puppet
                 dir = self.resource[:cwd] || Dir.pwd
 
                 event = :executed_command
+                tries = self.resource[:tries]
+                try_sleep = self.resource[:try_sleep]
 
                 begin
-                    @output, status = @resource.run(self.resource[:command])
+                    tries.times do |try|
+                        # Only add debug messages for tries > 1 to reduce log 
spam.
+                        debug("Exec try #{try+1}/#{tries}") if tries > 1
+                        @output, @status = 
@resource.run(self.resource[:command])
+                        break if self.should.include?(@status.exitstatus.to_s)
+                        if try_sleep > 0 and tries > 1
+                          debug("Sleeping for #{try_sleep} seconds between 
tries")
+                          sleep try_sleep
+                        end
+                    end
                 rescue Timeout::Error
                     self.fail "Command exceeded timeout" % value.inspect
                 end
@@ -123,7 +134,7 @@ module Puppet
                     when :true
                         log = @resource[:loglevel]
                     when :on_failure
-                        if status.exitstatus.to_s != self.should.to_s
+                        unless self.should.include?(@status.exitstatus.to_s)
                             log = @resource[:loglevel]
                         else
                             log = :false
@@ -136,9 +147,9 @@ module Puppet
                     end
                 end
 
-                unless self.should.include?(status.exitstatus.to_s)
+                unless self.should.include?(@status.exitstatus.to_s)
                     self.fail("%s returned %s instead of one of [%s]" %
-                        [self.resource[:command], status.exitstatus, 
self.should.join(",")])
+                        [self.resource[:command], @status.exitstatus, 
self.should.join(",")])
                 end
 
                 return event
@@ -276,6 +287,49 @@ module Puppet
             defaultto 300
         end
 
+        newparam(:tries) do
+            desc "The number of times execution of the command should be tried.
+                Defaults to '1'. This many attempts will be made to execute
+                the command until an acceptable return code is returned.
+                Note that the timeout paramater applies to each try rather than
+                to the complete set of tries."
+
+            munge do |value|
+                if value.is_a?(String)
+                    unless value =~ /^[\d]+$/
+                        raise ArgumentError, "Tries must be an integer"
+                    end
+                    value = Integer(value)
+                end
+                if value < 1
+                    raise ArgumentError, "Tries must be an integer >= 1"
+                end
+                value
+            end
+
+            defaultto 1
+        end
+
+        newparam(:try_sleep) do
+            desc "The time to sleep in seconds between 'tries'."
+
+            munge do |value|
+                if value.is_a?(String)
+                    unless value =~ /^[-\d.]+$/
+                        raise ArgumentError, "try_sleep must be a number"
+                    end
+                    value = Float(value)
+                end
+                if value < 0
+                    raise ArgumentError, "try_sleep cannot be a negative 
number"
+                end
+                value
+            end
+
+            defaultto 0
+        end
+
+
         newcheck(:refreshonly) do
             desc "The command should only be run as a
                 refresh mechanism for when a dependent object is changed.  It 
only
diff --git a/spec/unit/type/exec.rb b/spec/unit/type/exec.rb
index 813bdae..17be1d0 100755
--- a/spec/unit/type/exec.rb
+++ b/spec/unit/type/exec.rb
@@ -3,7 +3,7 @@
 require File.dirname(__FILE__) + '/../../spec_helper'
 
 module ExecModuleTesting
-    def create_resource(command, output, exitstatus, returns = [0])
+    def create_resource(command, output, exitstatus, returns = 0)
         @user_name = 'some_user_name'
         @group_name = 'some_group_name'
         Puppet.features.stubs(:root?).returns(true)
@@ -15,8 +15,8 @@ module ExecModuleTesting
         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)
+    def create_logging_resource(command, output, exitstatus, logoutput, 
loglevel, returns = 0)
+        create_resource(command, output, exitstatus, returns)
         @execer[:logoutput] = logoutput
         @execer[:loglevel] = loglevel
     end
@@ -99,6 +99,16 @@ describe Puppet::Type.type(:exec), " when 
logoutput=>on_failure is set," do
         proc { @execer.refresh }.should raise_error(Puppet::Error)
     end
 
+    it "should log the output on failure when returns is specified as an 
array" do
+        #Puppet::Util::Log.newdestination :console
+        command = "false"
+        output = "output1\noutput2\n"
+        create_logging_resource(command, output, 1, :on_failure, :err, [0, 
100])
+        expect_output(output, :err)
+
+        proc { @execer.refresh }.should raise_error(Puppet::Error)
+    end
+
     it "shouldn't log the output on success" do
         #Puppet::Util::Log.newdestination :console
         command = "true"
@@ -107,6 +117,32 @@ describe Puppet::Type.type(:exec), " when 
logoutput=>on_failure is set," do
         @execer.property(:returns).expects(:err).never
         @execer.refresh
     end
+
+    it "shouldn't log the output on success when non-zero exit status is in a 
returns array" do
+        #Puppet::Util::Log.newdestination :console
+        command = "true"
+        output = "output1\noutput2\n"
+        create_logging_resource(command, output, 100, :on_failure, :err, 
[1,100])
+        @execer.property(:returns).expects(:err).never
+        @execer.refresh
+    end
+end
+
+describe Puppet::Type.type(:exec), " when multiple tries are set," do
+    include ExecModuleTesting
+
+    it "should repeat the command attempt 'tries' times on failure and produce 
an error" do
+        Puppet.features.stubs(:root?).returns(true)
+        command = "false"
+        user = "user"
+        group = "group"
+        tries = 5
+        retry_exec = Puppet::Type.type(:exec).new(:name => command, :path => 
%w{/usr/bin /bin}, :user => user, :group => group, :returns => 0, :tries => 
tries, :try_sleep => 0)
+        status = stub "process"
+        status.stubs(:exitstatus).returns(1)
+        Puppet::Util::SUIDManager.expects(:run_and_capture).with([command], 
user, group).times(tries).returns(["", status])
+        proc { retry_exec.refresh }.should raise_error(Puppet::Error)
+    end
 end
 
 describe Puppet::Type.type(:exec) do
-- 
1.7.0.4

-- 
You received this message because you are subscribed to the Google Groups 
"Puppet Developers" group.
To post to this group, send email to [email protected].
To unsubscribe from this group, send email to 
[email protected].
For more options, visit this group at 
http://groups.google.com/group/puppet-dev?hl=en.

Reply via email to