Drupal6: Juju Charm for Superchared Drupal.

here is the branch until I can link it, I seems to have run into a LP glitch in Chrome on OSX

https://code.launchpad.net/~imbrandon/charms/oneiric/quickdrop/trunk

Moved code to github, charm is now complete and ready for review

git://github.com/bholtsclaw/jujucharm-drupal.git

https://code.launchpad.net/~imbrandon/charms/oneiric/drupal/trunk

Hey Brandon, your install hook is has some remnants of omgubuntu specific stuff:

rm -f /etc/nginx/sites-available/omgubuntu.co.uk
install -o root -g root -m 0644 files/nginx/etc_nginx_sites-enabled_omgubuntu.co.uk /etc/nginx/sites-available/omgubuntu.co.uk
ln -sf ../sites-available/omgubuntu.co.uk /etc/nginx/sites-enabled/omgubuntu.co.uk

rm -f /etc/nginx/sites-available/admin.omgubuntu.co.uk
install -o root -g root -m 0644 files/nginx/etc_nginx_sites-enabled_admin.omgubuntu.co.uk /etc/nginx/sites-available/admin.omgubuntu.co.uk
ln -sf ../sites-available/admin.omgubuntu.co.uk /etc/nginx/sites-enabled/admin.omgubuntu.co.uk

Consider instead of using `unit-get public-address` to have the unit's public address replace things like "omgubuntu.co.uk" in the filenames

Also, this charm requires AWS S3 in order for things like backups to work properly. This was okay for OMG Ubuntu charm since it isn't going to be in the charm store. However, charms that are landing in the store should be provider agnostic. Since there isn't really any support for cloud storage built in to Juju yet you'll probably need to drop this.

During the db-relation-joined hook you clone off of git:// - I don't think the git protocol does any host check or operates over SSL, it'd be preferred if you used the GitHub https:// url for cloning
.
In the website-relation-joined hook, replace `hostname -f` with `unit-get private-address` as that's the preferred method.

Consider including examples on how to use and configure drupal in the README file

----

During the deployment of the charm I get the following error on adding relation to the database:

2012-04-04 15:21:44,081: unit.hook.api@INFO: Grabbing the latest stable Drupal
2012-04-04 15:21:44,081: hook.output@INFO: {}

2012-04-04 15:21:44,124: hook.output@ERROR: /var/lib/juju/units/drupal-0/charm/hooks/db-relation-changed: line 33: git: command not found

make sure git and anything else used in the charm is included during installation.

There are a few more errors in the charm. At the end of the db-relation-changed hook juju-log calls $pub which doesn't exist. I assume this is supposed to be $hostname (it'll need to be updated).

During installation this error occurs:

http://i.imgur.com/mWr39.png

Brandon, I finally got some time to take a look at this. I am reviewing the one on launchpad,

lp:~imbrandon/charms/oneiric/drupal/trunk/

At revno 11.

If your github version has progressed and they are different, please sync to launchpad so we're on the same page. Overall it looks *great*. I really like the way most of the smarts is left up to nginx/php and this just puts files in place.

==== Fixes needed ====

* install does not set -e
 - Almost all of these commands may fail. Install needs to detect and report errors. set -e will solve that.

* Once you do that, install is not idempotent. While it does not run multiple times, it may have an error and need to be retried:
 - line 9, it uses 'mv'. This will fail on a retry of install. See below where 'install' is used.
 - everywhere "ln -s" is used should be replaced with 'ln -sf' so symlinks are created/overwritten unconditionally.

* Remove all mention of s3cmd, its not used.

* Do the git clone of pressflow in install, not db-relation-changed. There is no need to keep doing that over and over again if the database is unrelated/related again. Think of the scenario where you migrate from a small to a large DB instance. Also you need to check for its existence before doing it, otherwise the hook will fail because it already exists.

* in db-relation-changed:

if [ -f "$config_file_path" ]; then
  juju-log "Drupal is already setup, just silently going away"
  exit 0
fi

Since you don't remove this file on departed/broken, this will cause the config file to not be updated if the relation is removed/added again. I see no reason to have this check, just re-write the config file with the new values every time.

==== Fixes that can wait ====

* start does not detect errors in php5-fpm (it will accidentally detect an error in nginx, because bash reports the exit code of its last command).
* stop also does not properly detect errors
* private_name=`hostname -f` -- line 7 of db-relation-changed, dead code, remove it.

==== Next steps ====

Marking as Incomplete. When your fixes are done, mark it as Fix Committed and I'll review agian.

Thanks!

Fix Commited and Moved to Precise

Looks great, thanks Brandon!