[Pkg-puppet-devel] [SCM] Puppet packaging for Debian branch, master, updated. debian/2.6.1-1-69-gb75a307

Markus Roberts Markus at reality.com
Tue Nov 9 22:10:56 UTC 2010


The following commit has been merged in the master branch:
commit 65ef24e5c1c33b7d42012891d368917fd6aaf68c
Author: Paul Berry <paul at puppetlabs.com>
Date:   Fri Oct 8 15:26:28 2010 -0700

    (#4534/#4778) -- Normalize parameterized classes
    
    This is a reconciliation/melding of Paul's
        (#4534) Class inheritance with parameterized classes is no longer ignored
    and Markus's
        Fix for #4778 -- evaluate parameterized classes when they are instantiated
    
    Extracted the code from Resource::Type#mk_plain_resource that evaluates
    parents and tags the catalog, and moved that into a new method called
    instantiate_resource.  Instantiate_resource is now also called from
    Parser::Ast::Resource#evaluate, so that the notation
    
        "class { classname: }"
    
    now executes this code too.  Likewise adds class evaluation so that it behaves
    the same (with regard to lazy / strict evaluation) as
    
        include classname

diff --git a/lib/puppet/parser/ast/resource.rb b/lib/puppet/parser/ast/resource.rb
index 6909c85..b019e6a 100644
--- a/lib/puppet/parser/ast/resource.rb
+++ b/lib/puppet/parser/ast/resource.rb
@@ -49,10 +49,11 @@ class Resource < AST::ResourceReference
           :strict => true
         )
 
-        # And then store the resource in the compiler.
-        # At some point, we need to switch all of this to return
-        # resources instead of storing them like this.
+        if resource.resource_type.is_a? Puppet::Resource::Type
+          resource.resource_type.instantiate_resource(scope, resource)
+        end
         scope.compiler.add_resource(scope, resource)
+        scope.compiler.evaluate_classes([resource_title],scope,false) if fully_qualified_type == 'class'
         resource
       end
     }.reject { |resource| resource.nil? }
diff --git a/lib/puppet/parser/compiler.rb b/lib/puppet/parser/compiler.rb
index e1227e7..c60e1d4 100644
--- a/lib/puppet/parser/compiler.rb
+++ b/lib/puppet/parser/compiler.rb
@@ -144,7 +144,7 @@ class Puppet::Parser::Compiler
       if klass = scope.find_hostclass(name)
         found << name and next if scope.class_scope(klass)
 
-        resource = klass.mk_plain_resource(scope)
+        resource = klass.ensure_in_catalog(scope)
 
         # If they've disabled lazy evaluation (which the :include function does),
         # then evaluate our resource immediately.
@@ -220,7 +220,7 @@ class Puppet::Parser::Compiler
 
     # Create a resource to model this node, and then add it to the list
     # of resources.
-    resource = astnode.mk_plain_resource(topscope)
+    resource = astnode.ensure_in_catalog(topscope)
 
     resource.evaluate
 
diff --git a/lib/puppet/resource/type.rb b/lib/puppet/resource/type.rb
index 7b21e55..d40adc1 100644
--- a/lib/puppet/resource/type.rb
+++ b/lib/puppet/resource/type.rb
@@ -138,21 +138,15 @@ class Puppet::Resource::Type
     end
   end
 
-  # Make an instance of our resource type.  This is only possible
-  # for those classes and nodes that don't have any arguments, and is
-  # only useful for things like the 'include' function.
-  def mk_plain_resource(scope)
+  # Make an instance of the resource type, and place it in the catalog
+  # if it isn't in the catalog already.  This is only possible for
+  # classes and nodes.  No parameters are be supplied--if this is a
+  # parameterized class, then all parameters take on their default
+  # values.
+  def ensure_in_catalog(scope)
     type == :definition and raise ArgumentError, "Cannot create resources for defined resource types"
     resource_type = type == :hostclass ? :class : :node
 
