[james-page,r=gnuoy] Add HTTPS+HA support with network split configurations

Makefile

@@ -5,7 +5,7 @@ lint:
 	@flake8 --exclude hooks/charmhelpers hooks unit_tests
 	@charm proof
 
-test:
+unit_test:
 	@echo Starting tests...
 	@$(PYTHON) /usr/bin/nosetests --nologcapture unit_tests
 
@@ -17,6 +17,6 @@ bin/charm_helpers_sync.py:
 sync: bin/charm_helpers_sync.py
 	@$(PYTHON) bin/charm_helpers_sync.py -c charm-helpers-sync.yaml
 
-publish: lint test
+publish: lint unit_test
 	bzr push lp:charms/neutron-api
 	bzr push lp:charms/trusty/neutron-api

hooks/charmhelpers/contrib/network/ip.py

@@ -57,6 +57,8 @@ def get_address_in_network(network, fallback=None, fatal=False):
         else:
             if fatal:
                 not_found_error_out()
+            else:
+                return None
 
     _validate_cidr(network)
     network = netaddr.IPNetwork(network)

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

@@ -1,6 +1,3 @@
-from bzrlib.branch import Branch
-import os
-import re
 from charmhelpers.contrib.amulet.deployment import (
     AmuletDeployment
 )
@@ -13,62 +10,62 @@ class OpenStackAmuletDeployment(AmuletDeployment):
        that is specifically for use by OpenStack charms.
        """
 
-    def __init__(self, series=None, openstack=None, source=None):
+    def __init__(self, series=None, openstack=None, source=None, stable=True):
         """Initialize the deployment environment."""
         super(OpenStackAmuletDeployment, self).__init__(series)
         self.openstack = openstack
         self.source = source
-
-    def _is_dev_branch(self):
-        """Determine if branch being tested is a dev (i.e. next) branch."""
-        branch = Branch.open(os.getcwd())
-        parent = branch.get_parent()
-        pattern = re.compile("^.*/next/$")
-        if (pattern.match(parent)):
-            return True
-        else:
-            return False
+        self.stable = stable
+        # Note(coreycb): this needs to be changed when new next branches come
+        # out.
+        self.current_next = "trusty"
 
     def _determine_branch_locations(self, other_services):
         """Determine the branch locations for the other services.
 
-           If the branch being tested is a dev branch, then determine the
-           development branch locations for the other services. Otherwise,
-           the default charm store branches will be used."""
-        name = 0
-        if self._is_dev_branch():
-            updated_services = []
+           Determine if the local branch being tested is derived from its
+           stable or next (dev) branch, and based on this, use the corresonding
+           stable or next branches for the other_services."""
+        base_charms = ['mysql', 'mongodb', 'rabbitmq-server']
+
+        if self.stable:
             for svc in other_services:
-                if svc[name] in ['mysql', 'mongodb', 'rabbitmq-server']:
-                    location = 'lp:charms/{}'.format(svc[name])
+                temp = 'lp:charms/{}'
+                svc['location'] = temp.format(svc['name'])
+        else:
+            for svc in other_services:
+                if svc['name'] in base_charms:
+                    temp = 'lp:charms/{}'
+                    svc['location'] = temp.format(svc['name'])
                 else:
-                    temp = 'lp:~openstack-charmers/charms/trusty/{}/next'
-                    location = temp.format(svc[name])
-                updated_services.append(svc + (location,))
-            other_services = updated_services
+                    temp = 'lp:~openstack-charmers/charms/{}/{}/next'
+                    svc['location'] = temp.format(self.current_next,
+                                                  svc['name'])
         return other_services
 
     def _add_services(self, this_service, other_services):
         """Add services to the deployment and set openstack-origin/source."""
-        name = 0
         other_services = self._determine_branch_locations(other_services)
+
         super(OpenStackAmuletDeployment, self)._add_services(this_service,
                                                              other_services)
+
         services = other_services
         services.append(this_service)
-        use_source = ['mysql', 'mongodb', 'rabbitmq-server', 'ceph']
+        use_source = ['mysql', 'mongodb', 'rabbitmq-server', 'ceph',
+                      'ceph-osd', 'ceph-radosgw']
 
         if self.openstack:
             for svc in services:
-                if svc[name] not in use_source:
+                if svc['name'] not in use_source:
                     config = {'openstack-origin': self.openstack}
-                    self.d.configure(svc[name], config)
+                    self.d.configure(svc['name'], config)
 
         if self.source:
             for svc in services:
-                if svc[name] in use_source:
+                if svc['name'] in use_source:
                     config = {'source': self.source}
-                    self.d.configure(svc[name], config)
+                    self.d.configure(svc['name'], config)
 
     def _configure_services(self, configs):
         """Configure all of the services."""

hooks/charmhelpers/contrib/openstack/context.py

