From 2958c56e934f279cd2c60388fcc4252622938e0e Mon Sep 17 00:00:00 2001
From: Michael Hanselmann <hansmi@google.com>
Date: Thu, 27 Sep 2012 17:23:38 +0200
Subject: [PATCH] ganeti-cleaner: Separate queue cleaning code

This code does not need to run as root, therefore it's better to split
it out. It is now run with the same permissions as the master daemon.

Signed-off-by: Michael Hanselmann <hansmi@google.com>
Reviewed-by: Iustin Pop <iustin@google.com>
---
 .gitignore                        |  1 +
 Makefile.am                       |  9 ++++-
 daemons/ganeti-cleaner.in         | 14 -------
 daemons/ganeti-master-cleaner.in  | 67 +++++++++++++++++++++++++++++++
 doc/examples/ganeti.cron.in       |  5 ++-
 lib/tools/ensure_dirs.py          |  4 ++
 man/ganeti-cleaner.rst            | 11 +++--
 man/ganeti-master-cleaner.rst     | 27 +++++++++++++
 test/ganeti-cleaner_unittest.bash | 47 ++++++++++++++++++++--
 9 files changed, 158 insertions(+), 27 deletions(-)
 create mode 100644 daemons/ganeti-master-cleaner.in
 create mode 100644 man/ganeti-master-cleaner.rst

diff --git a/.gitignore b/.gitignore
index ff3a68f51..e843313ad 100644
--- a/.gitignore
+++ b/.gitignore
@@ -40,6 +40,7 @@
 # daemons
 /daemons/daemon-util
 /daemons/ganeti-cleaner
+/daemons/ganeti-master-cleaner
 /daemons/ganeti-confd
 /daemons/ganeti-masterd
 /daemons/ganeti-noded
diff --git a/Makefile.am b/Makefile.am
index cdad55bcb..88fd007f2 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -154,6 +154,7 @@ CLEANFILES = \
 	$(SHELL_ENV_INIT) \
 	daemons/daemon-util \
 	daemons/ganeti-cleaner \
+	daemons/ganeti-master-cleaner \
 	devel/upload \
 	$(BUILT_EXAMPLES) \
 	doc/examples/bash_completion \
@@ -652,7 +653,8 @@ dist_sbin_SCRIPTS = \
 
 nodist_sbin_SCRIPTS = \
 	$(PYTHON_BOOTSTRAP_SBIN) \
-	daemons/ganeti-cleaner
+	daemons/ganeti-cleaner \
+	daemons/ganeti-master-cleaner
 
 if HS_CONFD
 nodist_sbin_SCRIPTS += htools/hconfd
@@ -719,6 +721,7 @@ EXTRA_DIST = \
 	$(RUN_IN_TEMPDIR) \
 	daemons/daemon-util.in \
 	daemons/ganeti-cleaner.in \
+	daemons/ganeti-master-cleaner.in \
 	$(pkglib_python_scripts) \
 	devel/upload.in \
 	tools/kvm-ifup.in \
@@ -754,6 +757,7 @@ EXTRA_DIST = \
 man_MANS = \
 	man/ganeti.7 \
 	man/ganeti-cleaner.8 \
+	man/ganeti-master-cleaner.8 \
 	man/ganeti-confd.8 \
 	man/ganeti-listrunner.8 \
 	man/ganeti-masterd.8 \
@@ -1047,7 +1051,8 @@ pep8_python_code = \
 
 test/daemon-util_unittest.bash: daemons/daemon-util
 
-test/ganeti-cleaner_unittest.bash: daemons/ganeti-cleaner
+test/ganeti-cleaner_unittest.bash: daemons/ganeti-cleaner \
+  daemons/ganeti-master-cleaner
 
 test/bash_completion.bash: doc/examples/bash_completion-debug
 
diff --git a/daemons/ganeti-cleaner.in b/daemons/ganeti-cleaner.in
index cbc065967..c5e99d7dd 100644
--- a/daemons/ganeti-cleaner.in
+++ b/daemons/ganeti-cleaner.in
@@ -26,25 +26,12 @@ set -e -u
 : ${CHECK_CERT_EXPIRED:=$PKGLIBDIR/check-cert-expired}
 
 readonly CLEANER_LOG_DIR=$LOG_DIR/cleaner