-    # Make sure our parent class has been evaluated, if we have one.
-    if parent
-      parent_resource = scope.catalog.resource(resource_type, parent)
-      unless parent_resource
-        parent_type(scope).mk_plain_resource(scope)
-      end
-    end
-
     # Do nothing if the resource already exists; this makes sure we don't
     # get multiple copies of the class resource, which helps provide the
     # singleton nature of classes.
@@ -161,11 +155,22 @@ class Puppet::Resource::Type
     end
 
     resource = Puppet::Parser::Resource.new(resource_type, name, :scope => scope, :source => self)
+    instantiate_resource(scope, resource)
     scope.compiler.add_resource(scope, resource)
-    scope.catalog.tag(*resource.tags)
     resource
   end
 
+  def instantiate_resource(scope, resource)
+    # Make sure our parent class has been evaluated, if we have one.
+    if parent && !scope.catalog.resource(resource.type, parent)
+      parent_type(scope).ensure_in_catalog(scope)
+    end
+
+    if ['Class', 'Node'].include? resource.type
+      scope.catalog.tag(*resource.tags)
+    end
+  end
+
   def name
     return @name unless @name.is_a?(Regexp)
     @name.source.downcase.gsub(/[^-\w:.]/,'').sub(/^\.+/,'')
diff --git a/spec/unit/parser/ast/resource_spec.rb b/spec/unit/parser/ast/resource_spec.rb
index 5c94ac0..e1f73e1 100755
--- a/spec/unit/parser/ast/resource_spec.rb
+++ b/spec/unit/parser/ast/resource_spec.rb
@@ -5,123 +5,168 @@ require File.dirname(__FILE__) + '/../../../spec_helper'
 describe Puppet::Parser::AST::Resource do
   ast = Puppet::Parser::AST
 
-  before :each do
-    @title = Puppet::Parser::AST::String.new(:value => "mytitle")
-    @compiler = Puppet::Parser::Compiler.new(Puppet::Node.new("mynode"))
-    @scope = Puppet::Parser::Scope.new(:compiler => @compiler)
-    @scope.stubs(:resource).returns(stub_everything)
-    @resource = ast::Resource.new(:title => @title, :type => "file", :parameters => ast::ASTArray.new(:children => []) )
-    @resource.stubs(:qualified_type).returns("Resource")
-  end
-
-  it "should evaluate all its parameters" do
-    param = stub 'param'
-    param.expects(:safeevaluate).with(@scope).returns Puppet::Parser::Resource::Param.new(:name => "myparam", :value => "myvalue", :source => stub("source"))
-    @resource.stubs(:parameters).returns [param]
-
-    @resource.evaluate(@scope)
-  end
-
-  it "should evaluate its title" do
-    @resource.evaluate(@scope)[0].title.should == "mytitle"
-  end
-
-  it "should flatten the titles array" do
-    titles = []
-    %w{one two}.each do |title|
-      titles << Puppet::Parser::AST::String.new(:value => title)
+  describe "for builtin types" do
+    before :each do
+      @title = Puppet::Parser::AST::String.new(:value => "mytitle")
+      @compiler = Puppet::Parser::Compiler.new(Puppet::Node.new("mynode"))
+      @scope = Puppet::Parser::Scope.new(:compiler => @compiler)
+      @scope.stubs(:resource).returns(stub_everything)
+      @resource = ast::Resource.new(:title => @title, :type => "file", :parameters => ast::ASTArray.new(:children => []) )
+      @resource.stubs(:qualified_type).returns("Resource")
     end
 
-    array = Puppet::Parser::AST::ASTArray.new(:children => titles)
+    it "should evaluate all its parameters" do
+      param = stub 'param'
+      param.expects(:safeevaluate).with(@scope).returns Puppet::Parser::Resource::Param.new(:name => "myparam", :value => "myvalue", :source => stub("source"))
+      @resource.stubs(:parameters).returns [param]
 
-    @resource.title = array
-    result = @resource.evaluate(@scope).collect { |r| r.title }
-    result.should be_include("one")
-    result.should be_include("two")
-  end
+      @resource.evaluate(@scope)
+    end
 
-  it "should create and return one resource objects per title" do
-    titles = []
-    %w{one two}.each do |title|
-      titles << Puppet::Parser::AST::String.new(:value => title)
+    it "should evaluate its title" do
+      @resource.evaluate(@scope)[0].title.should == "mytitle"
     end
 
