Rationalize endpoint generation, rework tests, add support for multiple network support

.project

@@ -0,0 +1,17 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<projectDescription>
+	<name>neutron-api</name>
+	<comment></comment>
+	<projects>
+	</projects>
+	<buildSpec>
+		<buildCommand>
+			<name>org.python.pydev.PyDevBuilder</name>
+			<arguments>
+			</arguments>
+		</buildCommand>
+	</buildSpec>
+	<natures>
+		<nature>org.python.pydev.pythonNature</nature>
+	</natures>
+</projectDescription>

.pydevproject

@@ -0,0 +1,9 @@
+<?xml version="1.0" encoding="UTF-8" standalone="no"?>
+<?eclipse-pydev version="1.0"?><pydev_project>
+<pydev_property name="org.python.pydev.PYTHON_PROJECT_VERSION">python 2.7</pydev_property>
+<pydev_property name="org.python.pydev.PYTHON_PROJECT_INTERPRETER">Default</pydev_property>
+<pydev_pathproperty name="org.python.pydev.PROJECT_SOURCE_PATH">
+<path>/neutron-api/hooks</path>
+<path>/neutron-api/unit_tests</path>
+</pydev_pathproperty>
+</pydev_project>

charm-helpers-sync.yaml

@@ -1,4 +1,4 @@
-branch: lp:charm-helpers
+branch: lp:~james-page/charm-helpers/network-splits
 destination: hooks/charmhelpers
 include:
     - core
@@ -8,3 +8,4 @@ include:
     - contrib.network.ovs
     - contrib.storage.linux
     - payload.execd
+    - contrib.network.ip

config.yaml

@@ -89,3 +89,27 @@ options:
     default: False
     type: boolean
     description: Enable verbose logging
+  # Network configuration options
+  # by default all access is over 'private-address'
+  os-admin-network:
+    type: string
+    description: |
+      The IP address and netmask of the OpenStack Admin network (e.g.,
+      192.168.0.0/24)
+      .
+      This network will be used for admin endpoints.
+  os-internal-network:
+    type: string
+    description: |
+      The IP address and netmask of the OpenStack Internal network (e.g.,
+      192.168.0.0/24)
+      .
+      This network will be used for internal endpoints.
+  os-public-network:
+    type: string
+    description: |
+      The IP address and netmask of the OpenStack Public network (e.g.,
+      192.168.0.0/24)
+      .
+      This network will be used for public endpoints.
+

hooks/charmhelpers/contrib/hahelpers/cluster.py

@@ -163,13 +163,14 @@ def get_hacluster_config():
     return conf
 
 
-def canonical_url(configs, vip_setting='vip'):
+def canonical_url(configs, vip_setting='vip', address=None):
     '''
     Returns the correct HTTP URL to this host given the state of HTTPS
     configuration and hacluster.
 
     :configs    : OSTemplateRenderer: A config tempating object to inspect for
                                       a complete https context.
+
     :vip_setting:                str: Setting in charm config that specifies
                                       VIP address.
     '''
@@ -179,5 +180,5 @@ def canonical_url(configs, vip_setting='vip'):
     if is_clustered():
         addr = config_get(vip_setting)
     else:
-        addr = unit_get('private-address')
+        addr = address or unit_get('private-address')
     return '%s://%s' % (scheme, addr)

hooks/charmhelpers/contrib/network/ip.py

@@ -0,0 +1,69 @@
+import sys
+
+from charmhelpers.fetch import apt_install
+from charmhelpers.core.hookenv import (
+    ERROR, log,
+)
+
+try:
+    import netifaces
+except ImportError:
+    apt_install('python-netifaces')
+    import netifaces
+
+try:
+    import netaddr
+except ImportError:
+    apt_install('python-netaddr')
+    import netaddr
+
+
+def _validate_cidr(network):
+    try:
+        netaddr.IPNetwork(network)
+    except (netaddr.core.AddrFormatError, ValueError):
+        raise ValueError("Network (%s) is not in CIDR presentation format" %
+                         network)
+
+
+def get_address_in_network(network, fallback=None, fatal=False):
+    """
+    Get an IPv4 address within the network from the host.
+
+    :param network (str): CIDR presentation format. For example,
+        '192.168.1.0/24'.
+    :param fallback (str): If no address is found, return fallback.
+    :param fatal (boolean): If no address is found, fallback is not
+        set and fatal is True then exit(1).
+
+    """
+
+    def not_found_error_out():
+        log("No IP address found in network: %s" % network,
+            level=ERROR)
+        sys.exit(1)
+
+    if network is None:
+        if fallback is not None:
+            return fallback
+        else:
+            if fatal:
+                not_found_error_out()
+
+    _validate_cidr(network)
+    for iface in netifaces.interfaces():
+        addresses = netifaces.ifaddresses(iface)
+        if 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 netaddr.IPNetwork(network):
+                return str(cidr.ip)
+
+    if fallback is not None:
+        return fallback
+
+    if fatal:
+        not_found_error_out()
+
+    return None

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

