Merge lp:~xianghui/charm-helpers/support-ipv6 into lp:charm-helpers

[Xiang Hui]

[Hui Xiang]
Allow rabbitmq-server client to get its ipv6 address.
  (rabbitmq-server charm already set "hostname" with relation-set().)
Allow ceph-client to get ipv6 address from 'host-ip'.
  (will let ceph to set "host-ip" with relation-set for clients)
Add get_ipv6_addr() funtion for ipv6.

[Edward Hope-Morley (hopem)]

Need to fx merge conflict.

review: Needs Fixing

[lp:~xianghui/charm-helpers/support-ipv6]

169. By Xiang Hui on 2014-07-29

Fix conflict with trunk.

[Edward Hope-Morley (hopem)]

Hui, I'm getting some lint errors:

$ make lint
Checking for Python syntax...
charmhelpers/contrib/hahelpers/cluster.py:183:25: F821 undefined name 'get_ipv6_addr'
charmhelpers/cli/commands.py:1:1: F401 'CommandLine' imported but unused
charmhelpers/cli/commands.py:2:1: F401 'host' imported but unused
tests/contrib/network/test_ip.py:161:78: W291 trailing whitespace
make: *** [lint] Error 1

Also, could you please update/add unit tests to cover the change you are making.

review: Needs Fixing

[Edward Hope-Morley (hopem)]

Currently getting the following unit test failures as well: https://pastebin.canonical.com/114395/

[Xiang Hui (xianghui)]

> Hui, I'm getting some lint errors:
>
> $ make lint
> Checking for Python syntax...
> charmhelpers/contrib/hahelpers/cluster.py:183:25: F821 undefined name
> 'get_ipv6_addr'
> charmhelpers/cli/commands.py:1:1: F401 'CommandLine' imported but unused
> charmhelpers/cli/commands.py:2:1: F401 'host' imported but unused
> tests/contrib/network/test_ip.py:161:78: W291 trailing whitespace
> make: *** [lint] Error 1
>
> Also, could you please update/add unit tests to cover the change you are
> making.

I don't know why the conflict deleted new added function "get_ipv6_addr()", will added on next patch, and except this error, other errors are not caused by my changes, if these strict lint rules applied to everyone?thanks.

[Xiang Hui (xianghui)]

> Currently getting the following unit test failures as well:
> https://pastebin.canonical.com/114395/

Just the first two errors "get_ipv6_add.." caused by my changes due to the conflict mis-deleted.

[Edward Hope-Morley (hopem)]

If I remove all your changes I get no errors i.e. trunk has no errors.

[lp:~xianghui/charm-helpers/support-ipv6]

170. By Xiang Hui on 2014-07-31

Add ut and solved ut errors and conflicts.

[Xiang Hui (xianghui)]

> If I remove all your changes I get no errors i.e. trunk has no errors.
Done, thanks for your efforts.

[lp:~xianghui/charm-helpers/support-ipv6]

171. By Xiang Hui on 2014-08-04

MV get_ipv6_addr() to network/ip.py

172. By Xiang Hui on 2014-08-04

Merge from dev branch and fix unit test.

173. By Xiang Hui on 2014-08-04

Remove relation set 'hostname' on AMQP.

174. By Xiang Hui on 2014-08-04

If prefer ipv6 addr only, fallback to ipv6.

175. By Xiang Hui on 2014-08-05

Support haproxy for ipv6.

[Edward Hope-Morley (hopem)]

Couple of issues that need fixing.

review: Needs Fixing

[lp:~xianghui/charm-helpers/support-ipv6]

176. By Xiang Hui on 2014-08-06

Fix trivial issue.

[Xiang Hui (xianghui)]

> Couple of issues that need fixing.
Thanks, done.

[Edward Hope-Morley (hopem)]

LGTM +1

review: Approve

=== modified file 'charmhelpers/contrib/network/ip.py'
--- charmhelpers/contrib/network/ip.py	2014-07-24 08:44:12 +0000
+++ charmhelpers/contrib/network/ip.py	2014-08-06 03:04:25 +0000
@@ -4,7 +4,7 @@
 
 from charmhelpers.fetch import apt_install
 from charmhelpers.core.hookenv import (
-    ERROR, log,
+    ERROR, log, config,
 )
 
 try:
@@ -154,3 +154,21 @@
 get_iface_for_address = partial(_get_for_address, key='iface')
 
 get_netmask_for_address = partial(_get_for_address, key='netmask')
+
+
+def get_ipv6_addr(iface="eth0"):
+    try:
+        iface_addrs = netifaces.ifaddresses(iface)
+        if netifaces.AF_INET6 not in iface_addrs:
+            raise Exception("Interface '%s' doesn't have an ipv6 address." % iface)
+
+        addresses = netifaces.ifaddresses(iface)[netifaces.AF_INET6]
+        ipv6_addr = [a['addr'] for a in addresses if not a['addr'].startswith('fe80')
+                     and config('vip') != a['addr']]
+        if not ipv6_addr:
+            raise Exception("Interface '%s' doesn't have global ipv6 address." % iface)
+
+        return ipv6_addr[0]
+
+    except ValueError:
+        raise ValueError("Invalid interface '%s'" % iface)

