bergman on 26 Apr 2012 14:23:40 -0700


[Date Prev] [Date Next] [Thread Prev] [Thread Next] [Date Index] [Thread Index]

Re: [PLUG] First shell script for deployment...


In the message dated: Thu, 26 Apr 2012 16:34:04 EDT,
The pithy ruminations from Paul Walker on 
<[PLUG] First shell script for deployment...> were:
=> --===============1493056227==
=> Content-Type: multipart/alternative; boundary=047d7b15b09ba33d6b04be9ae88b

Ick. Why use HTML multipart MIME for plain ASCII text?

=> 
=> --047d7b15b09ba33d6b04be9ae88b
=> Content-Type: text/plain; charset=ISO-8859-1
=> 
=> This is the first shell script I've written... pretty basic but nice for
=> now since I need to maintain some consistency across installations.. (OT
=> but I'll use .gitignore to avoid having to do this in the future... for now
=> I want to eliminate as much inconsistency as possible).  Any thoughts or
=> constructive criticism appreciated:

Well, since you asked...


=> 
=> <code>
=> #!/bin/bash
=> 
=> # Run this from the directory **above** the public_html directory for the
=> site you are deploying

Missing info. re version, date, author. I find this ironic, as the script is
meant to manipulate info from a version control system.

=> 
=> #create dir and clone upstream repo

You should check if the directory exists before trying to do the mkdir...that
would be good indication that you're in the wrong parent dir, or that a
previous run of the script failed.

Does there need to be a check of what user is running the script (root?
apache? nobody? JoeRandomHacker?) before proceeding?

=> mkdir public_html_deploy1

There's an assumption about the directory where the script is being run. It's
much safer to cd to an /absolute/path/to/the/working/dir at the beginning.

The script is missing checks for the return value of each step. That's a
common--and serious--failure. I usually deal with that with a shell function,
such as:

######################################
checkandexit()
{
        if [ $1 != 0 ] ; then
                # There was an error, bail out!
                /bin/echo "$2" 1>&2
                exit $1
        fi
}
######################################

That also makes a convenient place to remove temp files, lock files, send
messages to syslog, etc.

The function is used as follows:

	mkdir public_html_deploy1
	checkandexit $? "mkdir of public_html_deploy1 failed"


=> cd public_html_deploy1
	checkandexit $? "Could not cd to public_html_deploy1"

	insert as needed after every command

=> git init
=> git clone git@foo.com:project/repo.git
=> git remote add foo git@foo.com:project/repo.git
=> git config remote.foo.push refs/heads/master:refs/heads/master
=> mv repo ../public_html_deploy

The naming (public_html_deploy vs public_html_deploy1) is confusing--the names
are so similar that it's difficult to tell what operations are being
done...and all to easy to rm the wrong dir. I'd suggest changing
"public_html_deploy" to something like "staging" or "git_checkout" or "Fred".

=> 
=> #move untracked files from current webroot into deployment directory tree
=> cd ../public_html_deploy
=> cp -r ../public_html/articleImport .
=> cp -r ../public_html/themes .
=> cp -r ../public_html/submissions .
=> cp -r ../public_html/sites/all/themes/ sites/all/themes
=> cp ../public_html/sites/default/settings.php sites/default/settings.php
=> cp -r ../public_html/sites/default/files/ sites/default/files

That's too hard for me to parse, given the lack of comments. Will this make
sense to you in 6 months, or to your replacement in a year, when the directory
structure changes?

=> 
=> #get rid of the stepping stone directory
=> rm -rf ../public_html_deploy_1
          ^^^^^^^^^^^^^^^^^^^^^^^

Critical error! You use "public_html_deploy1" when creating the directory and
use "public_html_deploy_1" when removing a different directory.

=> 
=> #set file permission
=> chmod -R 777 sites/default/files

Why 777? Why set that permission on files? If I understand the intent
of the script (difficult, given the lack of comments), it looks like
this has something to do with web content. In that case, I understand
that files and directories may need to be world readable, but there's
probably no need to make them world writeable & executable.

This is probably sufficient:

	find sites/default/files -type f -print0 | xargs -0 chmod o+r
	find sites/default/files -type d -print0 | xargs -0 chmod o+rx

and maybe add this, if stictly necessary (but unlikely):
	find sites/default/files -type f -iname "*php" -print0 | xargs -0 chmod o+rx


The script should probably exit with an explicit return value, so that other
processes can check the status:

	exit 0



Overall, the script will probably work (once the critical error is
fixed), but I'd consider it to be very fragile--there are way too many
undocumented assumptions about directory paths, no error checking,
virtually no documentation, no consideration of security or possible
failure modes, no logging (optional or default) to any destination
(stderr or syslog), etc.

There are lots of good books and on-line examples about shell scripting.
Perhaps you should check some of them.


Mark "now back to writing my own script" Bergman

=> </code>
=> 
___________________________________________________________________________
Philadelphia Linux Users Group         --        http://www.phillylinux.org
Announcements - http://lists.phillylinux.org/mailman/listinfo/plug-announce
General Discussion  --   http://lists.phillylinux.org/mailman/listinfo/plug