Page MenuHomePhabricator

Broken log rotation for many services (was nginx and varnishkafka on cpXXXX)
Closed, ResolvedPublic

Description

Thanks to ori that pointed out that cp3040 was finishing the space and the Nginx log was not rotated.

The logrotation of nginx and varnishkafka files on all (I guess) cpXXXX servers is broken since July 1st 2015 because the file permission on the logrotate files are wrong:

0 = root

root@cp1052:/var/log/nginx# ls -lna /etc/logrotate.d/
total 84
drwxr-xr-x   2   0   0  4096 Jan 29 15:48 .
drwxr-xr-x 115   0   0 12288 Feb 15 13:33 ..
-rw-r--r--   1   0   0   173 Feb 23  2015 apt
-rw-r--r--   1   0   0    79 Nov  8  2014 aptitude
-r--r--r--   1   0   0   289 Jul  1  2015 confd
-rw-r--r--   1   0   0   232 Feb 22  2015 dpkg
-rw-r--r--   1   0   0   146 Feb 17  2015 exim4-base
-rw-r--r--   1   0   0   126 Feb 17  2015 exim4-paniclog
-rw-r--r--   1   0   0   117 Jan 31  2014 graphite-carbon
-rw-r--r--   1   0   0   129 Mar  9  2015 logster
-rw-rw-r--   1 998 998   289 Mar 11  2015 nginx
-r--r--r--   1   0   0   396 Mar 11  2015 puppet
-rw-r--r--   1   0   0   515 Oct  2  2014 rsyslog
-r--r--r--   1   0   0   394 Jun 18  2015 salt-common
-rw-r--r--   1   0   0   338 Feb 26  2015 varnish
-rw-rw-r--   1 998 998   176 Mar 11  2015 varnishkafka
-rw-r--r--   1   0   0   222 Aug 31 15:33 varnishkafka-eventlogging-stats
-rw-r--r--   1   0   0   210 May 12  2015 varnishkafka-statsv-stats
-rw-r--r--   1   0   0   218 Mar 11  2015 varnishkafka-webrequest-stats
root@cp3040:~# logrotate --debug /etc/logrotate.d/nginx
Ignoring /etc/logrotate.d/nginx because of bad file mode.

Handling 0 logs

The symptoms is that the files are not rotating, space is filling up on the root partition and the log file are quite big (~3GB on some server):

root@cp3040:/etc/nginx# ls -larth /var/log/nginx/unified.error.log
-rw-r----- 1 www-data www-data 2.8G Feb 16 00:38 /var/log/nginx/unified.error.log

root@cp3040:/etc/nginx# ls -lah /var/log/varnishkafka.log
-rw-r----- 1 root adm 564M Feb  9 16:00 /var/log/varnishkafka.log

We should be careful with the fix, because just fixing the issue will cause ALL servers to rotate the log and start compressing a quite big existing log at the same time (cron.daily runs at 06:25) and might cause CPU overload across the fleet based on the usage around that time.

Event Timeline

Volans raised the priority of this task from to Needs Triage.
Volans updated the task description. (Show Details)
Volans added a project: SRE.
Volans subscribed.
Volans renamed this task from Broken logrotation for nginx and varnishkafka on cpXXXX to Broken log rotation for nginx and varnishkafka on cpXXXX.Feb 16 2016, 12:46 AM
Volans set Security to None.
Volans updated the task description. (Show Details)

Actually digging a bit we have a LOT of logrotate broken around, because by default seems that puppet create the files with owner:group = 998:998 if we don't force it in the puppet config (I basically don't know puppet, so I guess is the user:group used by puppet somewhere along the process [in the master maybe?]).

Another example:

root@ms-fe1002:~# ls -lah /var/log/swift-access.log
-rw-r----- 1 syslog adm 8.7G Feb 16 01:11 /var/log/swift-access.log

We should really add:

owner => 'root',
group => 'root',
mode => '0644'

in ALL the logrotate files added by puppet.

[Renaming the task to be more generic]

Volans renamed this task from Broken log rotation for nginx and varnishkafka on cpXXXX to Broken log rotation for many services (was nginx and varnishkafka on cpXXXX).Feb 16 2016, 1:19 AM

They need to be checked in each server groups to verify the issue, but from a quick

grep -rns -A10 "logrotate.d" * | view -

I think that this is the list of the broken ones, because they seems to not have owner/group set on puppet (I omitted the following lines):

