Merge lp:~niedbalski/charms/trusty/rabbitmq-server/fix-lp-1489053 into lp:~openstack-charmers-archive/charms/trusty/rabbitmq-server/next

[Jorge Niedbalski]

- Enforce the update of /etc/hosts file on both cases (now it's just enforced
  on ipv6) for fixing cases like LP: #1489053.

- Remove the socket.gethostname() on cluster_joined relation, this is incorrect
if the intention is to determine the remote unit ip address, instead
uses the charmhelpers.contrib.network.ip.get_hostname method that
makes all the resolution logic.

[Jorge Niedbalski (niedbalski)]

This writes the /etc/hosts file and also makes the cluster relation properly.

https://gist.github.com/niedbalski/5d2ad08efd2af5938862

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

charm_lint_check #8762 rabbitmq-server-next for niedbalski mp269261
    LINT FAIL: lint-test failed

LINT Results (max last 2 lines):
make: *** [lint] Error 1
ERROR:root:Make target returned non-zero.

Full lint test output: http://paste.ubuntu.com/12202004/
Build: http://10.245.162.77:8080/job/charm_lint_check/8762/

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

charm_unit_test #8093 rabbitmq-server-next for niedbalski mp269261
    UNIT OK: passed

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

[Felipe Reyes (freyes)]

Works as expected. Thanks @niedbalski.

 juju run --service rabbit 'cat /etc/hosts'
- MachineId: "10"
  Stderr: "Warning: Permanently added '10.0.3.234' (ECDSA) to the list of known hosts.\r\n"
  Stdout: |
    127.0.0.1 localhost

    # The following lines are desirable for IPv6 capable hosts
    ::1 ip6-localhost ip6-loopback
    fe00::0 ip6-localnet
    ff00::0 ip6-mcastprefix
    ff02::1 ip6-allnodes
    ff02::2 ip6-allrouters
    ff02::3 ip6-allhosts
    10.0.3.194 freyes-lxc-machine-8
    10.0.3.204 freyes-lxc-machine-9
  UnitId: rabbit/2
- MachineId: "8"
  Stderr: "Warning: Permanently added '10.0.3.194' (ECDSA) to the list of known hosts.\r\n"
  Stdout: |
    127.0.0.1 localhost

    # The following lines are desirable for IPv6 capable hosts
    ::1 ip6-localhost ip6-loopback
    fe00::0 ip6-localnet
    ff00::0 ip6-mcastprefix
    ff02::1 ip6-allnodes
    ff02::2 ip6-allrouters
    ff02::3 ip6-allhosts
    10.0.3.204 freyes-lxc-machine-9
    10.0.3.234 freyes-lxc-machine-10
  UnitId: rabbit/0
- MachineId: "9"
  Stderr: "Warning: Permanently added '10.0.3.204' (ECDSA) to the list of known hosts.\r\n"
  Stdout: |
    127.0.0.1 localhost

    # The following lines are desirable for IPv6 capable hosts
    ::1 ip6-localhost ip6-loopback
    fe00::0 ip6-localnet
    ff00::0 ip6-mcastprefix
    ff02::1 ip6-allnodes
    ff02::2 ip6-allrouters
    ff02::3 ip6-allhosts
    10.0.3.194 freyes-lxc-machine-8
    10.0.3.234 freyes-lxc-machine-10
  UnitId: rabbit/1

review: Approve

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

charm_amulet_test #6039 rabbitmq-server-next for niedbalski mp269261
    AMULET OK: passed

Build: http://10.245.162.77:8080/job/charm_amulet_test/6039/

[Billy Olsen (billy-olsen)]

I think this change makes sense, especially in the general sense (not just in the OpenStack specific use case). For the openstack deployment case, dns resolution needs to be available for other services to properly work - however it has been argued (and I tend to agree) that simply deploying a rabbitmq-server cluster should "just work".

There's a minor lint issue that needs to be resolved, but other than that I'm happy to land this in the /next branches.

review: Needs Fixing

[Jorge Niedbalski (niedbalski)]

Fixed the lint issue.

[Billy Olsen (billy-olsen)]

Thanks for fixing this Jorge.

review: Approve

[Ryan Beisner (1chb1n)]

+1 to preserving at least the existing level of user feedback.

review: Needs Resubmitting

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

charm_lint_check #9164 rabbitmq-server-next for niedbalski mp269261
    LINT OK: passed

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

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

charm_unit_test #8469 rabbitmq-server-next for niedbalski mp269261
    UNIT OK: passed

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

[Ryan Beisner (1chb1n)]

Removal of this conditional:
83 - if config('prefer-ipv6') and rdata.get('hostname'):

Makes this:
hostname = rdata.get('hostname', None)

Throw error:
http://paste.ubuntu.com/12242485/

