-
Notifications
You must be signed in to change notification settings - Fork 34
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
Update gmetad.conf and add custom RRAs #22
Conversation
| @@ -101,8 +113,12 @@ gridname "<%= @gridname %>" | |||
| #------------------------------------------------------------------------------- | |||
| # User gmetad will setuid to (defaults to "nobody") | |||
| # default: "nobody" | |||
| setuid_username "<%= @gmetad_user %>" | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm guessing that was accidental?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The default gmetad.conf doesn't have this set. I'll add it back in for debian support though.
| @@ -48,6 +48,7 @@ | |||
| class ganglia::gmetad( | |||
| $clusters = [ { 'name' => 'my cluster', 'address' => 'localhost' } ], | |||
| $gridname = undef, | |||
| $rras = $ganglia::params::rras, | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It should be nice if this was validated as an array.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Was unsure how to do that, but yes I agree it should be validated. Will look into it
|
Thank you for working on this! By any chance did you test with the epel5 version? el5 may need to get it's own templates. Any docs you could add the readme for this new feature would be helpful. |
|
I haven't got a rhel5 box to play with unfortunately. Is el5 still being actively used with el6 and el7? |
|
Have added some documentation on the RRAs |
|
Ok, this should be ready to go apart from the array validation. What's the best way to go about that? Should I just copy the example in ganglia_validate_clusters.rb? |
…r and integer respectively. This makes it easier to validate
|
Sorry, had some issues with Ruby. Can you review and see if I've done everything correctly? I can see the validation working on my local puppet. |
|
@jhoblitt is there anything else that needs to be to done to merge this? |
|
@NoodlesNZ It looks good skimming through it. I just need to give it a try myself (and sadly I've been swamped). |
|
I haven't forgotten about this -- I needed to get the acceptance tests back into a working state, which I just merged in PR #23. |
|
I rebased this PR on the current master (one line manifests/params.pp needs to be removed after the merge) and the acceptance tests pass on debian but fail on el6. Passes: Fails: with Are you willing to take a look? |
|
I'll take a look |
|
Sorry, missed adding case_sensitive_hostnames to one of the cases. Should be fine now. |
|
Great - thank you! I'm going to merge this. Let me know if want to take a stab at writing unit tests for |
|
I'll give it a go. I've never used ruby unit tests before, but I'll try and follow ones that you've already done. |
|
Let me know if you need any help. Rspec is easy to work with but it can |
Update gmetad.conf and add custom RRAs
|
That was weird -- GH closed the PR without merging it... |
I have updated gmetad.conf from the ganglia repo and added custom RRAs with default params. I have also changed the gmetad.conf template to remove el6 from the filename (as it's used as a common template across all supported OSes)