Merge lp:~matsubara/charms/precise/oops-tools/trunk into lp:charms/oops-tools

[Diogo Matsubara]

Add basic README, support to use amqp and list Diogo Matsubara as the maintainer of the charm.

Hi,

this branch adds a README file, myself as the maintainer and support to use an amqp queue.

charm proof doesn't return anything, so I think it's free of lint.

Cheers,

Diogo

[Clint Byrum (clint-fewbar)]

Diogo, this does not look right.

I see config-get's mixed with relation-get's in db-relation-changed/joined.

There is a rabbitmq charm, so I'd like to see a rabbitmq relation in addition to the config.yaml values for the external AMQP server.

Also the check for empty relation values seems to have been removed in db-relation-changed, which may cause race conditions since we may run that hook before the database server has set the values we want.

review: Needs Fixing

[Diogo Matsubara (matsubara)]

> Diogo, this does not look right.

Hi Clint, thanks for the review

>
> I see config-get's mixed with relation-get's in db-relation-changed/joined.

The way I understand the workflow is:

db-relation-joined sets the values for USER and DATABASE from the config values and pass those to the postgresql charm
Once postgresql charm finishes doing its job, it triggers the db-relation-changed and oops-tools sets its configs from those values. Before, oops-tools charm didn't have a db-relation-joined hook, so that's why there was a check if the user was set. Now that it has a joined and changed hooks, when the changed runs I can safely assume the db charm has been run.
Is this a misunderstanding on my part? Please help clarify so I can fix this.

>
> There is a rabbitmq charm, so I'd like to see a rabbitmq relation in addition
> to the config.yaml values for the external AMQP server.
>

Not sure I understand this, I added a amqp-relation-changed hook. What else is missing?

> Also the check for empty relation values seems to have been removed in db-
> relation-changed, which may cause race conditions since we may run that hook
> before the database server has set the values we want.

Explained above.

Cheers,

Diogo

[Clint Byrum (clint-fewbar)]

Excerpts from Diogo Matsubara's message of 2012-07-02 08:40:28 UTC:
> > Diogo, this does not look right.
>
> Hi Clint, thanks for the review
>
> >
> > I see config-get's mixed with relation-get's in db-relation-changed/joined.
>
> The way I understand the workflow is:
>
> db-relation-joined sets the values for USER and DATABASE from the config values and pass those to the postgresql charm
> Once postgresql charm finishes doing its job, it triggers the db-relation-changed and oops-tools sets its configs from those values. Before, oops-tools charm didn't have a db-relation-joined hook, so that's why there was a check if the user was set. Now that it has a joined and changed hooks, when the changed runs I can safely assume the db charm has been run.

Ahh, no you cannot make this assumption. joined and changed *always*
run in series at the beginning of a relationship. You cannot assume that
the value has been set when changed runs, as it may just be running in
that initial first-pass mode before relation-set has happened on the
other side. It will be run again when the values are set.

> Is this a misunderstanding on my part? Please help clarify so I can fix this.
>
> >
> > There is a rabbitmq charm, so I'd like to see a rabbitmq relation in addition
> > to the config.yaml values for the external AMQP server.
> >
>
> Not sure I understand this, I added a amqp-relation-changed hook. What else is missing?

I think I just must have missed that. Will review again once the check
for empty relation values is restored.

[lp:~matsubara/charms/precise/oops-tools/trunk]

12. By Diogo Matsubara on 2012-07-03

restore check for relation values

[Diogo Matsubara (matsubara)]

> Excerpts from Diogo Matsubara's message of 2012-07-02 08:40:28 UTC:
> > > Diogo, this does not look right.
> >
> > Hi Clint, thanks for the review
> >
> > >
> > > I see config-get's mixed with relation-get's in db-relation-
> changed/joined.
> >
> > The way I understand the workflow is:
> >
> > db-relation-joined sets the values for USER and DATABASE from the config
> values and pass those to the postgresql charm
> > Once postgresql charm finishes doing its job, it triggers the db-relation-
> changed and oops-tools sets its configs from those values. Before, oops-tools
> charm didn't have a db-relation-joined hook, so that's why there was a check
> if the user was set. Now that it has a joined and changed hooks, when the
> changed runs I can safely assume the db charm has been run.
>
> Ahh, no you cannot make this assumption. joined and changed *always*
> run in series at the beginning of a relationship. You cannot assume that
> the value has been set when changed runs, as it may just be running in
> that initial first-pass mode before relation-set has happened on the
> other side. It will be run again when the values are set.

Thanks! I restored the check in changed.

>
> > Is this a misunderstanding on my part? Please help clarify so I can fix
> this.
> >
> > >
> > > There is a rabbitmq charm, so I'd like to see a rabbitmq relation in
> addition
> > > to the config.yaml values for the external AMQP server.
> > >
> >
> > Not sure I understand this, I added a amqp-relation-changed hook. What else
> is missing?
>
> I think I just must have missed that. Will review again once the check
> for empty relation values is restored.

