2
\$\begingroup\$

I have this little server at home, which is hosting some websites, school stuff and a minecraft server. I made this backup script and I would like some feedback since I'm not very good at scripting.

#!/bin/bash
# declare folders to backup
dirs=(
    "/home/ferre/Minecraft"
    "/var/lib/automysqlbackup/daily"
);

# backup names
names=(
  "Minecraft"
  "Mysql"
);                                                                                                                                                                                                                                        

# backup folder
backupdir="/mnt/backup";

# backup algorithm
for (( i=0; i<=$(( ${#dirs[*]} -1 )); i++))
do
  # info
  echo "starting backup of ${names[i]}";

  # create folder
  path="${backupdir}/${names[i]}";
  $(mkdir -p "$path");

  # create backup
  file="${path}/$(date +%F).tar";
  $(tar -czf "$file" --absolute-names "${dirs[i]}");

  # reporting
  result=$?;
  if [ $result -eq 0 ]; then
    echo "SUCCESS";
  elif [ $result -eq 1 ]; then
    echo "WARNING some files where changed while copying!";
  else
    echo "FATAL ERROR";
  fi
done
\$\endgroup\$
  • \$\begingroup\$ Welcome to Code Review! Are you running this manually or by automatic (cron) timed job? \$\endgroup\$ – Mast May 27 at 18:04
  • 1
    \$\begingroup\$ This is running by a cron job \$\endgroup\$ – Ferre Vekemans May 27 at 19:00
1
\$\begingroup\$

There is not a lot to say, the script is fairly simple.

For the date format:

file="${path}/$(date +%F).tar";

I think I would define a constant variable at the top of the code. You might want to change the pattern latter (use something different than YYYY-MM-DD).

This:

$(mkdir -p "$path");

can simply be written as:

mkdir -p "$path"

unless you want to save the output of the bash command to a variable but this is not the case here.


You have some key/pair values to define your backup sources:

dirs=(
    "/home/ferre/Minecraft"
    "/var/lib/automysqlbackup/daily"
);

# backup names
names=(
  "Minecraft"
  "Mysql"
);

I would combine both into an associative array.

declare -A sources=(
    ["Minecraft"]="/home/ferre/Minecraft"
    ["Mysql"]="/var/lib/automysqlbackup/daily"
)

for item in "${!sources[@]}"; do
  echo "Name: $item => Directory: ${sources[$item]}"
done

Output:

Name: Mysql => Directory: /var/lib/automysqlbackup/daily
Name: Minecraft => Directory: /home/ferre/Minecraft

Thus you can easily loop on the array and extract name and target directory. Warning: please check the syntax. Bash has many pitfalls and I may have made mistakes.


Logging: I think it is important to retain a trace of script execution. Especially when the script is unattended. The console can quickly fill up and you could miss critical messages.

You have a few options like:

  • define a variable for a log file then use tee -a next next to each command, so that you get output to console and to a file at the same time. But this is tedious and not flexible.
  • Call your script like this: /path/to/your/script.sh > backup.log (use >> to append) or: /path/to/your/script.sh | tee backup.log
  • or better yet /path/to/your/script.sh 2>&1 | tee backup.log to capture the output of stderr.

Last but not least, your script could return an exit code. This is useful if your script is going to be handled by another script or even set up as a service.


Suggestions:

  • Have the script send you the log file by E-mail after execution. Or archive the log file somewhere for later review if desired.
  • Add error handling to make your script more reliable and more robust. If something wrong happens, or at least a fatal error, the script should stop and alert you. Here is an intro: Bash Error Handling

A backup script is usually critical, it has to perform reliably. One day, you may need to restore some important files, or recover from a system crash. There is nothing more tragic than useless/incomplete backups.

So you should also test your backups manually from time to time. Attempt to restore a random file and verify the result.

| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ Can you split the declare -A into multiple lines? This would make them easier readable. \$\endgroup\$ – Roland Illig May 28 at 6:35
  • \$\begingroup\$ I don't think the date format will ever change. Once you have come to the ISO format, there's no reason to change to anything else. \$\endgroup\$ – Roland Illig May 28 at 6:37

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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