3
\$\begingroup\$

I'm running Debian 12 and Docker compose containers. Once a day, crontab should start the script, but the script should also be called manually. Could you rate my upgrade script file or recommend an alternative approach?

Requirements for the script:

  • Debian 12 should be (fully) upgraded
  • Docker compose container should be stopped, pulled an updated
  • Docker compose should then be restarted automatically
  • docker system prune should be called afterward
  • If a system reboot is required, the script should reboot
  • The normal and error output should be to appear in console AND in an output file
  • Between several steps there, should be a time waiting
#!/bin/sh
spath="/home/<...>/<folder>/"
sname="script_all.sh"
if [ $# -eq 0 ]
then
    sh "${spath}${sname}" "arg1" 2>&1 | tee -a "${spath}log_$(date +%F_%T_%N).txt"
    exit
fi
cd $spath || exit
docker compose down
sleep 5
sudo apt update && sudo DEBIAN_FRONTEND=noninteractive apt upgrade -y && sudo apt autoremove -y && sudo apt autoclean
sleep 5
if [ -f /var/run/reboot-required ]
then
        at now + 2 minute -f "${spath}${sname}"
        sudo reboot
else
        docker compose pull
        docker compose build --pull
        docker compose up -d
        sleep 5
        docker system prune -a -f
        docker system df
        sleep 5
        docker stats --no-stream | sort -k 2
fi

Thanks in advance.

\$\endgroup\$
3
  • \$\begingroup\$ Please do not edit the question, especially the code, after an answer has been posted. Changing the question may cause answer invalidation. Everyone needs to be able to see what the reviewer was referring to. What to do after the question has been answered. You can ask a follow up question with a link back to this question with any new code you want reviewed. \$\endgroup\$
    – pacmaninbw
    Oct 7, 2023 at 14:44
  • \$\begingroup\$ Oki, that was my fault, I didn't read the rules/FAQs enough (sorry) ... may I ask a new question for a new review. \$\endgroup\$ Oct 7, 2023 at 14:49
  • \$\begingroup\$ Yes you can ask a new question, or you can ask a follow up question. \$\endgroup\$
    – pacmaninbw
    Oct 7, 2023 at 14:59

1 Answer 1

3
\$\begingroup\$

shebang

#! /bin/sh

No bash ? Ok, suit yourself. Bash definitely offers more builtin functionality than Bourne shell.


uniformly capture errors

cd $spath || exit

I like the sentiment, here. Consider making set -e -o pipefail the second line of this script. Then cd would bail with error status if there's trouble.

If you had run the shellcheck linter, I imagine it would have asked for extra quotes around the spath expansion. Not a problem, here. But you should probably be linting, if this is code you care about.


log file

    sh "${spath}${sname}" "arg1" ...

The literal arg1 is odd. You might have helpfully called it logging. But the whole approach is odd, better to avoid it.

You can grab the output of several commands with (cmd_1; ... ; cmd_N) | tee ..., where the commands may span several lines.

Better, package the body of the script as a function, perhaps named upgrade. And then upgrade 2>&1 | tee ... suffices.


magic numbers

docker compose down
sleep 5

That 5 tuning parameter makes me nervous. Couldn't we sleep 1 in a while loop that keeps asking docker "is it down yet?", "is it down yet?" Similarly when we're waiting for it to come up.

The apt stuff looks fine. But then, why are we sleeping after it? With no explanatory comment? It's all synchronous, there's nothing going on in the background.

There's a bunch of sudo calls, and maybe that's what you want, for auditing. Consider consolidating them with sudo bash -c " ... "

The now + 2 minute thing is just weird. Is that to give you a chance to manually poke around at the last moment? Reboot at once and be done with it. The larger system is composed of machines + people, and its reliability goes down when people invent mythologies about what's happening at the moment and what will hopefully happen "soon".

        docker system df
        sleep 5

Again, offer a # comment describing why a sleep is required at all, and why 5 is exactly what's needed there. Consider replacing it with a looping test, similar to awaiting docker down / up transitions.

\$\endgroup\$
5
  • \$\begingroup\$ Thank you. sudo bash -c " ... " looks pretty good, it's ok to apply sudo to all the commands. The sleep 5 calls are not really necessary. My thought was to give Linux time to complete the file operations... docker system df and docker stats are just control outputs to see if everything is fine. \$\endgroup\$ Oct 7, 2023 at 5:04
  • \$\begingroup\$ "No bash ?" - I found out, that at ... -f ... can't handle bash scripts, but I don't know why. \$\endgroup\$ Oct 7, 2023 at 5:07
  • 1
    \$\begingroup\$ Certainly at (and cron, which runs it) can handle bash scripts. But as you observe they default to using the more limited Bourne sh. Let us step back to ask a different question: "can they handle python scripts?" Yes, of course they can. How? With python myscript.py or perhaps via shebang line of #! /usr/bin/env python. Now return to running with bash. We accomplish that in the very same way, with a command of bash myscript.sh or perhaps a suitable initial shebang line. No one will confuse python source for shell script. They might with Bourne / bash, so take care when executing. \$\endgroup\$
    – J_H
    Oct 7, 2023 at 16:25
  • \$\begingroup\$ exit without argument is the same as exit $?. So not sure what your point is with that one. \$\endgroup\$ Oct 9, 2023 at 12:13
  • 1
    \$\begingroup\$ Hmmm, interesting. Guess I've always seen authors being explicit about error status (exit 1) or implicit about early successful exit. Ok, thanks, duly updated in the answer text. \$\endgroup\$
    – J_H
    Oct 9, 2023 at 16:14

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.