Fix healthcheck

README.md

@@ -74,7 +74,9 @@ Thus, the following sets of environment variables are valid for multi-target bac
 
 ### Healthcheck
 
-When a backup succeeds, it writes down the success time to a file. The healthcheck compares the current time with the last backup's time and it reports healthy if the two are between `MAX_BACKUP_AGE_SECONDS` of each other. This value can be overriden.
+The container is healthy when its last successful backup is less than `MAX_BACKUP_AGE_SECONDS` old (default: `86400`, or one day). All configured targets must succeed. Set this to a positive integer that allows enough time for backups and `SLEEP_TIME`.
+
+Before the first successful backup, the same limit provides a startup grace period. Failed backups leave the last success time unchanged, and restarting does not hide an overdue backup.
 
 ## Repository layout
 
@@ -94,6 +96,16 @@ When a backup succeeds, it writes down the success time to a file. The healthche
 
 ## Development
 
+Run the healthcheck tests in a disposable container (backups are mocked):
+
+```sh
+docker build -t backup-healthcheck-tests .
+tar -c tests | docker run --rm -i --network none --entrypoint /bin/bash \
+  -e BACKUP_HEALTHCHECK_TEST=1 backup-healthcheck-tests \
+  -c 'tar -x -C / && bash /tests/healthcheck.sh'
+shellcheck src/*.sh tests/*.sh
+```
+
 Create a new tag:
 
 ```sh

src/backup-wrapper.sh

@@ -14,63 +14,70 @@ execute_script() {
 }
 
 configure_environment() {
-    local index=$1
-    local all_vars_set=true
-    # required variables
-    for var in BORG_PASSPHRASE BORG_REPO; do
-        local indexed_var_name="${var}_${index}"
-        if [[ -n "${!indexed_var_name}" ]]; then
-            export "$var"="${!indexed_var_name}"
-        else
-            all_vars_set=false
-            break
-        fi
-    done
+    local repo_var="BORG_REPO_$1" passphrase_var="BORG_PASSPHRASE_$1" remote_var="BORG_REMOTE_PATH_$1"
+    [[ -n ${!repo_var} && -n ${!passphrase_var} ]] || return 1
+    export BORG_REPO="${!repo_var}" BORG_PASSPHRASE="${!passphrase_var}"
 
-    # optional variables
-    local indexed_var_name="BORG_REMOTE_PATH_${index}"
-    if [[ -n "${!indexed_var_name}" ]]; then
-        export BORG_REMOTE_PATH="${!indexed_var_name}"
+    if [[ -n ${!remote_var} ]]; then
+        export BORG_REMOTE_PATH="${!remote_var}"
     else
         unset BORG_REMOTE_PATH
     fi
-
-    [[ $all_vars_set == true ]]
 }
 
 main() {
     if [ -n "$BORG_REPO" ]; then
         # fallback case if multi-target backup isn't needed
-        if execute_script; then
-            date > /health/backup_completion_time.log
-        else
+        if ! execute_script; then
             echo "Skipping completion log due to backup failure"
+            return 1
         fi
     else
         local index=0
-        local configurations_found=false
         local any_failed=false
+        local completed_indices=" "
 
-        while configure_environment $index; do
+        while configure_environment "$index"; do
             execute_script || any_failed=true
-            configurations_found=true
+            completed_indices+="$index "
             unset BORG_PASSPHRASE BORG_REMOTE_PATH BORG_REPO
             ((index++))
         done
 
         echo "Finished backup script at $(date)"
-        if [[ $any_failed == false ]]; then
-            date > /health/backup_completion_time.log # this used by the healtcheck
-        else
-            echo "Skipping completion log due to backup failure(s)"
+        if (( index == 0 )); then
+            echo "No valid configuration found. Please ensure environment variables are set properly."
+            return 1
         fi
 
-        if [[ $configurations_found == false ]]; then
-            echo "No valid configuration found. Please ensure environment variables are set properly."
+        # A missing or incomplete target must not silently hide later targets.
+        local indexed_var_name
+        for indexed_var_name in ${!BORG_REPO_@} ${!BORG_PASSPHRASE_@} ${!BORG_REMOTE_PATH_@}; do
+            [[ -n ${!indexed_var_name} ]] || continue
+            case "$completed_indices" in
+                *" ${indexed_var_name##*_} "*) ;;
+                *)
+                    echo "Invalid or non-contiguous backup configuration: $indexed_var_name. Skipping completion log."
+                    return 1
+                    ;;
+            esac
+        done
+
+        if [[ $any_failed == true ]]; then
+            echo "Skipping completion log due to backup failure(s)"
+            return 1
         fi
     fi
+
+    # Rename only a complete timestamp, leaving the previous success intact on failure.
+    mkdir -p /health || return 1
+    trap 'rm -f /health/backup_completion_time.log.$$' EXIT
+    date -u '+%Y-%m-%dT%H:%M:%SZ' > /health/backup_completion_time.log.$$ &&
+        mv -fT /health/backup_completion_time.log.$$ /health/backup_completion_time.log
 }
 
 main
+backup_status=$?
 
 echo "Finished backup wrapper script at $(date)"
+exit "$backup_status"

src/healthcheck.sh

@@ -4,25 +4,31 @@ set -e
 
 MAX_BACKUP_AGE_SECONDS="${MAX_BACKUP_AGE_SECONDS:-86400}"
 
-if [ -f /health/backup_completion_time.log ]; then
-    current_time=$(date +%s)
-    backup_time=$(date --file /health/backup_completion_time.log +%s)
-    age_in_seconds=$((current_time - backup_time))
+unhealthy() {
+    echo "$* Healthcheck failed."
+    exit 1
+}
 
-    if [ "${age_in_seconds}" -lt "${MAX_BACKUP_AGE_SECONDS}" ]; then
-        echo "Backup completed within the last ${MAX_BACKUP_AGE_SECONDS} seconds. Healthcheck passed."
-        exit 0
-    fi
-elif [ -f /health/container_start_time.log ]; then
-    current_time=$(date +%s)
-    start_time=$(date --file /health/container_start_time.log +%s)
-    age_in_seconds=$((current_time - start_time))
-
-    if [ "${age_in_seconds}" -lt "${MAX_BACKUP_AGE_SECONDS}" ]; then
-        echo "No backup completed yet, but container started less than ${MAX_BACKUP_AGE_SECONDS} seconds ago. Healthcheck passed."
-        exit 0
-    fi
+if ! [ "$MAX_BACKUP_AGE_SECONDS" -gt 0 ] 2>/dev/null; then
+    unhealthy "MAX_BACKUP_AGE_SECONDS must be a positive integer."
 fi
 
-echo "Backup not completed within the last ${MAX_BACKUP_AGE_SECONDS} seconds. Healthcheck failed."
-exit 1
+timestamp_file=/health/backup_completion_time.log
+[ -f "$timestamp_file" ] || timestamp_file=/health/container_start_time.log
+
+timestamp=$(cat "$timestamp_file") || unhealthy "Cannot read $timestamp_file."
+case "$timestamp" in
+    *[![:space:]]*) ;;
+    *) unhealthy "Empty timestamp in $timestamp_file." ;;
+esac
+recorded_time=$(date --date="$timestamp" +%s 2>/dev/null) || unhealthy "Invalid timestamp in $timestamp_file."
+
+# Read the clock after the marker, which may have just been replaced by a backup.
+current_time=$(date +%s)
+age_in_seconds=$((current_time - recorded_time))
+if [ "$age_in_seconds" -ge 0 ] && [ "$age_in_seconds" -lt "$MAX_BACKUP_AGE_SECONDS" ]; then
+    echo "Timestamp in $timestamp_file is ${age_in_seconds} seconds old. Healthcheck passed."
+    exit 0
+fi
+
+unhealthy "Timestamp in $timestamp_file is outside the allowed age of ${MAX_BACKUP_AGE_SECONDS} seconds."

src/schedule.sh

@@ -11,7 +11,10 @@ get_log_file_name() {
 }
 
 echo "Starting schedule script at $(date)" | log_message
-date > /health/container_start_time.log
+mkdir -p /health || exit 1
+trap 'rm -f /health/container_start_time.log.$$' EXIT
+date -u '+%Y-%m-%dT%H:%M:%SZ' > /health/container_start_time.log.$$ &&
+    mv -fT /health/container_start_time.log.$$ /health/container_start_time.log || exit 1
 
 while true; do
     /src/backup-wrapper.sh 2>&1 | log_message