SplunkForwarder has no concept of a server_service. #130
Conversation
|
I am not familiar with this module or Splunk, and also, can't even test it since the acceptance tests won't work even for the forwarder without a locally hosted package. That said, would adding: Maybe ask someone who has more experience with this module to review? |
|
I agree with your recommendation. This module needs to be cleaned up but that is a bigger beast alltogether. ATM noone can run puppet-rspec against this because of it blows up saying hey there is a variable in this module that doesnt exist! |
|
Done. @wyardley I'm going to try and add a unit test for this to prevent regression and to prove out the fix. Please wait before merging this. |
|
The fix you recommended still has this issue. Going to revert my revert ;)
|
|
My fix does indeed work. It also reveals another variable with the same issue.. I will open another issue for this.
This is ready for merge unless you have more refactor suggestions @wyardley |
…n error because the default is specified as splunk::server_service which isn't used when creating a splunk forwarder.
TraGicCode
requested review from
wyardley
and removed request for
bastelfreak
| @@ -146,7 +146,13 @@ | |||
| # there is non-generic configuration that needs to be declared in addition | |||
| # to the agnostic resources declared here. | |||
| case $::kernel { | |||
There was a problem hiding this comment.
on a long term point of view, we should switch to $facts['kernel']
when setting up a splunk forwarder on a posix based system you get a warning because the the default parameter for the splunk::platform::posix class is set to $splunk::server_service which doesn't exist when you create a forwarder. It doesn't exist because it doesn't make sense for a forwarder. While there should be more cleanup this is a simple non-breaking change to fix the annoying warning. A bigger refactor would be required to get this done correctly.
#119