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 --- spec/integration/transaction_spec.rb | 61 ++++++++++++++++++++---------------- 1 file changed, 34 insertions(+), 27 deletions(-) (limited to 'spec') 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 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 --- spec/unit/node/environment_spec.rb | 29 +++++++++++++++-------------- spec/unit/resource/type_collection_spec.rb | 9 ++++++++- 2 files changed, 23 insertions(+), 15 deletions(-) (limited to 'spec') 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. --- spec/unit/provider/package/gem_spec.rb | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) (limited to 'spec') 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 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 --- spec/integration/configurer_spec.rb | 43 ++++++++++++++++++++++++++++---- spec/unit/configurer_spec.rb | 48 ++++++++++++++++++++++++++++++++++++ spec/unit/transaction/report_spec.rb | 15 +++++++++-- 3 files changed, 99 insertions(+), 7 deletions(-) (limited to 'spec') 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 --- spec/unit/application/agent_spec.rb | 7 ++++++ spec/unit/application/apply_spec.rb | 6 +++++ spec/unit/indirector/report/yaml_spec.rb | 38 ++++++++++++++++++++++++++++++++ 3 files changed, 51 insertions(+) create mode 100644 spec/unit/indirector/report/yaml_spec.rb (limited to 'spec') 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