From 40b3977d16ef6a8b3b39500d342d933fc4c0e40f Mon Sep 17 00:00:00 2001 From: Frode Nordahl Date: Mon, 23 Sep 2019 18:29:07 +0200 Subject: [PATCH] Expose ML2 context and config to new style plugins When a plugin does not override the ``core_plugin`` and ``neutron_plugin_config`` and leaves them to the ML2 default the charm will now register the ``ml2_conf.ini`` config with both the default Neutron and subordinate plugin contexts. Any exposed context variables not provided by the plugin will no longer be returned as empty values on the context, allowing for passing of the Neutron API charm deduced and configured context values. The ``neutron.conf`` and ``ml2_conf.ini`` templates have been updated to allow adding of new sections. Partial-Bug: #1845212 Change-Id: I90ca77ad16c1a0f59deb34c4faa7e7a89f22aea9 --- hooks/neutron_api_context.py | 67 ++++++++++++++++++++++---- hooks/neutron_api_utils.py | 23 ++++++++- templates/queens/ml2_conf.ini | 9 ++++ templates/queens/neutron.conf | 9 ++++ templates/rocky/neutron.conf | 9 ++++ unit_tests/test_neutron_api_context.py | 39 ++++++++++----- unit_tests/test_neutron_api_utils.py | 11 +++-- 7 files changed, 141 insertions(+), 26 deletions(-) diff --git a/hooks/neutron_api_context.py b/hooks/neutron_api_context.py index 373567d6..63b979e5 100644 --- a/hooks/neutron_api_context.py +++ b/hooks/neutron_api_context.py @@ -674,15 +674,24 @@ class EtcdContext(context.OSContextGenerator): class NeutronApiSDNContext(context.SubordinateConfigContext): interfaces = 'neutron-plugin-api-subordinate' - def __init__(self): + def __init__(self, config_file='/etc/neutron/neutron.conf'): + """Initialize context for plugin subordinates. + + :param config_file: Which config file we accept custom sections for + :type config_file: str + """ super(NeutronApiSDNContext, self).__init__( interface='neutron-plugin-api-subordinate', service='neutron-api', - config_file='/etc/neutron/neutron.conf') - - def __call__(self): - ctxt = super(NeutronApiSDNContext, self).__call__() - defaults = { + config_file=config_file) + # NOTE: The defaults dict serve a dual purpose. + # 1. Only the keys listed here are picked up from the relation. + # 2. Any keys listed here with a value will be used as a default + # if not specified on the relation. + # + # Any empty values will not be returned on this context to allow + # values to be passed on from other contexts. + self.defaults = { 'core-plugin': { 'templ_key': 'core_plugin', 'value': 'neutron.plugins.ml2.plugin.Ml2Plugin', @@ -693,7 +702,7 @@ class NeutronApiSDNContext(context.SubordinateConfigContext): }, 'service-plugins': { 'templ_key': 'service_plugins', - 'value': 'router,firewall,lbaas,vpnaas,metering', + 'value': '', }, 'restart-trigger': { 'templ_key': 'restart_trigger', @@ -707,7 +716,42 @@ class NeutronApiSDNContext(context.SubordinateConfigContext): 'templ_key': 'api_extensions_path', 'value': '', }, + 'extension-drivers': { + 'templ_key': 'extension_drivers', + 'value': '', + }, + 'mechanism-drivers': { + 'templ_key': 'mechanism_drivers', + 'value': '', + }, + 'tenant-network-types': { + 'templ_key': 'tenant_network_types', + 'value': '', + }, + 'neutron-security-groups': { + 'templ_key': 'neutron_security_groups', + 'value': '', + }, } + + def is_default(self, templ_key): + """Check whether value associated with specified key is the default. + + :param templ_key: Key to look up + :type templ_key: str + :returns: True if default, False if not, None if key does not exist. + :rtype: Option[bool, NoneValue] + """ + ctxt = self.__call__() + for interface_key in self.defaults: + if self.defaults[interface_key]['templ_key'] == templ_key: + break + else: + return None + return ctxt.get(templ_key) == self.defaults[interface_key]['value'] + + def __call__(self): + ctxt = super(NeutronApiSDNContext, self).__call__() for rid in relation_ids('neutron-plugin-api-subordinate'): for unit in related_units(rid): rdata = relation_get(rid=rid, unit=unit) @@ -715,13 +759,16 @@ class NeutronApiSDNContext(context.SubordinateConfigContext): if not plugin: continue ctxt['neutron_plugin'] = plugin - for key in defaults.keys(): + for key in self.defaults.keys(): remote_value = rdata.get(key) - ctxt_key = defaults[key]['templ_key'] + ctxt_key = self.defaults[key]['templ_key'] if remote_value: ctxt[ctxt_key] = remote_value + elif self.defaults[key]['value']: + ctxt[ctxt_key] = self.defaults[key]['value'] else: - ctxt[ctxt_key] = defaults[key]['value'] + # Do not set empty values + pass return ctxt return ctxt diff --git a/hooks/neutron_api_utils.py b/hooks/neutron_api_utils.py index c7fa202c..94699c46 100755 --- a/hooks/neutron_api_utils.py +++ b/hooks/neutron_api_utils.py @@ -491,8 +491,29 @@ def resource_map(release=None): resource_map[ML2_SRIOV_INI]['services'] = services resource_map[ML2_SRIOV_INI]['contexts'] = [] else: + plugin_ctxt_instance = neutron_api_context.NeutronApiSDNContext() + if (plugin_ctxt_instance.is_default('core_plugin') and + plugin_ctxt_instance.is_default('neutron_plugin_config')): + # The default core plugin is ML2. If the driver provided by plugin + # subordinate is built on top of ML2, the subordinate will have use + # for influencing existing template variables as well as injecting + # sections into the ML2 configuration file. + conf = neutron_plugin_attribute('ovs', 'config', 'neutron') + services = neutron_plugin_attribute('ovs', 'server_services', + 'neutron') + if conf not in resource_map: + resource_map[conf] = {} + resource_map[conf]['services'] = services + resource_map[conf]['contexts'] = [ + neutron_api_context.NeutronCCContext(), + ] + resource_map[conf]['contexts'].append( + neutron_api_context.NeutronApiSDNContext( + config_file=conf) + ) + resource_map[NEUTRON_CONF]['contexts'].append( - neutron_api_context.NeutronApiSDNContext() + plugin_ctxt_instance, ) resource_map[NEUTRON_DEFAULT]['contexts'] = \ [neutron_api_context.NeutronApiSDNConfigFileContext()] diff --git a/templates/queens/ml2_conf.ini b/templates/queens/ml2_conf.ini index af8aea30..6bc00dc4 100644 --- a/templates/queens/ml2_conf.ini +++ b/templates/queens/ml2_conf.ini @@ -59,3 +59,12 @@ enable_security_group = False [ml2_sriov] supported_pci_vendor_devs = {{ supported_pci_vendor_devs }} {% endif -%} + +{% for section in sections -%} +{% if section != 'DEFAULT' -%} +[{{ section }}] +{% for key, value in sections[section] -%} +{{ key }} = {{ value }} +{% endfor %} +{% endif %} +{%- endfor %} diff --git a/templates/queens/neutron.conf b/templates/queens/neutron.conf index 8328cacb..d81bba55 100644 --- a/templates/queens/neutron.conf +++ b/templates/queens/neutron.conf @@ -137,3 +137,12 @@ lock_path = $state_path/lock service_provider = LOADBALANCERV2:Haproxy:neutron_lbaas.drivers.haproxy.plugin_driver.HaproxyOnHostPluginDriver:default {% include "section-oslo-middleware" %} + +{% for section in sections -%} +{% if section != 'DEFAULT' -%} +[{{ section }}] +{% for key, value in sections[section] -%} +{{ key }} = {{ value }} +{% endfor %} +{% endif %} +{%- endfor %} diff --git a/templates/rocky/neutron.conf b/templates/rocky/neutron.conf index 4f0c5189..5cc1ff2d 100644 --- a/templates/rocky/neutron.conf +++ b/templates/rocky/neutron.conf @@ -139,3 +139,12 @@ service_provider = FIREWALL_V2:fwaas_db:neutron_fwaas.services.firewall.service_ {% endif %} {% include "section-oslo-middleware" %} + +{% for section in sections -%} +{% if section != 'DEFAULT' -%} +[{{ section }}] +{% for key, value in sections[section] -%} +{{ key }} = {{ value }} +{% endfor %} +{% endif %} +{%- endfor %} diff --git a/unit_tests/test_neutron_api_context.py b/unit_tests/test_neutron_api_context.py index 0f770b79..6a989112 100644 --- a/unit_tests/test_neutron_api_context.py +++ b/unit_tests/test_neutron_api_context.py @@ -1096,30 +1096,35 @@ class NeutronApiSDNContextTest(CharmTestCase): @patch.object(charmhelpers.contrib.openstack.context, 'relation_get') @patch.object(charmhelpers.contrib.openstack.context, 'related_units') @patch.object(charmhelpers.contrib.openstack.context, 'relation_ids') - def ctxt_check(self, rel_settings, expect, _rids, _runits, _rget, _log): + def ctxt_check(self, rel_settings, expect, _rids, _runits, _rget, _log, + defaults=None, not_defaults=None): self.test_relation.set(rel_settings) _runits.return_value = ['unit1'] _rids.return_value = ['rid2'] _rget.side_effect = self.test_relation.get self.relation_ids.return_value = ['rid2'] self.related_units.return_value = ['unit1'] - napisdn_ctxt = context.NeutronApiSDNContext()() + napisdn_instance = context.NeutronApiSDNContext() + napisdn_ctxt = napisdn_instance() self.assertEqual(napisdn_ctxt, expect) + defaults = defaults or [] + for templ_key in defaults: + self.assertTrue(napisdn_instance.is_default(templ_key)) + not_defaults = not_defaults or [] + for templ_key in not_defaults: + self.assertFalse(napisdn_instance.is_default(templ_key)) def test_defaults(self): self.ctxt_check( {'neutron-plugin': 'ovs'}, { - 'api_extensions_path': '', 'core_plugin': 'neutron.plugins.ml2.plugin.Ml2Plugin', 'neutron_plugin_config': ('/etc/neutron/plugins/ml2/' 'ml2_conf.ini'), - 'service_plugins': 'router,firewall,lbaas,vpnaas,metering', - 'restart_trigger': '', - 'quota_driver': '', 'neutron_plugin': 'ovs', 'sections': {}, - } + }, + defaults=['core_plugin', 'neutron_plugin_config'], ) def test_overrides(self): @@ -1132,6 +1137,10 @@ class NeutronApiSDNContextTest(CharmTestCase): 'service-plugins': 'router,unicorn,rainbows', 'restart-trigger': 'restartnow', 'quota-driver': 'quotadriver', + 'extension-drivers': 'dns,port_security', + 'mechanism-drivers': 'ovn', + 'tenant-network-types': 'geneve,gre,vlan,flat,local', + 'neutron-security-groups': 'true', }, { 'api_extensions_path': '/usr/local/share/neutron/extensions', @@ -1141,8 +1150,18 @@ class NeutronApiSDNContextTest(CharmTestCase): 'restart_trigger': 'restartnow', 'quota_driver': 'quotadriver', 'neutron_plugin': 'ovs', + 'extension_drivers': 'dns,port_security', + 'mechanism_drivers': 'ovn', + 'tenant_network_types': 'geneve,gre,vlan,flat,local', + 'neutron_security_groups': 'true', 'sections': {}, - } + }, + not_defaults=[ + 'api_extensions_path', 'core_plugin', 'neutron_plugin_config', + 'service_plugins', 'restart_trigger', 'quota_driver', + 'extension_drivers', 'mechanism_drivers', + 'tenant_network_types', 'neutron_security_groups', + ], ) def test_subordinateconfig(self): @@ -1163,13 +1182,9 @@ class NeutronApiSDNContextTest(CharmTestCase): 'subordinate_configuration': json.dumps(principle_config), }, { - 'api_extensions_path': '', 'core_plugin': 'neutron.plugins.ml2.plugin.Ml2Plugin', 'neutron_plugin_config': ('/etc/neutron/plugins/ml2/' 'ml2_conf.ini'), - 'service_plugins': 'router,firewall,lbaas,vpnaas,metering', - 'restart_trigger': '', - 'quota_driver': '', 'neutron_plugin': 'ovs', 'sections': {u'DEFAULT': [[u'neutronboost', True]]}, } diff --git a/unit_tests/test_neutron_api_utils.py b/unit_tests/test_neutron_api_utils.py index 7d5466c5..90c6c361 100644 --- a/unit_tests/test_neutron_api_utils.py +++ b/unit_tests/test_neutron_api_utils.py @@ -139,7 +139,8 @@ class TestNeutronAPIUtils(CharmTestCase): expect.extend(nutils.KILO_PACKAGES) self.assertEqual(sorted(pkg_list), sorted(expect)) - def test_determine_packages_noplugin(self): + @patch.object(nutils.neutron_api_context, 'NeutronApiSDNContext') + def test_determine_packages_noplugin(self, _NeutronApiSDNContext): self.os_release.return_value = 'havana' self.get_os_codename_install_source.return_value = 'havana' self.test_config.set('manage-neutron-plugin-legacy-mode', False) @@ -203,9 +204,13 @@ class TestNeutronAPIUtils(CharmTestCase): [self.assertIn(q_conf, _map.keys()) for q_conf in confs] self.assertTrue(nutils.APACHE_CONF not in _map.keys()) + @patch.object(nutils.neutron_api_context, 'NeutronApiSDNContext') @patch.object(nutils, 'manage_plugin') @patch('os.path.exists') - def test_resource_map_noplugin(self, _path_exists, _manage_plugin): + def test_resource_map_noplugin(self, + _path_exists, + _manage_plugin, + _NeutronApiSDNContext): self.os_release.return_value = 'havana' _path_exists.return_value = True _manage_plugin.return_value = False @@ -213,7 +218,7 @@ class TestNeutronAPIUtils(CharmTestCase): found_sdn_ctxt = False found_sdnconfig_ctxt = False for ctxt in _map[nutils.NEUTRON_CONF]['contexts']: - if isinstance(ctxt, ncontext.NeutronApiSDNContext): + if isinstance(ctxt, MagicMock): found_sdn_ctxt = True for ctxt in _map[nutils.NEUTRON_DEFAULT]['contexts']: if isinstance(ctxt, ncontext.NeutronApiSDNConfigFileContext):