manifests/role/eventlogging.pp:294:    file { '/etc/logrotate.d/eventlogging-files':
modules/eventlogging/manifests/init.pp:105:    file { '/etc/logrotate.d/eventlogging':
modules/geoip/manifests/data/maxmind.pp:92:  file { '/etc/logrotate.d/geoipupdate':
modules/icinga/manifests/init.pp:96:    file { '/etc/logrotate.d/icinga':
modules/phabricator/manifests/phd.pp:58:    file { '/etc/logrotate.d/phd':
modules/pybal/manifests/init.pp:26:    file { '/etc/logrotate.d/pybal':
modules/scap/manifests/l10nupdate.pp:105:    file { '/etc/logrotate.d/l10nupdate':
modules/swift/manifests/proxy.pp:42:    file { '/etc/logrotate.d/swift-proxy':
modules/tlsproxy/manifests/instance.pp:16:    file { '/etc/logrotate.d/nginx':

All the other looks to have the user/group set to root/root and mode to 0444, except modules/librenms/manifests/init.pp:52 that doesn't set the mode.

I think we should verify and fix it ASAP.

Reference to https://phabricator.wikimedia.org/T126616 for the SSL logging issue that is filling cpXXXX quickly.

In general we need to ensure that there is enough space in the servers to allow for the file compression (and 1 additional day of logs if delaycompress is set) before fixing the rotation to avoid to go out of space.
In those case we might need to truncate/move the files before fixing it.

I just checked pybal which was the most immediate risk: all files are small enough we're not at risk.

I think this is a classic case where having a define in puppet that does the right thing would be better.

Joe triaged this task as High priority.

Change 270895 had a related patch set uploaded (by Giuseppe Lavagetto):
logrotate: add centralized define, apply to pybal

https://gerrit.wikimedia.org/r/270895

Change 270895 merged by Giuseppe Lavagetto:
logrotate: add centralized define, apply to pybal

https://gerrit.wikimedia.org/r/270895

Change 270898 had a related patch set uploaded (by Giuseppe Lavagetto):
logrotate: convert nginx to logrotate::conf

https://gerrit.wikimedia.org/r/270898

Change 270900 had a related patch set uploaded (by Giuseppe Lavagetto):
logrotate: convert swift-proxy to logrotate::conf

https://gerrit.wikimedia.org/r/270900

Change 270901 had a related patch set uploaded (by Giuseppe Lavagetto):
logrotate: convert phd to logrotate::conf

https://gerrit.wikimedia.org/r/270901

Change 270898 merged by Giuseppe Lavagetto:
logrotate: convert nginx to logrotate::conf

https://gerrit.wikimedia.org/r/270898

Change 270900 merged by Giuseppe Lavagetto:
logrotate: convert swift-proxy to logrotate::conf

https://gerrit.wikimedia.org/r/270900

Change 270914 had a related patch set uploaded (by Volans):
logrotate: convert icinga to logrotate::conf

https://gerrit.wikimedia.org/r/270914

Change 270918 had a related patch set uploaded (by Volans):
logrotate: convert l10nupdate to logrotate::conf

https://gerrit.wikimedia.org/r/270918

Change 270923 had a related patch set uploaded (by Volans):
logrotate: Convert eventlogging-files to logrotate::conf

https://gerrit.wikimedia.org/r/270923

Change 270928 had a related patch set uploaded (by Volans):
logrotate: Convert eventlogging to logrotate::conf

https://gerrit.wikimedia.org/r/270928

Change 270930 had a related patch set uploaded (by Volans):
logrotate: Convert geoipupdate to logrotate::conf

https://gerrit.wikimedia.org/r/270930

Varnish ones to be fixed:

modules/varnishkafka/manifests/init.pp:36:    file { '/etc/logrotate.d/varnishkafka':
modules/varnishkafka/manifests/instance.pp:122:    file { "/etc/logrotate.d/varnishkafka-${name}-stats":

Change 270901 merged by Giuseppe Lavagetto:
logrotate: convert phd to logrotate::conf

https://gerrit.wikimedia.org/r/270901

Change 270914 merged by Volans:
logrotate: convert icinga to logrotate::conf

https://gerrit.wikimedia.org/r/270914

Change 270918 merged by Giuseppe Lavagetto:
logrotate: convert l10nupdate to logrotate::conf

https://gerrit.wikimedia.org/r/270918

Change 270923 merged by Giuseppe Lavagetto:
logrotate: Convert eventlogging-files to logrotate::conf

https://gerrit.wikimedia.org/r/270923

Change 270928 merged by Giuseppe Lavagetto:
logrotate: Convert eventlogging to logrotate::conf

https://gerrit.wikimedia.org/r/270928

Change 270930 merged by Giuseppe Lavagetto:
logrotate: Convert geoipupdate to logrotate::conf

https://gerrit.wikimedia.org/r/270930

confirmed working for swift, thanks @Volans @Joe !

ms-fe2001:~$ sudo ls -latr /var/log/swift-access.log* -h
-rw-r----- 1 syslog adm  44G Feb 18 06:25 /var/log/swift-access.log.1
-rw-r----- 1 syslog adm 184M Feb 18 09:08 /var/log/swift-access.log

Change 271505 had a related patch set uploaded (by Giuseppe Lavagetto):
logrotate: explicit ownership and permissions

https://gerrit.wikimedia.org/r/271505

Change 271505 merged by Giuseppe Lavagetto:
logrotate: explicit ownership and permissions

https://gerrit.wikimedia.org/r/271505