Merge lp:~gnuoy/charms/trusty/hacluster/unicast-support into lp:~openstack-charmers/charms/trusty/hacluster/next

[Liam Young]

* Add unicast support
* Add decorator to limit number of corosync restarts
* Shutdown corosync and pacemaker services when unit leaves cluster otherwise services can continue to run on the departed nodes

[Xiang Hui (xianghui)]

Some comments inside, and is the case that switching between multicast and unicast transport mode considered?

[James Page (james-page)]

Some more feedback; Hui - I think the change deals with switching between modes as corosync gets reconfigured in the config-changed hook.

review: Needs Fixing

[Xiang Hui (xianghui)]

> Some more feedback; Hui - I think the change deals with switching between
> modes as corosync gets reconfigured in the config-changed hook.

James, I see, thanks : )

[lp:~gnuoy/charms/trusty/hacluster/unicast-support]

38. By Liam Young on 2014-09-17

Fixed lint
Made config_corosync run unconditionally on config-changed since it is protected by the conditional_corosync_restart decorator.
Fixed typo s/conditional_corosyc_restart/conditional_corosync_restart/
Added None defaults to config.yaml to make charm proof happy

[Liam Young (gnuoy)]

Thanks for the reviews. I've updated the branch takking into account the suggestions made and have retested it.

[Ante Karamatić (ivoks)]

When using with MAAS as a backend, corosync.conf gets populated with hostnames instead of nodes' IPs. This results in broken cluster cause nodes can't really talk to each other. I've seen a case where one node is defined by hostname, and the other one by IP.

[James Page (james-page)]

Sounds like we need the good-ole get_host_ip translation to deal with MAAS hostnames from juju; when is an address not an address? why of course - when its the MAAS provider!

[Liam Young (gnuoy)]

Superseded by https://code.launchpad.net/~gnuoy/charms/trusty/hacluster/unicast-support-take2/+merge/238124

review: Disapprove

[Edward Hope-Morley (hopem)]

This has been superseded by the (now merged) https://code.launchpad.net/~gnuoy/charms/trusty/hacluster/unicast-support-take2/+merge/238124 which was released in 15.01

Unmerged revisions

38. By Liam Young on 2014-09-17

Fixed lint
Made config_corosync run unconditionally on config-changed since it is protected by the conditional_corosync_restart decorator.
Fixed typo s/conditional_corosyc_restart/conditional_corosync_restart/
Added None defaults to config.yaml to make charm proof happy

37. By Liam Young on 2014-07-29

Add standard smarts to charmhelpers sync in Makefile. Sync charmhelpers

36. By Liam Young on 2014-07-29

Fix bug in peer_ips

35. By Liam Young on 2014-07-29

Maka corosync_transport options match the corosync.conf transport directive (ie use udpu rather than unicast). Support switching corosync_transport modes

34. By Liam Young on 2014-07-27

Fix lint

33. By Liam Young on 2014-07-27

Add unicast support to hacluster charm

32. By James Page on 2014-04-16

Fixup abuse of relation_set during redux

31. By James Page on 2014-04-11

Fixup unicode string handling

30. By James Page on 2014-04-11

Fixup use of relation_*

29. By James Page on 2014-04-11

Add icon and category

=== added file '.bzrignore'
--- .bzrignore	1970-01-01 00:00:00 +0000
+++ .bzrignore	2014-09-17 13:16:52 +0000
@@ -0,0 +1,2 @@
+bin
+revision

=== modified file 'Makefile'
--- Makefile	2014-04-11 09:57:50 +0000
+++ Makefile	2014-09-17 13:16:52 +0000
@@ -9,5 +9,10 @@
 	@echo Starting tests...
 	@$(PYTHON) /usr/bin/nosetests --nologcapture --with-coverage  unit_tests
 
-sync:
-	@charm-helper-sync -c charm-helpers.yaml
+bin/charm_helpers_sync.py:
+	@mkdir -p bin
+	@bzr cat lp:charm-helpers/tools/charm_helpers_sync/charm_helpers_sync.py \
+	> bin/charm_helpers_sync.py
+
+sync: bin/charm_helpers_sync.py
+	@$(PYTHON) bin/charm_helpers_sync.py -c charm-helpers.yaml

