Merge lp:~billy-olsen/charms/trusty/cinder/public-endpoint-host into lp:~openstack-charmers-archive/charms/trusty/cinder/next

Proposed by Billy Olsen
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
Reviewer Review Type Date Requested Status
Corey Bryant Approve
Review via email: mp+261010@code.launchpad.net

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.

To post a comment you must log in.
Revision history for this message
uosci-testing-bot (uosci-testing-bot) wrote :

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/
Build: http://10.245.162.77:8080/job/charm_lint_check/5048/

Revision history for this message
uosci-testing-bot (uosci-testing-bot) wrote :

charm_unit_test #4728 cinder-next for billy-olsen mp261010
    UNIT OK: passed

Build: http://10.245.162.77:8080/job/charm_unit_test/4728/

Revision history for this message
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://paste.ubuntu.com/11564175/
Build: http://10.245.162.77:8080/job/charm_lint_check/5052/

Revision history for this message
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://paste.ubuntu.com/11564175/
Build: http://10.245.162.77:8080/job/charm_lint_check/5052/

Revision history for this message
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://paste.ubuntu.com/11564175/
Build: http://10.245.162.77:8080/job/charm_lint_check/5052/

Revision history for this message
uosci-testing-bot (uosci-testing-bot) wrote :

charm_amulet_test #4454 cinder-next for billy-olsen mp261010
    AMULET OK: passed

Build: http://10.245.162.77:8080/job/charm_amulet_test/4454/

98. By Billy Olsen

Sync with /next branch

99. By Billy Olsen

c-h sync

Revision history for this message
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://paste.ubuntu.com/11576118/
Build: http://10.245.162.77:8080/job/charm_lint_check/5072/

Revision history for this message
uosci-testing-bot (uosci-testing-bot) wrote :

charm_unit_test #4751 cinder-next for billy-olsen mp261010
    UNIT OK: passed

Build: http://10.245.162.77:8080/job/charm_unit_test/4751/

100. By Billy Olsen

Fix lint error

Revision history for this message
Billy Olsen (billy-olsen) wrote :

Apparently the flake8 version used is different than mine and (vivid) doesn't like indented map values.

Revision history for this message
Billy Olsen (billy-olsen) wrote :

err (vivid) is mine - I'm assuming trusty for osci

Revision history for this message
uosci-testing-bot (uosci-testing-bot) wrote :

charm_lint_check #5075 cinder-next for billy-olsen mp261010
    LINT OK: passed

Build: http://10.245.162.77:8080/job/charm_lint_check/5075/

Revision history for this message
uosci-testing-bot (uosci-testing-bot) wrote :

charm_unit_test #4754 cinder-next for billy-olsen mp261010
    UNIT OK: passed

Build: http://10.245.162.77:8080/job/charm_unit_test/4754/

Revision history for this message
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://paste.ubuntu.com/11577184/
Build: http://10.245.162.77:8080/job/charm_amulet_test/4479/

Revision history for this message
uosci-testing-bot (uosci-testing-bot) wrote :

charm_amulet_test #4482 cinder-next for billy-olsen mp261010
    AMULET OK: passed

Build: http://10.245.162.77:8080/job/charm_amulet_test/4482/

Revision history for this message
Corey Bryant (corey.bryant) :
review: Approve
Revision history for this message
Corey Bryant (corey.bryant) wrote :

Hmm cinder tests seem to be failing after the merge. Can you take a look?

Revision history for this message
Corey Bryant (corey.bryant) wrote :

Unit tests, that is.

101. By Billy Olsen

