Fix alphanumeric comparisons for openstack and ubuntu releases

- sync charmhelpers with fix-alpha helpers
- fix up code where the alpha comparisons are done
- fix tests which assumed mocks would just work on os_release()

Change-Id: Ifa495c37adeb24aa98e4e5e181b90cbbd5c0cddb
Related-Bug: #1659575

charm-helpers-tests.yaml

@@ -4,3 +4,4 @@ include:
     - contrib.amulet
     - contrib.openstack.amulet
     - core
+    - osplatform

hooks/charmhelpers/contrib/network/ip.py

@@ -31,6 +31,7 @@ from charmhelpers.core.hookenv import (
 
 from charmhelpers.core.host import (
     lsb_release,
+    CompareHostReleases,
 )
 
 try:
@@ -67,6 +68,24 @@ def no_ip_found_error_out(network):
     raise ValueError(errmsg)
 
 
+def _get_ipv6_network_from_address(address):
+    """Get an netaddr.IPNetwork for the given IPv6 address
+    :param address: a dict as returned by netifaces.ifaddresses
+    :returns netaddr.IPNetwork: None if the address is a link local or loopback
+    address
+    """
+    if address['addr'].startswith('fe80') or address['addr'] == "::1":
+        return None
+
+    prefix = address['netmask'].split("/")
+    if len(prefix) > 1:
+        netmask = prefix[1]
+    else:
+        netmask = address['netmask']
+    return netaddr.IPNetwork("%s/%s" % (address['addr'],
+                                        netmask))
+
+
 def get_address_in_network(network, fallback=None, fatal=False):
     """Get an IPv4 or IPv6 address within the network from the host.
 
@@ -92,19 +111,17 @@ def get_address_in_network(network, fallback=None, fatal=False):
         for iface in netifaces.interfaces():
             addresses = netifaces.ifaddresses(iface)
             if network.version == 4 and netifaces.AF_INET in addresses:
-                addr = addresses[netifaces.AF_INET][0]['addr']
+                for addr in addresses[netifaces.AF_INET]:
-                netmask = addresses[netifaces.AF_INET][0]['netmask']
+                    cidr = netaddr.IPNetwork("%s/%s" % (addr['addr'],
-                cidr = netaddr.IPNetwork("%s/%s" % (addr, netmask))
+                                                        addr['netmask']))
-                if cidr in network:
+                    if cidr in network:
-                    return str(cidr.ip)
+                        return str(cidr.ip)
 
             if network.version == 6 and netifaces.AF_INET6 in addresses:
                 for addr in addresses[netifaces.AF_INET6]:
-                    if not addr['addr'].startswith('fe80'):
+                    cidr = _get_ipv6_network_from_address(addr)
-                        cidr = netaddr.IPNetwork("%s/%s" % (addr['addr'],
+                    if cidr and cidr in network:
-                                                            addr['netmask']))
+                        return str(cidr.ip)
-                        if cidr in network:
-                            return str(cidr.ip)
 
     if fallback is not None:
         return fallback
@@ -180,18 +197,18 @@ def _get_for_address(address, key):
 
         if address.version == 6 and netifaces.AF_INET6 in addresses:
             for addr in addresses[netifaces.AF_INET6]:
-                if not addr['addr'].startswith('fe80'):
+                network = _get_ipv6_network_from_address(addr)
-                    network = netaddr.IPNetwork("%s/%s" % (addr['addr'],
+                if not network:
-                                                           addr['netmask']))
+                    continue
-                    cidr = network.cidr
-                    if address in cidr:
-                        if key == 'iface':
-                            return iface
-                        elif key == 'netmask' and cidr:
-                            return str(cidr).split('/')[1]
-                        else:
-                            return addr[key]
 
+                cidr = network.cidr
+                if address in cidr:
+                    if key == 'iface':
+                        return iface
+                    elif key == 'netmask' and cidr:
+                        return str(cidr).split('/')[1]
+                    else:
+                        return addr[key]
     return None
 
 
@@ -521,7 +538,8 @@ def port_has_listener(address, port):
 
 def assert_charm_supports_ipv6():
     """Check whether we are able to support charms ipv6."""
-    if lsb_release()['DISTRIB_CODENAME'].lower() < "trusty":
+    release = lsb_release()['DISTRIB_CODENAME'].lower()
+    if CompareHostReleases(release) < "trusty":
         raise Exception("IPv6 is not supported in the charms for Ubuntu "
                         "versions less than Trusty 14.04")
 

hooks/charmhelpers/contrib/openstack/amulet/utils.py

@@ -40,6 +40,7 @@ from charmhelpers.contrib.amulet.utils import (
     AmuletUtils
 )
 from charmhelpers.core.decorators import retry_on_exception
+from charmhelpers.core.host import CompareHostReleases
 
 DEBUG = logging.DEBUG
 ERROR = logging.ERROR
@@ -1255,7 +1256,7 @@ class OpenStackAmuletUtils(AmuletUtils):
         contents = self.file_contents_safe(sentry_unit, '/etc/memcached.conf',
                                            fatal=True)
         ubuntu_release, _ = self.run_cmd_unit(sentry_unit, 'lsb_release -cs')
-        if ubuntu_release <= 'trusty':
+        if CompareHostReleases(ubuntu_release) <= 'trusty':
             memcache_listen_addr = 'ip6-localhost'
         else:
             memcache_listen_addr = '::1'

hooks/charmhelpers/contrib/openstack/context.py

@@ -59,6 +59,8 @@ from charmhelpers.core.host import (
     write_file,
     pwgen,
     lsb_release,
+    CompareHostReleases,
+    is_container,
 )
 from charmhelpers.contrib.hahelpers.cluster import (
     determine_apache_port,
@@ -155,7 +157,8 @@ class OSContextGenerator(object):
 
         if self.missing_data:
             self.complete = False
-            log('Missing required data: %s' % ' '.join(self.missing_data), level=INFO)
+            log('Missing required data: %s' % ' '.join(self.missing_data),
+                level=INFO)
         else:
             self.complete = True
         return self.complete
@@ -213,8 +216,9 @@ class SharedDBContext(OSContextGenerator):
                 hostname_key = "{}_hostname".format(self.relation_prefix)
             else:
                 hostname_key = "hostname"
-            access_hostname = get_address_in_network(access_network,
+            access_hostname = get_address_in_network(
-                                                     unit_get('private-address'))
+                access_network,
+                unit_get('private-address'))
             set_hostname = relation_get(attribute=hostname_key,
                                         unit=local_unit())
             if set_hostname != access_hostname:
@@ -308,7 +312,10 @@ def db_ssl(rdata, ctxt, ssl_dir):
 
 class IdentityServiceContext(OSContextGenerator):
 
-    def __init__(self, service=None, service_user=None, rel_name='identity-service'):
+    def __init__(self,
+                 service=None,
+                 service_user=None,
+                 rel_name='identity-service'):
         self.service = service
         self.service_user = service_user
         self.rel_name = rel_name
@@ -457,19 +464,17 @@ class AMQPContext(OSContextGenerator):
                     host = format_ipv6_addr(host) or host
                     rabbitmq_hosts.append(host)
 
-                ctxt['rabbitmq_hosts'] = ','.join(sorted(rabbitmq_hosts))
+                rabbitmq_hosts = sorted(rabbitmq_hosts)
+                ctxt['rabbitmq_hosts'] = ','.join(rabbitmq_hosts)
                 transport_hosts = rabbitmq_hosts
 
             if transport_hosts:
-                transport_url_hosts = ''
+                transport_url_hosts = ','.join([
-                for host in transport_hosts:
+                    "{}:{}@{}:{}".format(ctxt['rabbitmq_user'],
-                    if transport_url_hosts:
+                                         ctxt['rabbitmq_password'],
-                        format_string = ",{}:{}@{}:{}"
+                                         host_,
-                    else:
+                                         rabbitmq_port)
-                        format_string = "{}:{}@{}:{}"
+                    for host_ in transport_hosts])
-                    transport_url_hosts += format_string.format(
-                        ctxt['rabbitmq_user'], ctxt['rabbitmq_password'],
-                        host, rabbitmq_port)
                 ctxt['transport_url'] = "rabbit://{}/{}".format(
                     transport_url_hosts, vhost)
 
@@ -1217,6 +1222,10 @@ class BindHostContext(OSContextGenerator):
             return {'bind_host': '0.0.0.0'}
 
 
+MAX_DEFAULT_WORKERS = 4
+DEFAULT_MULTIPLIER = 2
+
+
 class WorkerConfigContext(OSContextGenerator):
 
     @property
@@ -1228,10 +1237,19 @@ class WorkerConfigContext(OSContextGenerator):
             return psutil.NUM_CPUS
 
     def __call__(self):
-        multiplier = config('worker-multiplier') or 0
+        multiplier = config('worker-multiplier') or DEFAULT_MULTIPLIER
         count = int(self.num_cpus * multiplier)
         if multiplier > 0 and count == 0:
             count = 1
+
+        if config('worker-multiplier') is None and is_container():
+            # NOTE(jamespage): Limit unconfigured worker-multiplier
+            #                  to MAX_DEFAULT_WORKERS to avoid insane
+            #                  worker configuration in LXD containers
+            #                  on large servers
+            # Reference: https://pad.lv/1665270
+            count = min(count, MAX_DEFAULT_WORKERS)
+
         ctxt = {"workers": count}
         return ctxt
 
@@ -1601,7 +1619,8 @@ class MemcacheContext(OSContextGenerator):
         if ctxt['use_memcache']:
             # Trusty version of memcached does not support ::1 as a listen
             # address so use host file entry instead
-            if lsb_release()['DISTRIB_CODENAME'].lower() > 'trusty':
+            release = lsb_release()['DISTRIB_CODENAME'].lower()
+            if CompareHostReleases(release) > 'trusty':
                 ctxt['memcache_server'] = '::1'
             else:
                 ctxt['memcache_server'] = 'ip6-localhost'

hooks/charmhelpers/contrib/openstack/neutron.py

@@ -23,7 +23,10 @@ from charmhelpers.core.hookenv import (
     ERROR,
 )
 
-from charmhelpers.contrib.openstack.utils import os_release
+from charmhelpers.contrib.openstack.utils import (
+    os_release,
+    CompareOpenStackReleases,
+)
 
 
 def headers_package():
@@ -198,7 +201,8 @@ def neutron_plugins():
         },
         'plumgrid': {
             'config': '/etc/neutron/plugins/plumgrid/plumgrid.ini',
-            'driver': 'neutron.plugins.plumgrid.plumgrid_plugin.plumgrid_plugin.NeutronPluginPLUMgridV2',
+            'driver': ('neutron.plugins.plumgrid.plumgrid_plugin'
+                       '.plumgrid_plugin.NeutronPluginPLUMgridV2'),
             'contexts': [
                 context.SharedDBContext(user=config('database-user'),
                                         database=config('database'),
@@ -225,7 +229,7 @@ def neutron_plugins():
             'server_services': ['neutron-server']
         }
     }
-    if release >= 'icehouse':
+    if CompareOpenStackReleases(release) >= 'icehouse':
         # NOTE: patch in ml2 plugin for icehouse onwards
         plugins['ovs']['config'] = '/etc/neutron/plugins/ml2/ml2_conf.ini'
         plugins['ovs']['driver'] = 'neutron.plugins.ml2.plugin.Ml2Plugin'
@@ -233,10 +237,10 @@ def neutron_plugins():
                                              'neutron-plugin-ml2']
         # NOTE: patch in vmware renames nvp->nsx for icehouse onwards
         plugins['nvp'] = plugins['nsx']
-    if release >= 'kilo':
+    if CompareOpenStackReleases(release) >= 'kilo':
         plugins['midonet']['driver'] = (
             'neutron.plugins.midonet.plugin.MidonetPluginV2')
-    if release >= 'liberty':
+    if CompareOpenStackReleases(release) >= 'liberty':
         plugins['midonet']['driver'] = (
             'midonet.neutron.plugin_v1.MidonetPluginV2')
         plugins['midonet']['server_packages'].remove(
@@ -244,10 +248,11 @@ def neutron_plugins():
         plugins['midonet']['server_packages'].append(
             'python-networking-midonet')
         plugins['plumgrid']['driver'] = (
-            'networking_plumgrid.neutron.plugins.plugin.NeutronPluginPLUMgridV2')
+            'networking_plumgrid.neutron.plugins'
+            '.plugin.NeutronPluginPLUMgridV2')
         plugins['plumgrid']['server_packages'].remove(
             'neutron-plugin-plumgrid')
-    if release >= 'mitaka':
+    if CompareOpenStackReleases(release) >= 'mitaka':
         plugins['nsx']['server_packages'].remove('neutron-plugin-vmware')
         plugins['nsx']['server_packages'].append('python-vmware-nsx')
         plugins['nsx']['config'] = '/etc/neutron/nsx.ini'

hooks/charmhelpers/contrib/openstack/templates/haproxy.cfg

@@ -5,6 +5,8 @@ global
     user haproxy
     group haproxy
     spread-checks 0
+    stats socket /var/run/haproxy/admin.sock mode 600 level admin
+    stats timeout 2m
 
 defaults
     log global
@@ -58,6 +60,15 @@ frontend tcp-in_{{ service }}
 {% for frontend in frontends -%}
 backend {{ service }}_{{ frontend }}
     balance leastconn
+    {% if backend_options -%}
+    {% if backend_options[service] -%}
+    {% for option in backend_options[service] -%}
+    {% for key, value in option.items() -%}
+    {{ key }} {{ value }}
+    {% endfor -%}
+    {% endfor -%}
+    {% endif -%}
+    {% endif -%}
     {% for unit, address in frontends[frontend]['backends'].items() -%}
     server {{ unit }} {{ address }}:{{ ports[1] }} check
     {% endfor %}

hooks/charmhelpers/contrib/openstack/utils.py

@@ -33,9 +33,7 @@ import yaml
 
 from charmhelpers.contrib.network import ip
 
-from charmhelpers.core import (
+from charmhelpers.core import unitdata
-    unitdata,
-)
 
 from charmhelpers.core.hookenv import (
     action_fail,
@@ -55,6 +53,8 @@ from charmhelpers.core.hookenv import (
     application_version_set,
 )
 
+from charmhelpers.core.strutils import BasicStringComparator
+
 from charmhelpers.contrib.storage.linux.lvm import (
     deactivate_lvm_volume_group,
     is_lvm_physical_volume,
@@ -97,6 +97,22 @@ CLOUD_ARCHIVE_KEY_ID = '5EDB1B62EC4926EA'
 DISTRO_PROPOSED = ('deb http://archive.ubuntu.com/ubuntu/ %s-proposed '
                    'restricted main multiverse universe')
 
+OPENSTACK_RELEASES = (
+    'diablo',
+    'essex',
+    'folsom',
+    'grizzly',
+    'havana',
+    'icehouse',
+    'juno',
+    'kilo',
+    'liberty',
+    'mitaka',
+    'newton',
+    'ocata',
+    'pike',
+)
+
 UBUNTU_OPENSTACK_RELEASE = OrderedDict([
     ('oneiric', 'diablo'),
     ('precise', 'essex'),
@@ -238,6 +254,17 @@ GIT_DEFAULT_BRANCHES = {
 DEFAULT_LOOPBACK_SIZE = '5G'
 
 
+class CompareOpenStackReleases(BasicStringComparator):
+    """Provide comparisons of OpenStack releases.
+
+    Use in the form of
+
+    if CompareOpenStackReleases(release) > 'mitaka':
+        # do something with mitaka
+    """
+    _list = OPENSTACK_RELEASES
+
+
 def error_out(msg):
     juju_log("FATAL ERROR: %s" % msg, level='ERROR')
     sys.exit(1)
@@ -1066,7 +1093,8 @@ def git_generate_systemd_init_files(templates_dir):
 
             shutil.copyfile(init_in_source, init_source)
             with open(init_source, 'a') as outfile:
-                template = '/usr/share/openstack-pkg-tools/init-script-template'
+                template = ('/usr/share/openstack-pkg-tools/'
+                            'init-script-template')
                 with open(template) as infile:
                     outfile.write('\n\n{}'.format(infile.read()))
 
@@ -1971,9 +1999,7 @@ def enable_memcache(source=None, release=None, package=None):
     if not _release:
         _release = get_os_codename_install_source(source)
 
-    # TODO: this should be changed to a numeric comparison using a known list
+    return CompareOpenStackReleases(_release) >= 'mitaka'
-    # of releases and comparing by index.
-    return _release >= 'mitaka'
 
 
 def token_cache_pkgs(source=None, release=None):

hooks/charmhelpers/contrib/storage/linux/ceph.py

@@ -987,18 +987,20 @@ def ensure_ceph_storage(service, pool, rbd_img, sizemb, mount_point,
             service_start(svc)
 
 
-def ensure_ceph_keyring(service, user=None, group=None, relation='ceph'):
+def ensure_ceph_keyring(service, user=None, group=None,
+                        relation='ceph', key=None):
     """Ensures a ceph keyring is created for a named service and optionally
     ensures user and group ownership.
 
-    Returns False if no ceph key is available in relation state.
+    @returns boolean: Flag to indicate whether a key was successfully written
+                      to disk based on either relation data or a supplied key
     """
-    key = None
+    if not key:
-    for rid in relation_ids(relation):
+        for rid in relation_ids(relation):
-        for unit in related_units(rid):
+            for unit in related_units(rid):
-            key = relation_get('key', rid=rid, unit=unit)
+                key = relation_get('key', rid=rid, unit=unit)
-            if key:
+                if key:
-                break
+                    break
 
     if not key:
         return False

hooks/charmhelpers/core/host.py

@@ -45,6 +45,7 @@ if __platform__ == "ubuntu":
         add_new_group,
         lsb_release,
         cmp_pkgrevno,
+        CompareHostReleases,
     )  # flake8: noqa -- ignore F401 for this import
 elif __platform__ == "centos":
     from charmhelpers.core.host_factory.centos import (
@@ -52,6 +53,7 @@ elif __platform__ == "centos":
         add_new_group,
         lsb_release,
         cmp_pkgrevno,
+        CompareHostReleases,
     )  # flake8: noqa -- ignore F401 for this import
 
 UPDATEDB_PATH = '/etc/updatedb.conf'

hooks/charmhelpers/core/host_factory/centos.py

@@ -2,6 +2,22 @@ import subprocess
 import yum
 import os
 
+from charmhelpers.core.strutils import BasicStringComparator
+
+
+class CompareHostReleases(BasicStringComparator):
+    """Provide comparisons of Host releases.
+
+    Use in the form of
+
+    if CompareHostReleases(release) > 'trusty':
+        # do something with mitaka
+    """
+
+    def __init__(self, item):
+        raise NotImplementedError(
+            "CompareHostReleases() is not implemented for CentOS")
+
 
 def service_available(service_name):
     # """Determine whether a system service is available."""

hooks/charmhelpers/core/host_factory/ubuntu.py

@@ -1,5 +1,37 @@
 import subprocess
 
+from charmhelpers.core.strutils import BasicStringComparator
+
+
+UBUNTU_RELEASES = (
+    'lucid',
+    'maverick',
+    'natty',
+    'oneiric',
+    'precise',
+    'quantal',
+    'raring',
+    'saucy',
+    'trusty',
+    'utopic',
+    'vivid',
+    'wily',
+    'xenial',
+    'yakkety',
+    'zesty',
+)
+
+
+class CompareHostReleases(BasicStringComparator):
+    """Provide comparisons of Ubuntu releases.
+
+    Use in the form of
+
+    if CompareHostReleases(release) > 'trusty':
+        # do something with mitaka
+    """
+    _list = UBUNTU_RELEASES
+
 
 def service_available(service_name):
     """Determine whether a system service is available"""

hooks/charmhelpers/core/strutils.py

@@ -68,3 +68,56 @@ def bytes_from_string(value):
         msg = "Unable to interpret string value '%s' as bytes" % (value)
         raise ValueError(msg)
     return int(matches.group(1)) * (1024 ** BYTE_POWER[matches.group(2)])
+
+
+class BasicStringComparator(object):
+    """Provides a class that will compare strings from an iterator type object.
+    Used to provide > and < comparisons on strings that may not necessarily be
+    alphanumerically ordered.  e.g. OpenStack or Ubuntu releases AFTER the
+    z-wrap.
+    """
+
+    _list = None
+
+    def __init__(self, item):
+        if self._list is None:
+            raise Exception("Must define the _list in the class definition!")
+        try:
+            self.index = self._list.index(item)
+        except Exception:
+            raise KeyError("Item '{}' is not in list '{}'"
+                           .format(item, self._list))
+
+    def __eq__(self, other):
+        assert isinstance(other, str) or isinstance(other, self.__class__)
+        return self.index == self._list.index(other)
+
+    def __ne__(self, other):
+        return not self.__eq__(other)
+
+    def __lt__(self, other):
+        assert isinstance(other, str) or isinstance(other, self.__class__)
+        return self.index < self._list.index(other)
+
+    def __ge__(self, other):
+        return not self.__lt__(other)
+
+    def __gt__(self, other):
+        assert isinstance(other, str) or isinstance(other, self.__class__)
+        return self.index > self._list.index(other)
+
+    def __le__(self, other):
+        return not self.__gt__(other)
+
+    def __str__(self):
+        """Always give back the item at the index so it can be used in
+        comparisons like:
+
+        s_mitaka = CompareOpenStack('mitaka')
+        s_newton = CompareOpenstack('newton')
+
+        assert s_newton > s_mitaka
+
+        @returns: <string>
+        """
+        return self._list[self.index]

hooks/neutron_ovs_utils.py

@@ -34,6 +34,7 @@ from charmhelpers.contrib.openstack.utils import (
     is_unit_paused_set,
     os_application_version_set,
     remote_restart,
+    CompareOpenStackReleases,
 )
 from collections import OrderedDict
 from charmhelpers.contrib.openstack.utils import (
@@ -70,6 +71,7 @@ from charmhelpers.core.host import (
     service_restart,
     service_running,
     write_file,
+    CompareHostReleases,
 )
 
 from charmhelpers.core.templating import render
@@ -259,8 +261,9 @@ def determine_packages():
             if p in pkgs:
                 pkgs.remove(p)
 
-    release = os_release('neutron-common', base='icehouse')
+    cmp_release = CompareOpenStackReleases(
-    if release >= 'mitaka' and 'neutron-plugin-openvswitch-agent' in pkgs:
+        os_release('neutron-common', base='icehouse'))
+    if cmp_release >= 'mitaka' and 'neutron-plugin-openvswitch-agent' in pkgs:
         pkgs.remove('neutron-plugin-openvswitch-agent')
         pkgs.append('neutron-openvswitch-agent')
 
@@ -300,7 +303,8 @@ def resource_map():
         metadata_services = ['neutron-metadata-agent', 'neutron-dhcp-agent']
         resource_map[NEUTRON_CONF]['services'] += metadata_services
     # Remap any service names as required
-    if os_release('neutron-common', base='icehouse') >= 'mitaka':
+    _os_release = os_release('neutron-common', base='icehouse')
+    if CompareOpenStackReleases(_os_release) >= 'mitaka':
         # ml2_conf.ini -> openvswitch_agent.ini
         drop_config.append(ML2_CONF)
         # drop of -plugin from service name
@@ -320,7 +324,7 @@ def resource_map():
         drop_config.extend([OVS_CONF, DPDK_INTERFACES])
 
     # Use MAAS1.9 for MTU and external port config on xenial and above
-    if float(lsb_release()['DISTRIB_RELEASE']) >= 16.04:
+    if CompareHostReleases(lsb_release()['DISTRIB_CODENAME']) >= 'xenial':
         drop_config.extend([EXT_PORT_CONF, PHY_NIC_MTU_CONF])
 
     for _conf in drop_config:
@@ -499,18 +503,16 @@ def determine_datapath_type():
 
 def use_dpdk():
     '''Determine whether DPDK should be used'''
-    release = os_release('neutron-common', base='icehouse')
+    cmp_release = CompareOpenStackReleases(
-    if (release >= 'mitaka' and config('enable-dpdk')):
+        os_release('neutron-common', base='icehouse'))
-        return True
+    return (cmp_release >= 'mitaka' and config('enable-dpdk'))
-    return False
 
 
 def enable_sriov_agent():
     '''Determine with SR-IOV agent should be used'''
-    release = os_release('neutron-common', base='icehouse')
+    cmp_release = CompareOpenStackReleases(
-    if (release >= 'mitaka' and config('enable-sriov')):
+        os_release('neutron-common', base='icehouse'))
-        return True
+    return (cmp_release >= 'mitaka' and config('enable-sriov'))
-    return False
 
 
 # TODO: update into charm-helpers to add port_type parameter
@@ -598,8 +600,10 @@ def git_post_install(projects_yaml):
            perms=0o440)
 
     bin_dir = os.path.join(git_pip_venv_dir(projects_yaml), 'bin')
+    cmp_os_release = CompareOpenStackReleases(os_release('neutron-common'))
     # Use systemd init units/scripts from ubuntu wily onward
-    if lsb_release()['DISTRIB_RELEASE'] >= '15.10':
+    _release = lsb_release()['DISTRIB_CODENAME']
+    if CompareHostReleases(_release) >= 'wily':
         templates_dir = os.path.join(charm_dir(), 'templates/git')
         daemons = ['neutron-openvswitch-agent', 'neutron-ovs-cleanup']
         for daemon in daemons:
@@ -608,7 +612,7 @@ def git_post_install(projects_yaml):
             }
             filename = daemon
             if daemon == 'neutron-openvswitch-agent':
-                if os_release('neutron-common') < 'mitaka':
+                if cmp_os_release < 'mitaka':
                     filename = 'neutron-plugin-openvswitch-agent'
             template_file = 'git/{}.init.in.template'.format(filename)
             init_in_file = '{}.init.in'.format(filename)
@@ -619,7 +623,7 @@ def git_post_install(projects_yaml):
         for daemon in daemons:
             filename = daemon
             if daemon == 'neutron-openvswitch-agent':
-                if os_release('neutron-common') < 'mitaka':
+                if cmp_os_release < 'mitaka':
                     filename = 'neutron-plugin-openvswitch-agent'
             service('enable', filename)
     else:
@@ -642,7 +646,7 @@ def git_post_install(projects_yaml):
             'log_file': '/var/log/neutron/ovs-cleanup.log',
         }
 
-        if os_release('neutron-common') < 'mitaka':
+        if cmp_os_release < 'mitaka':
             render('git/upstart/neutron-plugin-openvswitch-agent.upstart',
                    '/etc/init/neutron-plugin-openvswitch-agent.conf',
                    neutron_ovs_agent_context, perms=0o644)

tests/charmhelpers/contrib/openstack/amulet/utils.py

@@ -40,6 +40,7 @@ from charmhelpers.contrib.amulet.utils import (
     AmuletUtils
 )
 from charmhelpers.core.decorators import retry_on_exception
+from charmhelpers.core.host import CompareHostReleases
 
 DEBUG = logging.DEBUG
 ERROR = logging.ERROR
@@ -1255,7 +1256,7 @@ class OpenStackAmuletUtils(AmuletUtils):
         contents = self.file_contents_safe(sentry_unit, '/etc/memcached.conf',
                                            fatal=True)
         ubuntu_release, _ = self.run_cmd_unit(sentry_unit, 'lsb_release -cs')
-        if ubuntu_release <= 'trusty':
+        if CompareHostReleases(ubuntu_release) <= 'trusty':
             memcache_listen_addr = 'ip6-localhost'
         else:
             memcache_listen_addr = '::1'

tests/charmhelpers/core/host.py

@@ -45,6 +45,7 @@ if __platform__ == "ubuntu":
         add_new_group,
         lsb_release,
         cmp_pkgrevno,
+        CompareHostReleases,
     )  # flake8: noqa -- ignore F401 for this import
 elif __platform__ == "centos":
     from charmhelpers.core.host_factory.centos import (
@@ -52,6 +53,7 @@ elif __platform__ == "centos":
         add_new_group,
         lsb_release,
         cmp_pkgrevno,
+        CompareHostReleases,
     )  # flake8: noqa -- ignore F401 for this import
 
 UPDATEDB_PATH = '/etc/updatedb.conf'

tests/charmhelpers/core/host_factory/centos.py

@@ -2,6 +2,22 @@ import subprocess
 import yum
 import os
 
+from charmhelpers.core.strutils import BasicStringComparator
+
+
+class CompareHostReleases(BasicStringComparator):
+    """Provide comparisons of Host releases.
+
+    Use in the form of
+
+    if CompareHostReleases(release) > 'trusty':
+        # do something with mitaka
+    """
+
+    def __init__(self, item):
+        raise NotImplementedError(
+            "CompareHostReleases() is not implemented for CentOS")
+
 
 def service_available(service_name):
     # """Determine whether a system service is available."""

tests/charmhelpers/core/host_factory/ubuntu.py

@@ -1,5 +1,37 @@
 import subprocess
 
+from charmhelpers.core.strutils import BasicStringComparator
+
+
+UBUNTU_RELEASES = (
+    'lucid',
+    'maverick',
+    'natty',
+    'oneiric',
+    'precise',
+    'quantal',
+    'raring',
+    'saucy',
+    'trusty',
+    'utopic',
+    'vivid',
+    'wily',
+    'xenial',
+    'yakkety',
+    'zesty',
+)
+
+
+class CompareHostReleases(BasicStringComparator):
+    """Provide comparisons of Ubuntu releases.
+
+    Use in the form of
+
+    if CompareHostReleases(release) > 'trusty':
+        # do something with mitaka
+    """
+    _list = UBUNTU_RELEASES
+
 
 def service_available(service_name):
     """Determine whether a system service is available"""

tests/charmhelpers/core/strutils.py

@@ -68,3 +68,56 @@ def bytes_from_string(value):
         msg = "Unable to interpret string value '%s' as bytes" % (value)
         raise ValueError(msg)
     return int(matches.group(1)) * (1024 ** BYTE_POWER[matches.group(2)])
+
+
+class BasicStringComparator(object):
+    """Provides a class that will compare strings from an iterator type object.
+    Used to provide > and < comparisons on strings that may not necessarily be
+    alphanumerically ordered.  e.g. OpenStack or Ubuntu releases AFTER the
+    z-wrap.
+    """
+
+    _list = None
+
+    def __init__(self, item):
+        if self._list is None:
+            raise Exception("Must define the _list in the class definition!")
+        try:
+            self.index = self._list.index(item)
+        except Exception:
+            raise KeyError("Item '{}' is not in list '{}'"
+                           .format(item, self._list))
+
+    def __eq__(self, other):
+        assert isinstance(other, str) or isinstance(other, self.__class__)
+        return self.index == self._list.index(other)
+
+    def __ne__(self, other):
+        return not self.__eq__(other)
+
+    def __lt__(self, other):
+        assert isinstance(other, str) or isinstance(other, self.__class__)
+        return self.index < self._list.index(other)
+
+    def __ge__(self, other):
+        return not self.__lt__(other)
+
+    def __gt__(self, other):
+        assert isinstance(other, str) or isinstance(other, self.__class__)
+        return self.index > self._list.index(other)
+
+    def __le__(self, other):
+        return not self.__gt__(other)
+
+    def __str__(self):
+        """Always give back the item at the index so it can be used in
+        comparisons like:
+
+        s_mitaka = CompareOpenStack('mitaka')
+        s_newton = CompareOpenstack('newton')
+
+        assert s_newton > s_mitaka
+
+        @returns: <string>
+        """
+        return self._list[self.index]

tests/charmhelpers/osplatform.py

@@ -0,0 +1,25 @@
+import platform
+
+
+def get_platform():
+    """Return the current OS platform.
+
+    For example: if current os platform is Ubuntu then a string "ubuntu"
+    will be returned (which is the name of the module).
+    This string is used to decide which platform module should be imported.
+    """
+    # linux_distribution is deprecated and will be removed in Python 3.7
+    # Warings *not* disabled, as we certainly need to fix this.
+    tuple_platform = platform.linux_distribution()
+    current_platform = tuple_platform[0]
+    if "Ubuntu" in current_platform:
+        return "ubuntu"
+    elif "CentOS" in current_platform:
+        return "centos"
+    elif "debian" in current_platform:
+        # Stock Python does not detect Ubuntu and instead returns debian.
+        # Or at least it does in some build environments like Travis CI
+        return "ubuntu"
+    else:
+        raise RuntimeError("This module is not supported on {}."
+                           .format(current_platform))

tox.ini

@@ -14,7 +14,7 @@ install_command =
   pip install --allow-unverified python-apt {opts} {packages}
 commands = ostestr {posargs}
 whitelist_externals = juju
-passenv = HOME TERM AMULET_*
+passenv = HOME TERM AMULET_* CS_API_*
 
 [testenv:py27]
 basepython = python2.7

unit_tests/test_neutron_ovs_utils.py

@@ -203,6 +203,7 @@ class TestNeutronOVSUtils(CharmTestCase):
 
         _use_dvr.return_value = False
         self.os_release.return_value = 'icehouse'
+        self.lsb_release.return_value = {'DISTRIB_CODENAME': 'precise'}
         templating.OSConfigRenderer.side_effect = _mock_OSConfigRenderer
         _regconfs = nutils.register_configs()
         confs = ['/etc/neutron/neutron.conf',
@@ -224,6 +225,7 @@ class TestNeutronOVSUtils(CharmTestCase):
 
         _use_dvr.return_value = False
         self.os_release.return_value = 'mitaka'
+        self.lsb_release.return_value = {'DISTRIB_CODENAME': 'trusty'}
         templating.OSConfigRenderer.side_effect = _mock_OSConfigRenderer
         _regconfs = nutils.register_configs()
         confs = ['/etc/neutron/neutron.conf',
@@ -236,6 +238,7 @@ class TestNeutronOVSUtils(CharmTestCase):
     def test_resource_map(self, _use_dvr):
         _use_dvr.return_value = False
         self.os_release.return_value = 'icehouse'
+        self.lsb_release.return_value = {'DISTRIB_CODENAME': 'precise'}
         _map = nutils.resource_map()
         svcs = ['neutron-plugin-openvswitch-agent']
         confs = [nutils.NEUTRON_CONF]
@@ -246,6 +249,7 @@ class TestNeutronOVSUtils(CharmTestCase):
     def test_resource_map_mitaka(self, _use_dvr):
         _use_dvr.return_value = False
         self.os_release.return_value = 'mitaka'
+        self.lsb_release.return_value = {'DISTRIB_CODENAME': 'xenial'}
         _map = nutils.resource_map()
         svcs = ['neutron-openvswitch-agent']
         confs = [nutils.NEUTRON_CONF]
@@ -256,6 +260,7 @@ class TestNeutronOVSUtils(CharmTestCase):
     def test_resource_map_dvr(self, _use_dvr):
         _use_dvr.return_value = True
         self.os_release.return_value = 'icehouse'
+        self.lsb_release.return_value = {'DISTRIB_CODENAME': 'xenial'}
         _map = nutils.resource_map()
         svcs = ['neutron-plugin-openvswitch-agent', 'neutron-metadata-agent',
                 'neutron-l3-agent']
@@ -268,6 +273,8 @@ class TestNeutronOVSUtils(CharmTestCase):
     def test_resource_map_dhcp(self, _use_dvr, _enable_local_dhcp):
         _enable_local_dhcp.return_value = True
         _use_dvr.return_value = False
+        self.os_release.return_value = 'diablo'
+        self.lsb_release.return_value = {'DISTRIB_CODENAME': 'lucid'}
         _map = nutils.resource_map()
         svcs = ['neutron-plugin-openvswitch-agent', 'neutron-metadata-agent',
                 'neutron-dhcp-agent']
@@ -280,7 +287,7 @@ class TestNeutronOVSUtils(CharmTestCase):
     def test_resource_map_mtu_trusty(self, _use_dvr):
         _use_dvr.return_value = False
         self.os_release.return_value = 'mitaka'
-        self.lsb_release.return_value = {'DISTRIB_RELEASE': '14.04'}
+        self.lsb_release.return_value = {'DISTRIB_CODENAME': 'trusty'}
         _map = nutils.resource_map()
         self.assertTrue(nutils.NEUTRON_CONF in _map.keys())
         self.assertTrue(nutils.PHY_NIC_MTU_CONF in _map.keys())
@@ -293,7 +300,7 @@ class TestNeutronOVSUtils(CharmTestCase):
     def test_resource_map_mtu_xenial(self, _use_dvr):
         _use_dvr.return_value = False
         self.os_release.return_value = 'mitaka'
-        self.lsb_release.return_value = {'DISTRIB_RELEASE': '16.04'}
+        self.lsb_release.return_value = {'DISTRIB_CODENAME': 'xenial'}
         _map = nutils.resource_map()
         self.assertTrue(nutils.NEUTRON_CONF in _map.keys())
         self.assertFalse(nutils.PHY_NIC_MTU_CONF in _map.keys())
@@ -305,6 +312,8 @@ class TestNeutronOVSUtils(CharmTestCase):
     @patch.object(nutils, 'use_dvr')
     def test_restart_map(self, _use_dvr):
         _use_dvr.return_value = False
+        self.os_release.return_value = "diablo"
+        self.lsb_release.return_value = {'DISTRIB_CODENAME': 'lucid'}
         _restart_map = nutils.restart_map()
         ML2CONF = "/etc/neutron/plugins/ml2/ml2_conf.ini"
         expect = OrderedDict([
@@ -476,7 +485,8 @@ class TestNeutronOVSUtils(CharmTestCase):
                                       join, listdir):
         projects_yaml = openstack_origin_git
         join.return_value = 'joined-string'
-        self.lsb_release.return_value = {'DISTRIB_RELEASE': '15.04'}
+        self.lsb_release.return_value = {'DISTRIB_CODENAME': 'vivid'}
+        self.os_release.return_value = 'diablo'
         nutils.git_post_install(projects_yaml)
         expected = [
             call('joined-string', '/etc/neutron'),
@@ -530,7 +540,8 @@ class TestNeutronOVSUtils(CharmTestCase):
                                       join, listdir):
         projects_yaml = openstack_origin_git
         join.return_value = 'joined-string'
-        self.lsb_release.return_value = {'DISTRIB_RELEASE': '15.10'}
+        self.lsb_release.return_value = {'DISTRIB_CODENAME': 'wily'}
+        self.os_release.return_value = 'diablo'
         nutils.git_post_install(projects_yaml)
         expected = [
             call('git/neutron_sudoers', '/etc/sudoers.d/neutron_sudoers',