Why Your Puppet Module Sucks

Posted: Tue, 29 October 2013 | permalink | 6 Comments

I use Puppet a lot at work, and I use a lot of modules written by other people, as well as writing quite a number of my own. Here’s a brief list of reasons why I might say that your module sucks.

1. You use global variables

This would have to be both the most common idiom that just makes my teeth grind. Defined types exist for a bloody reason. Global variables make it incredibly difficult to reason about what is going to happen when I use a particular global variable (see also “You don’t write documentation”), and I get no feedback if I typo a global variable name. Add in a healthy heaping of “lack of namespacing”, and you’ve basically guaranteed that your module will be loudly cursed.

2. You use parameterised classes

I’ve never managed to work out why parameterised classes even exist. They’ve got all the problems of regular classes, as well as all the problems of types. There was a fantastic opportunity with parameterised classes to fix some of the really annoying things about regular resources, such as doing conflict checking on parameters and only barfing if there was a conflict… but no, if you define the same class twice, even with identical parameters, Puppet will smack you on the hand and take away your biscuit. FFFFFFUUUUUUUUUUUU-

3. You fail at using fail()

Modules are supposed to be reusable. That’s their whole reason for existence. One of the benefits of Puppet is that you can provide an OS-independent interface for configuring something. However, unless you’re some absolute God of cross-platform compatibility, you will only be writing your module to support those OSes or distros that you, personally, care about.

That’s cool – if you don’t know anything about OS X, you probably shouldn’t be guessing at how to support something on that platform anyway. However, when you do have some sort of platform-specific code, for the love of pete, have a default or else clause that calls fail() with a useful and descriptive error message, to indicate clearly that you haven’t included support for whatever environment the module’s being used in. Failing to do this can cause some really spectacular explosions, because the rest of your module assumes that certain things have been done in the platform-specific code, and when it hasn’t… hoo boy.

4. You don’t write documentation

Yes, it isn’t easy to write good documentation. I know that. But without documentation, your module is going to be practically unuseable. So if you don’t have docs, your module is basically useless. Well done, you.

5. You have undeclared dependencies

This also includes declaring your dependencies in a non-machine-parseable manner; I’m not going to grovel through your README for the list of other modules I might need; I have machines to do that kind of thing for me.

If I try to use your module, and it craps out on trying to reference some sort of type or class that doesn’t appear at all related to your module, I will call into question your ancestry, and die a little more inside.

6. You use common packages without even trying to avoid the pitfalls

OK, to be fair this is, ultimately, Puppet’s grand fuckup, not yours, but you at least need to pretend to care…

There is no sane, standardised way in Puppet for multiple modules to install the same package. Let’s say that a module I write needs to have a compiler. So I package { "gcc": }. Then someone else’s module also wants a compiler, so it also package { "gcc": }. FWAKOOM! says Puppet. “What the fuck?” says the poor sysadmin, who just wanted both a virtualenv and an RVM environment on the one machine.

Basically, using packages is going to make your module suck. Does that mean that this makes wide distribution of a large repository of modules written by different people nigh-on impossible? Yes. Fantastic.


6 Comments

From: Arnaud Gomes
2013-10-29 22:01

I beg to differ regarding parametrized classes. I grant you that by themselves, they are at best mildly harmful, but if you use them with Hiera it is a very good fit for the global variables issue.

My view on this is probably strongly related to my environment; basically I use a custom-written ENC and the more work I can outsource (to Hiera in this case), the better it is.

From: Warren
2013-10-30 02:22

I am learning puppet, and trying to follow the best practices documentation.

I am having problems with paramaterised classes - even those that I am writing. Can you point me at an alternate approach? I have been using them to consolidate the install status of a package + service in a single location rather than 3+ locations.

What can I do to make my code more resilient to the multiple definitions of package{‘gcc’:}?

From: Matt Palmer
2013-10-30 08:25

Hi Arnaud, if you’re using Hiera, do you even need parameterised classes? Couldn’t you just do the lookup inside the class? If you’re using the parameterisation to pass values from Hiera into the class, you’re still going to suffer from the same problems as using literals. Also, have you come up with a way to use Hiera in a way that doesn’t tie your module intimately to your local environment?

Warren, I just stick with defined types. They’re no worse than parameterised classes, and they encourage you to think about reuse, rather than being able to say “well nobody will ever be able to use my class twice!”, which I’ve found to be useful in many cases. My response to the Puppet “best practices” documentation mirrors that of Linus’ comments on the GNU coding standards:

First off, I’d suggest printing out a copy of the GNU coding standards, and NOT read it. Burn them, it’s a great symbolic gesture.

But then, I’ve been accused of being excessively opinionated…

Finally, the package { "gcc": } problem… this is a big topic, and I’ve decided to make this a blog post all of its own.

From: Arnaud Gomes
2013-10-30 21:23

I’m not sure I need parametrized classes but I’m lazy enough to like them for avoiding the explicit Hiera lookup boilerplate. :-)

Actually I think my use of Hiera is fairly typical. I use a mix of modules, some homemade, the other taken from the Forge. Some of these modules (basically all of the later and a few of the former) are as generic as possible, that is they rely heavily on parametrized classes; others are basically wrappers around the “building block modules” which just set the right parameters. Hiera is used for passing parameters which the wrapper modules don’t care about (say the upload size limit for PHP, for example), and for using generic modules directly (NTP comes to mind here; I use the puppetlabs/ntp module and rely on Hiera to set sane default values for most machines and different ones for my NTP servers).

I don’t really understand what you mean with Hiera tying my modules to my environment; on the contrary I use it to separate the local data from the generic code.

From: John Cooper
2013-10-31 00:58

With respect to parametrised classes I think this should be one:

https://github.com/anchor/puppet-module-ssh/blob/master/manifests/server.pp

It suggests from the fact that it is a defined type that I can set up multiple versions of it but if I do it will blow up. Classes are there to show that this type should only set up the server once.

You will of course suggest that I may want to setup more than one ssh server and that is a valid use case but not one that the module has any support for in its current state.

The use of classes make you think in a declarative way rather than a functional one and should be encouraged.

john

From: Matt Palmer
2013-11-04 09:08

Hi John, while I agree that ssh::server there could be a parameterised class, I’m not going to make it one. Attempts to instantiate ssh::server twice will explode at compile time, rather than doing something unexpected at run time – which is perfectly fine by me (although I’d certainly consider a better error message to be useful). The main reason I won’t do it, though, is that when, in the future, I modify the module to support multiple servers, I’d need to switch to a defined type then anyway, and that would be a non-backwards-compatible change, requiring me to edit every single manifest that uses ssh::server. Bugger that.

Post a comment

All comments are held for moderation; markdown formatting accepted.

This is a honeypot form. Do not use this form unless you want to get your IP address blacklisted. Use the second form below for comments.
Name: (required)
E-mail: (required, not published)
Website: (optional)
Name: (required)
E-mail: (required, not published)
Website: (optional)