Ok, let me know if anything else is needed.

Cheers,

Diogo

=== added file '.bzrignore'
--- .bzrignore	1970-01-01 00:00:00 +0000
+++ .bzrignore	2012-07-03 07:24:22 +0000
@@ -0,0 +1,1 @@
+local.yaml

=== added file 'README.txt'
--- README.txt	1970-01-01 00:00:00 +0000
+++ README.txt	2012-07-03 07:24:22 +0000
@@ -0,0 +1,7 @@
+To deploy:
+
+juju deploy --repository=. local:oops-tools
+juju deploy --repository=. local:postgresql
+juju add-relation postgresql:db oops-tools
+juju add-relation rabbitmq oops-tools
+juju expose oops-tools

=== added file 'config.yaml'
--- config.yaml	1970-01-01 00:00:00 +0000
+++ config.yaml	2012-07-03 07:24:22 +0000
@@ -0,0 +1,33 @@
+options:
+    oops-web-amqp-host:
+        type: string
+        default: "localhost"
+        description: The rabbitMQ server.
+    oops-web-amqp-user:
+        type: string
+        default: 'oopstools'
+        description: The user to log into the rabbitMQ server.
+    oops-web-amqp-password:
+        type: string
+        default: 'guest'
+        description: The password for the user to log into the rabbitMQ server.
+    oops-web-amqp-vhost:
+        type: string
+        default: '/'
+        description: The vhost in the rabbitMQ server.
+    oops-web-amqp-exchange:
+        type: string
+        default: 'oopses'
+        description: The exchange to use on the rabbitMQ server.
+    oops-web-amqp-routing:
+        type: string
+        default: ''
+        description: The routing key to use on the rabbitMQ server.
+    db-user:
+        type: string
+        default: 'oopstools'
+        description: The database user for oops-tools
+    db-name:
+        type: string
+        default: 'oopstools'
+        description: The database name for oops-tools

=== added file 'hooks/amqp-relation-changed'
--- hooks/amqp-relation-changed	1970-01-01 00:00:00 +0000
+++ hooks/amqp-relation-changed	2012-07-03 07:24:22 +0000
@@ -0,0 +1,62 @@
+#!/bin/bash
+set -eux
+OOPS_HOME="/var/www/oops-tools"
+OOPS_DIR="$OOPS_HOME/logs/oops-amqp/"
+if [[ ! -d $OOPS_DIR ]]; then
+    mkdir -p $OOPS_DIR
+fi
+RABBIT_USER=`config-get oops-web-amqp-user`
+RABBIT_VHOST=`config-get oops-web-amqp-vhost`
+RABBIT_EXCHANGE=`config-get oops-web-amqp-exchange`
+RABBIT_ROUTING=`config-get oops-web-amqp-exchange`
+
+juju-log "amqp_joined: requesting credentials for $RABBIT_USER"
+echo "amqp_joined: requesting credentials for $RABBIT_USER"
+relation-set username=$RABBIT_USER
+relation-set vhost=$RABBIT_VHOST
+
+RABBIT_HOST=`relation-get private-address`
+RABBIT_PASSWORD=`relation-get password`
+if [[ -z $RABBIT_HOST ]] || [[ -z $RABBIT_PASSWORD ]] ; then
+  echo "amqp_changed: RABBIT_HOST||RABBIT_PASSWORD not set. Peer not ready? Exit 0 and retry"
+  exit 0
+fi
+echo "amqp_changed: Setting rabbit config on production.cfg: $RABBIT_HOST $RABBIT_USER $RABBIT_PASSWORD"
+# Use # as the separator since $OOPS_DIR has slashes on it
+sed -i "s#^oopsdir =.*#oopsdir = $OOPS_DIR#g" $OOPS_HOME/production.cfg
+sed -i "s/^oops-web-amqp-host =.*$/oops-web-amqp-host = $RABBIT_HOST/g" \
+    $OOPS_HOME/production.cfg
+sed -i "s/^oops-web-amqp-user =.*$/oops-web-amqp-user = $RABBIT_USER/g" \
+    $OOPS_HOME/production.cfg
+sed -i "s/^oops-web-amqp-password =.*$/oops-web-amqp-password = $RABBIT_PASSWORD/g" \
+    $OOPS_HOME/production.cfg
+# Use # as the separator since $RABBIT_VHOST has slashes on it
+sed -i "s#^oops-web-amqp-vhost =.*#oops-web-amqp-vhost = $RABBIT_VHOST#g" \
+    $OOPS_HOME/production.cfg
+sed -i \
+    "s/^oops-web-amqp-exchange =.*$/oops-web-amqp-exchange = $RABBIT_EXCHANGE/g" \
+    $OOPS_HOME/production.cfg
+sed -i \
+    "s/^oops-web-amqp-routing =.*$/oops-web-amqp-routing = $RABBIT_ROUTING/g" \
+    $OOPS_HOME/production.cfg
+# Remove old settings.py
+rm -f $OOPS_HOME/src/oopstools/settings.py
+# Update settings.py from production.cfg
+$OOPS_HOME/bin/buildout -c $OOPS_HOME/production.cfg
+# Reload oops-tools
+service apache2 restart
+# Setup the amqp2disk script.
+# First kill any amqp2disk scripts running.
+AMQP2DISK_PID=`ps afx|grep amqp2disk | sed '/grep/d' | awk '{print $1}'`
+if [[ -n $AMQP2DISK_PID ]]; then
+    kill -9 $AMQP2DISK_PID
+fi
+
+# Now run it with the updated config.
+$OOPS_HOME/bin/amqp2disk --host $RABBIT_HOST \
+    --output $OOPS_DIR \
+    --username $RABBIT_USER \
+    --password $RABBIT_PASSWORD \
+    --vhost $RABBIT_VHOST \
+    --queue $RABBIT_EXCHANGE \
+    --bind-to $RABBIT_EXCHANGE &

