From cdcbdc78bc399a60afaf36b6267688e72081fb6e Mon Sep 17 00:00:00 2001 From: Luke Kanies Date: Mon, 18 Jan 2010 17:26:06 -0800 Subject: Fixing #2914 - pre/post hooks now work for transactions This was built to be used with etckeeper to version control files in /etc, but can be used for essentially anything. This patch was built to be added to 0.25.4, so it's a least-modify approach. A better approach would be to refactor application/puppet.rb just a bit so it uses Configurer more. This is a simple patch - it just defines 'prerun_command' and 'postrun_command' settings, and runs the appropriate command around each transaction if they're set. Signed-off-by: Luke Kanies --- lib/puppet/application/puppet.rb | 10 ++++-- lib/puppet/configurer.rb | 24 ++++++++++++++ lib/puppet/defaults.rb | 7 ++++- spec/integration/defaults.rb | 8 +++++ spec/unit/application/puppet.rb | 13 ++++++++ spec/unit/configurer.rb | 67 ++++++++++++++++++++++++++++++++++++++-- 6 files changed, 123 insertions(+), 6 deletions(-) diff --git a/lib/puppet/application/puppet.rb b/lib/puppet/application/puppet.rb index 2929d54e8..273c7d0e7 100644 --- a/lib/puppet/application/puppet.rb +++ b/lib/puppet/application/puppet.rb @@ -1,5 +1,6 @@ require 'puppet' require 'puppet/application' +require 'puppet/configurer' require 'puppet/network/handler' require 'puppet/network/client' @@ -124,9 +125,14 @@ Puppet::Application.new(:puppet) do catalog.retrieval_duration = Time.now - starttime + configurer = Puppet::Configurer.new + configurer.execute_prerun_command + # And apply it transaction = catalog.apply + configurer.execute_postrun_command + status = 0 if not Puppet[:noop] and options[:detailed_exitcodes] then transaction.generate_report @@ -135,9 +141,7 @@ Puppet::Application.new(:puppet) do end exit(status) rescue => detail - if Puppet[:trace] - puts detail.backtrace - end + puts detail.backtrace if Puppet[:trace] if detail.is_a?(XMLRPC::FaultException) $stderr.puts detail.message else diff --git a/lib/puppet/configurer.rb b/lib/puppet/configurer.rb index 3e72fa574..ec61272ef 100644 --- a/lib/puppet/configurer.rb +++ b/lib/puppet/configurer.rb @@ -5,6 +5,8 @@ require 'puppet/network/http_pool' require 'puppet/util' class Puppet::Configurer + class CommandHookError < RuntimeError; end + require 'puppet/configurer/fact_handler' require 'puppet/configurer/plugin_handler' @@ -39,6 +41,14 @@ class Puppet::Configurer @catalog = nil end + def execute_postrun_command + execute_from_setting(:postrun_command) + end + + def execute_prerun_command + execute_from_setting(:prerun_command) + end + # Initialize and load storage def dostorage begin @@ -75,6 +85,8 @@ class Puppet::Configurer download_plugins() download_fact_plugins() + + execute_prerun_command end # Get the remote catalog, yo. Returns nil if no catalog can be found. @@ -160,6 +172,8 @@ class Puppet::Configurer # Now close all of our existing http connections, since there's no # reason to leave them lying open. Puppet::Network::HttpPool.clear_http_instances + ensure + execute_postrun_command end private @@ -180,4 +194,14 @@ class Puppet::Configurer return timeout end + + def execute_from_setting(setting) + return if (command = Puppet[setting]) == "" + + begin + Puppet::Util.execute([command]) + rescue => detail + raise CommandHookError, "Could not run command from #{setting}: #{detail}" + end + end end diff --git a/lib/puppet/defaults.rb b/lib/puppet/defaults.rb index 6b9c43b81..21cee7a05 100644 --- a/lib/puppet/defaults.rb +++ b/lib/puppet/defaults.rb @@ -199,7 +199,12 @@ module Puppet reports, allowing you to correlate changes on your hosts to the source version on the server."], :zlib => [true, "Boolean; whether to use the zlib library", - ] + ], + :prerun_command => ["", "A command to run before every agent run. If this command returns a non-zero + return code, the entire Puppet run will fail."], + :postrun_command => ["", "A command to run after every agent run. If this command returns a non-zero + return code, the entire Puppet run will be considered to have failed, even though it might have + performed work during the normal run."] ) hostname = Facter["hostname"].value diff --git a/spec/integration/defaults.rb b/spec/integration/defaults.rb index 292abbb87..e97035dea 100755 --- a/spec/integration/defaults.rb +++ b/spec/integration/defaults.rb @@ -215,4 +215,12 @@ describe "Puppet defaults" do Puppet.settings[:report_server].should == "report_server" end end + + it "should have a 'prerun_command' that defaults to the empty string" do + Puppet.settings[:prerun_command].should == "" + end + + it "should have a 'postrun_command' that defaults to the empty string" do + Puppet.settings[:postrun_command].should == "" + end end diff --git a/spec/unit/application/puppet.rb b/spec/unit/application/puppet.rb index 7703bac08..61dcf9061 100755 --- a/spec/unit/application/puppet.rb +++ b/spec/unit/application/puppet.rb @@ -173,6 +173,9 @@ describe "Puppet" do describe "the main command" do before :each do Puppet.stubs(:[]) + Puppet.settings.stubs(:use) + Puppet.stubs(:[]).with(:prerun_command).returns "" + Puppet.stubs(:[]).with(:postrun_command).returns "" Puppet.stubs(:[]).with(:trace).returns(true) @puppet.options.stubs(:[]) @@ -277,6 +280,16 @@ describe "Puppet" do @puppet.main end + it "should call the prerun and postrun commands on a Configurer instance" do + configurer = stub 'configurer' + + Puppet::Configurer.expects(:new).returns configurer + configurer.expects(:execute_prerun_command) + configurer.expects(:execute_postrun_command) + + @puppet.main + end + it "should apply the catalog" do @catalog.expects(:apply) diff --git a/spec/unit/configurer.rb b/spec/unit/configurer.rb index cd5102546..32c0772ec 100755 --- a/spec/unit/configurer.rb +++ b/spec/unit/configurer.rb @@ -7,6 +7,11 @@ require File.dirname(__FILE__) + '/../spec_helper' require 'puppet/configurer' describe Puppet::Configurer do + before do + Puppet.settings.stubs(:use).returns(true) + @agent = Puppet::Configurer.new + end + it "should include the Plugin Handler module" do Puppet::Configurer.ancestors.should be_include(Puppet::Configurer::PluginHandler) end @@ -19,6 +24,52 @@ describe Puppet::Configurer do Puppet.settings.expects(:value).with(:puppetdlockfile).returns("/my/lock") Puppet::Configurer.lockfile_path.should == "/my/lock" end + + describe "when executing a pre-run hook" do + it "should do nothing if the hook is set to an empty string" do + Puppet.settings[:prerun_command] = "" + Puppet::Util.expects(:exec).never + + @agent.execute_prerun_command + end + + it "should execute any pre-run command provided via the 'prerun_command' setting" do + Puppet.settings[:prerun_command] = "/my/command" + Puppet::Util.expects(:execute).with { |args| args[0] == "/my/command" } + + @agent.execute_prerun_command + end + + it "should fail if the command fails" do + Puppet.settings[:prerun_command] = "/my/command" + Puppet::Util.expects(:execute).raises Puppet::ExecutionFailure + + lambda { @agent.execute_prerun_command }.should raise_error(Puppet::Configurer::CommandHookError) + end + end + + describe "when executing a post-run hook" do + it "should do nothing if the hook is set to an empty string" do + Puppet.settings[:postrun_command] = "" + Puppet::Util.expects(:exec).never + + @agent.execute_postrun_command + end + + it "should execute any post-run command provided via the 'postrun_command' setting" do + Puppet.settings[:postrun_command] = "/my/command" + Puppet::Util.expects(:execute).with { |args| args[0] == "/my/command" } + + @agent.execute_postrun_command + end + + it "should fail if the command fails" do + Puppet.settings[:postrun_command] = "/my/command" + Puppet::Util.expects(:execute).raises Puppet::ExecutionFailure + + lambda { @agent.execute_postrun_command }.should raise_error(Puppet::Configurer::CommandHookError) + end + end end describe Puppet::Configurer, "when executing a catalog run" do @@ -74,6 +125,12 @@ describe Puppet::Configurer, "when executing a catalog run" do catalog.expects(:apply).never # because we're not yielding @agent.run end + + it "should execute post-run hooks after the run" do + @agent.expects(:execute_postrun_command) + + @agent.run + end end describe Puppet::Configurer, "when retrieving a catalog" do @@ -213,6 +270,9 @@ describe Puppet::Configurer, "when preparing for a run" do Puppet.settings.stubs(:use).returns(true) @agent = Puppet::Configurer.new @agent.stubs(:dostorage) + @agent.stubs(:download_fact_plugins) + @agent.stubs(:download_plugins) + @agent.stubs(:execute_prerun_command) @facts = {"one" => "two", "three" => "four"} end @@ -223,16 +283,19 @@ describe Puppet::Configurer, "when preparing for a run" do end it "should download fact plugins" do - @agent.stubs(:dostorage) @agent.expects(:download_fact_plugins) @agent.prepare end it "should download plugins" do - @agent.stubs(:dostorage) @agent.expects(:download_plugins) @agent.prepare end + + it "should perform the pre-run commands" do + @agent.expects(:execute_prerun_command) + @agent.prepare + end end -- cgit