...

Also, there's now a GitHub repository for the code to evolve: https://github.com/rvs/bigtop-puppet

jcbollinger says:

The initial draft of the zookeeper stuff looks pretty clean and reasonable. I have attached a handful of detail-level critiques to the GH commit. At a higher level, I am a bit surprised by the amount of data in your classes, given some of our earlier discussions about separating data from puppet code. For example, I don't see anything in class bigtop::params that could not easily be externalized. It may be too soon to make decisions about what should and shouldn't be externalized, and about how the external data should be organized, but that's something to keep an eye on.

A detailed use case scenario

...

   # all the data required for the class gets
   # to us via external lookups
   include bigtop::hdfs::secondarynamenode

jcbollinger says:

If you are going to rely on hiera or another external source for all class data, then you absolutely should use the 'include' form, not the parametrized form. The latter carries the constraint that it can be used only once for any given class, and that use must be the first one parsed. There are very good reasons, however, why sometimes you would like to declare a given class in more than one place. You can work around any need to do so with enough effort and code, but that generally makes your manifest set more brittle, and / or puts substantially greater demands on your ENC. I see from your subsequent comment (elided) that you recognize that, at least to some degree.

...

class foo(
  $param1 = bigtop_lookup('foo::param1','default-val'),
...

jcbollinger says:

A level of indirection would be useful if you want to minimize manifest modifications in the event that you switch data binding mechanisms. Is that a likely eventuality? Hiera can be used with Puppet as old as v2.6, and it doesn't look likely to be pulled from any foreseeable future Puppet. On the cost side, an indirection layer either provides a least-common-denominator feature set (notably, hiera's hiera_array(), hiera_hash(), and hiera_include() functions are not covered by the example), or else ties the indirection layer tightly enough to a particular data provider that switching providers is difficult or even impossible in practice. (extlookup(), for example, has no associated analog of the additional hiera functions I just mentioned, and can supply only string values). Much depends on the nature of the data binding features you want/need to use. As a special case, if you end up relying on Puppet 3's automatic class parameter binding, then you might as well embrace Hiera all the way.

...

It seems like class parameters is an obvious choice here

jcbollinger says:

You are supposing that these will be modeled as class parameters in the first place. Certainly they are data that characterize the configuration to be deployed, and it is possible to model them as class parameters, but that is not the only – and maybe not the best – alternative available to you. Class parametrization is a protocol for declaring and documenting that data on which a class relies, and it enables mechanisms for obtaining that data that non-parametrized classes cannot use, but the same configuration objectives can be accomplished without them.

...

Seems like class parametrization should be a prime method, but allow some exceptions. One exception may be to handle what I would call "fine tuning parameters" to affect the configuration variables that may be in, for example, the hdfs-site, mapred-site . config files. If relied solely on class paramterization issue would be you may have classes with 30 - 50 parameters which is very cumbersome. Instead you might treat these with "global variables". Also related is keeping in mind easing the process of iterating on modules and adding new parameters; some of the signatures seem very set while other will evolve more rapidly.

jcbollinger says:

The implications of class parameterization should be understood before committing to that approach, and those implications depend in part on which Puppet versions will be supported. I have historically been very negative about using parameterized classes with Puppet 2.x, but I am fine with parameterized classes themselves in Puppet 3, as long as hiera or maybe an ENC is used for data binding. Parameterized-style class declarations (i.e. {{ class { 'foo': param1 => 'bar' } } }) in Puppet 3 DSL still have all the problems that I took issue with in Puppet 2. This is a pernicious design issue, because although parameterized-style class declarations seem appealing and natural on the surface, they are in fact fundamentally inconsistent with Puppet's overall design. The inconsistencies play out in a variety of troublesome ways. Nevertheless, people certainly do use parameterized classes successfully, including in Puppet 2. If that is the direction chosen, however, then you will want to adopt appropriate coding practices to avoid the associated issues.

...

class common_code($var1, ... $varN) {
}
....
   class { "common_code":
       var1 => $var1,
       ....
       varN => $var1
   }

jcbollinger says:

With newer Puppet, the key is that each class needs to know from where, proximally, the data it consumes come (and coding to that principle is good practice in every version of Puppet). Class parameters are probably the most obvious proximal source on which a class can rely for data (in 2.6+), but classes can also rely on class variables of other classes by referring to them by fully-qualified name, and it is discussed above how classes can obtain shared external data via an appropriate function, such as hiera() or extlookup(). To the extent that the current codebase appears generally to declare each class in just one place, the most direct accommodation for Puppet3 (and good practices generally) would probably be to just qualify all the variable references. Indeed, that was typical advice to anyone preparing to move from Puppet 2.x to Puppet 3. That is not meant to discount the opportunity to perform a deeper redesign, however.

...