This change is part of an ongoing effort to remove functionality from TypeCollection that is not related to keeping track of a collection of types. This reduces TypeCollection's linkage to the environment, which is a step toward decoupling it from the type loading mechanism.
Signed-off-by: Paul Berry <[email protected]> --- lib/puppet/node/environment.rb | 36 +++++++++++- lib/puppet/parser/compiler.rb | 2 +- lib/puppet/parser/parser_support.rb | 2 +- lib/puppet/resource/type_collection.rb | 32 ---------- spec/unit/application/apply_spec.rb | 11 ++-- spec/unit/application/master_spec.rb | 9 +-- spec/unit/node/environment_spec.rb | 87 ++++++++++++++++++++++++++- spec/unit/parser/compiler_spec.rb | 4 +- spec/unit/parser/parser_spec.rb | 2 +- spec/unit/resource/type_collection_spec.rb | 81 -------------------------- 10 files changed, 132 insertions(+), 134 deletions(-) diff --git a/lib/puppet/node/environment.rb b/lib/puppet/node/environment.rb index 3f67474..e43dca7 100644 --- a/lib/puppet/node/environment.rb +++ b/lib/puppet/node/environment.rb @@ -81,7 +81,7 @@ class Puppet::Node::Environment Thread.current[:known_resource_types] ||= synchronize { if @known_resource_types.nil? or @known_resource_types.stale? @known_resource_types = Puppet::Resource::TypeCollection.new(self) - @known_resource_types.perform_initial_import + perform_initial_import end @known_resource_types } @@ -143,5 +143,39 @@ class Puppet::Node::Environment end end + def version + return @version if defined?(@version) + + if self[:config_version] == "" + @version = Time.now.to_i + return @version + end + + @version = Puppet::Util.execute([self[:config_version]]).strip + + rescue Puppet::ExecutionFailure => e + raise Puppet::ParseError, "Unable to set config_version: #{e.message}" + end + + private + + def perform_initial_import + return if Puppet.settings[:ignoreimport] + parser = Puppet::Parser::Parser.new(self) + if code = Puppet.settings.uninterpolated_value(:code, name.to_s) and code != "" + parser.string = code + else + file = Puppet.settings.value(:manifest, name.to_s) + return unless File.exist?(file) + parser.file = file + end + parser.parse + rescue => detail + msg = "Could not parse for environment #{self}: #{detail}" + error = Puppet::Error.new(msg) + error.set_backtrace(detail.backtrace) + raise error + end + @root = new(:'*root*') end diff --git a/lib/puppet/parser/compiler.rb b/lib/puppet/parser/compiler.rb index 61bb13c..21b6d84 100644 --- a/lib/puppet/parser/compiler.rb +++ b/lib/puppet/parser/compiler.rb @@ -426,7 +426,7 @@ class Puppet::Parser::Compiler # For maintaining the relationship between scopes and their resources. @catalog = Puppet::Resource::Catalog.new(@node.name) - @catalog.version = known_resource_types.version + @catalog.version = environment.version # Create our initial scope and a resource that will evaluate main. @topscope = Puppet::Parser::Scope.new(:compiler => self) diff --git a/lib/puppet/parser/parser_support.rb b/lib/puppet/parser/parser_support.rb index 97d985c..f3f98e8 100644 --- a/lib/puppet/parser/parser_support.rb +++ b/lib/puppet/parser/parser_support.rb @@ -223,7 +223,7 @@ class Puppet::Parser::Parser end def version - known_resource_types.version + environment.version end # Add a new file to be checked when we're checking to see if we should be diff --git a/lib/puppet/resource/type_collection.rb b/lib/puppet/resource/type_collection.rb index 3a327e2..1a013bc 100644 --- a/lib/puppet/resource/type_collection.rb +++ b/lib/puppet/resource/type_collection.rb @@ -110,42 +110,10 @@ class Puppet::Resource::TypeCollection end end - def perform_initial_import - return if Puppet.settings[:ignoreimport] - parser = Puppet::Parser::Parser.new(environment) - if code = Puppet.settings.uninterpolated_value(:code, environment.to_s) and code != "" - parser.string = code - else - file = Puppet.settings.value(:manifest, environment.to_s) - return unless File.exist?(file) - parser.file = file - end - parser.parse - rescue => detail - msg = "Could not parse for environment #{environment}: #{detail}" - error = Puppet::Error.new(msg) - error.set_backtrace(detail.backtrace) - raise error - end - def stale? @watched_files.values.detect { |file| file.changed? } end - def version - return @version if defined?(@version) - - if environment[:config_version] == "" - @version = Time.now.to_i - return @version - end - - @version = Puppet::Util.execute([environment[:config_version]]).strip - - rescue Puppet::ExecutionFailure => e - raise Puppet::ParseError, "Unable to set config_version: #{e.message}" - end - def watch_file(file) @watched_files[file] = Puppet::Util::LoadedFile.new(file) end diff --git a/spec/unit/application/apply_spec.rb b/spec/unit/application/apply_spec.rb index 0b00d1a..a79ddba 100755 --- a/spec/unit/application/apply_spec.rb +++ b/spec/unit/application/apply_spec.rb @@ -142,17 +142,16 @@ describe Puppet::Application::Apply do describe "the parseonly command" do before :each do - Puppet.stubs(:[]).with(:environment) + @environment = Puppet::Node::Environment.new("env") + Puppet.stubs(:[]).with(:environment).returns(@environment) Puppet.stubs(:[]).with(:manifest).returns("site.pp") Puppet.stubs(:err) @apply.stubs(:exit) @apply.options.stubs(:[]).with(:code).returns "some code" - @collection = stub_everything - Puppet::Resource::TypeCollection.stubs(:new).returns(@collection) end - it "should use a Puppet Resource Type Collection to parse the file" do - @collection.expects(:perform_initial_import) + it "should use the environment to parse the file" do + @environment.stubs(:perform_initial_import) @apply.parseonly end @@ -162,7 +161,7 @@ describe Puppet::Application::Apply do end it "should exit with exit code 1 if error" do - @collection.stubs(:perform_initial_import).raises(Puppet::ParseError) + @environment.stubs(:perform_initial_import).raises(Puppet::ParseError) @apply.expects(:exit).with(1) @apply.parseonly end diff --git a/spec/unit/application/master_spec.rb b/spec/unit/application/master_spec.rb index 0baa822..d230213 100644 --- a/spec/unit/application/master_spec.rb +++ b/spec/unit/application/master_spec.rb @@ -257,16 +257,15 @@ describe Puppet::Application::Master do describe "the parseonly command" do before :each do - Puppet.stubs(:[]).with(:environment) + @environment = Puppet::Node::Environment.new("env") + Puppet.stubs(:[]).with(:environment).returns(@environment) Puppet.stubs(:[]).with(:manifest).returns("site.pp") Puppet.stubs(:err) @master.stubs(:exit) - @collection = stub_everything - Puppet::Resource::TypeCollection.stubs(:new).returns(@collection) end it "should use a Puppet Resource Type Collection to parse the file" do - @collection.expects(:perform_initial_import) + @environment.expects(:perform_initial_import) @master.parseonly end @@ -276,7 +275,7 @@ describe Puppet::Application::Master do end it "should exit with exit code 1 if error" do - @collection.stubs(:perform_initial_import).raises(Puppet::ParseError) + @environment.stubs(:perform_initial_import).raises(Puppet::ParseError) @master.expects(:exit).with(1) @master.parseonly end diff --git a/spec/unit/node/environment_spec.rb b/spec/unit/node/environment_spec.rb index 6edcce5..1bc0221 100755 --- a/spec/unit/node/environment_spec.rb +++ b/spec/unit/node/environment_spec.rb @@ -52,7 +52,7 @@ describe Puppet::Node::Environment do before do @env = Puppet::Node::Environment.new("dev") @collection = Puppet::Resource::TypeCollection.new(@env) - @collection.stubs(:perform_initial_import) + @env.stubs(:perform_initial_import) Thread.current[:known_resource_types] = nil end @@ -66,9 +66,8 @@ describe Puppet::Node::Environment do end it "should perform the initial import when creating a new collection" do - @collection.expects(:perform_initial_import) - Puppet::Resource::TypeCollection.expects(:new).returns @collection - + @env = Puppet::Node::Environment.new("dev") + @env.expects(:perform_initial_import) @env.known_resource_types end @@ -274,4 +273,84 @@ describe Puppet::Node::Environment do @helper.environment.name.should == :foo end end + + describe "when performing initial import" do + before do + @parser = stub 'parser', :file= => nil, :string => nil, :parse => nil + Puppet::Parser::Parser.stubs(:new).returns @parser + @env = Puppet::Node::Environment.new("env") + end + + it "should create a new parser instance" do + Puppet::Parser::Parser.expects(:new).returns @parser + @env.instance_eval { perform_initial_import } + end + + it "should set the parser's string to the 'code' setting and parse if code is available" do + Puppet.settings[:code] = "my code" + @parser.expects(:string=).with "my code" + @parser.expects(:parse) + @env.instance_eval { perform_initial_import } + end + + it "should set the parser's file to the 'manifest' setting and parse if no code is available and the manifest is available" do + File.stubs(:expand_path).with("/my/file").returns "/my/file" + File.expects(:exist?).with("/my/file").returns true + Puppet.settings[:manifest] = "/my/file" + @parser.expects(:file=).with "/my/file" + @parser.expects(:parse) + @env.instance_eval { perform_initial_import } + end + + it "should not attempt to load a manifest if none is present" do + File.stubs(:expand_path).with("/my/file").returns "/my/file" + File.expects(:exist?).with("/my/file").returns false + Puppet.settings[:manifest] = "/my/file" + @parser.expects(:file=).never + @parser.expects(:parse).never + @env.instance_eval { perform_initial_import } + end + + it "should fail helpfully if there is an error importing" do + File.stubs(:exist?).returns true + @parser.expects(:parse).raises ArgumentError + lambda { @env.instance_eval { perform_initial_import } }.should raise_error(Puppet::Error) + end + + it "should not do anything if the ignore_import settings is set" do + Puppet.settings[:ignoreimport] = true + @parser.expects(:string=).never + @parser.expects(:file=).never + @parser.expects(:parse).never + @env.instance_eval { perform_initial_import } + end + end + + describe "when determining the configuration version" do + before do + @env = Puppet::Node::Environment.new("env") + end + + it "should default to the current time" do + time = Time.now + + Time.stubs(:now).returns time + @env.version.should == time.to_i + end + + it "should use the output of the environment's config_version setting if one is provided" do + @env.stubs(:[]).with(:config_version).returns("/my/foo") + + Puppet::Util.expects(:execute).with(["/my/foo"]).returns "output\n" + @env.version.should == "output" + end + + it "should raise a puppet parser error if executing config_version fails" do + @env.stubs(:[]).with(:config_version).returns("test") + Puppet::Util.expects(:execute).raises(Puppet::ExecutionFailure.new("msg")) + + lambda { @env.version }.should raise_error(Puppet::ParseError) + end + + end end diff --git a/spec/unit/parser/compiler_spec.rb b/spec/unit/parser/compiler_spec.rb index e8c06dd..8befccc 100755 --- a/spec/unit/parser/compiler_spec.rb +++ b/spec/unit/parser/compiler_spec.rb @@ -96,8 +96,8 @@ describe Puppet::Parser::Compiler do @compiler.ast_nodes?.should be_true end - it "should copy the known_resource_types version to the catalog" do - @compiler.catalog.version.should == @known_resource_types.version + it "should copy the environment version to the catalog" do + @compiler.catalog.version.should == @compiler.environment.version end it "should copy any node classes into the class list" do diff --git a/spec/unit/parser/parser_spec.rb b/spec/unit/parser/parser_spec.rb index 0657ab3..7d0428f 100755 --- a/spec/unit/parser/parser_spec.rb +++ b/spec/unit/parser/parser_spec.rb @@ -334,7 +334,7 @@ describe Puppet::Parser do describe "when determining the configuration version" do it "should determine it from the resource type collection" do - @parser.known_resource_types.expects(:version).returns "foo" + @parser.environment.expects(:version).returns "foo" @parser.version.should == "foo" end end diff --git a/spec/unit/resource/type_collection_spec.rb b/spec/unit/resource/type_collection_spec.rb index 4bad291..9115d90 100644 --- a/spec/unit/resource/type_collection_spec.rb +++ b/spec/unit/resource/type_collection_spec.rb @@ -415,85 +415,4 @@ describe Puppet::Resource::TypeCollection do @loader.should_not be_stale end end - - describe "when performing initial import" do - before do - @parser = stub 'parser', :file= => nil, :string => nil, :parse => nil - Puppet::Parser::Parser.stubs(:new).returns @parser - @code = Puppet::Resource::TypeCollection.new("env") - end - - it "should create a new parser instance" do - Puppet::Parser::Parser.expects(:new).returns @parser - @code.perform_initial_import - end - - it "should set the parser's string to the 'code' setting and parse if code is available" do - Puppet.settings[:code] = "my code" - @parser.expects(:string=).with "my code" - @parser.expects(:parse) - @code.perform_initial_import - end - - it "should set the parser's file to the 'manifest' setting and parse if no code is available and the manifest is available" do - File.stubs(:expand_path).with("/my/file").returns "/my/file" - File.expects(:exist?).with("/my/file").returns true - Puppet.settings[:manifest] = "/my/file" - @parser.expects(:file=).with "/my/file" - @parser.expects(:parse) - @code.perform_initial_import - end - - it "should not attempt to load a manifest if none is present" do - File.stubs(:expand_path).with("/my/file").returns "/my/file" - File.expects(:exist?).with("/my/file").returns false - Puppet.settings[:manifest] = "/my/file" - @parser.expects(:file=).never - @parser.expects(:parse).never - @code.perform_initial_import - end - - it "should fail helpfully if there is an error importing" do - File.stubs(:exist?).returns true - @parser.expects(:parse).raises ArgumentError - lambda { @code.perform_initial_import }.should raise_error(Puppet::Error) - end - - it "should not do anything if the ignore_import settings is set" do - Puppet.settings[:ignoreimport] = true - @parser.expects(:string=).never - @parser.expects(:file=).never - @parser.expects(:parse).never - @code.perform_initial_import - end - end - - describe "when determining the configuration version" do - before do - @code = Puppet::Resource::TypeCollection.new("env") - end - - it "should default to the current time" do - time = Time.now - - Time.stubs(:now).returns time - @code.version.should == time.to_i - end - - it "should use the output of the environment's config_version setting if one is provided" do - @code.environment.stubs(:[]).with(:config_version).returns("/my/foo") - - Puppet::Util.expects(:execute).with(["/my/foo"]).returns "output\n" - @code.version.should == "output" - end - - it "should raise a puppet parser error if executing config_version fails" do - @code.environment.stubs(:[]).with(:config_version).returns("test") - Puppet::Util.expects(:execute).raises(Puppet::ExecutionFailure.new("msg")) - - lambda { @code.version }.should raise_error(Puppet::ParseError) - end - - end - end -- 1.7.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.