=== modified file 'config.yaml'
--- config.yaml	2014-04-11 10:25:09 +0000
+++ config.yaml	2014-09-17 13:16:52 +0000
@@ -18,6 +18,11 @@
       .
       This configuration element is mandatory and the service will fail on
       install if it is not provided.  The value must be base64 encoded.
+  corosync_transport:
+    type: string
+    default: "udp"
+    description: |
+      Two supported modes are udp (multicast) or udpu (unicast)
   stonith_enabled:
     type: string
     default: 'False'
@@ -27,9 +32,11 @@
      parameters are properly configured in its invenvory.
   maas_url:
     type: string
+    default:
     description: MAAS API endpoint (required for STONITH).
   maas_credentials:
     type: string
+    default:
     description: MAAS credentials (required for STONITH).
   cluster_count:
     type: int
@@ -37,6 +44,7 @@
     description: Number of peer units required to bootstrap cluster services.
   monitor_host:
     type: string
+    default:
     description: |
      One or more IPs, separated by space, that will be used as a saftey check
      for avoiding split brain situations. Nodes in the cluster will ping these

=== modified file 'hooks/charmhelpers/contrib/hahelpers/cluster.py'
--- hooks/charmhelpers/contrib/hahelpers/cluster.py	2014-04-11 11:02:09 +0000
+++ hooks/charmhelpers/contrib/hahelpers/cluster.py	2014-09-17 13:16:52 +0000
@@ -62,6 +62,15 @@
     return peers
 
 
+def peer_ips(peer_relation='cluster', addr_key='private-address'):
+    '''Return a dict of peers and their private-address'''
+    peers = {}
+    for r_id in relation_ids(peer_relation):
+        for unit in relation_list(r_id):
+            peers[unit] = relation_get(addr_key, rid=r_id, unit=unit)
+    return peers
+
+
 def oldest_peer(peers):
     local_unit_no = int(os.getenv('JUJU_UNIT_NAME').split('/')[1])
     for peer in peers:
@@ -146,12 +155,12 @@
     Obtains all relevant configuration from charm configuration required
     for initiating a relation to hacluster:
 
-        ha-bindiface, ha-mcastport, vip, vip_iface, vip_cidr
+        ha-bindiface, ha-mcastport, vip
 
     returns: dict: A dict containing settings keyed by setting name.
     raises: HAIncompleteConfig if settings are missing.
     '''
-    settings = ['ha-bindiface', 'ha-mcastport', 'vip', 'vip_iface', 'vip_cidr']
+    settings = ['ha-bindiface', 'ha-mcastport', 'vip']
     conf = {}
     for setting in settings:
         conf[setting] = config_get(setting)
@@ -170,6 +179,7 @@
 
     :configs    : OSTemplateRenderer: A config tempating object to inspect for
                                       a complete https context.
+
     :vip_setting:                str: Setting in charm config that specifies
                                       VIP address.
     '''

