trjhtr

I wrote up script for backup of file system's log files,
Please suggest me if any modification can be done on it for better working. (script)

#!/bin/sh
##### SCRIPT CREATED AND MAINTAINED BY #<Keshav Piplani> ######
#####FOR ANY QUERIES PLEASE CONTACT <keshavpiplani@gmail.com> ######

#Directory under which files backup needs to be taken
DIR="/data/jboss6/jboss-6.1.0.Final/server/default/log"
#backup location
backup="/data/jboss6/sonata_log_backup"
DAY=`date +%d` 
MONTH=`date +%m` 
YEAR=`date +%Y` 
MIN=`date +%M`
SEC=`date +%S`
date=`date +%F`

#Directory under which files backup needs to be taken

home="/data/jboss6/jboss-6.1.0.Final/server/default/log"
#script output folder
script="/data/jboss6/sonata_log_backup/dailyscript_output"
#1day old files
days=1
mkdir -p "$backup/dailyscript_output"
mkdir -p "$backup/dailyscript_output"/"$date"
mkdir -p "$backup"/log/
mkdir -p "$backup"/log/"$date"

mkdir -p "$backup/dailyscript_output" 2>>/$script/$date/error_log
mkdir -p "$backup/dailyscript_output"/"$date" 2>>/$script/$date/error_log
mkdir -p "$backup"/log/ 2>>/$script/$date/error_log
mkdir -p "$backup"/log/"$date" 2>>/$script/$date/error_log

files=($(find $DIR -maxdepth 1 -type f -mtime +"$days" | grep -v gz | grep -i log )) 2>>/$script/find_error_log
echo "=================================================================="
echo "Zipping Files "
echo "Start of script `date` "

echo "------------------------------------------------$date------------------------------------------------" >>"$script/$date"/In_use@"$DAY.$MONTH.$YEAR"notzipped.txt
echo "------------------------------------------------$date------------------------------------------------" >>"$script/$date"/zip_details_"$YEAR"_"$MONTH"_"$DAY".txt
echo "------------------------------------------------$date------------------------------------------------" >>"$script/$date"/onedayoldfiles_"$YEAR"_"$MONTH"_"$DAY".txt

for files in $files[*]
do


# Check whether files are in use (might still be being copied into directory)
/sbin/fuser "$files" > /dev/null 2>&1
fuser_output=`echo $?`


ls -l $files>>"$script/$date"/onedayoldfiles_"$YEAR"_"$MONTH"_"$DAY".txt
if [[ "$fuser_output" != "0" ]]; then


 # echo $files
#zip files details in zip_details_date.txt
#ls -l $files>>"$script/$date"/zip_details_"$YEAR"_"$MONTH"_"$DAY".txt

 gzip "$files" 
 mv -i "$files".gz "$files"zipped@"$DAY.$MONTH.$YEAR".log.gz
ls -l "$files"zipped@"$DAY.$MONTH.$YEAR".log.gz | sed 's//log//log/old_log_backup/'$date'/g' >>"$script/$date"/zip_details_"$YEAR"_"$MONTH"_"$DAY".txt
 mv -i "$files"zipped@"$DAY.$MONTH.$YEAR".log.gz "$home"/old_log_backup/"$date"/

else
 echo "$files is in use as on $date" >>"$script/$date"/In_use@"$DAY.$MONTH.$YEAR"notzipped.txt 
 echo " " >>$script/email"$date".txt

fi


done

all=`cat "$script/$date"/1dayoldfiles_"$YEAR"_"$MONTH"_"$DAY".txt | grep -v "$date" | wc -l `

Total=`cat "$script/$date"/zip_details_"$YEAR"_"$MONTH"_"$DAY".txt `
Totalno=`cat "$script/$date"/zip_details_"$YEAR"_"$MONTH"_"$DAY".txt |grep -v "$date-" | wc -l `
inuseno=`cat "$script/$date"/In_use@"$DAY.$MONTH.$YEAR"notzipped.txt | grep -v "$date-" | wc -l `
inuse=`cat "$script/$date"/In_use@"$DAY.$MONTH.$YEAR"notzipped.txt `
#echo $Total

echo "Start of Email " >>$script/email"$date".txt

echo " " >>$script/email"$date".txt
#echo " " >>$script/email.txt
echo "Zip file location: $home/old_log_backup at Box `hostname` " >>$script/email"$date".txt
echo " " >>$script/email"$date".txt
echo "Report Output stored in : $script at Box `hostname`" >>$script/email"$date".txt
echo " " >>$script/email"$date".txt
echo " "$all" files are 1day old, See Below --- :">>$script/email"$date".txt

echo " `cat "$script/$date"/1dayoldfiles_"$YEAR"_"$MONTH"_"$DAY".txt ` " >>$script/email"$date".txt
echo " " >>$script/email"$date".txt

echo "Total $Totalno Files got Zipped to $home/old_log_backup/$date , See Below ---" >>$script/email"$date".txt
echo "$Total" >>$script/email"$date".txt
echo " " >>$script/email"$date".txt

echo " $inuseno Files are in use are Below " >>$script/email"$date".txt
echo " $inuse " >>$script/email"$date".txt
echo " " >>$script/email"$date".txt

echo "End of Email " >>$script/email"$date".txt
echo "SCRIPT CREATED BY <keshavpiplani@gmail.com> " >>$script/email"$date".txt


cat $script/email"$date".txt | mailx -s "FILE Got zipped to 'old_log_backup' directory on date $date for server `hostname` " "keshavpiplani@gmail.com"
>$script/email.txt

echo "End of script `date`"
echo "=================================================================="

Nice to see you here at Code Review.

There are lots of things to talk about, so let's get started.

But before we actually get to the code we'll deal with comments and formating.

The first one you used is called "shebang".
I like that you used #!/bin/sh but not bash, zsh or whatever interactive shell is there on your system.
Try stick to sh, most of the time it is all you need.
Besides /bin/sh is often a symlink to dash shell, which is pretty fast and POSIX compilant.

It is matter of personal style, but try not to use all caps for regular comments. This is really annoying. To get reader attention add one more newline before and after your comment.

 

See, that works just fine.

 

Try to place at least one space after #. This way your comments will be a lot more readable. Consider this:

#!/bin/sh

# Created by <Keshav Piplani>
# You can contact me via <keshavpiplani@gmail.com>


# This script will move some system log files around.

...

Last but not least: indent your code.

 

Aha, I see that you don't follow any particular naming convention. Some of your variables are UPPERCASE= others are lowercase=.

Also try to use meaningful variable names. Don't make them short, don't drop random characters from them.

And most important: make your choice and be consistent.

Pick one, that you like:

SOURCE_DIR="/data/jboss6/jboss-6.1.0.Final/server/default/log"
TARGET_DIR="/data/jboss6/sonata_log_backup"

or

source_dir="/data/jboss6/jboss-6.1.0.Final/server/default/log"
target_dir="/data/jboss6/sonata_log_backup"

I'll use the latter.

See this two variable declarations:

home="/data/jboss6/jboss-6.1.0.Final/server/default/log"
script="/data/jboss6/sonata_log_backup/dailyscript_output"

home variable is exact copy of source_dir defined several lines above. I guess one of them is unnecessary. script may be set to "$target_dir/dailyscript_output", no need to use same path twice.

In short: don't repeat yourself.

mkdir -p "$backup/dailyscript_output"
mkdir -p "$backup/dailyscript_output"/"$date"
mkdir -p "$backup"/log/
mkdir -p "$backup"/log/"$date"

mkdir -p "$backup/dailyscript_output" 2>>/$script/$date/error_log
mkdir -p "$backup/dailyscript_output"/"$date" 2>>/$script/$date/error_log
mkdir -p "$backup"/log/ 2>>/$script/$date/error_log
mkdir -p "$backup"/log/"$date" 2>>/$script/$date/error_log

mkdir -p or mkdir --parents will create all missing path components.
So the upper may be replaced with this:

mkdir -p 
 "$target_dir/dailyscript_output/$date" 
 "$target_dir/log/$date"

It is very tedious to redirect each command to log file. You may redirect your script stderr to some log. Also you may wrap all your script body with braces and redirect them instead:

#!/bin/sh

# comments...



 ...

 2>"$logfile"

 

Try to follow suggested points and rewrite your code. After that you may create a follow up question with your new code to see your progress.