The following code copy a complete folder (that has 3 folders inside), then check if errors are generated and send email if there's errors, the it proceeds to create 3 different tar files and finally move this tar files to a mounted network drive, every name has been changed or somehow altered in order to comply with my company security policy.
#! /usr/bin/env bash
readonly SUBJECT="BACKUP"
readonly TO="[email protected]"
readonly MESSAGE="~/backupMessageError.txt"
readonly TOALL="[email protected], [email protected]"
readonly MESSAGEALL="~/backupMessageSuccess.txt"
backup () {
if grep -qs '/mount/dir/' /proc/mounts; then
rsync --exclude /a/folder/ -ravz /backup/source /backup/destination/ 2> ~/backupMessageError.txt
if checker 1ドル; then
mail
else
compression
fi
else
mount -o hard,nolock IP:/volume1/folder /mount/dir/ 2> ~/backupMessageError.txt
if checker 1ドル; then
mail
else
rsync --exclude /a/folder/ -ravz /backup/source/ /backup/destination/ 2>> ~/backupMessageError.txt
if checker 1ドル; then
mail
else
compression
fi
fi
fi
}
compression (){
tar -I pigz -cf backupFolder1.tar.gz folder1 2>> ~/backupMessageError.txt
tar -I pigz -cf backupFolder2.tar.gz folder2 2>> ~/backupMessageError.txt
tar -I pigz -cf backupFolder3.tar.gz folder3 2>> ~/backupMessageError.txt
if checker 1ドル; then
mail
else
storage
fi
}
storage (){
mv /a/folder/backupFolder1.tar.gz /mount/dir/ 2>> ~/backupMessageError.txt
mv /a/folder/backupFolder2.tar.gz /mount/dir/ 2>> ~/backupMessageError.txt
mv /a/folder/backupFolder3.tar.gz /mount/dir/ 2>> ~/backupMessageError.txt
if checker 1ドル; then
mail
fi
}
checker (){
if [ "$(wc -l < ~/backupMessageError.txt)" -ge 1 ];then
return 0;
else
return 1;
fi
}
mail () {
if checker 1ドル;then
mail -s "$SUBJECT" "$TO" < $MESSAGE
else
mail -s "$SUBJECT" "$TOALL" < $MESSAGEALL
fi
}
backup
-
2\$\begingroup\$ Welcome to CR. Are the folder names completely different in the uncensored version or is it like folder1, folder2, .. (A repeatable pattern)? \$\endgroup\$JaDogg– JaDogg2019年03月26日 20:24:37 +00:00Commented Mar 26, 2019 at 20:24
-
2\$\begingroup\$ Folders have completely different names. Thank you for welcoming me :) \$\endgroup\$Mateo Guty– Mateo Guty2019年03月26日 20:29:21 +00:00Commented Mar 26, 2019 at 20:29
1 Answer 1
What is 1ドル
?
All the functions except checker
include a call checker 1ドル
.
But none of those functions are called with a parameter,
so 1ドル
is never actually defined.
The only function that is called with a parameter is checker
,
but it doesn't actually use a parameter.
As such, you could remove all the 1ドル
without changing the behavior of the program.
More importantly, when you want to use 1ドル
for something,
it's good to assign it to a variable with a descriptive name.
That way the reader can understand the purpose.
In the current program it's hard to tell if 1ドル
is simple negligence and oversight,
or a bug waiting to explode.
If it had a descriptive name, I could make a more educated guess.
Always quote variables used in command parameters
Instead of checker 1ドル
always write checker "1ドル"
.
Checking if a file is empty
checker
checks if a file is empty by counting lines.
A simpler way exists using the [ -s ... ]
builtin:
checker() {
[ -s ~/backupMessageError.txt ]
}
Notice that there's no need to write if [ -s ... ]; then return 0; else return 1; fi
,
since the exit code of a function is the exit code of the last command,
so we can simply use the command without the if-else
.
Use better names
The current function names don't help understand what the program is doing.
In fact they are all nouns, when the natural choice would be verbs, or questions.
For example checker
checks if there were any errors.
A more natural naming would be seenAnyErrors
.
Notice how the code could read like prose:
if seenAnyErrors; then
sendErrorReport
else
createBackups
fi
Improve error handling
The current error handling is not so good. Let's take a closer look at for example:
tar -I pigz -cf backupFolder1.tar.gz folder1 2>> ~/backupMessageError.txt tar -I pigz -cf backupFolder2.tar.gz folder2 2>> ~/backupMessageError.txt tar -I pigz -cf backupFolder3.tar.gz folder3 2>> ~/backupMessageError.txt if seenAnyErrors; then ...; fi
What if the first tar
command fails?
Is your intention to continue with the others anyway?
The current way of checking for errors expects that a failing command writes something to stderr
. That's not necessarily the case always, therefore it would be fragile to rely on that. A more reliable way is using the exit code.
A better way to write the above, relying on exit code, would be:
failures=0
tar -I pigz -cf backupFolder1.tar.gz folder1 2>> "$errors"
((failures += $?))
tar -I pigz -cf backupFolder2.tar.gz folder2 2>> "$errors"
((failures += $?))
tar -I pigz -cf backupFolder3.tar.gz folder3 2>> "$errors"
((failures += $?))
if [ "$failures" != 0 ]; then ...; fi
Don't double-quote ~
I believe this is an error:
readonly MESSAGE="~/backupMessageError.txt"
When ~
is double-quoted, the shell won't expand it to $HOME
.
As it is, I think the command mail -s "$SUBJECT" "$TO" < $MESSAGE
will fail with "No such file or directory" error. That variable definition should have been written as:
readonly MESSAGE=~/backupMessageError.txt