Merge with /next

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-04-02 12:13:45 +0000
3+++ config.yaml 2015-06-10 21:37:32 +0000
4@@ -207,6 +207,19 @@
5 192.168.0.0/24)
6 .
7 This network will be used for public endpoints.
8+ os-public-hostname:
9+ type: string
10+ default:
11+ description: |
12+ The hostname or address of the public endpoints created for cinder
13+ in the keystone identity provider.
14+ .
15+ This value will be used for public endpoints. For example, an
16+ os-public-hostname set to 'cinder.example.com' with ssl enabled will
17+ create two public endpoints for cinder.
18+ .
19+ https://cinder.example.com:443/v1/$(tenant_id)s and
20+ https://cinder.example.com:443/v2/$(tenant_id)s
21 prefer-ipv6:
22 type: boolean
23 default: False
24
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 from charmhelpers.core.hookenv import (
30 config,
31 unit_get,
32+ service_name,
33 )
34 from charmhelpers.contrib.network.ip import (
35 get_address_in_network,
36@@ -26,8 +27,6 @@
37 )
38 from charmhelpers.contrib.hahelpers.cluster import is_clustered
39
40-from functools import partial
41-
42 PUBLIC = 'public'
43 INTERNAL = 'int'
44 ADMIN = 'admin'
45@@ -35,15 +34,18 @@
46 ADDRESS_MAP = {
47 PUBLIC: {
48 'config': 'os-public-network',
49- 'fallback': 'public-address'
50+ 'fallback': 'public-address',
51+ 'override': 'os-public-hostname',
52 },
53 INTERNAL: {
54 'config': 'os-internal-network',
55- 'fallback': 'private-address'
56+ 'fallback': 'private-address',
57+ 'override': 'os-internal-hostname',
58 },
59 ADMIN: {
60 'config': 'os-admin-network',
61- 'fallback': 'private-address'
62+ 'fallback': 'private-address',
63+ 'override': 'os-admin-hostname',
64 }
65 }
66
67@@ -57,15 +59,50 @@
68 :param endpoint_type: str endpoint type to resolve.
69 :param returns: str base URL for services on the current service unit.
70 """
71- scheme = 'http'
72- if 'https' in configs.complete_contexts():
73- scheme = 'https'
74+ scheme = _get_scheme(configs)
75+
76 address = resolve_address(endpoint_type)
77 if is_ipv6(address):
78 address = "[{}]".format(address)
79+
80 return '%s://%s' % (scheme, address)
81
82
83+def _get_scheme(configs):
84+ """Returns the scheme to use for the url (either http or https)
85+ depending upon whether https is in the configs value.
86+
87+ :param configs: OSTemplateRenderer config templating object to inspect
88+ for a complete https context.
89+ :returns: either 'http' or 'https' depending on whether https is
90+ configured within the configs context.
91+ """
92+ scheme = 'http'
93+ if configs and 'https' in configs.complete_contexts():
94+ scheme = 'https'
95+ return scheme
96+
97+
98+def _get_address_override(endpoint_type=PUBLIC):
99+ """Returns any address overrides that the user has defined based on the
100+ endpoint type.
101+
102+ Note: this function allows for the service name to be inserted into the
103+ address if the user specifies {service_name}.somehost.org.
104+
105+ :param endpoint_type: the type of endpoint to retrieve the override
106+ value for.
107+ :returns: any endpoint address or hostname that the user has overridden
108+ or None if an override is not present.
109+ """
110+ override_key = ADDRESS_MAP[endpoint_type]['override']
111+ addr_override = config(override_key)
112+ if not addr_override:
113+ return None
114+ else:
115+ return addr_override.format(service_name=service_name())
116+
117+
118 def resolve_address(endpoint_type=PUBLIC):
119 """Return unit address depending on net config.
120
121@@ -77,7 +114,10 @@
122
123 :param endpoint_type: Network endpoing type
124 """
125- resolved_address = None
126+ resolved_address = _get_address_override(endpoint_type)
127+ if resolved_address:
128+ return resolved_address
129+
130 vips = config('vip')
131 if vips:
132 vips = vips.split()
133@@ -109,38 +149,3 @@
134 "clustered=%s)" % (net_type, clustered))
135
136 return resolved_address
137-
138-
139-def endpoint_url(configs, url_template, port, endpoint_type=PUBLIC,
140- override=None):
141- """Returns the correct endpoint URL to advertise to Keystone.
142-
143- This method provides the correct endpoint URL which should be advertised to
144- the keystone charm for endpoint creation. This method allows for the url to
145- be overridden to force a keystone endpoint to have specific URL for any of
146- the defined scopes (admin, internal, public).
147-
148- :param configs: OSTemplateRenderer config templating object to inspect
149- for a complete https context.
150- :param url_template: str format string for creating the url template. Only
151- two values will be passed - the scheme+hostname
152- returned by the canonical_url and the port.
153- :param endpoint_type: str endpoint type to resolve.
154- :param override: str the name of the config option which overrides the
155- endpoint URL defined by the charm itself. None will
156- disable any overrides (default).
157- """
158- if override:
159- # Return any user-defined overrides for the keystone endpoint URL.
160- user_value = config(override)
161- if user_value:
162- return user_value.strip()
163-
164- return url_template % (canonical_url(configs, endpoint_type), port)
165-
166-
167-public_endpoint = partial(endpoint_url, endpoint_type=PUBLIC)
168-
169-internal_endpoint = partial(endpoint_url, endpoint_type=INTERNAL)
170-
171-admin_endpoint = partial(endpoint_url, endpoint_type=ADMIN)
172
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 'openstack_upgrade_available',
178 'os_release',
179 # charmhelpers.contrib.hahelpers.cluster_utils
180- 'canonical_url',
181 'is_elected_leader',
182 'get_hacluster_config',
183 'execd_preinstall',
184@@ -413,12 +412,13 @@
185 vhost='openstack',
186 relation_id='amqp:1')
187
188- def test_identity_service_joined(self):
189+ @patch.object(hooks, 'canonical_url')
190+ def test_identity_service_joined(self, _canonical_url):
191 'It properly requests unclustered endpoint via identity-service'
192 self.os_release.return_value = 'havana'
193 self.unit_get.return_value = 'cindernode1'
194 self.config.side_effect = self.test_config.get
195- self.canonical_url.return_value = 'http://cindernode1'
196+ _canonical_url.return_value = 'http://cindernode1'
197 hooks.hooks.execute(['hooks/identity-service-relation-joined'])
198 expected = {
199 'region': None,
200@@ -435,12 +435,13 @@
201 }
202 self.relation_set.assert_called_with(**expected)
203
204- def test_identity_service_joined_icehouse(self):
205+ @patch.object(hooks, 'canonical_url')
206+ def test_identity_service_joined_icehouse(self, _canonical_url):
207 'It properly requests unclustered endpoint via identity-service'
208 self.os_release.return_value = 'icehouse'
209 self.unit_get.return_value = 'cindernode1'
210 self.config.side_effect = self.test_config.get
211- self.canonical_url.return_value = 'http://cindernode1'
212+ _canonical_url.return_value = 'http://cindernode1'
213 hooks.hooks.execute(['hooks/identity-service-relation-joined'])
214 expected = {
215 'region': None,
216@@ -463,6 +464,42 @@
217 }
218 self.relation_set.assert_called_with(**expected)
219
220+ @patch('charmhelpers.contrib.openstack.ip.config')
221+ @patch('charmhelpers.contrib.openstack.ip.unit_get')
222+ @patch('charmhelpers.contrib.openstack.ip.is_clustered')
223+ def test_identity_service_joined_public_name(self, _is_clustered,
224+ _unit_get, _config):
225+ self.os_release.return_value = 'icehouse'
226+ self.unit_get.return_value = 'cindernode1'
227+ _unit_get.return_value = 'cindernode1'
228+ self.config.side_effect = self.test_config.get
229+ _config.side_effect = self.test_config.get
230+ self.test_config.set('os-public-hostname', 'public.example.com')
231+ _is_clustered.return_value = False
232+ hooks.hooks.execute(['hooks/identity-service-relation-joined'])
233+ v1_url = 'http://public.example.com:8776/v1/$(tenant_id)s'
234+ v2_url = 'http://public.example.com:8776/v2/$(tenant_id)s'
235+ expected = {
236+ 'region': None,
237+ 'service': None,
238+ 'public_url': None,
239+ 'internal_url': None,
240+ 'admin_url': None,
241+ 'cinder_service': 'cinder',
242+ 'cinder_region': 'RegionOne',
243+ 'cinder_public_url': v1_url,
244+ 'cinder_admin_url': 'http://cindernode1:8776/v1/$(tenant_id)s',
245+ 'cinder_internal_url': 'http://cindernode1:8776/v1/$(tenant_id)s',
246+ 'cinderv2_service': 'cinderv2',
247+ 'cinderv2_region': 'RegionOne',
248+ 'cinderv2_public_url': v2_url,
249+ 'cinderv2_admin_url': 'http://cindernode1:8776/v2/$(tenant_id)s',
250+ 'cinderv2_internal_url': ('http://cindernode1:8776/'
251+ 'v2/$(tenant_id)s'),
252+ 'relation_id': None,
253+ }
254+ self.relation_set.assert_called_with(**expected)
255+
256 @patch('os.mkdir')
257 def test_ceph_joined(self, mkdir):
258 'It correctly prepares for a ceph changed hook'
259@@ -502,8 +539,10 @@
260 self.assertNotIn(c, self.CONFIGS.write.call_args_list)
261 self.assertFalse(self.set_ceph_env_variables.called)
262
263+ @patch('charmhelpers.core.host.service')
264 @patch("cinder_hooks.relation_get", autospec=True)
265- def test_ceph_changed_broker_success(self, mock_relation_get):
266+ def test_ceph_changed_broker_success(self, mock_relation_get,
267+ _service):
268 'It ensures ceph assets created on ceph changed'
269 self.CONFIGS.complete_contexts.return_value = ['ceph']
270 self.service_name.return_value = 'cinder'

Subscribers

People subscribed via source and target branches