diff --git a/hooks/neutron_api_context.py b/hooks/neutron_api_context.py index 9a6cf76a..b88bbf76 100644 --- a/hooks/neutron_api_context.py +++ b/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 - elif self.defaults[key]['value']: + values = [v.strip() for v in remote_value.split(',')] + 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 - # context as incomplete and will allow end user messaging of missing - # relations - return {} + + # Sanity check to ensure that the ovn and openvswitch mechanism + # drivers are not both set. + mechanism_drivers = ctxt.get('mechanism_drivers', None) + 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): diff --git a/unit_tests/test_neutron_api_context.py b/unit_tests/test_neutron_api_context.py index bd090c28..303d5d96 100644 --- a/unit_tests/test_neutron_api_context.py +++ b/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( {},