Merge lp:~ajkavanagh/charms/trusty/memcached/add-spaces-support into lp:charms/trusty/memcached

[Alex Kavanagh]

Add Juju space support to the interfaces so that Juju 2.0+ bindings work with the charm.

[David Ames (thedac)]

Alex,

It feels like we are re-inventing the wheel with space_aware_unit_address.

We should use charmhelprs.contrib.network.ip.get_relation_ip just as we have in other charms. Though it does not look like it, if get_relation_ip is missing some functionality let's add it there.

review: Needs Fixing

[Alex Kavanagh (ajkavanagh)]

Good catch David. I wrote the space_aware_unit_address() function before the get_relation_ip() came into existence. I'll re-work the changes to use that.

[lp:~ajkavanagh/charms/trusty/memcached/add-spaces-support]

78. By Alex Kavanagh on 2017-05-05

Removed the 'space_aware_unit_address()' function and replaced it with
the (new) charmhelpers.contrib.network.ip.get_relation_ip() function.

Updated the README.md file to indicate what space name strings exist in
the charm and what interfaces they are used for.

Synced charmhelpers to get the 'get_relation_ip()' function.

[David Ames (thedac)]

This looks good. Has it been tested?

[David Ames (thedac)]

Verbal confirmation from Alex that the amulet tests were run.

review: Approve

=== modified file 'README.md'
--- README.md	2015-03-26 01:01:14 +0000
+++ README.md	2017-05-05 10:32:43 +0000
@@ -83,6 +83,29 @@
 
 ## Known Limitations and Issues
 
+# Juju Network Space support
+
+The charm supports Juju network space bindings on its interfaces.  This is
+activated in Juju 2.0+ by either using a --bind during a deploy, or using
+--constraints.  If an interface is not bound to a space then the charm falls
+back to the default behaviour of using the unit's private address.
+
+The following network space strings are used in the charm to support binding to
+specific interfaces:
+
+ - 'cluster' for the interface: memcached-replication
+ - 'cache' for the interface: memcache
+ - 'munin' for the interface: munin-node
+ - 'monitors' for the interface: monitors
+ - 'nrpe-external-master' for the interface: nrpe-external-master
+
+Thus, for a Juju network space called 'memcache-space', the binding would be
+
+juju ... --bind='cache:memcache-space' ...
+
+
+Juju 1.25.x continues to use the private address of the unit.
+
 # Configuration
 
 Standard configuration options are provided, we recommend scanning the [Memcached documentation](https://code.google.com/p/memcached/wiki/NewConfiguringServer) before tweaking the default configuration.

=== modified file 'hooks/charmhelpers/contrib/network/ip.py'
--- hooks/charmhelpers/contrib/network/ip.py	2016-09-09 12:18:22 +0000
+++ hooks/charmhelpers/contrib/network/ip.py	2017-05-05 10:32:43 +0000
@@ -20,25 +20,38 @@
 
 from functools import partial
 
-from charmhelpers.core.hookenv import unit_get
 from charmhelpers.fetch import apt_install, apt_update
 from charmhelpers.core.hookenv import (
+    config,
     log,
+    network_get_primary_address,
+    unit_get,
     WARNING,
 )
 
+from charmhelpers.core.host import (
+    lsb_release,
+    CompareHostReleases,
+)
+
 try:
     import netifaces
 except ImportError:
     apt_update(fatal=True)
-    apt_install('python-netifaces', fatal=True)
+    if six.PY2:
+        apt_install('python-netifaces', fatal=True)
+    else:
+        apt_install('python3-netifaces', fatal=True)
     import netifaces
 
 try:
     import netaddr
 except ImportError:
     apt_update(fatal=True)
-    apt_install('python-netaddr', fatal=True)
+    if six.PY2:
+        apt_install('python-netaddr', fatal=True)
+    else:
+        apt_install('python3-netaddr', fatal=True)
     import netaddr
 
 
@@ -55,6 +68,24 @@
     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.
 
@@ -80,19 +111,17 @@
         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']
-                netmask = addresses[netifaces.AF_INET][0]['netmask']
-                cidr = netaddr.IPNetwork("%s/%s" % (addr, netmask))
-                if cidr in network:
-                    return str(cidr.ip)
+                for addr in addresses[netifaces.AF_INET]:
+                    cidr = netaddr.IPNetwork("%s/%s" % (addr['addr'],
+                                                        addr['netmask']))
+                    if cidr in network:
+                        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 = netaddr.IPNetwork("%s/%s" % (addr['addr'],
-                                                            addr['netmask']))
-                        if cidr in network:
-                            return str(cidr.ip)
+                    cidr = _get_ipv6_network_from_address(addr)
+                    if cidr and cidr in network:
+                        return str(cidr.ip)
 
     if fallback is not None:
         return fallback
@@ -168,18 +197,18 @@
 
         if address.version == 6 and netifaces.AF_INET6 in addresses:
             for addr in addresses[netifaces.AF_INET6]:
-                if not addr['addr'].startswith('fe80'):
-                    network = netaddr.IPNetwork("%s/%s" % (addr['addr'],
-                                                           addr['netmask']))
-                    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]
+                network = _get_ipv6_network_from_address(addr)
+                if not network:
+                    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]
     return None
 
 
@@ -210,6 +239,16 @@
     return None
 
 
