Merge lp:~billy-olsen/charms/trusty/cinder/backport-lp1398182 into lp:~openstack-charmers-archive/charms/trusty/cinder/trunk
- Trusty Tahr (14.04)
- backport-lp1398182
- Merge into trunk
Status: | Merged |
---|---|
Merged at revision: | 96 |
Proposed branch: | lp:~billy-olsen/charms/trusty/cinder/backport-lp1398182 |
Merge into: | lp:~openstack-charmers-archive/charms/trusty/cinder/trunk |
Diff against target: |
320 lines (+132/-50) 4 files modified
config.yaml (+13/-0) hooks/charmhelpers/contrib/hahelpers/cluster.py (+25/-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/backport-lp1398182 |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Corey Bryant (community) | Approve | ||
Review via email: mp+262001@code.launchpad.net |
Commit message
Description of the change
uosci-testing-bot (uosci-testing-bot) wrote : | # |
uosci-testing-bot (uosci-testing-bot) wrote : | # |
charm_unit_test #5036 cinder for billy-olsen mp262001
UNIT OK: passed
uosci-testing-bot (uosci-testing-bot) wrote : | # |
charm_amulet_test #4647 cinder for billy-olsen mp262001
AMULET OK: passed
Build: http://
Corey Bryant (corey.bryant) : | # |
- 97. By Billy Olsen
-
[billy-olsen] Official c-h stable sync
uosci-testing-bot (uosci-testing-bot) wrote : | # |
charm_lint_check #5462 cinder for billy-olsen mp262001
LINT OK: passed
uosci-testing-bot (uosci-testing-bot) wrote : | # |
charm_unit_test #5094 cinder for billy-olsen mp262001
UNIT OK: passed
uosci-testing-bot (uosci-testing-bot) wrote : | # |
charm_amulet_test #4666 cinder for billy-olsen mp262001
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 #4679 cinder for billy-olsen mp262001
AMULET OK: passed
Build: http://
Preview Diff
1 | === modified file 'config.yaml' |
2 | --- config.yaml 2015-04-02 12:13:45 +0000 |
3 | +++ config.yaml 2015-06-18 23:23:26 +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/hahelpers/cluster.py' |
26 | --- hooks/charmhelpers/contrib/hahelpers/cluster.py 2015-03-13 13:00:03 +0000 |
27 | +++ hooks/charmhelpers/contrib/hahelpers/cluster.py 2015-06-18 23:23:26 +0000 |
28 | @@ -52,6 +52,8 @@ |
29 | bool_from_string, |
30 | ) |
31 | |
32 | +DC_RESOURCE_NAME = 'DC' |
33 | + |
34 | |
35 | class HAIncompleteConfig(Exception): |
36 | pass |
37 | @@ -95,6 +97,27 @@ |
38 | return False |
39 | |
40 | |
41 | +def is_crm_dc(): |
42 | + """ |
43 | + Determine leadership by querying the pacemaker Designated Controller |
44 | + """ |
45 | + cmd = ['crm', 'status'] |
46 | + try: |
47 | + status = subprocess.check_output(cmd, stderr=subprocess.STDOUT) |
48 | + if not isinstance(status, six.text_type): |
49 | + status = six.text_type(status, "utf-8") |
50 | + except subprocess.CalledProcessError: |
51 | + return False |
52 | + current_dc = '' |
53 | + for line in status.split('\n'): |
54 | + if line.startswith('Current DC'): |
55 | + # Current DC: juju-lytrusty-machine-2 (168108163) - partition with quorum |
56 | + current_dc = line.split(':')[1].split()[0] |
57 | + if current_dc == get_unit_hostname(): |
58 | + return True |
59 | + return False |
60 | + |
61 | + |
62 | @retry_on_exception(5, base_delay=2, exc_type=CRMResourceNotFound) |
63 | def is_crm_leader(resource, retry=False): |
64 | """ |
65 | @@ -104,6 +127,8 @@ |
66 | We allow this operation to be retried to avoid the possibility of getting a |
67 | false negative. See LP #1396246 for more info. |
68 | """ |
69 | + if resource == DC_RESOURCE_NAME: |
70 | + return is_crm_dc() |
71 | cmd = ['crm', 'resource', 'show', resource] |
72 | try: |
73 | status = subprocess.check_output(cmd, stderr=subprocess.STDOUT) |
74 | |
75 | === modified file 'hooks/charmhelpers/contrib/openstack/ip.py' |
76 | --- hooks/charmhelpers/contrib/openstack/ip.py 2015-03-13 13:00:03 +0000 |
77 | +++ hooks/charmhelpers/contrib/openstack/ip.py 2015-06-18 23:23:26 +0000 |
78 | @@ -17,6 +17,7 @@ |
79 | from charmhelpers.core.hookenv import ( |
80 | config, |
81 | unit_get, |
82 | + service_name, |
83 | ) |
84 | from charmhelpers.contrib.network.ip import ( |
85 | get_address_in_network, |
86 | @@ -26,8 +27,6 @@ |
87 | ) |
88 | from charmhelpers.contrib.hahelpers.cluster import is_clustered |
89 | |
90 | -from functools import partial |
91 | - |
92 | PUBLIC = 'public' |
93 | INTERNAL = 'int' |
94 | ADMIN = 'admin' |
95 | @@ -35,15 +34,18 @@ |
96 | ADDRESS_MAP = { |
97 | PUBLIC: { |
98 | 'config': 'os-public-network', |
99 | - 'fallback': 'public-address' |
100 | + 'fallback': 'public-address', |
101 | + 'override': 'os-public-hostname', |
102 | }, |
103 | INTERNAL: { |
104 | 'config': 'os-internal-network', |
105 | - 'fallback': 'private-address' |
106 | + 'fallback': 'private-address', |
107 | + 'override': 'os-internal-hostname', |
108 | }, |
109 | ADMIN: { |
110 | 'config': 'os-admin-network', |
111 | - 'fallback': 'private-address' |
112 | + 'fallback': 'private-address', |
113 | + 'override': 'os-admin-hostname', |
114 | } |
115 | } |
116 | |
117 | @@ -57,15 +59,50 @@ |
118 | :param endpoint_type: str endpoint type to resolve. |
119 | :param returns: str base URL for services on the current service unit. |
120 | """ |
121 | - scheme = 'http' |
122 | - if 'https' in configs.complete_contexts(): |
123 | - scheme = 'https' |
124 | + scheme = _get_scheme(configs) |
125 | + |
126 | address = resolve_address(endpoint_type) |
127 | if is_ipv6(address): |
128 | address = "[{}]".format(address) |
129 | + |
130 | return '%s://%s' % (scheme, address) |
131 | |
132 | |
133 | +def _get_scheme(configs): |
134 | + """Returns the scheme to use for the url (either http or https) |
135 | + depending upon whether https is in the configs value. |
136 | + |
137 | + :param configs: OSTemplateRenderer config templating object to inspect |
138 | + for a complete https context. |
139 | + :returns: either 'http' or 'https' depending on whether https is |
140 | + configured within the configs context. |
141 | + """ |
142 | + scheme = 'http' |
143 | + if configs and 'https' in configs.complete_contexts(): |
144 | + scheme = 'https' |
145 | + return scheme |
146 | + |
147 | + |
148 | +def _get_address_override(endpoint_type=PUBLIC): |
149 | + """Returns any address overrides that the user has defined based on the |
150 | + endpoint type. |
151 | + |
152 | + Note: this function allows for the service name to be inserted into the |
153 | + address if the user specifies {service_name}.somehost.org. |
154 | + |
155 | + :param endpoint_type: the type of endpoint to retrieve the override |
156 | + value for. |
157 | + :returns: any endpoint address or hostname that the user has overridden |
158 | + or None if an override is not present. |
159 | + """ |
160 | + override_key = ADDRESS_MAP[endpoint_type]['override'] |
161 | + addr_override = config(override_key) |
162 | + if not addr_override: |
163 | + return None |
164 | + else: |
165 | + return addr_override.format(service_name=service_name()) |
166 | + |
167 | + |
168 | def resolve_address(endpoint_type=PUBLIC): |
169 | """Return unit address depending on net config. |
170 | |
171 | @@ -77,7 +114,10 @@ |
172 | |
173 | :param endpoint_type: Network endpoing type |
174 | """ |
175 | - resolved_address = None |
176 | + resolved_address = _get_address_override(endpoint_type) |
177 | + if resolved_address: |
178 | + return resolved_address |
179 | + |
180 | vips = config('vip') |
181 | if vips: |
182 | vips = vips.split() |
183 | @@ -109,38 +149,3 @@ |
184 | "clustered=%s)" % (net_type, clustered)) |
185 | |
186 | return resolved_address |
187 | - |
188 | - |
189 | -def endpoint_url(configs, url_template, port, endpoint_type=PUBLIC, |
190 | - override=None): |
191 | - """Returns the correct endpoint URL to advertise to Keystone. |
192 | - |
193 | - This method provides the correct endpoint URL which should be advertised to |
194 | - the keystone charm for endpoint creation. This method allows for the url to |
195 | - be overridden to force a keystone endpoint to have specific URL for any of |
196 | - the defined scopes (admin, internal, public). |
197 | - |
198 | - :param configs: OSTemplateRenderer config templating object to inspect |
199 | - for a complete https context. |
200 | - :param url_template: str format string for creating the url template. Only |
201 | - two values will be passed - the scheme+hostname |
202 | - returned by the canonical_url and the port. |
203 | - :param endpoint_type: str endpoint type to resolve. |
204 | - :param override: str the name of the config option which overrides the |
205 | - endpoint URL defined by the charm itself. None will |
206 | - disable any overrides (default). |
207 | - """ |
208 | - if override: |
209 | - # Return any user-defined overrides for the keystone endpoint URL. |
210 | - user_value = config(override) |
211 | - if user_value: |
212 | - return user_value.strip() |
213 | - |
214 | - return url_template % (canonical_url(configs, endpoint_type), port) |
215 | - |
216 | - |
217 | -public_endpoint = partial(endpoint_url, endpoint_type=PUBLIC) |
218 | - |
219 | -internal_endpoint = partial(endpoint_url, endpoint_type=INTERNAL) |
220 | - |
221 | -admin_endpoint = partial(endpoint_url, endpoint_type=ADMIN) |
222 | |
223 | === modified file 'unit_tests/test_cinder_hooks.py' |
224 | --- unit_tests/test_cinder_hooks.py 2015-04-21 15:20:20 +0000 |
225 | +++ unit_tests/test_cinder_hooks.py 2015-06-18 23:23:26 +0000 |
226 | @@ -67,7 +67,6 @@ |
227 | 'openstack_upgrade_available', |
228 | 'os_release', |
229 | # charmhelpers.contrib.hahelpers.cluster_utils |
230 | - 'canonical_url', |
231 | 'eligible_leader', |
232 | 'get_hacluster_config', |
233 | 'execd_preinstall', |
234 | @@ -412,12 +411,13 @@ |
235 | vhost='openstack', |
236 | relation_id='amqp:1') |
237 | |
238 | - def test_identity_service_joined(self): |
239 | + @patch.object(hooks, 'canonical_url') |
240 | + def test_identity_service_joined(self, _canonical_url): |
241 | 'It properly requests unclustered endpoint via identity-service' |
242 | self.os_release.return_value = 'havana' |
243 | self.unit_get.return_value = 'cindernode1' |
244 | self.config.side_effect = self.test_config.get |
245 | - self.canonical_url.return_value = 'http://cindernode1' |
246 | + _canonical_url.return_value = 'http://cindernode1' |
247 | hooks.hooks.execute(['hooks/identity-service-relation-joined']) |
248 | expected = { |
249 | 'region': None, |
250 | @@ -434,12 +434,13 @@ |
251 | } |
252 | self.relation_set.assert_called_with(**expected) |
253 | |
254 | - def test_identity_service_joined_icehouse(self): |
255 | + @patch.object(hooks, 'canonical_url') |
256 | + def test_identity_service_joined_icehouse(self, _canonical_url): |
257 | 'It properly requests unclustered endpoint via identity-service' |
258 | self.os_release.return_value = 'icehouse' |
259 | self.unit_get.return_value = 'cindernode1' |
260 | self.config.side_effect = self.test_config.get |
261 | - self.canonical_url.return_value = 'http://cindernode1' |
262 | + _canonical_url.return_value = 'http://cindernode1' |
263 | hooks.hooks.execute(['hooks/identity-service-relation-joined']) |
264 | expected = { |
265 | 'region': None, |
266 | @@ -462,6 +463,42 @@ |
267 | } |
268 | self.relation_set.assert_called_with(**expected) |
269 | |
270 | + @patch('charmhelpers.contrib.openstack.ip.config') |
271 | + @patch('charmhelpers.contrib.openstack.ip.unit_get') |
272 | + @patch('charmhelpers.contrib.openstack.ip.is_clustered') |
273 | + def test_identity_service_joined_public_name(self, _is_clustered, |
274 | + _unit_get, _config): |
275 | + self.os_release.return_value = 'icehouse' |
276 | + self.unit_get.return_value = 'cindernode1' |
277 | + _unit_get.return_value = 'cindernode1' |
278 | + self.config.side_effect = self.test_config.get |
279 | + _config.side_effect = self.test_config.get |
280 | + self.test_config.set('os-public-hostname', 'public.example.com') |
281 | + _is_clustered.return_value = False |
282 | + hooks.hooks.execute(['hooks/identity-service-relation-joined']) |
283 | + v1_url = 'http://public.example.com:8776/v1/$(tenant_id)s' |
284 | + v2_url = 'http://public.example.com:8776/v2/$(tenant_id)s' |
285 | + expected = { |
286 | + 'region': None, |
287 | + 'service': None, |
288 | + 'public_url': None, |
289 | + 'internal_url': None, |
290 | + 'admin_url': None, |
291 | + 'cinder_service': 'cinder', |
292 | + 'cinder_region': 'RegionOne', |
293 | + 'cinder_public_url': v1_url, |
294 | + 'cinder_admin_url': 'http://cindernode1:8776/v1/$(tenant_id)s', |
295 | + 'cinder_internal_url': 'http://cindernode1:8776/v1/$(tenant_id)s', |
296 | + 'cinderv2_service': 'cinderv2', |
297 | + 'cinderv2_region': 'RegionOne', |
298 | + 'cinderv2_public_url': v2_url, |
299 | + 'cinderv2_admin_url': 'http://cindernode1:8776/v2/$(tenant_id)s', |
300 | + 'cinderv2_internal_url': ('http://cindernode1:8776/' |
301 | + 'v2/$(tenant_id)s'), |
302 | + 'relation_id': None, |
303 | + } |
304 | + self.relation_set.assert_called_with(**expected) |
305 | + |
306 | @patch('os.mkdir') |
307 | def test_ceph_joined(self, mkdir): |
308 | 'It correctly prepares for a ceph changed hook' |
309 | @@ -501,8 +538,10 @@ |
310 | self.assertNotIn(c, self.CONFIGS.write.call_args_list) |
311 | self.assertFalse(self.set_ceph_env_variables.called) |
312 | |
313 | + @patch('charmhelpers.core.host.service') |
314 | @patch("cinder_hooks.relation_get", autospec=True) |
315 | - def test_ceph_changed_broker_success(self, mock_relation_get): |
316 | + def test_ceph_changed_broker_success(self, mock_relation_get, |
317 | + _service): |
318 | 'It ensures ceph assets created on ceph changed' |
319 | self.CONFIGS.complete_contexts.return_value = ['ceph'] |
320 | self.service_name.return_value = 'cinder' |
charm_lint_check #5404 cinder for billy-olsen mp262001
LINT OK: passed
Build: http:// 10.245. 162.77: 8080/job/ charm_lint_ check/5404/