Refactor shell scripts

Use a more sensible timestamp for backups
Use arrays when possible
Lowercase variables
More safely process backup retention

Author: m0dular

files/puppet_enterprise_database_backup.sh

@@ -1,65 +1,82 @@
 #!/bin/bash
 
-PG_VERSION=$(/usr/local/bin/facter -p pe_postgresql_info.installed_server_version)
-BACKUPDIR=/opt/puppetlabs/server/data/postgresql/$PG_VERSION/backups
-LOGDIR=/var/log/puppetlabs/pe_databases_backup
-RETENTION=2
-
-while [[ $# -gt 0 ]]; do
-  arg="$1"
-  case $arg in
+while [[ $1 ]]; do
+  case "$1" in
     -t)
-      BACKUPDIR="$2"
-      shift; shift
+      backup_dir="$2"
+      shift 2
       ;;
     -l)
-      LOGDIR="$2"
-      shift; shift
+      log_dir="$2"
+      shift 2
       ;;
     -r)
-      RETENTION="$2"
-      shift; shift
+      retention="$2"
+      shift 2
       ;;
-    *)
-      DATABASES="${DATABASES} $1"
+    # If given the end of options string, shift it out and break
+    --)
       shift
+      break
+      ;;
+    # No need to shift if we've processed all options
+    *)
+      break
       ;;
   esac
 done
 
-if [[ -z "${DATABASES}" ]]; then
-  echo "Usage: $0  [-t BACKUP_TARGET] [-l LOG_DIRECTORY] [-r RETENTION] <DATABASE> [DATABASE_N ...]"
+# The remaining parameters will be the databases to backup
+databases=("$@")
+[[ $databases ]] || {
+  echo "Usage: $0  [-t BACKUP_TARGET] [-l LOG_DIRECTORY] [-r retention] <DATABASE> [DATABASE_N ...]"
   exit 1
-fi
-
-RETENTION_ENFORCE=$((RETENTION-1))
+}
 
-for db in $DATABASES; do
-  echo "Enforcing retention policy of storing only ${RETENTION_ENFORCE} backups for ${db}" >> "${LOGDIR}/${db}.log" 2>&1
+[[ $pg_version ]] || pg_version="$(/usr/local/bin/facter -p pe_postgresql_info.installed_server_version)"
+backup_dir="${backup_dir:-/opt/puppetlabs/server/data/postgresql/$pg_version/backups}"
+log_dir="${log_dir:-/var/log/puppetlabs/pe_databases_backup}"
+retention="${retention:-2}"
 
-  ls -1tr ${BACKUPDIR}/${db}_* | head -n -${RETENTION_ENFORCE} | xargs -d '\n' rm -f --
+for db in "${databases[@]}"; do
+  # For each db, redirect all output the the log inside the backup dir
+  exec &>"${log_dir}/${db}.log"
+  echo "Enforcing retention policy of storing only $retention backups for $db"
 
-  echo "Starting dump of database: ${db}" >> "${LOGDIR}/${db}.log" 2>&1
+  unset backups
+  # Starting inside <(), use stat to print mtime and the filename and pipe to sort
+  # Add the filename to the backups array, giving us a sorted list of filenames
+  while IFS= read -r -d '' line; do
+    backups+=("${line#* }")
+  done < <(stat --printf '%Y %n\0' ${backup_dir}/${db}_* 2>/dev/null | sort -nz)
 