+def is_ipv6_disabled():
+    try:
+        result = subprocess.check_output(
+            ['sysctl', 'net.ipv6.conf.all.disable_ipv6'],
+            stderr=subprocess.STDOUT)
+        return "net.ipv6.conf.all.disable_ipv6 = 1" in result
+    except subprocess.CalledProcessError:
+        return True
+
+
 def get_iface_addr(iface='eth0', inet_type='AF_INET', inc_aliases=False,
                    fatal=True, exc_list=None):
     """Return the assigned IP address for a given interface, if any.
@@ -406,7 +445,7 @@
         # Test to see if already an IPv4/IPv6 address
         address = netaddr.IPAddress(address)
         return True
-    except netaddr.AddrFormatError:
+    except (netaddr.AddrFormatError, ValueError):
         return False
 
 
@@ -414,7 +453,10 @@
     try:
         import dns.resolver
     except ImportError:
-        apt_install('python-dnspython', fatal=True)
+        if six.PY2:
+            apt_install('python-dnspython', fatal=True)
+        else:
+            apt_install('python3-dnspython', fatal=True)
         import dns.resolver
 
     if isinstance(address, dns.name.Name):
@@ -424,7 +466,11 @@
     else:
         return None
 
-    answers = dns.resolver.query(address, rtype)
+    try:
+        answers = dns.resolver.query(address, rtype)
+    except dns.resolver.NXDOMAIN:
+        return None
+
     if answers:
         return str(answers[0])
     return None
@@ -458,7 +504,10 @@
         try:
             import dns.reversename
         except ImportError:
-            apt_install("python-dnspython", fatal=True)
+            if six.PY2:
+                apt_install("python-dnspython", fatal=True)
+            else:
+                apt_install("python3-dnspython", fatal=True)
             import dns.reversename
 
         rev = dns.reversename.from_address(address)
@@ -495,3 +544,48 @@
     cmd = ['nc', '-z', address, str(port)]
     result = subprocess.call(cmd)
     return not(bool(result))
+
+
+def assert_charm_supports_ipv6():
+    """Check whether we are able to support charms ipv6."""
+    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")
+
+
+def get_relation_ip(interface, cidr_network=None):
+    """Return this unit's IP for the given interface.
+
+    Allow for an arbitrary interface to use with network-get to select an IP.
+    Handle all address selection options including passed cidr network and
+    IPv6.
+
+    Usage: get_relation_ip('amqp', cidr_network='10.0.0.0/8')
+
+    @param interface: string name of the relation.
+    @param cidr_network: string CIDR Network to select an address from.
+    @raises Exception if prefer-ipv6 is configured but IPv6 unsupported.
+    @returns IPv6 or IPv4 address
+    """
+    # Select the interface address first
+    # For possible use as a fallback bellow with get_address_in_network
+    try:
+        # Get the interface specific IP
+        address = network_get_primary_address(interface)
+    except NotImplementedError:
+        # If network-get is not available
+        address = get_host_ip(unit_get('private-address'))
+
+    if config('prefer-ipv6'):
+        # Currently IPv6 has priority, eventually we want IPv6 to just be
+        # another network space.
+        assert_charm_supports_ipv6()
+        return get_ipv6_addr()[0]
+    elif cidr_network:
+        # If a specific CIDR network is passed get the address from that
+        # network.
+        return get_address_in_network(cidr_network, address)
+
+    # Return the interface address
+    return address

=== modified file 'hooks/charmhelpers/contrib/network/ovs/__init__.py'
--- hooks/charmhelpers/contrib/network/ovs/__init__.py	2016-09-09 12:18:22 +0000
+++ hooks/charmhelpers/contrib/network/ovs/__init__.py	2017-05-05 10:32:43 +0000
@@ -15,13 +15,30 @@
 ''' Helpers for interacting with OpenvSwitch '''
 import subprocess
 import os
+import six
+
+from charmhelpers.fetch import apt_install
+
+
 from charmhelpers.core.hookenv import (
-    log, WARNING
+    log, WARNING, INFO, DEBUG
 )
 from charmhelpers.core.host import (
     service
 )
 
+BRIDGE_TEMPLATE = """\
+# This veth pair is required when neutron data-port is mapped to an existing linux bridge. lp:1635067
+
+auto {linuxbridge_port}
+iface {linuxbridge_port} inet manual
+    pre-up ip link add name {linuxbridge_port} type veth peer name {ovsbridge_port}
+    pre-up ip link set {ovsbridge_port} master {bridge}
+    pre-up ip link set {ovsbridge_port} up
+    up ip link set {linuxbridge_port} up
+    down ip link del {linuxbridge_port}
+"""
+
 
 def add_bridge(name, datapath_type=None):
     ''' Add the named bridge to openvswitch '''
@@ -60,6 +77,54 @@
     subprocess.check_call(["ip", "link", "set", port, "promisc", "off"])
 
 
+def add_ovsbridge_linuxbridge(name, bridge):
+    ''' Add linux bridge to the named openvswitch bridge
+    :param name: Name of ovs bridge to be added to Linux bridge
+    :param bridge: Name of Linux bridge to be added to ovs bridge
+    :returns: True if veth is added between ovs bridge and linux bridge,
+    False otherwise'''
+    try:
+        import netifaces
+    except ImportError:
+        if six.PY2:
+            apt_install('python-netifaces', fatal=True)
+        else:
+            apt_install('python3-netifaces', fatal=True)
+        import netifaces
+
+    ovsbridge_port = "veth-" + name
+    linuxbridge_port = "veth-" + bridge
+    log('Adding linuxbridge {} to ovsbridge {}'.format(bridge, name),
+        level=INFO)
+    interfaces = netifaces.interfaces()
+    for interface in interfaces:
+        if interface == ovsbridge_port or interface == linuxbridge_port:
+            log('Interface {} already exists'.format(interface), level=INFO)
+            return
+
+    with open('/etc/network/interfaces.d/{}.cfg'.format(
+            linuxbridge_port), 'w') as config:
+        config.write(BRIDGE_TEMPLATE.format(linuxbridge_port=linuxbridge_port,
+                                            ovsbridge_port=ovsbridge_port,
+                                            bridge=bridge))
+
+    subprocess.check_call(["ifup", linuxbridge_port])
+    add_bridge_port(name, linuxbridge_port)
+
+
+def is_linuxbridge_interface(port):
+    ''' Check if the interface is a linuxbridge bridge
+    :param port: Name of an interface to check whether it is a Linux bridge
+    :returns: True if port is a Linux bridge'''
+
+    if os.path.exists('/sys/class/net/' + port + '/bridge'):
+        log('Interface {} is a Linux bridge'.format(port), level=DEBUG)
+        return True
+    else:
+        log('Interface {} is not a Linux bridge'.format(port), level=DEBUG)
+        return False
+
+
 def set_manager(manager):
     ''' Set the controller for the local openvswitch '''
     log('Setting manager for local ovs to {}'.format(manager))

=== modified file 'hooks/charmhelpers/core/hookenv.py'
--- hooks/charmhelpers/core/hookenv.py	2016-09-09 12:18:22 +0000
+++ hooks/charmhelpers/core/hookenv.py	2017-05-05 10:32:43 +0000
@@ -332,6 +332,8 @@
     config_cmd_line = ['config-get']
     if scope is not None:
         config_cmd_line.append(scope)
+    else:
+        config_cmd_line.append('--all')
     config_cmd_line.append('--format=json')
     try:
         config_data = json.loads(
@@ -614,6 +616,20 @@
     subprocess.check_call(_args)
 
 
+def open_ports(start, end, protocol="TCP"):
+    """Opens a range of service network ports"""
+    _args = ['open-port']
+    _args.append('{}-{}/{}'.format(start, end, protocol))
+    subprocess.check_call(_args)
+
+
+def close_ports(start, end, protocol="TCP"):
+    """Close a range of service network ports"""
+    _args = ['close-port']
+    _args.append('{}-{}/{}'.format(start, end, protocol))
+    subprocess.check_call(_args)
+
+
 @cached
 def unit_get(attribute):
     """Get the unit ID for the remote unit"""
@@ -1019,3 +1035,34 @@
     '''
     cmd = ['network-get', '--primary-address', binding]
     return subprocess.check_output(cmd).decode('UTF-8').strip()
+
+
+def add_metric(*args, **kwargs):
+    """Add metric values. Values may be expressed with keyword arguments. For
+    metric names containing dashes, these may be expressed as one or more
+    'key=value' positional arguments. May only be called from the collect-metrics
+    hook."""
+    _args = ['add-metric']
+    _kvpairs = []
+    _kvpairs.extend(args)
+    _kvpairs.extend(['{}={}'.format(k, v) for k, v in kwargs.items()])
+    _args.extend(sorted(_kvpairs))
+    try:
+        subprocess.check_call(_args)
+        return
+    except EnvironmentError as e:
+        if e.errno != errno.ENOENT:
+            raise
+    log_message = 'add-metric failed: {}'.format(' '.join(_kvpairs))
+    log(log_message, level='INFO')
+
+
+def meter_status():
+    """Get the meter status, if running in the meter-status-changed hook."""
+    return os.environ.get('JUJU_METER_STATUS')
+
+
+def meter_info():
+    """Get the meter status information, if running in the meter-status-changed
+    hook."""
+    return os.environ.get('JUJU_METER_INFO')

=== modified file 'hooks/charmhelpers/core/host.py'
--- hooks/charmhelpers/core/host.py	2016-09-09 12:18:22 +0000
+++ hooks/charmhelpers/core/host.py	2017-05-05 10:32:43 +0000
@@ -45,6 +45,7 @@
         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,44 +53,145 @@
         add_new_group,
         lsb_release,
         cmp_pkgrevno,
+        CompareHostReleases,
     )  # flake8: noqa -- ignore F401 for this import
 
-
-def service_start(service_name):
-    """Start a system service"""
-    return service('start', service_name)
-
-
-def service_stop(service_name):
-    """Stop a system service"""
-    return service('stop', service_name)
-
-
-def service_restart(service_name):
-    """Restart a system service"""
+UPDATEDB_PATH = '/etc/updatedb.conf'
+
+def service_start(service_name, **kwargs):
+    """Start a system service.
+
+    The specified service name is managed via the system level init system.
+    Some init systems (e.g. upstart) require that additional arguments be
+    provided in order to directly control service instances whereas other init
+    systems allow for addressing instances of a service directly by name (e.g.
+    systemd).
+
+    The kwargs allow for the additional parameters to be passed to underlying
+    init systems for those systems which require/allow for them. For example,
+    the ceph-osd upstart script requires the id parameter to be passed along
+    in order to identify which running daemon should be reloaded. The follow-
+    ing example stops the ceph-osd service for instance id=4:
+
+    service_stop('ceph-osd', id=4)
+
+    :param service_name: the name of the service to stop
+    :param **kwargs: additional parameters to pass to the init system when
+                     managing services. These will be passed as key=value
+                     parameters to the init system's commandline. kwargs
+                     are ignored for systemd enabled systems.
+    """
+    return service('start', service_name, **kwargs)
+
+
+def service_stop(service_name, **kwargs):
+    """Stop a system service.
+
+    The specified service name is managed via the system level init system.
+    Some init systems (e.g. upstart) require that additional arguments be
+    provided in order to directly control service instances whereas other init
+    systems allow for addressing instances of a service directly by name (e.g.
+    systemd).
+
+    The kwargs allow for the additional parameters to be passed to underlying
+    init systems for those systems which require/allow for them. For example,
+    the ceph-osd upstart script requires the id parameter to be passed along
+    in order to identify which running daemon should be reloaded. The follow-
+    ing example stops the ceph-osd service for instance id=4:
+
+    service_stop('ceph-osd', id=4)
+
+    :param service_name: the name of the service to stop
+    :param **kwargs: additional parameters to pass to the init system when
+                     managing services. These will be passed as key=value
+                     parameters to the init system's commandline. kwargs
+                     are ignored for systemd enabled systems.
+    """
+    return service('stop', service_name, **kwargs)
+
+
+def service_restart(service_name, **kwargs):
+    """Restart a system service.
+
+    The specified service name is managed via the system level init system.
+    Some init systems (e.g. upstart) require that additional arguments be
+    provided in order to directly control service instances whereas other init
+    systems allow for addressing instances of a service directly by name (e.g.
+    systemd).
+
+    The kwargs allow for the additional parameters to be passed to underlying
+    init systems for those systems which require/allow for them. For example,
+    the ceph-osd upstart script requires the id parameter to be passed along
+    in order to identify which running daemon should be restarted. The follow-
+    ing example restarts the ceph-osd service for instance id=4:
+
+    service_restart('ceph-osd', id=4)
+
+    :param service_name: the name of the service to restart
+    :param **kwargs: additional parameters to pass to the init system when
+                     managing services. These will be passed as key=value
+                     parameters to the  init system's commandline. kwargs
+                     are ignored for init systems not allowing additional
+                     parameters via the commandline (systemd).
+    """
     return service('restart', service_name)
 
 
-def service_reload(service_name, restart_on_failure=False):
+def service_reload(service_name, restart_on_failure=False, **kwargs):
     """Reload a system service, optionally falling back to restart if
