Do not use copytruncate for xcat log rotation

[Obihoernchen]

PR https://github.com/xcat2/xcat-core/pull/6510 tried to fix missing logs for goconserver, but also added copytruncate to all xcat logs in /var/log/xcat*.log. This is not needed because these logs are written by rsyslog and xcat itself, not goconserver.

The main rsyslog developer does not recommend to use copytruncate for rsyslog: https://serverfault.com/a/901366

For HA setups with logs on NFS etc. copytruncate can be very slow. A simple move/rename is way faster and cleaner.

I'm keeping delaycompress because I think it's nice to have some more days/weeks of logs uncompressed. Furthermore, some users might be used to it already.

[Obihoernchen]

Yes but this shouldn't be the case here because we do:

 test -f /var/run/rsyslogd.pid && kill -HUP `cat /var/run/rsyslogd.pid 2> /dev/null` 2> /dev/null || true

The official rsyslog-logrotate package is doing the same:

[root@service01 ~]# cat /etc/logrotate.d/rsyslog
/var/log/cron
/var/log/maillog
/var/log/messages
/var/log/secure
/var/log/spooler
{
    missingok
    sharedscripts
    postrotate
        /usr/bin/systemctl -s HUP kill rsyslog.service >/dev/null 2>&1 || true
    endscript
}

I tested the change and for me it's working fine. And before https://github.com/xcat2/xcat-core/pull/6510 there was no issue with xCAT logs either.

But please test on your system, too.

[samveen]

Given that rsyslog honors the HUP signal to close and reopen logs, copytruncate isn't required for it. The postrotate script just needs to ensure that the rsyslogd is correctly signaled about log rotation completion.

Ubuntu 20.04 uses the following:

samveen@prod-1:~$ grep '^VERSION=' /etc/os-release 
VERSION="20.04.6 LTS (Focal Fossa)"
samveen@prod-1:~$ grep postrotate -A2 /etc/logrotate.d/rsyslog 
        postrotate
                /usr/lib/rsyslog/rsyslog-rotate
        endscript
--
        postrotate
                /usr/lib/rsyslog/rsyslog-rotate
        endscript

Ubuntu 22.04 uses the following:

samveen@Ubuntu-2204-jammy-amd64-base ~ # grep '^VERSION=' /etc/os-release; grep -A2 postrotate /etc/logrotate.d/rsyslog 
VERSION="22.04.3 LTS (Jammy Jellyfish)"
        postrotate
                /usr/lib/rsyslog/rsyslog-rotate
        endscript

Debian 12 Bookworm uses the following:

samveen@zero2w:~/workspace/xcat-core $ grep '^VERSION=' /etc/os-release 
VERSION="12 (bookworm)"
samveen@zero2w:~/workspace/xcat-core $ grep -A2 postrotate /etc/logrotate.d/rsyslog 
        postrotate
                /usr/lib/rsyslog/rsyslog-rotate
        endscript

I'd suggest adding the debian/ubuntu check+execute into the postrotate section of xCAT/etc/logrotate.d/xcat , or I can create a PR for this change this is merged.

[Obihoernchen]

There is no /usr/lib/rsyslog/rsyslog-rotate on EL, therefore I would just stick to the current implementation of sending the HUP signal to /var/run/rsyslogd.pid. This should work on most OSs.

[samveen]

@Obihoernchen Yes that script doesn't exist for the RH based distributions. However:

• https://xcat-docs.readthedocs.io/en/stable/overview/support_matrix.html?highlight=matrix lists Ubuntu as a support platform
• Ubuntu 20.04 and 22.04 both do NOT have a rsyslog pid file in the run directory.
• There are already a list of conditional signaling commands in the postrotate at xCAT/etc/logrotate.d/xcat

  samveen@zero2w:~/workspace/xcat-core $ awk '/postrotate/{P=1} /endscript/{print;P=0} {if (P==1)print}'<xCAT/etc/logrotate.d/xcat
  postrotate
      test -f /var/run/rsyslogd.pid && kill -HUP `cat /var/run/rsyslogd.pid 2> /dev/null` 2> /dev/null || true
      test -f /var/run/syslogd.pid && kill -HUP `cat /var/run/syslogd.pid 2> /dev/null` 2> /dev/null || true
      test -f /var/run/xcat/cmdlogservice.pid && kill -HUP `cat /var/run/xcat/cmdlogservice.pid 2> /dev/null` 2> /dev/null || true
  endscript

What I am proposing is to add the following to the list in the postrotate above:

        test -x  /usr/lib/rsyslog/rsyslog-rotate && /usr/lib/rsyslog/rsyslog-rotate || true

This fixes a hole in Ubuntu support, as well as increases Debian compatibility. It's also possible that copytruncate was specifically added just because such signalling is missing on Ubuntu, thus leading to the error #6510 was trying to fix.

[Obihoernchen]

@samveen Very good point thank you! Added.

[samveen]

@Obihoernchen Trying to get closet to the tip than the base in Graham's Hierarchy of Disagreement :rofl: