Simple shell script to backup to Amazon over iSCSI

Question

The script works but I know that it can be improved. Please note that my scripting skills are very basic :) (as it can be seen in my Frankenstein script – I took bits and pieces of the code from different tutorials).

I would appreciated if someone could review this script and identify some major flaws.

Some other questions that I have:

1. After the backup is complete is it necessary/better to logout from the amazon and unmount the volume?
2. Is it better to check if the volume is mounted based on the UUID?

#!/bin/bash
##
## VARIABLES
##
BACKUP_FROM="/folder/"
BACKUP_TO="/folder/"
LOG_FILE="/var/log/backup.log"
SCSI="/sbin/iscsiadm"
VOL="ghrryhfd-56655565-456456-3453-dfhthjtrrgg"
##
## SCRIPT
##
# Check that the log file exists
if [ ! -e "$LOG_FILE" ]; then
 touch "$LOG_FILE"
fi
# Check if the volume is mounted if not login to amazon and mount
if mountpoint -q /mount_folder; then
echo "$(date "+%Y-%m-%d %k:%M:%S") - Volume mounted" >> "$LOG_FILE"
else
echo "$(date "+%Y-%m-%d %k:%M:%S") - Mounting!" >> "$LOG_FILE"
$SCSI --mode node --targetname abc.222-99.com.amazon:volume_name --portal 1.1.1.1:9999 --login
sleep 10
mount -U "$VOL" /mount_folder
sleep 8
fi
# Start entry in the log
echo "$(date "+%Y-%m-%d %k:%M:%S") - Sync started." >> "$LOG_FILE"
# Start sync
if rsync -a -v "$BACKUP_FROM" "$BACKUP_TO" &>> "$LOG_FILE"; then
 echo "$(date "+%Y-%m-%d %k:%M:%S") - Sync completed succesfully." >> "$LOG_FILE"
else
 echo "$(date "+%Y-%m-%d %k:%M:%S") - ERROR: rsync-command failed." >> "$LOG_FILE"
 echo "$(date "+%Y-%m-%d %k:%M:%S") - ERROR: Unable to sync." >> "$LOG_FILE"
 echo "" >> "$LOG_FILE"
 exit 1
fi
# End entry in the log
echo "" >> "$LOG_FILE"
exit 0

Answer 1

Extract common logic to a helper function

Notice the similarity in these statements:

echo "$(date "+%Y-%m-%d %k:%M:%S") - Volume mounted" >> "$LOG_FILE"
...
echo "$(date "+%Y-%m-%d %k:%M:%S") - Mounting!" >> "$LOG_FILE"
...
echo "$(date "+%Y-%m-%d %k:%M:%S") - Sync started." >> "$LOG_FILE"

They all have the same pattern:

echo "$(date "+%Y-%m-%d %k:%M:%S") - MESSAGE" >> "$LOG_FILE"

So it's a good candidate to turn into a function:

log() {
 echo "$(date "+%Y-%m-%d %k:%M:%S") - $@" >> "$LOG_FILE"
}

So that you can simplify the uses like this:

log "Volume mounted"
...
log "Mounting!"
...
log "Sync started."

Result of mount

The mount command might fail. When that happens, you might want to abort:

if ! mount -U "$VOL" /mount_folder; then
 log "Unable to Mount"
 exit 1
fi

Pointless touch

This code is completely unnecessary. Whether the file existed or not, whether you touch it or not, it won't affect the rest of the script. You can safely remove it.

if [ ! -e "$LOG_FILE" ]; then
 touch "$LOG_FILE"
fi

Simplify

This is kind of tedious:

echo "" >> "$LOG_FILE"

You can just drop the "" to simplify:

echo >> "$LOG_FILE"

And, you don't need exit 0 at the end of the script.

• Pete: Many thanks for taking the time to look over my code. I made those changes you suggested and the code looks much better and works very well! I am thinking of adding additional if statement to stop the script if the mount fails. Can I do something like that: ... sleep 10 mount -U "$VOL" /mount_folder if [ ! $? -eq 0 ]; then log "Unable to Mount" exit 1; fi sleep 8 fi ...
• janos: @Pete good thinking! Actually I should have pointed that out for you myself... I added a section for it now, good catch!