Merge lp:~james-page/charms/trusty/ceph-radosgw/fix-apt-race into lp:~openstack-charmers-archive/charms/trusty/ceph-radosgw/next
- Trusty Tahr (14.04)
- fix-apt-race
- Merge into next
Proposed by
James Page
Status: | Merged |
---|---|
Merged at revision: | 24 |
Proposed branch: | lp:~james-page/charms/trusty/ceph-radosgw/fix-apt-race |
Merge into: | lp:~openstack-charmers-archive/charms/trusty/ceph-radosgw/next |
Diff against target: |
450 lines (+185/-56) 9 files modified
hooks/ceph.py (+0/-19) hooks/charmhelpers/contrib/storage/linux/utils.py (+1/-0) hooks/charmhelpers/core/fstab.py (+116/-0) hooks/charmhelpers/core/hookenv.py (+5/-4) hooks/charmhelpers/core/host.py (+32/-12) hooks/charmhelpers/fetch/__init__.py (+24/-16) hooks/charmhelpers/fetch/bzrurl.py (+2/-1) hooks/hooks.py (+4/-3) templates/ceph.conf (+1/-1) |
To merge this branch: | bzr merge lp:~james-page/charms/trusty/ceph-radosgw/fix-apt-race |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Liam Young (community) | Approve | ||
Review via email: mp+228069@code.launchpad.net |
Commit message
Description of the change
To post a comment you must log in.
Preview Diff
[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1 | === modified file 'hooks/ceph.py' |
2 | --- hooks/ceph.py 2014-07-23 07:54:11 +0000 |
3 | +++ hooks/ceph.py 2014-07-24 09:55:08 +0000 |
4 | @@ -11,7 +11,6 @@ |
5 | import subprocess |
6 | import time |
7 | import os |
8 | -import apt_pkg |
9 | |
10 | from socket import gethostname as get_unit_hostname |
11 | |
12 | @@ -220,21 +219,3 @@ |
13 | if 'key' in element: |
14 | key = element.split(' = ')[1].strip() # IGNORE:E1103 |
15 | return key |
16 | - |
17 | - |
18 | -def get_ceph_version(package=None): |
19 | - apt_pkg.init() |
20 | - # Force Apt to build its cache in memory. That way we avoid race |
21 | - # conditions with other applications building the cache in the same |
22 | - # place. |
23 | - apt_pkg.config.set("Dir::Cache::pkgcache", "") |
24 | - cache = apt_pkg.Cache() |
25 | - pkg = cache[package or 'ceph'] |
26 | - if pkg.current_ver: |
27 | - return apt_pkg.upstream_version(pkg.current_ver.ver_str) |
28 | - else: |
29 | - return None |
30 | - |
31 | - |
32 | -def version_compare(a, b): |
33 | - return apt_pkg.version_compare(a, b) |
34 | |
35 | === modified file 'hooks/charmhelpers/contrib/storage/linux/utils.py' |
36 | --- hooks/charmhelpers/contrib/storage/linux/utils.py 2014-05-19 11:37:27 +0000 |
37 | +++ hooks/charmhelpers/contrib/storage/linux/utils.py 2014-07-24 09:55:08 +0000 |
38 | @@ -37,6 +37,7 @@ |
39 | check_call(['dd', 'if=/dev/zero', 'of=%s' % (block_device), |
40 | 'bs=512', 'count=100', 'seek=%s' % (gpt_end)]) |
41 | |
42 | + |
43 | def is_device_mounted(device): |
44 | '''Given a device path, return True if that device is mounted, and False |
45 | if it isn't. |
46 | |
47 | === added file 'hooks/charmhelpers/core/fstab.py' |
48 | --- hooks/charmhelpers/core/fstab.py 1970-01-01 00:00:00 +0000 |
49 | +++ hooks/charmhelpers/core/fstab.py 2014-07-24 09:55:08 +0000 |
50 | @@ -0,0 +1,116 @@ |
51 | +#!/usr/bin/env python |
52 | +# -*- coding: utf-8 -*- |
53 | + |
54 | +__author__ = 'Jorge Niedbalski R. <jorge.niedbalski@canonical.com>' |
55 | + |
56 | +import os |
57 | + |
58 | + |
59 | +class Fstab(file): |
60 | + """This class extends file in order to implement a file reader/writer |
61 | + for file `/etc/fstab` |
62 | + """ |
63 | + |
64 | + class Entry(object): |
65 | + """Entry class represents a non-comment line on the `/etc/fstab` file |
66 | + """ |
67 | + def __init__(self, device, mountpoint, filesystem, |
68 | + options, d=0, p=0): |
69 | + self.device = device |
70 | + self.mountpoint = mountpoint |
71 | + self.filesystem = filesystem |
72 | + |
73 | + if not options: |
74 | + options = "defaults" |
75 | + |
76 | + self.options = options |
77 | + self.d = d |
78 | + self.p = p |
79 | + |
80 | + def __eq__(self, o): |
81 | + return str(self) == str(o) |
82 | + |
83 | + def __str__(self): |
84 | + return "{} {} {} {} {} {}".format(self.device, |
85 | + self.mountpoint, |
86 | + self.filesystem, |
87 | + self.options, |
88 | + self.d, |
89 | + self.p) |
90 | + |
91 | + DEFAULT_PATH = os.path.join(os.path.sep, 'etc', 'fstab') |
92 | + |
93 | + def __init__(self, path=None): |
94 | + if path: |
95 | + self._path = path |
96 | + else: |
97 | + self._path = self.DEFAULT_PATH |
98 | + file.__init__(self, self._path, 'r+') |
99 | + |
100 | + def _hydrate_entry(self, line): |
101 | + # NOTE: use split with no arguments to split on any |
102 | + # whitespace including tabs |
103 | + return Fstab.Entry(*filter( |
104 | + lambda x: x not in ('', None), |
105 | + line.strip("\n").split())) |
106 | + |
107 | + @property |
108 | + def entries(self): |
109 | + self.seek(0) |
110 | + for line in self.readlines(): |
111 | + try: |
112 | + if not line.startswith("#"): |
113 | + yield self._hydrate_entry(line) |
114 | + except ValueError: |
115 | + pass |
116 | + |
117 | + def get_entry_by_attr(self, attr, value): |
118 | + for entry in self.entries: |
119 | + e_attr = getattr(entry, attr) |
120 | + if e_attr == value: |
121 | + return entry |
122 | + return None |
123 | + |
124 | + def add_entry(self, entry): |
125 | + if self.get_entry_by_attr('device', entry.device): |
126 | + return False |
127 | + |
128 | + self.write(str(entry) + '\n') |
129 | + self.truncate() |
130 | + return entry |
131 | + |
132 | + def remove_entry(self, entry): |
133 | + self.seek(0) |
134 | + |
135 | + lines = self.readlines() |
136 | + |
137 | + found = False |
138 | + for index, line in enumerate(lines): |
139 | + if not line.startswith("#"): |
140 | + if self._hydrate_entry(line) == entry: |
141 | + found = True |
142 | + break |
143 | + |
144 | + if not found: |
145 | + return False |
146 | + |
147 | + lines.remove(line) |
148 | + |
149 | + self.seek(0) |
150 | + self.write(''.join(lines)) |
151 | + self.truncate() |
152 | + return True |
153 | + |
154 | + @classmethod |
155 | + def remove_by_mountpoint(cls, mountpoint, path=None): |
156 | + fstab = cls(path=path) |
157 | + entry = fstab.get_entry_by_attr('mountpoint', mountpoint) |
158 | + if entry: |
159 | + return fstab.remove_entry(entry) |
160 | + return False |
161 | + |
162 | + @classmethod |
163 | + def add(cls, device, mountpoint, filesystem, options=None, path=None): |
164 | + return cls(path=path).add_entry(Fstab.Entry(device, |
165 | + mountpoint, filesystem, |
166 | + options=options)) |
167 | |
168 | === modified file 'hooks/charmhelpers/core/hookenv.py' |
169 | --- hooks/charmhelpers/core/hookenv.py 2014-05-19 11:37:27 +0000 |
170 | +++ hooks/charmhelpers/core/hookenv.py 2014-07-24 09:55:08 +0000 |
171 | @@ -25,7 +25,7 @@ |
172 | def cached(func): |
173 | """Cache return values for multiple executions of func + args |
174 | |
175 | - For example: |
176 | + For example:: |
177 | |
178 | @cached |
179 | def unit_get(attribute): |
180 | @@ -445,18 +445,19 @@ |
181 | class Hooks(object): |
182 | """A convenient handler for hook functions. |
183 | |
184 | - Example: |
185 | + Example:: |
186 | + |
187 | hooks = Hooks() |
188 | |
189 | # register a hook, taking its name from the function name |
190 | @hooks.hook() |
191 | def install(): |
192 | - ... |
193 | + pass # your code here |
194 | |
195 | # register a hook, providing a custom hook name |
196 | @hooks.hook("config-changed") |
197 | def config_changed(): |
198 | - ... |
199 | + pass # your code here |
200 | |
201 | if __name__ == "__main__": |
202 | # execute a hook based on the name the program is called by |
203 | |
204 | === modified file 'hooks/charmhelpers/core/host.py' |
205 | --- hooks/charmhelpers/core/host.py 2014-05-19 11:37:27 +0000 |
206 | +++ hooks/charmhelpers/core/host.py 2014-07-24 09:55:08 +0000 |
207 | @@ -12,11 +12,11 @@ |
208 | import string |
209 | import subprocess |
210 | import hashlib |
211 | -import apt_pkg |
212 | |
213 | from collections import OrderedDict |
214 | |
215 | from hookenv import log |
216 | +from fstab import Fstab |
217 | |
218 | |
219 | def service_start(service_name): |
220 | @@ -35,7 +35,8 @@ |
221 | |
222 | |
223 | def service_reload(service_name, restart_on_failure=False): |
224 | - """Reload a system service, optionally falling back to restart if reload fails""" |
225 | + """Reload a system service, optionally falling back to restart if |
226 | + reload fails""" |
227 | service_result = service('reload', service_name) |
228 | if not service_result and restart_on_failure: |
229 | service_result = service('restart', service_name) |
230 | @@ -144,7 +145,19 @@ |
231 | target.write(content) |
232 | |
233 | |
234 | -def mount(device, mountpoint, options=None, persist=False): |
235 | +def fstab_remove(mp): |
236 | + """Remove the given mountpoint entry from /etc/fstab |
237 | + """ |
238 | + return Fstab.remove_by_mountpoint(mp) |
239 | + |
240 | + |
241 | +def fstab_add(dev, mp, fs, options=None): |
242 | + """Adds the given device entry to the /etc/fstab file |
243 | + """ |
244 | + return Fstab.add(dev, mp, fs, options=options) |
245 | + |
246 | + |
247 | +def mount(device, mountpoint, options=None, persist=False, filesystem="ext3"): |
248 | """Mount a filesystem at a particular mountpoint""" |
249 | cmd_args = ['mount'] |
250 | if options is not None: |
251 | @@ -155,9 +168,9 @@ |
252 | except subprocess.CalledProcessError, e: |
253 | log('Error mounting {} at {}\n{}'.format(device, mountpoint, e.output)) |
254 | return False |
255 | + |
256 | if persist: |
257 | - # TODO: update fstab |
258 | - pass |
259 | + return fstab_add(device, mountpoint, filesystem, options=options) |
260 | return True |
261 | |
262 | |
263 | @@ -169,9 +182,9 @@ |
264 | except subprocess.CalledProcessError, e: |
265 | log('Error unmounting {}\n{}'.format(mountpoint, e.output)) |
266 | return False |
267 | + |
268 | if persist: |
269 | - # TODO: update fstab |
270 | - pass |
271 | + return fstab_remove(mountpoint) |
272 | return True |
273 | |
274 | |
275 | @@ -198,13 +211,13 @@ |
276 | def restart_on_change(restart_map, stopstart=False): |
277 | """Restart services based on configuration files changing |
278 | |
279 | - This function is used a decorator, for example |
280 | + This function is used a decorator, for example:: |
281 | |
282 | @restart_on_change({ |
283 | '/etc/ceph/ceph.conf': [ 'cinder-api', 'cinder-volume' ] |
284 | }) |
285 | def ceph_client_changed(): |
286 | - ... |
287 | + pass # your code here |
288 | |
289 | In this example, the cinder-api and cinder-volume services |
290 | would be restarted if /etc/ceph/ceph.conf is changed by the |
291 | @@ -300,12 +313,19 @@ |
292 | |
293 | def cmp_pkgrevno(package, revno, pkgcache=None): |
294 | '''Compare supplied revno with the revno of the installed package |
295 | - 1 => Installed revno is greater than supplied arg |
296 | - 0 => Installed revno is the same as supplied arg |
297 | - -1 => Installed revno is less than supplied arg |
298 | + |
299 | + * 1 => Installed revno is greater than supplied arg |
300 | + * 0 => Installed revno is the same as supplied arg |
301 | + * -1 => Installed revno is less than supplied arg |
302 | + |
303 | ''' |
304 | + import apt_pkg |
305 | if not pkgcache: |
306 | apt_pkg.init() |
307 | + # Force Apt to build its cache in memory. That way we avoid race |
308 | + # conditions with other applications building the cache in the same |
309 | + # place. |
310 | + apt_pkg.config.set("Dir::Cache::pkgcache", "") |
311 | pkgcache = apt_pkg.Cache() |
312 | pkg = pkgcache[package] |
313 | return apt_pkg.version_compare(pkg.current_ver.ver_str, revno) |
314 | |
315 | === modified file 'hooks/charmhelpers/fetch/__init__.py' |
316 | --- hooks/charmhelpers/fetch/__init__.py 2014-06-04 13:07:17 +0000 |
317 | +++ hooks/charmhelpers/fetch/__init__.py 2014-07-24 09:55:08 +0000 |
318 | @@ -13,7 +13,6 @@ |
319 | config, |
320 | log, |
321 | ) |
322 | -import apt_pkg |
323 | import os |
324 | |
325 | |
326 | @@ -117,6 +116,7 @@ |
327 | |
328 | def filter_installed_packages(packages): |
329 | """Returns a list of packages that require installation""" |
330 | + import apt_pkg |
331 | apt_pkg.init() |
332 | |
333 | # Tell apt to build an in-memory cache to prevent race conditions (if |
334 | @@ -235,31 +235,39 @@ |
335 | sources_var='install_sources', |
336 | keys_var='install_keys'): |
337 | """ |
338 | - Configure multiple sources from charm configuration |
339 | + Configure multiple sources from charm configuration. |
340 | + |
341 | + The lists are encoded as yaml fragments in the configuration. |
342 | + The frament needs to be included as a string. |
343 | |
344 | Example config: |
345 | - install_sources: |
346 | + install_sources: | |
347 | - "ppa:foo" |
348 | - "http://example.com/repo precise main" |
349 | - install_keys: |
350 | + install_keys: | |
351 | - null |
352 | - "a1b2c3d4" |
353 | |
354 | Note that 'null' (a.k.a. None) should not be quoted. |
355 | """ |
356 | - sources = safe_load(config(sources_var)) |
357 | - keys = config(keys_var) |
358 | - if keys is not None: |
359 | - keys = safe_load(keys) |
360 | - if isinstance(sources, basestring) and ( |
361 | - keys is None or isinstance(keys, basestring)): |
362 | - add_source(sources, keys) |
363 | + sources = safe_load((config(sources_var) or '').strip()) or [] |
364 | + keys = safe_load((config(keys_var) or '').strip()) or None |
365 | + |
366 | + if isinstance(sources, basestring): |
367 | + sources = [sources] |
368 | + |
369 | + if keys is None: |
370 | + for source in sources: |
371 | + add_source(source, None) |
372 | else: |
373 | - if not len(sources) == len(keys): |
374 | - msg = 'Install sources and keys lists are different lengths' |
375 | - raise SourceConfigError(msg) |
376 | - for src_num in range(len(sources)): |
377 | - add_source(sources[src_num], keys[src_num]) |
378 | + if isinstance(keys, basestring): |
379 | + keys = [keys] |
380 | + |
381 | + if len(sources) != len(keys): |
382 | + raise SourceConfigError( |
383 | + 'Install sources and keys lists are different lengths') |
384 | + for source, key in zip(sources, keys): |
385 | + add_source(source, key) |
386 | if update: |
387 | apt_update(fatal=True) |
388 | |
389 | |
390 | === modified file 'hooks/charmhelpers/fetch/bzrurl.py' |
391 | --- hooks/charmhelpers/fetch/bzrurl.py 2014-01-24 16:02:57 +0000 |
392 | +++ hooks/charmhelpers/fetch/bzrurl.py 2014-07-24 09:55:08 +0000 |
393 | @@ -39,7 +39,8 @@ |
394 | def install(self, source): |
395 | url_parts = self.parse_url(source) |
396 | branch_name = url_parts.path.strip("/").split("/")[-1] |
397 | - dest_dir = os.path.join(os.environ.get('CHARM_DIR'), "fetched", branch_name) |
398 | + dest_dir = os.path.join(os.environ.get('CHARM_DIR'), "fetched", |
399 | + branch_name) |
400 | if not os.path.exists(dest_dir): |
401 | mkdir(dest_dir, perms=0755) |
402 | try: |
403 | |
404 | === modified file 'hooks/hooks.py' |
405 | --- hooks/hooks.py 2014-03-25 18:44:22 +0000 |
406 | +++ hooks/hooks.py 2014-07-24 09:55:08 +0000 |
407 | @@ -38,6 +38,7 @@ |
408 | ) |
409 | |
410 | from charmhelpers.payload.execd import execd_preinstall |
411 | +from charmhelpers.core.host import cmp_pkgrevno |
412 | from socket import gethostname as get_unit_hostname |
413 | |
414 | hooks = Hooks() |
415 | @@ -73,13 +74,13 @@ |
416 | 'auth_supported': get_auth() or 'none', |
417 | 'mon_hosts': ' '.join(get_mon_hosts()), |
418 | 'hostname': get_unit_hostname(), |
419 | - 'version': ceph.get_ceph_version('radosgw'), |
420 | + 'old_auth': cmp_pkgrevno('radosgw', "0.51") < 0, |
421 | 'use_syslog': str(config('use-syslog')).lower() |
422 | } |
423 | |
424 | # Check to ensure that correct version of ceph is |
425 | # in use |
426 | - if ceph.get_ceph_version('radosgw') >= "0.55": |
427 | + if cmp_pkgrevno('radosgw', '0.55') >= 0: |
428 | # Add keystone configuration if found |
429 | ks_conf = get_keystone_conf() |
430 | if ks_conf: |
431 | @@ -208,7 +209,7 @@ |
432 | |
433 | @hooks.hook('identity-service-relation-joined') |
434 | def identity_joined(relid=None): |
435 | - if ceph.get_ceph_version('radosgw') < "0.55": |
436 | + if cmp_pkgrevno('radosgw', '0.55') < 0: |
437 | log('Integration with keystone requires ceph >= 0.55') |
438 | sys.exit(1) |
439 | |
440 | |
441 | === modified file 'templates/ceph.conf' |
442 | --- templates/ceph.conf 2014-03-25 18:44:22 +0000 |
443 | +++ templates/ceph.conf 2014-07-24 09:55:08 +0000 |
444 | @@ -1,5 +1,5 @@ |
445 | [global] |
446 | -{% if version < "0.51" %} |
447 | +{% if old_auth %} |
448 | auth supported = {{ auth_supported }} |
449 | {% else %} |
450 | auth cluster required = {{ auth_supported }} |
LGTM