search

thias / puppet-sysctl

Puppet module to manage sysctl parameters
Other
35 stars 86 forks source link

add support for freebsd #13

Closed sethlyons closed 10 years ago

sethlyons commented 10 years ago

this PR adds support for freebsd, which handles sysctl a bit differently. i still need to update the tests and cleanup a few things, but i figured i'd submit this now so you can see the framework i'm using.

thias commented 10 years ago

I think the approach you're leaving for Linux is wrong : There, we manage both a file and a directory with file snippets. The "problem" being that a few default values ship inside the file, and we want to update them there too.

So if FreeBSD doesn't support the sysctl.d directory, it would be best to keep some common management of the main file, and make the directory optional. The added benefit would be that this would also make older GNU/Linux distributions work.

Last, using file_line adds a dependency on the stdlib module, and it looks like what you're doing doesn't support "absent" (yet?).

sethlyons commented 10 years ago

i made the requested updates. there is now a user-defined option for using either /etc/sysctl.d/ (directory) or just managing /etc/sysctl.conf (file). default values are file for freebsd and directory for everything else, but the architecture is modular so it is easy to add other OS defaults as needed. i thought it was important to use file_line for managing the file instead of a template because there are probably many cases where a user has a system with a large sysctl.conf that is not being managed by puppet, but they want to use puppet to add (or remove) a specific value.

once you're happy, i'll update tests and documentation.

thias commented 10 years ago

I've made many changes recently, and this doesn't apply cleanly any more, sorry. In any case, there are changes that I would have required anyway. For instance the $management variable : When something is either one way or another, no need to decide on special keywords, it's much easier to use a boolean.

In commit 40615fd3d8bdf725cb1774f4296598b3ad20fd51 I've added what you need to the params.pp and base.pp files. With minor changes to init.pp is should now be possible to do what you need.

Could you please re-implement your changes on top of the current code? Note that since the $sysctl_dir parameter can be overriden (using hiera), your changes would also enable anyone on Linux to decide not to use the sysctl.d directory, but have everything managed in the main file instead.

thias commented 10 years ago

Closing for now. Maybe you've found a module that better suited your needs...

If you still want to get your changes in, please reopen or open a new pull request, taking into account what I've written previously. Thanks!