Merge lp:~niedbalski/charms/trusty/rabbitmq-server/fix-1442443 into lp:~openstack-charmers-archive/charms/trusty/rabbitmq-server/next
- Trusty Tahr (14.04)
- fix-1442443
- Merge into next
Status: | Merged | ||||
---|---|---|---|---|---|
Merged at revision: | 115 | ||||
Proposed branch: | lp:~niedbalski/charms/trusty/rabbitmq-server/fix-1442443 | ||||
Merge into: | lp:~openstack-charmers-archive/charms/trusty/rabbitmq-server/next | ||||
Diff against target: |
768 lines (+423/-153) 11 files modified
config.yaml (+23/-1) hooks/rabbit_utils.py (+47/-39) hooks/rabbitmq_context.py (+122/-0) hooks/rabbitmq_server_relations.py (+7/-99) hooks/ssl_utils.py (+58/-0) templates/rabbitmq.config (+25/-11) tests/basic_deployment.py (+26/-0) tests/charmhelpers/contrib/openstack/amulet/utils.py (+1/-1) unit_tests/test_rabbit_utils.py (+33/-0) unit_tests/test_rabbitmq_context.py (+79/-0) unit_tests/test_rabbitmq_server_relations.py (+2/-2) |
||||
To merge this branch: | bzr merge lp:~niedbalski/charms/trusty/rabbitmq-server/fix-1442443 | ||||
Related bugs: |
|
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Billy Olsen | Approve | ||
OpenStack Charmers | Pending | ||
Ryan Beisner | Pending | ||
Edward Hope-Morley | Pending | ||
Felipe Reyes | Pending | ||
James Page | Pending | ||
Review via email: mp+272653@code.launchpad.net |
This proposal supersedes a proposal from 2015-09-24.
Commit message
Description of the change
- Added the cluster_
- Refactored the ssl handling code.
- Added a generic config loader
Billy Olsen (billy-olsen) wrote : Posted in a previous version of this proposal | # |
uosci-testing-bot (uosci-testing-bot) wrote : Posted in a previous version of this proposal | # |
charm_lint_check #3202 rabbitmq-server for niedbalski mp255770
LINT OK: passed
uosci-testing-bot (uosci-testing-bot) wrote : Posted in a previous version of this proposal | # |
charm_unit_test #2990 rabbitmq-server for niedbalski mp255770
UNIT OK: passed
uosci-testing-bot (uosci-testing-bot) wrote : Posted in a previous version of this proposal | # |
charm_amulet_test #3019 rabbitmq-server for niedbalski mp255770
AMULET FAIL: amulet-test failed
AMULET Results (max last 2 lines):
make: *** [functional_test] Error 1
ERROR:root:Make target returned non-zero.
Full amulet test output: http://
Build: http://
uosci-testing-bot (uosci-testing-bot) wrote : Posted in a previous version of this proposal | # |
charm_lint_check #3210 rabbitmq-
LINT OK: passed
uosci-testing-bot (uosci-testing-bot) wrote : Posted in a previous version of this proposal | # |
charm_unit_test #2998 rabbitmq-
UNIT OK: passed
uosci-testing-bot (uosci-testing-bot) wrote : Posted in a previous version of this proposal | # |
charm_amulet_test #3027 rabbitmq-
AMULET FAIL: amulet-test failed
AMULET Results (max last 2 lines):
make: *** [functional_test] Error 1
ERROR:root:Make target returned non-zero.
Full amulet test output: http://
Build: http://
Edward Hope-Morley (hopem) : Posted in a previous version of this proposal | # |
uosci-testing-bot (uosci-testing-bot) wrote : Posted in a previous version of this proposal | # |
charm_unit_test #3005 rabbitmq-
UNIT OK: passed
uosci-testing-bot (uosci-testing-bot) wrote : Posted in a previous version of this proposal | # |
charm_lint_check #3217 rabbitmq-
LINT OK: passed
uosci-testing-bot (uosci-testing-bot) wrote : Posted in a previous version of this proposal | # |
charm_lint_check #3224 rabbitmq-
LINT OK: passed
uosci-testing-bot (uosci-testing-bot) wrote : Posted in a previous version of this proposal | # |
charm_unit_test #3012 rabbitmq-
UNIT OK: passed
uosci-testing-bot (uosci-testing-bot) wrote : Posted in a previous version of this proposal | # |
charm_amulet_test #3041 rabbitmq-
AMULET FAIL: amulet-test failed
AMULET Results (max last 2 lines):
make: *** [functional_test] Error 1
ERROR:root:Make target returned non-zero.
Full amulet test output: http://
Build: http://
James Page (james-page) wrote : Posted in a previous version of this proposal | # |
Please can you review the amulet test failures - they appear to be related to SSL.
Jorge Niedbalski (niedbalski) wrote : Posted in a previous version of this proposal | # |
@james-page:
Seems that this tests has been failing since Feb 23 (I tested rev 86), because the certificates being used in amulet tests expired:
verify error:num=
notAfter=Feb 23 20:08:18 2015 GMT
This is not directly implied to my proposed change, and seems that https:/
I would appreciate if you can review this proposal.
uosci-testing-bot (uosci-testing-bot) wrote : Posted in a previous version of this proposal | # |
charm_unit_test #3086 rabbitmq-
UNIT OK: passed
uosci-testing-bot (uosci-testing-bot) wrote : Posted in a previous version of this proposal | # |
charm_lint_check #3298 rabbitmq-
LINT OK: passed
uosci-testing-bot (uosci-testing-bot) wrote : Posted in a previous version of this proposal | # |
charm_amulet_test #3115 rabbitmq-
AMULET FAIL: amulet-test failed
AMULET Results (max last 2 lines):
make: *** [functional_test] Error 1
ERROR:root:Make target returned non-zero.
Full amulet test output: http://
Build: http://
Billy Olsen (billy-olsen) wrote : Posted in a previous version of this proposal | # |
With these changes, this LGTM. I tested some of the partitioning logic here and things seem fine by me. It appears that an unknown value for cluster_
The fix for LP#1436014 is already in the queue and I don't think it should particularly hold this up, however osci bails as soon as the first test fails and I can't see the full output.
I'm running the amulet tests against your change + the fix for 1436014 and will mark it based on the findings of that.
As always, thanks for the submission Jorge!
uosci-testing-bot (uosci-testing-bot) wrote : Posted in a previous version of this proposal | # |
charm_unit_test #3235 rabbitmq-
UNIT OK: passed
uosci-testing-bot (uosci-testing-bot) wrote : Posted in a previous version of this proposal | # |
charm_lint_check #3447 rabbitmq-
LINT OK: passed
uosci-testing-bot (uosci-testing-bot) wrote : Posted in a previous version of this proposal | # |
charm_amulet_test #3245 rabbitmq-
AMULET FAIL: amulet-test failed
AMULET Results (max last 2 lines):
make: *** [functional_test] Error 1
ERROR:root:Make target returned non-zero.
Full amulet test output: http://
Build: http://
Billy Olsen (billy-olsen) wrote : Posted in a previous version of this proposal | # |
Ah the amulet tests look like they failed because the changes were likely based on the lp:charms/trusty/rabbitmq-server which is the trunk and not ~openstack-
In local runs, the code changes look good and I'll be happy to merge them after OSCI does a + vote on them.
Billy Olsen (billy-olsen) wrote : Posted in a previous version of this proposal | # |
Jorge,
After the rebase and test with OSCI it was failing lint test (see Full lint test output: http://
I missed the lint issue so I rechecked what I was running and I made a mistake. The runs from my local amulet tests result in two failures (http://
If this is based off of the ~openstack-
uosci-testing-bot (uosci-testing-bot) wrote : Posted in a previous version of this proposal | # |
charm_unit_test #3367 rabbitmq-
UNIT OK: passed
uosci-testing-bot (uosci-testing-bot) wrote : Posted in a previous version of this proposal | # |
charm_lint_check #3579 rabbitmq-
LINT OK: passed
uosci-testing-bot (uosci-testing-bot) wrote : Posted in a previous version of this proposal | # |
charm_amulet_test #3336 rabbitmq-
AMULET FAIL: amulet-test failed
AMULET Results (max last 2 lines):
make: *** [functional_test] Error 1
ERROR:root:Make target returned non-zero.
Full amulet test output: http://
Build: http://
uosci-testing-bot (uosci-testing-bot) wrote : Posted in a previous version of this proposal | # |
charm_lint_check #4238 rabbitmq-
LINT FAIL: lint-test failed
LINT Results (max last 2 lines):
make: *** [.venv] Error 127
ERROR:root:Make target returned non-zero.
Full lint test output: http://
Build: http://
uosci-testing-bot (uosci-testing-bot) wrote : Posted in a previous version of this proposal | # |
charm_unit_test #3980 rabbitmq-
UNIT OK: passed
uosci-testing-bot (uosci-testing-bot) wrote : Posted in a previous version of this proposal | # |
charm_lint_check #4254 rabbitmq-
LINT OK: passed
Felipe Reyes (freyes) wrote : Posted in a previous version of this proposal | # |
I did a round of tests exercising th default value (ignore), autoheal and pause_minority, so force a split I used lxc-freeze/
Thanks Jorge.
Billy Olsen (billy-olsen) wrote : Posted in a previous version of this proposal | # |
Jorge,
The amulet test continues to fail due to the SSL setup per - http://
If the behavior is going to be changed, we need to properly document the behavior change thoroughly and deprecate it appropriately. I think there's no need to revert this specific behavior though, I think the three modes of operation are perfectly viable, though I think in production ssl_only mode should be the only one used.
Please correct and resubmit.
Thanks,
Billy
Edward Hope-Morley (hopem) wrote : Posted in a previous version of this proposal | # |
Nice work for implementing this Jorge, i think we can definitely attribute some issues we are seeing in the field to this. I have one minor request for you to add some docstrings to a new class you created though.
Ryan Beisner (1chb1n) wrote : Posted in a previous version of this proposal | # |
Take note that the rabbitmq-
Those new and refactored tests have are still in review, but have exposed two critical bugs. I would recommend holding on this proposal until the following bugs are resolved and the corresponding branches have landed in next. Then, I'd rebase your proposal and let the updated tests re-run to make sure everything still passes.
bug tldrs:
cluster race bug, affects stable and next
>50% fail to cluster with leadership election
>90% fail to cluster without leadership election
https:/
outright fail on vivid, affects stable and next
100% fail, rmq's pidfile location changed, cron job errors out
https:/
uosci-testing-bot (uosci-testing-bot) wrote : Posted in a previous version of this proposal | # |
charm_unit_test #9751 rabbitmq-
UNIT FAIL: unit-test failed
UNIT Results (max last 2 lines):
make: *** [test] Error 1
ERROR:root:Make target returned non-zero.
Full unit test output: http://
Build: http://
uosci-testing-bot (uosci-testing-bot) wrote : Posted in a previous version of this proposal | # |
charm_lint_check #10563 rabbitmq-
LINT OK: passed
Build: http://
Edward Hope-Morley (hopem) wrote : Posted in a previous version of this proposal | # |
A few nits. The refactor of the ssl config rendering is a little heavy handed since it is not directly related to the addition of a new config option but you seem to have added some decent tests. Lets see what OSCI amulet has to say.
And fwiw I can't reproduce the unit test failure locally so not sure what happened there.
uosci-testing-bot (uosci-testing-bot) wrote : Posted in a previous version of this proposal | # |
charm_unit_test #9763 rabbitmq-
UNIT FAIL: unit-test failed
UNIT Results (max last 2 lines):
make: *** [test] Error 1
ERROR:root:Make target returned non-zero.
Full unit test output: http://
Build: http://
uosci-testing-bot (uosci-testing-bot) wrote : Posted in a previous version of this proposal | # |
charm_lint_check #10575 rabbitmq-
LINT OK: passed
Build: http://
uosci-testing-bot (uosci-testing-bot) wrote : Posted in a previous version of this proposal | # |
charm_amulet_test #6702 rabbitmq-
AMULET FAIL: amulet-test failed
AMULET Results (max last 2 lines):
make: *** [functional_test] Error 1
ERROR:root:Make target returned non-zero.
Full amulet test output: http://
Build: http://
Edward Hope-Morley (hopem) wrote : Posted in a previous version of this proposal | # |
Unit and amulet errors. See inline for unit test fixups. Try running your new amulet test in isolation to see if you can get it to run.
Jorge Niedbalski (niedbalski) wrote : Posted in a previous version of this proposal | # |
@hopem: Addressed your comments, still looking to one of the failed amulet tests.
uosci-testing-bot (uosci-testing-bot) wrote : Posted in a previous version of this proposal | # |
charm_unit_test #9886 rabbitmq-
UNIT OK: passed
uosci-testing-bot (uosci-testing-bot) wrote : Posted in a previous version of this proposal | # |
charm_lint_check #10704 rabbitmq-
LINT OK: passed
Build: http://
uosci-testing-bot (uosci-testing-bot) wrote : Posted in a previous version of this proposal | # |
charm_lint_check #10706 rabbitmq-
LINT OK: passed
Build: http://
uosci-testing-bot (uosci-testing-bot) wrote : Posted in a previous version of this proposal | # |
charm_unit_test #9888 rabbitmq-
UNIT OK: passed
uosci-testing-bot (uosci-testing-bot) wrote : Posted in a previous version of this proposal | # |
charm_amulet_test #6755 rabbitmq-
AMULET FAIL: amulet-test failed
AMULET Results (max last 2 lines):
make: *** [functional_test] Error 1
ERROR:root:Make target returned non-zero.
Full amulet test output: http://
Build: http://
uosci-testing-bot (uosci-testing-bot) wrote : Posted in a previous version of this proposal | # |
charm_amulet_test #6757 rabbitmq-
AMULET FAIL: amulet-test failed
AMULET Results (max last 2 lines):
make: *** [functional_test] Error 1
ERROR:root:Make target returned non-zero.
Full amulet test output: http://
Build: http://
Ryan Beisner (1chb1n) wrote : Posted in a previous version of this proposal | # |
Please do not add new amulet test files, as those will only be exercised against one target (trusty-icehouse by default).
Please add new test cases to the existing basic_deploymen
Be aware, that all of the old/original amulet tests have been refactored into this format to be consistent with the other os-charms.
Thank you.
Ryan Beisner (1chb1n) wrote : Posted in a previous version of this proposal | # |
To clarify and expand, the tests proposed in the new file (tests/
Ryan Beisner (1chb1n) wrote : Posted in a previous version of this proposal | # |
To-do #2:
To address the error hit in the existing tests, the following change in the test logic will also be necessary (please propose that 1-line change @ lp:charm-helpers and resync tests/charmhelpers in your branch).
http://
FYI, I ran locally with that change, and the existing tests began to pass.
Thank you.
- 98. By Jorge Niedbalski
-
Addressed @beisner comments
uosci-testing-bot (uosci-testing-bot) wrote : | # |
charm_unit_test #10178 rabbitmq-
UNIT OK: passed
uosci-testing-bot (uosci-testing-bot) wrote : | # |
charm_lint_check #10967 rabbitmq-
LINT OK: passed
Build: http://
uosci-testing-bot (uosci-testing-bot) wrote : | # |
charm_amulet_test #6862 rabbitmq-
AMULET FAIL: amulet-test failed
AMULET Results (max last 2 lines):
make: *** [functional_test] Error 1
ERROR:root:Make target returned non-zero.
Full amulet test output: http://
Build: http://
Jorge Niedbalski (niedbalski) wrote : | # |
All the amulet tests are passing , except one for vivid-kilo, (according to the log juju-test.
@1chb1n: Do you know if this is related to any environment failure?
Ryan Beisner (1chb1n) wrote : | # |
@niedbalski
http://
Everything looks good with regard to nova console logs, forward/reverse host records, connectivity to units, juju agent states, and log collection. I'd say the environment was sound.
Ryan Beisner (1chb1n) wrote : | # |
s/two 2-node/one 2-node/ of course is what I really meant.
At any rate, two clusters formed. One cluster with two units. One cluster with one unit. ;-)
uosci-testing-bot (uosci-testing-bot) wrote : | # |
charm_amulet_test #6868 rabbitmq-
AMULET FAIL: amulet-test failed
AMULET Results (max last 2 lines):
make: *** [functional_test] Error 1
ERROR:root:Make target returned non-zero.
Full amulet test output: http://
Build: http://
uosci-testing-bot (uosci-testing-bot) wrote : | # |
charm_amulet_test #6875 rabbitmq-
AMULET OK: passed
Build: http://
Billy Olsen (billy-olsen) wrote : | # |
Approved, thanks Jorge!
Preview Diff
1 | === modified file 'config.yaml' |
2 | --- config.yaml 2015-04-20 11:13:39 +0000 |
3 | +++ config.yaml 2015-09-28 19:34:56 +0000 |
4 | @@ -8,7 +8,7 @@ |
5 | type: string |
6 | default: "off" |
7 | description: | |
8 | - Enable SSL connections on rabbitmq, valid values are 'off', 'on', 'only'. If ssl_key, |
9 | + Enable SSL connections on rabbitmq, valid values are 'off', 'on', 'only'. If ssl_key, |
10 | ssl_cert, ssl_ca are provided then then those values will be used. Otherwise |
11 | the service will act as its own certificate authority and pass its ca cert to clients. |
12 | For HA or clustered rabbits ssl key/cert must be provided. |
13 | @@ -92,6 +92,28 @@ |
14 | description: | |
15 | When set to true the 'ha-mode: all' policy is applied to all the exchages |
16 | that match the expression '^(?!amq\.).*' |
17 | + cluster-partition-handling: |
18 | + type: string |
19 | + default: ignore |
20 | + description: | |
21 | + RabbitMQ offers three ways to deal with network partitions automatically. |
22 | + Available modes: |
23 | + |
24 | + ignore - Your network is reliable. All your nodes are in a rack, |
25 | + connected with a switch, and that switch is also the route to the outside world. |
26 | + You don't want to run any risk of any of your cluster shutting down if any other part of it fails |
27 | + (or you have a two node cluster). |
28 | + |
29 | + pause_minority - Your network is maybe less reliable. You have clustered across 3 AZs in EC2, |
30 | + and you assume that only one AZ will fail at once. |
31 | + In that scenario you want the remaining two AZs to continue working and the nodes from the failed AZ |
32 | + to rejoin automatically and without fuss when the AZ comes back. |
33 | + |
34 | + autoheal - Your network may not be reliable. |
35 | + You are more concerned with continuity of service than with data integrity. |
36 | + You may have a two node cluster. |
37 | + |
38 | + For more information see http://www.rabbitmq.com/partitions.html. |
39 | rbd-size: |
40 | type: string |
41 | default: 5G |
42 | |
43 | === modified file 'hooks/rabbit_utils.py' |
44 | --- hooks/rabbit_utils.py 2015-09-16 17:56:59 +0000 |
45 | +++ hooks/rabbit_utils.py 2015-09-28 19:34:56 +0000 |
46 | @@ -1,12 +1,17 @@ |
47 | import os |
48 | -import pwd |
49 | -import grp |
50 | import re |
51 | import sys |
52 | import subprocess |
53 | import glob |
54 | import tempfile |
55 | |
56 | +from rabbitmq_context import ( |
57 | + RabbitMQSSLContext, |
58 | + RabbitMQClusterContext, |
59 | +) |
60 | + |
61 | +from charmhelpers.core.templating import render |
62 | + |
63 | from charmhelpers.contrib.openstack.utils import ( |
64 | get_hostname, |
65 | ) |
66 | @@ -29,8 +34,6 @@ |
67 | cmp_pkgrevno |
68 | ) |
69 | |
70 | -from charmhelpers.core.templating import render |
71 | - |
72 | from charmhelpers.contrib.peerstorage import ( |
73 | peer_store, |
74 | peer_retrieve |
75 | @@ -58,7 +61,10 @@ |
76 | # the charm doesn't concern itself with template specifics etc. |
77 | CONFIG_FILES = OrderedDict([ |
78 | (RABBITMQ_CONF, { |
79 | - 'hook_contexts': None, |
80 | + 'hook_contexts': [ |
81 | + RabbitMQSSLContext(), |
82 | + RabbitMQClusterContext(), |
83 | + ], |
84 | 'services': ['rabbitmq-server'] |
85 | }), |
86 | (ENV_CONF, { |
87 | @@ -72,6 +78,42 @@ |
88 | ]) |
89 | |
90 | |
91 | +class ConfigRenderer(object): |
92 | + """ |
93 | + This class is a generic configuration renderer for |
94 | + a given dict mapping configuration files and hook_contexts. |
95 | + """ |
96 | + def __init__(self, config): |
97 | + """ |
98 | + :param config: see CONFIG_FILES |
99 | + :type config: dict |
100 | + """ |
101 | + self.config_data = {} |
102 | + |
103 | + for config_path, data in config.items(): |
104 | + hook_contexts = data.get('hook_contexts', None) |
105 | + if hook_contexts: |
106 | + ctxt = {} |
107 | + for svc_context in hook_contexts: |
108 | + ctxt.update(svc_context()) |
109 | + self.config_data[config_path] = ctxt |
110 | + |
111 | + def write(self, config_path): |
112 | + data = self.config_data.get(config_path, None) |
113 | + if data: |
114 | + log("writing config file: %s , data: %s" % (config_path, |
115 | + str(data)), |
116 | + level='DEBUG') |
117 | + |
118 | + render(os.path.basename(config_path), config_path, |
119 | + data, perms=0o644) |
120 | + |
121 | + def write_all(self): |
122 | + """Write all the defined configuration files""" |
123 | + for service in self.config_data.keys(): |
124 | + self.write(service) |
125 | + |
126 | + |
127 | class RabbitmqError(Exception): |
128 | pass |
129 | |
130 | @@ -391,40 +433,6 @@ |
131 | def disable_plugin(plugin): |
132 | _manage_plugin(plugin, 'disable') |
133 | |
134 | -ssl_key_file = "/etc/rabbitmq/rabbit-server-privkey.pem" |
135 | -ssl_cert_file = "/etc/rabbitmq/rabbit-server-cert.pem" |
136 | -ssl_ca_file = "/etc/rabbitmq/rabbit-server-ca.pem" |
137 | - |
138 | - |
139 | -def enable_ssl(ssl_key, ssl_cert, ssl_port, |
140 | - ssl_ca=None, ssl_only=False, ssl_client=None): |
141 | - uid = pwd.getpwnam("root").pw_uid |
142 | - gid = grp.getgrnam("rabbitmq").gr_gid |
143 | - |
144 | - for contents, path in ( |
145 | - (ssl_key, ssl_key_file), |
146 | - (ssl_cert, ssl_cert_file), |
147 | - (ssl_ca, ssl_ca_file)): |
148 | - if not contents: |
149 | - continue |
150 | - with open(path, 'w') as fh: |
151 | - fh.write(contents) |
152 | - os.chmod(path, 0o640) |
153 | - os.chown(path, uid, gid) |
154 | - |
155 | - data = { |
156 | - "ssl_port": ssl_port, |
157 | - "ssl_cert_file": ssl_cert_file, |
158 | - "ssl_key_file": ssl_key_file, |
159 | - "ssl_client": ssl_client, |
160 | - "ssl_ca_file": "", |
161 | - "ssl_only": ssl_only} |
162 | - |
163 | - if ssl_ca: |
164 | - data["ssl_ca_file"] = ssl_ca_file |
165 | - |
166 | - render(os.path.basename(RABBITMQ_CONF), RABBITMQ_CONF, data, perms=0o644) |
167 | - |
168 | |
169 | def execute(cmd, die=False, echo=False): |
170 | """ Executes a command |
171 | |
172 | === added file 'hooks/rabbitmq_context.py' |
173 | --- hooks/rabbitmq_context.py 1970-01-01 00:00:00 +0000 |
174 | +++ hooks/rabbitmq_context.py 2015-09-28 19:34:56 +0000 |
175 | @@ -0,0 +1,122 @@ |
176 | +from charmhelpers.contrib.ssl.service import ServiceCA |
177 | + |
178 | +from charmhelpers.core.hookenv import ( |
179 | + open_port, |
180 | + close_port, |
181 | + config, |
182 | + log, |
183 | + ERROR, |
184 | +) |
185 | + |
186 | +import sys |
187 | +import pwd |
188 | +import grp |
189 | +import os |
190 | +import base64 |
191 | + |
192 | +import ssl_utils |
193 | + |
194 | +ssl_key_file = "/etc/rabbitmq/rabbit-server-privkey.pem" |
195 | +ssl_cert_file = "/etc/rabbitmq/rabbit-server-cert.pem" |
196 | +ssl_ca_file = "/etc/rabbitmq/rabbit-server-ca.pem" |
197 | + |
198 | + |
199 | +def convert_from_base64(v): |
200 | + # Rabbit originally supported pem encoded key/cert in config, play |
201 | + # nice on upgrades as we now expect base64 encoded key/cert/ca. |
202 | + if not v: |
203 | + return v |
204 | + if v.startswith('-----BEGIN'): |
205 | + return v |
206 | + try: |
207 | + return base64.b64decode(v) |
208 | + except TypeError: |
209 | + return v |
210 | + |
211 | + |
212 | +class RabbitMQSSLContext(object): |
213 | + |
214 | + def enable_ssl(self, ssl_key, ssl_cert, ssl_port, |
215 | + ssl_ca=None, ssl_only=False, ssl_client=None): |
216 | + |
217 | + uid = pwd.getpwnam("root").pw_uid |
218 | + gid = grp.getgrnam("rabbitmq").gr_gid |
219 | + |
220 | + for contents, path in ( |
221 | + (ssl_key, ssl_key_file), |
222 | + (ssl_cert, ssl_cert_file), |
223 | + (ssl_ca, ssl_ca_file)): |
224 | + |
225 | + if not contents: |
226 | + continue |
227 | + |
228 | + with open(path, 'w') as fh: |
229 | + fh.write(contents) |
230 | + |
231 | + os.chmod(path, 0o640) |
232 | + os.chown(path, uid, gid) |
233 | + |
234 | + data = { |
235 | + "ssl_port": ssl_port, |
236 | + "ssl_cert_file": ssl_cert_file, |
237 | + "ssl_key_file": ssl_key_file, |
238 | + "ssl_client": ssl_client, |
239 | + "ssl_ca_file": "", |
240 | + "ssl_only": ssl_only |
241 | + } |
242 | + |
243 | + if ssl_ca: |
244 | + data["ssl_ca_file"] = ssl_ca_file |
245 | + |
246 | + return data |
247 | + |
248 | + def __call__(self): |
249 | + """ |
250 | + The legacy config support adds some additional complications. |
251 | + |
252 | + ssl_enabled = True, ssl = off -> ssl enabled |
253 | + ssl_enabled = False, ssl = on -> ssl enabled |
254 | + """ |
255 | + ssl_mode, external_ca = ssl_utils.get_ssl_mode() |
256 | + |
257 | + ctxt = { |
258 | + 'ssl_mode': ssl_mode, |
259 | + } |
260 | + |
261 | + if ssl_mode == 'off': |
262 | + close_port(config('ssl_port')) |
263 | + ssl_utils.reconfigure_client_ssl() |
264 | + return ctxt |
265 | + |
266 | + ssl_key = convert_from_base64(config('ssl_key')) |
267 | + ssl_cert = convert_from_base64(config('ssl_cert')) |
268 | + ssl_ca = convert_from_base64(config('ssl_ca')) |
269 | + ssl_port = config('ssl_port') |
270 | + |
271 | + # If external managed certs then we need all the fields. |
272 | + if (ssl_mode in ('on', 'only') and any((ssl_key, ssl_cert)) and |
273 | + not all((ssl_key, ssl_cert))): |
274 | + log('If ssl_key or ssl_cert are specified both are required.', |
275 | + level=ERROR) |
276 | + sys.exit(1) |
277 | + |
278 | + if not external_ca: |
279 | + ssl_cert, ssl_key, ssl_ca = ServiceCA.get_service_cert() |
280 | + |
281 | + ctxt.update(self.enable_ssl( |
282 | + ssl_key, ssl_cert, ssl_port, ssl_ca, |
283 | + ssl_only=(ssl_mode == "only"), ssl_client=False |
284 | + )) |
285 | + |
286 | + ssl_utils.reconfigure_client_ssl(True) |
287 | + open_port(ssl_port) |
288 | + |
289 | + return ctxt |
290 | + |
291 | + |
292 | +class RabbitMQClusterContext(object): |
293 | + |
294 | + def __call__(self): |
295 | + return { |
296 | + 'cluster_partition_handling': config('cluster-partition-handling'), |
297 | + } |
298 | |
299 | === modified file 'hooks/rabbitmq_server_relations.py' |
300 | --- hooks/rabbitmq_server_relations.py 2015-09-22 13:55:23 +0000 |
301 | +++ hooks/rabbitmq_server_relations.py 2015-09-28 19:34:56 +0000 |
302 | @@ -1,5 +1,5 @@ |
303 | #!/usr/bin/python |
304 | -import base64 |
305 | + |
306 | import os |
307 | import shutil |
308 | import sys |
309 | @@ -17,6 +17,8 @@ |
310 | import yaml # flake8: noqa |
311 | |
312 | import rabbit_utils as rabbit |
313 | +import ssl_utils |
314 | + |
315 | from lib.utils import ( |
316 | chown, chmod, |
317 | is_newer, |
318 | @@ -73,7 +75,6 @@ |
319 | write_file, |
320 | ) |
321 | from charmhelpers.contrib.charmsupport import nrpe |
322 | -from charmhelpers.contrib.ssl.service import ServiceCA |
323 | |
324 | from charmhelpers.contrib.peerstorage import ( |
325 | peer_echo, |
326 | @@ -232,7 +233,7 @@ |
327 | get_address_in_network(config('access-network'), |
328 | unit_get('private-address')) |
329 | |
330 | - configure_client_ssl(relation_settings) |
331 | + ssl_utils.configure_client_ssl(relation_settings) |
332 | |
333 | if is_clustered(): |
334 | relation_settings['clustered'] = 'true' |
335 | @@ -627,103 +628,10 @@ |
336 | MAN_PLUGIN = 'rabbitmq_management' |
337 | |
338 | |
339 | -def configure_client_ssl(relation_data): |
340 | - """Configure client with ssl |
341 | - """ |
342 | - ssl_mode, external_ca = _get_ssl_mode() |
343 | - if ssl_mode == 'off': |
344 | - return |
345 | - relation_data['ssl_port'] = config('ssl_port') |
346 | - if external_ca: |
347 | - if config('ssl_ca'): |
348 | - relation_data['ssl_ca'] = base64.b64encode( |
349 | - config('ssl_ca')) |
350 | - return |
351 | - ca = ServiceCA.get_ca() |
352 | - relation_data['ssl_ca'] = base64.b64encode(ca.get_ca_bundle()) |
353 | - |
354 | - |
355 | -def _get_ssl_mode(): |
356 | - ssl_mode = config('ssl') |
357 | - external_ca = False |
358 | - # Legacy config boolean option |
359 | - ssl_on = config('ssl_enabled') |
360 | - if ssl_mode == 'off' and ssl_on is False: |
361 | - ssl_mode = 'off' |
362 | - elif ssl_mode == 'off' and ssl_on: |
363 | - ssl_mode = 'on' |
364 | - ssl_key = config('ssl_key') |
365 | - ssl_cert = config('ssl_cert') |
366 | - if all((ssl_key, ssl_cert)): |
367 | - external_ca = True |
368 | - return ssl_mode, external_ca |
369 | - |
370 | - |
371 | -def _convert_from_base64(v): |
372 | - # Rabbit originally supported pem encoded key/cert in config, play |
373 | - # nice on upgrades as we now expect base64 encoded key/cert/ca. |
374 | - if not v: |
375 | - return v |
376 | - if v.startswith('-----BEGIN'): |
377 | - return v |
378 | - try: |
379 | - return base64.b64decode(v) |
380 | - except TypeError: |
381 | - return v |
382 | - |
383 | - |
384 | -def reconfigure_client_ssl(ssl_enabled=False): |
385 | - ssl_config_keys = set(('ssl_key', 'ssl_cert', 'ssl_ca')) |
386 | - for rid in relation_ids('amqp'): |
387 | - rdata = relation_get(rid=rid, unit=os.environ['JUJU_UNIT_NAME']) |
388 | - if not ssl_enabled and ssl_config_keys.intersection(rdata): |
389 | - # No clean way to remove entirely, but blank them. |
390 | - relation_set(relation_id=rid, ssl_key='', ssl_cert='', ssl_ca='') |
391 | - elif ssl_enabled and not ssl_config_keys.intersection(rdata): |
392 | - configure_client_ssl(rdata) |
393 | - relation_set(relation_id=rid, **rdata) |
394 | - |
395 | - |
396 | -def configure_rabbit_ssl(): |
397 | - """ |
398 | - The legacy config support adds some additional complications. |
399 | - |
400 | - ssl_enabled = True, ssl = off -> ssl enabled |
401 | - ssl_enabled = False, ssl = on -> ssl enabled |
402 | - """ |
403 | - ssl_mode, external_ca = _get_ssl_mode() |
404 | - |
405 | - if ssl_mode == 'off': |
406 | - if os.path.exists(rabbit.RABBITMQ_CONF): |
407 | - os.remove(rabbit.RABBITMQ_CONF) |
408 | - close_port(config('ssl_port')) |
409 | - reconfigure_client_ssl() |
410 | - return |
411 | - ssl_key = _convert_from_base64(config('ssl_key')) |
412 | - ssl_cert = _convert_from_base64(config('ssl_cert')) |
413 | - ssl_ca = _convert_from_base64(config('ssl_ca')) |
414 | - ssl_port = config('ssl_port') |
415 | - |
416 | - # If external managed certs then we need all the fields. |
417 | - if (ssl_mode in ('on', 'only') and any((ssl_key, ssl_cert)) and |
418 | - not all((ssl_key, ssl_cert))): |
419 | - log('If ssl_key or ssl_cert are specified both are required.', |
420 | - level=ERROR) |
421 | - sys.exit(1) |
422 | - |
423 | - if not external_ca: |
424 | - ssl_cert, ssl_key, ssl_ca = ServiceCA.get_service_cert() |
425 | - |
426 | - rabbit.enable_ssl( |
427 | - ssl_key, ssl_cert, ssl_port, ssl_ca, |
428 | - ssl_only=(ssl_mode == "only"), ssl_client=False) |
429 | - reconfigure_client_ssl(True) |
430 | - open_port(ssl_port) |
431 | - |
432 | - |
433 | @hooks.hook('config-changed') |
434 | @restart_on_change(rabbit.restart_map()) |
435 | def config_changed(): |
436 | + |
437 | if config('prefer-ipv6'): |
438 | rabbit.assert_charm_supports_ipv6() |
439 | |
440 | @@ -752,9 +660,9 @@ |
441 | rabbit.disable_plugin(MAN_PLUGIN) |
442 | close_port(55672) |
443 | |
444 | - configure_rabbit_ssl() |
445 | - |
446 | rabbit.set_all_mirroring_queues(config('mirroring-queues')) |
447 | + rabbit.ConfigRenderer( |
448 | + rabbit.CONFIG_FILES).write_all() |
449 | |
450 | if is_relation_made("ha"): |
451 | ha_is_active_active = config("ha-vip-only") |
452 | |
453 | === added file 'hooks/ssl_utils.py' |
454 | --- hooks/ssl_utils.py 1970-01-01 00:00:00 +0000 |
455 | +++ hooks/ssl_utils.py 2015-09-28 19:34:56 +0000 |
456 | @@ -0,0 +1,58 @@ |
457 | +from charmhelpers.contrib.ssl.service import ServiceCA |
458 | + |
459 | +from charmhelpers.core.hookenv import ( |
460 | + config, |
461 | + relation_ids, |
462 | + relation_set, |
463 | + relation_get, |
464 | + local_unit, |
465 | +) |
466 | + |
467 | +import base64 |
468 | + |
469 | + |
470 | +def get_ssl_mode(): |
471 | + ssl_mode = config('ssl') |
472 | + external_ca = False |
473 | + |
474 | + # Legacy config boolean option |
475 | + ssl_on = config('ssl_enabled') |
476 | + if ssl_mode == 'off' and ssl_on is False: |
477 | + ssl_mode = 'off' |
478 | + elif ssl_mode == 'off' and ssl_on: |
479 | + ssl_mode = 'on' |
480 | + |
481 | + ssl_key = config('ssl_key') |
482 | + ssl_cert = config('ssl_cert') |
483 | + |
484 | + if all((ssl_key, ssl_cert)): |
485 | + external_ca = True |
486 | + return ssl_mode, external_ca |
487 | + |
488 | + |
489 | +def configure_client_ssl(relation_data): |
490 | + """Configure client with ssl |
491 | + """ |
492 | + ssl_mode, external_ca = get_ssl_mode() |
493 | + if ssl_mode == 'off': |
494 | + return |
495 | + relation_data['ssl_port'] = config('ssl_port') |
496 | + if external_ca: |
497 | + if config('ssl_ca'): |
498 | + relation_data['ssl_ca'] = base64.b64encode( |
499 | + config('ssl_ca')) |
500 | + return |
501 | + ca = ServiceCA.get_ca() |
502 | + relation_data['ssl_ca'] = base64.b64encode(ca.get_ca_bundle()) |
503 | + |
504 | + |
505 | +def reconfigure_client_ssl(ssl_enabled=False): |
506 | + ssl_config_keys = set(('ssl_key', 'ssl_cert', 'ssl_ca')) |
507 | + for rid in relation_ids('amqp'): |
508 | + rdata = relation_get(rid=rid, unit=local_unit()) |
509 | + if not ssl_enabled and ssl_config_keys.intersection(rdata): |
510 | + # No clean way to remove entirely, but blank them. |
511 | + relation_set(relation_id=rid, ssl_key='', ssl_cert='', ssl_ca='') |
512 | + elif ssl_enabled and not ssl_config_keys.intersection(rdata): |
513 | + configure_client_ssl(rdata) |
514 | + relation_set(relation_id=rid, **rdata) |
515 | |
516 | === modified file 'templates/rabbitmq.config' |
517 | --- templates/rabbitmq.config 2014-05-23 08:13:05 +0000 |
518 | +++ templates/rabbitmq.config 2015-09-28 19:34:56 +0000 |
519 | @@ -1,21 +1,35 @@ |
520 | [ |
521 | - {rabbit, [ |
522 | -{% if ssl_only %} |
523 | + {rabbit, [ |
524 | +{% if ssl_only %} |
525 | {tcp_listeners, []}, |
526 | {% else %} |
527 | {tcp_listeners, [5672]}, |
528 | {% endif %} |
529 | - {ssl_listeners, [{{ ssl_port }}]}, |
530 | +{% if ssl_port %} |
531 | + {ssl_listeners, [{{ ssl_port }}]}, |
532 | +{% endif %} |
533 | +{% if ssl_mode == "on" %} |
534 | {ssl_options, [ |
535 | {verify, verify_peer}, |
536 | -{% if ssl_client %} |
537 | - {fail_if_no_peer_cert, true}, |
538 | +{% if ssl_client %} |
539 | + {fail_if_no_peer_cert, true}, |
540 | {% else %} |
541 | {fail_if_no_peer_cert, false}, |
542 | -{% endif %}{% if ssl_ca_file %} |
543 | - {cacertfile, "{{ ssl_ca_file }}"}, {% endif %} |
544 | - {certfile, "{{ ssl_cert_file }}"}, |
545 | - {keyfile, "{{ ssl_key_file }}"} |
546 | - ]} |
547 | +{% endif %} |
548 | + {% if ssl_ca_file %} |
549 | + {cacertfile, "{{ ssl_ca_file }}"}, |
550 | + {% endif %} |
551 | + {% if ssl_cert_file %} |
552 | + {certfile, "{{ ssl_cert_file }}"}, |
553 | + {% endif %} |
554 | + {% if ssl_key_file %} |
555 | + {keyfile, "{{ ssl_key_file }}"} |
556 | + {% endif %} |
557 | + ]}, |
558 | +{% endif %} |
559 | + |
560 | + {% if cluster_partition_handling %} |
561 | + {cluster_partition_handling, {{ cluster_partition_handling }}} |
562 | + {% endif %} |
563 | ]} |
564 | -]. |
565 | \ No newline at end of file |
566 | +]. |
567 | |
568 | === modified file 'tests/basic_deployment.py' |
569 | --- tests/basic_deployment.py 2015-09-03 18:58:14 +0000 |
570 | +++ tests/basic_deployment.py 2015-09-28 19:34:56 +0000 |
571 | @@ -490,3 +490,29 @@ |
572 | amulet.raise_status(amulet.FAIL, msg=ret) |
573 | |
574 | u.log.info('OK\n') |
575 | + |
576 | + def test_415_cluster_partitioning(self): |
577 | + """ |
578 | + Test if the cluster-partition-handling configuration is applied |
579 | + correctly. |
580 | + """ |
581 | + sentry_units = self._get_rmq_sentry_units() |
582 | + |
583 | + configuration = {'cluster-partition-handling': "autoheal"} |
584 | + self.d.configure('rabbitmq-server', configuration) |
585 | + |
586 | + # Wait for configuration to be effectively applied |
587 | + time.sleep(10) |
588 | + |
589 | + u.log.debug('Checking if cluster-partition-handling has' |
590 | + ' been correclty applied...') |
591 | + for sentry_unit in sentry_units: |
592 | + cmds = [ |
593 | + "grep autoheal /etc/rabbitmq/rabbitmq.config" |
594 | + ] |
595 | + |
596 | + ret = u.check_commands_on_units(cmds, [sentry_unit]) |
597 | + if ret: |
598 | + amulet.raise_status(amulet.FAIL, msg=ret) |
599 | + |
600 | + u.log.info('OK\n') |
601 | |
602 | === modified file 'tests/charmhelpers/contrib/openstack/amulet/utils.py' |
603 | --- tests/charmhelpers/contrib/openstack/amulet/utils.py 2015-09-04 21:43:07 +0000 |
604 | +++ tests/charmhelpers/contrib/openstack/amulet/utils.py 2015-09-28 19:34:56 +0000 |
605 | @@ -752,7 +752,7 @@ |
606 | self.log.debug('SSL is enabled @{}:{} ' |
607 | '({})'.format(host, port, unit_name)) |
608 | return True |
609 | - elif not port and not conf_ssl: |
610 | + elif not conf_ssl: |
611 | self.log.debug('SSL not enabled @{}:{} ' |
612 | '({})'.format(host, port, unit_name)) |
613 | return False |
614 | |
615 | === modified file 'unit_tests/test_rabbit_utils.py' |
616 | --- unit_tests/test_rabbit_utils.py 2015-04-13 15:42:57 +0000 |
617 | +++ unit_tests/test_rabbit_utils.py 2015-09-28 19:34:56 +0000 |
618 | @@ -3,11 +3,44 @@ |
619 | import unittest |
620 | import tempfile |
621 | import sys |
622 | +import collections |
623 | |
624 | import rabbit_utils |
625 | sys.modules['MySQLdb'] = mock.Mock() |
626 | |
627 | |
628 | +class ConfigRendererTests(unittest.TestCase): |
629 | + |
630 | + class FakeContext(object): |
631 | + def __call__(self, *a, **k): |
632 | + return {'foo': 'bar'} |
633 | + |
634 | + config_map = collections.OrderedDict( |
635 | + [('/this/is/a/config', { |
636 | + 'hook_contexts': [ |
637 | + FakeContext() |
638 | + ] |
639 | + })] |
640 | + ) |
641 | + |
642 | + def setUp(self): |
643 | + super(ConfigRendererTests, self).setUp() |
644 | + self.renderer = rabbit_utils.ConfigRenderer( |
645 | + self.config_map) |
646 | + |
647 | + def test_has_config_data(self): |
648 | + self.assertTrue( |
649 | + '/this/is/a/config' in self.renderer.config_data.keys()) |
650 | + |
651 | + @mock.patch("rabbit_utils.log") |
652 | + @mock.patch("rabbit_utils.render") |
653 | + def test_write_all(self, log, render): |
654 | + self.renderer.write_all() |
655 | + |
656 | + self.assertTrue(render.called) |
657 | + self.assertTrue(log.called) |
658 | + |
659 | + |
660 | class UtilsTests(unittest.TestCase): |
661 | def setUp(self): |
662 | super(UtilsTests, self).setUp() |
663 | |
664 | === added file 'unit_tests/test_rabbitmq_context.py' |
665 | --- unit_tests/test_rabbitmq_context.py 1970-01-01 00:00:00 +0000 |
666 | +++ unit_tests/test_rabbitmq_context.py 2015-09-28 19:34:56 +0000 |
667 | @@ -0,0 +1,79 @@ |
668 | +import rabbitmq_context |
669 | + |
670 | +import mock |
671 | +import unittest |
672 | + |
673 | + |
674 | +class TestRabbitMQSSLContext(unittest.TestCase): |
675 | + |
676 | + @mock.patch("rabbitmq_context.config") |
677 | + @mock.patch("rabbitmq_context.close_port") |
678 | + @mock.patch("rabbitmq_context.ssl_utils.reconfigure_client_ssl") |
679 | + @mock.patch("rabbitmq_context.ssl_utils.get_ssl_mode") |
680 | + def test_context_ssl_off(self, get_ssl_mode, reconfig_ssl, close_port, |
681 | + config): |
682 | + get_ssl_mode.return_value = ("off", "off") |
683 | + self.assertEqual(rabbitmq_context.RabbitMQSSLContext().__call__(), { |
684 | + "ssl_mode": "off" |
685 | + }) |
686 | + |
687 | + self.assertTrue(close_port.called) |
688 | + self.assertTrue(reconfig_ssl.called) |
689 | + |
690 | + @mock.patch("rabbitmq_context.open_port") |
691 | + @mock.patch("rabbitmq_context.os.chmod") |
692 | + @mock.patch("rabbitmq_context.os.chown") |
693 | + @mock.patch("rabbitmq_context.pwd.getpwnam") |
694 | + @mock.patch("rabbitmq_context.grp.getgrnam") |
695 | + @mock.patch("rabbitmq_context.config") |
696 | + @mock.patch("rabbitmq_context.close_port") |
697 | + @mock.patch("rabbitmq_context.ssl_utils.reconfigure_client_ssl") |
698 | + @mock.patch("rabbitmq_context.ssl_utils.get_ssl_mode") |
699 | + def test_context_ssl_on(self, get_ssl_mode, reconfig_ssl, close_port, |
700 | + config, gr, pw, chown, chmod, open_port): |
701 | + |
702 | + get_ssl_mode.return_value = ("on", "on") |
703 | + |
704 | + def config_get(n): |
705 | + return None |
706 | + |
707 | + config.side_effect = config_get |
708 | + |
709 | + def pw(name): |
710 | + class Uid(object): |
711 | + pw_uid = 1 |
712 | + gr_gid = 100 |
713 | + return Uid() |
714 | + |
715 | + pw.side_effect = pw |
716 | + gr.side_effect = pw |
717 | + |
718 | + m = mock.mock_open() |
719 | + with mock.patch('rabbitmq_context.open', m, create=True): |
720 | + self.assertEqual( |
721 | + rabbitmq_context.RabbitMQSSLContext().__call__(), { |
722 | + "ssl_port": None, |
723 | + "ssl_cert_file": "/etc/rabbitmq/rabbit-server-cert.pem", |
724 | + "ssl_key_file": '/etc/rabbitmq/rabbit-server-privkey.pem', |
725 | + "ssl_client": False, |
726 | + "ssl_ca_file": "", |
727 | + "ssl_only": False, |
728 | + "ssl_mode": "on", |
729 | + }) |
730 | + |
731 | + self.assertTrue(reconfig_ssl.called) |
732 | + self.assertTrue(open_port.called) |
733 | + |
734 | + |
735 | +class TestRabbitMQClusterContext(unittest.TestCase): |
736 | + |
737 | + @mock.patch("rabbitmq_context.config") |
738 | + def test_context_ssl_off(self, config): |
739 | + config.return_value = "ignore" |
740 | + |
741 | + self.assertEqual( |
742 | + rabbitmq_context.RabbitMQClusterContext().__call__(), { |
743 | + 'cluster_partition_handling': "ignore" |
744 | + }) |
745 | + |
746 | + config.assert_called_once_with("cluster-partition-handling") |
747 | |
748 | === modified file 'unit_tests/test_rabbitmq_server_relations.py' |
749 | --- unit_tests/test_rabbitmq_server_relations.py 2015-04-20 11:13:39 +0000 |
750 | +++ unit_tests/test_rabbitmq_server_relations.py 2015-09-28 19:34:56 +0000 |
751 | @@ -37,7 +37,7 @@ |
752 | @patch('rabbitmq_server_relations.relation_set') |
753 | @patch('apt_pkg.Cache') |
754 | @patch('rabbitmq_server_relations.is_clustered') |
755 | - @patch('rabbitmq_server_relations.configure_client_ssl') |
756 | + @patch('rabbitmq_server_relations.ssl_utils.configure_client_ssl') |
757 | @patch('rabbitmq_server_relations.unit_get') |
758 | @patch('rabbitmq_server_relations.relation_get') |
759 | @patch('rabbitmq_server_relations.is_elected_leader') |
760 | @@ -87,7 +87,7 @@ |
761 | @patch('rabbitmq_server_relations.relation_set') |
762 | @patch('apt_pkg.Cache') |
763 | @patch('rabbitmq_server_relations.is_clustered') |
764 | - @patch('rabbitmq_server_relations.configure_client_ssl') |
765 | + @patch('rabbitmq_server_relations.ssl_utils.configure_client_ssl') |
766 | @patch('rabbitmq_server_relations.unit_get') |
767 | @patch('rabbitmq_server_relations.relation_get') |
768 | @patch('rabbitmq_server_relations.is_elected_leader') |
Jorge,
I think some of the refactor in here is a definite improvement and makes things cleaner, thanks! I've got a few inline comments included, but the biggest thing is that the rabbitmq-server charm now has a /next and /trunk branch to match the rest of the flow of the openstack charms (since this has recently come under ownership of the ~openstack-charmers team). Can you retarget for /next?
Also, I think the README file should be updated with some discussions regarding high availability.
I'll run through some tests on this but wanted to drop some feedback now.