-
-
Notifications
You must be signed in to change notification settings - Fork 98
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
Add a new define telegraf::input #13
Conversation
Add tests Extent the tests Update readme Fix a minor template issue [add_input_define] Alter the define to accept a plugin type, allows for multiple inputs of the same type [add_input_define] Fix a typo in the README [add_input_define] Aaaand again, too much caffeine [add_input_define] Typo [add_input_define] Make sure not to quote values passed in via a manifest
|
|
||
| # Install module | ||
| # We assume the module is in auto load layout | ||
| puppet_module_install(:source => proj_root, :module_name => "#{File.basename(proj_root)}") |
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.
We can't assume this as for acceptance tests the module ends up being copied in as 'puppet-telegraf' which then breaks things. :module_name needs to be set to telegraf.
|
All rspec tests are now passing for me (acceptance and unit) |
| # Install module | ||
| # We assume the module is in auto load layout | ||
| puppet_module_install(:source => proj_root, :module_name => 'telegraf') | ||
| # Install dependancies | ||
| hosts.each do |host| |
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.
We still need the apt-transport-https package installed on Debian, otherwise the Telegraf package fails to install as the InfluxData repo is HTTPS. Re-adding the on 'debian', 'apt-get -y install apt-transport-https' line will do the trick.
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.
@stuartbfox I'm happy to merge this PR and update the acceptance testing helper stuff separately if you'd prefer. Keen to get this in!
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.
Sorry, I've been on vacation.
Whats outstanding?
|
+1 could you please merge this? The acceptance testing could be fixed separately. |
No description provided.