Merge lp:~niedbalski/charms/trusty/rabbitmq-server/hosts-ipv4 into lp:charms/trusty/rabbitmq-server

Proposed by Jorge Niedbalski
Status: Rejected
Rejected by: David Britton
Proposed branch: lp:~niedbalski/charms/trusty/rabbitmq-server/hosts-ipv4
Merge into: lp:charms/trusty/rabbitmq-server
Diff against target: 187 lines (+113/-13)
3 files modified
Makefile (+1/-1)
hooks/rabbitmq_server_relations.py (+19/-11)
unit_tests/test_rabbitmq_server_relations.py (+93/-1)
To merge this branch: bzr merge lp:~niedbalski/charms/trusty/rabbitmq-server/hosts-ipv4
Reviewer Review Type Date Requested Status
James Page Disapprove
Review Queue (community) automated testing Approve
Felipe Reyes (community) Approve
Review via email: mp+244492@code.launchpad.net

Description of the change

Prior to this change, the /etc/hosts file was populated only when ipv6-prefer is set, this change removes the ipv6 constraint.

- Added tests to cover the change.

To post a comment you must log in.
Revision history for this message
Felipe Reyes (freyes) wrote :

LGTM, the cluster was formed successfully with this change /etc/hosts contained IPv4 values.

$ juju run --service rabbitmq-server 'grep machine /etc/hosts'
- MachineId: "1"
  Stderr: "Warning: Permanently added '10.0.3.7' (ECDSA) to the list of known hosts.\r\n"
  Stdout: '10.0.3.176 freyes-local-machine-3

    10.0.3.241 freyes-local-machine-2

'
  UnitId: rabbitmq-server/0
- MachineId: "2"
  Stderr: "Warning: Permanently added '10.0.3.241' (ECDSA) to the list of known hosts.\r\n"
  Stdout: '10.0.3.176 freyes-local-machine-3

    10.0.3.7 freyes-local-machine-1

'
  UnitId: rabbitmq-server/1
- MachineId: "3"
  Stderr: "Warning: Permanently added '10.0.3.176' (ECDSA) to the list of known hosts.\r\n"
  Stdout: '10.0.3.7 freyes-local-machine-1

    10.0.3.241 freyes-local-machine-2

'
  UnitId: rabbitmq-server/2

review: Approve
Revision history for this message
uosci-testing-bot (uosci-testing-bot) wrote :

charm_lint_check #222 trusty-rabbitmq-server for niedbalski mp244492
    LINT OK: passed

Build: http://10.230.18.80:8080/job/charm_lint_check/222/

Revision history for this message
uosci-testing-bot (uosci-testing-bot) wrote :

charm_unit_test #185 trusty-rabbitmq-server for niedbalski mp244492
    UNIT OK: passed

Build: http://10.230.18.80:8080/job/charm_unit_test/185/

Revision history for this message
uosci-testing-bot (uosci-testing-bot) wrote :

charm_amulet_test #139 trusty-rabbitmq-server for niedbalski mp244492
    AMULET FAIL: amulet-test missing

AMULET Results (max last 2 lines):
INFO:root:Search string not found in makefile target commands.
ERROR:root:No make target was executed.

Full amulet test output: pastebin not avail., cmd error
Build: http://10.230.18.80:8080/job/charm_amulet_test/139/

Revision history for this message
Review Queue (review-queue) wrote :

The results (PASS) are in and available here: http://reports.vapour.ws/charm-tests/charm-bundle-test-10683-results

review: Approve (automated testing)
Revision history for this message
Felipe Reyes (freyes) wrote :
Download full text (3.5 KiB)

Everything worked OK with a 3 node cluster in a openstack cloud without dns support for the instances

Here is the evidence:

environment: openstack-freyes
machines:
  "0":
    agent-state: started
    agent-version: 1.20.14
    dns-name: 10.55.61.143
    instance-id: cfdbd16e-6ce8-4996-a552-105b0d905a32
    instance-state: ACTIVE
    series: trusty
    hardware: arch=amd64 cpu-cores=1 mem=1024M root-disk=10240M
    state-server-member-status: has-vote
  "1":
    agent-state: started
    agent-version: 1.20.14
    dns-name: 10.55.61.145
    instance-id: a96f334c-31fb-495d-a4ca-96e063f4b672
    instance-state: ACTIVE
    series: trusty
    hardware: arch=amd64 cpu-cores=1 mem=1024M root-disk=10240M
  "2":
    agent-state: started
    agent-version: 1.20.14
    dns-name: 10.55.61.146
    instance-id: fe3b66a5-4a2b-4cf1-9f03-cccbf56c7356
    instance-state: ACTIVE
    series: trusty
    hardware: arch=amd64 cpu-cores=1 mem=1024M root-disk=10240M
  "3":
    agent-state: started
    agent-version: 1.20.14
    dns-name: 10.55.61.147
    instance-id: 7a61ee9e-acd6-44a5-911d-61c1cb732f52
    instance-state: ACTIVE
    series: trusty
    hardware: arch=amd64 cpu-cores=1 mem=1024M root-disk=10240M