-  if [ "${db}" == "pe-classifier" ]; then
-    # Save space before backing up by clearing unused node_check_ins table.
-    /opt/puppetlabs/server/bin/psql -d pe-classifier -c 'TRUNCATE TABLE node_check_ins' >> "${LOGDIR}/${db}.log" 2>&1
+  # Our array offset will be the number of backups - $retention + 1
+  # e.g. if we have 2 existing backups and retention=2, offset will be one
+  # We'll delete from element 0 to 1 of the array, leaving one backup.
+  # The subsequent backup will leave us with 2 again
+  offset=$(( ${#backups[@]} - $retention + 1 ))
 
-    result=$?
-    if [ $result != 0 ]; then
-      echo "Failed to truncate node_check_ins table" >> "${LOGDIR}/${db}.log" 2>&1
-    fi
+  if (( offset > 0 )); then
+  # Continue if we're retaining more copies of the db than currently exist
+  # This will also be true if no backups currently exist
+    rm -f -- "${backups[@]:0:$offset}"
   fi
 
-  DATETIME=$(date +%m_%d_%y_%H_%M)
 
-  /opt/puppetlabs/server/bin/pg_dump -Fc "${db}" -f "${BACKUPDIR}/${db}_$DATETIME.bin" >> "${LOGDIR}/${db}.log" 2>&1
+  echo "Starting dump of database: $db"
 
-  result=$?
-  if [[ $result -eq 0 ]]; then
-    echo "Completed dump of database: ${db}" >> "${LOGDIR}/${db}.log" 2>&1
-  else
-    echo "Failed to dump database ${db}. Exit code is: ${result}" >> "${LOGDIR}/${db}.log" 2>&1
-    exit 1
+  if [[ $db == 'pe-classifier' ]]; then
+    # Save space before backing up by clearing unused node_check_ins table.
+    /opt/puppetlabs/server/bin/psql -d pe-classifier -c 'TRUNCATE TABLE node_check_ins' || \
+      echo "Failed to truncate node_check_ins table"
   fi
+
+  datetime="$(date +%Y%m%d%S)"
+
+  /opt/puppetlabs/server/bin/pg_dump -Fc "$db" -f "${backup_dir}/${db}_$datetime.bin" || {
+    echo "Failed to dump database $db"
+    exit 1
+  }
+
+  echo "Completed dump of database: ${db}"
 done

files/vacuum_full_tables.sh

@@ -1,39 +1,43 @@
 #!/bin/bash
 
-if [ "$1" = "" ]; then
-  echo "Usage: $0 <(facts, catalogs, or other) tables to VACUUM FULL> "
-  exit
-fi
+usage() {
+  cat <<EOF
+echo "usage: $0 <table>"
+where <table> is one of: facts catalogs other
+EOF
+  exit 1
+}
+# Print usage if given 0 arguments
+(( $# == 0 )) && usage
 
-if [ "$2" = "" ]; then
-  SLEEP=300
-else
-  SLEEP=$2
-fi
+sleep_duration="${2:-300}"
 
 # TODO: Is this used in PE 2018 and newer? RE: fact_values
+case "$1" in
+  'facts')
+    vacuum_tables=("'facts'" "'factsets'" "'fact_paths'" "'fact_values'")
+    ;;
+  'catalogs')
+    vacuum_tables=("'catalogs'" "'catalog_resources'" "'edges'" "'certnames'")
+    ;;
+  'other')
+    vacuum_tables=("'producers'" "'resource_params'" "'resource_params_cache'")
+    ;;
+  *)
+    usage
+esac
 
-if [ "$1" = 'facts' ]; then
-  WHERE="'facts', 'factsets', 'fact_paths', 'fact_values'"
-elif [ "$1" = 'catalogs' ]; then
-  WHERE="'catalogs', 'catalog_resources', 'edges', 'certnames'"
-elif [ "$1" = 'other' ]; then
-  WHERE="'producers', 'resource_params', 'resource_params_cache'"
-else
-  echo "Must pass facts, catalogs, or other as first argument"
-  exit 1
-fi
 
 SQL="SELECT t.relname::varchar AS table_name
   FROM pg_class t
   JOIN pg_namespace n
     ON n.oid = t.relnamespace
   WHERE t.relkind = 'r'
-    AND t.relname IN ( $WHERE )"
+    AND t.relname IN ( $(IFS=,; echo "${vacuum_tables[*]}") )"
+
+tables=($(su - pe-postgres -s /bin/bash -c "/opt/puppetlabs/server/bin/psql -d pe-puppetdb -c \"$SQL\" --tuples-only"))
 
-for TABLE in $(su - pe-postgres -s /bin/bash -c "/opt/puppetlabs/server/bin/psql -d pe-puppetdb -c \"$SQL\" --tuples-only")
-do
-  # echo "$TABLE"
-  su - pe-postgres -s /bin/bash -c "/opt/puppetlabs/server/bin/vacuumdb -d pe-puppetdb -t $TABLE --full --analyze"
-  sleep "$SLEEP"
+for table in "${tables[@]}"; do
+  su - pe-postgres -s /bin/bash -c "/opt/puppetlabs/server/bin/vacuumdb -d pe-puppetdb -t $table --full --analyze"
+  sleep "$sleep_duration"
 done