-    array = Puppet::Parser::AST::ASTArray.new(:children => titles)
+    it "should flatten the titles array" do
+      titles = []
+      %w{one two}.each do |title|
+        titles << Puppet::Parser::AST::String.new(:value => title)
+      end
 
-    @resource.title = array
-    result = @resource.evaluate(@scope).collect { |r| r.title }
-    result.should be_include("one")
-    result.should be_include("two")
-  end
+      array = Puppet::Parser::AST::ASTArray.new(:children => titles)
 
-  it "should handover resources to the compiler" do
-    titles = []
-    %w{one two}.each do |title|
-      titles << Puppet::Parser::AST::String.new(:value => title)
+      @resource.title = array
+      result = @resource.evaluate(@scope).collect { |r| r.title }
+      result.should be_include("one")
+      result.should be_include("two")
     end
 
-    array = Puppet::Parser::AST::ASTArray.new(:children => titles)
+    it "should create and return one resource objects per title" do
+      titles = []
+      %w{one two}.each do |title|
+        titles << Puppet::Parser::AST::String.new(:value => title)
+      end
 
-    @resource.title = array
-    result = @resource.evaluate(@scope)
+      array = Puppet::Parser::AST::ASTArray.new(:children => titles)
 
-    result.each do |res|
-      @compiler.catalog.resource(res.ref).should be_instance_of(Puppet::Parser::Resource)
+      @resource.title = array
+      result = @resource.evaluate(@scope).collect { |r| r.title }
+      result.should be_include("one")
+      result.should be_include("two")
     end
-  end
-  it "should generate virtual resources if it is virtual" do
-    @resource.virtual = true
 
-    result = @resource.evaluate(@scope)
-    result[0].should be_virtual
-  end
+    it "should handover resources to the compiler" do
+      titles = []
+      %w{one two}.each do |title|
+        titles << Puppet::Parser::AST::String.new(:value => title)
+      end
 
-  it "should generate virtual and exported resources if it is exported" do
-    @resource.exported = true
+      array = Puppet::Parser::AST::ASTArray.new(:children => titles)
 
-    result = @resource.evaluate(@scope)
-    result[0].should be_virtual
-    result[0].should be_exported
-  end
+      @resource.title = array
+      result = @resource.evaluate(@scope)
 
-  # Related to #806, make sure resources always look up the full path to the resource.
-  describe "when generating qualified resources" do
-    before do
-      @scope = Puppet::Parser::Scope.new :compiler => Puppet::Parser::Compiler.new(Puppet::Node.new("mynode"))
-      @parser = Puppet::Parser::Parser.new(Puppet::Node::Environment.new)
-      @parser.newdefine "one"
-      @parser.newdefine "one::two"
-      @parser.newdefine "three"
-      @twoscope = @scope.newscope(:namespace => "one")
-      @twoscope.resource = @scope.resource
+      result.each do |res|
+        @compiler.catalog.resource(res.ref).should be_instance_of(Puppet::Parser::Resource)
+      end
     end
 
-    def resource(type, params = nil)
-      params ||= Puppet::Parser::AST::ASTArray.new(:children => [])
-      Puppet::Parser::AST::Resource.new(:type => type, :title => Puppet::Parser::AST::String.new(:value => "myresource"), :parameters => params)
+    it "should generate virtual resources if it is virtual" do
+      @resource.virtual = true
+
+      result = @resource.evaluate(@scope)
+      result[0].should be_virtual
     end
 
-    it "should be able to generate resources with fully qualified type information" do
-      resource("two").evaluate(@twoscope)[0].type.should == "One::Two"
+    it "should generate virtual and exported resources if it is exported" do
+      @resource.exported = true
+
+      result = @resource.evaluate(@scope)
+      result[0].should be_virtual
+      result[0].should be_exported
     end
 
