Skip to content
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

Why concat multiple smaller configuration files? #227

Closed
apple-corps opened this issue Jun 20, 2015 · 18 comments
Closed

Why concat multiple smaller configuration files? #227

apple-corps opened this issue Jun 20, 2015 · 18 comments
Assignees

Comments

@apple-corps
Copy link

Is this for backwards compatibility with previous versions? Doesn't the conf.d directory support using multiple files for configuration? Wouldn't it be neater and easier to read to keep the multiple smaller configuration files, instead of generating a large one?

Furthermore I also experienced issues with using your concat module. I copied it into my modules folder, but it wasn't found. Then I used somebody else's hack to use the puppet concat. But not sure why I'm bothering to concatenate the configuration files in the first place.

@snobear
Copy link

snobear commented Jul 3, 2015

@Drocsid I'm running into an issue myself related to the combined conf.d/logstash.conf file that is created from multiple sources. It's worked fine for me up until now, so it must be a weird bug with whatever I'm using in this particular logstash instace: multiline and json_lines? I don't know. I experienced this in logstash 1.4 and 1.5. I'll probably just fork and have logstash::configfile create separate files.

On further thought, I propose an optional parameter to disable concatenation if desired:

 logstash::configfile { 'configname':
   source => 'puppet:///path/to/config.file',
   concat => false,
 }

which would default to true, i.e. the current behavior. Anything with concat => false would be deployed as its own separate file in conf.d, and anything with true would be added to the combined logstash.conf.

Thoughts?

@apple-corps
Copy link
Author

Hi @snobear ,

That makes sense. I was trying to get the author @electrical to comment. Anyhow, somewhere else around here somebody shows how to modify the module to use puppetlabs-concat , and that worked for me. I thought I read that you could drop config files into the logstash conf.d directory without needing to restart logstash, and I was trying to confirm that functionality. I should just try it out.

@electrical
Copy link

Hi @snobear @Drocsid sorry for not responding earlier.
There was a reason for doing the concat part but can't exactly remember why.
This will most likely be removed in future releases.

@sergii-zemlianyi
Copy link

Hi,

So it seems there is no way to split config by separate file and not combine them in one huge config?
What is the practical reason of combining all in one file?

Thanks
Sergey

@sergii-zemlianyi
Copy link

Hey guys,

seems purge of config files located under ${logstash::configdir}/conf.d is not working!
Purge on ${logstash::configdir} level works, but not further.
I turned purging on by passing hiera value
logstash::purge_configdir: true

Sounds like an issue or I missed something?

Regards
Sergey

@snobear
Copy link

snobear commented Jul 10, 2015

Hi @szemlyanoy - you may want to open a separate issue for your question since it isn't specific to this particular issue.

@sergii-zemlianyi
Copy link

makes sense! ))

@gislifreyr
Copy link

Bumped into this "issue" yesterday and I agree with earlier commenters, the module should not concatenate template-files into one file.
As @snobear proposes, a parameter to configure this would be a good solution although I think that by default it should be set to false (concat => false) as that is, in my opinion, the expected way you deal with the conf.d directory.
Looking forward to this getting fixed in a future release.

Anyone know of a module that handles this "correctly" that might be worth looking into to see how it should be done?

@peterg-bcc
Copy link

Hi guys,

I ran into the same problem with the config files, I downloaded the zip at the commit (https://github.com/imsweb/puppet-logstash/commit/c67c6c800203975f1be81d5f7da016a2e155a230), re-tar.gz-d and installed as a module the realized that:

  • there is no refresh on the individual files after creation
  • there is no proper ordering on the individual files (copied after logstash start)
  • deleting / modifying the files under conf.d won't trigger a refresh for some reason

I have a patch for the first two issues ( http://pastebin.com/7g1YWdKf ), any ideas on how to submit the patch and implement the refreshes on modifications?

Many thanks!

@fgimian
Copy link

fgimian commented May 24, 2016

Hey guys, we are affected by this issue too. Having one large config file is very difficult to work with. The suggestion above for a contcat parameter that defaults to false sounds great. For now, I am using a regular file resource to create my configs but of course, the module still creates an empty logstash.conf which I ignore 😄

Any help would be greatly appreciated!

@fgimian
Copy link

fgimian commented May 24, 2016

P.S.: another problem with the current approach is that it always executes the following, even if the config is up to date:

Notice: /Stage[main]/Logstash::Config/File_concat[ls-config]/content:

@fgimian
Copy link

fgimian commented May 24, 2016

Here's my workaround to this in case it helps anyone:

  # We have to manually create individual config files and disable creation
  # of logstash.conf due to issue
  # https://github.com/elastic/puppet-logstash/issues/227

  # Disable creation of logstash.conf
  File_concat <| title == 'ls-config' |> {
    ensure => absent,
  }

  # Define defaults for all config files to be copied
  File <| title == $title |> {
    ensure  => present,
    owner   => 'root',
    group   => 'root',
    mode    => '0644',
    require => Class['logstash'],
    notify  => Service['logstash'],
  }

  # Input
  file { '/etc/logstash/conf.d/100-input.conf':
    content => template('profile/logstash/100-input.conf.erb'),
  }

  # Processing
  file { '/etc/logstash/conf.d/201-app1.conf':
    source => 'puppet:///modules/profile/logstash/201-app.conf',
  }
  file { '/etc/logstash/conf.d/202-app2.conf':
    source => 'puppet:///modules/profile/logstash/202-app2.conf',
  }
  ...

  # Output
  file { '/etc/logstash/conf.d/300-output.conf':
    content => template('profile/logstash/300-output.conf.erb'),
  }

Cheers
Fotis

@ninaspitfire
Copy link
Contributor

I think we can probably go further and just remove concatenation altogether. Logstash has been happy to take directories and globs for the config file argument since version 1.0.14, and the current module won't work with versions earlier than 1.5 anyway.

@ninaspitfire ninaspitfire self-assigned this May 27, 2016
@fgimian
Copy link

fgimian commented May 27, 2016

@jarpy Completely agree, that would be great! Thanks 😄

@joshuaspence
Copy link
Contributor

+1

@ikoniaris
Copy link

I also stumbled upon the issue with concat doing this:
Notice: /Stage[main]/Logstash::Config/File_concat[ls-config]/content:
every time Puppet runs, which in turn restarts the process. This is annoying.
@fgimian did you find a solution to this specific problem above?

@ikoniaris
Copy link

This is related to the issue with Puppet restarting Logstash every time it runs due to file concatenation: electrical/puppet-lib-file_concat#30 (comment)

@ninaspitfire
Copy link
Contributor

ninaspitfire commented Dec 19, 2016

Fixed in master. No more concat! 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

10 participants