=== modified file 'charmhelpers/contrib/openstack/context.py'
--- charmhelpers/contrib/openstack/context.py	2014-07-28 11:28:49 +0000
+++ charmhelpers/contrib/openstack/context.py	2014-08-06 03:04:25 +0000
@@ -44,7 +44,10 @@
     neutron_plugin_attribute,
 )
 
-from charmhelpers.contrib.network.ip import get_address_in_network
+from charmhelpers.contrib.network.ip import (
+    get_address_in_network,
+    get_ipv6_addr,
+)
 
 CA_CERT_PATH = '/usr/local/share/ca-certificates/keystone_juju_ca_cert.crt'
 
@@ -401,9 +404,12 @@
 
         cluster_hosts = {}
         l_unit = local_unit().replace('/', '-')
-        cluster_hosts[l_unit] = \
-            get_address_in_network(config('os-internal-network'),
-                                   unit_get('private-address'))
+        if config('prefer-ipv6'):
+            addr = get_ipv6_addr()
+        else:
+            addr = unit_get('private-address')
+        cluster_hosts[l_unit] = get_address_in_network(config('os-internal-network'),
+                                                       addr)
 
         for rid in relation_ids('cluster'):
             for unit in related_units(rid):
@@ -414,6 +420,16 @@
         ctxt = {
             'units': cluster_hosts,
         }
+
+        if config('prefer-ipv6'):
+            ctxt['local_host'] = 'ip6-localhost'
+            ctxt['haproxy_host'] = '::'
+            ctxt['stat_port'] = ':::8888'
+        else:
+            ctxt['local_host'] = '127.0.0.1'
+            ctxt['haproxy_host'] = '0.0.0.0'
+            ctxt['stat_port'] = ':8888'
+
         if len(cluster_hosts.keys()) > 1:
             # Enable haproxy when we have enough peers.
             log('Ensuring haproxy enabled in /etc/default/haproxy.')

=== modified file 'charmhelpers/contrib/openstack/ip.py'
--- charmhelpers/contrib/openstack/ip.py	2014-07-03 15:15:30 +0000
+++ charmhelpers/contrib/openstack/ip.py	2014-08-06 03:04:25 +0000
@@ -7,6 +7,7 @@
     get_address_in_network,
     is_address_in_network,
     is_ipv6,
+    get_ipv6_addr,
 )
 
 from charmhelpers.contrib.hahelpers.cluster import is_clustered
@@ -64,10 +65,13 @@
                         vip):
                     resolved_address = vip
     else:
+        if config('prefer-ipv6'):
+            fallback_addr = get_ipv6_addr()
+        else:
+            fallback_addr = unit_get(_address_map[endpoint_type]['fallback'])
         resolved_address = get_address_in_network(
-            config(_address_map[endpoint_type]['config']),
-            unit_get(_address_map[endpoint_type]['fallback'])
-        )
+            config(_address_map[endpoint_type]['config']), fallback_addr)
+
     if resolved_address is None:
         raise ValueError('Unable to resolve a suitable IP address'
                          ' based on charm state and configuration')

=== modified file 'charmhelpers/contrib/openstack/templates/haproxy.cfg'
--- charmhelpers/contrib/openstack/templates/haproxy.cfg	2014-07-03 12:50:03 +0000
+++ charmhelpers/contrib/openstack/templates/haproxy.cfg	2014-08-06 03:04:25 +0000
@@ -1,6 +1,6 @@
 global
-    log 127.0.0.1 local0
-    log 127.0.0.1 local1 notice
+    log {{ local_host }} local0
+    log {{ local_host }} local1 notice
     maxconn 20000
     user haproxy
     group haproxy
@@ -17,7 +17,7 @@
     timeout client 30000
     timeout server 30000
 
-listen stats :8888
+listen stats {{ stat_port }}
     mode http
     stats enable
     stats hide-version

=== modified file 'tests/contrib/network/test_ip.py'
--- tests/contrib/network/test_ip.py	2014-07-24 08:29:56 +0000
+++ tests/contrib/network/test_ip.py	2014-08-06 03:04:25 +0000
@@ -158,4 +158,50 @@
     def test_is_ipv6(self):
         self.assertFalse(net_ip.is_ipv6('myhost'))
         self.assertFalse(net_ip.is_ipv6('172.4.5.5'))