-    it "should be able to generate resources with unqualified type information" do
-      resource("one").evaluate(@twoscope)[0].type.should == "One"
+    # Related to #806, make sure resources always look up the full path to the resource.
+    describe "when generating qualified resources" do
+      before do
+        @scope = Puppet::Parser::Scope.new :compiler => Puppet::Parser::Compiler.new(Puppet::Node.new("mynode"))
+        @parser = Puppet::Parser::Parser.new(Puppet::Node::Environment.new)
+        @parser.newdefine "one"
+        @parser.newdefine "one::two"
+        @parser.newdefine "three"
+        @twoscope = @scope.newscope(:namespace => "one")
+        @twoscope.resource = @scope.resource
+      end
+
+      def resource(type, params = nil)
+        params ||= Puppet::Parser::AST::ASTArray.new(:children => [])
+        Puppet::Parser::AST::Resource.new(:type => type, :title => Puppet::Parser::AST::String.new(:value => "myresource"), :parameters => params)
+      end
+
+      it "should be able to generate resources with fully qualified type information" do
+        resource("two").evaluate(@twoscope)[0].type.should == "One::Two"
+      end
+
+      it "should be able to generate resources with unqualified type information" do
+        resource("one").evaluate(@twoscope)[0].type.should == "One"
+      end
+
+      it "should correctly generate resources that can look up builtin types" do
+        resource("file").evaluate(@twoscope)[0].type.should == "File"
+      end
+
+      it "should correctly generate resources that can look up defined classes by title" do
+        @scope.known_resource_types.add_hostclass Puppet::Resource::Type.new(:hostclass, "Myresource", {})
+        @scope.compiler.stubs(:evaluate_classes)
+        res = resource("class").evaluate(@twoscope)[0]
+        res.type.should == "Class"
+        res.title.should == "Myresource"
+      end
+
+      it "should evaluate parameterized classes when they are instantiated" do
+        @scope.known_resource_types.add_hostclass Puppet::Resource::Type.new(:hostclass, "Myresource", {})
+        @scope.compiler.expects(:evaluate_classes).with(['myresource'], at twoscope,false)
+        resource("class").evaluate(@twoscope)[0]
+      end
+
+      it "should fail for resource types that do not exist" do
+        lambda { resource("nosuchtype").evaluate(@twoscope) }.should raise_error(Puppet::ParseError)
+      end
     end
+  end
 
-    it "should correctly generate resources that can look up builtin types" do
-      resource("file").evaluate(@twoscope)[0].type.should == "File"
+  describe "for class resources" do
+    before do
+      @title = Puppet::Parser::AST::String.new(:value => "classname")
+      @compiler = Puppet::Parser::Compiler.new(Puppet::Node.new("mynode"))
+      @scope = Puppet::Parser::Scope.new(:compiler => @compiler)
+      @scope.stubs(:resource).returns(stub_everything)
+      @resource = ast::Resource.new(:title => @title, :type => "Class", :parameters => ast::ASTArray.new(:children => []) )
+      @resource.stubs(:qualified_type).returns("Resource")
+      @type = Puppet::Resource::Type.new(:hostclass, "classname")
+      @compiler.known_resource_types.add(@type)
     end
 
-    it "should correctly generate resources that can look up defined classes by title" do
-      @scope.known_resource_types.add_hostclass Puppet::Resource::Type.new(:hostclass, "Myresource", {})
-      res = resource("class").evaluate(@twoscope)[0]
-      res.type.should == "Class"
-      res.title.should == "Myresource"
+    it "should instantiate the class" do
+      @compiler.stubs(:evaluate_classes)
+      result = @resource.evaluate(@scope)
+      result.length.should == 1
+      result.first.ref.should == "Class[Classname]"
+      @compiler.catalog.resource("Class[Classname]").should equal result.first
     end
 
-    it "should fail for resource types that do not exist" do
-      lambda { resource("nosuchtype").evaluate(@twoscope) }.should raise_error(Puppet::ParseError)
+    it "should cause its parent to be evaluated" do
+      parent_type = Puppet::Resource::Type.new(:hostclass, "parentname")
+      @compiler.stubs(:evaluate_classes)
+      @compiler.known_resource_types.add(parent_type)
+      @type.parent = "parentname"
+      result = @resource.evaluate(@scope)
+      result.length.should == 1
+      result.first.ref.should == "Class[Classname]"
+      @compiler.catalog.resource("Class[Classname]").should equal result.first
+      @compiler.catalog.resource("Class[Parentname]").should be_instance_of(Puppet::Parser::Resource)
     end
