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

systemd rsyslog service does not create PID file, that is used by logrotate scripts for SIGHUP #79

Closed
selivan opened this issue May 25, 2018 · 34 comments

Comments

@selivan
Copy link

selivan commented May 25, 2018

OS: Ubuntu 16.04 Xenial
rsyslog package: 8.34.0-0adiscon2xenial1 from ppa:adiscon/v8-stable

Logrotate scripts often use invoke-rc.d rsyslog rotate to tell rsyslog to close all open files. /etc/init.d/rsyslog script expects PID file to be /var/run/rsyslogd.pid.

Systemd script launches rsyslog with this arguments: ExecStart=/usr/sbin/rsyslogd -n -iNONE. PID file is not created. Logrotate is broken.

To fix this, systemd rsyslog script should launch it like that:

ExecStart=/usr/sbin/rsyslogd -n -i/var/run/rsyslogd.pid

@selivan selivan changed the title systemd rsyslog service does not create pid file, that is used by logrotate scripts for SIGHUP systemd rsyslog service does not create PID file, that is used by logrotate scripts for SIGHUP May 25, 2018
@selivan
Copy link
Author

selivan commented May 25, 2018

systemd script is inside rsyslog source, here is the PR: rsyslog/rsyslog#2739

@davidelang
Copy link

davidelang commented May 27, 2018 via email

@selivan
Copy link
Author

selivan commented May 27, 2018

@davidelang How are logrotate scripts supposed to send SIGHUP then?

@davidelang
Copy link

davidelang commented May 28, 2018 via email

@selivan
Copy link
Author

selivan commented May 28, 2018

@davidelang Here is what stock rsyslog in Ubuntu Xenial do:

root@ubuntu-2gb-nbg1-dc3-1 ~ # dpkg -l | grep rsyslog
ii  rsyslog                          8.16.0-1ubuntu3                            amd64        reliable system and kernel logging daemon
root@ubuntu-2gb-nbg1-dc3-1 ~ # systemctl cat rsyslog.service  | grep Exec
ExecStart=/usr/sbin/rsyslogd -n
root@ubuntu-2gb-nbg1-dc3-1 ~ # cat /etc/logrotate.d/rsyslog  | grep -A2 postrotate
	postrotate
		invoke-rc.d rsyslog rotate > /dev/null
	endscript
--
	postrotate
		invoke-rc.d rsyslog rotate > /dev/null
	endscript

I suggest to have PID file for interaction with rsyslog process in upstream. It won't hurt anybody. killall is sloppy approach, for example it may affect lxc containers also running rsyslog like main system.

Now if you install rsyslog from ppa:adiscon/v8-stable on Ubuntu Xenial, logrotate is broken.

friedl added a commit that referenced this issue Jun 15, 2018
@friedl
Copy link
Contributor

friedl commented Jun 15, 2018

We have just published a new revision of the packages. The new ones are now creating the pid file again on service startup. This should also solve the logrotate issue.

@wsldankers
Copy link

Why not systemctl kill --kill-who=main --signal=HUP rsyslog?

@rgerhards
Copy link
Member

Why not systemctl kill --kill-who=main --signal=HUP rsyslog?

There was a lot of complaints on the rsyslog mailing list (see archive) that other programs need the pid file as well.

@mbiebl
Copy link

mbiebl commented Apr 29, 2019

pid files are not needed under systemd and should be avoided if possible. The official Debian package does not need a pid file and uses -iNONE and systemctl kill -s HUP rsyslog.service in the logrotate postrotate. I'm not aware of other programs needing the rsyslog pid file.

@selivan
Copy link
Author

selivan commented Apr 29, 2019

@mbiebl
Configurations that use imfile to forward log files with rsyslog may rely on pid file. Right now rsyslog 8.1904.0-0adiscon1bionic1 from official ppa comes with logrotate file using invoke-rc.d rsyslog rotate, which uses pid file. Syslog daemon is core system software, it should keep compatibility with old configurations as long as possible.

@mbiebl
Copy link

mbiebl commented Apr 29, 2019

Configurations that use imfile to forward log files with rsyslog may rely on pid file.

How so?

@selivan
Copy link
Author

selivan commented Apr 29, 2019

@mbiebl

My mistake, files rotated externally do not need it. OK, then software writing to syslog, something like this:

/etc/rsyslog.d/foobar.conf:

if( $syslogtag == 'foobar:')  then {
        action(type="omfile" file="/var/log/foobar/main.log" template="OnlyMsg")
        stop
}

