Merge lp:~james-page/charms/trusty/ceph/fix-apt-race into lp:~openstack-charmers-archive/charms/trusty/ceph/trunk

[James Page]

Rework use of package version comparison to use charm-helper and avoid apt races.

Unmerged revisions

=== modified file 'hooks/ceph.py'
--- hooks/ceph.py	2014-05-19 12:15:38 +0000
+++ hooks/ceph.py	2014-07-24 09:53:27 +0000
@@ -11,10 +11,10 @@
 import subprocess
 import time
 import os
-import apt_pkg as apt
 from charmhelpers.core.host import (
     mkdir,
     service_restart,
+    cmp_pkgrevno,
 )
 from charmhelpers.core.hookenv import (
     log,
@@ -126,7 +126,7 @@
 def start_osds(devices):
     # Scan for ceph block devices
     rescan_osd_devices()
-    if get_ceph_version() >= "0.56.6":
+    if cmp_pkgrevno('ceph', "0.56.6") >= 0:
         # Use ceph-disk-activate for directory based OSD's
         for dev_or_path in devices:
             if os.path.exists(dev_or_path) and os.path.isdir(dev_or_path):
@@ -309,20 +309,6 @@
             os.unlink(keyring)
 
 
-def get_ceph_version():
-    apt.init()
-    cache = apt.Cache()
-    pkg = cache['ceph']
-    if pkg.current_ver:
-        return apt.upstream_version(pkg.current_ver.ver_str)
-    else:
-        return None
-
-
-def version_compare(a, b):
-    return apt.version_compare(a, b)
-
-
 def update_monfs():
     hostname = get_unit_hostname()
     monfs = '/var/lib/ceph/mon/ceph-{}'.format(hostname)
@@ -360,7 +346,7 @@
 
     cmd = ['ceph-disk-prepare']
     # Later versions of ceph support more options
-    if get_ceph_version() >= "0.48.3":
+    if cmp_pkgrevno('ceph', "0.48.3") >= 0:
         if osd_format:
             cmd.append('--fs-type')
             cmd.append(osd_format)
@@ -383,7 +369,7 @@
         log('Path {} is already configured as an OSD - bailing'.format(path))
         return
 
-    if get_ceph_version() < "0.56.6":
+    if cmp_pkgrevno('ceph', "0.56.6") < 0:
         log('Unable to use directories for OSDs with ceph < 0.56.6',
             level=ERROR)
         raise

=== modified file 'hooks/charmhelpers/contrib/storage/linux/utils.py'
--- hooks/charmhelpers/contrib/storage/linux/utils.py	2014-05-19 11:38:45 +0000
+++ hooks/charmhelpers/contrib/storage/linux/utils.py	2014-07-24 09:53:27 +0000
@@ -37,6 +37,7 @@
     check_call(['dd', 'if=/dev/zero', 'of=%s' % (block_device),
                 'bs=512', 'count=100', 'seek=%s' % (gpt_end)])
 
+
 def is_device_mounted(device):
     '''Given a device path, return True if that device is mounted, and False
     if it isn't.

=== added file 'hooks/charmhelpers/core/fstab.py'
--- hooks/charmhelpers/core/fstab.py	1970-01-01 00:00:00 +0000
+++ hooks/charmhelpers/core/fstab.py	2014-07-24 09:53:27 +0000
@@ -0,0 +1,116 @@
+#!/usr/bin/env python
+# -*- coding: utf-8 -*-
+
+__author__ = 'Jorge Niedbalski R. <[e-mail]>'
+
+import os
+
+
+class Fstab(file):
+    """This class extends file in order to implement a file reader/writer
+    for file `/etc/fstab`
+    """
+
+    class Entry(object):
+        """Entry class represents a non-comment line on the `/etc/fstab` file
+        """
+        def __init__(self, device, mountpoint, filesystem,
+                     options, d=0, p=0):
+            self.device = device
+            self.mountpoint = mountpoint
+            self.filesystem = filesystem
+
+            if not options:
+                options = "defaults"
+
+            self.options = options
+            self.d = d
+            self.p = p
+
+        def __eq__(self, o):
+            return str(self) == str(o)
+
+        def __str__(self):
+            return "{} {} {} {} {} {}".format(self.device,
+                                              self.mountpoint,
+                                              self.filesystem,
+                                              self.options,
+                                              self.d,
+                                              self.p)
+
+    DEFAULT_PATH = os.path.join(os.path.sep, 'etc', 'fstab')
+
+    def __init__(self, path=None):
+        if path:
+            self._path = path
+        else:
+            self._path = self.DEFAULT_PATH
+        file.__init__(self, self._path, 'r+')
+
+    def _hydrate_entry(self, line):
+        # NOTE: use split with no arguments to split on any
+        #       whitespace including tabs
+        return Fstab.Entry(*filter(
+            lambda x: x not in ('', None),
+            line.strip("\n").split()))
+
+    @property
+    def entries(self):
+        self.seek(0)
+        for line in self.readlines():
+            try:
+                if not line.startswith("#"):
+                    yield self._hydrate_entry(line)
+            except ValueError:
+                pass
+
+    def get_entry_by_attr(self, attr, value):
+        for entry in self.entries:
+            e_attr = getattr(entry, attr)
+            if e_attr == value:
+                return entry
+        return None
+
+    def add_entry(self, entry):
+        if self.get_entry_by_attr('device', entry.device):
+            return False
+
+        self.write(str(entry) + '\n')
+        self.truncate()
+        return entry
+
+    def remove_entry(self, entry):
+        self.seek(0)
+
+        lines = self.readlines()
+
+        found = False
+        for index, line in enumerate(lines):
+            if not line.startswith("#"):
+                if self._hydrate_entry(line) == entry:
+                    found = True
+                    break
+
+        if not found:
+            return False
+
+        lines.remove(line)
+
+        self.seek(0)
+        self.write(''.join(lines))
+        self.truncate()
+        return True
+
+    @classmethod
+    def remove_by_mountpoint(cls, mountpoint, path=None):
+        fstab = cls(path=path)
+        entry = fstab.get_entry_by_attr('mountpoint', mountpoint)
+        if entry:
+            return fstab.remove_entry(entry)
+        return False
+
+    @classmethod
+    def add(cls, device, mountpoint, filesystem, options=None, path=None):
+        return cls(path=path).add_entry(Fstab.Entry(device,
+                                                    mountpoint, filesystem,
+                                                    options=options))

=== modified file 'hooks/charmhelpers/core/hookenv.py'
--- hooks/charmhelpers/core/hookenv.py	2014-05-19 11:38:45 +0000
+++ hooks/charmhelpers/core/hookenv.py	2014-07-24 09:53:27 +0000
@@ -25,7 +25,7 @@
 def cached(func):
     """Cache return values for multiple executions of func + args
 
-    For example:
+    For example::
 
         @cached
         def unit_get(attribute):
@@ -445,18 +445,19 @@
 class Hooks(object):
     """A convenient handler for hook functions.
 
-    Example:
+    Example::
+
         hooks = Hooks()
 
         # register a hook, taking its name from the function name
         @hooks.hook()
         def install():
-            ...
+            pass  # your code here
 
         # register a hook, providing a custom hook name
         @hooks.hook("config-changed")
         def config_changed():
-            ...
+            pass  # your code here
 
         if __name__ == "__main__":
             # execute a hook based on the name the program is called by

=== modified file 'hooks/charmhelpers/core/host.py'
--- hooks/charmhelpers/core/host.py	2014-05-19 11:38:45 +0000
+++ hooks/charmhelpers/core/host.py	2014-07-24 09:53:27 +0000
@@ -12,11 +12,11 @@
 import string
 import subprocess
 import hashlib
-import apt_pkg
 
 from collections import OrderedDict
 
 from hookenv import log
+from fstab import Fstab
 
 
 def service_start(service_name):
@@ -35,7 +35,8 @@
 
 
 def service_reload(service_name, restart_on_failure=False):
-    """Reload a system service, optionally falling back to restart if reload fails"""
+    """Reload a system service, optionally falling back to restart if
+    reload fails"""
     service_result = service('reload', service_name)
     if not service_result and restart_on_failure:
         service_result = service('restart', service_name)
@@ -144,7 +145,19 @@
         target.write(content)
 
 
-def mount(device, mountpoint, options=None, persist=False):
+def fstab_remove(mp):
+    """Remove the given mountpoint entry from /etc/fstab
+    """
+    return Fstab.remove_by_mountpoint(mp)
+
+
+def fstab_add(dev, mp, fs, options=None):
+    """Adds the given device entry to the /etc/fstab file
+    """
+    return Fstab.add(dev, mp, fs, options=options)
+
+
+def mount(device, mountpoint, options=None, persist=False, filesystem="ext3"):
     """Mount a filesystem at a particular mountpoint"""
     cmd_args = ['mount']
     if options is not None:
@@ -155,9 +168,9 @@
     except subprocess.CalledProcessError, e:
         log('Error mounting {} at {}\n{}'.format(device, mountpoint, e.output))
         return False
+
     if persist:
-        # TODO: update fstab
-        pass
+        return fstab_add(device, mountpoint, filesystem, options=options)
     return True
 
 
@@ -169,9 +182,9 @@
     except subprocess.CalledProcessError, e:
         log('Error unmounting {}\n{}'.format(mountpoint, e.output))
         return False
+
     if persist:
-        # TODO: update fstab
-        pass
+        return fstab_remove(mountpoint)
     return True
 
 
@@ -198,13 +211,13 @@
 def restart_on_change(restart_map, stopstart=False):
     """Restart services based on configuration files changing
 
-    This function is used a decorator, for example
+    This function is used a decorator, for example::
 
         @restart_on_change({
             '/etc/ceph/ceph.conf': [ 'cinder-api', 'cinder-volume' ]
             })
         def ceph_client_changed():
-            ...
+            pass  # your code here
 
     In this example, the cinder-api and cinder-volume services
     would be restarted if /etc/ceph/ceph.conf is changed by the
@@ -300,12 +313,19 @@
 
 def cmp_pkgrevno(package, revno, pkgcache=None):
     '''Compare supplied revno with the revno of the installed package
-       1 => Installed revno is greater than supplied arg
-       0 => Installed revno is the same as supplied arg
-      -1 => Installed revno is less than supplied arg
+
+    *  1 => Installed revno is greater than supplied arg
+    *  0 => Installed revno is the same as supplied arg
+    * -1 => Installed revno is less than supplied arg
+
     '''
+    import apt_pkg
     if not pkgcache:
         apt_pkg.init()
+        # Force Apt to build its cache in memory. That way we avoid race
+        # conditions with other applications building the cache in the same
+        # place.
+        apt_pkg.config.set("Dir::Cache::pkgcache", "")
         pkgcache = apt_pkg.Cache()
     pkg = pkgcache[package]
     return apt_pkg.version_compare(pkg.current_ver.ver_str, revno)

=== modified file 'hooks/charmhelpers/fetch/__init__.py'
--- hooks/charmhelpers/fetch/__init__.py	2014-05-19 11:38:45 +0000
+++ hooks/charmhelpers/fetch/__init__.py	2014-07-24 09:53:27 +0000
@@ -13,7 +13,6 @@
     config,
     log,
 )
-import apt_pkg
 import os
 
 
@@ -56,6 +55,15 @@
     'icehouse/proposed': 'precise-proposed/icehouse',
     'precise-icehouse/proposed': 'precise-proposed/icehouse',
     'precise-proposed/icehouse': 'precise-proposed/icehouse',
+    # Juno
+    'juno': 'trusty-updates/juno',
+    'trusty-juno': 'trusty-updates/juno',
+    'trusty-juno/updates': 'trusty-updates/juno',
+    'trusty-updates/juno': 'trusty-updates/juno',
+    'juno/proposed': 'trusty-proposed/juno',
+    'juno/proposed': 'trusty-proposed/juno',
+    'trusty-juno/proposed': 'trusty-proposed/juno',
+    'trusty-proposed/juno': 'trusty-proposed/juno',
 }
 
 # The order of this list is very important. Handlers should be listed in from
@@ -108,6 +116,7 @@
 
 def filter_installed_packages(packages):
     """Returns a list of packages that require installation"""
+    import apt_pkg
     apt_pkg.init()
 
     # Tell apt to build an in-memory cache to prevent race conditions (if
@@ -226,31 +235,39 @@
                       sources_var='install_sources',
                       keys_var='install_keys'):
     """
-    Configure multiple sources from charm configuration
+    Configure multiple sources from charm configuration.
+
+    The lists are encoded as yaml fragments in the configuration.
+    The frament needs to be included as a string.
 
     Example config:
-        install_sources:
+        install_sources: |
           - "ppa:foo"
           - "http://example.com/repo precise main"
-        install_keys:
+        install_keys: |
           - null
           - "a1b2c3d4"
 
     Note that 'null' (a.k.a. None) should not be quoted.
     """
-    sources = safe_load(config(sources_var))
-    keys = config(keys_var)
-    if keys is not None:
-        keys = safe_load(keys)
-    if isinstance(sources, basestring) and (
-            keys is None or isinstance(keys, basestring)):
-        add_source(sources, keys)
+    sources = safe_load((config(sources_var) or '').strip()) or []
+    keys = safe_load((config(keys_var) or '').strip()) or None
+
+    if isinstance(sources, basestring):
+        sources = [sources]
+
+    if keys is None:
+        for source in sources:
+            add_source(source, None)
     else:
-        if not len(sources) == len(keys):
-            msg = 'Install sources and keys lists are different lengths'
-            raise SourceConfigError(msg)
-        for src_num in range(len(sources)):
-            add_source(sources[src_num], keys[src_num])
+        if isinstance(keys, basestring):
+            keys = [keys]
+
+        if len(sources) != len(keys):
+            raise SourceConfigError(
+                'Install sources and keys lists are different lengths')
+        for source, key in zip(sources, keys):
+            add_source(source, key)
     if update:
         apt_update(fatal=True)
 

=== modified file 'hooks/charmhelpers/fetch/bzrurl.py'
--- hooks/charmhelpers/fetch/bzrurl.py	2013-11-13 22:09:26 +0000
+++ hooks/charmhelpers/fetch/bzrurl.py	2014-07-24 09:53:27 +0000
@@ -39,7 +39,8 @@
     def install(self, source):
         url_parts = self.parse_url(source)
         branch_name = url_parts.path.strip("/").split("/")[-1]
-        dest_dir = os.path.join(os.environ.get('CHARM_DIR'), "fetched", branch_name)
+        dest_dir = os.path.join(os.environ.get('CHARM_DIR'), "fetched",
+                                branch_name)
         if not os.path.exists(dest_dir):
             mkdir(dest_dir, perms=0755)
         try:

=== modified file 'hooks/hooks.py'
--- hooks/hooks.py	2014-03-25 18:44:22 +0000
+++ hooks/hooks.py	2014-07-24 09:53:27 +0000
@@ -29,7 +29,8 @@
 from charmhelpers.core.host import (
     service_restart,
     umount,
-    mkdir
+    mkdir,
+    cmp_pkgrevno
 )
 from charmhelpers.fetch import (
     apt_install,
@@ -50,7 +51,7 @@
 
 def install_upstart_scripts():
     # Only install upstart configurations for older versions
-    if ceph.get_ceph_version() < "0.55.1":
+    if cmp_pkgrevno('ceph', "0.55.1") < 0:
         for x in glob.glob('files/upstart/*.conf'):
             shutil.copy(x, '/etc/init/')
 
@@ -71,7 +72,7 @@
         'auth_supported': config('auth-supported'),
         'mon_hosts': ' '.join(get_mon_hosts()),
         'fsid': config('fsid'),
-        'version': ceph.get_ceph_version(),
+        'old_auth': cmp_pkgrevno('ceph', "0.51") < 0,
         'osd_journal_size': config('osd-journal-size'),
         'use_syslog': str(config('use-syslog')).lower()
     }

=== modified file 'templates/ceph.conf'
--- templates/ceph.conf	2014-03-25 18:44:22 +0000
+++ templates/ceph.conf	2014-07-24 09:53:27 +0000
@@ -1,5 +1,5 @@
 [global]
-{% if version < "0.51" %}
+{% if old_auth %}
  auth supported = {{ auth_supported }}
 {% else %}
  auth cluster required = {{ auth_supported }}