@@ -52,6 +52,7 @@ from charmhelpers.contrib.openstack.neutron import (
 from charmhelpers.contrib.network.ip import (
     get_address_in_network,
     get_ipv6_addr,
+    get_netmask_for_address,
     format_ipv6_addr,
     is_address_in_network
 )
@@ -408,6 +409,9 @@ class CephContext(OSContextGenerator):
         return ctxt
 
 
+ADDRESS_TYPES = ['admin', 'internal', 'public']
+
+
 class HAProxyContext(OSContextGenerator):
     interfaces = ['cluster']
 
@@ -420,7 +424,6 @@ class HAProxyContext(OSContextGenerator):
         if not relation_ids('cluster'):
             return {}
 
-        cluster_hosts = {}
         l_unit = local_unit().replace('/', '-')
 
         if config('prefer-ipv6'):
@@ -428,17 +431,49 @@ class HAProxyContext(OSContextGenerator):
         else:
             addr = unit_get('private-address')
 
-        cluster_hosts[l_unit] = get_address_in_network(config('os-internal-network'),
-                                                       addr)
+        cluster_hosts = {}
 
-        for rid in relation_ids('cluster'):
-            for unit in related_units(rid):
-                _unit = unit.replace('/', '-')
-                addr = relation_get('private-address', rid=rid, unit=unit)
-                cluster_hosts[_unit] = addr
+        # NOTE(jamespage): build out map of configured network endpoints
+        # and associated backends
+        for addr_type in ADDRESS_TYPES:
+            laddr = get_address_in_network(
+                config('os-{}-network'.format(addr_type)))
+            if laddr:
+                cluster_hosts[laddr] = {}
+                cluster_hosts[laddr]['network'] = "{}/{}".format(
+                    laddr,
+                    get_netmask_for_address(laddr)
+                )
+                cluster_hosts[laddr]['backends'] = {}
+                cluster_hosts[laddr]['backends'][l_unit] = laddr
+                for rid in relation_ids('cluster'):
+                    for unit in related_units(rid):
+                        _unit = unit.replace('/', '-')
+                        _laddr = relation_get('{}-address'.format(addr_type),
+                                              rid=rid, unit=unit)
+                        if _laddr:
+                            cluster_hosts[laddr]['backends'][_unit] = _laddr
+
+        # NOTE(jamespage) no split configurations found, just use
+        # private addresses
+        if not cluster_hosts:
+            cluster_hosts[addr] = {}
+            cluster_hosts[addr]['network'] = "{}/{}".format(
+                addr,
+                get_netmask_for_address(addr)
+            )
+            cluster_hosts[addr]['backends'] = {}
+            cluster_hosts[addr]['backends'][l_unit] = addr
+            for rid in relation_ids('cluster'):
+                for unit in related_units(rid):
+                    _unit = unit.replace('/', '-')
+                    _laddr = relation_get('private-address',
+                                          rid=rid, unit=unit)
+                    if _laddr:
+                        cluster_hosts[addr]['backends'][_unit] = _laddr
 
         ctxt = {
-            'units': cluster_hosts,
+            'frontends': cluster_hosts,
         }
 
         if config('haproxy-server-timeout'):
@@ -455,12 +490,13 @@ class HAProxyContext(OSContextGenerator):
             ctxt['haproxy_host'] = '0.0.0.0'
             ctxt['stat_port'] = ':8888'
 
-        if len(cluster_hosts.keys()) > 1:
-            # Enable haproxy when we have enough peers.
-            log('Ensuring haproxy enabled in /etc/default/haproxy.')
-            with open('/etc/default/haproxy', 'w') as out:
-                out.write('ENABLED=1\n')
-            return ctxt
+        for frontend in cluster_hosts:
+            if len(cluster_hosts[frontend]['backends']) > 1:
+                # Enable haproxy when we have enough peers.
+                log('Ensuring haproxy enabled in /etc/default/haproxy.')
+                with open('/etc/default/haproxy', 'w') as out:
+                    out.write('ENABLED=1\n')
+                return ctxt
         log('HAProxy context is incomplete, this unit has no peers.')
         return {}
 
@@ -722,22 +758,22 @@ class NeutronContext(OSContextGenerator):
 
 class OSConfigFlagContext(OSContextGenerator):
 
-        """
-        Responsible for adding user-defined config-flags in charm config to a
-        template context.
+    """
+    Responsible for adding user-defined config-flags in charm config to a
+    template context.
 
-        NOTE: the value of config-flags may be a comma-separated list of
-              key=value pairs and some Openstack config files support
-              comma-separated lists as values.
-        """
+    NOTE: the value of config-flags may be a comma-separated list of
+          key=value pairs and some Openstack config files support
+          comma-separated lists as values.
+    """
 
-        def __call__(self):
-            config_flags = config('config-flags')
-            if not config_flags:
-                return {}
+    def __call__(self):
+        config_flags = config('config-flags')
+        if not config_flags:
+            return {}
 
-            flags = config_flags_parser(config_flags)
-            return {'user_config_flags': flags}
+        flags = config_flags_parser(config_flags)
+        return {'user_config_flags': flags}
 
 
 class SubordinateConfigContext(OSContextGenerator):

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

@@ -34,17 +34,21 @@ listen stats {{ stat_port }}
     stats uri /
     stats auth admin:password
 
-{% if units -%}
+{% if frontends -%}
 {% for service, ports in service_ports.iteritems() -%}
-listen {{ service }}_ipv4 0.0.0.0:{{ ports[0] }}
-    balance roundrobin
-    {% for unit, address in units.iteritems() -%}
-    server {{ unit }} {{ address }}:{{ ports[1] }} check
+frontend tcp-in_{{ service }}
+    bind *:{{ ports[0] }}
+    bind :::{{ ports[0] }}
+    {% for frontend in frontends -%}
+    acl net_{{ frontend }} dst {{ frontends[frontend]['network'] }}
+    use_backend {{ service }}_{{ frontend }} if net_{{ frontend }}
     {% endfor %}
-listen {{ service }}_ipv6 :::{{ ports[0] }}
-    balance roundrobin
-    {% for unit, address in units.iteritems() -%}
+{% for frontend in frontends -%}
+backend {{ service }}_{{ frontend }}
+    balance leastconn
+    {% for unit, address in frontends[frontend]['backends'].iteritems() -%}
     server {{ unit }} {{ address }}:{{ ports[1] }} check
     {% endfor %}
 {% endfor -%}
+{% endfor -%}
 {% endif -%}

hooks/neutron_api_hooks.py

@@ -74,6 +74,8 @@ from charmhelpers.contrib.network.ip import (
     is_ipv6
 )
 
+from charmhelpers.contrib.openstack.context import ADDRESS_TYPES
+
 hooks = Hooks()
 CONFIGS = register_configs()
 
@@ -128,6 +130,7 @@ def config_changed():
         amqp_joined(relation_id=r_id)
     for r_id in relation_ids('identity-service'):
         identity_joined(rid=r_id)
+    [cluster_joined(rid) for rid in relation_ids('cluster')]
 
 
 @hooks.hook('amqp-relation-joined')
@@ -301,15 +304,19 @@ def neutron_plugin_api_relation_joined(rid=None):
 
 @hooks.hook('cluster-relation-joined')
 def cluster_joined(relation_id=None):
+    for addr_type in ADDRESS_TYPES:
+        address = get_address_in_network(
+            config('os-{}-network'.format(addr_type))
+        )
+        if address:
+            relation_set(
+                relation_id=relation_id,
+                relation_settings={'{}-address'.format(addr_type): address}
+            )
     if config('prefer-ipv6'):
         private_addr = get_ipv6_addr(exc_list=[config('vip')])[0]
-    else:
-        private_addr = unit_get('private-address')
-
-    address = get_address_in_network(config('os-internal-network'),
-                                     private_addr)
-    relation_set(relation_id=relation_id,
-                 relation_settings={'private-address': address})
+        relation_set(relation_id=relation_id,
+                     relation_settings={'private-address': private_addr})
 
 
 @hooks.hook('cluster-relation-changed',
@@ -371,11 +378,8 @@ def ha_joined():
 def ha_changed():
     clustered = relation_get('clustered')
     if not clustered or clustered in [None, 'None', '']:
-        log('ha_changed: hacluster subordinate not fully clustered.:'
-            + str(clustered))
-        return
-    if not is_leader(CLUSTER_RES):
-        log('ha_changed: hacluster complete but we are not leader.')
+        log('ha_changed: hacluster subordinate'
+            ' not fully clustered: %s' % clustered)
         return
     log('Cluster configured, notifying other services and updating '
         'keystone endpoint configuration')

unit_tests/test_neutron_api_context.py

@@ -13,6 +13,7 @@ TO_PATCH = [
 
 
 class IdentityServiceContext(CharmTestCase):
+
     def setUp(self):
         super(IdentityServiceContext, self).setUp(context, TO_PATCH)
         self.relation_get.side_effect = self.test_relation.get
@@ -71,6 +72,10 @@ class HAProxyContextTest(CharmTestCase):
         with patch('__builtin__.__import__'):
             self.assertTrue('units' not in hap_ctxt())
 
+    @patch.object(
+        charmhelpers.contrib.openstack.context, 'get_netmask_for_address')
+    @patch.object(
+        charmhelpers.contrib.openstack.context, 'get_address_in_network')
     @patch.object(charmhelpers.contrib.openstack.context, 'config')
     @patch.object(charmhelpers.contrib.openstack.context, 'local_unit')
     @patch.object(charmhelpers.contrib.openstack.context, 'unit_get')
@@ -81,7 +86,8 @@ class HAProxyContextTest(CharmTestCase):
     @patch('__builtin__.__import__')
     @patch('__builtin__.open')
     def test_context_peers(self, _open, _import, _log, _rids, _runits, _rget,
-                           _uget, _lunit, _config):
+                           _uget, _lunit, _config,  _get_address_in_network,
+                           _get_netmask_for_address):
         unit_addresses = {
             'neutron-api-0': '10.10.10.10',
             'neutron-api-1': '10.10.10.11',
@@ -92,13 +98,21 @@ class HAProxyContextTest(CharmTestCase):
         _lunit.return_value = "neutron-api/1"
         _uget.return_value = unit_addresses['neutron-api-1']
         _config.return_value = None
+        _get_address_in_network.return_value = None
+        _get_netmask_for_address.return_value = '255.255.255.0'
         service_ports = {'neutron-server': [9696, 9686]}
-
+        self.maxDiff = None
         ctxt_data = {
             'local_host': '127.0.0.1',
             'haproxy_host': '0.0.0.0',
+            'local_host': '127.0.0.1',
             'stat_port': ':8888',
-            'units': unit_addresses,
+            'frontends': {
+                '10.10.10.11': {
+                    'network': '10.10.10.11/255.255.255.0',
+                    'backends': unit_addresses,
+                }
+            },
             'service_ports': service_ports,
             'neutron_bind_port': 9686,
         }

unit_tests/test_neutron_api_hooks.py

@@ -15,6 +15,8 @@ utils.restart_map = MagicMock()
 import neutron_api_hooks as hooks
 hooks.hooks._config_save = False
 
+hooks.hooks._config_save = False
+
 utils.register_configs = _reg
 utils.restart_map = _map
 
@@ -31,11 +33,8 @@ TO_PATCH = [
     'determine_ports',
     'do_openstack_upgrade',
     'execd_preinstall',
-    'get_iface_for_address',
     'get_l2population',
-    'get_netmask_for_address',
     'get_overlay_network_type',
-    'is_leader',
     'is_relation_made',
     'log',
     'neutron_plugin_attribute',
@@ -48,8 +47,10 @@ TO_PATCH = [
     'unit_get',
     'get_iface_for_address',
     'get_netmask_for_address',
+    'get_address_in_network',
     'migrate_neutron_database',
     'service_restart',
+    'is_leader',
 ]
 NEUTRON_CONF_DIR = "/etc/neutron"
 
@@ -101,11 +102,13 @@ class NeutronAPIHooksTests(CharmTestCase):
             self.patch('neutron_plugin_api_relation_joined')
         _amqp_rel_joined = self.patch('amqp_joined')
         _id_rel_joined = self.patch('identity_joined')
+        _id_cluster_joined = self.patch('cluster_joined')
         self._call_hook('config-changed')
         self.assertTrue(_n_api_rel_joined.called)
         self.assertTrue(_n_plugin_api_rel_joined.called)
         self.assertTrue(_amqp_rel_joined.called)
         self.assertTrue(_id_rel_joined.called)
+        self.assertTrue(_id_cluster_joined.called)
         self.assertTrue(self.CONFIGS.write_all.called)
         self.assertTrue(self.do_openstack_upgrade.called)
 
@@ -359,7 +362,6 @@ class NeutronAPIHooksTests(CharmTestCase):
         self.test_relation.set({
             'clustered': 'true',
         })
-        self.is_leader.return_value = True
         self.relation_ids.side_effect = self._fake_relids
         _n_api_rel_joined = self.patch('neutron_api_relation_joined')
         _id_rel_joined = self.patch('identity_joined')
@@ -367,23 +369,10 @@ class NeutronAPIHooksTests(CharmTestCase):
         self.assertTrue(_n_api_rel_joined.called)
         self.assertTrue(_id_rel_joined.called)
 
-    def test_ha_changed_not_leader(self):
-        self.test_relation.set({
-            'clustered': 'true',
-        })
-        self.is_leader.return_value = False
-        self.relation_ids.side_effect = self._fake_relids
-        _n_api_rel_joined = self.patch('neutron_api_relation_joined')
-        _id_rel_joined = self.patch('identity_joined')
-        self._call_hook('ha-relation-changed')
-        self.assertFalse(_n_api_rel_joined.called)
-        self.assertFalse(_id_rel_joined.called)
-
     def test_ha_changed_not_clustered(self):
         self.test_relation.set({
             'clustered': None,
         })
-        self.is_leader.return_value = False
         self.relation_ids.side_effect = self._fake_relids
         _n_api_rel_joined = self.patch('neutron_api_relation_joined')
         _id_rel_joined = self.patch('identity_joined')