-    reload fails"""
-    service_result = service('reload', service_name)
+    reload fails.
+
+    The specified service name is managed via the system level init system.
+    Some init systems (e.g. upstart) require that additional arguments be
+    provided in order to directly control service instances whereas other init
+    systems allow for addressing instances of a service directly by name (e.g.
+    systemd).
+
+    The kwargs allow for the additional parameters to be passed to underlying
+    init systems for those systems which require/allow for them. For example,
+    the ceph-osd upstart script requires the id parameter to be passed along
+    in order to identify which running daemon should be reloaded. The follow-
+    ing example restarts the ceph-osd service for instance id=4:
+
+    service_reload('ceph-osd', id=4)
+
+    :param service_name: the name of the service to reload
+    :param restart_on_failure: boolean indicating whether to fallback to a
+                               restart if the reload fails.
+    :param **kwargs: additional parameters to pass to the init system when
+                     managing services. These will be passed as key=value
+                     parameters to the  init system's commandline. kwargs
+                     are ignored for init systems not allowing additional
+                     parameters via the commandline (systemd).
+    """
+    service_result = service('reload', service_name, **kwargs)
     if not service_result and restart_on_failure:
-        service_result = service('restart', service_name)
+        service_result = service('restart', service_name, **kwargs)
     return service_result
 
 