@@ -1,4 +1,7 @@
 import logging
+import os
+import time
+import urllib
 
 import glanceclient.v1.client as glance_client
 import keystoneclient.v2_0 as keystone_client
@@ -149,3 +152,58 @@ class OpenStackAmuletUtils(AmuletUtils):
                                               endpoint_type='publicURL')
         return nova_client.Client(username=user, api_key=password,
                                   project_id=tenant, auth_url=ep)
+
+    def create_cirros_image(self, glance, image_name):
+        """Download the latest cirros image and upload it to glance."""
+        http_proxy = os.getenv('AMULET_HTTP_PROXY')
+        self.log.debug('AMULET_HTTP_PROXY: {}'.format(http_proxy))
+        if http_proxy:
+            proxies = {'http': http_proxy}
+            opener = urllib.FancyURLopener(proxies)
+        else:
+            opener = urllib.FancyURLopener()
+
+        f = opener.open("http://download.cirros-cloud.net/version/released")
+        version = f.read().strip()
+        cirros_img = "tests/cirros-{}-x86_64-disk.img".format(version)
+
+        if not os.path.exists(cirros_img):
+            cirros_url = "http://{}/{}/{}".format("download.cirros-cloud.net",
+                                                  version, cirros_img)
+            opener.retrieve(cirros_url, cirros_img)
+        f.close()
+
+        with open(cirros_img) as f:
+            image = glance.images.create(name=image_name, is_public=True,
+                                         disk_format='qcow2',
+                                         container_format='bare', data=f)
+        return image
+
+    def delete_image(self, glance, image):
+        """Delete the specified image."""
+        glance.images.delete(image)
+
+    def create_instance(self, nova, image_name, instance_name, flavor):
+        """Create the specified instance."""
+        image = nova.images.find(name=image_name)
+        flavor = nova.flavors.find(name=flavor)
+        instance = nova.servers.create(name=instance_name, image=image,
+                                       flavor=flavor)
+
+        count = 1
+        status = instance.status
+        while status == 'BUILD' and count < 10:
+            time.sleep(5)
+            instance = nova.servers.get(instance.id)
+            status = instance.status
+            self.log.debug('instance status: {}'.format(status))
+            count += 1
+
+        if status == 'BUILD':
+            return None
+
+        return instance
+
+    def delete_instance(self, nova, instance):
+        """Delete the specified instance."""
+        nova.servers.delete(instance)

hooks/charmhelpers/contrib/openstack/context.py

@@ -243,23 +243,31 @@ class IdentityServiceContext(OSContextGenerator):
 
 
 class AMQPContext(OSContextGenerator):
-    interfaces = ['amqp']
 
-    def __init__(self, ssl_dir=None):
+    def __init__(self, ssl_dir=None, rel_name='amqp', relation_prefix=None):
         self.ssl_dir = ssl_dir
+        self.rel_name = rel_name
+        self.relation_prefix = relation_prefix
+        self.interfaces = [rel_name]
 
     def __call__(self):
         log('Generating template context for amqp')
         conf = config()
+        user_setting = 'rabbit-user'
+        vhost_setting = 'rabbit-vhost'
+        if self.relation_prefix:
+            user_setting = self.relation_prefix + '-rabbit-user'
+            vhost_setting = self.relation_prefix + '-rabbit-vhost'
+
         try:
-            username = conf['rabbit-user']
+            username = conf[user_setting]
-            vhost = conf['rabbit-vhost']
+            vhost = conf[vhost_setting]
         except KeyError as e:
             log('Could not generate shared_db context. '
                 'Missing required charm config options: %s.' % e)
             raise OSContextError
         ctxt = {}
-        for rid in relation_ids('amqp'):
+        for rid in relation_ids(self.rel_name):
             ha_vip_only = False
             for unit in related_units(rid):
                 if relation_get('clustered', rid=rid, unit=unit):
@@ -332,10 +340,12 @@ class CephContext(OSContextGenerator):
         use_syslog = str(config('use-syslog')).lower()
         for rid in relation_ids('ceph'):
             for unit in related_units(rid):
