On Tue, Jan 4, 2011 at 5:31 PM, Nigel Kersten <[email protected]> wrote:
> > > 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? > > Forgot to test > 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 err >> >> > I agree. > 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]<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.
