Merge lp:~billy-olsen/charms/trusty/ceph-radosgw/backport-lp1398182 into lp:~openstack-charmers-archive/charms/trusty/ceph-radosgw/trunk
- Trusty Tahr (14.04)
- backport-lp1398182
- Merge into trunk
Proposed by
Billy Olsen
Status: | Merged |
---|---|
Merged at revision: | 40 |
Proposed branch: | lp:~billy-olsen/charms/trusty/ceph-radosgw/backport-lp1398182 |
Merge into: | lp:~openstack-charmers-archive/charms/trusty/ceph-radosgw/trunk |
Diff against target: |
308 lines (+106/-66) 4 files modified
config.yaml (+12/-0) hooks/charmhelpers/contrib/openstack/ip.py (+49/-44) hooks/hooks.py (+4/-15) unit_tests/test_hooks.py (+41/-7) |
To merge this branch: | bzr merge lp:~billy-olsen/charms/trusty/ceph-radosgw/backport-lp1398182 |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Corey Bryant (community) | Approve | ||
Review via email: mp+262000@code.launchpad.net |
Commit message
Description of the change
To post a comment you must log in.
Revision history for this message
uosci-testing-bot (uosci-testing-bot) wrote : | # |
Revision history for this message
uosci-testing-bot (uosci-testing-bot) wrote : | # |
charm_unit_test #5035 ceph-radosgw for billy-olsen mp262000
UNIT OK: passed
Revision history for this message
uosci-testing-bot (uosci-testing-bot) wrote : | # |
charm_amulet_test #4646 ceph-radosgw for billy-olsen mp262000
AMULET OK: passed
Build: http://
Revision history for this message
Corey Bryant (corey.bryant) : | # |
review:
Approve
Preview Diff
[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1 | === modified file 'config.yaml' | |||
2 | --- config.yaml 2015-01-14 13:27:02 +0000 | |||
3 | +++ config.yaml 2015-06-15 18:43:37 +0000 | |||
4 | @@ -87,3 +87,15 @@ | |||
5 | 87 | description: | | 87 | description: | |
6 | 88 | Default multicast port number that will be used to communicate between | 88 | Default multicast port number that will be used to communicate between |
7 | 89 | HA Cluster nodes. | 89 | HA Cluster nodes. |
8 | 90 | os-public-hostname: | ||
9 | 91 | type: string | ||
10 | 92 | default: | ||
11 | 93 | description: | | ||
12 | 94 | The hostname or address of the public endpoints created for ceph-radosgw | ||
13 | 95 | in the keystone identity provider. | ||
14 | 96 | . | ||
15 | 97 | This value will be used for public endpoints. For example, an | ||
16 | 98 | os-public-hostname set to 'files.example.com' with will create | ||
17 | 99 | the following public endpoint for the ceph-radosgw: | ||
18 | 100 | . | ||
19 | 101 | https://files.example.com:80/swift/v1 | ||
20 | 90 | 102 | ||
21 | === modified file 'hooks/charmhelpers/contrib/openstack/ip.py' | |||
22 | --- hooks/charmhelpers/contrib/openstack/ip.py 2015-02-24 11:02:02 +0000 | |||
23 | +++ hooks/charmhelpers/contrib/openstack/ip.py 2015-06-15 18:43:37 +0000 | |||
24 | @@ -17,6 +17,7 @@ | |||
25 | 17 | from charmhelpers.core.hookenv import ( | 17 | from charmhelpers.core.hookenv import ( |
26 | 18 | config, | 18 | config, |
27 | 19 | unit_get, | 19 | unit_get, |
28 | 20 | service_name, | ||
29 | 20 | ) | 21 | ) |
30 | 21 | from charmhelpers.contrib.network.ip import ( | 22 | from charmhelpers.contrib.network.ip import ( |
31 | 22 | get_address_in_network, | 23 | get_address_in_network, |
32 | @@ -26,8 +27,6 @@ | |||
33 | 26 | ) | 27 | ) |
34 | 27 | from charmhelpers.contrib.hahelpers.cluster import is_clustered | 28 | from charmhelpers.contrib.hahelpers.cluster import is_clustered |
35 | 28 | 29 | ||
36 | 29 | from functools import partial | ||
37 | 30 | |||
38 | 31 | PUBLIC = 'public' | 30 | PUBLIC = 'public' |
39 | 32 | INTERNAL = 'int' | 31 | INTERNAL = 'int' |
40 | 33 | ADMIN = 'admin' | 32 | ADMIN = 'admin' |
41 | @@ -35,15 +34,18 @@ | |||
42 | 35 | ADDRESS_MAP = { | 34 | ADDRESS_MAP = { |
43 | 36 | PUBLIC: { | 35 | PUBLIC: { |
44 | 37 | 'config': 'os-public-network', | 36 | 'config': 'os-public-network', |
46 | 38 | 'fallback': 'public-address' | 37 | 'fallback': 'public-address', |
47 | 38 | 'override': 'os-public-hostname', | ||
48 | 39 | }, | 39 | }, |
49 | 40 | INTERNAL: { | 40 | INTERNAL: { |
50 | 41 | 'config': 'os-internal-network', | 41 | 'config': 'os-internal-network', |
52 | 42 | 'fallback': 'private-address' | 42 | 'fallback': 'private-address', |
53 | 43 | 'override': 'os-internal-hostname', | ||
54 | 43 | }, | 44 | }, |
55 | 44 | ADMIN: { | 45 | ADMIN: { |
56 | 45 | 'config': 'os-admin-network', | 46 | 'config': 'os-admin-network', |
58 | 46 | 'fallback': 'private-address' | 47 | 'fallback': 'private-address', |
59 | 48 | 'override': 'os-admin-hostname', | ||
60 | 47 | } | 49 | } |
61 | 48 | } | 50 | } |
62 | 49 | 51 | ||
63 | @@ -57,15 +59,50 @@ | |||
64 | 57 | :param endpoint_type: str endpoint type to resolve. | 59 | :param endpoint_type: str endpoint type to resolve. |
65 | 58 | :param returns: str base URL for services on the current service unit. | 60 | :param returns: str base URL for services on the current service unit. |
66 | 59 | """ | 61 | """ |
70 | 60 | scheme = 'http' | 62 | scheme = _get_scheme(configs) |
71 | 61 | if 'https' in configs.complete_contexts(): | 63 | |
69 | 62 | scheme = 'https' | ||
72 | 63 | address = resolve_address(endpoint_type) | 64 | address = resolve_address(endpoint_type) |
73 | 64 | if is_ipv6(address): | 65 | if is_ipv6(address): |
74 | 65 | address = "[{}]".format(address) | 66 | address = "[{}]".format(address) |
75 | 67 | |||
76 | 66 | return '%s://%s' % (scheme, address) | 68 | return '%s://%s' % (scheme, address) |
77 | 67 | 69 | ||
78 | 68 | 70 | ||
79 | 71 | def _get_scheme(configs): | ||
80 | 72 | """Returns the scheme to use for the url (either http or https) | ||
81 | 73 | depending upon whether https is in the configs value. | ||
82 | 74 | |||
83 | 75 | :param configs: OSTemplateRenderer config templating object to inspect | ||
84 | 76 | for a complete https context. | ||
85 | 77 | :returns: either 'http' or 'https' depending on whether https is | ||
86 | 78 | configured within the configs context. | ||
87 | 79 | """ | ||
88 | 80 | scheme = 'http' | ||
89 | 81 | if configs and 'https' in configs.complete_contexts(): | ||
90 | 82 | scheme = 'https' | ||
91 | 83 | return scheme | ||
92 | 84 | |||
93 | 85 | |||
94 | 86 | def _get_address_override(endpoint_type=PUBLIC): | ||
95 | 87 | """Returns any address overrides that the user has defined based on the | ||
96 | 88 | endpoint type. | ||
97 | 89 | |||
98 | 90 | Note: this function allows for the service name to be inserted into the | ||
99 | 91 | address if the user specifies {service_name}.somehost.org. | ||
100 | 92 | |||
101 | 93 | :param endpoint_type: the type of endpoint to retrieve the override | ||
102 | 94 | value for. | ||
103 | 95 | :returns: any endpoint address or hostname that the user has overridden | ||
104 | 96 | or None if an override is not present. | ||
105 | 97 | """ | ||
106 | 98 | override_key = ADDRESS_MAP[endpoint_type]['override'] | ||
107 | 99 | addr_override = config(override_key) | ||
108 | 100 | if not addr_override: | ||
109 | 101 | return None | ||
110 | 102 | else: | ||
111 | 103 | return addr_override.format(service_name=service_name()) | ||
112 | 104 | |||
113 | 105 | |||
114 | 69 | def resolve_address(endpoint_type=PUBLIC): | 106 | def resolve_address(endpoint_type=PUBLIC): |
115 | 70 | """Return unit address depending on net config. | 107 | """Return unit address depending on net config. |
116 | 71 | 108 | ||
117 | @@ -77,7 +114,10 @@ | |||
118 | 77 | 114 | ||
119 | 78 | :param endpoint_type: Network endpoing type | 115 | :param endpoint_type: Network endpoing type |
120 | 79 | """ | 116 | """ |
122 | 80 | resolved_address = None | 117 | resolved_address = _get_address_override(endpoint_type) |
123 | 118 | if resolved_address: | ||
124 | 119 | return resolved_address | ||
125 | 120 | |||
126 | 81 | vips = config('vip') | 121 | vips = config('vip') |
127 | 82 | if vips: | 122 | if vips: |
128 | 83 | vips = vips.split() | 123 | vips = vips.split() |
129 | @@ -109,38 +149,3 @@ | |||
130 | 109 | "clustered=%s)" % (net_type, clustered)) | 149 | "clustered=%s)" % (net_type, clustered)) |
131 | 110 | 150 | ||
132 | 111 | return resolved_address | 151 | return resolved_address |
133 | 112 | |||
134 | 113 | |||
135 | 114 | def endpoint_url(configs, url_template, port, endpoint_type=PUBLIC, | ||
136 | 115 | override=None): | ||
137 | 116 | """Returns the correct endpoint URL to advertise to Keystone. | ||
138 | 117 | |||
139 | 118 | This method provides the correct endpoint URL which should be advertised to | ||
140 | 119 | the keystone charm for endpoint creation. This method allows for the url to | ||
141 | 120 | be overridden to force a keystone endpoint to have specific URL for any of | ||
142 | 121 | the defined scopes (admin, internal, public). | ||
143 | 122 | |||
144 | 123 | :param configs: OSTemplateRenderer config templating object to inspect | ||
145 | 124 | for a complete https context. | ||
146 | 125 | :param url_template: str format string for creating the url template. Only | ||
147 | 126 | two values will be passed - the scheme+hostname | ||
148 | 127 | returned by the canonical_url and the port. | ||
149 | 128 | :param endpoint_type: str endpoint type to resolve. | ||
150 | 129 | :param override: str the name of the config option which overrides the | ||
151 | 130 | endpoint URL defined by the charm itself. None will | ||
152 | 131 | disable any overrides (default). | ||
153 | 132 | """ | ||
154 | 133 | if override: | ||
155 | 134 | # Return any user-defined overrides for the keystone endpoint URL. | ||
156 | 135 | user_value = config(override) | ||
157 | 136 | if user_value: | ||
158 | 137 | return user_value.strip() | ||
159 | 138 | |||
160 | 139 | return url_template % (canonical_url(configs, endpoint_type), port) | ||
161 | 140 | |||
162 | 141 | |||
163 | 142 | public_endpoint = partial(endpoint_url, endpoint_type=PUBLIC) | ||
164 | 143 | |||
165 | 144 | internal_endpoint = partial(endpoint_url, endpoint_type=INTERNAL) | ||
166 | 145 | |||
167 | 146 | admin_endpoint = partial(endpoint_url, endpoint_type=ADMIN) | ||
168 | 147 | 152 | ||
169 | === modified file 'hooks/hooks.py' | |||
170 | --- hooks/hooks.py 2015-04-28 21:18:24 +0000 | |||
171 | +++ hooks/hooks.py 2015-06-15 18:43:37 +0000 | |||
172 | @@ -50,10 +50,9 @@ | |||
173 | 50 | from charmhelpers.contrib.network.ip import ( | 50 | from charmhelpers.contrib.network.ip import ( |
174 | 51 | get_iface_for_address, | 51 | get_iface_for_address, |
175 | 52 | get_netmask_for_address, | 52 | get_netmask_for_address, |
176 | 53 | is_ipv6, | ||
177 | 54 | ) | 53 | ) |
178 | 55 | from charmhelpers.contrib.openstack.ip import ( | 54 | from charmhelpers.contrib.openstack.ip import ( |
180 | 56 | resolve_address, | 55 | canonical_url, |
181 | 57 | PUBLIC, INTERNAL, ADMIN, | 56 | PUBLIC, INTERNAL, ADMIN, |
182 | 58 | ) | 57 | ) |
183 | 59 | 58 | ||
184 | @@ -273,16 +272,6 @@ | |||
185 | 273 | open_port(port=80) | 272 | open_port(port=80) |
186 | 274 | 273 | ||
187 | 275 | 274 | ||
188 | 276 | # XXX Define local canonical_url until charm has been updated to use the | ||
189 | 277 | # standard context architecture. | ||
190 | 278 | def canonical_url(configs, endpoint_type=PUBLIC): | ||
191 | 279 | scheme = 'http' | ||
192 | 280 | address = resolve_address(endpoint_type) | ||
193 | 281 | if is_ipv6(address): | ||
194 | 282 | address = "[{}]".format(address) | ||
195 | 283 | return '%s://%s' % (scheme, address) | ||
196 | 284 | |||
197 | 285 | |||
198 | 286 | @hooks.hook('identity-service-relation-joined') | 275 | @hooks.hook('identity-service-relation-joined') |
199 | 287 | def identity_joined(relid=None): | 276 | def identity_joined(relid=None): |
200 | 288 | if cmp_pkgrevno('radosgw', '0.55') < 0: | 277 | if cmp_pkgrevno('radosgw', '0.55') < 0: |
201 | @@ -290,11 +279,11 @@ | |||
202 | 290 | sys.exit(1) | 279 | sys.exit(1) |
203 | 291 | 280 | ||
204 | 292 | port = 80 | 281 | port = 80 |
206 | 293 | admin_url = '%s:%i/swift' % (canonical_url(ADMIN), port) | 282 | admin_url = '%s:%i/swift' % (canonical_url(None, ADMIN), port) |
207 | 294 | internal_url = '%s:%s/swift/v1' % \ | 283 | internal_url = '%s:%s/swift/v1' % \ |
209 | 295 | (canonical_url(INTERNAL), port) | 284 | (canonical_url(None, INTERNAL), port) |
210 | 296 | public_url = '%s:%s/swift/v1' % \ | 285 | public_url = '%s:%s/swift/v1' % \ |
212 | 297 | (canonical_url(PUBLIC), port) | 286 | (canonical_url(None, PUBLIC), port) |
213 | 298 | relation_set(service='swift', | 287 | relation_set(service='swift', |
214 | 299 | region=config('region'), | 288 | region=config('region'), |
215 | 300 | public_url=public_url, internal_url=internal_url, | 289 | public_url=public_url, internal_url=internal_url, |
216 | 301 | 290 | ||
217 | === modified file 'unit_tests/test_hooks.py' | |||
218 | --- unit_tests/test_hooks.py 2015-04-28 21:18:24 +0000 | |||
219 | +++ unit_tests/test_hooks.py 2015-06-15 18:43:37 +0000 | |||
220 | @@ -8,6 +8,7 @@ | |||
221 | 8 | CharmTestCase, | 8 | CharmTestCase, |
222 | 9 | patch_open | 9 | patch_open |
223 | 10 | ) | 10 | ) |
224 | 11 | from charmhelpers.contrib.openstack.ip import PUBLIC | ||
225 | 11 | 12 | ||
226 | 12 | dnsmock = MagicMock() | 13 | dnsmock = MagicMock() |
227 | 13 | modules = { | 14 | modules = { |
228 | @@ -45,7 +46,6 @@ | |||
229 | 45 | 'relation_set', | 46 | 'relation_set', |
230 | 46 | 'relation_get', | 47 | 'relation_get', |
231 | 47 | 'render_template', | 48 | 'render_template', |
232 | 48 | 'resolve_address', | ||
233 | 49 | 'shutil', | 49 | 'shutil', |
234 | 50 | 'subprocess', | 50 | 'subprocess', |
235 | 51 | 'sys', | 51 | 'sys', |
236 | @@ -323,14 +323,22 @@ | |||
237 | 323 | cmd = ['service', 'radosgw', 'restart'] | 323 | cmd = ['service', 'radosgw', 'restart'] |
238 | 324 | self.subprocess.call.assert_called_with(cmd) | 324 | self.subprocess.call.assert_called_with(cmd) |
239 | 325 | 325 | ||
241 | 326 | def test_identity_joined_early_version(self): | 326 | @patch('charmhelpers.contrib.openstack.ip.service_name', |
242 | 327 | lambda *args: 'ceph-radosgw') | ||
243 | 328 | @patch('charmhelpers.contrib.openstack.ip.config') | ||
244 | 329 | def test_identity_joined_early_version(self, _config): | ||
245 | 327 | self.cmp_pkgrevno.return_value = -1 | 330 | self.cmp_pkgrevno.return_value = -1 |
246 | 328 | ceph_hooks.identity_joined() | 331 | ceph_hooks.identity_joined() |
247 | 329 | self.sys.exit.assert_called_with(1) | 332 | self.sys.exit.assert_called_with(1) |
248 | 330 | 333 | ||
250 | 331 | def test_identity_joined(self): | 334 | @patch('charmhelpers.contrib.openstack.ip.service_name', |
251 | 335 | lambda *args: 'ceph-radosgw') | ||
252 | 336 | @patch('charmhelpers.contrib.openstack.ip.resolve_address') | ||
253 | 337 | @patch('charmhelpers.contrib.openstack.ip.config') | ||
254 | 338 | def test_identity_joined(self, _config, _resolve_address): | ||
255 | 332 | self.cmp_pkgrevno.return_value = 1 | 339 | self.cmp_pkgrevno.return_value = 1 |
257 | 333 | self.resolve_address.return_value = 'myserv' | 340 | _resolve_address.return_value = 'myserv' |
258 | 341 | _config.side_effect = self.test_config.get | ||
259 | 334 | self.test_config.set('region', 'region1') | 342 | self.test_config.set('region', 'region1') |
260 | 335 | self.test_config.set('operator-roles', 'admin') | 343 | self.test_config.set('operator-roles', 'admin') |
261 | 336 | self.unit_get.return_value = 'myserv' | 344 | self.unit_get.return_value = 'myserv' |
262 | @@ -344,6 +352,27 @@ | |||
263 | 344 | relation_id='rid', | 352 | relation_id='rid', |
264 | 345 | admin_url='http://myserv:80/swift') | 353 | admin_url='http://myserv:80/swift') |
265 | 346 | 354 | ||
266 | 355 | @patch('charmhelpers.contrib.openstack.ip.service_name', | ||
267 | 356 | lambda *args: 'ceph-radosgw') | ||
268 | 357 | @patch('charmhelpers.contrib.openstack.ip.is_clustered') | ||
269 | 358 | @patch('charmhelpers.contrib.openstack.ip.unit_get') | ||
270 | 359 | @patch('charmhelpers.contrib.openstack.ip.config') | ||
271 | 360 | def test_identity_joined_public_name(self, _config, _unit_get, | ||
272 | 361 | _is_clustered): | ||
273 | 362 | _config.side_effect = self.test_config.get | ||
274 | 363 | self.test_config.set('os-public-hostname', 'files.example.com') | ||
275 | 364 | _unit_get.return_value = 'myserv' | ||
276 | 365 | _is_clustered.return_value = False | ||
277 | 366 | ceph_hooks.identity_joined(relid='rid') | ||
278 | 367 | self.relation_set.assert_called_with( | ||
279 | 368 | service='swift', | ||
280 | 369 | region='RegionOne', | ||
281 | 370 | public_url='http://files.example.com:80/swift/v1', | ||
282 | 371 | internal_url='http://myserv:80/swift/v1', | ||
283 | 372 | requested_roles='Member,Admin', | ||
284 | 373 | relation_id='rid', | ||
285 | 374 | admin_url='http://myserv:80/swift') | ||
286 | 375 | |||
287 | 347 | def test_identity_changed(self): | 376 | def test_identity_changed(self): |
288 | 348 | _emit_cephconf = self.patch('emit_cephconf') | 377 | _emit_cephconf = self.patch('emit_cephconf') |
289 | 349 | _restart = self.patch('restart') | 378 | _restart = self.patch('restart') |
290 | @@ -351,10 +380,15 @@ | |||
291 | 351 | _emit_cephconf.assert_called() | 380 | _emit_cephconf.assert_called() |
292 | 352 | _restart.assert_called() | 381 | _restart.assert_called() |
293 | 353 | 382 | ||
295 | 354 | def test_canonical_url_ipv6(self): | 383 | @patch('charmhelpers.contrib.openstack.ip.is_clustered') |
296 | 384 | @patch('charmhelpers.contrib.openstack.ip.unit_get') | ||
297 | 385 | @patch('charmhelpers.contrib.openstack.ip.config') | ||
298 | 386 | def test_canonical_url_ipv6(self, _config, _unit_get, _is_clustered): | ||
299 | 355 | ipv6_addr = '2001:db8:85a3:8d3:1319:8a2e:370:7348' | 387 | ipv6_addr = '2001:db8:85a3:8d3:1319:8a2e:370:7348' |
302 | 356 | self.resolve_address.return_value = ipv6_addr | 388 | _config.side_effect = self.test_config.get |
303 | 357 | self.assertEquals(ceph_hooks.canonical_url({}), | 389 | _unit_get.return_value = ipv6_addr |
304 | 390 | _is_clustered.return_value = False | ||
305 | 391 | self.assertEquals(ceph_hooks.canonical_url({}, PUBLIC), | ||
306 | 358 | 'http://[%s]' % ipv6_addr) | 392 | 'http://[%s]' % ipv6_addr) |
307 | 359 | 393 | ||
308 | 360 | @patch.object(ceph_hooks, 'CONFIGS') | 394 | @patch.object(ceph_hooks, 'CONFIGS') |
charm_lint_check #5403 ceph-radosgw for billy-olsen mp262000
LINT OK: passed
Build: http:// 10.245. 162.77: 8080/job/ charm_lint_ check/5403/