2015-09-01 01:24:22 INFO leader-settings-changed hostname = rdata.get('hostname', None)
2015-09-01 01:24:22 INFO leader-settings-changed AttributeError: 'NoneType' object has no attribute 'get'

review: Needs Fixing

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

charm_amulet_test #6179 rabbitmq-server-next for niedbalski mp269261
    AMULET OK: passed

Build: http://10.245.162.77:8080/job/charm_amulet_test/6179/

[Jorge Niedbalski (niedbalski)]

Fixed both comments, @beisner and @dosaboy.

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

charm_unit_test #8476 rabbitmq-server-next for niedbalski mp269261
    UNIT OK: passed

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

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

charm_lint_check #9172 rabbitmq-server-next for niedbalski mp269261
    LINT OK: passed

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

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

charm_amulet_test #6181 rabbitmq-server-next for niedbalski mp269261
    AMULET FAIL: amulet-test failed

AMULET Results (max last 2 lines):
make: *** [functional_test] Error 124
ERROR:root:Make target returned non-zero.

Full amulet test output: http://paste.ubuntu.com/12248128/
Build: http://10.245.162.77:8080/job/charm_amulet_test/6181/

[lp:~niedbalski/charms/trusty/rabbitmq-server/fix-lp-1489053]

110. By Liam Young on 2015-09-03

[gnuoy,trivial] Charmhelper sync (+1'd by mojo)

111. By Liam Young on 2015-09-09

[thedac, r=gnuoy] Fix clustering race conditions for LP Bug#1486177

112. By Liam Young on 2015-09-09

[1chb1n, r=gnuoy] Refactor amulet tests, deprecate old tests

113. By Jorge Niedbalski on 2015-09-16

Rebased again

Unmerged revisions

=== modified file 'hooks/charmhelpers/contrib/network/ip.py'
--- hooks/charmhelpers/contrib/network/ip.py	2015-09-01 04:53:33 +0000
+++ hooks/charmhelpers/contrib/network/ip.py	2015-09-01 19:52:23 +0000
@@ -435,8 +435,12 @@
 
         rev = dns.reversename.from_address(address)
         result = ns_query(rev)
+
         if not result:
-            return None
+            try:
+                result = socket.gethostbyaddr(address)[0]
+            except:
+                return None
     else:
         result = address
 
 
=== modified file 'hooks/rabbit_utils.py'
--- hooks/rabbit_utils.py	2015-09-01 04:53:33 +0000
+++ hooks/rabbit_utils.py	2015-09-01 19:52:23 +0000
@@ -2,7 +2,6 @@
 import pwd
 import grp
 import re
-import socket
 import sys
 import subprocess
 import glob
@@ -270,18 +269,13 @@
                 address = relation_get('private-address',
                                        rid=r_id, unit=unit)
             if address is not None:
-                try:
-                    node = get_hostname(address, fqdn=False)
-                except:
+                node = get_hostname(address, fqdn=False)
+                if node:
+                    available_nodes.append(node)
+                else:
                     log('Cannot resolve hostname for {} '
-                        'using DNS servers'.format(address), level='WARNING')
-                    log('Falling back to use socket.gethostname()',
+                        'using DNS servers'.format(address),
                         level='WARNING')
-                    # If the private-address is not resolvable using DNS
-                    # then use the current hostname
-                    node = socket.gethostname()
-
-                available_nodes.append(node)
 
     if len(available_nodes) == 0:
         log('No nodes available to cluster with')
 
=== modified file 'hooks/rabbitmq_server_relations.py'
--- hooks/rabbitmq_server_relations.py	2015-09-01 04:53:33 +0000
+++ hooks/rabbitmq_server_relations.py	2015-09-01 19:52:23 +0000
@@ -272,11 +272,18 @@
 
 @hooks.hook('cluster-relation-joined')
 def cluster_joined(relation_id=None):
+    relation_settings = {
+        'hostname': get_local_nodename(),
+    }
+
     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)
+        relation_settings['private-address'] = get_ipv6_addr()[0]
+    else:
+        relation_settings['private-address'] = get_host_ip(
+            unit_get('private-address'))
+
+    relation_set(relation_id=relation_id,
+                 relation_settings=relation_settings)
 
     if is_relation_made('ha') and \
             config('ha-vip-only') is False:
@@ -316,10 +323,11 @@
         return
 
     rdata = relation_get()
-    if config('prefer-ipv6') and rdata.get('hostname'):
-        private_address = rdata['private-address']
-        hostname = rdata['hostname']
-        if hostname:
+    if rdata:
+        hostname = rdata.get('hostname', None)
+        private_address = rdata.get('private-address', None)
+
+        if hostname and private_address:
             rabbit.update_hosts_file({private_address: hostname})
 
     # sync passwords