voxpupuli / puppet-collectd

Collectd module for Puppet
https://forge.puppet.com/puppet/collectd
Apache License 2.0
69 stars 273 forks source link

Augeas instead of ERB templating? #585

Open OmegaVVeapon opened 7 years ago

OmegaVVeapon commented 7 years ago

I've noticed a small annoyance with the way the collectd.conf is generated using ERB templates.

The issue arises from the fact that the module requires the user to know all possible TypesDB values in advance in order to provide them to the class. I.e:

class { '::collectd':
  purge_config => true,
  typesdb => ["/path/to/types.db"]
}

The problem is that it is often the case that one uses custom plugins in a system. The plugins needed depend on certain conditions such as:

if $::monitor_gpu {
//install collectd GPU package
//add custom GPU typeDB to collectd.conf
}
if $::monitor_iostat {
//install collectd iostat package
//add custom iostat typeDB to collectd.conf
}

Since arrays are not mutable in Puppet, it becomes complicated to handle such a case cleanly as one cannot simply append items to an array and call the class when everything is set.

I've started work on a branch to tackle this issue using Augeas (https://github.com/OmegaVVeapon/puppet-collectd/tree/augeas_typesdb). The previous use case now works cleanly. However, idempotency is broken. I'll probably have to replace the entire template with Augeas in order to fix it.

I know the module provides facilities to create new types with type and typesdb but there are already several existing upstream plugins that provide their own types (The iostat plugin I've been using recently comes to mind). So it makes no sense to recreate them.

Do you guys have any thoughts on: 1) The use case of flexibly adding entries to TypesDB to collectd.conf using the existing class 2) Moving entirely away from ERB templating and using Augeas instead.

dhoppe commented 7 years ago

@OmegaVVeapon Did you compare the execution time? In my experience Augeas does not perform very well, but that might have changed.

bastelfreak commented 7 years ago

Getting rid of ebb templates is a good approach. I think augeas or epp templates are the way to go. I am not 100% familiar with all the epp functionality but I guess we run into the same issues as we currently have with erb so augeas has to be used here?

OmegaVVeapon commented 7 years ago

@dhoppe I'd need to retest but do remember a small increase in execution time. However, I do believe the added flexibility is well worth the added couple of seconds.

jamtur01 commented 7 years ago

Or could look at something like: https://github.com/pyr/collectd-dsl - instantiated as the camptocamp folks do: https://github.com/camptocamp/puppet-collectd/blob/master/lib/puppet/parser/functions/collectd_dsl.rb