+
   end
+
 end
diff --git a/spec/unit/parser/compiler_spec.rb b/spec/unit/parser/compiler_spec.rb
index 22d52f2..95f3853 100755
--- a/spec/unit/parser/compiler_spec.rb
+++ b/spec/unit/parser/compiler_spec.rb
@@ -580,7 +580,7 @@ describe Puppet::Parser::Compiler do
     it "should evaluate each class" do
       @compiler.catalog.stubs(:tag)
 
-      @class.expects(:mk_plain_resource).with(@scope)
+      @class.expects(:ensure_in_catalog).with(@scope)
       @scope.stubs(:class_scope).with(@class)
 
       @compiler.evaluate_classes(%w{myclass}, @scope)
@@ -591,7 +591,7 @@ describe Puppet::Parser::Compiler do
 
       @resource.expects(:evaluate).never
 
-      @class.expects(:mk_plain_resource).returns(@resource)
+      @class.expects(:ensure_in_catalog).returns(@resource)
       @scope.stubs(:class_scope).with(@class)
 
       @compiler.evaluate_classes(%w{myclass}, @scope)
@@ -601,7 +601,7 @@ describe Puppet::Parser::Compiler do
       @compiler.catalog.stubs(:tag)
 
       @resource.expects(:evaluate)
-      @class.expects(:mk_plain_resource).returns(@resource)
+      @class.expects(:ensure_in_catalog).returns(@resource)
       @scope.stubs(:class_scope).with(@class)
 
       @compiler.evaluate_classes(%w{myclass}, @scope, false)
@@ -638,7 +638,7 @@ describe Puppet::Parser::Compiler do
       @scope.stubs(:class_scope).with(@class)
 
       Puppet::Parser::Resource.stubs(:new).returns(@resource)
-      @class.stubs :mk_plain_resource
+      @class.stubs :ensure_in_catalog
       @compiler.evaluate_classes(%w{myclass notfound}, @scope).should == %w{myclass}
     end
   end
@@ -678,7 +678,7 @@ describe Puppet::Parser::Compiler do
       @compiler.known_resource_types.stubs(:node).with("c").returns(node_class)
 
       node_resource = stub 'node resource', :ref => "Node[c]", :evaluate => nil, :type => "node"
-      node_class.expects(:mk_plain_resource).returns(node_resource)
+      node_class.expects(:ensure_in_catalog).returns(node_resource)
 
       @compiler.compile
     end
@@ -688,7 +688,7 @@ describe Puppet::Parser::Compiler do
       @compiler.known_resource_types.stubs(:node).with("default").returns(node_class)
 
       node_resource = stub 'node resource', :ref => "Node[default]", :evaluate => nil, :type => "node"
-      node_class.expects(:mk_plain_resource).returns(node_resource)
+      node_class.expects(:ensure_in_catalog).returns(node_resource)
 
       @compiler.compile
     end
@@ -698,7 +698,7 @@ describe Puppet::Parser::Compiler do
       @compiler.known_resource_types.stubs(:node).with("c").returns(node_class)
 
       node_resource = stub 'node resource', :ref => "Node[c]", :type => "node"
-      node_class.expects(:mk_plain_resource).returns(node_resource)
+      node_class.expects(:ensure_in_catalog).returns(node_resource)
 
       node_resource.expects(:evaluate)
 
@@ -707,7 +707,7 @@ describe Puppet::Parser::Compiler do
 
     it "should set the node's scope as the top scope" do
       node_resource = stub 'node resource', :ref => "Node[c]", :evaluate => nil, :type => "node"
-      node_class = stub 'node', :name => "c", :mk_plain_resource => node_resource
+      node_class = stub 'node', :name => "c", :ensure_in_catalog => node_resource
 
       @compiler.known_resource_types.stubs(:node).with("c").returns(node_class)
 
