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 <[email protected]> Signed-off-by: Jacob Helwig <[email protected]> --- Local-branch: ticket/2.6.x/5437-report-manifest-errors-on-agent 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 b64fb8a..807479b 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 347e1c0..4210475 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 6edcce5..6109007 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 cf7039a..1576c06 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 -- 1.7.4.2 -- You received this message because you are subscribed to the Google Groups "Puppet Developers" group. To post to this group, send email to [email protected]. To unsubscribe from this group, send email to [email protected]. For more options, visit this group at http://groups.google.com/group/puppet-dev?hl=en.