-                mon_hosts.append(relation_get('private-address', rid=rid,
-                                              unit=unit))
                 auth = relation_get('auth', rid=rid, unit=unit)
                 key = relation_get('key', rid=rid, unit=unit)
+                ceph_addr = \
+                    relation_get('ceph-public-address', rid=rid, unit=unit) or \
+                    relation_get('private-address', rid=rid, unit=unit)
+                mon_hosts.append(ceph_addr)
 
         ctxt = {
             'mon_hosts': ' '.join(mon_hosts),
@@ -418,7 +428,8 @@ class ApacheSSLContext(OSContextGenerator):
     """
     Generates a context for an apache vhost configuration that configures
     HTTPS reverse proxying for one or many endpoints.  Generated context
-    looks something like:
+    looks something like::
+
         {
             'namespace': 'cinder',
             'private_address': 'iscsi.mycinderhost.com',
@@ -633,7 +644,7 @@ class SubordinateConfigContext(OSContextGenerator):
     The subordinate interface allows subordinates to export their
     configuration requirements to the principle for multiple config
     files and multiple serivces.  Ie, a subordinate that has interfaces
-    to both glance and nova may export to following yaml blob as json:
+    to both glance and nova may export to following yaml blob as json::
 
         glance:
             /etc/glance/glance-api.conf:
@@ -652,7 +663,8 @@ class SubordinateConfigContext(OSContextGenerator):
 
     It is then up to the principle charms to subscribe this context to
     the service+config file it is interestd in.  Configuration data will
-    be available in the template context, in glance's case, as:
+    be available in the template context, in glance's case, as::
+
         ctxt = {
             ... other context ...
             'subordinate_config': {

hooks/charmhelpers/contrib/openstack/templating.py

@@ -30,15 +30,15 @@ def get_loader(templates_dir, os_release):
     loading dir.
 
     A charm may also ship a templates dir with this module
-    and it will be appended to the bottom of the search list, eg:
+    and it will be appended to the bottom of the search list, eg::
-    hooks/charmhelpers/contrib/openstack/templates.
 
-    :param templates_dir: str: Base template directory containing release
+        hooks/charmhelpers/contrib/openstack/templates
+
+    :param templates_dir (str): Base template directory containing release
         sub-directories.
-    :param os_release   : str: OpenStack release codename to construct template
+    :param os_release (str): OpenStack release codename to construct template
         loader.
-
+    :returns: jinja2.ChoiceLoader constructed with a list of
-    :returns            : jinja2.ChoiceLoader constructed with a list of
         jinja2.FilesystemLoaders, ordered in descending
         order by OpenStack release.
     """
@@ -111,7 +111,8 @@ class OSConfigRenderer(object):
     and ease the burden of managing config templates across multiple OpenStack
     releases.
 
-    Basic usage:
+    Basic usage::
+
         # import some common context generates from charmhelpers
         from charmhelpers.contrib.openstack import context
 
@@ -131,10 +132,8 @@ class OSConfigRenderer(object):
         # write out all registered configs
         configs.write_all()
 
-    Details:
+    **OpenStack Releases and template loading**
 
-    OpenStack Releases and template loading
-    ---------------------------------------
     When the object is instantiated, it is associated with a specific OS
     release.  This dictates how the template loader will be constructed.
 
@@ -144,8 +143,8 @@ class OSConfigRenderer(object):
     - the base templates_dir
     - a template directory shipped in the charm with this helper file.
 
+    For the example above, '/tmp/templates' contains the following structure::
 
-    For the example above, '/tmp/templates' contains the following structure:
         /tmp/templates/nova.conf
         /tmp/templates/api-paste.ini
         /tmp/templates/grizzly/api-paste.ini
@@ -169,8 +168,8 @@ class OSConfigRenderer(object):
     $CHARM/hooks/charmhelpers/contrib/openstack/templates.  This allows
     us to ship common templates (haproxy, apache) with the helpers.
 
-    Context generators
+    **Context generators**
-    ---------------------------------------
+
     Context generators are used to generate template contexts during hook
     execution.  Doing so may require inspecting service relations, charm
     config, etc.  When registered, a config file is associated with a list

hooks/charmhelpers/contrib/openstack/utils.py

@@ -84,6 +84,8 @@ def get_os_codename_install_source(src):
     '''Derive OpenStack release codename from a given installation source.'''
     ubuntu_rel = lsb_release()['DISTRIB_CODENAME']
     rel = ''
+    if src is None:
+        return rel
     if src in ['distro', 'distro-proposed']:
         try:
             rel = UBUNTU_OPENSTACK_RELEASE[ubuntu_rel]

hooks/charmhelpers/core/hookenv.py

@@ -25,7 +25,7 @@ cache = {}
 def cached(func):
     """Cache return values for multiple executions of func + args
 
-    For example:
+    For example::
 
         @cached
         def unit_get(attribute):
@@ -445,18 +445,19 @@ class UnregisteredHookError(Exception):
 class Hooks(object):
     """A convenient handler for hook functions.
 
-    Example:
+    Example::
+
         hooks = Hooks()
 
         # register a hook, taking its name from the function name
         @hooks.hook()
         def install():
-            ...
+            pass  # your code here
 
         # register a hook, providing a custom hook name
         @hooks.hook("config-changed")
         def config_changed():
-            ...
+            pass  # your code here
 
         if __name__ == "__main__":
             # execute a hook based on the name the program is called by

hooks/charmhelpers/core/host.py

@@ -211,13 +211,13 @@ def file_hash(path):
 def restart_on_change(restart_map, stopstart=False):
     """Restart services based on configuration files changing
 
-    This function is used a decorator, for example
+    This function is used a decorator, for example::
 
         @restart_on_change({
             '/etc/ceph/ceph.conf': [ 'cinder-api', 'cinder-volume' ]
             })
         def ceph_client_changed():
-            ...
+            pass  # your code here
 
     In this example, the cinder-api and cinder-volume services
     would be restarted if /etc/ceph/ceph.conf is changed by the
@@ -313,9 +313,11 @@ def get_nic_hwaddr(nic):
 
 def cmp_pkgrevno(package, revno, pkgcache=None):
     '''Compare supplied revno with the revno of the installed package
-       1 => Installed revno is greater than supplied arg
+
-       0 => Installed revno is the same as supplied arg
+    *  1 => Installed revno is greater than supplied arg
-      -1 => Installed revno is less than supplied arg
+    *  0 => Installed revno is the same as supplied arg
+    * -1 => Installed revno is less than supplied arg
+
     '''
     import apt_pkg
     if not pkgcache:

hooks/charmhelpers/fetch/__init__.py

@@ -235,31 +235,39 @@ def configure_sources(update=False,
                       sources_var='install_sources',
                       keys_var='install_keys'):
     """
-    Configure multiple sources from charm configuration
+    Configure multiple sources from charm configuration.
+
+    The lists are encoded as yaml fragments in the configuration.
+    The frament needs to be included as a string.
 
     Example config:
-        install_sources:
+        install_sources: |
           - "ppa:foo"
           - "http://example.com/repo precise main"
-        install_keys:
+        install_keys: |
           - null
           - "a1b2c3d4"
 
     Note that 'null' (a.k.a. None) should not be quoted.
     """
-    sources = safe_load(config(sources_var))
+    sources = safe_load((config(sources_var) or '').strip()) or []
-    keys = config(keys_var)
+    keys = safe_load((config(keys_var) or '').strip()) or None
-    if keys is not None:
+
-        keys = safe_load(keys)
+    if isinstance(sources, basestring):
-    if isinstance(sources, basestring) and (
+        sources = [sources]
-            keys is None or isinstance(keys, basestring)):
+
-        add_source(sources, keys)
+    if keys is None:
+        for source in sources:
+            add_source(source, None)
     else:
-        if not len(sources) == len(keys):
+        if isinstance(keys, basestring):
-            msg = 'Install sources and keys lists are different lengths'
+            keys = [keys]
-            raise SourceConfigError(msg)
+
-        for src_num in range(len(sources)):
+        if len(sources) != len(keys):
-            add_source(sources[src_num], keys[src_num])
+            raise SourceConfigError(
+                'Install sources and keys lists are different lengths')
+        for source, key in zip(sources, keys):
+            add_source(source, key)
     if update:
         apt_update(fatal=True)
 

hooks/neutron_api_hooks.py

@@ -33,7 +33,6 @@ from charmhelpers.contrib.openstack.neutron import (
 )
 
 from neutron_api_utils import (
-    determine_endpoints,
     determine_packages,
     determine_ports,
     register_configs,
@@ -51,6 +50,7 @@ from charmhelpers.contrib.hahelpers.cluster import (
 )
 
 from charmhelpers.payload.execd import execd_preinstall
+from charmhelpers.contrib.network.ip import get_address_in_network
 
 hooks = Hooks()
 CONFIGS = register_configs()
@@ -152,8 +152,35 @@ def relation_broken():
 
 @hooks.hook('identity-service-relation-joined')
 def identity_joined(rid=None):
-    base_url = canonical_url(CONFIGS)
+    public_url = '{}:{}'.format(canonical_url(CONFIGS,
-    relation_set(relation_id=rid, **determine_endpoints(base_url))
+                                              address=get_address_in_network(
+                                                  config('os-public-network'),
+                                                  unit_get('public-address')
+                                                  )),
+                                api_port('neutron-server'))
+                                                        
+    admin_url = '{}:{}'.format(canonical_url(CONFIGS,
+                                             address=get_address_in_network(
+                                                  config('os-admin-network'),
+                                                  unit_get('private-address')
+                                                  )),
+                               api_port('neutron-server'))
+
+    internal_url = '{}:{}'.format(canonical_url(CONFIGS,
+                                                address=get_address_in_network(
+                                                  config('os-internal-network'),
+                                                  unit_get('private-address')
+                                                  )),
+                               api_port('neutron-server'))
+    
+    endpoints = {
+        'quantum_service': 'quantum',
+        'quantum_region': config('region'),
+        'quantum_public_url': public_url,
+        'quantum_admin_url': admin_url,
+        'quantum_internal_url': internal_url,
+    }
+    relation_set(relation_id=rid, relation_settings=endpoints)
 
 
 @hooks.hook('identity-service-relation-changed')

hooks/neutron_api_utils.py

@@ -72,23 +72,6 @@ def api_port(service):
     return API_PORTS[service]
 
 
-def determine_endpoints(url):
-    '''Generates a dictionary containing all relevant endpoints to be
-    passed to keystone as relation settings.'''
-    region = config('region')
-
-    neutron_url = '%s:%s' % (url, api_port('neutron-server'))
-
-    endpoints = ({
-        'quantum_service': 'quantum',
-        'quantum_region': region,
-        'quantum_public_url': neutron_url,
-        'quantum_admin_url': neutron_url,
-        'quantum_internal_url': neutron_url,
-    })
-    return endpoints
-
-
 def determine_packages():
     # currently all packages match service names
     packages = [] + BASE_PACKAGES

unit_tests/test_neutron_api_hooks.py

@@ -25,7 +25,6 @@ TO_PATCH = [
     'config',
     'CONFIGS',
     'configure_installation_source',
-    'determine_endpoints',
     'determine_packages',
     'determine_ports',
     'do_openstack_upgrade',
@@ -170,7 +169,10 @@ class NeutronAPIHooksTests(CharmTestCase):
         self.assertTrue(self.CONFIGS.write_all.called)
 
     def test_identity_joined(self):
-        _neutron_url = 'http://127.0.0.1:1234'
+        self.canonical_url.return_value = 'http://127.0.0.1'
+        self.api_port.return_value = '9696'
+        self.test_config.set('region','region1')
+        _neutron_url = 'http://127.0.0.1:9696'
         _endpoints = {
             'quantum_service': 'quantum',
             'quantum_region': 'region1',
@@ -178,11 +180,10 @@ class NeutronAPIHooksTests(CharmTestCase):
             'quantum_admin_url': _neutron_url,
             'quantum_internal_url': _neutron_url,
         }
-        self.determine_endpoints.return_value = _endpoints
         self._call_hook('identity-service-relation-joined')
         self.relation_set.assert_called_with(
             relation_id=None,
-            **_endpoints
+            relation_settings=_endpoints
         )
 
     def test_identity_changed_partial_ctxt(self):

unit_tests/test_neutron_api_utils.py

@@ -61,20 +61,6 @@ class TestNeutronAPIUtils(CharmTestCase):
         port = nutils.api_port('neutron-server')
         self.assertEqual(port, nutils.API_PORTS['neutron-server'])
 
-    def test_determine_endpoints(self):
-        test_url = 'http://127.0.0.1'
-        endpoints = nutils.determine_endpoints(test_url)
-        neutron_url = '%s:%s' % (test_url,
-                                 nutils.api_port('neutron-server'))
-        expect = {
-            'quantum_service': 'quantum',
-            'quantum_region': 'region101',
-            'quantum_public_url': neutron_url,
-            'quantum_admin_url': neutron_url,
-            'quantum_internal_url': neutron_url,
-        }
-        self.assertEqual(endpoints, expect)
-
     def test_determine_packages(self):
         pkg_list = nutils.determine_packages()
         expect = nutils.BASE_PACKAGES