From ce272320f45c88a5e66f4762a15fcf02ce7c217b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Piotrowski?= Date: Wed, 3 Jun 2015 09:17:19 +0200 Subject: [PATCH] Make log rotation more bulletproof - use flock to ensure that only one instance of fuel-logrotate is running - combine centos and ubuntu versions of fuel-logrotate to a single file - remove unused versions of fuel-logrotate file - enable delayed compression in system-wide logrotate.conf to avoid race condition between fuel-logrotate and regular logrotate - enable delaycompress also on master node - default minsize (30M) and maxsize (100M) in system-wide logrotate.conf - use file_line for logrotate.conf updates - use bash to execute fuel-logrotate - fix path to cron's logrotate script in logrotate-debug This commit changes default behavior of logrotate by enabling compress and delaycompress. The result is that compression is enabled for all log files managed by logrotate and rotated logs are compressed only on the next rotation. It allows us to resolve rotation cycle that might happen when both regular logrotate and fuel-logrotate were executed at the same time, causing both to rotate the same file. Closes-Bug: 1461400 Doc-Impact: ops guide Change-Id: I94c497a8916fa6fac87ea2ddc51f6de56ab73f98 Co-Authored-By: Alex Schultz (cherry picked from commit dd0eacc4606308c6ef54f86d2d6bf2c224a8c713) --- debian/fuel-misc.install | 2 +- debian/rules | 2 +- .../puppet/anacron/files/logrotate-debug | 3 +- deployment/puppet/openstack/files/logrotate | 21 -------- .../puppet/openstack/files/logrotate-ubuntu | 27 ---------- .../puppet/openstack/manifests/logrotate.pp | 54 +++++++++++-------- .../openstack/templates/10-fuel.conf.erb | 3 ++ files/fuel-misc/logrotate | 48 ++++++++++++----- files/fuel-misc/logrotate-ubuntu | 27 ---------- 9 files changed, 74 insertions(+), 113 deletions(-) delete mode 100644 deployment/puppet/openstack/files/logrotate delete mode 100644 deployment/puppet/openstack/files/logrotate-ubuntu delete mode 100644 files/fuel-misc/logrotate-ubuntu diff --git a/debian/fuel-misc.install b/debian/fuel-misc.install index 7b544bf2d2..f12961fafa 100644 --- a/debian/fuel-misc.install +++ b/debian/fuel-misc.install @@ -1,2 +1,2 @@ files/fuel-misc/haproxy-status.sh /usr/bin -files/fuel-misc/logrotate-ubuntu /usr/bin +files/fuel-misc/logrotate /usr/bin diff --git a/debian/rules b/debian/rules index bb818c6b30..e04b115065 100755 --- a/debian/rules +++ b/debian/rules @@ -11,6 +11,6 @@ override_dh_install: dh_install mv debian/fuel-ha-utils/usr/lib/ocf/resource.d/fuel/rabbitmq debian/fuel-ha-utils/usr/lib/ocf/resource.d/fuel/rabbitmq-server mv debian/fuel-ha-utils/usr/lib/ocf/resource.d/fuel/heat_engine_ubuntu debian/fuel-ha-utils/usr/lib/ocf/resource.d/fuel/heat-engine - mv debian/fuel-misc/usr/bin/logrotate-ubuntu debian/fuel-misc/usr/bin/fuel-logrotate + mv debian/fuel-misc/usr/bin/logrotate debian/fuel-misc/usr/bin/fuel-logrotate rm debian/fuel-ha-utils/usr/lib/ocf/resource.d/fuel/heat_engine_centos diff --git a/deployment/puppet/anacron/files/logrotate-debug b/deployment/puppet/anacron/files/logrotate-debug index 121dc76508..94d74db75f 100644 --- a/deployment/puppet/anacron/files/logrotate-debug +++ b/deployment/puppet/anacron/files/logrotate-debug @@ -2,5 +2,4 @@ SHELL=/bin/bash PATH=/sbin:/bin:/usr/sbin:/usr/bin MAILTO=root HOME=/ -10,20,30,40,50 * * * * root nice ionice -c3 /etc/cron.hourly/logrotate - +*/10 * * * * root nice ionice -c3 /usr/bin/fuel-logrotate diff --git a/deployment/puppet/openstack/files/logrotate b/deployment/puppet/openstack/files/logrotate deleted file mode 100644 index defad24158..0000000000 --- a/deployment/puppet/openstack/files/logrotate +++ /dev/null @@ -1,21 +0,0 @@ -#!/bin/sh -# managed by puppet -# -# Due to bug in logrotate, it always returns 0. Use grep for detect errors; -# exit code 1 is considered a success as no errors were found. - -if ! pgrep -x logrotate &>/dev/null; then - nice ionice -c3 /usr/sbin/logrotate /etc/logrotate.d/fuel.nodaily >& /tmp/logrotate - grep -q error /tmp/logrotate - EXITVALUE=$? -else - /usr/bin/logger -t logrotate "WARNING another logrotate instance is already running, exiting" - exit 1 -fi - -if [ $EXITVALUE != 1 ]; then - /usr/bin/logger -t logrotate "ALERT exited abnormally with [$EXITVALUE] (1 was expected)" - exit 1 -fi - -exit 0 diff --git a/deployment/puppet/openstack/files/logrotate-ubuntu b/deployment/puppet/openstack/files/logrotate-ubuntu deleted file mode 100644 index 4c58254137..0000000000 --- a/deployment/puppet/openstack/files/logrotate-ubuntu +++ /dev/null @@ -1,27 +0,0 @@ -#!/bin/sh -# managed by puppet -# Clean non existent log file entries from status file -cd /var/lib/logrotate -test -e status || touch status -head -1 status > status.clean -sed 's/"//g' status | while read logfile date -do - [ -e "$logfile" ] && echo "\"$logfile\" $date" -done >> status.clean -mv status.clean status -test -x /usr/sbin/logrotate || exit 0 - -if ! pgrep -x logrotate &>/dev/null; then - nice ionice -c3 /usr/sbin/logrotate /etc/logrotate.d/fuel.nodaily - EXITVALUE=$? -else - /usr/bin/logger -t logrotate "WARNING another logrotate instance is already running, exiting" - exit 1 -fi - -if [ $EXITVALUE != 0 ]; then - /usr/bin/logger -t logrotate "ALERT exited abnormally with [$EXITVALUE] (0 was expected)" -fi - -exit $EXITVALUE - diff --git a/deployment/puppet/openstack/manifests/logrotate.pp b/deployment/puppet/openstack/manifests/logrotate.pp index 304d15a67f..cc13a2528e 100644 --- a/deployment/puppet/openstack/manifests/logrotate.pp +++ b/deployment/puppet/openstack/manifests/logrotate.pp @@ -33,33 +33,43 @@ class openstack::logrotate ( debug => $debug, } - # Our custom cronjob overlaps with daily schedule, so we need to address it - exec { 'logrotate-tabooext': - command => 'sed -i "/^include/i tabooext + .nodaily" /etc/logrotate.conf', - path => [ '/bin', '/usr/bin' ], - onlyif => 'test -f /etc/logrotate.conf', - unless => 'grep -q tabooext /etc/logrotate.conf', + # TODO(aschultz): should move these to augeas when augeas is upgraded to + # >=1.4.0 because maxsize isn't supported until 1.4.0 which breaks everything. + File_line { + ensure => 'present', + path => '/etc/logrotate.conf', + } + # We're using after here to place these options above the include + # /etc/logrotate.d as file_line does not have a before option. + file_line { 'logrotate-tabooext': + line => 'tabooext + .nodaily', + match => '^tabooext', + after => '^create', + } -> + file_line { 'logrotate-compress': + line => 'compress', + match => '^compress', + after => '^tabooext', + } -> + file_line { 'logrotate-delaycompress': + line => 'delaycompress', + match => '^delaycompress', + after => '^compress', + } -> + file_line { 'logrotate-minsize': + line => "minsize ${minsize}", + match => '^minsize', + after => '^delaycompress', + } -> + file_line { 'logrotate-maxsize': + line => "maxsize ${maxsize}", + match => '^maxsize', + after => '^minsize', } - - # case $::osfamily { - # 'RedHat': { - # file { '/usr/bin/fuel-logrotate': - # mode => '0755', - # source => 'puppet:///modules/openstack/logrotate', - # } - # } - # 'Debian': { - # file { '/usr/bin/fuel-logrotate': - # mode => '0755', - # source => 'puppet:///modules/openstack/logrotate-ubuntu', - # } - # } - #} cron { 'fuel-logrotate': command => '/usr/bin/fuel-logrotate', user => 'root', minute => '*/30', - #require => File['/usr/bin/fuel-logrotate'], } } diff --git a/deployment/puppet/openstack/templates/10-fuel.conf.erb b/deployment/puppet/openstack/templates/10-fuel.conf.erb index eabb4ce12f..ae9900bbd8 100644 --- a/deployment/puppet/openstack/templates/10-fuel.conf.erb +++ b/deployment/puppet/openstack/templates/10-fuel.conf.erb @@ -37,6 +37,9 @@ # compress rotated files with gzip compress + # postpone compression to the next rotation + delaycompress + # ignore missing files missingok diff --git a/files/fuel-misc/logrotate b/files/fuel-misc/logrotate index 8d037f9eb1..283028866d 100644 --- a/files/fuel-misc/logrotate +++ b/files/fuel-misc/logrotate @@ -1,21 +1,45 @@ -#!/bin/sh -# managed by puppet -# -# Due to bug in logrotate, it always returns 0. Use grep for detect errors; -# exit code 1 is considered a success as no errors were found. +#!/bin/bash +lock() { + exec 903>/var/lock/fuel-logrotate + flock -n 903 && return 0 || return 1 +} -if ! pgrep -x logrotate >/dev/null 2>&1; then - nice ionice -c3 /usr/sbin/logrotate /etc/logrotate.d/fuel.nodaily >& /tmp/logrotate +unlock() { + flock -u 903 +} + +fail() { + if [ -z "$1" ] + then + MESSAGE="WARNING logrotate failed, no reason provided" + else + MESSAGE=$1 + fi + /usr/bin/logger -t logrotate "${MESSAGE}" + unlock + exit 1 +} + +lock || fail "WARNING logrotate flock failed, exiting" + +nice ionice -c3 /usr/sbin/logrotate /etc/logrotate.d/fuel.nodaily >& /tmp/logrotate +EXITVALUE=$? + +if [ -f /etc/redhat-release ] || [ -f /etc/centos-release ]; +then + # Due to bug in logrotate on centos/rhel, it always returns 0. Use grep for + # detect errors; exit code 1 is considered a success as no errors were + # found. grep -q error /tmp/logrotate EXITVALUE=$? + EXPECTEDVALUE=1 else - /usr/bin/logger -t logrotate "WARNING another logrotate instance is already running, exiting" - exit 1 + EXPECTEDVALUE=0 fi -if [ $EXITVALUE != 1 ]; then - /usr/bin/logger -t logrotate "ALERT exited abnormally with [$EXITVALUE] (1 was expected)" - exit 1 +if [ "${EXITVALUE}" != "${EXPECTEDVALUE}" ]; then + fail "ALERT exited abnormally with [${EXITVALUE}] (${EXPECTEDVALUE} was expected)" fi +unlock exit 0 diff --git a/files/fuel-misc/logrotate-ubuntu b/files/fuel-misc/logrotate-ubuntu deleted file mode 100644 index 8e2ae5e7b8..0000000000 --- a/files/fuel-misc/logrotate-ubuntu +++ /dev/null @@ -1,27 +0,0 @@ -#!/bin/sh -# managed by puppet -# Clean non existent log file entries from status file -cd /var/lib/logrotate -test -e status || touch status -head -1 status > status.clean -sed 's/"//g' status | while read logfile date -do - [ -e "$logfile" ] && echo "\"$logfile\" $date" -done >> status.clean -mv status.clean status -test -x /usr/sbin/logrotate || exit 0 - -if ! pgrep -x logrotate >/dev/null 2>&1; then - nice ionice -c3 /usr/sbin/logrotate /etc/logrotate.d/fuel.nodaily - EXITVALUE=$? -else - /usr/bin/logger -t logrotate "WARNING another logrotate instance is already running, exiting" - exit 1 -fi - -if [ $EXITVALUE != 0 ]; then - /usr/bin/logger -t logrotate "ALERT exited abnormally with [$EXITVALUE] (0 was expected)" -fi - -exit $EXITVALUE -