Debian and Docker compose upgrade script, 2nd version

Question

As a continuation of this question, I would like a second review from you.

Here is the updated sh script:

#!/bin/sh
set -e
spath="/home/<user>/<dir>/"
sname="<scriptname>.sh"
if [ $# -eq 0 ]
then
    sh "${spath}${sname}" "logging" 2>&1 | tee -a "${spath}log_$(date +%F_%T_%N).txt"
    exit
fi
cd $spath
# This three commands for logging...
pwd
date
whoami
docker compose down
# To go sure, docker is down and file operations are complete
sleep 5
sudo sh -c "apt update && DEBIAN_FRONTEND=noninteractive apt upgrade -y && apt autoremove -y && apt autoclean"
# To go sure, upgrades are done
sleep 5
if [ -f /var/run/reboot-required ]
then
        # To go sure, system is up again after reboot
        at now + 2 minute -f "${spath}${sname}"
        sudo reboot
else
        docker compose pull
        docker compose build --pull
        docker compose up -d
        # To go sure, docker is running again
        sleep 5
        docker system prune -a -f
        # Just for information/debug logging purposes, everything is up
        sleep 5
        docker system df
        docker stats --no-stream | sort -k 2
fi

The requirements are almost the same. I think it's safer to user sh instead of bash because I use the at command.

Answer 1

The base path and the name of the current script

I don't understand the purpose of these lines:

spath="/home/<user>/<dir>/"
sname="<scriptname>.sh"

From the rest of the script I'd guess it's the base path and name of the script. Instead of hardcoding, you could write like this:

spath=$(cd "$(dirname "$0")"; pwd)
sname=$(basename "$0")

Double-quote variables on the command line

Unquoted variables on the command line are subject to word splitting and globbing by the shell. Most of the time it's not intended, therefore it's better to write cd "$spath" instead of cd $spath.

Even if you know for sure that in this use case $spath will not contain any characters that may lead to word splitting or globbing, it's a good habit to build.

More blank lines please

The script is a "wall of text". Adding blank lines in between sections with tightly related commands would make it easier to read.

Avoid arbitrary sleep

Sleeping for some arbitrary amount of time and hoping that something is ready by the time the sleep ends is error prone. It's better to think of some observable event to wait for.

For example, this code hopes that the container will be down after 5 minutes:

docker compose down
# To go sure, docker is down and file operations are complete
sleep 5

This will fail if sometimes it takes a bit longer, or if you increase the sleep, then sometimes it will be unnecessarily slow. It would be better to sleep 1 in a loop until the container is actually down, and up to some maximum number of iterations.

Hint: use docker-compose ps -q <service_name> to check if the container is down.

Avoid sudo in scripts

After starting a script, I like to assume that I can step away for a coffee and it will be done by the time I'm back. If the script has sudo in the middle, then I might not be there to enter a password.

I recommend to make it mandatory to run the script as root user. That is, when $(id -u) is not 0, print a helpful message and exit 1.

You could add this check after the top part, right before the cd "$spath".