puppet-mongodb icon indicating copy to clipboard operation
puppet-mongodb copied to clipboard

Wrong "pidFilePath:" setting in `/etc/mongodb.conf` on Debian

Open dhs-rec opened this issue 3 years ago • 8 comments

Affected Puppet, Ruby, OS and module versions/distributions

  • Puppet: 7.x
  • Ruby: as delivered with Puppet
  • Distribution: Debian Buster
  • Module version: 4.1.1

How to reproduce (e.g Puppet code you use)

  class { 'mongodb::globals':
    manage_package_repo   => true,
    mongod_service_manage => true,
    version               => $version,
  }

  class { 'mongodb::server':
    ...
    fork           => false,
    pidfilepath    => undef,
    manage_pidfile => false,
    ...
    require        => Class['mongodb::globals'],
  }

What are you seeing

There's a pidFilePath: setting in /etc/mongodb.conf that shouldn't be there:

#Process Management
processManagement:
  pidFilePath: /var/run/mongod.pid

which leads to a server start failure on reboots because the pidfile isn't there and the mongod process has no permission to create it.

What behaviour did you expect instead

There should be no pidFilePath: setting in the file at all. The (MongoDB) documentation says:

If the processManagement.pidFilePath option is not specified, the process does not create a PID file. This option is generally only useful in combination with the processManagement.fork setting.

and

On Linux, PID file management is generally the responsibility of your distro's init system: usually a service file in the /etc/init.d directory, or a systemd unit file registered with systemctl. Only use the processManagement.pidFilePath option if you are not using one of these init systems.

As you can see above, $fork is explicitly set to "false" (the default of "undef" didn't work either). I worked around this by modifying templates/mongodb.conf.2.6.erb like so:

# git diff mongodb/templates/mongodb.conf.2.6.erb
diff --git a/mongodb/templates/mongodb.conf.2.6.erb b/mongodb/templates/mongodb.conf.2.6.erb
index 32c38be3..dfa66aaa 100644
--- a/mongodb/templates/mongodb.conf.2.6.erb
+++ b/mongodb/templates/mongodb.conf.2.6.erb
@@ -32,7 +32,7 @@ systemLog.verbosity: 5
 <% end -%>
 
 #Process Management
-<% if @fork or @pidfilepath -%>
+<% if @fork -%>
 processManagement:
   <%- if @fork -%>
   fork: <%= @fork %>

but I guess this is really just this, a workaround...

dhs-rec avatar Jul 21 '22 07:07 dhs-rec

An update to 4.2.0 has now overwritten my workaround and thus has put the processManagement.pidFilePath into the file again, leading to a non-starting mongod.service again.

This means that the handling of this setting is still broken in 4.2.0. Can this please be fixed?

dhs-rec avatar Dec 13 '22 11:12 dhs-rec

Is there a pull request for this?

kenyon avatar Dec 14 '22 07:12 kenyon

The diff is in the description above.

dhs-rec avatar Dec 14 '22 07:12 dhs-rec

I think it would be better to fix this line: https://github.com/voxpupuli/puppet-mongodb/blob/94cbca3f1341e51e2fb78c4ad2b99af5a7521805/manifests/params.pp#L60

Since we only support Debian versions that use systemd, $mongodb::params::pidfilepath should end up undef.

kenyon avatar Dec 14 '22 08:12 kenyon

Yeah, as I wrote: My change was only a workaround.

dhs-rec avatar Dec 14 '22 08:12 dhs-rec

Hi @dhs-rec, thanks for raising the issue. We're a small collective of module authors and maintain > 150 modules. It's not possible to provide fixes for all the issues that people raise. If you like, please provide a PR with the proposed change. We're also happy to guide you through our CI system or how to write tests. Feel free to hop into our IRC channel #voxpupuli on libera.chat or #voxpupuli on https://slack.puppet.com/.

bastelfreak avatar Dec 14 '22 08:12 bastelfreak

@bastelfreak, I don't get this. Obviously, @kenyon already found the right place and what's needed to do the fix. He could as well have just committed it...

dhs-rec avatar Dec 15 '22 13:12 dhs-rec

@dhs-rec Good luck, I certainly won't be helping you get anything fixed thanks to your ungrateful attitude.

kenyon avatar Dec 16 '22 07:12 kenyon

@dhs-rec release 6.0.0 should solve this. Could you please verify?

h-haaks avatar Apr 29 '24 18:04 h-haaks

Sure, I'll take a look as soon as possible. Thanks a lot for the hint.

dhs-rec avatar Apr 30 '24 05:04 dhs-rec

Yes, looks good. Thanks for fixing this.

dhs-rec avatar Apr 30 '24 06:04 dhs-rec