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

[Jorge Niedbalski]

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.

[Felipe Reyes (freyes)]

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

[uosci-testing-bot (uosci-testing-bot)]

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/

[uosci-testing-bot (uosci-testing-bot)]

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/

[uosci-testing-bot (uosci-testing-bot)]

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/

[Review Queue (review-queue)]

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

review: Approve (automated testing)

[Felipe Reyes (freyes)]

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...

[Jorge Niedbalski (niedbalski)]

Thanks Felipe for your review.

[uosci-testing-bot (uosci-testing-bot)]

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/

[uosci-testing-bot (uosci-testing-bot)]

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/

[uosci-testing-bot (uosci-testing-bot)]

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/

[uosci-testing-bot (uosci-testing-bot)]

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/

[uosci-testing-bot (uosci-testing-bot)]

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/

[uosci-testing-bot (uosci-testing-bot)]

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/

[James Page (james-page)]

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

[David Britton (dpb)]

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 on 2014-12-11

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

=== modified file 'Makefile'
--- Makefile	2014-10-08 15:57:57 +0000
+++ Makefile	2014-12-11 18:56:59 +0000
@@ -22,5 +22,5 @@
 
 unit_test:
 	@echo Starting tests...
-	CHARM_DIR=$(CHARM_DIR) $(TEST_PREFIX) nosetests unit_tests
+	CHARM_DIR=$(CHARM_DIR) $(TEST_PREFIX) nosetests -s --nologcapture unit_tests
 

=== 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 @@
 import socket
 
 import rabbit_utils as rabbit
+
 from lib.utils import (
     chown, chmod,
     is_newer,
@@ -167,11 +168,6 @@
 
 @hooks.hook('cluster-relation-joined')
 def cluster_joined(relation_id=None):
-    if config('prefer-ipv6'):
-        relation_settings = {'hostname': socket.gethostname(),
-                             'private-address': get_ipv6_addr()[0]}
-        relation_set(relation_id=relation_id,
-                     relation_settings=relation_settings)
 
     if is_relation_made('ha') and \
             config('ha-vip-only') is False:
@@ -182,7 +178,12 @@
     # Set RABBITMQ_NODENAME to something that's resolvable by my peers
     # get_host_ip() is called to sanitize private-address in case it
     # doesn't return an IP address
-    ip_addr = get_host_ip(unit_get('private-address'))
+
+    if config('prefer-ipv6'):
+        ip_addr = get_ipv6_addr()[0]
+    else:
+        ip_addr = get_host_ip(unit_get('private-address'))
+
     try:
         nodename = get_hostname(ip_addr, fqdn=False)
     except:
@@ -194,6 +195,12 @@
         # then use the current hostname
         nodename = socket.gethostname()
 
+    relation_set(relation_id=relation_id,
+                 relation_settings={
+                     'hostname': nodename,
+                     'private-address': ip_addr,
+                 })
+
     if nodename:
         log('forcing nodename=%s' % nodename)
         # need to stop it under current nodename
@@ -222,11 +229,12 @@
         log('cluster_joined: cookie not yet set.')
         return
 
-    if config('prefer-ipv6') and rdata.get('hostname'):
-        private_address = rdata['private-address']
-        hostname = rdata['hostname']
-        if hostname:
-            rabbit.update_hosts_file({private_address: hostname})
+    hostname = rdata.get('hostname', None)
+    if hostname:
+        private_addr = rdata.get('private-address')
+        rabbit.update_hosts_file({
+            private_addr: hostname,
+        })
 
     # sync passwords
     blacklist = ['hostname', 'private-address', 'public-address']

=== 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 @@
 import os
+import io
+
 from testtools import TestCase
-from mock import patch, MagicMock
+from mock import patch, MagicMock, mock_open
 
 os.environ['JUJU_UNIT_NAME'] = 'UNIT_TEST/0'
 import rabbitmq_server_relations
@@ -125,3 +127,93 @@
         mock_peer_store_and_set.assert_called_with(
             relation_settings={'private-address': ipv6_addr},
             relation_id=None)
+
+    @patch.object(rabbitmq_server_relations, 'rabbit')
+    @patch('rabbitmq_server_relations.relation_ids')
+    @patch('rabbitmq_server_relations.relation_get')
+    @patch('rabbitmq_server_relations.peer_echo')
+    @patch('rabbitmq_server_relations.peer_retrieve')
+    @patch('rabbitmq_server_relations.is_relation_made')
+    @patch('rabbitmq_server_relations.config')
+    @patch('rabbitmq_server_relations.log')
+    @patch('rabbitmq_server_relations.is_newer')
+    @patch('rabbitmq_server_relations.update_nrpe_checks')
+    @patch('rabbitmq_server_relations.related_units')
+    def test_cluster_relation_changed_update_hosts(self,
+                                                   related_units,
+                                                   update_nrpe_checks,
+                                                   is_newer,
+                                                   log,
+                                                   mock_config,
+                                                   is_relation_made,
+                                                   peer_retrieve,
+                                                   peer_echo,
+                                                   relation_get,
+                                                   relation_id,
+                                                   rabbit):
+        relation_get.return_value = {
+            'cookie': 'cookie-contents',
+            'hostname': 'rabbitmq.test',
+            'private-address': '10.0.0.1',
+        }
+
+        peer_retrieve.return_value = relation_get.return_value.get('cookie')
+        open_ = mock_open()
+        open_.return_value = io.BytesIO()
+
+        with patch('rabbitmq_server_relations.open', open_, create=True):
+            rabbitmq_server_relations.cluster_changed()
+
+        rabbit.update_hosts_file.assert_called_once_with({
+            relation_get.return_value.get('private-address'):
+            relation_get.return_value.get('hostname')
+        })
+
+    @patch.object(rabbitmq_server_relations, 'rabbit')
+    @patch('rabbitmq_server_relations.service_stop')
+    @patch('rabbitmq_server_relations.service_restart')
+    @patch('rabbitmq_server_relations.unit_get')
+    @patch('rabbitmq_server_relations.relation_set')
+    @patch('rabbitmq_server_relations.get_ipv6_addr')
+    @patch('rabbitmq_server_relations.get_host_ip')
+    @patch('rabbitmq_server_relations.get_hostname')
+    @patch('rabbitmq_server_relations.peer_store')
+    @patch('rabbitmq_server_relations.is_relation_made')
+    @patch('rabbitmq_server_relations.config')
+    @patch('rabbitmq_server_relations.log')
+    @patch('rabbitmq_server_relations.is_newer')
+    def test_cluster_relation_joined_hosts(self,
+                                           is_newer,
+                                           log,
+                                           mock_config,
+                                           is_relation_made,
+                                           peer_store,
+                                           get_hostname,
+                                           get_host_ip,
+                                           get_ipv6_addr,
+                                           relation_set,
+                                           unit_get,
+                                           service_restart,
+                                           service_stop,
+                                           rabbit):
+
+        get_hostname.return_value = "rabbit.test"
+        get_host_ip.return_value = "10.0.0.1"
+        is_relation_made.return_value = False
+
+        def config(key):
+            if key == 'ha-vip-only':
+                return True
+            return None
+
+        mock_config.side_effect = config
+        open_ = mock_open()
+        open_.return_value = io.BytesIO()
+
+        with patch('rabbitmq_server_relations.open', open_, create=True):
+            rabbitmq_server_relations.cluster_joined()
+
+        relation_set.assert_called_with(
+            relation_settings={'private-address': get_host_ip.return_value,
+                               'hostname': get_hostname.return_value},
+            relation_id=None)