-
-
Notifications
You must be signed in to change notification settings - Fork 127
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
Refactor defined type Splunk::Addon #224
Conversation
fbffdcc
to
0c128b3
Compare
Transitions the defined type Splunk::Addon to using the splunk*_input types instead of concat, supports installation from splunkbase provided archive, and valid for use on more than just forwarder nodes.
Commit introduces Puppet Strings documentation to the newly refactored defined type Splunk::Addon
0c128b3
to
23c5d8d
Compare
Minor tweak to supporting spec test to reflect the ability to install from splunkbase archives
Commit adds defults to case statements with a fail message that should trigger if Class[splunk::enterprise] or Class[splunk::forwarder] are not declared that informs the use of the reason for the failure.
Thanks! |
concat { "splunk::addon::inputs_${name}": | ||
path => "${_splunk_home}/etc/apps/${name}/local/inputs.conf", | ||
require => File["${_splunk_home}/etc/apps/${name}/local"], | ||
unless $inputs.empty { |
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.
Why? Iterating an empty hash is fine.
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.
You are correct. Just didn't realize. Tested just now and works fine without testing first
case $mode { | ||
'forwarder': { $_splunk_home = $splunk::params::forwarder_homedir } | ||
'enterprise': { $_splunk_home = $splunk::params::enterprise_homedir } | ||
default: { fail('Instances of Splunk::Addon require the declaration of one of either Class[splunk::enterprise] or Class[splunk::forwarder]') } |
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.
This is unreachable?
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.
Correct and the reason I didn't originally add the default statement. Only added it to get it to pass lint checks. Not a bad thing to have, I suppose...?
If the manifest parses this far then something strange is happening an it'd be good to explicitly fail before damage is done.
|
||
if defined(Class['splunk::forwarder']) { | ||
$mode = 'forwarder' | ||
} else { |
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.
Is it worth checking that Class['splunk']
has been defined?
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.
Class['splunk'] is now completely empty since the 7.2 refactor: https://github.com/voxpupuli/puppet-splunk/pull/215/files#diff-60ae41fd0a31977447947f59940ee9a4
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, splunk::enterprise
then.
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.
Wouldn't hurt.
Optional[String[1]] $package_name = undef, | ||
Hash $inputs = {}, | ||
) { | ||
|
||
include 'splunk::params' | ||
include splunk::params |
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.
Shouldn’t be necessary if it’s a requirement that one of splunk
or splunk:forwarder
has been included already?
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.
Yes, that should be correct. Didn't dawn me at the time.
Removes declaration of Class[splunk::params] since it should already be in scope and declared by either Class[splunk::enterprise] or Class[splunk::forwarder] and removes an unnecessary test of an empty hash. Modifies Splunk::Addon spec test to address the removal of Class[splunk::params].
Removes declaration of Class[splunk::params] since it should already be in scope and declared by either Class[splunk::enterprise] or Class[splunk::forwarder] and removes an unnecessary test of an empty hash. Modifies Splunk::Addon spec test to address the removal of Class[splunk::params].
Removes declaration of Class[splunk::params] since it should already be in scope and declared by either Class[splunk::enterprise] or Class[splunk::forwarder] and removes an unnecessary test of an empty hash. Modifies Splunk::Addon spec test to address the removal of Class[splunk::params].
Removes declaration of Class[splunk::params] since it should already be in scope and declared by either Class[splunk::enterprise] or Class[splunk::forwarder] and removes an unnecessary test of an empty hash. Modifies Splunk::Addon spec test to address the removal of Class[splunk::params].
Removes declaration of Class[splunk::params] since it should already be in scope and declared by either Class[splunk::enterprise] or Class[splunk::forwarder] and removes an unnecessary test of an empty hash. Modifies Splunk::Addon spec test to address the removal of Class[splunk::params].
Removes declaration of Class[splunk::params] since it should already be in scope and declared by either Class[splunk::enterprise] or Class[splunk::forwarder] and removes an unnecessary test of an empty hash. Modifies Splunk::Addon spec test to address the removal of Class[splunk::params].
Address post-merge comments on PR #224
Transitions the defined type Splunk::Addon to using the splunk*_input types instead of concat, supports installation from splunkbase provided archive, and valid for use on more than just forwarder nodes.