From ee4b20ee8d0ee3bb3b4e272ae8a18c7b5d904b68 Mon Sep 17 00:00:00 2001 From: "R.I.Pienaar" Date: Mon, 9 Jan 2012 20:53:47 +0000 Subject: [PATCH] 11280 - mco should exit != 0 when no nodes are found There are various failure states for a typical client and the unix convention is that failure should be indicated by non 0 exit code This commit introduce a halt() helper on the Applications class and changes all the built in application plugins to use this helper to standardize the exit codes as per the list: * Exit with 0 if nodes were discovered and all passed * Exit with 0 if no discovery were done and > 0 responses were received * Exit with 1 if no nodes were discovered * Exit with 2 if nodes were discovered but some RPC requests failed * Exit with 3 if no responses were received but discovery were done * Exit with 4 if no discovery were done and no responses were received --- lib/mcollective/application.rb | 42 ++++++++++ plugins/mcollective/application/controller.rb | 4 +- plugins/mcollective/application/facts.rb | 2 + plugins/mcollective/application/find.rb | 2 + plugins/mcollective/application/inventory.rb | 4 +- plugins/mcollective/application/ping.rb | 2 + plugins/mcollective/application/rpc.rb | 2 + spec/unit/application_spec.rb | 78 +++++++++++++++++++ website/changelog.md | 1 + website/reference/plugins/application.md | 38 ++++++++- 10 files changed, 168 insertions(+), 7 deletions(-) diff --git a/lib/mcollective/application.rb b/lib/mcollective/application.rb index 1bb36032..f78e9b65 100644 --- a/lib/mcollective/application.rb +++ b/lib/mcollective/application.rb @@ -252,6 +252,48 @@ def main exit 1 end + # A helper that creates a consistent exit code for applications by looking at an + # instance of MCollective::RPC::Stats + # + # Exit with 0 if nodes were discovered and all passed + # Exit with 0 if no discovery were done and > 0 responses were received + # Exit with 1 if no nodes were discovered + # Exit with 2 if nodes were discovered but some RPC requests failed + # Exit with 3 if nodes were discovered, but not responses receivedif + # Exit with 4 if no discovery were done and no responses were received + def halt(stats) + request_stats = {:discoverytime => 0, + :discovered => 0, + :failcount => 0}.merge(stats.to_hash) + + # was discovery done? + if request_stats[:discoverytime] != 0 + # was any nodes discovered + if request_stats[:discovered] == 0 + exit 1 + + # nodes were discovered, did we get responses + elsif request_stats[:responses] == 0 + exit 3 + + else + # we got responses and discovery was done, no failures + if request_stats[:failcount] == 0 + exit 0 + else + exit 2 + end + end + else + # discovery wasnt done and we got no responses + if request_stats[:responses] == 0 + exit 4 + else + exit 0 + end + end + end + # Wrapper around MC::RPC#rpcclient that forcably supplies our options hash # if someone forgets to pass in options in an application the filters and other # cli options wouldnt take effect which could have a disasterous outcome diff --git a/plugins/mcollective/application/controller.rb b/plugins/mcollective/application/controller.rb index d52e7cfa..99dbc95b 100644 --- a/plugins/mcollective/application/controller.rb +++ b/plugins/mcollective/application/controller.rb @@ -85,8 +85,8 @@ def main client.disconnect client.display_stats(statistics, false, "mcollectived controller summary") + + halt statistics end end end - -# vim: set ts=4 sw=4 et : diff --git a/plugins/mcollective/application/facts.rb b/plugins/mcollective/application/facts.rb index 41489962..859d9fa3 100644 --- a/plugins/mcollective/application/facts.rb +++ b/plugins/mcollective/application/facts.rb @@ -47,5 +47,7 @@ def main show_single_fact_report(configuration[:fact], facts, options[:verbose]) printrpcstats + + halt rpcutil.stats end end diff --git a/plugins/mcollective/application/find.rb b/plugins/mcollective/application/find.rb index 15292776..5174ddea 100644 --- a/plugins/mcollective/application/find.rb +++ b/plugins/mcollective/application/find.rb @@ -10,5 +10,7 @@ def main end client.display_stats(stats) if options[:verbose] + + halt stats end end diff --git a/plugins/mcollective/application/inventory.rb b/plugins/mcollective/application/inventory.rb index 3f2f24d7..a1997a4d 100644 --- a/plugins/mcollective/application/inventory.rb +++ b/plugins/mcollective/application/inventory.rb @@ -1,5 +1,3 @@ -require 'pp' - class MCollective::Application::Inventory "#{configuration[:agent]}##{configuration[:action]} call stats" + + halt mc.stats end end end diff --git a/spec/unit/application_spec.rb b/spec/unit/application_spec.rb index a779e8b6..20d7eb01 100755 --- a/spec/unit/application_spec.rb +++ b/spec/unit/application_spec.rb @@ -353,6 +353,84 @@ module MCollective end end + describe "#halt" do + before do + @stats = {:discoverytime => 0, :discovered => 0, :failcount => 0, :responses => 0} + end + + it "should exit with code 0 if discovery was done and all responses passed" do + app = Application.new + + @stats[:discoverytime] = 2 + @stats[:discovered] = 2 + @stats[:responses] = 2 + + app.expects(:exit).with(0) + + app.halt(@stats) + end + + it "should exit with code 0 if no discovery were done but responses were received" do + app = Application.new + + @stats[:responses] = 1 + + app.expects(:exit).with(0) + + app.halt(@stats) + end + + it "should exit with code 0 if discovery info is missing" do + app = Application.new + + app.expects(:exit).with(0) + + app.halt({}) + end + + it "should exit with code 1 if no nodes were discovered and discovery was done" do + app = Application.new + + @stats[:discoverytime] = 2 + + app.expects(:exit).with(1) + + app.halt(@stats) + end + + it "should exit with code 2 if a request failed for some nodes" do + app = Application.new + + @stats[:discovered] = 1 + @stats[:failcount] = 1 + @stats[:discoverytime] = 2 + @stats[:responses] = 1 + + app.expects(:exit).with(2) + + app.halt(@stats) + end + + it "should exit with code 3 if no responses were received after discovery" do + app = Application.new + + @stats[:discovered] = 1 + @stats[:discoverytime] = 2 + + app.expects(:exit).with(3) + + app.halt(@stats) + end + + it "should exit with code 4 if no discovery was done and no responses were received" do + app = Application.new + + app.expects(:exit).with(4) + + app.halt(@stats) + end + end + describe "#disconnect" do it "should disconnect from the connector plugin" do connector = mock diff --git a/website/changelog.md b/website/changelog.md index afd39ccc..fb4171ee 100644 --- a/website/changelog.md +++ b/website/changelog.md @@ -11,6 +11,7 @@ title: Changelog |Date|Description|Ticket| |----|-----------|------| +|2012/01/09|Add a halt method to the Application framework and standardize exit codes|11280| |2011/11/21|Remove unintended dependency on _pp_ in the ActiveMQ plugin|10992| |2011/11/17|Allow reply to destinations to be supplied on the command line or API|9847| |*2011/11/17*|*Release 1.3.2*|*10830*| diff --git a/website/reference/plugins/application.md b/website/reference/plugins/application.md index 753a5fbf..58ed64db 100644 --- a/website/reference/plugins/application.md +++ b/website/reference/plugins/application.md @@ -90,8 +90,6 @@ class MCollective::Application::Echo configuration[:message], :options => options) printrpcstats - - mc.disconnect end end {% endhighlight %} @@ -222,7 +220,7 @@ class MCollective::Application::Echo "Hello World", :options => options) + + printrpcstats + + halt_ mc.stats + end +end +{% endhighlight %} + +As you can see you pass the _halt_ helper an instance of the RPC Client statistics and it will then +use that to do the right exit code.