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. > > +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 >> @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]<puppet-dev%[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]<puppet-dev%[email protected]> > . > For more options, visit this group at > http://groups.google.com/group/puppet-dev?hl=en. > > -- 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.