=== modified file 'hooks/charmhelpers/contrib/storage/linux/ceph.py'
--- hooks/charmhelpers/contrib/storage/linux/ceph.py	2014-04-11 11:02:09 +0000
+++ hooks/charmhelpers/contrib/storage/linux/ceph.py	2014-09-17 13:16:52 +0000
@@ -303,7 +303,7 @@
                         blk_device, fstype, system_services=[]):
     """
     NOTE: This function must only be called from a single service unit for
-          the same rbd_img otherwise data loss will occur.
+    the same rbd_img otherwise data loss will occur.
 
     Ensures given pool and RBD image exists, is mapped to a block device,
     and the device is formatted and mounted at the given mount_point.

=== modified file 'hooks/charmhelpers/contrib/storage/linux/lvm.py'
--- hooks/charmhelpers/contrib/storage/linux/lvm.py	2014-04-11 11:02:09 +0000
+++ hooks/charmhelpers/contrib/storage/linux/lvm.py	2014-09-17 13:16:52 +0000
@@ -62,7 +62,7 @@
     pvd = check_output(['pvdisplay', block_device]).splitlines()
     for l in pvd:
         if l.strip().startswith('VG Name'):
-            vg = ' '.join(l.split()).split(' ').pop()
+            vg = ' '.join(l.strip().split()[2:])
     return vg
 
 

=== modified file 'hooks/charmhelpers/contrib/storage/linux/utils.py'
--- hooks/charmhelpers/contrib/storage/linux/utils.py	2014-04-11 11:02:09 +0000
+++ hooks/charmhelpers/contrib/storage/linux/utils.py	2014-09-17 13:16:52 +0000
@@ -1,8 +1,11 @@
-from os import stat
+import os
+import re
 from stat import S_ISBLK
 
 from subprocess import (
-    check_call
+    check_call,
+    check_output,
+    call
 )
 
 
@@ -12,7 +15,9 @@
 
     :returns: boolean: True if path is a block device, False if not.
     '''
-    return S_ISBLK(stat(path).st_mode)
+    if not os.path.exists(path):
+        return False
+    return S_ISBLK(os.stat(path).st_mode)
 
 
 def zap_disk(block_device):
@@ -22,5 +27,24 @@
 
     :param block_device: str: Full path of block device to clean.
     '''
-    check_call(['sgdisk', '--zap-all', '--clear',
-                '--mbrtogpt', block_device])
+    # sometimes sgdisk exits non-zero; this is OK, dd will clean up
+    call(['sgdisk', '--zap-all', '--mbrtogpt',
+          '--clear', block_device])
+    dev_end = check_output(['blockdev', '--getsz', block_device])
+    gpt_end = int(dev_end.split()[0]) - 100
+    check_call(['dd', 'if=/dev/zero', 'of=%s' % (block_device),
+                'bs=1M', 'count=1'])
+    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.
+
+    :param device: str: Full path of the device to check.
+    :returns: boolean: True if the path represents a mounted device, False if
+        it doesn't.
+    '''
+    out = check_output(['mount'])
+    return bool(re.search(device + r"[0-9]+\b", out))

=== 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-09-17 13:16:52 +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-04-11 11:02:09 +0000
+++ hooks/charmhelpers/core/hookenv.py	2014-09-17 13:16:52 +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):
@@ -155,6 +155,100 @@
     return os.path.basename(sys.argv[0])
 
 
+class Config(dict):
+    """A Juju charm config dictionary that can write itself to
+    disk (as json) and track which values have changed since
+    the previous hook invocation.
+
+    Do not instantiate this object directly - instead call
+    ``hookenv.config()``
+
+    Example usage::
+
+        >>> # inside a hook
+        >>> from charmhelpers.core import hookenv
+        >>> config = hookenv.config()
+        >>> config['foo']
+        'bar'
+        >>> config['mykey'] = 'myval'
+        >>> config.save()
+
+
+        >>> # user runs `juju set mycharm foo=baz`
+        >>> # now we're inside subsequent config-changed hook
+        >>> config = hookenv.config()
+        >>> config['foo']
+        'baz'
+        >>> # test to see if this val has changed since last hook
+        >>> config.changed('foo')
+        True
+        >>> # what was the previous value?
+        >>> config.previous('foo')
+        'bar'
+        >>> # keys/values that we add are preserved across hooks
+        >>> config['mykey']
+        'myval'
+        >>> # don't forget to save at the end of hook!
+        >>> config.save()
+
+    """
+    CONFIG_FILE_NAME = '.juju-persistent-config'
+
+    def __init__(self, *args, **kw):
+        super(Config, self).__init__(*args, **kw)
+        self._prev_dict = None
+        self.path = os.path.join(charm_dir(), Config.CONFIG_FILE_NAME)
+        if os.path.exists(self.path):
+            self.load_previous()
+
+    def load_previous(self, path=None):
+        """Load previous copy of config from disk so that current values
+        can be compared to previous values.
+
+        :param path:
+
+            File path from which to load the previous config. If `None`,
+            config is loaded from the default location. If `path` is
+            specified, subsequent `save()` calls will write to the same
+            path.
+
+        """
+        self.path = path or self.path
+        with open(self.path) as f:
+            self._prev_dict = json.load(f)
+
+    def changed(self, key):
+        """Return true if the value for this key has changed since
+        the last save.
+
+        """
+        if self._prev_dict is None:
+            return True
+        return self.previous(key) != self.get(key)
+
+    def previous(self, key):
+        """Return previous value for this key, or None if there
+        is no "previous" value.
+
+        """
+        if self._prev_dict:
+            return self._prev_dict.get(key)
+        return None
+
+    def save(self):
+        """Save this config to disk.
+
+        Preserves items in _prev_dict that do not exist in self.
+
+        """
+        if self._prev_dict:
+            for k, v in self._prev_dict.iteritems():
+                if k not in self:
+                    self[k] = v
+        with open(self.path, 'w') as f:
+            json.dump(self, f)
+
+
 @cached
 def config(scope=None):
     """Juju charm configuration"""
@@ -163,7 +257,10 @@
         config_cmd_line.append(scope)
     config_cmd_line.append('--format=json')
     try:
-        return json.loads(subprocess.check_output(config_cmd_line))
+        config_data = json.loads(subprocess.check_output(config_cmd_line))
+        if scope is not None:
+            return config_data
+        return Config(config_data)
     except ValueError:
         return None
 
@@ -348,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-04-11 11:02:09 +0000
+++ hooks/charmhelpers/core/host.py	2014-09-17 13:16:52 +0000
@@ -16,6 +16,7 @@
 from collections import OrderedDict
 
 from hookenv import log
+from fstab import Fstab
 
 
 def service_start(service_name):
@@ -34,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)
@@ -143,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:
@@ -154,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
 
 
@@ -168,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
 
 
@@ -197,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
@@ -295,3 +309,23 @@
     if 'link/ether' in words:
         hwaddr = words[words.index('link/ether') + 1]
     return hwaddr
+
+
+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
+
+    '''
+    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-04-11 11:02:19 +0000
+++ hooks/charmhelpers/fetch/__init__.py	2014-09-17 13:16:52 +0000
@@ -1,4 +1,5 @@
 import importlib
+import time
 from yaml import safe_load
 from charmhelpers.core.host import (
     lsb_release
@@ -12,9 +13,9 @@
     config,
     log,
 )
-import apt_pkg
 import os
 
+
 CLOUD_ARCHIVE = """# Ubuntu Cloud Archive
 deb http://ubuntu-cloud.archive.canonical.com/ubuntu {} main
 """
@@ -54,12 +55,74 @@
     '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
+# least- to most-specific URL matching.
+FETCH_HANDLERS = (
+    'charmhelpers.fetch.archiveurl.ArchiveUrlFetchHandler',
+    'charmhelpers.fetch.bzrurl.BzrUrlFetchHandler',
+)
+
+APT_NO_LOCK = 100  # The return code for "couldn't acquire lock" in APT.
+APT_NO_LOCK_RETRY_DELAY = 10  # Wait 10 seconds between apt lock checks.
+APT_NO_LOCK_RETRY_COUNT = 30  # Retry to acquire the lock X times.
+
+
+class SourceConfigError(Exception):
+    pass
+
+
+class UnhandledSource(Exception):
+    pass
+
+
+class AptLockError(Exception):
+    pass
+
+
+class BaseFetchHandler(object):
+
+    """Base class for FetchHandler implementations in fetch plugins"""
+
+    def can_handle(self, source):
+        """Returns True if the source can be handled. Otherwise returns
+        a string explaining why it cannot"""
+        return "Wrong source type"
+
+    def install(self, source):
+        """Try to download and unpack the source. Return the path to the
+        unpacked files or raise UnhandledSource."""
+        raise UnhandledSource("Wrong source type {}".format(source))
+
+    def parse_url(self, url):
+        return urlparse(url)
+
+    def base_url(self, url):
+        """Return url without querystring or fragment"""
+        parts = list(self.parse_url(url))
+        parts[4:] = ['' for i in parts[4:]]
+        return urlunparse(parts)
+
 
 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
+    # another process is already building the cache).
+    apt_pkg.config.set("Dir::Cache::pkgcache", "")
+
     cache = apt_pkg.Cache()
     _pkgs = []
     for package in packages:
@@ -87,14 +150,7 @@
         cmd.extend(packages)
     log("Installing {} with options: {}".format(packages,
                                                 options))
-    env = os.environ.copy()
-    if 'DEBIAN_FRONTEND' not in env:
-        env['DEBIAN_FRONTEND'] = 'noninteractive'
-
-    if fatal:
-        subprocess.check_call(cmd, env=env)
-    else:
-        subprocess.call(cmd, env=env)
+    _run_apt_command(cmd, fatal)
 
 
 def apt_upgrade(options=None, fatal=False, dist=False):
@@ -109,24 +165,13 @@
     else:
         cmd.append('upgrade')
     log("Upgrading with options: {}".format(options))
-
-    env = os.environ.copy()
-    if 'DEBIAN_FRONTEND' not in env:
-        env['DEBIAN_FRONTEND'] = 'noninteractive'
-
-    if fatal:
-        subprocess.check_call(cmd, env=env)
-    else:
-        subprocess.call(cmd, env=env)
+    _run_apt_command(cmd, fatal)
 
 
 def apt_update(fatal=False):
     """Update local apt cache"""
     cmd = ['apt-get', 'update']
-    if fatal:
-        subprocess.check_call(cmd)
-    else:
-        subprocess.call(cmd)
+    _run_apt_command(cmd, fatal)
 
 
 def apt_purge(packages, fatal=False):
@@ -137,10 +182,7 @@
     else:
         cmd.extend(packages)
     log("Purging {}".format(packages))
-    if fatal:
-        subprocess.check_call(cmd)
-    else:
-        subprocess.call(cmd)
+    _run_apt_command(cmd, fatal)
 
 
 def apt_hold(packages, fatal=False):
@@ -151,6 +193,7 @@
     else:
         cmd.extend(packages)
     log("Holding {}".format(packages))
+
     if fatal:
         subprocess.check_call(cmd)
     else:
@@ -184,57 +227,50 @@
             apt.write(PROPOSED_POCKET.format(release))
     if key:
         subprocess.check_call(['apt-key', 'adv', '--keyserver',
-                               'keyserver.ubuntu.com', '--recv',
+                               'hkp://keyserver.ubuntu.com:80', '--recv',
                                key])
 
 
-class SourceConfigError(Exception):
-    pass
-
-
 def configure_sources(update=False,
                       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)
 
-# The order of this list is very important. Handlers should be listed in from
-# least- to most-specific URL matching.
-FETCH_HANDLERS = (
-    'charmhelpers.fetch.archiveurl.ArchiveUrlFetchHandler',
-    'charmhelpers.fetch.bzrurl.BzrUrlFetchHandler',
-)
-
-
-class UnhandledSource(Exception):
-    pass
-
 
 def install_remote(source):
     """
