Permalink
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Browse files
Don't write pid file when running under systemd
systemd does not require a pid file to track the rsyslogd process. By not writing a pid file, the rsyslog service can be locked down even further as it no longer needs write access to /var/run.
- Loading branch information
6fafe7cThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just noticed that with RHEL 7, installation of the new rsyslog.service file breaks the existing "/etc/logrotate.d/syslog" rotation rule. This rule uses the no longer created "pid" file to send the HUP to rsyslogd following log rotation. This change also breaks at least the "/etc/logrotate.d/haproxy" rule and perhaps others. At a minimum, the "syslog" file provided by the rsyslog rpm should also be updated to use "pkill" or something similar. You might want to consider reverting this commit.
6fafe7cThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The /etc/logrotate.d/syslog should use
systemctl kill -s HUP rsyslog.service6fafe7cThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, if you're only concerned with rsyslog package itself. But, there are many other packages that install log rotation rules which depend upon the pid file. (I've commented on an open issue, #2641, and am happy to move additional conversation there if that's a more appropriate venue....)
6fafe7cThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
many other packages such as? Can you provide a complete list?
6fafe7cThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
6fafe7cThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, an exhaustive list is beyond me. I can confirm that "haproxy" references the pid file. But, I'd bet there are numerous other 3rd party packages which use rsyslog for logging.
It'll be a noisier issue when the enterprise distros pick up the newer version of rsyslog.
Sometimes the ripple effect of a change isn't worth it. I agree with @DavidLang. I see no harm in reverting the commit and restoring the pid file.
6fafe7cThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the pid file is an internal implementation detail of rsyslog. I heavily doubt there are many 3rd party packages referencing it.
6fafe7cThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
using rsyslog for logging doesn't mean that 3rd party packages need to know about the pid file, fwiw
6fafe7cThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I checked codesearch.debian.net: https://codesearch.debian.net/search?q=%2Fvar%2Frun%2Frsyslog
(and index of the complete Debian archive
The list of package referencing /var/run/rsyslogd.pid is very short.
6fafe7cThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agree, 3rd party packages don't need to know about the file. But they have to adapt their product to accommodate a change which seems to me to have minimal value.
Did you also search /var/run/syslogd.pid (without the "r")? That's the name of the pid file on RHEL 7.
FWIW, while I remain in favor of just reverting this mod as it eliminates a source of consternation in the field, at a minimum, the "syslog" logrotate.d file needs to be updated as it is provided by the rsyslog package.
My concern is that others will install an updated rsyslog in the hope of correcting logging issues and introduce problems on their systems. And it's not immediately obvious that the source of logging issues may be an interaction with logrotate and the lack of the HUP being sent to rsyslog. (Note that your own log rotate rule directs stderr to /dev/null and returns "true".) The symptoms can be subtle and perhaps go undetected during an evaluation period prior to putting the new version into production.
6fafe7cThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree that the rhel package needs to be fixed in that regard.
There is rsyslog/rsyslog-pkg-rhel-centos#42 for that
6fafe7cThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1 and FYI: same occurs on Ubuntu 16.04 LTS.
Ubuntu stock install of logrotate does this:
postrotate invoke-rc.d rsyslog rotate >/dev/nullwhich relies on the pid file existing.
6fafe7cThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@friedl can you please check and update the logrotate scripts inside the packages. We should use the systemd method in any case. We also need to decide what's the best to use in our packages.