launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging ~cjwatson/turnip:charm-store-rabbitmq-url into turnip:master.

Commit message:
charm: Store RabbitMQ broker URL in unitdata

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/turnip/+git/turnip/+merge/389651

It looks as though amqp relation data is only accessible when running hooks for the amqp relation, so get_rabbitmq_url can fail when called from an nfs relation hook.  Stash the result in unitdata instead so that we can access it from other hooks.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/turnip:charm-store-rabbitmq-url into turnip:master.

diff --git a/charm/layer/turnip-base/lib/charms/turnip/base.py b/charm/layer/turnip-base/lib/charms/turnip/base.py
index 35ebf76..bc2dda3 100644
--- a/charm/layer/turnip-base/lib/charms/turnip/base.py
+++ b/charm/layer/turnip-base/lib/charms/turnip/base.py
@@ -366,6 +366,11 @@ def publish_website(website, name, port):
 
 
 def get_rabbitmq_url():
+    """Get the RabbitMQ URL from relation data.
+
+    The relation data is only accessible from an amqp relation hook, so the
+    caller is expected to store the result for use by other hooks.
+    """
     rabbitmq = endpoint_from_name('amqp')
 
     if not rabbitmq.username() or not rabbitmq.password():
diff --git a/charm/turnip-api/lib/charms/turnip/api.py b/charm/turnip-api/lib/charms/turnip/api.py
index 7635b10..f1e421c 100644
--- a/charm/turnip-api/lib/charms/turnip/api.py
+++ b/charm/turnip-api/lib/charms/turnip/api.py
@@ -10,13 +10,13 @@ from charmhelpers.core import (
     hookenv,
     host,
     templating,
+    unitdata,
     )
 
 from charms.turnip.base import (
     code_dir,
     data_dir,
     data_mount_unit,
-    get_rabbitmq_url,
     logs_dir,
     reload_systemd,
     venv_dir,
@@ -33,7 +33,7 @@ def configure_wsgi():
         'data_mount_unit': data_mount_unit(),
         'logs_dir': logs_dir(),
         'venv_dir': venv_dir(),
-        'celery_broker': get_rabbitmq_url(),
+        'celery_broker': unitdata.kv().get('turnip.amqp.url'),
         })
     if context['wsgi_workers'] == 0:
         context['wsgi_workers'] = cpu_count() * 2 + 1
diff --git a/charm/turnip-api/reactive/turnip-api.py b/charm/turnip-api/reactive/turnip-api.py
index dc304cf..60c156f 100644
--- a/charm/turnip-api/reactive/turnip-api.py
+++ b/charm/turnip-api/reactive/turnip-api.py
@@ -3,7 +3,10 @@
 
 from __future__ import absolute_import, print_function, unicode_literals
 