@@ -265,30 +301,6 @@
     return install_remote(source)
 
 
-class BaseFetchHandler(object):
-
-    """Base class for FetchHandler implementations in fetch plugins"""
-
-    def can_handle(self, source):
-        """Returns True if the source can be handled. Otherwise returns
-        a string explaining why it cannot"""
-        return "Wrong source type"
-
-    def install(self, source):
-        """Try to download and unpack the source. Return the path to the
-        unpacked files or raise UnhandledSource."""
-        raise UnhandledSource("Wrong source type {}".format(source))
-
-    def parse_url(self, url):
-        return urlparse(url)
-
-    def base_url(self, url):
-        """Return url without querystring or fragment"""
-        parts = list(self.parse_url(url))
-        parts[4:] = ['' for i in parts[4:]]
-        return urlunparse(parts)
-
-
 def plugins(fetch_handlers=None):
     if not fetch_handlers:
         fetch_handlers = FETCH_HANDLERS
@@ -306,3 +318,40 @@
             log("FetchHandler {} not found, skipping plugin".format(
                 handler_name))
     return plugin_list
+
+
+def _run_apt_command(cmd, fatal=False):
+    """
+    Run an APT command, checking output and retrying if the fatal flag is set
+    to True.
+
+    :param: cmd: str: The apt command to run.
+    :param: fatal: bool: Whether the command's output should be checked and
+        retried.
+    """
+    env = os.environ.copy()
+
+    if 'DEBIAN_FRONTEND' not in env:
+        env['DEBIAN_FRONTEND'] = 'noninteractive'
+
+    if fatal:
+        retry_count = 0
+        result = None
+
+        # If the command is considered "fatal", we need to retry if the apt
+        # lock was not acquired.
+
+        while result is None or result == APT_NO_LOCK:
+            try:
+                result = subprocess.check_call(cmd, env=env)
+            except subprocess.CalledProcessError, e:
+                retry_count = retry_count + 1
+                if retry_count > APT_NO_LOCK_RETRY_COUNT:
+                    raise
+                result = e.returncode
+                log("Couldn't acquire DPKG lock. Will retry in {} seconds."
+                    "".format(APT_NO_LOCK_RETRY_DELAY))
+                time.sleep(APT_NO_LOCK_RETRY_DELAY)
+
+    else:
+        subprocess.call(cmd, env=env)

