On Jan 24, 2011, at 4:46 PM, Matt Robinson wrote:

> From: Dan Bode <[email protected]>
> 
> It facilitates the support for param classes from the ENC. It adds
> support for classes to be passed as a hash to the evaluate_classes
> method. If a hash of classes is specified, it also evaluates duplicates.
> I also had to convert the hash to an array for tags to be applied
> correctly.
> 
> Reviewed-by: Matt Robinson
> 
> Signed-off-by: Matt Robinson <[email protected]>
> ---
> lib/puppet/parser/compiler.rb     |   21 ++++++++++++--
> spec/unit/parser/compiler_spec.rb |   55 +++++++++++++++++++++++++++++++++++++
> 2 files changed, 73 insertions(+), 3 deletions(-)
> 
> diff --git a/lib/puppet/parser/compiler.rb b/lib/puppet/parser/compiler.rb
> index c60e1d4..3134e03 100644
> --- a/lib/puppet/parser/compiler.rb
> +++ b/lib/puppet/parser/compiler.rb
> @@ -139,12 +139,23 @@ class Puppet::Parser::Compiler
>   def evaluate_classes(classes, scope, lazy_evaluate = true)
>     raise Puppet::DevError, "No source for scope passed to evaluate_classes" 
> unless scope.source
>     found = []
> +    param_classes = nil
> +    # if we are a param class, save the classes hash
> +    # and transform classes to be the keys
> +    if classes.class == Hash
> +      param_classes = classes
> +      classes = classes.keys
> +    end
>     classes.each do |name|
>       # If we can find the class, then make a resource that will evaluate it.
>       if klass = scope.find_hostclass(name)
> -        found << name and next if scope.class_scope(klass)
> 
> -        resource = klass.ensure_in_catalog(scope)
> +        if param_classes
> +          resource = klass.ensure_in_catalog(scope, param_classes[name] || 
> {})
> +        else
> +          found << name and next if scope.class_scope(klass) and 
> !param_classes
> +          resource = klass.ensure_in_catalog(scope)
> +        end
> 
>         # If they've disabled lazy evaluation (which the :include function 
> does),
>         # then evaluate our resource immediately.
> @@ -432,7 +443,11 @@ class Puppet::Parser::Compiler
>     @resources = []
> 
>     # Make sure any external node classes are in our class list
> -    @catalog.add_class(*@node.classes)
> +    if @node.classes.class == Hash
> +      @catalog.add_class(*@node.classes.keys)
> +    else
> +      @catalog.add_class(*@node.classes)
> +    end
>   end

It'd sure be nice to be able to make node.classes always be a hash.  I doubt we 
can do it without worrying about backward compatibility with existing ENC 
plugins, but we should default to using hashes everywhere and, if possible, 
just have one place where we convert everything, rather than this form where 
everyone who ever looks at the 'classes' attribute needs to test whether it's 
an array or hash.