-from charmhelpers.core import hookenv
+from charmhelpers.core import (
+    hookenv,
+    unitdata,
+    )
 from charms.layer import status
 from charms.reactive import (
     clear_flag,
@@ -18,11 +21,12 @@ from charms.turnip.api import configure_wsgi
 from charms.turnip.base import (
     configure_service,
     deconfigure_service,
+    get_rabbitmq_url,
     publish_website,
     )
 
 
-@when('turnip.installed', 'turnip.storage.available', 'amqp.available')
+@when('turnip.installed', 'turnip.storage.available', 'turnip.amqp.available')
 @when_not('turnip.configured')
 def configure_turnip():
     configure_wsgi()
@@ -35,7 +39,7 @@ def configure_turnip():
 
 
 @when('turnip.configured')
-@when_not_all('turnip.storage.available', 'amqp.available')
+@when_not_all('turnip.storage.available', 'turnip.amqp.available')
 def deconfigure_turnip():
     deconfigure_service('turnip-api')
     clear_flag('turnip.configured')
@@ -58,6 +62,20 @@ def unrequest_amqp_access():
     clear_flag('turnip.amqp.requested-access')
 
 
+@when('amqp.available')
+@when_not('turnip.amqp.available')
+def get_amqp_broker(rabbitmq):
+    unitdata.kv().set('turnip.amqp.url', get_rabbitmq_url())
+    set_flag('turnip.amqp.available')
+
+
+@when('turnip.amqp.available')
+@when_not('amqp.available')
+def clear_amqp_broker():
+    unitdata.kv().unset('turnip.amqp.url')
+    clear_flag('turnip.amqp.available')
+
+
 @when('nrpe-external-master.available', 'turnip.configured')
 @when_not('turnip.nrpe-external-master.published')
 def nrpe_available():
diff --git a/charm/turnip-celery/lib/charms/turnip/celery.py b/charm/turnip-celery/lib/charms/turnip/celery.py
index 8120e77..4217c25 100644
--- a/charm/turnip-celery/lib/charms/turnip/celery.py
+++ b/charm/turnip-celery/lib/charms/turnip/celery.py
@@ -8,13 +8,13 @@ from charmhelpers.core import (
     hookenv,
     host,
     templating,
+    unitdata,
     )
 
 from charms.turnip.base import (
     code_dir,
     data_dir,
     data_mount_unit,
-    get_rabbitmq_url,
     logs_dir,
     reload_systemd,
     venv_dir,
@@ -31,7 +31,7 @@ def configure_celery():
         'data_mount_unit': data_mount_unit(),
         'logs_dir': logs_dir(),
         'venv_dir': venv_dir(),
-        'celery_broker': get_rabbitmq_url(),
+        'celery_broker': unitdata.kv().get('turnip.amqp.url'),
         })
     templating.render(
         'turnip-celery.service.j2',
diff --git a/charm/turnip-celery/reactive/turnip-celery.py b/charm/turnip-celery/reactive/turnip-celery.py
index 84e3513..f13d8f3 100644
--- a/charm/turnip-celery/reactive/turnip-celery.py
+++ b/charm/turnip-celery/reactive/turnip-celery.py
@@ -3,7 +3,10 @@
 
 from __future__ import absolute_import, print_function, unicode_literals
 
-from charmhelpers.core import hookenv
+from charmhelpers.core import (
+    hookenv,
+    unitdata,
+    )
 from charms.layer import status
 from charms.reactive import (
     clear_flag,
@@ -18,10 +21,11 @@ from charms.turnip.celery import configure_celery
 from charms.turnip.base import (
     configure_service,
     deconfigure_service,
+    get_rabbitmq_url,
     )
 
 
-@when('turnip.installed', 'turnip.storage.available', 'amqp.available')
+@when('turnip.installed', 'turnip.storage.available', 'turnip.amqp.available')
 @when_not('turnip.configured')
 def configure_turnip():
     configure_celery()
@@ -34,7 +38,7 @@ def configure_turnip():
 
 
 @when('turnip.configured')
-@when_not_all('turnip.storage.available', 'amqp.available')
+@when_not_all('turnip.storage.available', 'turnip.amqp.available')
 def deconfigure_turnip():
     deconfigure_service('turnip-celery')
     clear_flag('turnip.configured')
@@ -57,6 +61,20 @@ def unrequest_amqp_access():
     clear_flag('turnip.amqp.requested-access')
 
 
+@when('amqp.available')
+@when_not('turnip.amqp.available')
+def get_amqp_broker(rabbitmq):
+    unitdata.kv().set('turnip.amqp.url', get_rabbitmq_url())
+    set_flag('turnip.amqp.available')
+
+
+@when('turnip.amqp.available')
+@when_not('amqp.available')
+def clear_amqp_broker():
+    unitdata.kv().unset('turnip.amqp.url')
+    clear_flag('turnip.amqp.available')
+
+
 @when('nrpe-external-master.available', 'turnip.configured')
 @when_not('turnip.nrpe-external-master.published')
 def nrpe_available():