=== modified file 'hooks/charmhelpers/fetch/bzrurl.py'
--- hooks/charmhelpers/fetch/bzrurl.py	2014-04-11 11:02:19 +0000
+++ hooks/charmhelpers/fetch/bzrurl.py	2014-09-17 13:16:52 +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:

=== added symlink 'hooks/hanode-relation-broken'
=== target is u'hooks.py'
=== added symlink 'hooks/hanode-relation-departed'
=== target is u'hooks.py'
=== modified file 'hooks/hooks.py'
--- hooks/hooks.py	2014-04-16 21:57:13 +0000
+++ hooks/hooks.py	2014-09-17 13:16:52 +0000
@@ -25,7 +25,10 @@
     relation_set,
     unit_get,
     config,
-    Hooks, UnregisteredHookError
+    Hooks,
+    local_unit,
+    UnregisteredHookError,
+    unit_private_ip,
 )
 
 from charmhelpers.core.host import (
@@ -33,6 +36,7 @@
     service_start,
     service_restart,
     service_running,
+    file_hash,
 )
 
 from charmhelpers.fetch import (
@@ -41,11 +45,15 @@
 
 from charmhelpers.contrib.hahelpers.cluster import (
     peer_units,
+    peer_ips,
     oldest_peer
 )
 
 hooks = Hooks()
 
+HAMARKER = '/var/lib/juju/haconfigured'
+COROSYNC_CONF = '/etc/corosync/corosync.conf'
+
 
 @hooks.hook()
 def install():
@@ -60,6 +68,14 @@
 
 
 def get_corosync_conf():
+    ha_units = peer_ips(peer_relation='hanode')
+    ha_units[local_unit()] = unit_private_ip()
+    ha_nodes = {}
+    # Corosync nodeid 0 is reserved so increase all the nodeids to avoid it
+    off_set = 1000
+    for unit in ha_units:
+        unit_no = off_set + int(unit.split('/')[1])
+        ha_nodes[unit_no] = ha_units[unit]
     conf = {}
     for relid in relation_ids('ha'):
         for unit in related_units(relid):
@@ -72,6 +88,7 @@
                 'corosync_mcastport': relation_get('corosync_mcastport',
                                                    unit, relid),
                 'corosync_mcastaddr': config('corosync_mcastaddr'),
+                'ha_nodes': ha_nodes,
             }
             if None not in conf.itervalues():
                 return conf