services:
  rabbitmq-server:
    charm: local:trusty/rabbitmq-server-128
    exposed: false
    relations:
      cluster:
      - rabbitmq-server
    units:
      rabbitmq-server/0:
        agent-state: started
        agent-version: 1.20.14
        machine: "1"
        open-ports:
        - 5672/tcp
        public-address: 10.55.61.145
      rabbitmq-server/1:
        agent-state: started
        agent-version: 1.20.14
        machine: "2"
        open-ports:
        - 5672/tcp
        public-address: 10.55.61.146
      rabbitmq-server/2:
        agent-state: started
        agent-version: 1.20.14
        machine: "3"
        open-ports:
        - 5672/tcp
        public-address: 10.55.61.147
$ for i in 0 1 2; do juju ssh rabbitmq-server/$i 'sudo rabbitmqctl cluster_status'; done
Cluster status of node 'rabbit@juju-openstack-freyes-machine-1' ...
[{nodes,[{disc,['rabbit@juju-openstack-freyes-machine-1',
                'rabbit@juju-openstack-freyes-machine-2',
                'rabbit@juju-openstack-freyes-machine-3']}]},
 {running_nodes,['rabbit@juju-openstack-freyes-machine-3',
                 'rabbit@juju-openstack-freyes-machine-2',
                 'rabbit@juju-openstack-freyes-machine-1']},
 {partitions,[]}]
...done.
Connection to 10.55.61.145 closed.
Warning: Permanently added '10.55.61.146' (ECDSA) to the list of known hosts.
Cluster status of node 'rabbit@juju-openstack-freyes-machine-2' ...
[{nodes,[{disc,['rabbit@juju-openstack-freyes-machine-1',
                'rabbit@juju-openstack-freyes-machine-2',
                'rabbit@juju-openstack-freyes-machine-3']}]},
 {running_nodes,['rabbit@juju-openstack-freyes-machine-3',
                 'rabbit@juju-openstack-freyes-machine-1',
                 'rabbit@juju-openstack-freyes-machine-2']},
 {partitions,[]}]
