jhoblitt / puppet-ganglia

Manages ganglia gmond & gmetad daemons + web front end
Other
12 stars 34 forks source link

Added option mcast_if for multicast configuration #53

Closed mrolli closed 9 years ago

mrolli commented 9 years ago

As previously already announced, I'd like to contribute a new option to multicast configuraton of gmond:

Often it is desirable to limit multicast traffic to certain interfaces only. gmond features the configuration option mcast_if for udp_send_channel and udp_recv_channels, which was added to gmond erb templates.

Cheers Michael

jhoblitt commented 9 years ago

This looks good. However, it does make me wonder if at some point the templates need to be redone to iterate over keys in a hash.

Are you willing to take a stab at unit test coverage of the template changes?

mrolli commented 9 years ago

@jhoblitt Yes, I felt willing. Here you are. Sufficient and in a sensible manner?

Regarding your considerations. In my opinion for a configuration file like gmond.conf (or config.php for web) it's almost impossible to offer an all-round carefree package. There are always use cases where certain additional stuff could be useful. So, I'd try to provide something that's quite complete in the way that the often (and well documented) stuff is covered, but give the module's users the possibility to override the template itself with a self-written template file or even the content itself (the user could use tempalte() himself if willing or provide a static file); something like the following in case of static file:

    class ganglia::gmond (
      $config_source = undef
) {

### and in ganglia::gmond::config

   file { $::ganglia::params::gmond_service_config:
      ensure  => present,
      owner   => 'root',
      group   => 'root',
      mode    => '0644',
      notify  => Class['ganglia::gmond::service'],
    }

    if $::ganglia::gmond::config_source {
        File[$::ganglia::params::gmond_service_config] {
            source => $::ganglia::gmond::config_source,
        }
    } else {
       File[$::ganglia::params::gmond_service_config] {
            content => template($::ganglia::params::gmond_service_erb),
        }
    }

Like this your templates are useful for 95% and the rest may override the config completely (either by templating/static file) but could use the rest of module. Just an idea...

jhoblitt commented 9 years ago

@mrolli :+1: This looks great.

I agree that template overrides are useful and would be more than happy to merge a PR for that.

I'm working on sorting out puppet-4 compatibility/testing today and plan to make a 2.0 release soon.

jhoblitt commented 9 years ago

@mrolli Released to the forge as part of 2.0.0

mrolli commented 9 years ago

@jhoblitt Great news. Many thanks. Switched back to your release in my Puppetfile.