-def service_pause(service_name, init_dir="/etc/init", initd_dir="/etc/init.d"):
+def service_pause(service_name, init_dir="/etc/init", initd_dir="/etc/init.d",
+                  **kwargs):
     """Pause a system service.
 
-    Stop it, and prevent it from starting again at boot."""
+    Stop it, and prevent it from starting again at boot.
+
+    :param service_name: the name of the service to pause
+    :param init_dir: path to the upstart init directory
+    :param initd_dir: path to the sysv init directory
+    :param **kwargs: additional parameters to pass to the init system when
+                     managing services. These will be passed as key=value
+                     parameters to the init system's commandline. kwargs
+                     are ignored for init systems which do not support
+                     key=value arguments via the commandline.
+    """
     stopped = True
-    if service_running(service_name):
-        stopped = service_stop(service_name)
+    if service_running(service_name, **kwargs):
+        stopped = service_stop(service_name, **kwargs)
     upstart_file = os.path.join(init_dir, "{}.conf".format(service_name))
     sysv_file = os.path.join(initd_dir, service_name)
     if init_is_systemd():
-        service('disable', service_name)
+        service('mask', service_name)
     elif os.path.exists(upstart_file):
         override_path = os.path.join(
             init_dir, '{}.override'.format(service_name))
@@ -106,14 +208,23 @@
 
 
 def service_resume(service_name, init_dir="/etc/init",
-                   initd_dir="/etc/init.d"):
+                   initd_dir="/etc/init.d", **kwargs):
     """Resume a system service.
 
-    Reenable starting again at boot. Start the service"""
+    Reenable starting again at boot. Start the service.
+
+    :param service_name: the name of the service to resume
+    :param init_dir: the path to the init dir
+    :param initd dir: the path to the initd dir
+    :param **kwargs: additional parameters to pass to the init system when
+                     managing services. These will be passed as key=value
+                     parameters to the init system's commandline. kwargs
+                     are ignored for systemd enabled systems.
+    """
     upstart_file = os.path.join(init_dir, "{}.conf".format(service_name))
     sysv_file = os.path.join(initd_dir, service_name)
     if init_is_systemd():
-        service('enable', service_name)
+        service('unmask', service_name)
     elif os.path.exists(upstart_file):
         override_path = os.path.join(
             init_dir, '{}.override'.format(service_name))
@@ -126,19 +237,28 @@
             "Unable to detect {0} as SystemD, Upstart {1} or"
             " SysV {2}".format(
                 service_name, upstart_file, sysv_file))
+    started = service_running(service_name, **kwargs)
 
-    started = service_running(service_name)
     if not started:
-        started = service_start(service_name)
+        started = service_start(service_name, **kwargs)
     return started
 
 
-def service(action, service_name):
-    """Control a system service"""
+def service(action, service_name, **kwargs):
+    """Control a system service.
+
+    :param action: the action to take on the service
+    :param service_name: the name of the service to perform th action on
+    :param **kwargs: additional params to be passed to the service command in
+                    the form of key=value.
+    """
     if init_is_systemd():
         cmd = ['systemctl', action, service_name]
     else:
         cmd = ['service', service_name, action]
+        for key, value in six.iteritems(kwargs):
+            parameter = '%s=%s' % (key, value)
+            cmd.append(parameter)
     return subprocess.call(cmd) == 0
 
 
@@ -146,15 +266,26 @@
 _INIT_D_CONF = "/etc/init.d/{}"
 
 
-def service_running(service_name):
-    """Determine whether a system service is running"""
+def service_running(service_name, **kwargs):
+    """Determine whether a system service is running.
+
+    :param service_name: the name of the service
+    :param **kwargs: additional args to pass to the service command. This is
+                     used to pass additional key=value arguments to the
+                     service command line for managing specific instance
+                     units (e.g. service ceph-osd status id=2). The kwargs
+                     are ignored in systemd services.
+    """
     if init_is_systemd():
         return service('is-active', service_name)
     else:
         if os.path.exists(_UPSTART_CONF.format(service_name)):
             try:
