Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added option mcast_if for multicast configuration #53

Merged
merged 2 commits into from
Jun 30, 2015
Merged

Added option mcast_if for multicast configuration #53

merged 2 commits into from
Jun 30, 2015

Conversation

mrolli
Copy link
Contributor

@mrolli mrolli commented Jun 24, 2015

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

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.
@jhoblitt
Copy link
Owner

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?

The test suite additionally features gmond configuration
tests for multicast channel configuration within its own
context.
@mrolli
Copy link
Contributor Author

mrolli commented Jun 26, 2015

@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
Copy link
Owner

@mrolli 👍 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 pushed a commit that referenced this pull request Jun 30, 2015
Added option mcast_if for multicast configuration
@jhoblitt jhoblitt merged commit b8ad5d6 into jhoblitt:master Jun 30, 2015
@jhoblitt
Copy link
Owner

jhoblitt commented Jul 1, 2015

@mrolli Released to the forge as part of 2.0.0

@mrolli
Copy link
Contributor Author

mrolli commented Jul 2, 2015

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

@mrolli mrolli deleted the feature/option_mcastif branch July 20, 2017 05:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants