Code Review Stack Exchange is a question and answer site for peer programmer code reviews. Join them; it only takes a minute:

Sign up
Here's how it works:
  1. Anybody can ask a question
  2. Anybody can answer
  3. The best answers are voted up and rise to the top

I have written a simple script for managing our Tomcat and Apache instances for deployment. What this script basically does is, when called, it copies the ROOT.war from its pwd and pushes them to two Tomcat instances. Before that, it removes the old ROOT.war file and ROOT folder, restarts them, cleans the log files, restarts both Tomcat instances and Apache webserver (load-balancer and failover guy).

I have just pasted commands in a script and thought would learn more about it when I optimize this script, such as adding checks.

#!/bin/bash
sh /home/deploy/tomcatfirst/bin/catalina.sh stop
rm -rf /home/deploy/tomcatfirst/webapps/ROOT/
rm -rf /home/deploy/tomcatfirst/logs/
rm /home/deploy/tomcatfirst/webapps/ROOT.war
cp ROOT.war /home/deploy/tomcatfirst/webapps/
mkdir /home/deploy/tomcatfirst/logs


sh /home/deploy/tomcatfirst/bin/catalina.sh stop
rm -rf /home/deploy/tomcatsecond/webapps/ROOT/
rm /home/deploy/tomcatsecond/webapps/ROOT.war
rm -rf /home/deploy/tomcatsecond/logs/
mv ROOT.war /home/deploy/tomcatsecond/webapps/
mkdir /home/deploy/tomcatsecond/logs

sh /home/deploy/tomcatfirst/bin/catalina.sh start
sh /home/deploy/tomcatsecond/bin/catalina.sh start
service apache2 restart
share|improve this question
2  
Welcome to Code Review! I have rolled back the last edit. Please see what you may and may not do after receiving answers. – Quill Jan 14 at 15:41
1  
No, it is not okay. Please do not edit updates to your code into the question. Post it as a new question if you want it reviewed. – Quill Jan 14 at 15:47
    
But I want to make sure that the changes I did are also correct, depending upon suggestion as of now from choroba. – We are Borg Jan 14 at 15:52
    
Then you'd be best to post it as a new question and get your changes reviewed too :-) – Quill Jan 14 at 15:53
    
I've removed the userscript tag from your post as userscripts are JavaScript addons designed to run in the browser, not a general term – Quill Jan 14 at 15:54
up vote 8 down vote accepted

Adding checks? Just specify

set -e

at the beginning of the script. Should any command fail, the script will stop running, instead of wreaking more damage.

Using variables for repeated paths might make the script more readable.

tomcat1=/home/deploy/tomcatfirst
tomcat2=/home/deploy/tomcatsecond

sh "$tomcat1"/bin/catalina.sh stop

(BTW, shouldn't the second stop use $tomcat2?)

Once you have them, changing the above line to

set -eu

might be another improvement - the script will fail if a variable is not defined, which can happen if you mistype its name.

As the commands for both the server are the same, you can wrap them in a loop:

for tomcat /home/deploy/tomcat{first,second} ; do
    sh "$tomcat"/bin/catalina.sh stop
    # ...
done

If the servers are critical, you should specify the commands with full paths (e.g. /bin/mkdir).

share|improve this answer
    
I will check about assigning variables in bash as I don't know how. Haven't I already specified full paths? – We are Borg Jan 14 at 15:16
    
@WeareBorg: Updated. – choroba Jan 14 at 15:20
    
Found it. The updated script at bottom of main post. Just one thing more I can think of, how can I make sure that there is a ROOT.war file present at /home/deploy/ and then only start further operations? – We are Borg Jan 14 at 15:22
    
Dont't include the final slash in the variable value. You use another one when using the variable, anyway. Double quote the variables - it's a good habit for cases when they can contain whitespace. To check file existence, use [[ -f filename ]], but note that the file might be removed between the check and actual cp. – choroba Jan 14 at 15:27
    
Script updated(3rd update) as per your suggestions. Kindly have a look.. Thank you. – We are Borg Jan 14 at 15:35

I'd suggest not purging logs - they can be useful for figuring out what happened. Simply remove the rm line for the log directories. Your webserver will simply append its new info to the existing log files, and something like logrotate should be used to keep the last X weeks.

Another option is to use the find command to purge old files but not the newest ones.

/usr/bin/find /home/deploy/tomcatfirst/logs/ -mtime +7 -type f -exec /bin/rm -f "{}" \;

This will delete any file in the given directory that is over 7 days old.


Also use exit status to figure out if you want to continue. Simple example might be

sh /home/deploy/tomcatfirst/bin/catalina.sh stop || exit 1

If the first command finishes with an exit level other than 0 it will stop the script.

share|improve this answer
1  
Basically, set -e adds || exit 1 to all the commands that don't end in || something. – choroba Jan 15 at 7:28
1  
We are only using the latest log and we keep an eye on it as we are testing as well. But deleting after the 7 days sounds like a good idea. My main problem was TOmcat was creating multiple log files based upon days/weeks/month. – We are Borg Jan 15 at 8:01

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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