@@ -84,8 +101,9 @@
     # read config variables
     corosync_conf_context = get_corosync_conf()
     # write config file (/etc/corosync/corosync.conf
-    with open('/etc/corosync/corosync.conf', 'w') as corosync_conf:
-        corosync_conf.write(render_template('corosync.conf',
+    template_file = 'corosync.conf.' + config('corosync_transport')
+    with open(COROSYNC_CONF, 'w') as corosync_conf:
+        corosync_conf.write(render_template(template_file,
                                             corosync_conf_context))
 
 
@@ -116,6 +134,8 @@
     # Create a new config file
     emit_base_conf()
 
+    config_corosync()
+
     # Reconfigure the cluster if required
     configure_cluster()
 
@@ -136,12 +156,52 @@
     time.sleep(5)
     service_start("pacemaker")
 
-HAMARKER = '/var/lib/juju/haconfigured'
+
[e-mail]('hanode-relation-broken')
+def hanode_relation_broken():
+    for service in ["pacemaker", "corosync"]:
+        if service_running(service):
+            service_stop(service)
+
+
+def conditional_corosync_restart():
+    def wrap(f):
+        def wrapped_f(*args):
+            checksum = file_hash(COROSYNC_CONF)
+            f(*args)
+            if checksum != file_hash(COROSYNC_CONF):
+                restart_corosync()
+        return wrapped_f
+    return wrap
+
+
+@conditional_corosync_restart()
+def config_corosync():
+    supported_transports = ['udp', 'udpu']
+    if config('corosync_transport') not in supported_transports:
+        raise Exception('The corosync_transport type %s is not supported.'
+                        'Supported types are: %s' %
+                        (config('corosync_transport'),
+                         str(supported_transports)))
+    if get_corosync_conf():
+        log('Configuring and restarting corosync')
+        emit_corosync_conf()
+    else:
+        log('Not ready for corosync config')
+
+
[e-mail]('hanode-relation-departed')
[e-mail]('hanode-relation-joined')
+def hanode_relation_member_change():
+    # In udpu (unicast) mode a list of nodes is maintained in corosync.conf
+    # so it always needs to be generated if a peer joins or leaves
+    if config('corosync_transport') == 'udpu':
+        config_corosync()
+    configure_cluster()
 
 
 @hooks.hook('ha-relation-joined',
             'ha-relation-changed',
-            'hanode-relation-joined',
             'hanode-relation-changed')
 def configure_cluster():
     # Check that we are not already configured
@@ -223,9 +283,7 @@
                 for ra in resources.itervalues()]:
         apt_install('ceph-resource-agents')
 
-    log('Configuring and restarting corosync')
-    emit_corosync_conf()
-    restart_corosync()
+    config_corosync()
 
     log('Waiting for PCMK to start')
     pcmk.wait_for_pcmk()

=== removed file 'revision'
--- revision	2014-01-29 20:48:59 +0000
+++ revision	1970-01-01 00:00:00 +0000
@@ -1,1 +0,0 @@
-68

=== renamed file 'templates/corosync.conf' => 'templates/corosync.conf.udp'
=== added file 'templates/corosync.conf.udpu'
--- templates/corosync.conf.udpu	1970-01-01 00:00:00 +0000
+++ templates/corosync.conf.udpu	2014-09-17 13:16:52 +0000
@@ -0,0 +1,73 @@
+# Config file generated by the ha charm.
+# udpu.template
+totem {
+	version: 2
+
+	# How long before declaring a token lost (ms)
+	token: 3000
+
+	# How many token retransmits before forming a new configuration
+	token_retransmits_before_loss_const: 10
+
+	# How long to wait for join messages in the membership protocol (ms)
+	join: 60
+
+	# How long to wait for consensus to be achieved before starting a new round of membership configuration (ms)
+	consensus: 3600
+
+	# Turn off the virtual synchrony filter
+	vsftype: none
+
+	# Number of messages that may be sent by one processor on receipt of the token
+	max_messages: 20
+
+	# Limit generated nodeids to 31-bits (positive signed integers)
+	clear_node_high_bit: yes
+
+	# Disable encryption
+ 	secauth: off
+
+	# How many threads to use for encryption/decryption
+ 	threads: 0
+
+	# This specifies the mode of redundant ring, which may be none, active, or passive.
+ 	rrp_mode: none
+
+ 	interface {
+		# The following values need to be set based on your environment
+		ringnumber: 0
+		bindnetaddr: {{ corosync_bindnetaddr }}
+		mcastport: {{ corosync_mcastport }}
+                ttl: 1
+	}
+        transport: udpu
+}
+
+quorum {
+        # Enable and configure quorum subsystem (default: off)
+        # see also corosync.conf.5 and votequorum.5
+        provider: corosync_votequorum
+        expected_votes: 2
+}
+
+nodelist {
+{% for nodeid, ip in ha_nodes.iteritems() %}
+        node {
+               ring0_addr: {{ ip }}
+               nodeid: {{ nodeid }}
+        }
+{% endfor %}
+}
+
+logging {
+        fileline: off
+        to_stderr: yes
+        to_logfile: no
+        to_syslog: yes
+        syslog_facility: daemon
+        debug: off
+        logger_subsys {
+                subsys: QUORUM
+                debug: off
+        }
+}