Support multiple subordinate api plugins

Support multiple subordinate api plugins providing services, mechanism
drivers, etc. This works by merging the dictionaries that are returned
from the different subordinates.

This is to support the case where a neutron-api-plugin-ovn and
neutron-api-plugin-ironic are both related in order to support an OVN +
Ironic based deployment.

Note: there is an additional check to ensure that the ovn and
openvswitch mechanism drivers are not both enabled at the same time.

Change-Id: I36b4a59dd6edf4dd3790dcab5279237af73bf712

hooks/neutron_api_context.py

@@ -913,7 +913,14 @@ class NeutronApiSDNContext(context.SubordinateConfigContext):
 
     def __call__(self):
         ctxt = super(NeutronApiSDNContext, self).__call__()
-        for rid in relation_ids('neutron-plugin-api-subordinate'):
+        rel_ids = relation_ids('neutron-plugin-api-subordinate')
+        # Return empty dict when there are no related units, this will flag the
+        # context as incomplete and will allow end user messaging of missing
+        # relations
+        if not rel_ids:
+            return {}
+
+        for rid in rel_ids:
             for unit in related_units(rid):
                 rdata = relation_get(rid=rid, unit=unit)
                 plugin = rdata.get('neutron-plugin')
@@ -923,18 +930,33 @@ class NeutronApiSDNContext(context.SubordinateConfigContext):
                 for key in self.defaults.keys():
                     remote_value = rdata.get(key)
                     ctxt_key = self.defaults[key]['templ_key']
+                    current = ctxt.get(ctxt_key, [])
+                    if current:
+                        current = [x.strip() for x in current.split(',')]
+
                     if remote_value:
-                        ctxt[ctxt_key] = remote_value
+                        values = [v.strip() for v in remote_value.split(',')]
-                    elif self.defaults[key]['value']:
+                        for value in values:
+                            if value not in current:
+                                current.append(value)
+                        ctxt[ctxt_key] = ','.join(current)
+                    elif self.defaults[key]['value'] and not current:
                         ctxt[ctxt_key] = self.defaults[key]['value']
                     else:
                         # Do not set empty values
                         pass
-                return ctxt
+
-        # Return empty dict when there are no related units, this will flag the
+        # Sanity check to ensure that the ovn and openvswitch mechanism
-        # context as incomplete and will allow end user messaging of missing
+        # drivers are not both set.
-        # relations
+        mechanism_drivers = ctxt.get('mechanism_drivers', None)
-        return {}
+        if mechanism_drivers:
+            mechanism_drivers = mechanism_drivers.split(',')
+            if ('ovn' in mechanism_drivers and
+                    'openvswitch' in mechanism_drivers):
+                mechanism_drivers.remove('openvswitch')
+                ctxt['mechanism_drivers'] = ','.join(mechanism_drivers)
+
+        return ctxt
 
 
 class NeutronApiSDNConfigFileContext(context.OSContextGenerator):

unit_tests/test_neutron_api_context.py

@@ -20,7 +20,7 @@ from unittest.mock import MagicMock, patch
 import neutron_api_context as context
 import charmhelpers
 
-from test_utils import CharmTestCase
+from test_utils import CharmTestCase, TestRelation
 
 TO_PATCH = [
     'config',
@@ -1343,6 +1343,115 @@ class NeutronApiSDNContextTest(CharmTestCase):
             }
         )
 
