Merge ~cjwatson/lp-mailman:kombu-amqp into lp-mailman:master

Proposed by Colin Watson
Status: Merged
Approved by: Colin Watson
Approved revision: 707070eb8c7c60330a80f2c4db112621feb7a82c
Merge reported by: Otto Co-Pilot
Merged at revision: not available
Proposed branch: ~cjwatson/lp-mailman:kombu-amqp
Merge into: lp-mailman:master
Diff against target: 141 lines (+33/-16)
6 files modified
configs/development/mailman-lazr.conf (+1/-4)
configs/testrunner/mailman-lazr.conf (+1/-4)
constraints.txt (+3/-2)
lib/lp/services/config/schema-lazr.conf (+12/-1)
lib/lp/services/messaging/rabbit.py (+15/-5)
setup.py (+1/-0)
Reviewer Review Type Date Requested Status
Simone Pelosi Approve
Review via email: mp+452286@code.launchpad.net

Commit message

Support multiple RabbitMQ broker URLs

Description of the change

`kombu` is a higher-level messaging library than `amqp`. For our purposes, we can treat it mostly as a wrapper for `amqp` with some slightly more convenient interfaces, but it has one key feature that's of interest to us: it supports multiple RabbitMQ broker URLs with round-robin failover between them. This makes it possible to configure lp-mailman to use RabbitMQ with high availability: if we have a RabbitMQ cluster, then we can configure lp-mailman with broker URLs for all the nodes in the cluster, and if one fails then `kombu` will automatically fail over to the next.

To make it practical to configure this, I had to add a `rabbitmq.broker_urls` configuration option, which supersedes the existing broken-out configuration options (`rabbitmq.host`, `rabbitmq.userid`, `rabbitmq.password`, and `rabbitmq.virtual_host`). For backward compatibility, the old options continue to work as long as `rabbitmq.broker_urls` is unset.

This also includes upgrading to oops-amqp 0.2.0, since that includes a change to accept connection factories that return `kombu` connections.

This is a reduced version of https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/432065 in Launchpad, picked up when I realized that it was going to be needed by the lp-mailman charm. Testing is difficult since we stripped out much of the supporting code when extracting lp-mailman from Launchpad, but at least similar code is already tested as part of Launchpad.

Dependencies MP: https://code.launchpad.net/~cjwatson/lp-mailman/+git/dependencies/+merge/452285

To post a comment you must log in.
Revision history for this message
Simone Pelosi (pelpsi) wrote :