=== modified file 'hooks/db-relation-changed'
--- hooks/db-relation-changed	2012-01-19 00:54:52 +0000
+++ hooks/db-relation-changed	2012-07-03 07:24:22 +0000
@@ -1,22 +1,24 @@
 #!/bin/bash
 set -eux # -x for verbose logging to juju debug-log
 hooksdir=$PWD
-user=`relation-get user`
-password=`relation-get password`
-host=`relation-get host`
-database=`relation-get database`
+USER=`relation-get user`
+DATABASE=`relation-get database`
+HOST=`relation-get host`
+PASSWORD=`relation-get password`
+
 # All values are set together, so checking on a single value is enough
 # If $user is not set, DB is still setting itself up, we exit awaiting next run
 [ -z "$user" ] && exit 0
-juju-log "Setting up Oops tools for the first time"
-# Write production.cfg
-sed -i "s/^db-name =.*$/db-name = $database/g" /var/www/oops-tools/production.cfg
-sed -i "s/^db-host =.*$/db-host = $host/g" /var/www/oops-tools/production.cfg
-sed -i "s/^db-user =.*$/db-user = $user/g" /var/www/oops-tools/production.cfg
-sed -i "s/^db-password =.*$/db-password = $password/g" /var/www/oops-tools/production.cfg
+
+# Update oops-tools config with db relation values.
+sed -i "s/^db-name =.*$/db-name = $DATABASE/g" /var/www/oops-tools/production.cfg
+sed -i "s/^db-host =.*$/db-host = $HOST/g" /var/www/oops-tools/production.cfg
+sed -i "s/^db-user =.*$/db-user = $USER/g" /var/www/oops-tools/production.cfg
+sed -i "s/^db-password =.*$/db-password = $PASSWORD/g" /var/www/oops-tools/production.cfg
 sed -i "s/^db-port =.*$/db-port = 5432/g" /var/www/oops-tools/production.cfg
 cd /var/www/oops-tools
 rm -f src/oopstools/settings.py
+# Update settings.py
 bin/buildout -c production.cfg
 bin/django syncdb --noinput
 bin/django migrate

=== added file 'hooks/db-relation-joined'
--- hooks/db-relation-joined	1970-01-01 00:00:00 +0000
+++ hooks/db-relation-joined	2012-07-03 07:24:22 +0000
@@ -0,0 +1,16 @@
+#!/bin/bash
+set -eux # -x for verbose logging to juju debug-log
+hooksdir=$PWD
+OOPS_HOME=/var/www/oops-tools
+USER=`config-get db-user`
+DATABASE=`config-get db-name`
+relation-set user=$USER
+relation-set database=$DATABASE
+$HOST=`relation-get host`
+$PASSWORD=`relation-get password`
+
+sed -i "s/^db-name =.*$/db-name = $DATABASE/g" $OOPS_HOME/production.cfg
+sed -i "s/^db-host =.*$/db-host = $HOST/g" $OOPS_HOME/production.cfg
+sed -i "s/^db-user =.*$/db-user = $USER/g" $OOPS_HOME/production.cfg
+sed -i "s/^db-password =.*$/db-password = $PASSWORD/g" $OOPS_HOME/production.cfg
+sed -i "s/^db-port =.*$/db-port = 5433/g" $OOPS_HOME/production.cfg

=== modified file 'metadata.yaml'
--- metadata.yaml	2012-01-30 18:54:42 +0000
+++ metadata.yaml	2012-07-03 07:24:22 +0000
@@ -1,10 +1,13 @@
 name: oops-tools
+maintainer: Diogo Matsubara <[e-mail]>
 summary: "Oops tools"
 description: |
     Django app to analyse and aggregate OOPS reports
 requires:
   db:
     interface: pgsql
+  amqp:
+    interface: rabbitmq
 provides:
   website:
     interface: http