/etc/logrotate.d/foobar.conf:

/var/log/foobar/*.log {
        daily
        rotate 7
        sharedscripts
        postrotate
            [ -s /run/rsyslogd.pid ] && kill -HUP `cat /run/rsyslogd.pid`
        endscript
}

@mbiebl
Copy link

mbiebl commented Apr 29, 2019

Using systemctl kill -s HUP rsyslog.service as I mentioned earlier is a much more reliable way to trigger a reload under systemd. And as said, the whole Debian archive ships no single 3rd party package which require rsyslogd.pid (checked via codesearch.debian.net)

@davidelang
Copy link

davidelang commented Apr 29, 2019 via email

@mbiebl
Copy link

mbiebl commented Apr 29, 2019

@davidelang Can you point to such reports?

@mbiebl
Copy link

mbiebl commented Apr 29, 2019

I'm asking because Debian is a rather conservative distro and I have the pid file disabled there for over a year now without a single bug report.

@selivan
Copy link
Author

selivan commented Apr 29, 2019

I am not saying to use it in new configuration, I am saying there are a lot of productions systems using this already, together with official rsyslog packages from ppa, and people won't be happy to suddenly get broken system. Or upgrade to new distro version, that got rid of pid file, and suddenly stumble over broken configuration.

Again, you say it's reliable? What if rsyslog launches additional processes, with mmexternal for example or some output plugins that require it? Are this processes going to get the signal too or just the main process?

Backward compatibility is important, system core software should keep it as long as possible.

@mbiebl
Copy link

mbiebl commented Apr 29, 2019

Again, you say it's reliable? What if rsyslog launches additional processes, with mmexternal for example or some output plugins that require it? Are this processes going to get the signal too or just the main process?

If you use [ -s /run/rsyslogd.pid ] && kill -HUP $(cat /run/rsyslogd.pid) those child processes don't get the HUP signal, do they?

@mbiebl
Copy link

mbiebl commented Apr 29, 2019

Really, pid files are a terrible idea. The less there are of them, the better.

@selivan
Copy link
Author

selivan commented Apr 29, 2019

f you use [ -s /run/rsyslogd.pid ] && kill -HUP $(cat /run/rsyslogd.pid) those child processes don't get the HUP signal, do they?

That's the point, this processes should not get the signal, they might do unexpected things. Signal is for the main process only.

Really, pid files are a terrible idea. The less there are of them, the better.

It's called backward compatibility, it should be kept as long as possible.

@mbiebl
Copy link

mbiebl commented Apr 29, 2019

f you use [ -s /run/rsyslogd.pid ] && kill -HUP $(cat /run/rsyslogd.pid) those child processes don't get the HUP signal, do they?

That's the point, this processes should not get the signal, they might do unexpected things. Signal is for the main process only.

If you want to send a signal to the main process only, that is easily doable with systemctl.

Really, pid files are a terrible idea. The less there are of them, the better.

It's called backward compatibility, it should be kept as long as possible.

I don't agree. Sometimes bad practices should not be prolonged unnecessarily.

@davidelang
Copy link

davidelang commented Apr 29, 2019 via email

@mbiebl
Copy link

mbiebl commented Apr 29, 2019

On Mon, 29 Apr 2019, Michael Biebl wrote: I'm asking because Debian is a rather conservative distro and I have the pid file disabled there for over a year now without a single bug report.
note that not everyone uses the latest rsyslog build on the latest versions of the distros. I would have to go back through the mailing list to see the bug reports, but there were a bunch of them, which is why we reverted back to creating the PID file by default.

I'd be honestly interested in such bug reports, because the only breakage I'm aware of is in the Ubuntu rsyslog package. They messed up the rsyslog package by not updating the logrotate script when they stopped creating a PID file.

not all distros use systemd (OpenWRT is a significant one that doesn't)

Irrelevant here as this change discussed here is specifically for distros using systemd.

@davidelang
Copy link

davidelang commented Apr 29, 2019 via email

@mbiebl
Copy link

mbiebl commented Apr 29, 2019

How is this not relevant? you are asking us to change the default for compiling rsyslog, aren't you? A change of the default will affect all distros.

This here is for rsyslog-pkg-ubuntu, i.e. rsyslog packages that are provided by Adiscon for Ubuntu.
I'm saying that https://github.com/rsyslog/rsyslog-pkg-ubuntu/blob/master/rsyslog/cosmic/master/debian/patches/02-rsyslog-systemd.patch is a bad idea and should be dropped.

@mbiebl
Copy link

mbiebl commented Apr 29, 2019

to be specific: The rsylogd.pid part of that patch.
Then again, I'm no fan of /etc/default files either, so probably the whole patch could go.

@selivan
Copy link
Author

selivan commented Apr 29, 2019

How is this not relevant? you are asking us to change the default for compiling rsyslog, aren't you? A change of the default will affect all distros.

This here is for rsyslog-pkg-ubuntu, i.e. rsyslog packages that are provided by Adiscon for Ubuntu.
I'm saying that https://github.com/rsyslog/rsyslog-pkg-ubuntu/blob/master/rsyslog/cosmic/master/debian/patches/02-rsyslog-systemd.patch is a bad idea and should be dropped.

This will break at least one thing in my production configuration, I posted simplified version in upper comments. It will probably break somebody's else configuration. And it will even break the ppa rsyslog package itself, because it comes with logrotate config using invoke-rc.d rsyslog rotate.

Oh yeah, and it will make harder for people to maintain configurations working both on Linux and *BSD, which does not use systemd.

And I completely miss the whole point of making this calamity.

@mbiebl
Copy link

mbiebl commented Apr 29, 2019

If you don't know why pid files are bad, I suggest you read up on that topic.

@davidelang
Copy link

davidelang commented Apr 29, 2019 via email

@mbiebl
Copy link

mbiebl commented Apr 29, 2019

Why are pid files so bad that it's worth breaking anyone's systems who are still configured to use them?

I'd argue that the upstream defaults should be sane. If there are user setups which need a pid file for whatever reason, they are free to configure their system this way.

@mbiebl
Copy link

mbiebl commented Apr 29, 2019

(or better, update their configuration to not rely on pid files)

@rgerhards
Copy link
Member

rgerhards commented Apr 30, 2019

I don't like to go into systemd politics. This here is the repository Adiscon maintains primarily for it's customers - those folks that pay to make it happen. So in contrast to the main project we have a very clear focus here: whoever pays decides how things should be. Our customers have clearly told us they want the pid file. And so the patch will remain.

I do not say this is the best way to do it. I say this is the way folks pay us to do it.

@rgerhards
Copy link
Member

I should also mention that I personally tend to say that "no pid file" is a good idea. But from a pragmatical PoV I really don't care that much and avoid hassle if possible.

@atc0005
Copy link
Contributor

atc0005 commented May 3, 2019

Sorry for adding any noise, but I had to use rsyslog's "debug on demand" feature today and noticed that the docs reference use of a pid file to determine the current process id:

https://www.rsyslog.com/doc/v8-stable/troubleshooting/debug.html#getting-debug-information-from-a-running-instance

Example:

kill -USR1 $(cat /var/run/rsyslogd.pid)

Intent of this post:

Note that if the PID files are not created by default then that example would need to be updated to use a different approach of retrieving the PID.

openstack-mirroring pushed a commit to openstack/openstack-ansible-rsyslog_server that referenced this issue May 15, 2020
The use of pidfiles seems to be one which causes much debate [1] and
debain buster no longer creates a pidfile for rsyslog so the tests
in this role which rely on it are broken.

Given that systemd now knows the process pid and more distros are
likley to drop the use of the pidfile, and also that the rsyslog
functionality in OSA is largely replaced with systemd journal
remote forwarding, we can remove these tests that break without a
pidfile.

[1] rsyslog/rsyslog-pkg-ubuntu#79

Change-Id: Ia2be525c75fb8bbc549cbc3aa53b3c0db05002fa
openstack-mirroring pushed a commit to openstack/openstack that referenced this issue May 15, 2020
* Update openstack-ansible-rsyslog_server from branch 'master'
  - Drop tests that rely on the presence of a pidfile
    
    The use of pidfiles seems to be one which causes much debate [1] and
    debain buster no longer creates a pidfile for rsyslog so the tests
    in this role which rely on it are broken.
    
    Given that systemd now knows the process pid and more distros are
    likley to drop the use of the pidfile, and also that the rsyslog
    functionality in OSA is largely replaced with systemd journal
    remote forwarding, we can remove these tests that break without a
    pidfile.
    
    [1] rsyslog/rsyslog-pkg-ubuntu#79
    
    Change-Id: Ia2be525c75fb8bbc549cbc3aa53b3c0db05002fa
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

7 participants