...done.
Connection to 10.55.61.146 closed.
Warning: Permanently added '10.55.61.147' (ECDSA) to the list of known hosts.
Cluster status of node 'rabbit@juju-openstack-freyes-machine-3' ...
[{n...

Read more...

Revision history for this message
Jorge Niedbalski (niedbalski) wrote :

Thanks Felipe for your review.

Revision history for this message
uosci-testing-bot (uosci-testing-bot) wrote :

charm_lint_check #82 trusty-rabbitmq-server for niedbalski mp244492
    LINT OK: passed

Build: http://10.245.162.77:8080/job/charm_lint_check/82/

Revision history for this message
uosci-testing-bot (uosci-testing-bot) wrote :

charm_lint_check #84 trusty-rabbitmq-server for niedbalski mp244492
    LINT OK: passed

Build: http://10.245.162.77:8080/job/charm_lint_check/84/

Revision history for this message
uosci-testing-bot (uosci-testing-bot) wrote :

charm_lint_check #90 trusty-rabbitmq-server for niedbalski mp244492
    LINT OK: passed

Build: http://10.245.162.77:8080/job/charm_lint_check/90/

Revision history for this message
uosci-testing-bot (uosci-testing-bot) wrote :

charm_unit_test #119 trusty-rabbitmq-server for niedbalski mp244492
    UNIT OK: passed

Build: http://10.245.162.77:8080/job/charm_unit_test/119/

Revision history for this message
uosci-testing-bot (uosci-testing-bot) wrote :

charm_unit_test #120 trusty-rabbitmq-server for niedbalski mp244492
    UNIT OK: passed

Build: http://10.245.162.77:8080/job/charm_unit_test/120/

Revision history for this message
uosci-testing-bot (uosci-testing-bot) wrote :

charm_unit_test #122 trusty-rabbitmq-server for niedbalski mp244492
    UNIT OK: passed

Build: http://10.245.162.77:8080/job/charm_unit_test/122/

Revision history for this message
James Page (james-page) wrote :

It's not a unrealistic expectation that IP address resolution should be functional within a MAAS managed environment; I'm reticent to put in a workaround in the charm as this feels like we're fixing the problem in the wrong place, and this will not just be a problem with the rabbitmq-server charm.

review: Disapprove
Revision history for this message
David Britton (dpb) wrote :

Hi Jorge -- Given feedback from James, I'd look into a fix in the environment where you are hitting this issue so other charms don't fall over.

Unmerged revisions

75. By Jorge Niedbalski

- Added /etc/hosts addition on ipv4 and ipv6 cases.
- Added tests for cover the changes

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'Makefile'
--- Makefile 2014-10-08 15:57:57 +0000
+++ Makefile 2014-12-11 18:56:59 +0000
@@ -22,5 +22,5 @@
2222
23unit_test:23unit_test:
24 @echo Starting tests...24 @echo Starting tests...
25 CHARM_DIR=$(CHARM_DIR) $(TEST_PREFIX) nosetests unit_tests25 CHARM_DIR=$(CHARM_DIR) $(TEST_PREFIX) nosetests -s --nologcapture unit_tests
2626
2727
=== modified file 'hooks/rabbitmq_server_relations.py'
--- hooks/rabbitmq_server_relations.py 2014-11-27 11:07:01 +0000
+++ hooks/rabbitmq_server_relations.py 2014-12-11 18:56:59 +0000
@@ -8,6 +8,7 @@
8import socket8import socket
99
10import rabbit_utils as rabbit10import rabbit_utils as rabbit
11
11from lib.utils import (12from lib.utils import (
12 chown, chmod,13 chown, chmod,
13 is_newer,14 is_newer,
@@ -167,11 +168,6 @@
167168
168@hooks.hook('cluster-relation-joined')169@hooks.hook('cluster-relation-joined')
169def cluster_joined(relation_id=None):170def cluster_joined(relation_id=None):
170 if config('prefer-ipv6'):
171 relation_settings = {'hostname': socket.gethostname(),
172 'private-address': get_ipv6_addr()[0]}
173 relation_set(relation_id=relation_id,
174 relation_settings=relation_settings)
175171
176 if is_relation_made('ha') and \172 if is_relation_made('ha') and \
177 config('ha-vip-only') is False:173 config('ha-vip-only') is False:
@@ -182,7 +178,12 @@
182 # Set RABBITMQ_NODENAME to something that's resolvable by my peers178 # Set RABBITMQ_NODENAME to something that's resolvable by my peers
183 # get_host_ip() is called to sanitize private-address in case it179 # get_host_ip() is called to sanitize private-address in case it
184 # doesn't return an IP address180 # doesn't return an IP address
185 ip_addr = get_host_ip(unit_get('private-address'))181
182 if config('prefer-ipv6'):
183 ip_addr = get_ipv6_addr()[0]
184 else:
185 ip_addr = get_host_ip(unit_get('private-address'))
186
186 try:187 try:
187 nodename = get_hostname(ip_addr, fqdn=False)188 nodename = get_hostname(ip_addr, fqdn=False)
188 except:189 except:
@@ -194,6 +195,12 @@
194 # then use the current hostname195 # then use the current hostname
195 nodename = socket.gethostname()196 nodename = socket.gethostname()
196197
198 relation_set(relation_id=relation_id,
199 relation_settings={
200 'hostname': nodename,
201 'private-address': ip_addr,
202 })
203
197 if nodename:204 if nodename:
198 log('forcing nodename=%s' % nodename)205 log('forcing nodename=%s' % nodename)
199 # need to stop it under current nodename206 # need to stop it under current nodename
@@ -222,11 +229,12 @@
222 log('cluster_joined: cookie not yet set.')229 log('cluster_joined: cookie not yet set.')
223 return230 return
224231
225 if config('prefer-ipv6') and rdata.get('hostname'):232 hostname = rdata.get('hostname', None)
226 private_address = rdata['private-address']233 if hostname:
227 hostname = rdata['hostname']234 private_addr = rdata.get('private-address')
228 if hostname:235 rabbit.update_hosts_file({
229 rabbit.update_hosts_file({private_address: hostname})236 private_addr: hostname,
237 })
230238
231 # sync passwords239 # sync passwords
232 blacklist = ['hostname', 'private-address', 'public-address']240 blacklist = ['hostname', 'private-address', 'public-address']
233241
=== modified file 'unit_tests/test_rabbitmq_server_relations.py'
--- unit_tests/test_rabbitmq_server_relations.py 2014-11-27 10:52:50 +0000
+++ unit_tests/test_rabbitmq_server_relations.py 2014-12-11 18:56:59 +0000
@@ -1,6 +1,8 @@
1import os1import os
2import io
3
2from testtools import TestCase4from testtools import TestCase
3from mock import patch, MagicMock5from mock import patch, MagicMock, mock_open
46
5os.environ['JUJU_UNIT_NAME'] = 'UNIT_TEST/0'7os.environ['JUJU_UNIT_NAME'] = 'UNIT_TEST/0'
6import rabbitmq_server_relations8import rabbitmq_server_relations
@@ -125,3 +127,93 @@
125 mock_peer_store_and_set.assert_called_with(127 mock_peer_store_and_set.assert_called_with(
126 relation_settings={'private-address': ipv6_addr},128 relation_settings={'private-address': ipv6_addr},
127 relation_id=None)129 relation_id=None)
130
131 @patch.object(rabbitmq_server_relations, 'rabbit')
132 @patch('rabbitmq_server_relations.relation_ids')
133 @patch('rabbitmq_server_relations.relation_get')
134 @patch('rabbitmq_server_relations.peer_echo')
135 @patch('rabbitmq_server_relations.peer_retrieve')
136 @patch('rabbitmq_server_relations.is_relation_made')
137 @patch('rabbitmq_server_relations.config')
138 @patch('rabbitmq_server_relations.log')
139 @patch('rabbitmq_server_relations.is_newer')
140 @patch('rabbitmq_server_relations.update_nrpe_checks')
141 @patch('rabbitmq_server_relations.related_units')
142 def test_cluster_relation_changed_update_hosts(self,
143 related_units,
144 update_nrpe_checks,
145 is_newer,
146 log,
147 mock_config,
148 is_relation_made,
149 peer_retrieve,
150 peer_echo,
151 relation_get,
152 relation_id,
153 rabbit):
154 relation_get.return_value = {
155 'cookie': 'cookie-contents',
156 'hostname': 'rabbitmq.test',
157 'private-address': '10.0.0.1',
158 }
159
160 peer_retrieve.return_value = relation_get.return_value.get('cookie')
161 open_ = mock_open()
162 open_.return_value = io.BytesIO()
163
164 with patch('rabbitmq_server_relations.open', open_, create=True):
165 rabbitmq_server_relations.cluster_changed()
166
167 rabbit.update_hosts_file.assert_called_once_with({
168 relation_get.return_value.get('private-address'):
169 relation_get.return_value.get('hostname')
170 })
171
172 @patch.object(rabbitmq_server_relations, 'rabbit')
173 @patch('rabbitmq_server_relations.service_stop')
174 @patch('rabbitmq_server_relations.service_restart')
175 @patch('rabbitmq_server_relations.unit_get')
176 @patch('rabbitmq_server_relations.relation_set')
177 @patch('rabbitmq_server_relations.get_ipv6_addr')
178 @patch('rabbitmq_server_relations.get_host_ip')
179 @patch('rabbitmq_server_relations.get_hostname')
180 @patch('rabbitmq_server_relations.peer_store')
181 @patch('rabbitmq_server_relations.is_relation_made')
182 @patch('rabbitmq_server_relations.config')
183 @patch('rabbitmq_server_relations.log')
184 @patch('rabbitmq_server_relations.is_newer')
185 def test_cluster_relation_joined_hosts(self,
186 is_newer,
187 log,
188 mock_config,
189 is_relation_made,
190 peer_store,
191 get_hostname,
192 get_host_ip,
193 get_ipv6_addr,
194 relation_set,
195 unit_get,
196 service_restart,
197 service_stop,
198 rabbit):
199
200 get_hostname.return_value = "rabbit.test"
201 get_host_ip.return_value = "10.0.0.1"
202 is_relation_made.return_value = False
203
204 def config(key):
205 if key == 'ha-vip-only':
206 return True
207 return None
208
209 mock_config.side_effect = config
210 open_ = mock_open()
211 open_.return_value = io.BytesIO()
212
213 with patch('rabbitmq_server_relations.open', open_, create=True):
214 rabbitmq_server_relations.cluster_joined()
215
216 relation_set.assert_called_with(
217 relation_settings={'private-address': get_host_ip.return_value,
218 'hostname': get_hostname.return_value},
219 relation_id=None)

Subscribers

People subscribed via source and target branches