LGTM!

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/configs/development/mailman-lazr.conf b/configs/development/mailman-lazr.conf
2index 563b52a..b20036b 100644
3--- a/configs/development/mailman-lazr.conf
4+++ b/configs/development/mailman-lazr.conf
5@@ -30,7 +30,4 @@ hard_max_size: 1000000
6
7 [rabbitmq]
8 launch: True
9-host: localhost:56720
10-userid: guest
11-password: guest
12-virtual_host: /
13+broker_urls: amqp://guest:guest@localhost:56720//
14diff --git a/configs/testrunner/mailman-lazr.conf b/configs/testrunner/mailman-lazr.conf
15index 977ba21..37ab96e 100644
16--- a/configs/testrunner/mailman-lazr.conf
17+++ b/configs/testrunner/mailman-lazr.conf
18@@ -14,7 +14,4 @@ shared_secret: topsecret
19
20 [rabbitmq]
21 launch: False
22-host: none
23-userid: none
24-password: none
25-virtual_host: none
26+broker_urls: none
27diff --git a/constraints.txt b/constraints.txt
28index 1680cb0..b3e34a0 100644
29--- a/constraints.txt
30+++ b/constraints.txt
31@@ -165,7 +165,7 @@ zc.recipe.testrunner==2.1
32
33 # Alphabetical, case-insensitive, please! :-)
34
35-amqp==2.4.2
36+amqp==2.6.1
37 bson==0.5.9
38 contextlib2==0.6.0.post1
39 defusedxml==0.6.0
40@@ -173,6 +173,7 @@ distro==1.4.0
41 httplib2==0.8
42 iso8601==0.1.12
43 keyring==0.6.2
44+kombu==4.6.3
45 launchpadlib==1.10.9
46 lazr.config==2.2.2
47 lazr.delegates==2.0.4
48@@ -182,7 +183,7 @@ lazr.uri==1.0.3
49 mock==1.0.1
50 oauthlib==3.1.0
51 oops==0.0.14
52-oops-amqp==0.1.0
53+oops-amqp==0.2.0
54 oops-datedir-repo==0.0.24
55 oops-datedir2amqp==0.1.0
56 python-dateutil==2.8.1
57diff --git a/lib/lp/services/config/schema-lazr.conf b/lib/lp/services/config/schema-lazr.conf
58index 85b6330..6df2709 100644
59--- a/lib/lp/services/config/schema-lazr.conf
60+++ b/lib/lp/services/config/schema-lazr.conf
61@@ -168,12 +168,23 @@ build_host_name:
62 # Should RabbitMQ be launched by default?
63 # datatype: boolean
64 launch: False
65-# The host:port at which RabbitMQ is listening.
66+# The URL at which RabbitMQ is listening (in the form
67+# amqp://USERNAME:PASSWORD@HOSTNAME:PORT/VIRTUAL_HOST), or a space-separated
68+# list of such URLs to use round-robin failover between them.
69+broker_urls: none
70+# The host:port at which RabbitMQ is listening (ignored if broker_urls is
71+# set).
72 # datatype: string
73 host: none
74+# The username to use when connecting to RabbitMQ (ignored if broker_urls is
75+# set).
76 # datatype: string
77 userid: none
78+# The password to use when connecting to RabbitMQ (ignored if broker_urls is
79+# set).
80 # datatype: string
81 password: none
82+# The virtual host to use when connecting to RabbitMQ (ignored if
83+# broker_urls is set).
84 # datatype: string
85 virtual_host: none
86diff --git a/lib/lp/services/messaging/rabbit.py b/lib/lp/services/messaging/rabbit.py
87index d3cbcaa..72373b4 100644
88--- a/lib/lp/services/messaging/rabbit.py
89+++ b/lib/lp/services/messaging/rabbit.py
90@@ -9,7 +9,8 @@ __all__ = [
91 "is_configured",
92 ]
93
94-import amqp
95+import kombu
96+from lazr.config import as_host_port
97
98 from lp.services.config import config
99 from lp.services.messaging.interfaces import MessagingUnavailable
100@@ -20,6 +21,8 @@ LAUNCHPAD_EXCHANGE = "launchpad-exchange"
101
102 def is_configured():
103 """Return True if rabbit looks to be configured."""
104+ if config.rabbitmq.broker_urls is not None:
105+ return True
106 return not (
107 config.rabbitmq.host is None or
108 config.rabbitmq.userid is None or
109@@ -34,9 +37,16 @@ def connect():
110 """
111 if not is_configured():
112 raise MessagingUnavailable("Incomplete configuration")
113- connection = amqp.Connection(
114- host=config.rabbitmq.host, userid=config.rabbitmq.userid,
115- password=config.rabbitmq.password,
116- virtual_host=config.rabbitmq.virtual_host)
117+ if config.rabbitmq.broker_urls is not None:
118+ connection = kombu.Connection(config.rabbitmq.broker_urls.split())
119+ else:
120+ hostname, port = as_host_port(config.rabbitmq.host, default_port=5672)
121+ connection = kombu.Connection(
122+ hostname=hostname,
123+ userid=config.rabbitmq.userid,
124+ password=config.rabbitmq.password,
125+ virtual_host=config.rabbitmq.virtual_host,
126+ port=port,
127+ )
128 connection.connect()
129 return connection
130diff --git a/setup.py b/setup.py
131index 4d6dab1..fb3a50b 100644
132--- a/setup.py
133+++ b/setup.py
134@@ -144,6 +144,7 @@ setup(
135 'contextlib2; python_version < "3.3"',
136 'defusedxml',
137 'fixtures',
138+ 'kombu',
139 'lazr.config',
140 'lazr.enum',
141 'mock',

Subscribers

People subscribed via source and target branches