Charm Helpers

Merge lp:~freyes/charm-helpers/lp1642430 into lp:charm-helpers

  1. lp1642430
  2. Merge into devel
Proposed by Felipe Reyes
Status: Merged
Merged at revision: 703
Proposed branch: lp:~freyes/charm-helpers/lp1642430
Merge into: lp:charm-helpers
Diff against target: 303 lines (+211/-14)
2 files modified
charmhelpers/contrib/openstack/context.py (+9/-6)
tests/contrib/openstack/test_os_contexts.py (+202/-8)
To merge this branch: bzr merge lp:~freyes/charm-helpers/lp1642430
Reviewer Review Type Date Requested Status
Jorge Niedbalski (community) Approve
Chris MacNaughton (community) Approve
Review via email: mp+317557@code.launchpad.net

Description of the change

Allow ceph-public-address to contain a list of IPs

Ceph relation allows units to hand off an IP different from the
public-address or private-address using the ceph-public-address key, that
key can hold a single IP address, this patch extends CephContext to be
able to handle a list of space separated addresses.

This is needed to let ceph-proxy hand off the IPs of the monitor hosts from
an external Ceph cluster (LP: #1642430)

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

The current code doesn't handle ports, just IPs, in ceph-proxy is possible to use a list of IP:PORT , this needs to be extended to put the port as well.

Revision history for this message
Chris MacNaughton (chris.macnaughton) wrote :

Your description keeps saying that it's comma separated addresses but the code says space separated.

Revision history for this message
Chris MacNaughton (chris.macnaughton) wrote :

Otherwise, looks good to me

review: Approve
Revision history for this message
Felipe Reyes (freyes) wrote :

> Your description keeps saying that it's comma separated addresses but the code
> says space separated.

Right, amended it, thanks.

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

LGTM.

Thanks

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'charmhelpers/contrib/openstack/context.py'
2--- charmhelpers/contrib/openstack/context.py 2017-02-16 18:56:32 +0000
3+++ charmhelpers/contrib/openstack/context.py 2017-02-17 15:23:37 +0000
4@@ -502,13 +502,16 @@
5 ctxt['auth'] = relation_get('auth', rid=rid, unit=unit)
6 if not ctxt.get('key'):
7 ctxt['key'] = relation_get('key', rid=rid, unit=unit)
8- ceph_pub_addr = relation_get('ceph-public-address', rid=rid,
9+
10+ ceph_addrs = relation_get('ceph-public-address', rid=rid,
11+ unit=unit)
12+ if ceph_addrs:
13+ for addr in ceph_addrs.split(' '):
14+ mon_hosts.append(format_ipv6_addr(addr) or addr)
15+ else:
16+ priv_addr = relation_get('private-address', rid=rid,
17 unit=unit)
18- unit_priv_addr = relation_get('private-address', rid=rid,
19- unit=unit)
20- ceph_addr = ceph_pub_addr or unit_priv_addr
21- ceph_addr = format_ipv6_addr(ceph_addr) or ceph_addr
22- mon_hosts.append(ceph_addr)
23+ mon_hosts.append(format_ipv6_addr(priv_addr) or priv_addr)
24
25 ctxt['mon_hosts'] = ' '.join(sorted(mon_hosts))
26
27
28=== modified file 'tests/contrib/openstack/test_os_contexts.py'
29--- tests/contrib/openstack/test_os_contexts.py 2017-02-14 12:52:41 +0000
30+++ tests/contrib/openstack/test_os_contexts.py 2017-02-17 15:23:37 +0000
31@@ -275,6 +275,74 @@
32 }
33 }
34
35+CEPH_REL_WITH_PUBLIC_ADDR_PORT = {
36+ 'ceph:0': {
37+ 'ceph/0': {
38+ 'ceph-public-address': '192.168.1.10:1234',
39+ 'private-address': 'ceph_node1',
40+ 'auth': 'foo',
41+ 'key': 'bar',
42+ },
43+ 'ceph/1': {
44+ 'ceph-public-address': '192.168.1.11:4321',
45+ 'private-address': 'ceph_node2',
46+ 'auth': 'foo',
47+ 'key': 'bar',
48+ },
49+ }
50+}
51+
52+CEPH_REL_WITH_PUBLIC_IPv6_ADDR = {
53+ 'ceph:0': {
54+ 'ceph/0': {
55+ 'ceph-public-address': '2001:5c0:9168::1',
56+ 'private-address': 'ceph_node1',
57+ 'auth': 'foo',
58+ 'key': 'bar',
59+ },
60+ 'ceph/1': {
61+ 'ceph-public-address': '2001:5c0:9168::2',
62+ 'private-address': 'ceph_node2',
63+ 'auth': 'foo',
64+ 'key': 'bar',
65+ },
66+ }
67+}
68+
69+CEPH_REL_WITH_PUBLIC_IPv6_ADDR_PORT = {
70+ 'ceph:0': {
71+ 'ceph/0': {
72+ 'ceph-public-address': '[2001:5c0:9168::1]:1234',
73+ 'private-address': 'ceph_node1',
74+ 'auth': 'foo',
75+ 'key': 'bar',
76+ },
77+ 'ceph/1': {
78+ 'ceph-public-address': '[2001:5c0:9168::2]:4321',
79+ 'private-address': 'ceph_node2',
80+ 'auth': 'foo',
81+ 'key': 'bar',
82+ },
83+ }
84+}
85+
86+CEPH_REL_WITH_MULTI_PUBLIC_ADDR = {
87+ 'ceph:0': {
88+ 'ceph/0': {
89+ 'ceph-public-address': '192.168.1.10 192.168.1.20',
90+ 'private-address': 'ceph_node1',
91+ 'auth': 'foo',
92+ 'key': 'bar',
93+ },
94+ 'ceph/1': {
95+ 'ceph-public-address': '192.168.1.11 192.168.1.21',
96+ 'private-address': 'ceph_node2',
97+ 'auth': 'foo',
98+ 'key': 'bar',
99+ },
100+ }
101+}
102+
103 IDENTITY_RELATION_NO_CERT = {
104 'identity-service:0': {
105 'keystone/0': {
106@@ -513,7 +581,6 @@
107 'get_netmask_for_address',
108 'local_unit',
109 'get_ipv6_addr',
110- 'format_ipv6_addr',
111 'mkdir',
112 'write_file',
113 'get_host_ip',
114@@ -570,7 +637,6 @@
115 self.relation_ids.return_value = ['foo:0']
116 self.related_units.return_value = ['foo/0']
117 self.local_unit.return_value = 'localunit'
118- self.format_ipv6_addr.return_value = None
119 self.get_host_ip.side_effect = lambda hostname: hostname
120 self.kv.side_effect = TestDB
121 self.pwgen.return_value = 'testpassword'
122@@ -704,13 +770,14 @@
123 'database_host': 'bar',
124 'database_type': 'mysql'})
125
126- def test_shared_db_context_with_ipv6(self):
127+ @patch('charmhelpers.contrib.openstack.context.format_ipv6_addr')
128+ def test_shared_db_context_with_ipv6(self, format_ipv6_addr):
129 '''Test shared-db context with ipv6'''
130 shared_db = context.SharedDBContext(
131 database='quantum', user='quantum', relation_prefix='quantum')
132 relation = FakeRelation(relation_data=SHARED_DB_RELATION_NAMESPACED)
133 self.relation_get.side_effect = relation.get
134- self.format_ipv6_addr.return_value = '[2001:db8:1::1]'
135+ format_ipv6_addr.return_value = '[2001:db8:1::1]'
136 result = shared_db()
137 self.assertIn(
138 call(rid='foo:0', unit='foo/0'),
139@@ -903,11 +970,12 @@
140 }
141 self.assertEquals(result, expected)
142
143- def test_identity_service_context_with_ipv6(self):
144+ @patch('charmhelpers.contrib.openstack.context.format_ipv6_addr')
145+ def test_identity_service_context_with_ipv6(self, format_ipv6_addr):
146 '''Test identity-service context with ipv6'''
147 relation = FakeRelation(relation_data=IDENTITY_SERVICE_RELATION_HTTP)
148 self.relation_get.side_effect = relation.get
149- self.format_ipv6_addr.return_value = '[2001:db8:1::1]'
150+ format_ipv6_addr.return_value = '[2001:db8:1::1]'
151 identity_service = context.IdentityServiceContext()
152 result = identity_service()
153 expected = {
154@@ -1072,14 +1140,15 @@
155 amqp = context.AMQPContext()
156 self.assertRaises(context.OSContextError, amqp)
157
158- def test_amqp_context_with_ipv6(self):
159+ @patch('charmhelpers.contrib.openstack.context.format_ipv6_addr')
160+ def test_amqp_context_with_ipv6(self, format_ipv6_addr):
161 '''Test amqp context with ipv6'''
162 relation_data = copy(AMQP_AA_RELATION)
163 relation = FakeRelation(relation_data=relation_data)
164 self.relation_get.side_effect = relation.get
165 self.relation_ids.side_effect = relation.relation_ids
166 self.related_units.side_effect = relation.relation_units
167- self.format_ipv6_addr.return_value = '[2001:db8:1::1]'
168+ format_ipv6_addr.return_value = '[2001:db8:1::1]'
169 self.config.return_value = AMQP_CONFIG
170 amqp = context.AMQPContext()
171 result = amqp()
172@@ -1251,6 +1320,131 @@
173 @patch('os.path.isdir')
174 @patch('os.mkdir')
175 @patch.object(context, 'ensure_packages')
176+ def test_ceph_context_with_public_addr_and_port(
177+ self, ensure_packages, mkdir, isdir, mock_config):
178+ '''Test ceph context in host with multiple networks with all
179+ relation data'''
180+ isdir.return_value = False
181+ config_dict = {'use-syslog': True}
182+
183+ def fake_config(key):
184+ return config_dict.get(key)
185+
186+ mock_config.side_effect = fake_config
187+ relation = FakeRelation(relation_data=CEPH_REL_WITH_PUBLIC_ADDR_PORT)
188+ self.relation_get.side_effect = relation.get
189+ self.relation_ids.side_effect = relation.relation_ids
190+ self.related_units.side_effect = relation.relation_units
191+ ceph = context.CephContext()
192+ result = ceph()
193+ expected = {
194+ 'mon_hosts': '192.168.1.10:1234 192.168.1.11:4321',
195+ 'auth': 'foo',
196+ 'key': 'bar',
197+ 'use_syslog': 'true',
198+ }
199+ self.assertEquals(result, expected)
200+ ensure_packages.assert_called_with(['ceph-common'])
201+ mkdir.assert_called_with('/etc/ceph')
202+
203+ @patch.object(context, 'config')
204+ @patch('os.path.isdir')
205+ @patch('os.mkdir')
206+ @patch.object(context, 'ensure_packages')
207+ def test_ceph_context_with_public_ipv6_addr(self, ensure_packages, mkdir,
208+ isdir, mock_config):
209+ '''Test ceph context in host with multiple networks with all
210+ relation data'''
211+ isdir.return_value = False
212+ config_dict = {'use-syslog': True}
213+
214+ def fake_config(key):
215+ return config_dict.get(key)
216+
217+ mock_config.side_effect = fake_config
218+ relation = FakeRelation(relation_data=CEPH_REL_WITH_PUBLIC_IPv6_ADDR)
219+ self.relation_get.side_effect = relation.get
220+ self.relation_ids.side_effect = relation.relation_ids
221+ self.related_units.side_effect = relation.relation_units
222+ ceph = context.CephContext()
223+ result = ceph()
224+ expected = {
225+ 'mon_hosts': '[2001:5c0:9168::1] [2001:5c0:9168::2]',
226+ 'auth': 'foo',
227+ 'key': 'bar',
228+ 'use_syslog': 'true',
229+ }
230+ self.assertEquals(result, expected)
231+ ensure_packages.assert_called_with(['ceph-common'])
232+ mkdir.assert_called_with('/etc/ceph')
233+
234+ @patch.object(context, 'config')
235+ @patch('os.path.isdir')
236+ @patch('os.mkdir')
237+ @patch.object(context, 'ensure_packages')
238+ def test_ceph_context_with_public_ipv6_addr_port(
239+ self, ensure_packages, mkdir, isdir, mock_config):
240+ '''Test ceph context in host with multiple networks with all
241+ relation data'''
242+ isdir.return_value = False
243+ config_dict = {'use-syslog': True}
244+
245+ def fake_config(key):
246+ return config_dict.get(key)
247+
248+ mock_config.side_effect = fake_config
249+ relation = FakeRelation(
250+ relation_data=CEPH_REL_WITH_PUBLIC_IPv6_ADDR_PORT)
251+ self.relation_get.side_effect = relation.get
252+ self.relation_ids.side_effect = relation.relation_ids
253+ self.related_units.side_effect = relation.relation_units
254+ ceph = context.CephContext()
255+ result = ceph()
256+ expected = {
257+ 'mon_hosts': '[2001:5c0:9168::1]:1234 [2001:5c0:9168::2]:4321',
258+ 'auth': 'foo',
259+ 'key': 'bar',
260+ 'use_syslog': 'true',
261+ }
262+ self.assertEquals(result, expected)
263+ ensure_packages.assert_called_with(['ceph-common'])
264+ mkdir.assert_called_with('/etc/ceph')
265+
266+ @patch.object(context, 'config')
267+ @patch('os.path.isdir')
268+ @patch('os.mkdir')
269+ @patch.object(context, 'ensure_packages')
270+ def test_ceph_context_with_multi_public_addr(
271+ self, ensure_packages, mkdir, isdir, mock_config):
272+ '''Test ceph context in host with multiple networks with all
273+ relation data'''
274+ isdir.return_value = False
275+ config_dict = {'use-syslog': True}
276+
277+ def fake_config(key):
278+ return config_dict.get(key)
279+
280+ mock_config.side_effect = fake_config
281+ relation = FakeRelation(relation_data=CEPH_REL_WITH_MULTI_PUBLIC_ADDR)
282+ self.relation_get.side_effect = relation.get
283+ self.relation_ids.side_effect = relation.relation_ids
284+ self.related_units.side_effect = relation.relation_units
285+ ceph = context.CephContext()
286+ result = ceph()
287+ expected = {
288+ 'mon_hosts': '192.168.1.10 192.168.1.11 192.168.1.20 192.168.1.21',
289+ 'auth': 'foo',
290+ 'key': 'bar',
291+ 'use_syslog': 'true',
292+ }
293+ self.assertEquals(result, expected)
294+ ensure_packages.assert_called_with(['ceph-common'])
295+ mkdir.assert_called_with('/etc/ceph')
296+
297+ @patch.object(context, 'config')
298+ @patch('os.path.isdir')
299+ @patch('os.mkdir')
300+ @patch.object(context, 'ensure_packages')
301 def test_ceph_context_with_rbd_cache(self, ensure_packages, mkdir, isdir,
302 mock_config):
303 isdir.return_value = False

Subscribers

People subscribed via source and target branches