[RFC] Service restart and config reload should be two distinct things

[smortex]

The module currently change the restart parameter of the service resource to reload the configuration instead of restarting the daemon (line 16):

puppet-riemann/manifests/service.pp

Lines 13 to 19 in 6af7e72

	service { $riemann::service_name:
	ensure => running,
	enable => true,
	restart => $riemann::reload_command,
	hasstatus => true,
	hasrestart => true,
	}

This has the side effect that notifying the service in the end-user catalog will only reload the configuration and not restart the service as expected. When using mutual TLS authentication and rotating the riemann certificate, a config reload will not take into account the new certificates and the service will continue to use the old one that will eventually expire, leading to communication issues.

Proposal:

1. Revert the default behavior for the restart parameter to restart riemann completely;
2. Add a new resource (exec, with refreshonly => true) to handle the config reload only;
3. Notify this new resource instead of the service where applicable.

[faxm0dem]

I'm not sure I understand this fully.
Currently, the $riemann::reload command is mapped to :

./data/osfamily/FreeBSD/default.yaml:riemann::reload_command: /usr/sbin/service riemann reload
./data/osfamily/RedHat/8/default.yaml:riemann::reload_command: '/usr/bin/systemctl reload riemann'
./data/osfamily/RedHat/6/default.yaml:riemann::reload_command: '/sbin/service riemann reload'
./data/osfamily/RedHat/7/default.yaml:riemann::reload_command: 'kill -HUP $(</var/run/riemann.pid)'
./data/osfamily/Debian/default.yaml:riemann::reload_command: "/usr/sbin/service riemann reload"

So for RedHat/8 this maps to systemd / ExecReload=/bin/kill -HUP $MAINPID and it should do the right thing, should it not ?

[smortex]

it should do the right thing, should it not ?

It happens that Riemann does not reload certificates from disk on config reload.

In my case, syslog-ng could not re-connect to riemann because the presented certificate was expired (we renewed our certs, puppet installed it in the riemann config directory and notified the service but the process did not take that new certificate into account).

I could test it with the simple config used to setup riemann in riemann-ruby-client CI:

1. Setup Riemann following the CI recipe;
2. Run openssl s_client -connect '127.0.0.1:5554' to see the certificate used;
3. Re-generate a certificate;
4. Run systemctl reload riemann.service to reload the service
5. Run openssl s_client -connect '127.0.0.1:5554' to see that riemann still use the old certificate;
6. Run systemctl restart riemann.service
7. Run openssl s_client -connect '127.0.0.1:5554' to see that riemann now use the new certificate.

So any notification of the riemann service on certificate change will not result into the service taking this into account:

file { '/path/to/riemann/cert.crt':
  # ...
  notify => Service['riemann'], # does not work when reloading
}

[faxm0dem]

Ah, so you're saying riemann doesn't do the right thing when catching a HUP signal ?
If that's the case, we should (also) open an issue in riemann, shouldn't we ?