-readonly QUEUE_ARCHIVE_DIR=$DATA_DIR/queue/archive
 readonly CRYPTO_DIR=$RUN_DIR/crypto
 
 in_cluster() {
   [[ -e $DATA_DIR/ssconf_master_node ]]
 }
 
-cleanup_master() {
-  # Return if machine is not part of a cluster
-  in_cluster || return 0
-
-  # Return if queue archive directory doesn't exist
-  [[ -d $QUEUE_ARCHIVE_DIR ]] || return 0
-
-  # Remove old jobs
-  find $QUEUE_ARCHIVE_DIR -mindepth 2 -type f -mtime +$REMOVE_AFTER -print0 | \
-  xargs -r0 rm -vf
-}
-
 cleanup_node() {
   # Return if directory for crypto keys doesn't exist
   [[ -d $CRYPTO_DIR ]] || return 0
@@ -90,7 +77,6 @@ echo "Cleaner started at $(date)"
 find $CLEANER_LOG_DIR -maxdepth 1 -type f | sort | head -n -$KEEP_LOGS | \
 xargs -r rm -vf
 
-cleanup_master
 cleanup_node
 cleanup_watcher
 
diff --git a/daemons/ganeti-master-cleaner.in b/daemons/ganeti-master-cleaner.in
new file mode 100644
index 000000000..3130d95d1
--- /dev/null
+++ b/daemons/ganeti-master-cleaner.in
@@ -0,0 +1,67 @@
+#!/bin/bash
+#
+
+# Copyright (C) 2009, 2010, 2011, 2012 Google Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful, but
+# WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+# General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
+# 02110-1301, USA.
+
+set -e -u
+
+@SHELL_ENV_INIT@
+
+readonly CLEANER_LOG_DIR=$LOG_DIR/master-cleaner
+readonly QUEUE_ARCHIVE_DIR=$DATA_DIR/queue/archive
+
+in_cluster() {
+  [[ -e $DATA_DIR/ssconf_master_node ]]
+}
+
+cleanup_master() {
+  # Return if machine is not part of a cluster
+  in_cluster || return 0
+
+  # Return if queue archive directory doesn't exist
+  [[ -d $QUEUE_ARCHIVE_DIR ]] || return 0
+
+  # Remove old jobs
+  find $QUEUE_ARCHIVE_DIR -mindepth 2 -type f -mtime +$REMOVE_AFTER -print0 | \
+  xargs -r0 rm -vf
+}
+
+# Define how many days archived jobs should be left alone
+REMOVE_AFTER=21
+
+# Define how many log files to keep around (usually one per day)
+KEEP_LOGS=50
+
+# Log file for this run
+LOG_FILE=$CLEANER_LOG_DIR/cleaner-$(date +'%Y-%m-%dT%H_%M').$$.log
+
+# Create log directory
+mkdir -p $CLEANER_LOG_DIR
+
+# Redirect all output to log file
+exec >>$LOG_FILE 2>&1
+
+echo "Cleaner started at $(date)"
+
+# Remove old cleaner log files
+find $CLEANER_LOG_DIR -maxdepth 1 -type f | sort | head -n -$KEEP_LOGS | \
+xargs -r rm -vf
+
+cleanup_master
+
+exit 0
diff --git a/doc/examples/ganeti.cron.in b/doc/examples/ganeti.cron.in
index 6d7d9b052..62705a229 100644
--- a/doc/examples/ganeti.cron.in
+++ b/doc/examples/ganeti.cron.in
@@ -4,4 +4,7 @@ PATH=/sbin:/bin:/usr/sbin:/usr/bin:/usr/local/sbin:/usr/local/bin
 */5 * * * * root [ -x @SBINDIR@/ganeti-watcher ] && @SBINDIR@/ganeti-watcher
 
 # Clean job archive (at 01:45 AM)
-45 1 * * * root [ -x @SBINDIR@/ganeti-cleaner ] && @SBINDIR@/ganeti-cleaner
+45 1 * * * @GNTMASTERUSER@ [ -x @SBINDIR@/ganeti-master-cleaner ] && @SBINDIR@/ganeti-master-cleaner
+
+# Clean job archive (at 02:45 AM)
+45 2 * * * @GNTNODEDUSER@ [ -x @SBINDIR@/ganeti-cleaner ] && @SBINDIR@/ganeti-cleaner
diff --git a/lib/tools/ensure_dirs.py b/lib/tools/ensure_dirs.py
index 7ac5600fb..09655ccd2 100644
--- a/lib/tools/ensure_dirs.py
+++ b/lib/tools/ensure_dirs.py
@@ -124,6 +124,8 @@ def GetPaths():
   rapi_log = constants.DAEMONS_LOGFILES[constants.RAPI]
 
   rapi_dir = os.path.join(pathutils.DATA_DIR, "rapi")
+  cleaner_log_dir = os.path.join(pathutils.LOG_DIR, "cleaner")
+  master_cleaner_log_dir = os.path.join(pathutils.LOG_DIR, "master-cleaner")
 
   paths = [
     (pathutils.DATA_DIR, DIR, 0755, getent.masterd_uid,
@@ -192,6 +194,8 @@ def GetPaths():
     (rapi_log, FILE, 0600, getent.rapi_uid, getent.masterd_gid, False),
     (pathutils.LOG_OS_DIR, DIR, 0750, getent.masterd_uid,
      getent.daemons_gid),
+    (cleaner_log_dir, DIR, 0750, getent.noded_uid, getent.noded_gid),
+    (master_cleaner_log_dir, DIR, 0750, getent.masterd_uid, getent.masterd_gid),
     ])
 
   return paths
diff --git a/man/ganeti-cleaner.rst b/man/ganeti-cleaner.rst
index ca37a5f0f..600e2b98c 100644
--- a/man/ganeti-cleaner.rst
+++ b/man/ganeti-cleaner.rst
@@ -14,13 +14,12 @@ Synopsis
 DESCRIPTION
 -----------
 
-The **ganeti-cleaner** is a periodically run script to clean old job
-files from the job queue archive and to remove expired X509
-certificates and keys.
+The **ganeti-cleaner** is a periodically run script to remove expired
+X509 certificates and keys, as well as outdated **ganeti-watcher**
+information.
 
-**ganeti-cleaner** automatically removes all files older than 21 days
-from ``@LOCALSTATEDIR@/lib/ganeti/queue/archive`` and all expired
-certificates and keys from ``@LOCALSTATEDIR@/run/ganeti/crypto``.
+**ganeti-cleaner** automatically removes  all expired certificates and
+keys from ``@LOCALSTATEDIR@/run/ganeti/crypto``.
 
 .. vim: set textwidth=72 :
 .. Local Variables:
diff --git a/man/ganeti-master-cleaner.rst b/man/ganeti-master-cleaner.rst
new file mode 100644
index 000000000..dbb06b559
--- /dev/null
+++ b/man/ganeti-master-cleaner.rst
@@ -0,0 +1,27 @@
+ganeti-master-cleaner(8) Ganeti | Version @GANETI_VERSION@
+==========================================================
+
+Name
+----
+
+ganeti-master-cleaner - Ganeti job queue cleaner
+
+Synopsis
+--------
+
+**ganeti-master-cleaner**
+
+DESCRIPTION
+-----------
+
+The **ganeti-master-cleaner** is a periodically run script to clean old
+job files from the job queue archive.
+
+**ganeti-master-cleaner** automatically removes all files older than 21
+days from ``@LOCALSTATEDIR@/lib/ganeti/queue/archive``.
+
+.. vim: set textwidth=72 :
+.. Local Variables:
+.. mode: rst
+.. fill-column: 72
+.. End:
diff --git a/test/ganeti-cleaner_unittest.bash b/test/ganeti-cleaner_unittest.bash
index 93bf5f4e4..12b31de24 100755
--- a/test/ganeti-cleaner_unittest.bash
+++ b/test/ganeti-cleaner_unittest.bash
@@ -24,6 +24,7 @@ set -o pipefail
 export PYTHON=${PYTHON:=python}
 
 GNTC=daemons/ganeti-cleaner
+GNTMC=daemons/ganeti-master-cleaner
 CCE=tools/check-cert-expired
 
 err() {
@@ -44,8 +45,13 @@ gencert() {
 }
 
 check_logfiles() {
-  local n=$1
-  [[ "$(find $tmpls/log/ganeti/cleaner -mindepth 1 | wc -l)" -le "$n" ]] || \
+  local n=$1 p=$2 path
+  if [[ "$p2" = master ]]; then
+    path=$tmpls/log/ganeti/mastercleaner
+  else
+    path=$tmpls/log/ganeti/cleaner
+  fi
+  [[ "$(find $path -mindepth 1 | wc -l)" -le "$n" ]] || \
     err "Found more than $n logfiles"
 }
 
@@ -77,7 +83,15 @@ count_and_check_certs() {
 }
 
 run_cleaner() {
-  CHECK_CERT_EXPIRED=$CCE LOCALSTATEDIR=$tmpls $GNTC
+  local cmd
+
+  if [[ "$1" = master ]]; then
+    cmd=$GNTMC
+  else
+    cmd=$GNTC
+  fi
+
+  CHECK_CERT_EXPIRED=$CCE LOCALSTATEDIR=$tmpls $cmd
 }
 
 create_archived_jobs() {
@@ -162,10 +176,25 @@ run_cleaner
 test -d $tmpls/log/ganeti/cleaner || err 'log/ganeti/cleaner should exist'
 check_logfiles 1
 
-upto 'Checking number of retained log files'
+test -d $tmpls/log/ganeti/master-cleaner && \
+  err 'log/ganeti/master-cleaner should not exist yet'
+run_cleaner master
+test -d $tmpls/log/ganeti/master-cleaner || \
+  err 'log/ganeti/master-cleaner should exist'
+check_logfiles 1 master
+
+upto 'Checking number of retained log files (master)'
+for (( i=0; i < (maxlog + 10); ++i )); do
+  run_cleaner master
+  check_logfiles 1
+  check_logfiles $(( (i + 2) > $maxlog?$maxlog:(i + 2) )) master
+done
+
+upto 'Checking number of retained log files (node)'
 for (( i=0; i < (maxlog + 10); ++i )); do
   run_cleaner
   check_logfiles $(( (i + 2) > $maxlog?$maxlog:(i + 2) ))
+  check_logfiles $maxlog master
 done
 
 upto 'Removal of archived jobs (non-master)'
@@ -175,12 +204,16 @@ test -f $tmpls/lib/ganeti/ssconf_master_node && \
   err 'ssconf_master_node should not exist'
 run_cleaner
 count_jobs 55
+run_cleaner master
+count_jobs 55
 
 upto 'Removal of archived jobs (master node)'
 create_archived_jobs
 count_jobs 55
 echo $HOSTNAME > $tmpls/lib/ganeti/ssconf_master_node
 run_cleaner
+count_jobs 55
+run_cleaner master
 count_jobs 31
 
 upto 'Certificate expiration'
@@ -191,16 +224,22 @@ create_certdirs $tmpdir/validcert foo{a,b,c}123 trvRMH4Wvt OfDlh6Pc2n
 create_certdirs $tmpdir/expcert bar{x,y,z}999 fx0ljoImWr em3RBC0U8c
 create_certdirs '' empty{1,2,3} gd2HCvRc iFG55Z0a PP28v5kg
 count_and_check_certs 10
+run_cleaner master
+count_and_check_certs 10
 run_cleaner
 count_and_check_certs 5
 
 check_logfiles $maxlog
+check_logfiles $maxlog master
 count_jobs 31
 
 upto 'Watcher status files'
 create_watcher_state
 count_watcher data 10
 count_watcher instance-status 10
+run_cleaner master
+count_watcher data 10
+count_watcher instance-status 10
 run_cleaner
 count_watcher data 5
 count_watcher instance-status 5
-- 
GitLab