diff --git a/lib/puppet/parser/compiler.rb b/lib/puppet/parser/compiler.rb index 6cc71a62e..beba438a9 100644 --- a/lib/puppet/parser/compiler.rb +++ b/lib/puppet/parser/compiler.rb @@ -54,10 +54,10 @@ class Puppet::Parser::Compiler set_container_resource(scope, resource) end + # Add our container edge. If we're a class, then we get treated specially - we can + # control the stage that the class is applied in. Otherwise, we just + # get added to our parent container. def set_container_resource(scope, resource) - # Add our container edge. If we're a class, then we get treated specially - we can - # control the stage that the class is applied in. Otherwise, we just - # get added to our parent container. return if resource.type.to_s.downcase == "stage" if resource.type.to_s.downcase != "class" @@ -305,6 +305,8 @@ class Puppet::Parser::Compiler @resources << @main_resource @catalog.add_resource(@main_resource) + set_container_resource(@topscope, @main_resource) + @main_resource.evaluate end diff --git a/lib/puppet/resource.rb b/lib/puppet/resource.rb index d2979f930..34d609703 100644 --- a/lib/puppet/resource.rb +++ b/lib/puppet/resource.rb @@ -267,7 +267,7 @@ class Puppet::Resource # Translate our object to a backward-compatible transportable object. def to_trans - if builtin_type? + if builtin_type? and type.downcase.to_s != "stage" result = to_transobject else result = to_transbucket diff --git a/lib/puppet/resource/catalog.rb b/lib/puppet/resource/catalog.rb index 3a28f45c8..a7a4b5bd0 100644 --- a/lib/puppet/resource/catalog.rb +++ b/lib/puppet/resource/catalog.rb @@ -33,9 +33,6 @@ class Puppet::Resource::Catalog < Puppet::SimpleGraph # How long this catalog took to retrieve. Used for reporting stats. attr_accessor :retrieval_duration - # How we should extract the catalog for sending to the client. - attr_reader :extraction_format - # Whether this is a host catalog, which behaves very differently. # In particular, reports are sent, graphs are made, and state is # stored in the state database. If this is set incorrectly, then you often @@ -201,33 +198,22 @@ class Puppet::Resource::Catalog < Puppet::SimpleGraph end end - # Make sure we support the requested extraction format. - def extraction_format=(value) - unless respond_to?("extract_to_%s" % value) - raise ArgumentError, "Invalid extraction format %s" % value - end - @extraction_format = value - end - - # Turn our catalog graph into whatever the client is expecting. - def extract - send("extract_to_%s" % extraction_format) - end - - # Create the traditional TransBuckets and TransObjects from our catalog - # graph. LAK:NOTE(20081211): This is a pre-0.25 backward compatibility method. + # Turn our catalog graph into an old-style tree of TransObjects and TransBuckets. + # LAK:NOTE(20081211): This is a pre-0.25 backward compatibility method. # It can be removed as soon as xmlrpc is killed. - def extract_to_transportable + def extract top = nil current = nil buckets = {} - unless main = vertices.find { |res| res.type == "Class" and res.title == :main } - raise Puppet::DevError, "Could not find 'main' class; cannot generate catalog" + unless main = resource(:stage, "main") + raise Puppet::DevError, "Could not find 'main' stage; cannot generate catalog" + end + + if stages = vertices.find_all { |v| v.type == "Stage" and v.title != "main" } and ! stages.empty? + Puppet.warning "Stages are not supported by 0.24.x client; stage(s) #{stages.collect { |s| s.to_s }.join(', ') } will be ignored" end - # Create a proc for examining edges, which we'll use to build our tree - # of TransBuckets and TransObjects. bucket = nil walk(main, :out) do |source, target| # The sources are always non-builtins. @@ -285,7 +271,6 @@ class Puppet::Resource::Catalog < Puppet::SimpleGraph def initialize(name = nil) super() @name = name if name - @extraction_format ||= :transportable @classes = [] @resource_table = {} @transient_resources = [] diff --git a/lib/puppet/type.rb b/lib/puppet/type.rb index 7f8fb09f3..e8545413c 100644 --- a/lib/puppet/type.rb +++ b/lib/puppet/type.rb @@ -1407,24 +1407,24 @@ class Type a dependency on or from the named milestone. For instance, saying that this is in the 'bootstrap' stage creates a dependency on the 'bootstrap' milestone. - + By default, all classes get directly added to the 'main' stage. You can create new stages as resources: - + stage { [pre, post]: } - + To order stages, use standard relationships: - + stage { pre: before => Stage[main] } - + Or use the new relationship syntax: - + Stage[pre] -> Stage[main] -> Stage[post] - + Then use the new class parameters to specify a stage: - + class { foo: stage => pre } - + Stages can only be set on classes, not individual resources. This will fail:: diff --git a/lib/puppet/type/stage.rb b/lib/puppet/type/stage.rb index fba78764d..c11acdcd8 100644 --- a/lib/puppet/type/stage.rb +++ b/lib/puppet/type/stage.rb @@ -2,11 +2,11 @@ Puppet::Type.newtype(:stage) do desc "A resource type for specifying run stages. The actual stage should be specified on resources:: class { foo: stage => pre } - + And you must manually control stage order:: - + stage { pre: before => Stage[main] } - + You automatically get a 'main' stage created, and by default all resources get inserted into that stage. diff --git a/spec/unit/parser/compiler.rb b/spec/unit/parser/compiler.rb index e7d336583..fb210bc02 100755 --- a/spec/unit/parser/compiler.rb +++ b/spec/unit/parser/compiler.rb @@ -200,6 +200,14 @@ describe Puppet::Parser::Compiler do @known_resource_types.find_hostclass([""], "").should be_instance_of(Puppet::Resource::Type) end + it "should add an edge between the main stage and main class" do + @compiler.compile + (stage = @compiler.catalog.resource(:stage, "main")).should be_instance_of(Puppet::Parser::Resource) + (klass = @compiler.catalog.resource(:class, "")).should be_instance_of(Puppet::Parser::Resource) + + @compiler.catalog.edge?(stage, klass).should be_true + end + it "should evaluate any node classes" do @node.stubs(:classes).returns(%w{one two three four}) @compiler.expects(:evaluate_classes).with(%w{one two three four}, @compiler.topscope) diff --git a/spec/unit/resource.rb b/spec/unit/resource.rb index 3840db535..a2b214dae 100755 --- a/spec/unit/resource.rb +++ b/spec/unit/resource.rb @@ -533,6 +533,14 @@ describe Puppet::Resource do bucket.name.should == @resource.title end + it "should return a simple TransBucket if it is a stage" do + @resource = Puppet::Resource.new("stage", "bar") + bucket = @resource.to_trans + bucket.should be_instance_of(Puppet::TransBucket) + bucket.type.should == @resource.type + bucket.name.should == @resource.title + end + it "should copy over the resource's file" do @resource.file = "/foo/bar" @resource.to_trans.file.should == "/foo/bar" diff --git a/spec/unit/resource/catalog.rb b/spec/unit/resource/catalog.rb index 3020dc80e..6c6a5e15e 100755 --- a/spec/unit/resource/catalog.rb +++ b/spec/unit/resource/catalog.rb @@ -88,15 +88,6 @@ describe Puppet::Resource::Catalog, "when compiling" do end end - describe "when extracting" do - it "should return extraction result as the method result" do - config = Puppet::Resource::Catalog.new("mynode") - config.expects(:extraction_format).returns(:whatever) - config.expects(:extract_to_whatever).returns(:result) - config.extract.should == :result - end - end - describe "when extracting transobjects" do def mkscope @@ -112,23 +103,39 @@ describe Puppet::Resource::Catalog, "when compiling" do Puppet::Parser::Resource.new(type, name, :source => @source, :scope => @scope) end - it "should always create a TransBucket for the 'main' class" do + it "should fail if no 'main' stage can be found" do + lambda { Puppet::Resource::Catalog.new("mynode").extract }.should raise_error(Puppet::DevError) + end + + it "should warn if any non-main stages are present" do config = Puppet::Resource::Catalog.new("mynode") @scope = mkscope @source = mock 'source' - main = mkresource("class", :main) - config.add_vertex(main) + main = mkresource("stage", "main") + config.add_resource(main) - bucket = stub 'bucket', :file= => nil, :line= => nil, :classes= => nil - bucket.expects(:type=).with("Class") - bucket.expects(:name=).with(:main) - main.stubs(:builtin?).returns(false) + other = mkresource("stage", "other") + config.add_resource(other) - Puppet::TransBucket.expects(:new).returns bucket + Puppet.expects(:warning) - config.extract_to_transportable.should equal(bucket) + config.extract + end + + it "should always create a TransBucket for the 'main' stage" do + config = Puppet::Resource::Catalog.new("mynode") + + @scope = mkscope + @source = mock 'source' + + main = mkresource("stage", "main") + config.add_resource(main) + + result = config.extract + result.type.should == "Stage" + result.name.should == "main" end # Now try it with a more complicated graph -- a three tier graph, each tier @@ -140,7 +147,9 @@ describe Puppet::Resource::Catalog, "when compiling" do @source = mock 'source' # Create our scopes. - top = mkresource "class", :main + top = mkresource "stage", "main" + + config.add_resource top topbucket = [] topbucket.expects(:classes=).with([]) top.expects(:to_trans).returns(topbucket) @@ -162,7 +171,7 @@ describe Puppet::Resource::Catalog, "when compiling" do botres.expects(:to_trans).returns(:botres) config.add_edge bottom, botres - toparray = config.extract_to_transportable + toparray = config.extract # This is annoying; it should look like: # [[[:botres], :midres], :topres] diff --git a/test/language/scope.rb b/test/language/scope.rb index 17e97ecfb..0bed35c14 100755 --- a/test/language/scope.rb +++ b/test/language/scope.rb @@ -201,13 +201,9 @@ Host <<||>>" node = mknode node.merge "hostname" => node.name 2.times { |i| - config = Puppet::Parser::Compiler.new(node).compile - - flat = config.extract.flatten - - %w{puppet myhost}.each do |name| - assert(flat.find{|o| o.name == name }, "Did not find #{name}") - end + catalog = Puppet::Parser::Compiler.new(node).compile + assert_instance_of(Puppet::Parser::Resource, catalog.resource(:host, "puppet")) + assert_instance_of(Puppet::Parser::Resource, catalog.resource(:host, "myhost")) } ensure Puppet[:storeconfigs] = false diff --git a/test/language/snippets.rb b/test/language/snippets.rb index f083c355c..0d647f7de 100755 --- a/test/language/snippets.rb +++ b/test/language/snippets.rb @@ -204,11 +204,7 @@ class TestSnippets < Test::Unit::TestCase file = @catalog.resource(:file, path) assert(file, "did not create file %s" % path) - assert_nothing_raised { - assert_equal( - "//Testing/Mytype[componentname]/File[/tmp/classtest]", - file.path) - } + assert_equal( "/Stage[main]/Testing/Mytype[componentname]/File[/tmp/classtest]", file.path) end def snippet_argumentdefaults diff --git a/test/lib/puppettest/parsertesting.rb b/test/lib/puppettest/parsertesting.rb index 0cc3ddef4..5ca5123e8 100644 --- a/test/lib/puppettest/parsertesting.rb +++ b/test/lib/puppettest/parsertesting.rb @@ -297,21 +297,11 @@ module PuppetTest::ParserTesting # This assumes no nodes def assert_creates(manifest, *files) - interp = nil oldmanifest = Puppet[:manifest] Puppet[:manifest] = manifest - trans = nil - assert_nothing_raised { - trans = Puppet::Parser::Compiler.new(mknode).compile - } - - config = nil - assert_nothing_raised { - config = trans.extract.to_catalog - } - - config.apply + catalog = Puppet::Parser::Compiler.new(mknode).compile.to_ral + catalog.apply files.each do |file| assert(FileTest.exists?(file), "Did not create %s" % file) diff --git a/test/ral/type/file/target.rb b/test/ral/type/file/target.rb index f2a7de6f4..89792c4b6 100755 --- a/test/ral/type/file/target.rb +++ b/test/ral/type/file/target.rb @@ -69,7 +69,6 @@ class TestFileTarget < Test::Unit::TestCase assert_equal(file, File.readlink(linkpath)) # Use classes for comparison, because the resource inspection is so large - assert_equal(NilClass, catalog.resource(:file, sublink).class, "dynamically generated resources were not removed") assert_events([], link, "Link is not in sync") end