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

gmetad.conf - adding support for managing 'all_trusted' and 'trusted_hos... #48

Merged
merged 3 commits into from
Apr 16, 2015

Conversation

tskirvin
Copy link
Contributor

...ts'

Adds two optional parameters to gmetad.pp - 'all_trusted' (a boolean,
default false), and 'trusted_hosts' (array of strings, default empty).
These can be used to populate the corresponding fields in gmetad.conf.

Includes a spec file check for these parameters as well, though I haven't
tested that adequately yet.

@@ -102,12 +102,16 @@ gridname "<%= scope.lookupvar('::ganglia::gmetad::gridname') %>"
# is always trusted.
# default: There is no default value
# trusted_hosts 127.0.0.1 169.229.50.165 my.gmetad.org
trusted_hosts <%= @trusted_hosts.join(' ') %>
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This causes failures under the future parser (which are allowed to fail in the travis matrix due to a change in erb :undef handing) -- could you add a condition check for nil and check the output of the future parser runs?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Joshua Hoblitt notifications@github.com writes:

@@ -102,12 +102,16 @@ gridname "<%= scope.lookupvar('::ganglia::gmetad::gridname') %>"

is always trusted.

default: There is no default value

trusted_hosts 127.0.0.1 169.229.50.165 my.gmetad.org

+trusted_hosts <%= @trusted_hosts.join(' ') %>

This causes failures under the future parser (which are allowed to fail
in the travis matrix due to a change in erb :undef handing) -- could
you add a condition check for nil and check the output of the future
parser runs?

    I've added the condition check for nil?, along with some other

checks. I'm running into an issue with the .spec bit for a block like
this, though:

context 'default' do
it 'should have an empty trusted_hosts' do
should contain_file('/etc/ganglia/gmetad.conf').
with_content(/^# trusted_hosts .*$/)
end
end # default

    It's complaining like the 'trusted_hosts' block can't be parsed,

but I'm really not sure why. I've left it commented out for now; the
checks are still fairly useful but not as good as I'd like.

    As for the future parser, it's still griping, but apparently based

on something to do with case_sensitive_hostnames.

    I'm happy to take another round at this if you think I can help.

                       - Tim Skirvin (tskirvin@killfile.org)

http://wiki.killfile.org/ Skirv's Homepage < <*>
http://www.flickr.com/photos/tskirvin Skirv's Pictures

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Joshua Hoblitt notifications@github.com writes:

@@ -102,12 +102,16 @@ gridname "<%= scope.lookupvar('::ganglia::gmetad::gridname') %>"

is always trusted.

default: There is no default value

trusted_hosts 127.0.0.1 169.229.50.165 my.gmetad.org

+trusted_hosts <%= @trusted_hosts.join(' ') %>

This causes failures under the future parser (which are allowed to fail
in the travis matrix due to a change in erb :undef handing) -- could
you add a condition check for nil and check the output of the future
parser runs?

    Okay, I think I have it working the way you'd expect: 

https://github.com/jhoblitt/puppet-ganglia/pull/48

    The CI build passes, the only errors associated with

Jenkins appear to be associated with 'gridname' in gmetad.conf and
'override_hostname' in gmond.conf (neither of which I touched).

                       - Tim Skirvin (tskirvin@killfile.org)

http://wiki.killfile.org/ Skirv's Homepage < <*>
http://wiki.killfile.org/software/ Skirv's Software

…hosts'

Adds two optional parameters to gmetad.pp - 'all_trusted' (a boolean,
default false), and 'trusted_hosts' (array of strings, default empty).
These can be used to populate the corresponding fields in gmetad.conf.

Includes a spec file check for these parameters as well.
@tskirvin
Copy link
Contributor Author

Should be good to go now. The future parser is still griping, but not about changes I made (as far as I can tell).

@jhoblitt
Copy link
Owner

Thank you for putting effort into this!

I believe it was complaining before this PR about a change in undef behavior inside of ERB that I haven't fuly run down (I think it happened around 3.7.1). I'll merge this today after an acceptance test run.

@jhoblitt
Copy link
Owner

Also, would you add the the new params to the README? Hopefully, we'll be living in the future were the puppet docs are marchine generated soon but we're not there yet.

@tskirvin
Copy link
Contributor Author

Joshua Hoblitt notifications@github.com writes:

Also, would you add the the new params to the README? Hopefully, we'll
be living in the future were the puppet docs are marchine generated soon
but we're not there yet.

    Done.

                       - Tim Skirvin (tskirvin@killfile.org)

http://wiki.killfile.org/ Skirv's Homepage < <*>
http://www.facebook.com/tskirvin Skirv's Social Networking

@jhoblitt
Copy link
Owner

It passed an acceptance test run. Thank you for contributing!

jhoblitt pushed a commit that referenced this pull request Apr 16, 2015
gmetad.conf - adding support for managing 'all_trusted' and 'trusted_hos...
@jhoblitt jhoblitt merged commit c87d8eb into jhoblitt:master Apr 16, 2015
@jhoblitt jhoblitt mentioned this pull request Apr 16, 2015
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