search

voxpupuli / puppet-splunk

Manage Splunk servers and forwarders using Puppet
https://forge.puppet.com/puppet/splunk
Apache License 2.0
40 stars 120 forks source link

Duplicate declaration: Class[Splunk::Platform::Posix] #61

Open Vincent-- opened 7 years ago

Vincent-- commented 7 years ago

Affected Puppet, Ruby, OS and module versions/distributions

Puppet code:

    include '::splunk'
    include '::splunk::forwarder'

Hiera value:

splunk::params::version: '6.5.0'
splunk::params::build: '59c8927def0f'
splunk::params::src_root: 's3://rg-infrastructure/splunk_install'
splunk::params::server: 'siem'

What are you seeing

Error message:

Error: Could not retrieve catalog from remote server: Error 400 on SERVER: Duplicate declaration: Class[Splunk::Platform::Posix] is already declared; cannot redeclare at /var/lib/rg_data/puppet/environments/production/modules/splunk/manifests/forwarder.pp:133 on node siem
Warning: Not using cache on failed catalog
Error: Could not retrieve catalog; skipping run

What behaviour did you expect instead

No error message

Output log

Any additional information you'd like to impart

crayfishx commented 7 years ago

I presume this only happens when you declare both classes

crayfishx commented 7 years ago

This seems to be the same issue raised in #48 - FYI

Vincent-- commented 7 years ago

I don't think this is related to https://github.com/voxpupuli/puppet-splunk/pull/48 indeed, this issue only happens if I declare both classes. This is due to this part of the code in forwarder.pp:

  case $::kernel {
    'Linux': {
      class { '::splunk::platform::posix':
        splunkd_port => $splunkd_port,
        splunk_user  => $splunk_user,
      }
    }

A workaround might be to use ensure_resource or ensure_resources from https://github.com/puppetlabs/puppetlabs-stdlib

The real fix should be to define 2 different ports in the param class: one for the forwarder and an other for the splunk server and to use an include instead of a class call (see init.pp for instance).

Happy to create a PR if everybody agrees on that.

Vincent-- commented 7 years ago

Btw, I've been able to make it works on my setup using this dirty workaround:

Class['::splunk::forwarder'] -> Class['::splunk']

The class ::splunk::platform::posix in forwarder.pp is then called before the include of the same class in init.pp, but this is definitely not a proper fix

crayfishx commented 7 years ago

I don't think ensure_resource works on classes (clarification?)

I've been looking at a longer term fix to considerably rewrite a lot of that structure, including getting rid of the swathe of virtual resources, tagging, collectors...etc, it's overkill. But that won't be a quick fix - if you want to submit a PR in the mean time if you can solve the issue without too much engineering then we can release that as a patch release until a more permanent rewrite of those classes.

ffrank commented 7 years ago

ensure_resource does not work with classes. Declaring classes is much different from resources, despite the unified syntax.

Also please note that replacing a class { <name>: ... } declaration with include <name> is not a safe way of resolving multiple declaration issues, except if we can make sure that only the include syntax is being used.

The basic problem is that there is no good way to pass class parameters from multiple places in the manifest. I have not looked at the module's structure in detail, but generally, a safe workaround is to eliminate public classes outside the main class from init.pp, and provide parameters to allow including (only) the desired other classes from the main.

Vincent-- commented 7 years ago

There was another issue related to the 'splunk enable boot-start' command line so I've asked a question on the splunk forum about that. From the answer, it's not recommended to use both splunk-forwarder and splunk-server on the same server (as apparently splunk-server can provide the splunk-forwarder capabilities)

See https://answers.splunk.com/answers/469814/how-can-i-enable-both-splunk-server-and-splunk-uni.html

Might be better to generate an error message if someone tries to use both class on the same node (instead of the warning about different ports) and also update the doc about this possibility. Thoughts about that?

ffrank commented 7 years ago

Hi, sorry for the delay.

Well, "not recommended" is one thing, but outright forbidding it in the module feels a bit harsh. Some users will know what they're doing and that will cause problems for them.

I still believe that refactoring the module interface is the way to go forward, if possible.

Vincent-- commented 7 years ago

Hi,

No pb. When I first tried to install Splunk, I didnt know splunk server can act as a forwarder too and that mislead me. I wanted to install both the splunk-forwarder and the splunk-server. Now my setup is using only splunk-server on the master and splunk-agent on all the slave and that works perfectly.

There is a lot of issues if you try to install both of them on the same server (partially due to this module but mainly because splunk has not been build to be configured this way).

I think an error message or at least a warning might lead the user in the good direction. If you think of a particular case when both of them are required, we shoud avoid the warning but I cannot think of any real case on my side.

Regards, Vincent

ffrank commented 7 years ago

Oh that's good info. I'm thinking we could