On Mon, Jun 7, 2010 at 11:22 PM, Nigel Kersten <[email protected]> wrote: > > > On Mon, Jun 7, 2010 at 10:33 PM, Luke Kanies <[email protected]> wrote: >> >> I'm comfortable with this, although it's obviously a significant >> enhancement to exec, enough that it's almost a functionally different >> resource type. > > I've been thinking that as well. I'm going to let this churn around for a > bit before committing, as it feels like we could come up with something more > expressive and less like a bash loop. >
I'm going to mark this as ready to commit now, as I haven't thought of anything concrete. > >> >> +1 >> >> On Jun 3, 2010, at 9:05 AM, Nigel Kersten wrote: >> >>> * 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 >>> �[email protected](:returns).expects(:err).never >>> �[email protected] >>> 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]) >>> + �[email protected](:returns).expects(:err).never >>> + �[email protected] >>> + 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. >>> >> >> >> -- >> All power corrupts, but we need the electricity. >> -- Unknown >> --------------------------------------------------------------------- >> Luke Kanies -|- http://puppetlabs.com -|- +1(615)594-8199 >> >> -- >> 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. >> > > > > -- > nigel > -- nigel -- 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.
