Hi John, Thanks so much for your feedback. It's extremely useful for me at this stage of my education in the Puppet DSL.
Here is the Puppet Users group thread where R.I. Pienaar said that he felt that using create_resources() was bad: https://groups.google.com/forum/?fromgroups#!searchin/puppet-users/create_resources$20pienaar/puppet-users/lxYDKf7dgc0/TppS_7BFB9IJ Responses to comments: 1. That makes perfect sense. I'll do that from now on. Thanks. 2. As far as I can see, I'm using class inheritance solely to make sure that class parameters are available to the main and sub classes. Maybe you are seeing that I'm using inheritance where it's not yet strictly needed? I'll have to look more closely at my code and fix any instances of inheritance where it's unnecessary. Thanks. 3. You are right. I'm using the wrong kind of 'undef' here. 4. Yep, I'll have to look at the parameters and clean this up as well. Thanks much! Alex On Tuesday, April 29, 2014 7:10:36 AM UTC-7, jcbollinger wrote: > > > > On Monday, April 28, 2014 2:34:43 PM UTC-5, Alex Scoble wrote: >> >> Hi All, >> >> I'm working on a module that builds KVM/libvirt hosts and populates them >> with predefined VMs. >> >> So far I have the module to where it can create any number of virtual >> nets, storage pools and volumes using virsh, but it isn't pretty. >> >> I've read on various threads here that create_resources is not a good >> function to use. This was stated quite emphatically by R.I. Pienaar and >> others. >> > > > If anyone has been vocal (here) about Puppet functions not to use then > it's me, and create_resources() is not one I rant about. As a general rule > I do prefer expressing configuration in DSL where that's feasible, and you > can, in fact, do the job of create_resources() almost entirely with > ordinary Puppet DSL (you need to add only stdlib's keys() function). > Nevertheless, I'm not persuaded that going to substantial effort and length > to avoid create_resources() is necessarily a win. > > On the other hand, I have come out rather strongly against the > ensure_resource() function. Were you perhaps thinking of that? > > > >> >> Already I've run into the situation where it's hard to control order in >> which two separate create_resources functions are run and I've seen some >> kludges to fix it, but I'm looking for a better way and hoping that it >> doesn't involve the use of custom types because that's currently more than >> I want to deal with. >> >> > > Indeed, one of the issues that does exist with create_resources() is that > you cannot express resource references in Hiera data (though I have seen > one recent report that claims otherwise). When part of the point of using > that function in the first place is that you don't want to have to know > exactly which resources you are actually declaring, that can present a > challenge. It is, nevertheless, a challenge that can be worked around. In > some cases, the "workaround" is a mechanism, such as collector chaining, > that you might reasonably have used anyway. > > > It would be super helpful if anyone could point me to a puppet module on > github that presents me with a better pattern to use with the hiera data. > > [...] > > > Any other help or criticisms are also welcome. > > > That looks pretty clean to me already. It does appear that maybe you want > to express relationships between your Kvm::Virtpool and Kvm::Virtvol > instances, but Garrett appears already to have gotten you pointed in a > useful direction with that (collector chaining). > > If you want responses to particular commentary about the > create_resources() function then it would be helpful to provide a pointer > to that commentary. R.I. is a pretty smart guy, and he has thought a lot > about how Puppet can and should use data, so I'm not remotely going to try > to respond to anything he may have said on the topic without actually > reading it for myself > > Additional, minor comments about your code: > > 1. For any but the most simple Execs, I recommend using descriptive > resource titles, and expressing the commands themselves via Exec's > 'command' property. It is much, much easier that way to understand their > purpose, especially in log output. > 2. There are only two reasons for using class inheritance: to make a > variable of the inherited class reliably available for use as a class > parameter default, and to enable overriding properties of a resource > declared by the inherited class. The latter is the original purpose, and > the former is merely a side effect, but the former is by far the more > common reason for class inheritance these days. In all other cases, an > ordinary class declaration (include 'foo') can and should be used instead, > if indeed anything of the sort is needed at all. Avoid class inheritance > where it is unneeded. Especially avoid inheriting from a parameterized > class. > 3. Your class kvm has the strung 'undef' as the defined default value > for a bunch of parameters. That's quite different from the undef keyword; > is it what you really mean? If it were then I would expect to see some > kind of test for those values, but I don't. > 4. More generally, don't define class and type parameter default > values unless you really mean it. If you forget to declare a needed > resource or class parameter then Puppet will diagnose the issue if and > only > if the parameter has no defined default value. Do not write definitions > or > classes such that failure to declare all the needed parameters will > silently yield an invalid configuration, as appears may be the case for > some of your defined parameters. > > > John > > > -- You received this message because you are subscribed to the Google Groups "Puppet Users" group. To unsubscribe from this group and stop receiving emails from it, send an email to puppet-users+unsubscr...@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/puppet-users/50c6b810-6db8-492b-984c-f4501734a2c8%40googlegroups.com. For more options, visit https://groups.google.com/d/optout.