>   # Set the node's parameters into the top-scope as variables.
> diff --git a/spec/unit/parser/compiler_spec.rb 
> b/spec/unit/parser/compiler_spec.rb
> index 95f3853..c1e7c48 100755
> --- a/spec/unit/parser/compiler_spec.rb
> +++ b/spec/unit/parser/compiler_spec.rb
> @@ -109,6 +109,15 @@ describe Puppet::Parser::Compiler do
>       compiler.classlist.should include("bar")
>     end
> 
> +    it "should transform node class hashes into a class list" do
> +      node = Puppet::Node.new("mynode")
> +      node.classes = {'foo'=>{'one'=>'1'}, 'bar'=>{'two'=>'2'}}
> +      compiler = Puppet::Parser::Compiler.new(node)
> +
> +      compiler.classlist.should include("foo")
> +      compiler.classlist.should include("bar")
> +    end
> +
>     it "should add a 'main' stage to the catalog" do
>       @compiler.catalog.resource(:stage, :main).should 
> be_instance_of(Puppet::Parser::Resource)
>     end
> @@ -185,6 +194,14 @@ describe Puppet::Parser::Compiler do
>       @compiler.class.publicize_methods(:evaluate_node_classes) { 
> @compiler.evaluate_node_classes }
>     end
> 
> +    it "should evaluate any parameterized classes named in the node" do
> +      classes = {'foo'=>{'1'=>'one'}, 'bar'=>{'2'=>'two'}}
> +      @node.stubs(:classes).returns(classes)
> +      @compiler.expects(:evaluate_classes).with(classes, @compiler.topscope)
> +      @compiler.compile
> +    end
> +
> +
>     it "should evaluate the main class if it exists" do
>       compile_stub(:evaluate_main)
>       main_class = @known_resource_types.add 
> Puppet::Resource::Type.new(:hostclass, "")
> @@ -566,6 +583,14 @@ describe Puppet::Parser::Compiler do
>       @scope.expects(:find_hostclass).with("notfound").returns(nil)
>       @compiler.evaluate_classes(%w{notfound}, @scope)
>     end
> +    # I wish it would fail
> +    it "should log when it can't find class" do
> +      klasses = {'foo'=>nil}
> +      @node.classes = klasses
> +      @compiler.topscope.stubs(:find_hostclass).with('foo').returns(nil)
> +      Puppet.expects(:info).with('Could not find class foo for testnode')
> +      @compiler.compile
> +    end
>   end
> 
>   describe "when evaluating found classes" do
> @@ -586,6 +611,28 @@ describe Puppet::Parser::Compiler do
>       @compiler.evaluate_classes(%w{myclass}, @scope)
>     end
> 
> +    # test that ensure_in_catalog is called for existing classes
> +    it "should ensure each node class hash is in catalog" do
> +      klasses = {'foo'=>{'1'=>'one'}, 'bar'=>{'2'=>'two'}}
> +      @node.classes = klasses
> +      klasses.each do |name, params|
> +        klass = Puppet::Resource::Type.new(:hostclass, name)
> +        @compiler.topscope.stubs(:find_hostclass).with(name).returns(klass)
> +        klass.expects(:ensure_in_catalog).with(@compiler.topscope, params)
> +      end
> +      @compiler.compile
> +    end
> +
> +    it "should ensure class is in catalog without params" do
> +      @node.classes = klasses = {'foo'=>nil}
> +      foo = Puppet::Resource::Type.new(:hostclass, 'foo')
> +      @compiler.topscope.stubs(:find_hostclass).with('foo').returns(foo)
> +      foo.expects(:ensure_in_catalog).with(@compiler.topscope, {})
> +      @compiler.compile
> +    end
> +
> +
> +
>     it "should not evaluate the resources created for found classes unless 
> asked" do
>       @compiler.catalog.stubs(:tag)
> 
> @@ -620,6 +667,14 @@ describe Puppet::Parser::Compiler do
>       @compiler.evaluate_classes(%w{myclass}, @scope, false)
>     end
> 
> +    it "should not skip param classes that have already been evaluated" do
> +      @scope.stubs(:class_scope).with(@class).returns("something")
> +      @node.classes = klasses = {'foo'=>nil}
> +      foo = Puppet::Resource::Type.new(:hostclass, 'foo')
> +      @compiler.topscope.stubs(:find_hostclass).with('foo').returns(foo)
> +      foo.expects(:ensure_in_catalog).with(@compiler.topscope, {})
> +      @compiler.compile
> +    end
>     it "should skip classes previously evaluated with different 
> capitalization" do
>       @compiler.catalog.stubs(:tag)
>       @scope.stubs(:find_hostclass).with("MyClass").returns(@class)
> -- 
> 1.7.3.1
> 
> -- 
> 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.
> 


-- 
Those who speak most of progress measure it by quantity and not
by quality.     --George Santayana
---------------------------------------------------------------------
Luke Kanies  -|-   http://puppetlabs.com   -|-   +1(615)594-8199




-- 
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.

Reply via email to