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

Proposed by Xiang Hui
Status: Merged
Merged at revision: 193
Proposed branch: lp:~xianghui/charm-helpers/support-ipv6
Merge into: lp:charm-helpers
Diff against target: 278 lines (+150/-14)
6 files modified
charmhelpers/contrib/network/ip.py (+19/-1)
charmhelpers/contrib/openstack/context.py (+20/-4)
charmhelpers/contrib/openstack/ip.py (+7/-3)
charmhelpers/contrib/openstack/templates/haproxy.cfg (+3/-3)
tests/contrib/network/test_ip.py (+47/-1)
tests/contrib/openstack/test_os_contexts.py (+54/-2)
To merge this branch: bzr merge lp:~xianghui/charm-helpers/support-ipv6
Reviewer Review Type Date Requested Status
Edward Hope-Morley Approve
Review via email: mp+228637@code.launchpad.net

Description of the change

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

To post a comment you must log in.
Revision history for this message
Edward Hope-Morley (hopem) wrote :

Need to fx merge conflict.

review: Needs Fixing
169. By Xiang Hui

Fix conflict with trunk.

Revision history for this message
Edward Hope-Morley (hopem) wrote :

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
Revision history for this message
Edward Hope-Morley (hopem) wrote :

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

Revision history for this message
Xiang Hui (xianghui) wrote :

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

Revision history for this message
Xiang Hui (xianghui) wrote :

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

Revision history for this message
Edward Hope-Morley (hopem) wrote :

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

170. By Xiang Hui

Add ut and solved ut errors and conflicts.

Revision history for this message
Xiang Hui (xianghui) wrote :

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

Revision history for this message
James Page (james-page) :
Revision history for this message
Edward Hope-Morley (hopem) :
171. By Xiang Hui

MV get_ipv6_addr() to network/ip.py

172. By Xiang Hui

Merge from dev branch and fix unit test.

173. By Xiang Hui

Remove relation set 'hostname' on AMQP.

174. By Xiang Hui

If prefer ipv6 addr only, fallback to ipv6.

175. By Xiang Hui

Support haproxy for ipv6.

Revision history for this message
Edward Hope-Morley (hopem) wrote :

Couple of issues that need fixing.

review: Needs Fixing
176. By Xiang Hui

Fix trivial issue.

Revision history for this message
Xiang Hui (xianghui) wrote :

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

Revision history for this message
Edward Hope-Morley (hopem) wrote :

