As I wrote during/after the review on commit 2958c56, “ganeti-cleaner: Separate queue cleaning code”, while I appreciated the permission separation, I didn't like too much the file-based approach:
- it is a very simple script, and lots of the code is duplicated between the two; I wouldn't like to see "ganeti-vmcapable-cleaner", "ganeti-master-candidate-cleaner", etc. in the future - ganeti-master-cleaner "pollutes" the namespace, creating tab-completion conflicts with ganeti-masterd This patch simply merges the master-cleaner back into cleaner, while keeping the separate user permissions scheme, separate log files, etc. Additionally, it fixes two bugs in the unit-test (not run with set -u and wrong path in the master-cleaner log files test; yay for even worse safety than Python?). And finally, since we have now support for --help-completion, it adds bash completion support for this script :) (needs to be applied on top of my argument support patch series). Signed-off-by: Iustin Pop <[email protected]> --- Yay, ~50 lines removed! Passes distcheck but didn't run QA. .gitignore | 1 - Makefile.am | 10 ++---- autotools/build-bash-completion | 7 ++++ daemons/ganeti-cleaner.in | 53 +++++++++++++++++++++++++++-- daemons/ganeti-master-cleaner.in | 67 ------------------------------------- doc/examples/ganeti.cron.in | 4 +-- man/ganeti-cleaner.rst | 16 +++++---- man/ganeti-master-cleaner.rst | 27 --------------- test/ganeti-cleaner_unittest.bash | 46 +++++++++++-------------- 9 files changed, 91 insertions(+), 140 deletions(-) delete mode 100644 daemons/ganeti-master-cleaner.in delete mode 100644 man/ganeti-master-cleaner.rst diff --git a/.gitignore b/.gitignore index ff61bde..27695ca 100644 --- a/.gitignore +++ b/.gitignore @@ -40,7 +40,6 @@ # daemons /daemons/daemon-util /daemons/ganeti-cleaner -/daemons/ganeti-master-cleaner /daemons/ganeti-masterd /daemons/ganeti-noded /daemons/ganeti-rapi diff --git a/Makefile.am b/Makefile.am index 36e0302..f4dba20 100644 --- a/Makefile.am +++ b/Makefile.am @@ -154,7 +154,6 @@ CLEANFILES = \ $(SHELL_ENV_INIT) \ daemons/daemon-util \ daemons/ganeti-cleaner \ - daemons/ganeti-master-cleaner \ devel/upload \ $(BUILT_EXAMPLES) \ doc/examples/bash_completion \ @@ -645,8 +644,7 @@ dist_sbin_SCRIPTS = \ nodist_sbin_SCRIPTS = \ $(PYTHON_BOOTSTRAP_SBIN) \ - daemons/ganeti-cleaner \ - daemons/ganeti-master-cleaner + daemons/ganeti-cleaner if ENABLE_CONFD htools/ganeti-confd: htools/hconfd @@ -716,7 +714,6 @@ 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 \ @@ -753,7 +750,6 @@ man_MANS = \ man/ganeti-cleaner.8 \ man/ganeti-confd.8 \ man/ganeti-listrunner.8 \ - man/ganeti-master-cleaner.8 \ man/ganeti-masterd.8 \ man/ganeti-noded.8 \ man/ganeti-os-interface.7 \ @@ -1044,8 +1040,7 @@ pep8_python_code = \ test/daemon-util_unittest.bash: daemons/daemon-util -test/ganeti-cleaner_unittest.bash: daemons/ganeti-cleaner \ - daemons/ganeti-master-cleaner +test/ganeti-cleaner_unittest.bash: daemons/ganeti-cleaner test/bash_completion.bash: doc/examples/bash_completion-debug @@ -1074,6 +1069,7 @@ doc/examples/bash_completion-debug: BC_ARGS = doc/examples/bash_completion doc/examples/bash_completion-debug: \ $(BUILD_BASH_COMPLETION) $(RUN_IN_TEMPDIR) \ lib/cli.py $(gnt_scripts) $(client_PYTHON) tools/burnin \ + daemons/ganeti-cleaner \ $(GENERATED_FILES) $(HTOOLS_GENERATED_FILES) PYTHONPATH=. $(RUN_IN_TEMPDIR) \ $(CURDIR)/$(BUILD_BASH_COMPLETION) $(BC_ARGS) > $@ diff --git a/autotools/build-bash-completion b/autotools/build-bash-completion index 058ae77..511f521 100755 --- a/autotools/build-bash-completion +++ b/autotools/build-bash-completion @@ -668,6 +668,7 @@ def HaskellOptToOptParse(opts, kind): #: serialised kind to arg type _ARG_MAP = { + "choices": cli.ArgChoice, "command": cli.ArgCommand, "file": cli.ArgFile, "host": cli.ArgHost, @@ -677,6 +678,7 @@ _ARG_MAP = { "onenode": cli.ArgNode, "oneos": cli.ArgOs, "string": cli.ArgUnknown, + "suggests": cli.ArgSuggest, } @@ -724,6 +726,7 @@ def WriteHaskellCompletion(sw, script, htools=True, debug=True): env = {} script_name = os.path.basename(script) func_name = script_name + func_name = func_name.replace("-", "_") output = utils.RunCmd([cmd, "--help-completion"], env=env, cwd=".").output cli_opts = [] args = [] @@ -781,6 +784,10 @@ def main(): not options.compact, opts=burnin.OPTIONS, args=burnin.ARGUMENTS) + # ganeti-cleaner + WriteHaskellCompletion(sw, "daemons/ganeti-cleaner", htools=False, + debug=not options.compact) + # htools, if enabled if _autoconf.HTOOLS: for script in _autoconf.HTOOLS_PROGS: diff --git a/daemons/ganeti-cleaner.in b/daemons/ganeti-cleaner.in index c5e99d7..13b984c 100644 --- a/daemons/ganeti-cleaner.in +++ b/daemons/ganeti-cleaner.in @@ -25,8 +25,36 @@ set -e -u # Overridden by unittest : ${CHECK_CERT_EXPIRED:=$PKGLIBDIR/check-cert-expired} -readonly CLEANER_LOG_DIR=$LOG_DIR/cleaner +usage() { + echo "Usage: $0 node|master" 2>&1 + exit $1 +} + +if [ "$#" -ne 1 ]; then + usage 1 +fi + +case "$1" in + node) + readonly CLEANER_LOG_DIR=$LOG_DIR/cleaner + ;; + master) + readonly CLEANER_LOG_DIR=$LOG_DIR/master-cleaner + ;; + --help-completion) + echo "choices=node,master 1 1" + exit 0 + ;; + --help) + usage 0 + ;; + *) + usage 1 + ;; +esac + readonly CRYPTO_DIR=$RUN_DIR/crypto +readonly QUEUE_ARCHIVE_DIR=$DATA_DIR/queue/archive in_cluster() { [[ -e $DATA_DIR/ssconf_master_node ]] @@ -56,6 +84,18 @@ cleanup_watcher() { xargs -r0 rm -vf } +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 @@ -77,7 +117,14 @@ echo "Cleaner started at $(date)" find $CLEANER_LOG_DIR -maxdepth 1 -type f | sort | head -n -$KEEP_LOGS | \ xargs -r rm -vf -cleanup_node -cleanup_watcher +case "$1" in + node) + cleanup_node + cleanup_watcher + ;; + master) + cleanup_master + ;; +esac exit 0 diff --git a/daemons/ganeti-master-cleaner.in b/daemons/ganeti-master-cleaner.in deleted file mode 100644 index 3130d95..0000000 --- a/daemons/ganeti-master-cleaner.in +++ /dev/null @@ -1,67 +0,0 @@ -#!/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 62705a2..964af57 100644 --- a/doc/examples/ganeti.cron.in +++ b/doc/examples/ganeti.cron.in @@ -4,7 +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 * * * @GNTMASTERUSER@ [ -x @SBINDIR@/ganeti-master-cleaner ] && @SBINDIR@/ganeti-master-cleaner +45 1 * * * @GNTMASTERUSER@ [ -x @SBINDIR@/ganeti-master-cleaner ] && @SBINDIR@/ganeti-cleaner master # Clean job archive (at 02:45 AM) -45 2 * * * @GNTNODEDUSER@ [ -x @SBINDIR@/ganeti-cleaner ] && @SBINDIR@/ganeti-cleaner +45 2 * * * @GNTNODEDUSER@ [ -x @SBINDIR@/ganeti-cleaner ] && @SBINDIR@/ganeti-cleaner node diff --git a/man/ganeti-cleaner.rst b/man/ganeti-cleaner.rst index 600e2b9..4eb984c 100644 --- a/man/ganeti-cleaner.rst +++ b/man/ganeti-cleaner.rst @@ -9,17 +9,21 @@ ganeti-cleaner - Ganeti job queue cleaner Synopsis -------- -**ganeti-cleaner** +**ganeti-cleaner** node|master DESCRIPTION ----------- -The **ganeti-cleaner** is a periodically run script to remove expired -X509 certificates and keys, as well as outdated **ganeti-watcher** -information. +The **ganeti-cleaner** is a periodically run script to remove old +files. It can clean either node-specific or master-specific files. -**ganeti-cleaner** automatically removes all expired certificates and -keys from ``@LOCALSTATEDIR@/run/ganeti/crypto``. +When called with ``node`` as argument, it will cleanup expired X509 +certificates and keys from ``@LOCALSTATEDIR@/run/ganeti/crypto``, as +well as outdated **ganeti-watcher** information. + +When called with ``master`` as argument, it will instead automatically +remove all files older than 21 days from +``@LOCALSTATEDIR@/lib/ganeti/queue/archive``. .. vim: set textwidth=72 : .. Local Variables: diff --git a/man/ganeti-master-cleaner.rst b/man/ganeti-master-cleaner.rst deleted file mode 100644 index dbb06b5..0000000 --- a/man/ganeti-master-cleaner.rst +++ /dev/null @@ -1,27 +0,0 @@ -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 12b31de..74eb996 100755 --- a/test/ganeti-cleaner_unittest.bash +++ b/test/ganeti-cleaner_unittest.bash @@ -1,7 +1,7 @@ #!/bin/bash # -# Copyright (C) 2010 Google Inc. +# Copyright (C) 2010, 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 @@ -18,13 +18,12 @@ # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA # 02110-1301, USA. -set -e +set -e -u set -o pipefail export PYTHON=${PYTHON:=python} GNTC=daemons/ganeti-cleaner -GNTMC=daemons/ganeti-master-cleaner CCE=tools/check-cert-expired err() { @@ -46,11 +45,15 @@ gencert() { check_logfiles() { local n=$1 p=$2 path - if [[ "$p2" = master ]]; then - path=$tmpls/log/ganeti/mastercleaner + if [[ "$p" = master ]]; then + path=$tmpls/log/ganeti/master-cleaner else path=$tmpls/log/ganeti/cleaner fi + + test -d $path || \ + err "Log file directory '$path' not created" + [[ "$(find $path -mindepth 1 | wc -l)" -le "$n" ]] || \ err "Found more than $n logfiles" } @@ -83,15 +86,7 @@ count_and_check_certs() { } run_cleaner() { - local cmd - - if [[ "$1" = master ]]; then - cmd=$GNTMC - else - cmd=$GNTC - fi - - CHECK_CERT_EXPIRED=$CCE LOCALSTATEDIR=$tmpls $cmd + CHECK_CERT_EXPIRED=$CCE LOCALSTATEDIR=$tmpls $GNTC $1 } create_archived_jobs() { @@ -172,28 +167,25 @@ upto 'Checking log directory creation' test -d $tmpls/log/ganeti || err 'log/ganeti does not exist' test -d $tmpls/log/ganeti/cleaner && \ err 'log/ganeti/cleaner should not exist yet' -run_cleaner -test -d $tmpls/log/ganeti/cleaner || err 'log/ganeti/cleaner should exist' -check_logfiles 1 +run_cleaner node +check_logfiles 1 node 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 1 node 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) )) + run_cleaner node + check_logfiles $(( (i + 2) > $maxlog?$maxlog:(i + 2) )) node check_logfiles $maxlog master done @@ -202,7 +194,7 @@ create_archived_jobs count_jobs 55 test -f $tmpls/lib/ganeti/ssconf_master_node && \ err 'ssconf_master_node should not exist' -run_cleaner +run_cleaner node count_jobs 55 run_cleaner master count_jobs 55 @@ -211,7 +203,7 @@ upto 'Removal of archived jobs (master node)' create_archived_jobs count_jobs 55 echo $HOSTNAME > $tmpls/lib/ganeti/ssconf_master_node -run_cleaner +run_cleaner node count_jobs 55 run_cleaner master count_jobs 31 @@ -226,10 +218,10 @@ create_certdirs '' empty{1,2,3} gd2HCvRc iFG55Z0a PP28v5kg count_and_check_certs 10 run_cleaner master count_and_check_certs 10 -run_cleaner +run_cleaner node count_and_check_certs 5 -check_logfiles $maxlog +check_logfiles $maxlog node check_logfiles $maxlog master count_jobs 31 @@ -240,7 +232,7 @@ count_watcher instance-status 10 run_cleaner master count_watcher data 10 count_watcher instance-status 10 -run_cleaner +run_cleaner node count_watcher data 5 count_watcher instance-status 5 -- 1.7.10.4