-        self.assertTrue(net_ip.is_ipv6('2a01:348:2f4:0:685e:5748:ae62:209f'))        
\ No newline at end of file
+        self.assertTrue(net_ip.is_ipv6('2a01:348:2f4:0:685e:5748:ae62:209f'))
+
+    @patch.object(netifaces, 'ifaddresses')
+    def test_get_ipv6_addr_no_ipv6(self, _ifaddresses):
+        DUMMY_ADDRESSES = {
+            'eth0': {
+                2: [{'addr': '192.168.1.55',
+                     'broadcast': '192.168.1.255',
+                     'netmask': '255.255.255.0'}]
+            }
+        }
+
+        def mock_ifaddresses(iface):
+            return DUMMY_ADDRESSES[iface]
+
+        _ifaddresses.side_effect = mock_ifaddresses
+        self.assertRaises(Exception, net_ip.get_ipv6_addr)
+
+    @patch.object(netifaces, 'ifaddresses')
+    def test_get_ipv6_addr_no_global_ipv6(self, _ifaddresses):
+        DUMMY_ADDRESSES = {
+            'eth0': {
+                10: [{'addr': 'fe80::3e97:eff:fe8b:1cf7%eth0',
+                      'netmask': 'ffff:ffff:ffff:ffff::'}],
+            }
+        }
+
+        def mock_ifaddresses(iface):
+            return DUMMY_ADDRESSES[iface]
+
+        _ifaddresses.side_effect = mock_ifaddresses
+        self.assertRaises(Exception, net_ip.get_ipv6_addr)
+
+    @patch.object(netifaces, 'ifaddresses')
+    def test_get_ipv6_addr_invalid_nic(self, _ifaddresses):
+        DUMMY_ADDRESSES = {
+            'eth0': {
+                10: [{'addr': 'fe80::3e97:eff:fe8b:1cf7%eth0',
+                      'netmask': 'ffff:ffff:ffff:ffff::'}],
+            }
+        }
+
+        def mock_ifaddresses(iface):
+            return DUMMY_ADDRESSES[iface]
+
+        _ifaddresses.side_effect = ValueError()
+        self.assertRaises(ValueError, net_ip.get_ipv6_addr, 'eth1')

=== modified file 'tests/contrib/openstack/test_os_contexts.py'
--- tests/contrib/openstack/test_os_contexts.py	2014-07-28 11:28:49 +0000
+++ tests/contrib/openstack/test_os_contexts.py	2014-08-06 03:04:25 +0000
@@ -315,7 +315,8 @@
     'time',
     'https',
     'get_address_in_network',
-    'local_unit'
+    'local_unit',
+    'get_ipv6_addr'
 ]
 
 
@@ -835,6 +836,52 @@
         self.relation_get.side_effect = relation.get
         self.related_units.side_effect = relation.relation_units
         self.get_address_in_network.return_value = 'cluster-peer0.localnet'
+        self.config.return_value = False
+        haproxy = context.HAProxyContext()
+        with patch_open() as (_open, _file):
+            result = haproxy()
+        ex = {
+            'units': {
+                'peer-0': 'cluster-peer0.localnet',
+                'peer-1': 'cluster-peer1.localnet',
+                'peer-2': 'cluster-peer2.localnet',
+            },
+
+            'local_host': '127.0.0.1',
+            'haproxy_host': '0.0.0.0',
+            'stat_port': ':8888',
+        }
+        # the context gets generated.
+        self.assertEquals(ex, result)
+        # and /etc/default/haproxy is updated.
+        self.assertEquals(_file.write.call_args_list,
+                          [call('ENABLED=1\n')])
+
+    @patch('charmhelpers.contrib.openstack.context.unit_get')
+    @patch('charmhelpers.contrib.openstack.context.local_unit')
+    @patch('charmhelpers.contrib.openstack.context.get_ipv6_addr')
+    def test_haproxy_context_with_data_ipv6(
+            self, local_unit, unit_get, get_ipv6_addr):
+        '''Test haproxy context with all relation data'''
+        cluster_relation = {
+            'cluster:0': {
+                'peer/1': {
+                    'private-address': 'cluster-peer1.localnet',
+                },
+                'peer/2': {
+                    'private-address': 'cluster-peer2.localnet',
+                },
+            },
+        }
+
+        unit_get.return_value = 'peer/0'
+        relation = FakeRelation(cluster_relation)
+        self.relation_ids.side_effect = relation.relation_ids
+        self.relation_get.side_effect = relation.get
+        self.related_units.side_effect = relation.relation_units
+        self.get_address_in_network.return_value = 'cluster-peer0.localnet'
+        self.get_ipv6_addr.return_value = 'cluster-peer0.localnet'
+        self.config.side_effect = [True, None, True]
         haproxy = context.HAProxyContext()
         with patch_open() as (_open, _file):
             result = haproxy()
@@ -843,7 +890,11 @@
                 'peer-0': 'cluster-peer0.localnet',
                 'peer-1': 'cluster-peer1.localnet',
                 'peer-2': 'cluster-peer2.localnet'
-            }
+            },
+
+            'local_host': 'ip6-localhost',
+            'haproxy_host': '::',
+            'stat_port': ':::8888',
         }
         # the context gets generated.
         self.assertEquals(ex, result)
@@ -876,6 +927,7 @@
         self.relation_ids.side_effect = relation.relation_ids
         self.relation_get.side_effect = relation.get
         self.related_units.side_effect = relation.relation_units
+        self.config.return_value = False
         haproxy = context.HAProxyContext()
         self.assertEquals({}, haproxy())