LGTM +1

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== 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 @@
44
5from charmhelpers.fetch import apt_install5from charmhelpers.fetch import apt_install
6from charmhelpers.core.hookenv import (6from charmhelpers.core.hookenv import (
7 ERROR, log,7 ERROR, log, config,
8)8)
99
10try:10try:
@@ -154,3 +154,21 @@
154get_iface_for_address = partial(_get_for_address, key='iface')154get_iface_for_address = partial(_get_for_address, key='iface')
155155
156get_netmask_for_address = partial(_get_for_address, key='netmask')156get_netmask_for_address = partial(_get_for_address, key='netmask')
157
158
159def get_ipv6_addr(iface="eth0"):
160 try:
161 iface_addrs = netifaces.ifaddresses(iface)
162 if netifaces.AF_INET6 not in iface_addrs:
163 raise Exception("Interface '%s' doesn't have an ipv6 address." % iface)
164
165 addresses = netifaces.ifaddresses(iface)[netifaces.AF_INET6]
166 ipv6_addr = [a['addr'] for a in addresses if not a['addr'].startswith('fe80')
167 and config('vip') != a['addr']]
168 if not ipv6_addr:
169 raise Exception("Interface '%s' doesn't have global ipv6 address." % iface)
170
171 return ipv6_addr[0]
172
173 except ValueError:
174 raise ValueError("Invalid interface '%s'" % iface)
157175
=== 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 @@
44 neutron_plugin_attribute,44 neutron_plugin_attribute,
45)45)
4646
47from charmhelpers.contrib.network.ip import get_address_in_network47from charmhelpers.contrib.network.ip import (
48 get_address_in_network,
49 get_ipv6_addr,
50)
4851
49CA_CERT_PATH = '/usr/local/share/ca-certificates/keystone_juju_ca_cert.crt'52CA_CERT_PATH = '/usr/local/share/ca-certificates/keystone_juju_ca_cert.crt'
5053
@@ -401,9 +404,12 @@
401404
402 cluster_hosts = {}405 cluster_hosts = {}
403 l_unit = local_unit().replace('/', '-')406 l_unit = local_unit().replace('/', '-')
404 cluster_hosts[l_unit] = \407 if config('prefer-ipv6'):
405 get_address_in_network(config('os-internal-network'),408 addr = get_ipv6_addr()
406 unit_get('private-address'))409 else:
410 addr = unit_get('private-address')
411 cluster_hosts[l_unit] = get_address_in_network(config('os-internal-network'),
412 addr)
407413
408 for rid in relation_ids('cluster'):414 for rid in relation_ids('cluster'):
409 for unit in related_units(rid):415 for unit in related_units(rid):
@@ -414,6 +420,16 @@
414 ctxt = {420 ctxt = {
415 'units': cluster_hosts,421 'units': cluster_hosts,
416 }422 }
423
424 if config('prefer-ipv6'):
425 ctxt['local_host'] = 'ip6-localhost'
426 ctxt['haproxy_host'] = '::'
427 ctxt['stat_port'] = ':::8888'
428 else:
429 ctxt['local_host'] = '127.0.0.1'
430 ctxt['haproxy_host'] = '0.0.0.0'
431 ctxt['stat_port'] = ':8888'
432
417 if len(cluster_hosts.keys()) > 1:433 if len(cluster_hosts.keys()) > 1:
418 # Enable haproxy when we have enough peers.434 # Enable haproxy when we have enough peers.
419 log('Ensuring haproxy enabled in /etc/default/haproxy.')435 log('Ensuring haproxy enabled in /etc/default/haproxy.')
420436
=== 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 @@
7 get_address_in_network,7 get_address_in_network,
8 is_address_in_network,8 is_address_in_network,
9 is_ipv6,9 is_ipv6,
10 get_ipv6_addr,
10)11)
1112
12from charmhelpers.contrib.hahelpers.cluster import is_clustered13from charmhelpers.contrib.hahelpers.cluster import is_clustered
@@ -64,10 +65,13 @@
64 vip):65 vip):
65 resolved_address = vip66 resolved_address = vip
66 else:67 else:
68 if config('prefer-ipv6'):
69 fallback_addr = get_ipv6_addr()
70 else:
71 fallback_addr = unit_get(_address_map[endpoint_type]['fallback'])
67 resolved_address = get_address_in_network(72 resolved_address = get_address_in_network(
68 config(_address_map[endpoint_type]['config']),73 config(_address_map[endpoint_type]['config']), fallback_addr)
69 unit_get(_address_map[endpoint_type]['fallback'])74
70 )
71 if resolved_address is None:75 if resolved_address is None:
72 raise ValueError('Unable to resolve a suitable IP address'76 raise ValueError('Unable to resolve a suitable IP address'
73 ' based on charm state and configuration')77 ' based on charm state and configuration')
7478
=== 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 @@
1global1global
2 log 127.0.0.1 local02 log {{ local_host }} local0
3 log 127.0.0.1 local1 notice3 log {{ local_host }} local1 notice
4 maxconn 200004 maxconn 20000
5 user haproxy5 user haproxy
6 group haproxy6 group haproxy
@@ -17,7 +17,7 @@
17 timeout client 3000017 timeout client 30000
18 timeout server 3000018 timeout server 30000
1919
20listen stats :888820listen stats {{ stat_port }}
21 mode http21 mode http
22 stats enable22 stats enable
23 stats hide-version23 stats hide-version
2424
=== 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 @@
158 def test_is_ipv6(self):158 def test_is_ipv6(self):
159 self.assertFalse(net_ip.is_ipv6('myhost'))159 self.assertFalse(net_ip.is_ipv6('myhost'))
160 self.assertFalse(net_ip.is_ipv6('172.4.5.5'))160 self.assertFalse(net_ip.is_ipv6('172.4.5.5'))
161 self.assertTrue(net_ip.is_ipv6('2a01:348:2f4:0:685e:5748:ae62:209f'))
162\ No newline at end of file161\ No newline at end of file
162 self.assertTrue(net_ip.is_ipv6('2a01:348:2f4:0:685e:5748:ae62:209f'))
163
164 @patch.object(netifaces, 'ifaddresses')
165 def test_get_ipv6_addr_no_ipv6(self, _ifaddresses):
166 DUMMY_ADDRESSES = {
167 'eth0': {
168 2: [{'addr': '192.168.1.55',
169 'broadcast': '192.168.1.255',
170 'netmask': '255.255.255.0'}]
171 }
172 }
173
174 def mock_ifaddresses(iface):
175 return DUMMY_ADDRESSES[iface]
176
177 _ifaddresses.side_effect = mock_ifaddresses
178 self.assertRaises(Exception, net_ip.get_ipv6_addr)
179
180 @patch.object(netifaces, 'ifaddresses')
181 def test_get_ipv6_addr_no_global_ipv6(self, _ifaddresses):
182 DUMMY_ADDRESSES = {
183 'eth0': {
184 10: [{'addr': 'fe80::3e97:eff:fe8b:1cf7%eth0',
185 'netmask': 'ffff:ffff:ffff:ffff::'}],
186 }
187 }
188
189 def mock_ifaddresses(iface):
190 return DUMMY_ADDRESSES[iface]
191
192 _ifaddresses.side_effect = mock_ifaddresses
193 self.assertRaises(Exception, net_ip.get_ipv6_addr)
194
195 @patch.object(netifaces, 'ifaddresses')
196 def test_get_ipv6_addr_invalid_nic(self, _ifaddresses):
197 DUMMY_ADDRESSES = {
198 'eth0': {
199 10: [{'addr': 'fe80::3e97:eff:fe8b:1cf7%eth0',
200 'netmask': 'ffff:ffff:ffff:ffff::'}],
201 }
202 }
203
204 def mock_ifaddresses(iface):
205 return DUMMY_ADDRESSES[iface]
206
207 _ifaddresses.side_effect = ValueError()
208 self.assertRaises(ValueError, net_ip.get_ipv6_addr, 'eth1')
163209
=== 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 @@
315 'time',315 'time',
316 'https',316 'https',
317 'get_address_in_network',317 'get_address_in_network',
318 'local_unit'318 'local_unit',
319 'get_ipv6_addr'
319]320]
320321
321322
@@ -835,6 +836,52 @@
835 self.relation_get.side_effect = relation.get836 self.relation_get.side_effect = relation.get
836 self.related_units.side_effect = relation.relation_units837 self.related_units.side_effect = relation.relation_units
837 self.get_address_in_network.return_value = 'cluster-peer0.localnet'838 self.get_address_in_network.return_value = 'cluster-peer0.localnet'
839 self.config.return_value = False
840 haproxy = context.HAProxyContext()
841 with patch_open() as (_open, _file):
842 result = haproxy()
843 ex = {
844 'units': {
845 'peer-0': 'cluster-peer0.localnet',
846 'peer-1': 'cluster-peer1.localnet',
847 'peer-2': 'cluster-peer2.localnet',
848 },
849
850 'local_host': '127.0.0.1',
851 'haproxy_host': '0.0.0.0',
852 'stat_port': ':8888',
853 }
854 # the context gets generated.
855 self.assertEquals(ex, result)
856 # and /etc/default/haproxy is updated.
857 self.assertEquals(_file.write.call_args_list,
858 [call('ENABLED=1\n')])
859
860 @patch('charmhelpers.contrib.openstack.context.unit_get')
861 @patch('charmhelpers.contrib.openstack.context.local_unit')
862 @patch('charmhelpers.contrib.openstack.context.get_ipv6_addr')
863 def test_haproxy_context_with_data_ipv6(
864 self, local_unit, unit_get, get_ipv6_addr):
865 '''Test haproxy context with all relation data'''
866 cluster_relation = {
867 'cluster:0': {
868 'peer/1': {
869 'private-address': 'cluster-peer1.localnet',
870 },
871 'peer/2': {
872 'private-address': 'cluster-peer2.localnet',
873 },
874 },
875 }
876
877 unit_get.return_value = 'peer/0'
878 relation = FakeRelation(cluster_relation)
879 self.relation_ids.side_effect = relation.relation_ids
880 self.relation_get.side_effect = relation.get
881 self.related_units.side_effect = relation.relation_units
882 self.get_address_in_network.return_value = 'cluster-peer0.localnet'
883 self.get_ipv6_addr.return_value = 'cluster-peer0.localnet'
884 self.config.side_effect = [True, None, True]
838 haproxy = context.HAProxyContext()885 haproxy = context.HAProxyContext()
839 with patch_open() as (_open, _file):886 with patch_open() as (_open, _file):
840 result = haproxy()887 result = haproxy()
@@ -843,7 +890,11 @@
843 'peer-0': 'cluster-peer0.localnet',890 'peer-0': 'cluster-peer0.localnet',
844 'peer-1': 'cluster-peer1.localnet',891 'peer-1': 'cluster-peer1.localnet',
845 'peer-2': 'cluster-peer2.localnet'892 'peer-2': 'cluster-peer2.localnet'
846 }893 },
894
895 'local_host': 'ip6-localhost',
896 'haproxy_host': '::',
897 'stat_port': ':::8888',
847 }898 }
848 # the context gets generated.899 # the context gets generated.
849 self.assertEquals(ex, result)900 self.assertEquals(ex, result)
@@ -876,6 +927,7 @@
876 self.relation_ids.side_effect = relation.relation_ids927 self.relation_ids.side_effect = relation.relation_ids
877 self.relation_get.side_effect = relation.get928 self.relation_get.side_effect = relation.get
878 self.related_units.side_effect = relation.relation_units929 self.related_units.side_effect = relation.relation_units
930 self.config.return_value = False
879 haproxy = context.HAProxyContext()931 haproxy = context.HAProxyContext()
880 self.assertEquals({}, haproxy())932 self.assertEquals({}, haproxy())
881933

Subscribers

People subscribed via source and target branches