Archlinux service reload parameter is incorrect.

[gfokkema]

I first noticed that the initial postgresql used 'service postgresql reload' to reload itself, which caused an error as I'm using systemd.
However, nowhere in postgresql there's a reference to the 'service' command.

I haven't tested wheter deploying postgresql now works without manual intervention, but I have at least verified that the $service_provider value is not Undef.

[antaflos]

params.pp inherits from globals.pp where $service_provider is set to undef by default. You can set $postgresql::globals::service_provider yourself and it should be used in server/service.pp. The reload functionality is implemented in server/reload.pp and gets its actual command from params.pp. Both have little to do with one another and changing $service_provider does not affect the reload functionality.

But looking at params.pp I see that $service_provider should probably be set like this:

$service_provider           = $postgresql::globals::service_provider

Instead of:

$service_provider           = $service_provider

Although I am not sure if that is explicitly necessary, but it is in line with the way other variables in params.pp are populated from globals.pp.

However, your change doesn't make sense to me because params.pp now expects a top-scope variable ($::service_provider) and ignores $postgresql::globals::service_provider altogether.

Not sure what you are going for here, and I don't see how this affects your systemd's apparent inability to understand service postgresql reload (mine responds to this just fine on Ubuntu 15.10).

[gfokkema]

Thank you for that explanation.

The reason I thought it should be ::service_provider is that this specific fact is actually set by puppetlabs-stdlib.
So why would I need to manually specify this parameter, when puppetlabs-stdlib automatically defines it in the top scope?

After your explanation I'm not entirely sure that my fix is the correct one, but I'm pretty sure the value should be filled in automatically by the postgresql module (by reusing the value found by puppetlabs-std).

[antaflos]

You are right, puppetlabs-stdlib defines a fact named service_provider.

But I think the $service_provider variable of this module and the service_provider fact from puppetlabs-stdlib have "evolved" independently of one another (not sure though), and I don't think they should be confused in this module. There might, for example, be situations where a user of this module creates their own package of PostgreSQL which might use Upstart instead of systemd to start and stop PostgreSQL. In such a case the service_provider fact would be 'systemd' but the $service_provider variable would be set to 'upstart' by the user.

As I see it the "fix" should be to set

$service_provider           = $postgresql::globals::service_provider

in params.pp, then you can pass either the value of stdlib's fact service_provider to postgresql::globals::service_provider or set it yourself.

Examples (using Hiera):

postgresql::globals::service_provider: "%{::service_provider}"
# Alternatively:
postgresql::globals::service_provider: systemd

After your explanation I'm not entirely sure that my fix is the correct one, but I'm pretty sure the value should be filled in automatically by the postgresql module (by reusing the value found by puppetlabs-std).

I am not sure why this should be necessary. This module usually does The Right Thing for each OS and distribution and I have not yet come across a situation where have had to touch the $service_provider variable at all.

I guess I still don't understand your original problem. Your Puppet-managed PostgreSQL instance cannot be reloaded because your systemd rejects the service postgresql reload call? What OS and PostgreSQL version are you using?

[gfokkema]

I guess I still don't understand your original problem. Your Puppet-managed PostgreSQL instance cannot be reloaded because your systemd rejects the service postgresql reload call? What OS and PostgreSQL version are you using?

Yes indeed.
Since I'm using systemd and not upstart, there is no binary called service on my system.
Therefore the command service postgresql reload results in an error.

According to README.md of this repository:

service_provider

Overrides the default PostgreSQL service provider. Default: OS dependent.

What you're saying contradicts the readme.
service_provider should be filled by OS dependent values, unless otherwise specified.
Right now service_provider is undef unless otherwise specified.

This isn't as intended / documented.

[antaflos]

I think I understand now. What OS (distribution, version) are you using?

The reload command does not use the $service_provider variable at all. Instead the reload command is defined by the $service_reload parameter which you can set via the class parameter postgresql::server::service_reload. You would probably set it to systemd reload postgresql (not sure if that is the correct unit name) which might be the solution to your immediate problem.

The real problem seems to be that you are apparently using an OS that is not supported by this module. When you look at params.pp you can see that the $service_reload variable is set correctly for many different operating systems, almost always using the service command. What OS are you using that doesn't provide the service wrapper command?

[gfokkema]

Sorry, forgot to mention the OS.
I'm on Archlinux, and looking at the file params.pp there is indeed an error in the value of the $service_reload variable:
params.pp

Arch Linux has been using systemd for quite some time, so the correct fix should indeed be to change this value as you suggested.

[gfokkema]

I've corrected the Archlinux specific setting. Thank you for your explanation.

[HelenCampbell]

Hey @gfokkema and @antaflos , I have changed the name of this PR to better describe the problem you encountered. I'll merge this in now. Thank you @antaflos for all your help in figuring out the issue here :)