+    @patch('neutron_api_context.super')
+    @patch.object(context, 'relation_get')
+    @patch.object(context, 'related_units')
+    @patch.object(context, 'relation_ids')
+    def test_multiple_subordinates_no_relation_data(self, rel_ids, rel_units,
+                                                    rel_get, _super):
+        """Test multiple subordinates w/out data provide empty context."""
+        napisdn_instance = context.NeutronApiSDNContext()
+
+        # Mock the call to super as it invokes relation-get and other methods
+        # due to these methods being imported into the
+        # charmhelpers.contrib.openstack.context library which requires
+        # multiple mocks for the same function (which is confusing).
+        _super.return_value = MagicMock(return_value={})
+
+        rel_ids.return_value = []
+        rel_units.return_value = []
+
+        rel_1 = TestRelation()
+        rel_2 = TestRelation()
+
+        def _relation_get(attribute=None, unit=None, rid=None, app=None):
+            if rid == 'rel-1':
+                return rel_1
+            if rid == 'rel-2':
+                return rel_2
+
+            return None
+
+        rel_get.side_effect = _relation_get
+
+        ctxt = napisdn_instance()
+        self.assertEqual(ctxt, {})
+
+        rel_ids.return_value = ['rel-1', 'rel-2']
+        rel_units.return_value = ['subordinate-1', 'subordinate-2']
+
+        ctxt = napisdn_instance()
+        self.assertEqual(ctxt, {})
+
+    @patch('neutron_api_context.super')
+    @patch.object(context, 'relation_get')
+    @patch.object(context, 'related_units')
+    @patch.object(context, 'relation_ids')
+    def test_multiple_subordinates_ovn_ironic(self, _rel_ids, _rel_units,
+                                              _rel_get, _super):
+        """Test multiple subordinates provide merged context."""
+        napisdn_instance = context.NeutronApiSDNContext()
+
+        # Mock the call to super as it invokes relation-get and other methods
+        # due to these methods being imported into the
+        # charmhelpers.contrib.openstack.context library which requires
+        # multiple mocks for the same function (which is confusing).
+        _super.return_value = MagicMock(return_value={})
+
+        _rel_ids.return_value = ['rel-1', 'rel-2']
+        _rel_units.return_value = ['subordinate-1', 'subordinate-2']
+
+        ovn_rel = TestRelation({
+            'mechanism-drivers': 'ovn',
+            'neutron-plugin': 'ovn',
+            'service-plugins': (
+                'metering,segments,'
+                'neutron_dynamic_routing.services.bgp.bgp_plugin.BgpPlugin,'
+                'ovn-router,trunk'
+            ),
+            'subordinate_configuration': (
+                '{"neutron-api": {"/etc/neutron/plugins/ml2/ml2_conf.ini":'
+                '{"sections": {"ovn": "test"}}}}'
+            ),
+            'tenant-network-types': 'geneve, gre, vlan, flat, local'
+        })
+        ironic_rel = TestRelation({
+            'mechanism-drivers': 'openvswitch,l2population,baremetal',
+            'neutron-plugin': 'ironic',
+            'subordinate_configuration': '{}'
+        })
+
+        def _relation_get(attribute=None, unit=None, rid=None, app=None):
+            if rid == 'rel-1':
+                return ovn_rel
+            if rid == 'rel-2':
+                return ironic_rel
+
+            return None
+
+        _rel_get.side_effect = _relation_get
+
+        ctxt = napisdn_instance()
+
+        # Verify that when the ovn mechanism driver is specified, the
+        # openvswitch one is not available.
+        mechanism_drivers = ctxt.get('mechanism_drivers', '').split(',')
+        self.assertTrue(mechanism_drivers.index('ovn') >= 0)
+        self.assertRaises(ValueError, mechanism_drivers.index, 'openvswitch')
+
+        self.assertEqual(ctxt, {
+            'core_plugin': 'neutron.plugins.ml2.plugin.Ml2Plugin',
+            'mechanism_drivers': 'ovn,l2population,baremetal',
+            'neutron_plugin': 'ironic',  # last one wins
+            'neutron_plugin_config': '/etc/neutron/plugins/ml2/ml2_conf.ini',
+            'service_plugins': (
+                'metering,segments,'
+                'neutron_dynamic_routing.services.bgp.bgp_plugin.BgpPlugin,'
+                'ovn-router,trunk'
+            ),
+            'tenant_network_types': 'geneve,gre,vlan,flat,local',
+        })
+
     def test_empty(self):
         self.ctxt_check(
             {},