Machines without the pulp class included still run queries for mongodb version

[andymillar]

pulp registers a fact to identify the mongodb version so that it can configure $mongodb_pidfilepath in pulp::database

As this is defined as a fact it is run on every server associated with the puppetmaster, not just those including the pulp class.

This puts unnecessary load on systems and upstream repo servers as yum is hit with every facter run.

[ehelms]

@andymillar feel free to open a PR with your change and we'll review it! @iNecas thoughts?

[andymillar]

@ehelms done. Took a little longer to commit and make sure it actually worked.

[andymillar]

Converting the fact to a function doesn't work as desired as it gets run on the puppetmaster and not the client node.

I've closed the PR, going to have to find a different way of doing this.

[ekohl]

@andymillar I wonder if you could somehow cache the data and easily detect freshness.

[stbenjam]

yum is not called on every run, it tries rpmquery first, which puts no load on anything but the local host, and a very marginal amount.

yum also caches metadata for some time, so it handles the caching of the metadata for us. Is this causing a problem for you?

If you've got 10,000 hosts refreshing their yum metadata a few times a day, that is a lot of traffic. I'd just remove the yum command, honestly. I'm not sure why the yum as a last resort is there, because I'd think they'd all have rpmquery if they have yum.

It's puppet's design that all facts go everywhere without much way to control it. You can add a :confine to something like a Red Hat OS, which could solve some cases to avoid running these commands on an Ubuntu box, but otherwise I don't think you can do anything about this.

[stbenjam]

The amount of headaches this stupid pidfile location change has caused... 😠 Does it make mongo somehow more webscale when you just break everyone?

We could just strip out all the logic here - because puppetlabs-mongodb seems to partially handle the new improved mongo pidfile. The problem is that it does assume all fedora and >= EL7 operating systems use the new version.

RHEL does not, in Satellite. If we can get them to rebase on the newer mongo, then the >= EL 7 assumption would be (more) correct and this could go away.

[andymillar]

Better question, actually, might be why do we care what the mongodb pidfile is?

The pidfile passed to the puppetlabs-mongodb class is used to write the mongodb config file.

As far as I can tell, the difference in versions around 2.6.0 is the config file format and not where the pidfile is - just passing the pidfile location and not the version doesn't actually do what we want. It'll still have the same problems as if we don't pass the pidfile location.

To maintain the current behavior with puppetlabs-mongodb, the easiest solution might actually be to remove the version check in puppet-pulp and just pass class { '::mongodb::server': } and let the mongodb::server class work it out for us.

The only way that I can see of doing what you think you're doing by passing the pidfile location to mongodb is to specify mongodb::globals::version.

What am I missing?

Andy

[stbenjam]

Huh? mongodb.conf requires the pidfile to have the service even start correctly, and the solution to handle this problem in puppetlabs-mongodb isn't sufficient, as I explained in my previous comment why.

[andymillar]

Yes, mongodb.conf has to have a pidfile to start the service, why does it matter what it is?

The init script pulls the pidfilepath out of /etc/mongod.conf - so whatever is configured should work. This covers you for EL6.

Anything using EL7 or recent Fedora will be using systemd, which uses /var/run/mongodb/mongod.pid, which is what the puppetlabs-mongod module defaults to.

I still don't see why you have to pass the pidfilepath to it.

[iNecas]

What @stbenjam said, the puppet-mongod module has assumtions that are not valid for all our cases, therefore we need pass the param on our own

[stbenjam]

On el7, the pid file is in two locations - /etc/mongodb.conf and the service unit, as the service unit can't read the other config file.

This is the crux of the problem, if they have differing values, the service won't start on el7.

There's some historical reasons here:

I believe Fedora and EPEL long shipped their own service unit with the packaging, with the pid file being mongodb.pid The mongo folks finally got around to including their own, as part of https://jira.mongodb.org/browse/SERVER-7285, but called the PID mongod.pid. When this ended up in EPEL, this broke every person who had the old configuration in /etc/mongodb.conf.

Thus we introduced this fix.

I'd like to remove it too, but the assumptions puppetlabs-mongodb made are not representative of the whole world of mongo installations on el7.

If RHEL rebases in Satellite (I filed https://bugzilla.redhat.com/show_bug.cgi?id=1251950), it will be closer to reality, but it's still hard to say there won't be older mongos still out there even on users of CentOS + EPEL.

[andymillar]

Ok - that makes sense.

The real fix, probably, is then to configure mongodb.service in puppetlabs-mongodb to use $pidfilepath - but $pidfilepath has to match something that the downstream module installs.

If they configured mongodb.service as well, then this issue would go away and pulp wouldn't need to pass pidfilepath as the defaults would "just work".

I'll go see how receptive they are to the idea.

[stbenjam]

You can't and shouldn't really overwrite provided unit files, but yea, puppetlabs-mongodb could provide a systemd drop-in

[ekohl]

@stbenjam given there have been a few of these questions, maybe it's time for a reference document which can be linked to. It could even be #91 (comment) which we link to in the README.

[ekohl]

We've removed the custom fact for this.