This option only works when --onetime is specified, as it doesn't make much sense to worry about exit codes in the context of a long-running daemon.
This required a refactoring of the existing --detailed-exitcodes code, as "puppetd" wasn't directly creating a transaction object (like "puppet" does). Added Report::to_exit_status, which did what was previously hard-coded into the "puppet" executable. An Agent's "run" method now returns a value (the result of the individual client class' "run" method) The "puppetd" agent's "run" method now returns a transaction report, as that seems like the logical thing to return as the result of applying a catalog. Signed-off-by: Deepak Giridharagopal <[email protected]> --- lib/puppet/agent.rb | 4 ++- lib/puppet/application/puppet.rb | 7 ++--- lib/puppet/application/puppetd.rb | 16 ++++++++++++- lib/puppet/configurer.rb | 8 +++++- lib/puppet/transaction/report.rb | 11 +++++++++- sbin/puppetd | 9 +++++++- spec/unit/application/puppet.rb | 41 +++++++++++------------------------- spec/unit/application/puppetd.rb | 28 +++++++++++++++++++++++++ spec/unit/transaction/report.rb | 23 ++++++++++++++++++++ 9 files changed, 108 insertions(+), 39 deletions(-) diff --git a/lib/puppet/agent.rb b/lib/puppet/agent.rb index a188df3..7bd9741 100644 --- a/lib/puppet/agent.rb +++ b/lib/puppet/agent.rb @@ -48,14 +48,16 @@ class Puppet::Agent return end splay + result = nil with_client do |client| begin - sync.synchronize { lock { client.run(*args) } } + sync.synchronize { lock { result = client.run(*args) } } rescue => detail puts detail.backtrace if Puppet[:trace] Puppet.err "Could not run %s: %s" % [client_class, detail] end end + result end def stop diff --git a/lib/puppet/application/puppet.rb b/lib/puppet/application/puppet.rb index a87ae48..5ee589b 100644 --- a/lib/puppet/application/puppet.rb +++ b/lib/puppet/application/puppet.rb @@ -127,13 +127,12 @@ Puppet::Application.new(:puppet) do # And apply it transaction = catalog.apply - status = 0 if not Puppet[:noop] and options[:detailed_exitcodes] then transaction.generate_report - status |= 2 if transaction.report.metrics["changes"][:total] > 0 - status |= 4 if transaction.report.metrics["resources"][:failed] > 0 + exit(transaction.report.to_exit_status) + else + exit(0) end - exit(status) rescue => detail if Puppet[:trace] puts detail.backtrace diff --git a/lib/puppet/application/puppetd.rb b/lib/puppet/application/puppetd.rb index 4799d55..0620fc2 100644 --- a/lib/puppet/application/puppetd.rb +++ b/lib/puppet/application/puppetd.rb @@ -21,6 +21,7 @@ Puppet::Application.new(:puppetd) do { :waitforcert => 120, # Default to checking for certs every 5 minutes :onetime => false, + :detailed_exitcodes => false, :verbose => false, :debug => false, :centrallogs => false, @@ -65,6 +66,10 @@ Puppet::Application.new(:puppetd) do options[:waitforcert] = 0 unless @explicit_waitforcert end + option("--detailed-exitcodes") do |arg| + options[:detailed_exitcodes] = true + end + option("--logdest DEST", "-l DEST") do |arg| begin Puppet::Util::Log.newdestination(arg) @@ -95,19 +100,25 @@ Puppet::Application.new(:puppetd) do unless options[:client] $stderr.puts "onetime is specified but there is no client" exit(43) + return end @daemon.set_signal_traps begin - @agent.run + report = @agent.run rescue => detail if Puppet[:trace] puts detail.backtrace end Puppet.err detail.to_s end - exit(0) + + if not Puppet[:noop] and options[:detailed_exitcodes] then + exit(report.to_exit_status) + else + exit(0) + end end command(:main) do @@ -125,6 +136,7 @@ Puppet::Application.new(:puppetd) do Puppet.settings.handlearg("--no-daemonize") options[:verbose] = true options[:onetime] = true + options[:detailed_exitcodes] = true options[:waitforcert] = 0 end diff --git a/lib/puppet/configurer.rb b/lib/puppet/configurer.rb index efda545..b63df07 100644 --- a/lib/puppet/configurer.rb +++ b/lib/puppet/configurer.rb @@ -144,13 +144,17 @@ class Puppet::Configurer begin benchmark(:notice, "Finished catalog run") do - catalog.apply(options) + transaction = catalog.apply(options) + transaction.generate_report + report = transaction.report end + report rescue => detail puts detail.backtrace if Puppet[:trace] Puppet.err "Failed to apply catalog: %s" % detail + return end - + ensure # Now close all of our existing http connections, since there's no # reason to leave them lying open. Puppet::Network::HttpPool.clear_http_instances diff --git a/lib/puppet/transaction/report.rb b/lib/puppet/transaction/report.rb index e5b8650..5e9efb2 100644 --- a/lib/puppet/transaction/report.rb +++ b/lib/puppet/transaction/report.rb @@ -83,5 +83,14 @@ class Puppet::Transaction::Report end return ret end -end + # Based on the contents of this report's metrics, compute a single number + # that represents the report. The resulting number is a bitmask where + # individual bits represent the presence of different metrics. + def to_exit_status + status = 0 + status |= 2 if @metrics["changes"][:total] > 0 + status |= 4 if @metrics["resources"][:failed] > 0 + return status + end +end diff --git a/sbin/puppetd b/sbin/puppetd index 650a7e7..bf7d028 100755 --- a/sbin/puppetd +++ b/sbin/puppetd @@ -8,7 +8,8 @@ # # = Usage # -# puppetd [-D|--daemonize|--no-daemonize] [-d|--debug] [--disable] [--enable] +# puppetd [-D|--daemonize|--no-daemonize] [-d|--debug] +# [--detailed-exitcodes] [--disable] [--enable] # [-h|--help] [--fqdn <host name>] [-l|--logdest syslog|<file>|console] # [-o|--onetime] [--serve <handler>] [-t|--test] [--noop] # [-V|--version] [-v|--verbose] [-w|--waitforcert <seconds>] @@ -71,6 +72,12 @@ # debug:: # Enable full debugging. # +# detailed-exitcodes:: +# Provide transaction information via exit codes. If this is enabled, an +# exit code of '2' means there were changes, and an exit code of '4' means +# that there were failures during the transaction. This option only makes +# sense in conjunction with --onetime. +# # disable:: # Disable working on the local system. This puts a lock file in place, # causing +puppetd+ not to work on the system until the lock file is removed. diff --git a/spec/unit/application/puppet.rb b/spec/unit/application/puppet.rb index 3c11430..feee1ac 100755 --- a/spec/unit/application/puppet.rb +++ b/spec/unit/application/puppet.rb @@ -283,52 +283,37 @@ describe "Puppet" do @puppet.main end - it "should generate a report if not noop" do - Puppet.stubs(:[]).with(:noop).returns(false) - @puppet.options.stubs(:[]).with(:detailed_exitcodes).returns(true) - metrics = stub 'metrics', :[] => { :total => 10, :failed => 0} - report = stub 'report', :metrics => metrics - @transaction.stubs(:report).returns(report) - - @transaction.expects(:generate_report) - - @puppet.main - end - describe "with detailed_exitcodes" do - before :each do + it "should exit with report's computed exit status" do Puppet.stubs(:[]).with(:noop).returns(false) @puppet.options.stubs(:[]).with(:detailed_exitcodes).returns(true) - end - - it "should exit with exit code of 2 if changes" do - report = stub 'report', :metrics => { "changes" => {:total => 1}, "resources" => {:failed => 0} } - @transaction.stubs(:generate_report).returns(report) + report = stub 'report', :to_exit_status => 666 @transaction.stubs(:report).returns(report) - @puppet.expects(:exit).with(2) + @puppet.expects(:exit).with(666) @puppet.main end - it "should exit with exit code of 4 if failures" do - report = stub 'report', :metrics => { "changes" => {:total => 0}, "resources" => {:failed => 1} } - @transaction.stubs(:generate_report).returns(report) + it "should always exit with 0 if option is disabled" do + Puppet.stubs(:[]).with(:noop).returns(false) + @puppet.options.stubs(:[]).with(:detailed_exitcodes).returns(false) + report = stub 'report', :to_exit_status => 666 @transaction.stubs(:report).returns(report) - @puppet.expects(:exit).with(4) + @puppet.expects(:exit).with(0) @puppet.main end - it "should exit with exit code of 6 if changes and failures" do - report = stub 'report', :metrics => { "changes" => {:total => 1}, "resources" => {:failed => 1} } - @transaction.stubs(:generate_report).returns(report) + it "should always exit with 0 if --noop" do + Puppet.stubs(:[]).with(:noop).returns(true) + @puppet.options.stubs(:[]).with(:detailed_exitcodes).returns(true) + report = stub 'report', :to_exit_status => 666 @transaction.stubs(:report).returns(report) - @puppet.expects(:exit).with(6) + @puppet.expects(:exit).with(0) @puppet.main end end - end describe "the 'apply' command" do diff --git a/spec/unit/application/puppetd.rb b/spec/unit/application/puppetd.rb index d34ec9f..7e423d2 100755 --- a/spec/unit/application/puppetd.rb +++ b/spec/unit/application/puppetd.rb @@ -205,6 +205,10 @@ describe "puppetd" do @puppetd.options.expects(:[]=).with(:onetime,true) @puppetd.setup_test end + it "should set options[:detailed_exitcodes] to true" do + @puppetd.options.expects(:[]=).with(:detailed_exitcodes,true) + @puppetd.setup_test + end it "should set waitforcert to 0" do @puppetd.options.expects(:[]=).with(:waitforcert,0) @puppetd.setup_test @@ -453,6 +457,7 @@ describe "puppetd" do before :each do @puppetd.options.stubs(:[]).with(:client).returns(:client) + @puppetd.options.stubs(:[]).with(:detailed_exitcodes).returns(false) @puppetd.stubs(:exit).with(0) Puppet.stubs(:newservice) end @@ -484,6 +489,29 @@ describe "puppetd" do @puppetd.onetime end + describe "and --detailed-exitcodes" do + before :each do + @puppetd.options.stubs(:[]).with(:detailed_exitcodes).returns(true) + end + + it "should exit with report's computed exit status" do + Puppet.stubs(:[]).with(:noop).returns(false) + report = stub 'report', :to_exit_status => 666 + @agent.stubs(:run).returns(report) + @puppetd.expects(:exit).with(666) + + @puppetd.onetime + end + + it "should always exit with 0 if --noop" do + Puppet.stubs(:[]).with(:noop).returns(true) + report = stub 'report', :to_exit_status => 666 + @agent.stubs(:run).returns(report) + @puppetd.expects(:exit).with(0) + + @puppetd.onetime + end + end end describe "without --onetime" do diff --git a/spec/unit/transaction/report.rb b/spec/unit/transaction/report.rb index 1276573..646a3e0 100755 --- a/spec/unit/transaction/report.rb +++ b/spec/unit/transaction/report.rb @@ -38,3 +38,26 @@ describe Puppet::Transaction::Report, " when being indirect" do Puppet::Util::Cacher.expire end end + +describe Puppet::Transaction::Report, " when computing exit status" do + it "should compute 2 if changes present" do + report = Puppet::Transaction::Report.new + report.newmetric("changes", {:total => 1}) + report.newmetric("resources", {:failed => 0}) + report.to_exit_status.should == 2 + end + + it "should compute 4 if failures present" do + report = Puppet::Transaction::Report.new + report.newmetric("changes", {:total => 0}) + report.newmetric("resources", {:failed => 1}) + report.to_exit_status.should == 4 + end + + it "should compute 6 if both changes and present" do + report = Puppet::Transaction::Report.new + report.newmetric("changes", {:total => 1}) + report.newmetric("resources", {:failed => 1}) + report.to_exit_status.should == 6 + end +end -- 1.6.4.3 --~--~---------~--~----~------------~-------~--~----~ 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 -~----------~----~----~----~------~----~------~--~---