-                output = subprocess.check_output(
-                    ['status', service_name],
+                cmd = ['status', service_name]
+                for key, value in six.iteritems(kwargs):
+                    parameter = '%s=%s' % (key, value)
+                    cmd.append(parameter)
+                output = subprocess.check_output(cmd,
                     stderr=subprocess.STDOUT).decode('UTF-8')
             except subprocess.CalledProcessError:
                 return False
@@ -177,6 +308,8 @@
 
 def init_is_systemd():
     """Return True if the host system uses systemd, False otherwise."""
+    if lsb_release()['DISTRIB_CODENAME'] == 'trusty':
+        return False
     return os.path.isdir(SYSTEMD_SYSTEM)
 
 
@@ -306,15 +439,17 @@
     subprocess.check_call(cmd)
 
 
-def rsync(from_path, to_path, flags='-r', options=None):
+def rsync(from_path, to_path, flags='-r', options=None, timeout=None):
     """Replicate the contents of a path"""
     options = options or ['--delete', '--executability']
     cmd = ['/usr/bin/rsync', flags]
+    if timeout:
+        cmd = ['timeout', str(timeout)] + cmd
     cmd.extend(options)
     cmd.append(from_path)
     cmd.append(to_path)
     log(" ".join(cmd))
-    return subprocess.check_output(cmd).decode('UTF-8').strip()
+    return subprocess.check_output(cmd, stderr=subprocess.STDOUT).decode('UTF-8').strip()
 
 
 def symlink(source, destination):
@@ -684,7 +819,7 @@
     :param str path: The string path to start changing ownership.
     :param str owner: The owner string to use when looking up the uid.
     :param str group: The group string to use when looking up the gid.
-    :param bool follow_links: Also Chown links if True
+    :param bool follow_links: Also follow and chown links if True
     :param bool chowntopdir: Also chown path itself if True
     """
     uid = pwd.getpwnam(owner).pw_uid
@@ -698,7 +833,7 @@
         broken_symlink = os.path.lexists(path) and not os.path.exists(path)
         if not broken_symlink:
             chown(path, uid, gid)
-    for root, dirs, files in os.walk(path):
+    for root, dirs, files in os.walk(path, followlinks=follow_links):
         for name in dirs + files:
             full = os.path.join(root, name)
             broken_symlink = os.path.lexists(full) and not os.path.exists(full)
@@ -718,6 +853,20 @@
     chownr(path, owner, group, follow_links=False)
 
 
+def owner(path):
+    """Returns a tuple containing the username & groupname owning the path.
+
+    :param str path: the string path to retrieve the ownership
+    :return tuple(str, str): A (username, groupname) tuple containing the
+                             name of the user and group owning the path.
+    :raises OSError: if the specified path does not exist
+    """
+    stat = os.stat(path)
+    username = pwd.getpwuid(stat.st_uid)[0]
+    groupname = grp.getgrgid(stat.st_gid)[0]
+    return username, groupname
+
+
 def get_total_ram():
     """The total amount of system RAM in bytes.
 
@@ -732,3 +881,42 @@
                     assert unit == 'kB', 'Unknown unit'
                     return int(value) * 1024  # Classic, not KiB.
         raise NotImplementedError()
+
+
+UPSTART_CONTAINER_TYPE = '/run/container_type'
+
+
+def is_container():
+    """Determine whether unit is running in a container
+
+    @return: boolean indicating if unit is in a container
+    """
+    if init_is_systemd():
+        # Detect using systemd-detect-virt
+        return subprocess.call(['systemd-detect-virt',
+                                '--container']) == 0
+    else:
+        # Detect using upstart container file marker
+        return os.path.exists(UPSTART_CONTAINER_TYPE)
+
+
+def add_to_updatedb_prunepath(path, updatedb_path=UPDATEDB_PATH):
+    with open(updatedb_path, 'r+') as f_id:
+        updatedb_text = f_id.read()
+        output = updatedb(updatedb_text, path)
+        f_id.seek(0)
+        f_id.write(output)
+        f_id.truncate()
+
+
+def updatedb(updatedb_text, new_path):
+    lines = [line for line in updatedb_text.split("\n")]
+    for i, line in enumerate(lines):
+        if line.startswith("PRUNEPATHS="):
+            paths_line = line.split("=")[1].replace('"', '')
+            paths = paths_line.split(" ")
+            if new_path not in paths:
+                paths.append(new_path)
+                lines[i] = 'PRUNEPATHS="{}"'.format(' '.join(paths))
+    output = "\n".join(lines)
+    return output

=== modified file 'hooks/charmhelpers/core/host_factory/centos.py'
--- hooks/charmhelpers/core/host_factory/centos.py	2016-09-09 12:18:22 +0000
+++ hooks/charmhelpers/core/host_factory/centos.py	2017-05-05 10:32:43 +0000
@@ -2,6 +2,22 @@
 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."""

=== modified file 'hooks/charmhelpers/core/host_factory/ubuntu.py'
--- hooks/charmhelpers/core/host_factory/ubuntu.py	2016-09-09 12:18:22 +0000
+++ hooks/charmhelpers/core/host_factory/ubuntu.py	2017-05-05 10:32:43 +0000
@@ -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"""

=== modified file 'hooks/charmhelpers/core/kernel_factory/ubuntu.py'
--- hooks/charmhelpers/core/kernel_factory/ubuntu.py	2016-09-09 12:18:22 +0000
+++ hooks/charmhelpers/core/kernel_factory/ubuntu.py	2017-05-05 10:32:43 +0000
@@ -5,7 +5,7 @@
     """Load a kernel module and configure for auto-load on reboot."""
     with open('/etc/modules', 'r+') as modules:
         if module not in modules.read():
-            modules.write(module)
+            modules.write(module + "\n")
 
 
 def update_initramfs(version='all'):

=== modified file 'hooks/charmhelpers/core/strutils.py'
--- hooks/charmhelpers/core/strutils.py	2016-09-09 12:18:22 +0000
+++ hooks/charmhelpers/core/strutils.py	2017-05-05 10:32:43 +0000
@@ -68,3 +68,56 @@
         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]

=== modified file 'hooks/charmhelpers/fetch/__init__.py'
--- hooks/charmhelpers/fetch/__init__.py	2016-09-09 12:18:22 +0000
+++ hooks/charmhelpers/fetch/__init__.py	2017-05-05 10:32:43 +0000
@@ -92,6 +92,7 @@
     apt_mark = fetch.apt_mark
     apt_hold = fetch.apt_hold
     apt_unhold = fetch.apt_unhold
+    get_upstream_version = fetch.get_upstream_version
 elif __platform__ == "centos":
     yum_search = fetch.yum_search
 

