Merge lp:~matsubara/charms/precise/oops-tools/trunk into lp:charms/oops-tools
- Precise Pangolin (12.04)
- trunk
- Merge into trunk
Status: | Merged |
---|---|
Merged at revision: | 12 |
Proposed branch: | lp:~matsubara/charms/precise/oops-tools/trunk |
Merge into: | lp:charms/oops-tools |
Diff against target: |
200 lines (+134/-10) 7 files modified
.bzrignore (+1/-0) README.txt (+7/-0) config.yaml (+33/-0) hooks/amqp-relation-changed (+62/-0) hooks/db-relation-changed (+12/-10) hooks/db-relation-joined (+16/-0) metadata.yaml (+3/-0) |
To merge this branch: | bzr merge lp:~matsubara/charms/precise/oops-tools/trunk |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Clint Byrum (community) | Needs Fixing | ||
Review via email: mp+112017@code.launchpad.net |
Commit message
Add basic README, support to use amqp and list Diogo Matsubara as the maintainer of the charm.
Description of the change
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
Diogo Matsubara (matsubara) wrote : | # |
> 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-
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-
> 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) wrote : | # |
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-
>
> 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-
I think I just must have missed that. Will review again once the check
for empty relation values is restored.
- 12. By Diogo Matsubara
-
restore check for relation values
Diogo Matsubara (matsubara) wrote : | # |
> 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-
> 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
Preview Diff
1 | === added file '.bzrignore' | |||
2 | --- .bzrignore 1970-01-01 00:00:00 +0000 | |||
3 | +++ .bzrignore 2012-07-03 07:24:22 +0000 | |||
4 | @@ -0,0 +1,1 @@ | |||
5 | 1 | local.yaml | ||
6 | 0 | 2 | ||
7 | === added file 'README.txt' | |||
8 | --- README.txt 1970-01-01 00:00:00 +0000 | |||
9 | +++ README.txt 2012-07-03 07:24:22 +0000 | |||
10 | @@ -0,0 +1,7 @@ | |||
11 | 1 | To deploy: | ||
12 | 2 | |||
13 | 3 | juju deploy --repository=. local:oops-tools | ||
14 | 4 | juju deploy --repository=. local:postgresql | ||
15 | 5 | juju add-relation postgresql:db oops-tools | ||
16 | 6 | juju add-relation rabbitmq oops-tools | ||
17 | 7 | juju expose oops-tools | ||
18 | 0 | 8 | ||
19 | === added file 'config.yaml' | |||
20 | --- config.yaml 1970-01-01 00:00:00 +0000 | |||
21 | +++ config.yaml 2012-07-03 07:24:22 +0000 | |||
22 | @@ -0,0 +1,33 @@ | |||
23 | 1 | options: | ||
24 | 2 | oops-web-amqp-host: | ||
25 | 3 | type: string | ||
26 | 4 | default: "localhost" | ||
27 | 5 | description: The rabbitMQ server. | ||
28 | 6 | oops-web-amqp-user: | ||
29 | 7 | type: string | ||
30 | 8 | default: 'oopstools' | ||
31 | 9 | description: The user to log into the rabbitMQ server. | ||
32 | 10 | oops-web-amqp-password: | ||
33 | 11 | type: string | ||
34 | 12 | default: 'guest' | ||
35 | 13 | description: The password for the user to log into the rabbitMQ server. | ||
36 | 14 | oops-web-amqp-vhost: | ||
37 | 15 | type: string | ||
38 | 16 | default: '/' | ||
39 | 17 | description: The vhost in the rabbitMQ server. | ||
40 | 18 | oops-web-amqp-exchange: | ||
41 | 19 | type: string | ||
42 | 20 | default: 'oopses' | ||
43 | 21 | description: The exchange to use on the rabbitMQ server. | ||
44 | 22 | oops-web-amqp-routing: | ||
45 | 23 | type: string | ||
46 | 24 | default: '' | ||
47 | 25 | description: The routing key to use on the rabbitMQ server. | ||
48 | 26 | db-user: | ||
49 | 27 | type: string | ||
50 | 28 | default: 'oopstools' | ||
51 | 29 | description: The database user for oops-tools | ||
52 | 30 | db-name: | ||
53 | 31 | type: string | ||
54 | 32 | default: 'oopstools' | ||
55 | 33 | description: The database name for oops-tools | ||
56 | 0 | 34 | ||
57 | === added file 'hooks/amqp-relation-changed' | |||
58 | --- hooks/amqp-relation-changed 1970-01-01 00:00:00 +0000 | |||
59 | +++ hooks/amqp-relation-changed 2012-07-03 07:24:22 +0000 | |||
60 | @@ -0,0 +1,62 @@ | |||
61 | 1 | #!/bin/bash | ||
62 | 2 | set -eux | ||
63 | 3 | OOPS_HOME="/var/www/oops-tools" | ||
64 | 4 | OOPS_DIR="$OOPS_HOME/logs/oops-amqp/" | ||
65 | 5 | if [[ ! -d $OOPS_DIR ]]; then | ||
66 | 6 | mkdir -p $OOPS_DIR | ||
67 | 7 | fi | ||
68 | 8 | RABBIT_USER=`config-get oops-web-amqp-user` | ||
69 | 9 | RABBIT_VHOST=`config-get oops-web-amqp-vhost` | ||
70 | 10 | RABBIT_EXCHANGE=`config-get oops-web-amqp-exchange` | ||
71 | 11 | RABBIT_ROUTING=`config-get oops-web-amqp-exchange` | ||
72 | 12 | |||
73 | 13 | juju-log "amqp_joined: requesting credentials for $RABBIT_USER" | ||
74 | 14 | echo "amqp_joined: requesting credentials for $RABBIT_USER" | ||
75 | 15 | relation-set username=$RABBIT_USER | ||
76 | 16 | relation-set vhost=$RABBIT_VHOST | ||
77 | 17 | |||
78 | 18 | RABBIT_HOST=`relation-get private-address` | ||
79 | 19 | RABBIT_PASSWORD=`relation-get password` | ||
80 | 20 | if [[ -z $RABBIT_HOST ]] || [[ -z $RABBIT_PASSWORD ]] ; then | ||
81 | 21 | echo "amqp_changed: RABBIT_HOST||RABBIT_PASSWORD not set. Peer not ready? Exit 0 and retry" | ||
82 | 22 | exit 0 | ||
83 | 23 | fi | ||
84 | 24 | echo "amqp_changed: Setting rabbit config on production.cfg: $RABBIT_HOST $RABBIT_USER $RABBIT_PASSWORD" | ||
85 | 25 | # Use # as the separator since $OOPS_DIR has slashes on it | ||
86 | 26 | sed -i "s#^oopsdir =.*#oopsdir = $OOPS_DIR#g" $OOPS_HOME/production.cfg | ||
87 | 27 | sed -i "s/^oops-web-amqp-host =.*$/oops-web-amqp-host = $RABBIT_HOST/g" \ | ||
88 | 28 | $OOPS_HOME/production.cfg | ||
89 | 29 | sed -i "s/^oops-web-amqp-user =.*$/oops-web-amqp-user = $RABBIT_USER/g" \ | ||
90 | 30 | $OOPS_HOME/production.cfg | ||
91 | 31 | sed -i "s/^oops-web-amqp-password =.*$/oops-web-amqp-password = $RABBIT_PASSWORD/g" \ | ||
92 | 32 | $OOPS_HOME/production.cfg | ||
93 | 33 | # Use # as the separator since $RABBIT_VHOST has slashes on it | ||
94 | 34 | sed -i "s#^oops-web-amqp-vhost =.*#oops-web-amqp-vhost = $RABBIT_VHOST#g" \ | ||
95 | 35 | $OOPS_HOME/production.cfg | ||
96 | 36 | sed -i \ | ||
97 | 37 | "s/^oops-web-amqp-exchange =.*$/oops-web-amqp-exchange = $RABBIT_EXCHANGE/g" \ | ||
98 | 38 | $OOPS_HOME/production.cfg | ||
99 | 39 | sed -i \ | ||
100 | 40 | "s/^oops-web-amqp-routing =.*$/oops-web-amqp-routing = $RABBIT_ROUTING/g" \ | ||
101 | 41 | $OOPS_HOME/production.cfg | ||
102 | 42 | # Remove old settings.py | ||
103 | 43 | rm -f $OOPS_HOME/src/oopstools/settings.py | ||
104 | 44 | # Update settings.py from production.cfg | ||
105 | 45 | $OOPS_HOME/bin/buildout -c $OOPS_HOME/production.cfg | ||
106 | 46 | # Reload oops-tools | ||
107 | 47 | service apache2 restart | ||
108 | 48 | # Setup the amqp2disk script. | ||
109 | 49 | # First kill any amqp2disk scripts running. | ||
110 | 50 | AMQP2DISK_PID=`ps afx|grep amqp2disk | sed '/grep/d' | awk '{print $1}'` | ||
111 | 51 | if [[ -n $AMQP2DISK_PID ]]; then | ||
112 | 52 | kill -9 $AMQP2DISK_PID | ||
113 | 53 | fi | ||
114 | 54 | |||
115 | 55 | # Now run it with the updated config. | ||
116 | 56 | $OOPS_HOME/bin/amqp2disk --host $RABBIT_HOST \ | ||
117 | 57 | --output $OOPS_DIR \ | ||
118 | 58 | --username $RABBIT_USER \ | ||
119 | 59 | --password $RABBIT_PASSWORD \ | ||
120 | 60 | --vhost $RABBIT_VHOST \ | ||
121 | 61 | --queue $RABBIT_EXCHANGE \ | ||
122 | 62 | --bind-to $RABBIT_EXCHANGE & | ||
123 | 0 | 63 | ||
124 | === modified file 'hooks/db-relation-changed' | |||
125 | --- hooks/db-relation-changed 2012-01-19 00:54:52 +0000 | |||
126 | +++ hooks/db-relation-changed 2012-07-03 07:24:22 +0000 | |||
127 | @@ -1,22 +1,24 @@ | |||
128 | 1 | #!/bin/bash | 1 | #!/bin/bash |
129 | 2 | set -eux # -x for verbose logging to juju debug-log | 2 | set -eux # -x for verbose logging to juju debug-log |
130 | 3 | hooksdir=$PWD | 3 | hooksdir=$PWD |
135 | 4 | user=`relation-get user` | 4 | USER=`relation-get user` |
136 | 5 | password=`relation-get password` | 5 | DATABASE=`relation-get database` |
137 | 6 | host=`relation-get host` | 6 | HOST=`relation-get host` |
138 | 7 | database=`relation-get database` | 7 | PASSWORD=`relation-get password` |
139 | 8 | |||
140 | 8 | # All values are set together, so checking on a single value is enough | 9 | # All values are set together, so checking on a single value is enough |
141 | 9 | # If $user is not set, DB is still setting itself up, we exit awaiting next run | 10 | # If $user is not set, DB is still setting itself up, we exit awaiting next run |
142 | 10 | [ -z "$user" ] && exit 0 | 11 | [ -z "$user" ] && exit 0 |
149 | 11 | juju-log "Setting up Oops tools for the first time" | 12 | |
150 | 12 | # Write production.cfg | 13 | # Update oops-tools config with db relation values. |
151 | 13 | sed -i "s/^db-name =.*$/db-name = $database/g" /var/www/oops-tools/production.cfg | 14 | sed -i "s/^db-name =.*$/db-name = $DATABASE/g" /var/www/oops-tools/production.cfg |
152 | 14 | sed -i "s/^db-host =.*$/db-host = $host/g" /var/www/oops-tools/production.cfg | 15 | sed -i "s/^db-host =.*$/db-host = $HOST/g" /var/www/oops-tools/production.cfg |
153 | 15 | sed -i "s/^db-user =.*$/db-user = $user/g" /var/www/oops-tools/production.cfg | 16 | sed -i "s/^db-user =.*$/db-user = $USER/g" /var/www/oops-tools/production.cfg |
154 | 16 | sed -i "s/^db-password =.*$/db-password = $password/g" /var/www/oops-tools/production.cfg | 17 | sed -i "s/^db-password =.*$/db-password = $PASSWORD/g" /var/www/oops-tools/production.cfg |
155 | 17 | sed -i "s/^db-port =.*$/db-port = 5432/g" /var/www/oops-tools/production.cfg | 18 | sed -i "s/^db-port =.*$/db-port = 5432/g" /var/www/oops-tools/production.cfg |
156 | 18 | cd /var/www/oops-tools | 19 | cd /var/www/oops-tools |
157 | 19 | rm -f src/oopstools/settings.py | 20 | rm -f src/oopstools/settings.py |
158 | 21 | # Update settings.py | ||
159 | 20 | bin/buildout -c production.cfg | 22 | bin/buildout -c production.cfg |
160 | 21 | bin/django syncdb --noinput | 23 | bin/django syncdb --noinput |
161 | 22 | bin/django migrate | 24 | bin/django migrate |
162 | 23 | 25 | ||
163 | === added file 'hooks/db-relation-joined' | |||
164 | --- hooks/db-relation-joined 1970-01-01 00:00:00 +0000 | |||
165 | +++ hooks/db-relation-joined 2012-07-03 07:24:22 +0000 | |||
166 | @@ -0,0 +1,16 @@ | |||
167 | 1 | #!/bin/bash | ||
168 | 2 | set -eux # -x for verbose logging to juju debug-log | ||
169 | 3 | hooksdir=$PWD | ||
170 | 4 | OOPS_HOME=/var/www/oops-tools | ||
171 | 5 | USER=`config-get db-user` | ||
172 | 6 | DATABASE=`config-get db-name` | ||
173 | 7 | relation-set user=$USER | ||
174 | 8 | relation-set database=$DATABASE | ||
175 | 9 | $HOST=`relation-get host` | ||
176 | 10 | $PASSWORD=`relation-get password` | ||
177 | 11 | |||
178 | 12 | sed -i "s/^db-name =.*$/db-name = $DATABASE/g" $OOPS_HOME/production.cfg | ||
179 | 13 | sed -i "s/^db-host =.*$/db-host = $HOST/g" $OOPS_HOME/production.cfg | ||
180 | 14 | sed -i "s/^db-user =.*$/db-user = $USER/g" $OOPS_HOME/production.cfg | ||
181 | 15 | sed -i "s/^db-password =.*$/db-password = $PASSWORD/g" $OOPS_HOME/production.cfg | ||
182 | 16 | sed -i "s/^db-port =.*$/db-port = 5433/g" $OOPS_HOME/production.cfg | ||
183 | 0 | 17 | ||
184 | === modified file 'metadata.yaml' | |||
185 | --- metadata.yaml 2012-01-30 18:54:42 +0000 | |||
186 | +++ metadata.yaml 2012-07-03 07:24:22 +0000 | |||
187 | @@ -1,10 +1,13 @@ | |||
188 | 1 | name: oops-tools | 1 | name: oops-tools |
189 | 2 | maintainer: Diogo Matsubara <diogo.matsubara@canonical.com> | ||
190 | 2 | summary: "Oops tools" | 3 | summary: "Oops tools" |
191 | 3 | description: | | 4 | description: | |
192 | 4 | Django app to analyse and aggregate OOPS reports | 5 | Django app to analyse and aggregate OOPS reports |
193 | 5 | requires: | 6 | requires: |
194 | 6 | db: | 7 | db: |
195 | 7 | interface: pgsql | 8 | interface: pgsql |
196 | 9 | amqp: | ||
197 | 10 | interface: rabbitmq | ||
198 | 8 | provides: | 11 | provides: |
199 | 9 | website: | 12 | website: |
200 | 10 | interface: http | 13 | interface: http |
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.