From fff0fa464b8018f26fe5c201ec8003ab40496cf7 Mon Sep 17 00:00:00 2001 From: Billy Olsen Date: Tue, 23 Apr 2024 16:23:53 -0700 Subject: [PATCH] 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 | 38 +++++++-- unit_tests/test_neutron_api_context.py | 111 ++++++++++++++++++++++++- 2 files changed, 140 insertions(+), 9 deletions(-) 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( {},