=== added file 'hooks/charmhelpers/fetch/snap.py'
--- hooks/charmhelpers/fetch/snap.py	1970-01-01 00:00:00 +0000
+++ hooks/charmhelpers/fetch/snap.py	2017-05-05 10:32:43 +0000
@@ -0,0 +1,122 @@
+# Copyright 2014-2017 Canonical Limited.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#  http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+"""
+Charm helpers snap for classic charms.
+
+If writing reactive charms, use the snap layer:
+https://lists.ubuntu.com/archives/snapcraft/2016-September/001114.html
+"""
+import subprocess
+from os import environ
+from time import sleep
+from charmhelpers.core.hookenv import log
+
+__author__ = 'Joseph Borg <[e-mail]>'
+
+SNAP_NO_LOCK = 1  # The return code for "couldn't acquire lock" in Snap (hopefully this will be improved).
+SNAP_NO_LOCK_RETRY_DELAY = 10  # Wait X seconds between Snap lock checks.
+SNAP_NO_LOCK_RETRY_COUNT = 30  # Retry to acquire the lock X times.
+
+
+class CouldNotAcquireLockException(Exception):
+    pass
+
+
+def _snap_exec(commands):
+    """
+    Execute snap commands.
+
+    :param commands: List commands
+    :return: Integer exit code
+    """
+    assert type(commands) == list
+
+    retry_count = 0
+    return_code = None
+
+    while return_code is None or return_code == SNAP_NO_LOCK:
+        try:
+            return_code = subprocess.check_call(['snap'] + commands, env=environ)
+        except subprocess.CalledProcessError as e:
+            retry_count += + 1
+            if retry_count > SNAP_NO_LOCK_RETRY_COUNT:
+                raise CouldNotAcquireLockException('Could not aquire lock after %s attempts' % SNAP_NO_LOCK_RETRY_COUNT)
+            return_code = e.returncode
+            log('Snap failed to acquire lock, trying again in %s seconds.' % SNAP_NO_LOCK_RETRY_DELAY, level='WARN')
+            sleep(SNAP_NO_LOCK_RETRY_DELAY)
+
+    return return_code
+
+
+def snap_install(packages, *flags):
+    """
+    Install a snap package.
+
+    :param packages: String or List String package name
+    :param flags: List String flags to pass to install command
+    :return: Integer return code from snap
+    """
+    if type(packages) is not list:
+        packages = [packages]
+
+    flags = list(flags)
+
+    message = 'Installing snap(s) "%s"' % ', '.join(packages)
+    if flags:
+        message += ' with option(s) "%s"' % ', '.join(flags)
+
+    log(message, level='INFO')
+    return _snap_exec(['install'] + flags + packages)
+
+
+def snap_remove(packages, *flags):
+    """
+    Remove a snap package.
+
+    :param packages: String or List String package name
+    :param flags: List String flags to pass to remove command
+    :return: Integer return code from snap
+    """
+    if type(packages) is not list:
+        packages = [packages]
+
+    flags = list(flags)
+
+    message = 'Removing snap(s) "%s"' % ', '.join(packages)
+    if flags:
+        message += ' with options "%s"' % ', '.join(flags)
+
+    log(message, level='INFO')
+    return _snap_exec(['remove'] + flags + packages)
+
+
+def snap_refresh(packages, *flags):
+    """
+    Refresh / Update snap package.
+
+    :param packages: String or List String package name
+    :param flags: List String flags to pass to refresh command
+    :return: Integer return code from snap
+    """
+    if type(packages) is not list:
+        packages = [packages]
+
+    flags = list(flags)
+
+    message = 'Refreshing snap(s) "%s"' % ', '.join(packages)
+    if flags:
+        message += ' with options "%s"' % ', '.join(flags)
+
+    log(message, level='INFO')
+    return _snap_exec(['refresh'] + flags + packages)

=== modified file 'hooks/charmhelpers/fetch/ubuntu.py'
--- hooks/charmhelpers/fetch/ubuntu.py	2016-09-09 12:18:22 +0000
+++ hooks/charmhelpers/fetch/ubuntu.py	2017-05-05 10:32:43 +0000
@@ -24,11 +24,14 @@
 from charmhelpers.core.hookenv import log
 from charmhelpers.fetch import SourceConfigError
 
-CLOUD_ARCHIVE = ('# Ubuntu Cloud Archive deb'
-                 ' http://ubuntu-cloud.archive.canonical.com/ubuntu'
-                 ' {} main')
-PROPOSED_POCKET = ('# Proposed deb http://archive.ubuntu.com/ubuntu'
-                   ' {}-proposed main universe multiverse restricted')
+CLOUD_ARCHIVE = """# Ubuntu Cloud Archive
+deb http://ubuntu-cloud.archive.canonical.com/ubuntu {} main
+"""
+
+PROPOSED_POCKET = """# Proposed
+deb http://archive.ubuntu.com/ubuntu {}-proposed main universe multiverse restricted
+"""
+
 CLOUD_ARCHIVE_POCKETS = {
     # Folsom
     'folsom': 'precise-updates/folsom',
@@ -102,11 +105,19 @@
     'newton/proposed': 'xenial-proposed/newton',
     'xenial-newton/proposed': 'xenial-proposed/newton',
     'xenial-proposed/newton': 'xenial-proposed/newton',
+    # Ocata
+    'ocata': 'xenial-updates/ocata',
+    'xenial-ocata': 'xenial-updates/ocata',
+    'xenial-ocata/updates': 'xenial-updates/ocata',
+    'xenial-updates/ocata': 'xenial-updates/ocata',
+    'ocata/proposed': 'xenial-proposed/ocata',
+    'xenial-ocata/proposed': 'xenial-proposed/ocata',
+    'xenial-ocata/newton': 'xenial-proposed/ocata',
 }
 
 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.
+CMD_RETRY_DELAY = 10  # Wait 10 seconds between command retries.
+CMD_RETRY_COUNT = 30  # Retry a failing fatal command X times.
 
 
 def filter_installed_packages(packages):
@@ -238,7 +249,8 @@
         source.startswith('http') or
         source.startswith('deb ') or
             source.startswith('cloud-archive:')):
-        subprocess.check_call(['add-apt-repository', '--yes', source])
+        cmd = ['add-apt-repository', '--yes', source]
+        _run_with_retries(cmd)
     elif source.startswith('cloud:'):
         install(filter_installed_packages(['ubuntu-cloud-keyring']),
                 fatal=True)
@@ -275,39 +287,78 @@
                                    key])
 
 
+def _run_with_retries(cmd, max_retries=CMD_RETRY_COUNT, retry_exitcodes=(1,),
+                      retry_message="", cmd_env=None):
+    """Run a command and retry until success or max_retries is reached.
+
+    :param: cmd: str: The apt command to run.
+    :param: max_retries: int: The number of retries to attempt on a fatal
+        command. Defaults to CMD_RETRY_COUNT.
+    :param: retry_exitcodes: tuple: Optional additional exit codes to retry.
+        Defaults to retry on exit code 1.
+    :param: retry_message: str: Optional log prefix emitted during retries.
+    :param: cmd_env: dict: Environment variables to add to the command run.
+    """
+
+    env = os.environ.copy()
+    if cmd_env:
+        env.update(cmd_env)
+
+    if not retry_message:
+        retry_message = "Failed executing '{}'".format(" ".join(cmd))
+    retry_message += ". Will retry in {} seconds".format(CMD_RETRY_DELAY)
+
+    retry_count = 0
+    result = None
+
+    retry_results = (None,) + retry_exitcodes
+    while result in retry_results:
+        try:
+            result = subprocess.check_call(cmd, env=env)
+        except subprocess.CalledProcessError as e:
+            retry_count = retry_count + 1
+            if retry_count > max_retries:
+                raise
+            result = e.returncode
+            log(retry_message)
+            time.sleep(CMD_RETRY_DELAY)
+
+
 def _run_apt_command(cmd, fatal=False):
-    """Run an APT command.
-
-    Checks the output and retries if the fatal flag is set
-    to True.
-
-    :param: cmd: str: The apt command to run.
+    """Run an apt command with optional retries.
+
     :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'
+    # Provide DEBIAN_FRONTEND=noninteractive if not present in the environment.
+    cmd_env = {
+        'DEBIAN_FRONTEND': os.environ.get('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 as 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)
-
+        _run_with_retries(
+            cmd, cmd_env=cmd_env, retry_exitcodes=(1, APT_NO_LOCK,),
+            retry_message="Couldn't acquire DPKG lock")
     else:
+        env = os.environ.copy()
+        env.update(cmd_env)
         subprocess.call(cmd, env=env)
+
+
+def get_upstream_version(package):
+    """Determine upstream version based on installed package
+
+    @returns None (if not installed) or the upstream version
+    """
+    import apt_pkg
+    cache = apt_cache()
+    try:
+        pkg = cache[package]
+    except:
+        # the package is unknown to the current apt cache.
+        return None
+
+    if not pkg.current_ver:
+        # package is known, but no version is currently installed.
+        return None
+
+    return apt_pkg.upstream_version(pkg.current_ver.ver_str)

=== modified file 'hooks/charmhelpers/osplatform.py'
--- hooks/charmhelpers/osplatform.py	2016-09-09 13:00:43 +0000
+++ hooks/charmhelpers/osplatform.py	2017-05-05 10:32:43 +0000
@@ -8,12 +8,18 @@
     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))

=== modified file 'hooks/memcached_hooks.py'
--- hooks/memcached_hooks.py	2016-09-20 18:01:20 +0000
+++ hooks/memcached_hooks.py	2017-05-05 10:32:43 +0000
@@ -10,10 +10,10 @@
     config,
     local_unit,
     log,
+    open_port,
     relation_ids,
     relation_get,
     relation_set,
-    unit_get,
     hook_name,
     Hooks,
     UnregisteredHookError,
@@ -31,11 +31,16 @@
     peer_units,
 )
 
-from charmhelpers.core.hookenv import open_port
 from charmhelpers.fetch import apt_install, apt_update, add_source
 from charmhelpers.contrib.network import ufw
+from charmhelpers.contrib.network.ip import get_relation_ip
 
-import memcached_utils
+from memcached_utils import (
+    dpkg_info_contains,
+    grant_access,
+    munin_format_ip,
+    revoke_access,
+)
 import replication
 
 __author__ = 'Felipe Reyes <[e-mail]>'
@@ -127,7 +132,8 @@
             level='INFO')
         secondary = relation_get('replica')
         try:
-            if secondary == unit_get('private-address'):
+            this_address = get_relation_ip('cluster')
+            if secondary == this_address:
                 replication.store_replica(relation_get('master'), secondary)
                 return config_changed(replica=relation_get('master'))
             elif secondary is None:
@@ -148,7 +154,9 @@
 @hooks.hook('cache-relation-joined')
 def cache_relation_joined():
 
-    settings = {'host': unit_get('private-address'),
+    # advertise our network space bind address, if set, otherwise fall back to
+    # the unit_get
+    settings = {'host': get_relation_ip('cache'),
                 'port': config('tcp-port'),
                 'udp-port': config('udp-port')}
 
@@ -158,14 +166,14 @@
     addr = relation_get('private-address')
     if addr:
         log('Granting memcached access to {}'.format(addr), level='INFO')
-        memcached_utils.grant_access(addr)
+        grant_access(addr)
 
 
 @hooks.hook('cache-relation-departed')
 def cache_relation_departed():
     addr = relation_get('private-address')
     log('Revoking memcached access to {}'.format(addr))
-    memcached_utils.revoke_access(addr)
+    revoke_access(addr)
 
 
 @hooks.hook('config-changed')
@@ -182,9 +190,7 @@
         # If repcached was enabled after install, we need
         # to make sure to install the memcached package with replication
         # enabled.
-        if not memcached_utils.dpkg_info_contains('memcached',
-                                                  'version',
-                                                  'repcache'):
+        if not dpkg_info_contains('memcached', 'version', 'repcache'):
             add_source(config('repcached_origin'))
             apt_update(fatal=True)
             apt_install(["memcached"], fatal=True)
@@ -273,7 +279,7 @@
         log('Remote node must provide IP', level='INFO')
         sys.exit(0)
 
-    munin_server_ip = memcached_utils.munin_format_ip(remote_ip)
+    munin_server_ip = munin_format_ip(remote_ip)
 
     # make sure munin port is open, the access is restricted by munin
     ufw.service('4949', 'open')
@@ -284,7 +290,7 @@
     apt_install(['munin-node'], fatal=True)
     configs = {'munin_server': munin_server_ip}
     templating.render('munin-node.conf', ETC_MUNIN_NODE_CONF, configs)
-    relation_set(ip=unit_get('private-address'))
+    relation_set(ip=get_relation_ip('munin'))
 
 
 @hooks.hook('nrpe-external-master-relation-changed')
@@ -334,8 +340,9 @@
         with open('monitors.yaml') as mon_file:
             mon_data = mon_file.read()
 
+        target_address = get_relation_ip('monitors')
         relation_set(monitors=mon_data, target_id=nagios_hostname,
-                     target_address=unit_get('private-address'))
+                     target_address=target_address)
 
     service_reload('nagios-nrpe-server')
 

=== modified file 'hooks/replication.py'
--- hooks/replication.py	2016-11-03 17:07:37 +0000
+++ hooks/replication.py	2017-05-05 10:32:43 +0000
@@ -2,7 +2,6 @@
     log,
     relation_id,
     relation_set,
-    unit_get,
 )
 
 from charmhelpers.contrib.hahelpers.cluster import (
@@ -10,6 +9,8 @@
     peer_units,
     peer_ips,
 )
+from charmhelpers.contrib.network.ip import get_relation_ip
+
 
 REPCACHED_REPLICA_FILE = "/var/run/repcached_replica"
 REPCACHED_MASTER_WAIT = 10
@@ -25,7 +26,7 @@
 
     if replica:
         master, secondary = replica
-        ip_addr = unit_get('private-address')
+        ip_addr = get_relation_ip('cluster')
         if ip_addr == master:
             replica = secondary
         elif ip_addr == secondary:
@@ -44,12 +45,12 @@
                 peer_unit = list(peer_ips().values())[0]
 
                 if oldest_peer(peers):
-                    master, secondary = (unit_get('private-address'),
+                    master, secondary = (get_relation_ip('cluster'),
                                          peer_unit)
                     replica = secondary
                 else:
                     master, secondary = (peer_unit,
-                                         unit_get('private-address'))
+                                         get_relation_ip('cluster'))
                     replica = master
 
                 store_replica(master, secondary)
@@ -73,7 +74,7 @@
     replica = list(peer_ips().values())[0]
     log("Setting replica unit: %s" % replica)
 
-    master = unit_get('private-address')
+    master = get_relation_ip('cluster')
     relation_set(relation_id(), {
         'master': master,
         'replica': replica,

=== modified file 'unit_tests/test_memcached_hooks.py'
--- unit_tests/test_memcached_hooks.py	2016-11-23 21:14:14 +0000
+++ unit_tests/test_memcached_hooks.py	2017-05-05 10:32:43 +0000
@@ -17,7 +17,9 @@
     'relation_ids',
     'relation_set',
     'relation_get',
-    'unit_get',
+    'get_relation_ip',
+    'grant_access',
+    'dpkg_info_contains',
     'config',
     'log',
     'oldest_peer',
@@ -93,10 +95,9 @@
 
     @mock.patch('subprocess.check_output')
     @mock.patch('memcached_hooks.relation_get')
-    @mock.patch('memcached_utils.grant_access')
     @mock.patch('memcached_utils.config')
     @mock.patch('memcached_utils.log')
-    def test_cache_relation_joined(self, log, config, grant_access,
+    def test_cache_relation_joined(self, log, config,
                                    relation_get, check_output):
         configs = {'tcp-port': '1234', 'udp-port': '3456'}
 
@@ -106,7 +107,7 @@
         self.config.side_effect = f
         config.side_effect = f
         relation_get.return_value = '127.0.1.1'
-        self.unit_get.return_value = '127.0.0.1'
+        self.get_relation_ip.return_value = '127.0.0.1'
         self.relation_ids.return_value = ['cache:1', 'cache:2']
         memcached_hooks.cache_relation_joined()
         self.relation_set.assert_any_call('cache:1', **{'host': '127.0.0.1',
@@ -115,7 +116,7 @@
         self.relation_set.assert_any_call('cache:2', **{'host': '127.0.0.1',
                                                         'port': '1234',
                                                         'udp-port': '3456'})
-        grant_access.assert_called_with('127.0.1.1')
+        self.grant_access.assert_called_with('127.0.1.1')
 
     @mock.patch('subprocess.check_output')
     @mock.patch('memcached_hooks.relation_get')
@@ -287,7 +288,6 @@
     @mock.patch('os.fchown')
     @mock.patch('os.chown')
     @mock.patch('memcached_utils.log')
-    @mock.patch('memcached_utils.dpkg_info_contains')
     @mock.patch('subprocess.Popen')
     @mock.patch('memcached_utils.config')
     @mock.patch('subprocess.check_output')
@@ -299,7 +299,7 @@
     @mock.patch('memcached_hooks.config_changed')
     def test_upgrade_charm(self, config_changed, add_source, log,
                            charm_dir, service,
-                           enable, check_output, config, popen, dpkg, *args):
+                           enable, check_output, config, popen, *args):
         configs = {
             'tcp-port': '1234', 'udp-port': '3456',
             'allow-ufw-ip6-softfail': False,
@@ -323,7 +323,7 @@
         p.configure_mock(**{'communicate.return_value': ('stdout', 'stderr'),
                             'returncode': 0})
         popen.return_value = p
-        dpkg.return_value = True
+        self.dpkg_info_contains.return_value = True
 
         charm_dir.return_value = os.path.join(DOT, '..')
         config.return_value = 12111
@@ -406,7 +406,7 @@
             return relations.get(k, None)
 
         self.relation_get.side_effect = r
-        self.unit_get.return_value = '10.0.0.2'
+        self.get_relation_ip.return_value = '10.0.0.2'
 
         self.peer_units.return_value = [0, 0]
         self.oldest_peer.return_value = False
@@ -433,7 +433,7 @@
             return relations.get(k, None)
 
         self.relation_get.side_effect = r
-        self.unit_get.return_value = '10.0.0.3'
+        self.get_relation_ip.return_value = '10.0.0.3'
 
         self.peer_units.return_value = [0, 0]
         self.oldest_peer.return_value = False
@@ -442,16 +442,16 @@
 
     @mock.patch('memcached_hooks.open_port')
     @mock.patch('replication.get_current_replica')
-    @mock.patch('replication.unit_get')
+    @mock.patch('replication.get_relation_ip')
     @mock.patch('memcached_hooks.ufw.modify_access')
     @mock.patch('memcached_hooks.ufw.service')
     @mock.patch('memcached_hooks.cache_relation_joined')
     @mock.patch('charmhelpers.core.templating.render')
     @mock.patch('replication.get_repcached_replica')
-    @mock.patch('memcached_utils.dpkg_info_contains')
-    def test_config_changed_replica(self, dpkg, get_replica, render,
+    def test_config_changed_replica(self, get_replica, render,
                                     cache_joined, ufw_svc, ufw_modify,
-                                    unit_get, replica, open_port):
+                                    get_relation_ip,
+                                    replica, open_port):
         params = {
             'tcp_port': 11211,
             'disable_cas': False,
@@ -469,7 +469,7 @@
             'disable_auto_cleanup': False,
         }
 
-        dpkg.return_value = True
+        self.dpkg_info_contains.return_value = True
         replica.return_value = ('10.0.0.2')
 
         configs = get_default_config()