From 3e7ebbbb08120243f7e982f34b8256ec58af67b0 Mon Sep 17 00:00:00 2001 From: James Turnbull Date: Wed, 2 Mar 2011 16:49:13 +1100 Subject: Fixed #6554 - Missing $haveftool if/else conditional in install.rb breaking Ruby 1.9 --- install.rb | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/install.rb b/install.rb index 7627a8d11..2608e31da 100755 --- a/install.rb +++ b/install.rb @@ -92,8 +92,12 @@ def do_configs(configs, target, strip = 'conf/') Dir.mkdir(target) unless File.directory? target configs.each do |cf| ocf = File.join(InstallOptions.config_dir, cf.gsub(/#{strip}/, '')) - File.install(cf, ocf, 0644, true) - end + if $haveftools + File.install(cf, ocf, 0644, true) + else + FileUtils.install(cf, ocf, {:mode => 0644, :verbose => true}) + end + end end def do_bins(bins, target, strip = 's?bin/') -- cgit From 87ca3130c570df65ab1296cc18de697afa6a55f5 Mon Sep 17 00:00:00 2001 From: Nick Lewis Date: Wed, 30 Mar 2011 16:08:22 -0700 Subject: (#5670) Don't trigger refresh from a failed resource Resources were triggering their subscribing/notified resources when they failed, which is incorrect. Now, events are only queued if the resource was successful. Paired-With: Max Martin --- lib/puppet/transaction.rb | 2 +- spec/integration/transaction_spec.rb | 61 ++++++++++++++++++++---------------- 2 files changed, 35 insertions(+), 28 deletions(-) diff --git a/lib/puppet/transaction.rb b/lib/puppet/transaction.rb index aa650eea1..48154ad6f 100644 --- a/lib/puppet/transaction.rb +++ b/lib/puppet/transaction.rb @@ -47,7 +47,7 @@ class Puppet::Transaction def apply(resource, ancestor = nil) status = resource_harness.evaluate(resource) add_resource_status(status) - event_manager.queue_events(ancestor || resource, status.events) + event_manager.queue_events(ancestor || resource, status.events) unless status.failed? rescue => detail resource.err "Could not evaluate: #{detail}" end diff --git a/spec/integration/transaction_spec.rb b/spec/integration/transaction_spec.rb index 2c12b3d5f..66a049efe 100755 --- a/spec/integration/transaction_spec.rb +++ b/spec/integration/transaction_spec.rb @@ -135,33 +135,26 @@ describe Puppet::Transaction do it "should not let one failed refresh result in other refreshes failing" do path = tmpfile("path") newfile = tmpfile("file") - - file = Puppet::Type.type(:file).new( - + file = Puppet::Type.type(:file).new( :path => path, - :ensure => "file" ) - exec1 = Puppet::Type.type(:exec).new( - + exec1 = Puppet::Type.type(:exec).new( :path => ENV["PATH"], :command => "touch /this/cannot/possibly/exist", :logoutput => true, :refreshonly => true, :subscribe => file, - :title => "one" ) - exec2 = Puppet::Type.type(:exec).new( - + exec2 = Puppet::Type.type(:exec).new( :path => ENV["PATH"], :command => "touch #{newfile}", :logoutput => true, :refreshonly => true, :subscribe => [file, exec1], - :title => "two" ) @@ -178,22 +171,18 @@ describe Puppet::Transaction do Puppet[:ignoreschedules] = false - file = Puppet::Type.type(:file).new( - + file = Puppet::Type.type(:file).new( :name => tmpfile("file"), - :ensure => "file", :backup => false ) fname = tmpfile("exec") - exec = Puppet::Type.type(:exec).new( - + exec = Puppet::Type.type(:exec).new( :name => "touch #{fname}", :path => "/usr/bin:/bin", :schedule => "monthly", - :subscribe => Puppet::Resource.new("file", file.name) ) @@ -230,29 +219,21 @@ describe Puppet::Transaction do it "should not attempt to evaluate resources with failed dependencies" do - exec = Puppet::Type.type(:exec).new( - + exec = Puppet::Type.type(:exec).new( :command => "/bin/mkdir /this/path/cannot/possibly/exit", - :title => "mkdir" ) - - file1 = Puppet::Type.type(:file).new( - + file1 = Puppet::Type.type(:file).new( :title => "file1", :path => tmpfile("file1"), - :require => exec, :ensure => :file ) - - file2 = Puppet::Type.type(:file).new( - + file2 = Puppet::Type.type(:file).new( :title => "file2", :path => tmpfile("file2"), - :require => file1, :ensure => :file ) @@ -264,6 +245,32 @@ describe Puppet::Transaction do FileTest.should_not be_exists(file2[:path]) end + it "should not trigger subscribing resources on failure" do + file1 = tmpfile("file1") + file2 = tmpfile("file2") + + create_file1 = Puppet::Type.type(:exec).new( + :command => "/usr/bin/touch #{file1}" + ) + + exec = Puppet::Type.type(:exec).new( + :command => "/bin/mkdir /this/path/cannot/possibly/exit", + :title => "mkdir", + :notify => create_file1 + ) + + create_file2 = Puppet::Type.type(:exec).new( + :command => "/usr/bin/touch #{file2}", + :subscribe => exec + ) + + catalog = mk_catalog(exec, create_file1, create_file2) + catalog.apply + + FileTest.should_not be_exists(file1) + FileTest.should_not be_exists(file2) + end + # #801 -- resources only checked in noop should be rescheduled immediately. it "should immediately reschedule noop resources" do Puppet::Type.type(:schedule).mkdefaultschedules -- cgit From 647a640fcac281e3a8cda05b92b51c44c93f1d19 Mon Sep 17 00:00:00 2001 From: Ben Hughes Date: Wed, 30 Mar 2011 16:43:56 +1100 Subject: (#6893) Document the cron type in the case of specials. Add in a better desc block for "specials" in cron provider, and outline it's limitations. The previous text was purely a placeholder. --- lib/puppet/type/cron.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/puppet/type/cron.rb b/lib/puppet/type/cron.rb index 4b18e71f9..4f6ea733c 100755 --- a/lib/puppet/type/cron.rb +++ b/lib/puppet/type/cron.rb @@ -226,7 +226,9 @@ Puppet::Type.newtype(:cron) do end newproperty(:special) do - desc "Special schedules" + desc "A special value such as 'reboot' or 'annually'. + Only available on supported systems such as Vixie Cron. + Overrides more specific time of day/week settings." def specials %w{reboot yearly annually monthly weekly daily midnight hourly} -- cgit From 2cdadf9b7fa7a4a1b826150549e5faffc4e494f3 Mon Sep 17 00:00:00 2001 From: Ben Hughes Date: Fri, 1 Apr 2011 12:48:38 +1100 Subject: (#6937) Document the recurse parameter of File type. Update the desc block with information gleaned from #1469 and the code about recurse => remote and other types of recursion. The auto generated documentation was sparse and this is an area that often comes up on the mailing list/IRC. --- lib/puppet/type/file.rb | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/lib/puppet/type/file.rb b/lib/puppet/type/file.rb index e1a4ecbb9..79f4b81ab 100644 --- a/lib/puppet/type/file.rb +++ b/lib/puppet/type/file.rb @@ -122,7 +122,17 @@ Puppet::Type.newtype(:file) do newparam(:recurse) do desc "Whether and how deeply to do recursive - management." + management. Options are: + inf,true => Regular style recursion on both remote and local + directory structure. + remote => Descends recursively into the remote directory + but not the local directory. Allows copying of + a few files into a directory containing many + unmanaged files without scanning all the local files. + false => Default of no-recursion. + [0-9]+ => Both, but limit recursion. Warning: this syntax + is deprecated and has moved to recurselimit. + " newvalues(:true, :false, :inf, :remote, /^[0-9]+$/) -- cgit From 0675c9a7ae90b25e1d723c19a7691b12eac9becd Mon Sep 17 00:00:00 2001 From: nfagerlund Date: Fri, 1 Apr 2011 10:35:41 -0700 Subject: (#6937) Adjust formatting of recurse's desc Prevous formatting would result in broken Markdown after the docs were generated, as Markdown does not recognize a two-space tab as a syntactical element. This patch also changes the list of values to a bulleted list instead of a code block. --- lib/puppet/type/file.rb | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/lib/puppet/type/file.rb b/lib/puppet/type/file.rb index 79f4b81ab..630ebe5de 100644 --- a/lib/puppet/type/file.rb +++ b/lib/puppet/type/file.rb @@ -123,15 +123,16 @@ Puppet::Type.newtype(:file) do newparam(:recurse) do desc "Whether and how deeply to do recursive management. Options are: - inf,true => Regular style recursion on both remote and local - directory structure. - remote => Descends recursively into the remote directory - but not the local directory. Allows copying of - a few files into a directory containing many - unmanaged files without scanning all the local files. - false => Default of no-recursion. - [0-9]+ => Both, but limit recursion. Warning: this syntax - is deprecated and has moved to recurselimit. + + * `inf,true` --- Regular style recursion on both remote and local + directory structure. + * `remote` --- Descends recursively into the remote directory + but not the local directory. Allows copying of + a few files into a directory containing many + unmanaged files without scanning all the local files. + * `false` --- Default of no recursion. + * `[0-9]+` --- Same as true, but limit recursion. Warning: this syntax + has been deprecated in favor of the `recurselimit` attribute. " newvalues(:true, :false, :inf, :remote, /^[0-9]+$/) -- cgit From 1b66c28fb2ac5f5f10ce32b87ec459a480dcf161 Mon Sep 17 00:00:00 2001 From: Jacob Helwig Date: Fri, 1 Apr 2011 11:15:47 -0700 Subject: (#5437) Invalidate cached TypeCollection when there was an error parsing The caching of the TypeCollection in Puppet::Node::Environment would cause parse errors to occur (and be reported) only once and never again, until the file had changed on disk. This would also cause empty catalogs to be sent down to the agents further hiding the problem. Now, when a file fails to parse, it will be re-parsed every time on every following compilation, causing the parse error to be reported every time, and preventing sending down empty catalogs to agents. Paired-with: Nick Lewis --- lib/puppet/node/environment.rb | 2 +- lib/puppet/resource/type_collection.rb | 6 ++++++ spec/unit/node/environment_spec.rb | 29 +++++++++++++++-------------- spec/unit/resource/type_collection_spec.rb | 9 ++++++++- 4 files changed, 30 insertions(+), 16 deletions(-) diff --git a/lib/puppet/node/environment.rb b/lib/puppet/node/environment.rb index b64fb8a1f..807479bc8 100644 --- a/lib/puppet/node/environment.rb +++ b/lib/puppet/node/environment.rb @@ -79,7 +79,7 @@ class Puppet::Node::Environment # environment has changed do we delve deeper. Thread.current[:known_resource_types] = nil if (krt = Thread.current[:known_resource_types]) && krt.environment != self Thread.current[:known_resource_types] ||= synchronize { - if @known_resource_types.nil? or @known_resource_types.stale? + if @known_resource_types.nil? or @known_resource_types.require_reparse? @known_resource_types = Puppet::Resource::TypeCollection.new(self) @known_resource_types.perform_initial_import end diff --git a/lib/puppet/resource/type_collection.rb b/lib/puppet/resource/type_collection.rb index 347e1c0e0..4210475ad 100644 --- a/lib/puppet/resource/type_collection.rb +++ b/lib/puppet/resource/type_collection.rb @@ -166,12 +166,18 @@ class Puppet::Resource::TypeCollection end parser.parse rescue => detail + @parse_failed = true + msg = "Could not parse for environment #{environment}: #{detail}" error = Puppet::Error.new(msg) error.set_backtrace(detail.backtrace) raise error end + def require_reparse? + @parse_failed || stale? + end + def stale? @watched_files.values.detect { |file| file.changed? } end diff --git a/spec/unit/node/environment_spec.rb b/spec/unit/node/environment_spec.rb index 6edcce56c..6109007b9 100755 --- a/spec/unit/node/environment_spec.rb +++ b/spec/unit/node/environment_spec.rb @@ -85,28 +85,29 @@ describe Puppet::Node::Environment do @env.known_resource_types.should equal(@collection) end - it "should give to all threads the same collection if it didn't change" do - Puppet::Resource::TypeCollection.expects(:new).with(@env).returns @collection - @env.known_resource_types + it "should give to all threads using the same environment the same collection if the collection isn't stale" do + original_thread_type_collection = Puppet::Resource::TypeCollection.new(@env) + Puppet::Resource::TypeCollection.expects(:new).with(@env).returns original_thread_type_collection + @env.known_resource_types.should equal(original_thread_type_collection) + + original_thread_type_collection.expects(:require_reparse?).returns(false) + Puppet::Resource::TypeCollection.stubs(:new).with(@env).returns @collection t = Thread.new { - @env.known_resource_types.should equal(@collection) + @env.known_resource_types.should equal(original_thread_type_collection) } t.join end - it "should give to new threads a new collection if it isn't stale" do - Puppet::Resource::TypeCollection.expects(:new).with(@env).returns @collection - @env.known_resource_types.expects(:stale?).returns(true) - - Puppet::Resource::TypeCollection.expects(:new).returns @collection + it "should generate a new TypeCollection if the current one requires reparsing" do + old_type_collection = @env.known_resource_types + old_type_collection.stubs(:require_reparse?).returns true + Thread.current[:known_resource_types] = nil + new_type_collection = @env.known_resource_types - t = Thread.new { - @env.known_resource_types.should equal(@collection) - } - t.join + new_type_collection.should be_a Puppet::Resource::TypeCollection + new_type_collection.should_not equal(old_type_collection) end - end [:modulepath, :manifestdir].each do |setting| diff --git a/spec/unit/resource/type_collection_spec.rb b/spec/unit/resource/type_collection_spec.rb index cf7039a51..1576c0615 100644 --- a/spec/unit/resource/type_collection_spec.rb +++ b/spec/unit/resource/type_collection_spec.rb @@ -387,7 +387,7 @@ describe Puppet::Resource::TypeCollection do describe "when performing initial import" do before do - @parser = stub 'parser' + @parser = Puppet::Parser::Parser.new("test") Puppet::Parser::Parser.stubs(:new).returns @parser @code = Puppet::Resource::TypeCollection.new("env") end @@ -423,6 +423,13 @@ describe Puppet::Resource::TypeCollection do @parser.stubs(:file=) lambda { @code.perform_initial_import }.should raise_error(Puppet::Error) end + + it "should mark the type collection as needing a reparse when there is an error parsing" do + @parser.expects(:parse).raises Puppet::ParseError.new("Syntax error at ...") + + lambda { @code.perform_initial_import }.should raise_error(Puppet::Error, /Syntax error at .../) + @code.require_reparse?.should be_true + end end describe "when determining the configuration version" do -- cgit From d2bacd308ea214151f5c1f43ec62aad8075da41b Mon Sep 17 00:00:00 2001 From: James Turnbull Date: Mon, 4 Apr 2011 04:50:38 +1000 Subject: Fixed #3127 - Fixed gem selection regex Previously if the gem json_pure was installed and you requested the gem json then the regex matched too soon and falshly indicated that the json gem was already installed. Also updated to add the --no-ri and no-rdoc options and fix tests. --- lib/puppet/provider/package/gem.rb | 5 +++-- spec/unit/provider/package/gem_spec.rb | 12 +++++++++++- 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/lib/puppet/provider/package/gem.rb b/lib/puppet/provider/package/gem.rb index 19414cec4..64544db4e 100755 --- a/lib/puppet/provider/package/gem.rb +++ b/lib/puppet/provider/package/gem.rb @@ -1,5 +1,6 @@ require 'puppet/provider/package' require 'uri' +require 'pp' # Ruby gems support. Puppet::Type.type(:package).provide :gem, :parent => Puppet::Provider::Package do @@ -22,7 +23,7 @@ Puppet::Type.type(:package).provide :gem, :parent => Puppet::Provider::Package d end if name = hash[:justme] - command << name + command << name + "$" end begin @@ -94,7 +95,7 @@ Puppet::Type.type(:package).provide :gem, :parent => Puppet::Provider::Package d command << "--source" << "#{source}" << resource[:name] end else - command << resource[:name] + command << "--no-rdoc" << "--no-ri" << resource[:name] end output = execute(command) diff --git a/spec/unit/provider/package/gem_spec.rb b/spec/unit/provider/package/gem_spec.rb index 063e1474b..32c067a60 100644 --- a/spec/unit/provider/package/gem_spec.rb +++ b/spec/unit/provider/package/gem_spec.rb @@ -42,8 +42,18 @@ describe provider_class do @provider.install end + it "should specify that documentation should not be included" do + @provider.expects(:execute).with { |args| args[3] == "--no-rdoc" }.returns "" + @provider.install + end + + it "should specify that RI should not be included" do + @provider.expects(:execute).with { |args| args[4] == "--no-ri" }.returns "" + @provider.install + end + it "should specify the package name" do - @provider.expects(:execute).with { |args| args[3] == "myresource" }.returns "" + @provider.expects(:execute).with { |args| args[5] == "myresource" }.returns "" @provider.install end -- cgit From 9bb301814865db25a43b97869f0261f3bea9faf7 Mon Sep 17 00:00:00 2001 From: James Turnbull Date: Tue, 5 Apr 2011 01:48:23 +1000 Subject: Fixed #3127 - removed legacy debug code --- lib/puppet/provider/package/gem.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/puppet/provider/package/gem.rb b/lib/puppet/provider/package/gem.rb index 64544db4e..28731c849 100755 --- a/lib/puppet/provider/package/gem.rb +++ b/lib/puppet/provider/package/gem.rb @@ -1,6 +1,5 @@ require 'puppet/provider/package' require 'uri' -require 'pp' # Ruby gems support. Puppet::Type.type(:package).provide :gem, :parent => Puppet::Provider::Package do -- cgit From 306aa3032a3770c0d668907ae08042be88ec725e 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 | 11 ++++++++- lib/puppet/defaults.rb | 4 +++ lib/puppet/transaction/report.rb | 38 ++++++++++++++++++++-------- lib/puppet/util/metric.rb | 8 +++--- spec/integration/configurer_spec.rb | 43 ++++++++++++++++++++++++++++---- spec/unit/configurer_spec.rb | 48 ++++++++++++++++++++++++++++++++++++ spec/unit/transaction/report_spec.rb | 15 +++++++++-- 7 files changed, 144 insertions(+), 23 deletions(-) diff --git a/lib/puppet/configurer.rb b/lib/puppet/configurer.rb index d3c902576..9f68ca499 100644 --- a/lib/puppet/configurer.rb +++ b/lib/puppet/configurer.rb @@ -166,19 +166,28 @@ class Puppet::Configurer execute_postrun_command Puppet::Util::Log.close(report) - send_report(report, transaction) end def send_report(report, trans) report.finalize_report if trans puts report.summary if Puppet[:summarize] + save_last_run_summary(report) report.save if Puppet[:report] 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 36159bcf7..23b3b6652 100644 --- a/lib/puppet/defaults.rb +++ b/lib/puppet/defaults.rb @@ -607,6 +607,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 8e04759ad..16fee42ae 100644 --- a/lib/puppet/transaction/report.rb +++ b/lib/puppet/transaction/report.rb @@ -80,30 +80,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. @@ -142,7 +161,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.to_s] += 1 if status.send(state) end diff --git a/lib/puppet/util/metric.rb b/lib/puppet/util/metric.rb index 09bbb6137..835e1d610 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 @@ -133,7 +133,7 @@ class Puppet::Util::Metric def newvalue(name,value,label = nil) raise ArgumentError.new("metric name #{name.inspect} is not a string") unless name.is_a? String - label ||= labelize(name) + label ||= self.class.labelize(name) @values.push [name,label,value] end @@ -174,10 +174,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..780c9bbd0 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("apply") + 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 cf73a3706..a4b627c08 100755 --- a/spec/unit/configurer_spec.rb +++ b/spec/unit/configurer_spec.rb @@ -81,6 +81,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) @@ -225,9 +226,12 @@ describe Puppet::Configurer, "when executing a catalog run" do end describe Puppet::Configurer, "when sending a report" do + include PuppetSpec::Files + before do Puppet.settings.stubs(:use).returns(true) @configurer = Puppet::Configurer.new + Puppet[:lastrunfile] = tmpfile('last_run_file') @report = Puppet::Transaction::Report.new("apply") @trans = stub 'transaction' @@ -268,6 +272,20 @@ describe Puppet::Configurer, "when sending a report" do @configurer.send_report(@report, nil) 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, nil) + end + + it "should save the last run summary if reporting is disabled" do + Puppet.settings[:report] = false + + @configurer.expects(:save_last_run_summary).with(@report) + @configurer.send_report(@report, nil) + end + it "should log but not fail if saving the report fails" do Puppet.settings[:report] = true @@ -278,6 +296,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 766d4f14d..81efa340e 100755 --- a/spec/unit/transaction/report_spec.rb +++ b/spec/unit/transaction/report_spec.rb @@ -273,8 +273,19 @@ describe Puppet::Transaction::Report do @report.finalize_report 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 5d1cb02cd1ee509647a6dbe157152bd74d5e24ae 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 3749241f8..9f5921de1 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 cc733e1f5..7f0b95a89 100644 --- a/lib/puppet/application/apply.rb +++ b/lib/puppet/application/apply.rb @@ -148,6 +148,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 23b3b6652..2a1ded592 100644 --- a/lib/puppet/defaults.rb +++ b/lib/puppet/defaults.rb @@ -611,6 +611,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 8f498d4ba..c9d9a4509 100755 --- a/spec/unit/application/agent_spec.rb +++ b/spec/unit/application/agent_spec.rb @@ -179,6 +179,7 @@ describe Puppet::Application::Agent do Puppet.settings.stubs(:print_config) 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=) @@ -309,6 +310,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.expects(:[]=).with(:catalog_terminus, :rest) @puppetd.setup diff --git a/spec/unit/application/apply_spec.rb b/spec/unit/application/apply_spec.rb index d4f39abe0..67edd4ed7 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 From 99d78f2c15b6ccf543466d3fa2cdeda7eb3f9987 Mon Sep 17 00:00:00 2001 From: Markus Roberts Date: Fri, 25 Mar 2011 12:00:39 -0700 Subject: (#6490) Add plugin initialization callback system to core The recurring pattern "drop-in code needs to have something done at startup" is presently being solved with a variety of ad-hock mechanism. This commit adds a general, extensible, centralized system for adding such hooks and manages an extensible set of metadata about plugins which it collects by searching for files named "plugin_init.rb" in a series of directories. Initially, these are simply the $LOAD_PATH. Applications can add more places to look for plugins without risk of adding duplicates or changing the order of ones that have already been found with: Puppet::Plugins.look_in(*paths) The contents of each file found is executed in the context of a Puppet::Plugins object (and thus scoped). An example file might contain: ------------------------------------------------------- @name = "Greet the CA" @description = %q{ This plugin causes a friendly greeting to print out on a master that is operating as the CA, after it has been set up but before it does anything. } def after_application_setup(options) if options[:application_object].is_a?(Puppet::Application::Master) && Puppet::SSL::CertificateAuthority.ca? puts "Hey, this is the CA! Hi everyone!!!" end end ------------------------------------------------------- Note that the instance variables are local to this Puppet::Plugin (and so may be used for maintaining state, etc.) but the plugin system does not provide any thread safety assurances, so they may not be adequate for some complex use cases. Presently supported hooks: before_application_preinit( :application_object => ... ) after_application_preinit( :application_object => ... ) before_application_parse_options( :application_object => ... ) after_application_parse_options( :application_object => ... ) before_application_setup( :application_object => ... ) after_application_setup( :application_object => ... ) before_application_run_command( :application_object => ... ) after_application_run_command( :application_object => ... ) on_commandline_initialization(:command_line_object => ... ) on_application_initialization(:appliation_object => ... ) Paired-with: Daniel Pitman --- lib/puppet/application.rb | 16 ++++++-- lib/puppet/util/command_line.rb | 7 +++- lib/puppet/util/plugins.rb | 82 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 100 insertions(+), 5 deletions(-) create mode 100644 lib/puppet/util/plugins.rb diff --git a/lib/puppet/application.rb b/lib/puppet/application.rb index c3d7355f6..5e69baef4 100644 --- a/lib/puppet/application.rb +++ b/lib/puppet/application.rb @@ -1,4 +1,5 @@ require 'optparse' +require 'puppet/util/plugins' # This class handles all the aspects of a Puppet application/executable # * setting up options @@ -297,11 +298,11 @@ class Application # This is the main application entry point def run - exit_on_fail("initialize") { preinit } - exit_on_fail("parse options") { parse_options } + exit_on_fail("initialize") { hook('preinit') { preinit } } + exit_on_fail("parse options") { hook('parse_options') { parse_options } } exit_on_fail("parse configuration file") { Puppet.settings.parse } if should_parse_config? - exit_on_fail("prepare for execution") { setup } - exit_on_fail("run") { run_command } + exit_on_fail("prepare for execution") { hook('setup') { setup } } + exit_on_fail("run") { hook('run_command') { run_command } } end def main @@ -413,5 +414,12 @@ class Application $stderr.puts "Could not #{message}: #{detail}" exit(code) end + + def hook(step,&block) + Puppet::Plugins.send("before_application_#{step}",:application_object => self) + x = yield + Puppet::Plugins.send("after_application_#{step}",:application_object => self, :return_value => x) + x + end end end diff --git a/lib/puppet/util/command_line.rb b/lib/puppet/util/command_line.rb index 7f74d266a..fb56b2898 100644 --- a/lib/puppet/util/command_line.rb +++ b/lib/puppet/util/command_line.rb @@ -1,3 +1,5 @@ +require "puppet/util/plugins" + module Puppet module Util class CommandLine @@ -23,6 +25,7 @@ module Puppet @stdin = stdin @subcommand_name, @args = subcommand_and_args( @zero, @argv, @stdin ) + Puppet::Plugins.on_commandline_initialization(:command_line_object => self) end attr :subcommand_name @@ -56,7 +59,9 @@ module Puppet puts usage_message elsif available_subcommands.include?(subcommand_name) #subcommand require_application subcommand_name - Puppet::Application.find(subcommand_name).new(self).run + app = Puppet::Application.find(subcommand_name).new(self) + Puppet::Plugins.on_application_initialization(:appliation_object => self) + app.run else abort "Error: Unknown command #{subcommand_name}.\n#{usage_message}" unless execute_external_subcommand end diff --git a/lib/puppet/util/plugins.rb b/lib/puppet/util/plugins.rb new file mode 100644 index 000000000..105fdcd75 --- /dev/null +++ b/lib/puppet/util/plugins.rb @@ -0,0 +1,82 @@ +# +# This system manages an extensible set of metadata about plugins which it +# collects by searching for files named "plugin_init.rb" in a series of +# directories. Initially, these are simply the $LOAD_PATH. +# +# The contents of each file found is executed in the context of a Puppet::Plugins +# object (and thus scoped). An example file might contain: +# +# ------------------------------------------------------- +# @name = "Greet the CA" +# +# @description = %q{ +# This plugin causes a friendly greeting to print out on a master +# that is operating as the CA, after it has been set up but before +# it does anything. +# } +# +# def after_application_setup(options) +# if options[:application_object].is_a?(Puppet::Application::Master) && Puppet::SSL::CertificateAuthority.ca? +# puts "Hey, this is the CA!" +# end +# end +# ------------------------------------------------------- +# +# Note that the instance variables are local to this Puppet::Plugin (and so may be used +# for maintaining state, etc.) but the plugin system does not provide any thread safety +# assurances, so they may not be adequate for some complex use cases. +# +# +module Puppet + class Plugins + Paths = [] # Where we might find plugin initialization code + Loaded = [] # Code we have found (one-to-one with paths once searched) + # + # Return all the Puppet::Plugins we know about, searching any new paths + # + def self.known + Paths[Loaded.length...Paths.length].each { |path| + file = File.join(path,'plugin_init.rb') + Loaded << (File.exist?(file) && new(file)) + } + Loaded.compact + end + # + # Add more places to look for plugins without adding duplicates or changing the + # order of ones we've already found. + # + def self.look_in(*paths) + Paths.replace Paths | paths.flatten.collect { |path| File.expand_path(path) } + end + # + # Initially just look in $LOAD_PATH + # + look_in $LOAD_PATH + # + # Calling methods (hooks) on the class calls the method of the same name on + # all plugins that use that hook, passing in the same arguments to each + # and returning an array containing the results returned by each plugin as + # an array of [plugin_name,result] pairs. + # + def self.method_missing(hook,*args,&block) + known. + select { |p| p.respond_to? hook }. + collect { |p| [p.name,p.send(hook,*args,&block)] } + end + # + # + # + attr_reader :path,:name + def initialize(path) + @name = @path = path + class << self + private + def define_hooks + eval File.read(path),nil,path,1 + end + end + define_hooks + end + end +end + -- cgit From 1e4968e82a65b21ad5d075015830ef3f54efee05 Mon Sep 17 00:00:00 2001 From: Markus Roberts Date: Wed, 6 Apr 2011 20:25:45 -0700 Subject: (maint) Indentation fixes --- lib/puppet/application.rb | 8 ++++---- lib/puppet/util/command_line.rb | 20 +++++++++----------- 2 files changed, 13 insertions(+), 15 deletions(-) diff --git a/lib/puppet/application.rb b/lib/puppet/application.rb index 5e69baef4..a028a158f 100644 --- a/lib/puppet/application.rb +++ b/lib/puppet/application.rb @@ -408,11 +408,11 @@ class Application private def exit_on_fail(message, code = 1) - yield + yield rescue RuntimeError, NotImplementedError => detail - puts detail.backtrace if Puppet[:trace] - $stderr.puts "Could not #{message}: #{detail}" - exit(code) + puts detail.backtrace if Puppet[:trace] + $stderr.puts "Could not #{message}: #{detail}" + exit(code) end def hook(step,&block) diff --git a/lib/puppet/util/command_line.rb b/lib/puppet/util/command_line.rb index fb56b2898..52b5f81ef 100644 --- a/lib/puppet/util/command_line.rb +++ b/lib/puppet/util/command_line.rb @@ -4,8 +4,7 @@ module Puppet module Util class CommandLine - LegacyName = Hash.new{|h,k| k}.update( - { + LegacyName = Hash.new{|h,k| k}.update( 'agent' => 'puppetd', 'cert' => 'puppetca', 'doc' => 'puppetdoc', @@ -15,9 +14,8 @@ module Puppet 'queue' => 'puppetqd', 'resource' => 'ralsh', 'kick' => 'puppetrun', - 'master' => 'puppetmasterd', - - }) + 'master' => 'puppetmasterd' + ) def initialize( zero = $0, argv = ARGV, stdin = STDIN ) @zero = zero @@ -68,14 +66,14 @@ module Puppet end def execute_external_subcommand - external_command = "puppet-#{subcommand_name}" + external_command = "puppet-#{subcommand_name}" - require 'puppet/util' - path_to_subcommand = Puppet::Util.which( external_command ) - return false unless path_to_subcommand + require 'puppet/util' + path_to_subcommand = Puppet::Util.which( external_command ) + return false unless path_to_subcommand - system( path_to_subcommand, *args ) - true + system( path_to_subcommand, *args ) + true end def legacy_executable_name -- cgit