On Tue, Jan 4, 2011 at 4:40 PM, Nick Lewis <[email protected]> wrote:
> This mostly looks good, but what happens if you specify parameters for a
> parameterized class both in your manifest and in the ENC script? From our
> testing, it looks like the ENC parameters are just ignored. This is in
> contrast to specifying ordinary variables/parameters, in which case there is
> an error about redefining the variable. What is the correct behavior in this
> case? For instance,
>
> site.pp:
> class foo($x) {
> notify { "x is $x": }
> }
> class { foo:
> x => "bar",
> }
>
> output from ENC script:
> ---
> name: localhost.localdomain
> parameters: {}
> classes:
> x: baz
>
> Currently, this prints "x is bar", but it seems to me like it should
> probably give a duplicate definition error.
>
I agree.
We should be aiming in general for declaration of classes in an ENC to
behave the same way as declaring them in manifests, so we shouldn't fail
silently for non-existent classes, and we should throw a duplicate
definition error here.
I believe any alternative where one takes precedence over the other will
provoke confusion.
>
> On Mon, Jan 3, 2011 at 2:17 PM, Dan Bode <[email protected]> wrote:
>
>> Sorry for the double post.
>>
>> Anyways, some missing notes from the patch submission.
>>
>> First of all, the unit tests are clearly lacking, I wanted to hurry up and
>> get the source code out for review, and will continue to work on more robust
>> unit tests.
>>
>> I ran the following manual tests using Puppet (I would like to get unit
>> test coverages for these tests, but it seems daunting):
>>
>> 1. should be able to pass valid attributes - passed
>> 2. fails if required attributes are not passed - passed
>> 3. fails is invalid attributes are passed - passed
>> 4. allows multiple classes to be specified - passed
>> 5. works with class namespaces - passed
>> 6. attributes are not required for classes - passed
>> 7. can set stages? I can set the main stage without error, but I cannot
>> create stages
>> 8. can set meta-param:
>> - I can set tags - passed
>> - could not set noop - did not work?
>> - can I set dependencies on other classes from ENC? - NO
>> - can I set dependencies on resources not in the ENC? - NO
>> 10. fails if out parent class is missing attributes - passes
>> 11. class with parent with parameters - success (order does not matter)
>> 12. set classes with top scope variables (using dynamic scoping) - success
>> 13. are tags applied correctly? - they seem to be
>> 14. order of specifying parents and children should not matter - passed
>>
>> other considerations for the future:
>> - maybe we should be able to specify stages
>> - should be able to use params from parameters:
>>
>> usage example:
>>
>> parameters:
>> fooarray: encfooarray
>> foo: top
>> classes:
>> foo:
>> bar: blaz
>> baz: 'dudE'
>> blah::foo:
>> blah:
>> blah::parent:
>> needed: foo
>>
>> -Dan
>>
>>
>> On Mon, Jan 3, 2011 at 1:39 PM, Dan Bode <[email protected]> wrote:
>>
>>> This experimental patch is intended to add support for param classes
>>> to the ENC.
>>>
>>> The patch does the following:
>>> 1. adds the ability to accept attributes to ensure_in_catalog
>>> 2. added hash support for a node's classes to accept a hash
>>>
>>> Signed-off-by: Dan Bode <[email protected]>
>>> ---
>>> lib/puppet/parser/compiler.rb | 17 ++++++++++++++++-
>>> lib/puppet/resource/type.rb | 5 ++++-
>>> spec/unit/parser/compiler_spec.rb | 22 ++++++++++++++++++++++
>>> 3 files changed, 42 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/lib/puppet/parser/compiler.rb
>>> b/lib/puppet/parser/compiler.rb
>>> index c60e1d4..e29bdd7 100644
>>> --- a/lib/puppet/parser/compiler.rb
>>> +++ b/lib/puppet/parser/compiler.rb
>>> @@ -129,6 +129,17 @@ class Puppet::Parser::Compiler
>>>
>>> # Evaluate all of the classes specified by the node.
>>> def evaluate_node_classes
>>> + if @node.classes.class == Hash
>>> + @node.classes.each do |name, params|
>>> + params ||= {}
>>> + # I have to find the hostclass
>>> + if klass = topscope.find_hostclass(name)
>>> + #resource = Puppet::Resource::Type.new(:hostclass, name)
>>> + klass.ensure_in_catalog(topscope, params)
>>> + end
>>> + end
>>> + @node.classes = @node.classes.keys
>>> + end
>>> evaluate_classes(@node.classes, topscope)
>>> end
>>>
>>> @@ -432,7 +443,11 @@ class Puppet::Parser::Compiler
>>> @resources = []
>>>
>>> # Make sure any external node classes are in our class list
>>> - @catalog.add_class(*[email protected])
>>> + if @node.classes.class == Hash
>>> + @catalog.add_class(*[email protected])
>>> + else
>>> + @catalog.add_class(*[email protected])
>>> + end
>>> end
>>>
>>> # Set the node's parameters into the top-scope as variables.
>>> diff --git a/lib/puppet/resource/type.rb b/lib/puppet/resource/type.rb
>>> index d40adc1..80d1133 100644
>>> --- a/lib/puppet/resource/type.rb
>>> +++ b/lib/puppet/resource/type.rb
>>> @@ -143,7 +143,7 @@ class Puppet::Resource::Type
>>> # 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)
>>> + def ensure_in_catalog(scope, attributes={})
>>> type == :definition and raise ArgumentError, "Cannot create resources
>>> for defined resource types"
>>> resource_type = type == :hostclass ? :class : :node
>>>
>>> @@ -155,6 +155,9 @@ class Puppet::Resource::Type
>>> end
>>>
>>> resource = Puppet::Parser::Resource.new(resource_type, name, :scope
>>> => scope, :source => self)
>>> + attributes.each do |k,v|
>>> + resource.set_parameter(k,v)
>>> + end
>>> instantiate_resource(scope, resource)
>>> scope.compiler.add_resource(scope, resource)
>>> resource
>>> diff --git a/spec/unit/parser/compiler_spec.rb
>>> b/spec/unit/parser/compiler_spec.rb
>>> index 95f3853..f569328 100755
>>> --- a/spec/unit/parser/compiler_spec.rb
>>> +++ b/spec/unit/parser/compiler_spec.rb
>>> @@ -109,6 +109,16 @@ 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'}}
>>> + 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 +195,18 @@ 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'=>{'one'=>'1'}, 'bar'=>{'two'=>'2'}}
>>> + main = stub 'main'
>>> + one = stub 'one', :name => "one"
>>> + three = stub 'three', :name => "three"
>>> + @node.stubs(:name).returns("whatever")
>>> + @node.stubs(:classes).returns(classes)
>>> +
>>> + @compiler.expects(:evaluate_classes).with(classes,
>>> @compiler.topscope)
>>> + @compiler.class.publicize_methods(:evaluate_node_classes) {
>>> @compiler.evaluate_node_classes }
>>> + 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, "")
>>> --
>>> 1.5.5.6
>>>
>>> --
>>> 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]<puppet-dev%[email protected]>
>>> .
>>> For more options, visit this group at
>>> http://groups.google.com/group/puppet-dev?hl=en.
>>>
>>>
>> --
>> 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]<puppet-dev%[email protected]>
>> .
>> For more options, visit this group at
>> http://groups.google.com/group/puppet-dev?hl=en.
>>
>
> --
> 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]<puppet-dev%[email protected]>
> .
> For more options, visit this group at
> http://groups.google.com/group/puppet-dev?hl=en.
>
--
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.
