Merge lp:~billy-olsen/charms/trusty/cinder/public-endpoint-host into lp:~openstack-charmers-archive/charms/trusty/cinder/next
- Trusty Tahr (14.04)
- public-endpoint-host
- Merge into next
Status: | Merged |
---|---|
Merged at revision: | 97 |
Proposed branch: | lp:~billy-olsen/charms/trusty/cinder/public-endpoint-host |
Merge into: | lp:~openstack-charmers-archive/charms/trusty/cinder/next |
Diff against target: |
270 lines (+107/-50) 3 files modified
config.yaml (+13/-0) hooks/charmhelpers/contrib/openstack/ip.py (+49/-44) unit_tests/test_cinder_hooks.py (+45/-6) |
To merge this branch: | bzr merge lp:~billy-olsen/charms/trusty/cinder/public-endpoint-host |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Corey Bryant (community) | Approve | ||
Review via email: mp+261010@code.launchpad.net |
Commit message
Description of the change
Provides a config option which allows the user to specify the public hostname used to advertise to keystone when creating endpoints.
uosci-testing-bot (uosci-testing-bot) wrote : | # |
uosci-testing-bot (uosci-testing-bot) wrote : | # |
charm_unit_test #4728 cinder-next for billy-olsen mp261010
UNIT OK: passed
uosci-testing-bot (uosci-testing-bot) wrote : | # |
charm_lint_check #5052 cinder-next for billy-olsen mp261010
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://
Build: http://
uosci-testing-bot (uosci-testing-bot) wrote : | # |
charm_lint_check #5052 cinder-next for billy-olsen mp261010
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://
Build: http://
uosci-testing-bot (uosci-testing-bot) wrote : | # |
charm_lint_check #5052 cinder-next for billy-olsen mp261010
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://
Build: http://
uosci-testing-bot (uosci-testing-bot) wrote : | # |
charm_amulet_test #4454 cinder-next for billy-olsen mp261010
AMULET OK: passed
Build: http://
- 98. By Billy Olsen
-
Sync with /next branch
- 99. By Billy Olsen
-
c-h sync
uosci-testing-bot (uosci-testing-bot) wrote : | # |
charm_lint_check #5072 cinder-next for billy-olsen mp261010
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://
Build: http://
uosci-testing-bot (uosci-testing-bot) wrote : | # |
charm_unit_test #4751 cinder-next for billy-olsen mp261010
UNIT OK: passed
- 100. By Billy Olsen
-
Fix lint error
Billy Olsen (billy-olsen) wrote : | # |
Apparently the flake8 version used is different than mine and (vivid) doesn't like indented map values.
Billy Olsen (billy-olsen) wrote : | # |
err (vivid) is mine - I'm assuming trusty for osci
uosci-testing-bot (uosci-testing-bot) wrote : | # |
charm_lint_check #5075 cinder-next for billy-olsen mp261010
LINT OK: passed
uosci-testing-bot (uosci-testing-bot) wrote : | # |
charm_unit_test #4754 cinder-next for billy-olsen mp261010
UNIT OK: passed
uosci-testing-bot (uosci-testing-bot) wrote : | # |
charm_amulet_test #4479 cinder-next for billy-olsen mp261010
AMULET FAIL: amulet-test failed
AMULET Results (max last 2 lines):
make: *** [test] Error 1
ERROR:root:Make target returned non-zero.
Full amulet test output: http://
Build: http://
uosci-testing-bot (uosci-testing-bot) wrote : | # |
charm_amulet_test #4482 cinder-next for billy-olsen mp261010
AMULET OK: passed
Build: http://
Corey Bryant (corey.bryant) : | # |
Corey Bryant (corey.bryant) wrote : | # |
Hmm cinder tests seem to be failing after the merge. Can you take a look?
Corey Bryant (corey.bryant) wrote : | # |
Unit tests, that is.
- 101. By Billy Olsen
-
Merge with /next
Preview Diff
1 | === modified file 'config.yaml' | |||
2 | --- config.yaml 2015-04-02 12:13:45 +0000 | |||
3 | +++ config.yaml 2015-06-10 21:37:32 +0000 | |||
4 | @@ -207,6 +207,19 @@ | |||
5 | 207 | 192.168.0.0/24) | 207 | 192.168.0.0/24) |
6 | 208 | . | 208 | . |
7 | 209 | This network will be used for public endpoints. | 209 | This network will be used for public endpoints. |
8 | 210 | os-public-hostname: | ||
9 | 211 | type: string | ||
10 | 212 | default: | ||
11 | 213 | description: | | ||
12 | 214 | The hostname or address of the public endpoints created for cinder | ||
13 | 215 | in the keystone identity provider. | ||
14 | 216 | . | ||
15 | 217 | This value will be used for public endpoints. For example, an | ||
16 | 218 | os-public-hostname set to 'cinder.example.com' with ssl enabled will | ||
17 | 219 | create two public endpoints for cinder. | ||
18 | 220 | . | ||
19 | 221 | https://cinder.example.com:443/v1/$(tenant_id)s and | ||
20 | 222 | https://cinder.example.com:443/v2/$(tenant_id)s | ||
21 | 210 | prefer-ipv6: | 223 | prefer-ipv6: |
22 | 211 | type: boolean | 224 | type: boolean |
23 | 212 | default: False | 225 | default: False |
24 | 213 | 226 | ||
25 | === modified file 'hooks/charmhelpers/contrib/openstack/ip.py' | |||
26 | --- hooks/charmhelpers/contrib/openstack/ip.py 2015-03-13 13:00:03 +0000 | |||
27 | +++ hooks/charmhelpers/contrib/openstack/ip.py 2015-06-10 21:37:32 +0000 | |||
28 | @@ -17,6 +17,7 @@ | |||
29 | 17 | from charmhelpers.core.hookenv import ( | 17 | from charmhelpers.core.hookenv import ( |
30 | 18 | config, | 18 | config, |
31 | 19 | unit_get, | 19 | unit_get, |
32 | 20 | service_name, | ||
33 | 20 | ) | 21 | ) |
34 | 21 | from charmhelpers.contrib.network.ip import ( | 22 | from charmhelpers.contrib.network.ip import ( |
35 | 22 | get_address_in_network, | 23 | get_address_in_network, |
36 | @@ -26,8 +27,6 @@ | |||
37 | 26 | ) | 27 | ) |
38 | 27 | from charmhelpers.contrib.hahelpers.cluster import is_clustered | 28 | from charmhelpers.contrib.hahelpers.cluster import is_clustered |
39 | 28 | 29 | ||
40 | 29 | from functools import partial | ||
41 | 30 | |||
42 | 31 | PUBLIC = 'public' | 30 | PUBLIC = 'public' |
43 | 32 | INTERNAL = 'int' | 31 | INTERNAL = 'int' |
44 | 33 | ADMIN = 'admin' | 32 | ADMIN = 'admin' |
45 | @@ -35,15 +34,18 @@ | |||
46 | 35 | ADDRESS_MAP = { | 34 | ADDRESS_MAP = { |
47 | 36 | PUBLIC: { | 35 | PUBLIC: { |
48 | 37 | 'config': 'os-public-network', | 36 | 'config': 'os-public-network', |
50 | 38 | 'fallback': 'public-address' | 37 | 'fallback': 'public-address', |
51 | 38 | 'override': 'os-public-hostname', | ||
52 | 39 | }, | 39 | }, |
53 | 40 | INTERNAL: { | 40 | INTERNAL: { |
54 | 41 | 'config': 'os-internal-network', | 41 | 'config': 'os-internal-network', |
56 | 42 | 'fallback': 'private-address' | 42 | 'fallback': 'private-address', |
57 | 43 | 'override': 'os-internal-hostname', | ||
58 | 43 | }, | 44 | }, |
59 | 44 | ADMIN: { | 45 | ADMIN: { |
60 | 45 | 'config': 'os-admin-network', | 46 | 'config': 'os-admin-network', |
62 | 46 | 'fallback': 'private-address' | 47 | 'fallback': 'private-address', |
63 | 48 | 'override': 'os-admin-hostname', | ||
64 | 47 | } | 49 | } |
65 | 48 | } | 50 | } |
66 | 49 | 51 | ||
67 | @@ -57,15 +59,50 @@ | |||
68 | 57 | :param endpoint_type: str endpoint type to resolve. | 59 | :param endpoint_type: str endpoint type to resolve. |
69 | 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. |
70 | 59 | """ | 61 | """ |
74 | 60 | scheme = 'http' | 62 | scheme = _get_scheme(configs) |
75 | 61 | if 'https' in configs.complete_contexts(): | 63 | |
73 | 62 | scheme = 'https' | ||
76 | 63 | address = resolve_address(endpoint_type) | 64 | address = resolve_address(endpoint_type) |
77 | 64 | if is_ipv6(address): | 65 | if is_ipv6(address): |
78 | 65 | address = "[{}]".format(address) | 66 | address = "[{}]".format(address) |
79 | 67 | |||
80 | 66 | return '%s://%s' % (scheme, address) | 68 | return '%s://%s' % (scheme, address) |
81 | 67 | 69 | ||
82 | 68 | 70 | ||
83 | 71 | def _get_scheme(configs): | ||
84 | 72 | """Returns the scheme to use for the url (either http or https) | ||
85 | 73 | depending upon whether https is in the configs value. | ||
86 | 74 | |||
87 | 75 | :param configs: OSTemplateRenderer config templating object to inspect | ||
88 | 76 | for a complete https context. | ||
89 | 77 | :returns: either 'http' or 'https' depending on whether https is | ||
90 | 78 | configured within the configs context. | ||
91 | 79 | """ | ||
92 | 80 | scheme = 'http' | ||
93 | 81 | if configs and 'https' in configs.complete_contexts(): | ||
94 | 82 | scheme = 'https' | ||
95 | 83 | return scheme | ||
96 | 84 | |||
97 | 85 | |||
98 | 86 | def _get_address_override(endpoint_type=PUBLIC): | ||
99 | 87 | """Returns any address overrides that the user has defined based on the | ||
100 | 88 | endpoint type. | ||
101 | 89 | |||
102 | 90 | Note: this function allows for the service name to be inserted into the | ||
103 | 91 | address if the user specifies {service_name}.somehost.org. | ||
104 | 92 | |||
105 | 93 | :param endpoint_type: the type of endpoint to retrieve the override | ||
106 | 94 | value for. | ||
107 | 95 | :returns: any endpoint address or hostname that the user has overridden | ||
108 | 96 | or None if an override is not present. | ||
109 | 97 | """ | ||
110 | 98 | override_key = ADDRESS_MAP[endpoint_type]['override'] | ||
111 | 99 | addr_override = config(override_key) | ||
112 | 100 | if not addr_override: | ||
113 | 101 | return None | ||
114 | 102 | else: | ||
115 | 103 | return addr_override.format(service_name=service_name()) | ||
116 | 104 | |||
117 | 105 | |||
118 | 69 | def resolve_address(endpoint_type=PUBLIC): | 106 | def resolve_address(endpoint_type=PUBLIC): |
119 | 70 | """Return unit address depending on net config. | 107 | """Return unit address depending on net config. |
120 | 71 | 108 | ||
121 | @@ -77,7 +114,10 @@ | |||
122 | 77 | 114 | ||
123 | 78 | :param endpoint_type: Network endpoing type | 115 | :param endpoint_type: Network endpoing type |
124 | 79 | """ | 116 | """ |
126 | 80 | resolved_address = None | 117 | resolved_address = _get_address_override(endpoint_type) |
127 | 118 | if resolved_address: | ||
128 | 119 | return resolved_address | ||
129 | 120 | |||
130 | 81 | vips = config('vip') | 121 | vips = config('vip') |
131 | 82 | if vips: | 122 | if vips: |
132 | 83 | vips = vips.split() | 123 | vips = vips.split() |
133 | @@ -109,38 +149,3 @@ | |||
134 | 109 | "clustered=%s)" % (net_type, clustered)) | 149 | "clustered=%s)" % (net_type, clustered)) |
135 | 110 | 150 | ||
136 | 111 | return resolved_address | 151 | return resolved_address |
137 | 112 | |||
138 | 113 | |||
139 | 114 | def endpoint_url(configs, url_template, port, endpoint_type=PUBLIC, | ||
140 | 115 | override=None): | ||
141 | 116 | """Returns the correct endpoint URL to advertise to Keystone. | ||
142 | 117 | |||
143 | 118 | This method provides the correct endpoint URL which should be advertised to | ||
144 | 119 | the keystone charm for endpoint creation. This method allows for the url to | ||
145 | 120 | be overridden to force a keystone endpoint to have specific URL for any of | ||
146 | 121 | the defined scopes (admin, internal, public). | ||
147 | 122 | |||
148 | 123 | :param configs: OSTemplateRenderer config templating object to inspect | ||
149 | 124 | for a complete https context. | ||
150 | 125 | :param url_template: str format string for creating the url template. Only | ||
151 | 126 | two values will be passed - the scheme+hostname | ||
152 | 127 | returned by the canonical_url and the port. | ||
153 | 128 | :param endpoint_type: str endpoint type to resolve. | ||
154 | 129 | :param override: str the name of the config option which overrides the | ||
155 | 130 | endpoint URL defined by the charm itself. None will | ||
156 | 131 | disable any overrides (default). | ||
157 | 132 | """ | ||
158 | 133 | if override: | ||
159 | 134 | # Return any user-defined overrides for the keystone endpoint URL. | ||
160 | 135 | user_value = config(override) | ||
161 | 136 | if user_value: | ||
162 | 137 | return user_value.strip() | ||
163 | 138 | |||
164 | 139 | return url_template % (canonical_url(configs, endpoint_type), port) | ||
165 | 140 | |||
166 | 141 | |||
167 | 142 | public_endpoint = partial(endpoint_url, endpoint_type=PUBLIC) | ||
168 | 143 | |||
169 | 144 | internal_endpoint = partial(endpoint_url, endpoint_type=INTERNAL) | ||
170 | 145 | |||
171 | 146 | admin_endpoint = partial(endpoint_url, endpoint_type=ADMIN) | ||
172 | 147 | 152 | ||
173 | === modified file 'unit_tests/test_cinder_hooks.py' | |||
174 | --- unit_tests/test_cinder_hooks.py 2015-05-11 08:19:49 +0000 | |||
175 | +++ unit_tests/test_cinder_hooks.py 2015-06-10 21:37:32 +0000 | |||
176 | @@ -68,7 +68,6 @@ | |||
177 | 68 | 'openstack_upgrade_available', | 68 | 'openstack_upgrade_available', |
178 | 69 | 'os_release', | 69 | 'os_release', |
179 | 70 | # charmhelpers.contrib.hahelpers.cluster_utils | 70 | # charmhelpers.contrib.hahelpers.cluster_utils |
180 | 71 | 'canonical_url', | ||
181 | 72 | 'is_elected_leader', | 71 | 'is_elected_leader', |
182 | 73 | 'get_hacluster_config', | 72 | 'get_hacluster_config', |
183 | 74 | 'execd_preinstall', | 73 | 'execd_preinstall', |
184 | @@ -413,12 +412,13 @@ | |||
185 | 413 | vhost='openstack', | 412 | vhost='openstack', |
186 | 414 | relation_id='amqp:1') | 413 | relation_id='amqp:1') |
187 | 415 | 414 | ||
189 | 416 | def test_identity_service_joined(self): | 415 | @patch.object(hooks, 'canonical_url') |
190 | 416 | def test_identity_service_joined(self, _canonical_url): | ||
191 | 417 | 'It properly requests unclustered endpoint via identity-service' | 417 | 'It properly requests unclustered endpoint via identity-service' |
192 | 418 | self.os_release.return_value = 'havana' | 418 | self.os_release.return_value = 'havana' |
193 | 419 | self.unit_get.return_value = 'cindernode1' | 419 | self.unit_get.return_value = 'cindernode1' |
194 | 420 | self.config.side_effect = self.test_config.get | 420 | self.config.side_effect = self.test_config.get |
196 | 421 | self.canonical_url.return_value = 'http://cindernode1' | 421 | _canonical_url.return_value = 'http://cindernode1' |
197 | 422 | hooks.hooks.execute(['hooks/identity-service-relation-joined']) | 422 | hooks.hooks.execute(['hooks/identity-service-relation-joined']) |
198 | 423 | expected = { | 423 | expected = { |
199 | 424 | 'region': None, | 424 | 'region': None, |
200 | @@ -435,12 +435,13 @@ | |||
201 | 435 | } | 435 | } |
202 | 436 | self.relation_set.assert_called_with(**expected) | 436 | self.relation_set.assert_called_with(**expected) |
203 | 437 | 437 | ||
205 | 438 | def test_identity_service_joined_icehouse(self): | 438 | @patch.object(hooks, 'canonical_url') |
206 | 439 | def test_identity_service_joined_icehouse(self, _canonical_url): | ||
207 | 439 | 'It properly requests unclustered endpoint via identity-service' | 440 | 'It properly requests unclustered endpoint via identity-service' |
208 | 440 | self.os_release.return_value = 'icehouse' | 441 | self.os_release.return_value = 'icehouse' |
209 | 441 | self.unit_get.return_value = 'cindernode1' | 442 | self.unit_get.return_value = 'cindernode1' |
210 | 442 | self.config.side_effect = self.test_config.get | 443 | self.config.side_effect = self.test_config.get |
212 | 443 | self.canonical_url.return_value = 'http://cindernode1' | 444 | _canonical_url.return_value = 'http://cindernode1' |
213 | 444 | hooks.hooks.execute(['hooks/identity-service-relation-joined']) | 445 | hooks.hooks.execute(['hooks/identity-service-relation-joined']) |
214 | 445 | expected = { | 446 | expected = { |
215 | 446 | 'region': None, | 447 | 'region': None, |
216 | @@ -463,6 +464,42 @@ | |||
217 | 463 | } | 464 | } |
218 | 464 | self.relation_set.assert_called_with(**expected) | 465 | self.relation_set.assert_called_with(**expected) |
219 | 465 | 466 | ||
220 | 467 | @patch('charmhelpers.contrib.openstack.ip.config') | ||
221 | 468 | @patch('charmhelpers.contrib.openstack.ip.unit_get') | ||
222 | 469 | @patch('charmhelpers.contrib.openstack.ip.is_clustered') | ||
223 | 470 | def test_identity_service_joined_public_name(self, _is_clustered, | ||
224 | 471 | _unit_get, _config): | ||
225 | 472 | self.os_release.return_value = 'icehouse' | ||
226 | 473 | self.unit_get.return_value = 'cindernode1' | ||
227 | 474 | _unit_get.return_value = 'cindernode1' | ||
228 | 475 | self.config.side_effect = self.test_config.get | ||
229 | 476 | _config.side_effect = self.test_config.get | ||
230 | 477 | self.test_config.set('os-public-hostname', 'public.example.com') | ||
231 | 478 | _is_clustered.return_value = False | ||
232 | 479 | hooks.hooks.execute(['hooks/identity-service-relation-joined']) | ||
233 | 480 | v1_url = 'http://public.example.com:8776/v1/$(tenant_id)s' | ||
234 | 481 | v2_url = 'http://public.example.com:8776/v2/$(tenant_id)s' | ||
235 | 482 | expected = { | ||
236 | 483 | 'region': None, | ||
237 | 484 | 'service': None, | ||
238 | 485 | 'public_url': None, | ||
239 | 486 | 'internal_url': None, | ||
240 | 487 | 'admin_url': None, | ||
241 | 488 | 'cinder_service': 'cinder', | ||
242 | 489 | 'cinder_region': 'RegionOne', | ||
243 | 490 | 'cinder_public_url': v1_url, | ||
244 | 491 | 'cinder_admin_url': 'http://cindernode1:8776/v1/$(tenant_id)s', | ||
245 | 492 | 'cinder_internal_url': 'http://cindernode1:8776/v1/$(tenant_id)s', | ||
246 | 493 | 'cinderv2_service': 'cinderv2', | ||
247 | 494 | 'cinderv2_region': 'RegionOne', | ||
248 | 495 | 'cinderv2_public_url': v2_url, | ||
249 | 496 | 'cinderv2_admin_url': 'http://cindernode1:8776/v2/$(tenant_id)s', | ||
250 | 497 | 'cinderv2_internal_url': ('http://cindernode1:8776/' | ||
251 | 498 | 'v2/$(tenant_id)s'), | ||
252 | 499 | 'relation_id': None, | ||
253 | 500 | } | ||
254 | 501 | self.relation_set.assert_called_with(**expected) | ||
255 | 502 | |||
256 | 466 | @patch('os.mkdir') | 503 | @patch('os.mkdir') |
257 | 467 | def test_ceph_joined(self, mkdir): | 504 | def test_ceph_joined(self, mkdir): |
258 | 468 | 'It correctly prepares for a ceph changed hook' | 505 | 'It correctly prepares for a ceph changed hook' |
259 | @@ -502,8 +539,10 @@ | |||
260 | 502 | self.assertNotIn(c, self.CONFIGS.write.call_args_list) | 539 | self.assertNotIn(c, self.CONFIGS.write.call_args_list) |
261 | 503 | self.assertFalse(self.set_ceph_env_variables.called) | 540 | self.assertFalse(self.set_ceph_env_variables.called) |
262 | 504 | 541 | ||
263 | 542 | @patch('charmhelpers.core.host.service') | ||
264 | 505 | @patch("cinder_hooks.relation_get", autospec=True) | 543 | @patch("cinder_hooks.relation_get", autospec=True) |
266 | 506 | def test_ceph_changed_broker_success(self, mock_relation_get): | 544 | def test_ceph_changed_broker_success(self, mock_relation_get, |
267 | 545 | _service): | ||
268 | 507 | 'It ensures ceph assets created on ceph changed' | 546 | 'It ensures ceph assets created on ceph changed' |
269 | 508 | self.CONFIGS.complete_contexts.return_value = ['ceph'] | 547 | self.CONFIGS.complete_contexts.return_value = ['ceph'] |
270 | 509 | self.service_name.return_value = 'cinder' | 548 | self.service_name.return_value = 'cinder' |
charm_lint_check #5048 cinder-next for billy-olsen mp261010
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/11555857/ 10.245. 162.77: 8080/job/ charm_lint_ check/5048/
Build: http://