From 4d31430275016c9abbd4f621e731ff2eeb1718e5 Mon Sep 17 00:00:00 2001 From: Brice Figureau Date: Fri, 12 Nov 2010 13:39:41 +0100 Subject: Fix #4339 - Save a last run report summary to $statedir/last_run_summary.yaml Once a configuration run is done, puppetd will save on the node a yaml summary report roughly akin to: --- time: notify: 0.001025 last_run: 1289561427 schedule: 0.00071 config_retrieval: 0.039518 filebucket: 0.000126 resources: changed: 1 total: 8 out_of_sync: 1 events: total: 1 success: 1 changes: total: 1 This is almost an hash version of the current --summarize output, with the notable exception that the time section includes the last run unix timestamp. The whole idea is to be able to monitor locally if a puppetd does its job. For instance this could be used in a nagios check or to send an SNMP trap. The last_run information might help detect staleness, and this summary can also be used for performance monitoring (ie time section). The resource section can also show the number of failed resources. Signed-off-by: Brice Figureau --- lib/puppet/configurer.rb | 15 ++++++++++-- lib/puppet/defaults.rb | 4 ++++ lib/puppet/transaction/report.rb | 39 +++++++++++++++++++++--------- lib/puppet/util/metric.rb | 8 +++---- spec/integration/configurer_spec.rb | 43 +++++++++++++++++++++++++++++---- spec/unit/configurer_spec.rb | 46 ++++++++++++++++++++++++++++++++++++ spec/unit/transaction/report_spec.rb | 15 ++++++++++-- 7 files changed, 145 insertions(+), 25 deletions(-) diff --git a/lib/puppet/configurer.rb b/lib/puppet/configurer.rb index 31d31c2d2..e46e9a6e7 100644 --- a/lib/puppet/configurer.rb +++ b/lib/puppet/configurer.rb @@ -168,19 +168,30 @@ class Puppet::Configurer execute_postrun_command Puppet::Util::Log.close(report) - send_report(report, transaction) end def send_report(report, trans = nil) trans.generate_report if trans puts report.summary if Puppet[:summarize] - report.save if Puppet[:report] + save_last_run_summary(report) + if Puppet[:report] + report.save + end rescue => detail puts detail.backtrace if Puppet[:trace] Puppet.err "Could not send report: #{detail}" end + def save_last_run_summary(report) + Puppet::Util::FileLocking.writelock(Puppet[:lastrunfile], 0660) do |file| + file.print YAML.dump(report.raw_summary) + end + rescue => detail + puts detail.backtrace if Puppet[:trace] + Puppet.err "Could not save last run local report: #{detail}" + end + private def self.timeout diff --git a/lib/puppet/defaults.rb b/lib/puppet/defaults.rb index 7ae553827..9b80c9262 100644 --- a/lib/puppet/defaults.rb +++ b/lib/puppet/defaults.rb @@ -602,6 +602,10 @@ module Puppet :report => [false, "Whether to send reports after every transaction." ], + :lastrunfile => { :default => "$statedir/last_run_summary.yaml", + :mode => 0660, + :desc => "Where puppet agent stores the last run report summary in yaml format." + }, :graph => [false, "Whether to create dot graph files for the different configuration graphs. These dot files can be interpreted by tools like OmniGraffle or dot (which is part of ImageMagick)."], diff --git a/lib/puppet/transaction/report.rb b/lib/puppet/transaction/report.rb index e6d1e0528..1d3091428 100644 --- a/lib/puppet/transaction/report.rb +++ b/lib/puppet/transaction/report.rb @@ -62,30 +62,49 @@ class Puppet::Transaction::Report host end - # Provide a summary of this report. + # Provide a human readable textual summary of this report. def summary + report = raw_summary + ret = "" + report.keys.sort { |a,b| a.to_s <=> b.to_s }.each do |key| + ret += "#{Puppet::Util::Metric.labelize(key)}:\n" - @metrics.sort { |a,b| a[1].label <=> b[1].label }.each do |name, metric| - ret += "#{metric.label}:\n" - metric.values.sort { |a,b| + report[key].keys.sort { |a,b| # sort by label - if a[0] == :total + if a == :total 1 - elsif b[0] == :total + elsif b == :total -1 else - a[1] <=> b[1] + report[key][a].to_s <=> report[key][b].to_s end - }.each do |name, label, value| + }.each do |label| + value = report[key][label] next if value == 0 value = "%0.2f" % value if value.is_a?(Float) - ret += " %15s %s\n" % [label + ":", value] + ret += " %15s %s\n" % [Puppet::Util::Metric.labelize(label) + ":", value] end end ret end + # Provide a raw hash summary of this report. + def raw_summary + report = {} + + @metrics.each do |name, metric| + key = metric.name.to_s + report[key] = {} + metric.values.each do |name, label, value| + report[key][name.to_s] = value + end + report[key]["total"] = 0 unless key == "time" or report[key].include?("total") + end + (report["time"] ||= {})["last_run"] = Time.now.tv_sec + report + 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. @@ -103,7 +122,6 @@ class Puppet::Transaction::Report resource_statuses.each do |name, status| metrics[:total] += status.change_count if status.change_count end - add_metric(:changes, metrics) end @@ -124,7 +142,6 @@ class Puppet::Transaction::Report metrics[:total] = resource_statuses.length resource_statuses.each do |name, status| - Puppet::Resource::Status::STATES.each do |state| metrics[state] += 1 if status.send(state) end diff --git a/lib/puppet/util/metric.rb b/lib/puppet/util/metric.rb index 90a244836..d61fb3df6 100644 --- a/lib/puppet/util/metric.rb +++ b/lib/puppet/util/metric.rb @@ -122,7 +122,7 @@ class Puppet::Util::Metric def initialize(name,label = nil) @name = name.to_s - @label = label || labelize(name) + @label = label || self.class.labelize(name) @values = [] end @@ -132,7 +132,7 @@ class Puppet::Util::Metric end def newvalue(name,value,label = nil) - label ||= labelize(name) + label ||= self.class.labelize(name) @values.push [name,label,value] end @@ -173,10 +173,8 @@ class Puppet::Util::Metric @values.sort { |a, b| a[1] <=> b[1] } end - private - # Convert a name into a label. - def labelize(name) + def self.labelize(name) name.to_s.capitalize.gsub("_", " ") end end diff --git a/spec/integration/configurer_spec.rb b/spec/integration/configurer_spec.rb index 9a8b66fe4..cb7d3d779 100755 --- a/spec/integration/configurer_spec.rb +++ b/spec/integration/configurer_spec.rb @@ -5,6 +5,8 @@ require File.dirname(__FILE__) + '/../spec_helper' require 'puppet/configurer' describe Puppet::Configurer do + include PuppetSpec::Files + describe "when downloading plugins" do it "should use the :pluginsignore setting, split on whitespace, for ignoring remote files" do resource = Puppet::Type.type(:notify).new :name => "yay" @@ -17,19 +19,50 @@ describe Puppet::Configurer do end describe "when running" do - it "should send a transaction report with valid data" do - catalog = Puppet::Resource::Catalog.new - catalog.add_resource(Puppet::Type.type(:notify).new(:title => "testing")) + before(:each) do + @catalog = Puppet::Resource::Catalog.new + @catalog.add_resource(Puppet::Type.type(:notify).new(:title => "testing")) - configurer = Puppet::Configurer.new + # Make sure we don't try to persist the local state after the transaction ran, + # because it will fail during test (the state file is in an not existing directory) + # and we need the transaction to be successful to be able to produce a summary report + @catalog.host_config = false + + @configurer = Puppet::Configurer.new + end + + it "should send a transaction report with valid data" do + @configurer.stubs(:save_last_run_summary) Puppet::Transaction::Report.indirection.expects(:save).with do |x, report| report.time.class == Time and report.logs.length > 0 end Puppet[:report] = true - configurer.run :catalog => catalog + @configurer.run :catalog => @catalog + end + + it "should save a correct last run summary" do + report = Puppet::Transaction::Report.new + report.stubs(:save) + + Puppet[:lastrunfile] = tmpfile("lastrunfile") + Puppet[:report] = true + + @configurer.run :catalog => @catalog, :report => report + + summary = nil + File.open(Puppet[:lastrunfile], "r") do |fd| + summary = YAML.load(fd.read) + end + + summary.should be_a(Hash) + %w{time changes events resources}.each do |key| + summary.should be_key(key) + end + summary["time"].should be_key("notify") + summary["time"]["last_run"].should >= Time.now.tv_sec end end end diff --git a/spec/unit/configurer_spec.rb b/spec/unit/configurer_spec.rb index 0c9d06362..5a2c49406 100755 --- a/spec/unit/configurer_spec.rb +++ b/spec/unit/configurer_spec.rb @@ -89,6 +89,7 @@ describe Puppet::Configurer, "when executing a catalog run" do @catalog = Puppet::Resource::Catalog.new @catalog.stubs(:apply) @agent.stubs(:retrieve_catalog).returns @catalog + @agent.stubs(:save_last_run_summary) Puppet::Util::Log.stubs(:newdestination) Puppet::Util::Log.stubs(:close) @@ -236,6 +237,7 @@ describe Puppet::Configurer, "when sending a report" do before do Puppet.settings.stubs(:use).returns(true) @configurer = Puppet::Configurer.new + @configurer.stubs(:save_last_run_summary) @report = stub 'report' @trans = stub 'transaction' @@ -284,6 +286,20 @@ describe Puppet::Configurer, "when sending a report" do @configurer.send_report(@report) end + it "should save the last run summary if reporting is enabled" do + Puppet.settings[:report] = true + + @configurer.expects(:save_last_run_summary).with(@report) + @configurer.send_report(@report) + end + + it "should not save the last run summary if reporting is disabled" do + Puppet.settings[:report] = false + + @configurer.expects(:save_last_run_summary).never + @configurer.send_report(@report) + end + it "should log but not fail if saving the report fails" do Puppet.settings[:report] = true @@ -294,6 +310,36 @@ describe Puppet::Configurer, "when sending a report" do end end +describe Puppet::Configurer, "when saving the summary report file" do + before do + Puppet.settings.stubs(:use).returns(true) + @configurer = Puppet::Configurer.new + + @report = stub 'report' + @trans = stub 'transaction' + @lastrunfd = stub 'lastrunfd' + Puppet::Util::FileLocking.stubs(:writelock).yields(@lastrunfd) + end + + it "should write the raw summary to the lastrunfile setting value" do + Puppet::Util::FileLocking.expects(:writelock).with(Puppet[:lastrunfile], 0660) + @configurer.save_last_run_summary(@report) + end + + it "should write the raw summary as yaml" do + @report.expects(:raw_summary).returns("summary") + @lastrunfd.expects(:print).with(YAML.dump("summary")) + @configurer.save_last_run_summary(@report) + end + + it "should log but not fail if saving the last run summary fails" do + Puppet::Util::FileLocking.expects(:writelock).raises "exception" + Puppet.expects(:err) + lambda { @configurer.save_last_run_summary(@report) }.should_not raise_error + end + +end + describe Puppet::Configurer, "when retrieving a catalog" do before do Puppet.settings.stubs(:use).returns(true) diff --git a/spec/unit/transaction/report_spec.rb b/spec/unit/transaction/report_spec.rb index 7e0b0554b..b310713d2 100755 --- a/spec/unit/transaction/report_spec.rb +++ b/spec/unit/transaction/report_spec.rb @@ -225,8 +225,19 @@ describe Puppet::Transaction::Report do @report.calculate_metrics end - %w{Changes Total Resources}.each do |main| - it "should include information on #{main} in the summary" do + %w{changes time resources events}.each do |main| + it "should include the key #{main} in the raw summary hash" do + @report.raw_summary.should be_key main + end + end + + it "should include the last run time in the raw summary hash" do + Time.stubs(:now).returns(Time.utc(2010,11,10,12,0,24)) + @report.raw_summary["time"]["last_run"].should == 1289390424 + end + + %w{Changes Total Resources Time Events}.each do |main| + it "should include information on #{main} in the textual summary" do @report.summary.should be_include(main) end end -- cgit From 8ab1aba853231a5e541346955dcd502a67992eb6 Mon Sep 17 00:00:00 2001 From: Brice Figureau Date: Fri, 12 Nov 2010 13:41:00 +0100 Subject: Fix #4339 - Allow puppet apply to save last run summary Puppet apply inconditionally saves its last run summary like puppet agent. Signed-off-by: Brice Figureau --- lib/puppet/application/apply.rb | 1 + spec/unit/application/apply_spec.rb | 11 +++++++++++ 2 files changed, 12 insertions(+) diff --git a/lib/puppet/application/apply.rb b/lib/puppet/application/apply.rb index 59a95d35a..ed1331d63 100644 --- a/lib/puppet/application/apply.rb +++ b/lib/puppet/application/apply.rb @@ -139,6 +139,7 @@ class Puppet::Application::Apply < Puppet::Application configurer.send_report(report, transaction) else transaction.generate_report + configurer.save_last_run_summary(transaction.report) end exit( Puppet[:noop] ? 0 : options[:detailed_exitcodes] ? transaction.report.exit_status : 0 ) diff --git a/spec/unit/application/apply_spec.rb b/spec/unit/application/apply_spec.rb index edb41b5c3..9dcfd5f2f 100755 --- a/spec/unit/application/apply_spec.rb +++ b/spec/unit/application/apply_spec.rb @@ -316,6 +316,17 @@ describe Puppet::Application::Apply do @apply.main end + it "should save the last run summary" do + configurer = stub_everything 'configurer' + Puppet::Configurer.expects(:new).returns configurer + Puppet.stubs(:[]).with(:noop).returns(false) + report = stub 'report' + @transaction.stubs(:report).returns(report) + + configurer.expects(:save_last_run_summary).with(report) + @apply.main + end + describe "with detailed_exitcodes" do it "should exit with report's computed exit status" do Puppet.stubs(:[]).with(:noop).returns(false) -- cgit From ccc944f21a259f0216b0bfd4873c98d89127a753 Mon Sep 17 00:00:00 2001 From: Brice Figureau Date: Sat, 13 Nov 2010 11:41:18 +0100 Subject: Fix #4339 - Locally save the last report to $lastrunreport Using the cache terminus system, when --report is on, we are now caching the last report as a yaml file in the $lastrunreport file (which by default is $statedir/last_run_report.yaml). Signed-off-by: Brice Figureau --- lib/puppet/application/agent.rb | 2 ++ lib/puppet/application/apply.rb | 3 +++ lib/puppet/defaults.rb | 4 ++++ lib/puppet/indirector/report/yaml.rb | 11 +++++++++ spec/unit/application/agent_spec.rb | 7 ++++++ spec/unit/application/apply_spec.rb | 6 +++++ spec/unit/indirector/report/yaml_spec.rb | 38 ++++++++++++++++++++++++++++++++ 7 files changed, 71 insertions(+) create mode 100644 lib/puppet/indirector/report/yaml.rb create mode 100644 spec/unit/indirector/report/yaml_spec.rb diff --git a/lib/puppet/application/agent.rb b/lib/puppet/application/agent.rb index 2b75505fd..c5ad72068 100644 --- a/lib/puppet/application/agent.rb +++ b/lib/puppet/application/agent.rb @@ -229,6 +229,8 @@ class Puppet::Application::Agent < Puppet::Application Puppet::SSL::Host.ca_location = options[:fingerprint] ? :none : :remote Puppet::Transaction::Report.terminus_class = :rest + # we want the last report to be persisted locally + Puppet::Transaction::Report.cache_class = :yaml # Override the default; puppetd needs this, usually. # You can still override this on the command-line with, e.g., :compiler. diff --git a/lib/puppet/application/apply.rb b/lib/puppet/application/apply.rb index ed1331d63..97de5e1ff 100644 --- a/lib/puppet/application/apply.rb +++ b/lib/puppet/application/apply.rb @@ -165,6 +165,9 @@ class Puppet::Application::Apply < Puppet::Application exit(1) end + # we want the last report to be persisted locally + Puppet::Transaction::Report.cache_class = :yaml + if options[:debug] Puppet::Util::Log.level = :debug elsif options[:verbose] diff --git a/lib/puppet/defaults.rb b/lib/puppet/defaults.rb index 9b80c9262..f4ae88602 100644 --- a/lib/puppet/defaults.rb +++ b/lib/puppet/defaults.rb @@ -606,6 +606,10 @@ module Puppet :mode => 0660, :desc => "Where puppet agent stores the last run report summary in yaml format." }, + :lastrunreport => { :default => "$statedir/last_run_report.yaml", + :mode => 0660, + :desc => "Where puppet agent stores the last run report in yaml format." + }, :graph => [false, "Whether to create dot graph files for the different configuration graphs. These dot files can be interpreted by tools like OmniGraffle or dot (which is part of ImageMagick)."], diff --git a/lib/puppet/indirector/report/yaml.rb b/lib/puppet/indirector/report/yaml.rb new file mode 100644 index 000000000..bf7bf4fe5 --- /dev/null +++ b/lib/puppet/indirector/report/yaml.rb @@ -0,0 +1,11 @@ +require 'puppet/transaction/report' +require 'puppet/indirector/yaml' + +class Puppet::Transaction::Report::Yaml < Puppet::Indirector::Yaml + desc "Store last report as a flat file, serialized using YAML." + + # Force report to be saved there + def path(name,ext='.yaml') + Puppet[:lastrunreport] + end +end diff --git a/spec/unit/application/agent_spec.rb b/spec/unit/application/agent_spec.rb index 8fc98b8c1..50ef00c57 100755 --- a/spec/unit/application/agent_spec.rb +++ b/spec/unit/application/agent_spec.rb @@ -180,6 +180,7 @@ describe Puppet::Application::Agent do Puppet[:libdir] = "/dev/null/lib" Puppet::SSL::Host.stubs(:ca_location=) Puppet::Transaction::Report.stubs(:terminus_class=) + Puppet::Transaction::Report.stubs(:cache_class=) Puppet::Resource::Catalog.stubs(:terminus_class=) Puppet::Resource::Catalog.stubs(:cache_class=) Puppet::Node::Facts.stubs(:terminus_class=) @@ -311,6 +312,12 @@ describe Puppet::Application::Agent do @puppetd.setup end + it "should tell the report handler to cache locally as yaml" do + Puppet::Transaction::Report.expects(:cache_class=).with(:yaml) + + @puppetd.setup + end + it "should change the catalog_terminus setting to 'rest'" do Puppet[:catalog_terminus] = :foo @puppetd.setup diff --git a/spec/unit/application/apply_spec.rb b/spec/unit/application/apply_spec.rb index 9dcfd5f2f..922995ced 100755 --- a/spec/unit/application/apply_spec.rb +++ b/spec/unit/application/apply_spec.rb @@ -56,6 +56,7 @@ describe Puppet::Application::Apply do Puppet.stubs(:parse_config) Puppet::FileBucket::Dipper.stubs(:new) STDIN.stubs(:read) + Puppet::Transaction::Report.stubs(:cache_class=) @apply.options.stubs(:[]).with(any_parameters) end @@ -113,6 +114,11 @@ describe Puppet::Application::Apply do lambda { @apply.setup }.should raise_error(SystemExit) end + it "should tell the report handler to cache locally as yaml" do + Puppet::Transaction::Report.expects(:cache_class=).with(:yaml) + + @apply.setup + end end describe "when executing" do diff --git a/spec/unit/indirector/report/yaml_spec.rb b/spec/unit/indirector/report/yaml_spec.rb new file mode 100644 index 000000000..610c9ae43 --- /dev/null +++ b/spec/unit/indirector/report/yaml_spec.rb @@ -0,0 +1,38 @@ +#!/usr/bin/env ruby + +require File.dirname(__FILE__) + '/../../../spec_helper' + +require 'puppet/transaction/report' +require 'puppet/indirector/report/yaml' + +describe Puppet::Transaction::Report::Yaml do + it "should be a subclass of the Yaml terminus" do + Puppet::Transaction::Report::Yaml.superclass.should equal(Puppet::Indirector::Yaml) + end + + it "should have documentation" do + Puppet::Transaction::Report::Yaml.doc.should_not be_nil + end + + it "should be registered with the report indirection" do + indirection = Puppet::Indirector::Indirection.instance(:report) + Puppet::Transaction::Report::Yaml.indirection.should equal(indirection) + end + + it "should have its name set to :yaml" do + Puppet::Transaction::Report::Yaml.name.should == :yaml + end + + it "should inconditionnally save/load from the --lastrunreport setting" do + indirection = stub 'indirection', :name => :my_yaml, :register_terminus_type => nil + Puppet::Indirector::Indirection.stubs(:instance).with(:my_yaml).returns(indirection) + store_class = Class.new(Puppet::Transaction::Report::Yaml) do + def self.to_s + "MyYaml::MyType" + end + end + store = store_class.new + + store.path(:me).should == Puppet[:lastrunreport] + end +end -- cgit