diff --git a/spec/unit/resource/type_spec.rb b/spec/unit/resource/type_spec.rb
index f58092e..7b240bb 100755
--- a/spec/unit/resource/type_spec.rb
+++ b/spec/unit/resource/type_spec.rb
@@ -580,29 +580,29 @@ describe Puppet::Resource::Type do
     end
 
     it "should create a resource instance" do
-      @top.mk_plain_resource(@scope).should be_instance_of(Puppet::Parser::Resource)
+      @top.ensure_in_catalog(@scope).should be_instance_of(Puppet::Parser::Resource)
     end
 
     it "should set its resource type to 'class' when it is a hostclass" do
-      Puppet::Resource::Type.new(:hostclass, "top").mk_plain_resource(@scope).type.should == "Class"
+      Puppet::Resource::Type.new(:hostclass, "top").ensure_in_catalog(@scope).type.should == "Class"
     end
 
     it "should set its resource type to 'node' when it is a node" do
-      Puppet::Resource::Type.new(:node, "top").mk_plain_resource(@scope).type.should == "Node"
+      Puppet::Resource::Type.new(:node, "top").ensure_in_catalog(@scope).type.should == "Node"
     end
 
     it "should fail when it is a definition" do
-      lambda { Puppet::Resource::Type.new(:definition, "top").mk_plain_resource(@scope) }.should raise_error(ArgumentError)
+      lambda { Puppet::Resource::Type.new(:definition, "top").ensure_in_catalog(@scope) }.should raise_error(ArgumentError)
     end
 
     it "should add the created resource to the scope's catalog" do
-      @top.mk_plain_resource(@scope)
+      @top.ensure_in_catalog(@scope)
 
       @compiler.catalog.resource(:class, "top").should be_instance_of(Puppet::Parser::Resource)
     end
 
     it "should evaluate the parent class if one exists" do
-      @middle.mk_plain_resource(@scope)
+      @middle.ensure_in_catalog(@scope)
 
       @compiler.catalog.resource(:class, "top").should be_instance_of(Puppet::Parser::Resource)
     end
@@ -610,40 +610,40 @@ describe Puppet::Resource::Type do
     it "should fail to evaluate if a parent class is defined but cannot be found" do
       othertop = Puppet::Resource::Type.new :hostclass, "something", :parent => "yay"
       @code.add othertop
-      lambda { othertop.mk_plain_resource(@scope) }.should raise_error(Puppet::ParseError)
+      lambda { othertop.ensure_in_catalog(@scope) }.should raise_error(Puppet::ParseError)
     end
 
     it "should not create a new resource if one already exists" do
       @compiler.catalog.expects(:resource).with(:class, "top").returns("something")
       @compiler.catalog.expects(:add_resource).never
-      @top.mk_plain_resource(@scope)
+      @top.ensure_in_catalog(@scope)
     end
 
     it "should return the existing resource when not creating a new one" do
       @compiler.catalog.expects(:resource).with(:class, "top").returns("something")
       @compiler.catalog.expects(:add_resource).never
-      @top.mk_plain_resource(@scope).should == "something"
+      @top.ensure_in_catalog(@scope).should == "something"
     end
 
     it "should not create a new parent resource if one already exists and it has a parent class" do
-      @top.mk_plain_resource(@scope)
+      @top.ensure_in_catalog(@scope)
 
       top_resource = @compiler.catalog.resource(:class, "top")
 
-      @middle.mk_plain_resource(@scope)
+      @middle.ensure_in_catalog(@scope)
 
       @compiler.catalog.resource(:class, "top").should equal(top_resource)
     end
 
     # #795 - tag before evaluation.
     it "should tag the catalog with the resource tags when it is evaluated" do
-      @middle.mk_plain_resource(@scope)
+      @middle.ensure_in_catalog(@scope)
 
       @compiler.catalog.should be_tagged("middle")
     end
 
     it "should tag the catalog with the parent class tags when it is evaluated" do
-      @middle.mk_plain_resource(@scope)
+      @middle.ensure_in_catalog(@scope)
 
       @compiler.catalog.should be_tagged("top")
     end

-- 
Puppet packaging for Debian



More information about the Pkg-puppet-devel mailing list