diff --git a/neutron/manager.py b/neutron/manager.py index e35966488c3..c42557ec797 100644 --- a/neutron/manager.py +++ b/neutron/manager.py @@ -162,9 +162,11 @@ class NeutronManager(object): with excutils.save_and_reraise_exception(): LOG.error("Plugin '%s' not found.", plugin_provider) + def _get_plugin_class(self, namespace, plugin_provider): + return self.load_class_for_provider(namespace, plugin_provider) + def _get_plugin_instance(self, namespace, plugin_provider): - plugin_class = self.load_class_for_provider(namespace, plugin_provider) - return plugin_class() + return self._get_plugin_class(namespace, plugin_provider)() def _load_services_from_core_plugin(self, plugin): """Puts core plugin in service_plugins for supported services.""" @@ -200,35 +202,39 @@ class NeutronManager(object): continue LOG.info("Loading Plugin: %s", provider) - plugin_inst = self._get_plugin_instance('neutron.service_plugins', - provider) + plugin_class = self._get_plugin_class( + 'neutron.service_plugins', provider) + required_plugins = getattr( + plugin_class, "required_service_plugins", []) + for req_plugin in required_plugins: + LOG.info("Loading service plugin %s, it is required by %s", + req_plugin, provider) + self._create_and_add_service_plugin(req_plugin) + # NOTE(liuyulong): adding one plugin multiple times does not have + # bad effect for it. Since all the plugin has its own specific + # unique name. + self._create_and_add_service_plugin(provider) - # only one implementation of svc_type allowed - # specifying more than one plugin - # for the same type is a fatal exception - # TODO(armax): simplify this by moving the conditional into the - # directory itself. - plugin_type = plugin_inst.get_plugin_type() - if directory.get_plugin(plugin_type): - raise ValueError(_("Multiple plugins for service " - "%s were configured") % plugin_type) + def _create_and_add_service_plugin(self, provider): + plugin_inst = self._get_plugin_instance('neutron.service_plugins', + provider) + plugin_type = plugin_inst.get_plugin_type() + directory.add_plugin(plugin_type, plugin_inst) - directory.add_plugin(plugin_type, plugin_inst) + # search for possible agent notifiers declared in service plugin + # (needed by agent management extension) + plugin = directory.get_plugin() + if (hasattr(plugin, 'agent_notifiers') and + hasattr(plugin_inst, 'agent_notifiers')): + plugin.agent_notifiers.update(plugin_inst.agent_notifiers) - # search for possible agent notifiers declared in service plugin - # (needed by agent management extension) - plugin = directory.get_plugin() - if (hasattr(plugin, 'agent_notifiers') and - hasattr(plugin_inst, 'agent_notifiers')): - plugin.agent_notifiers.update(plugin_inst.agent_notifiers) + # disable incompatible extensions in core plugin if any + utils.disable_extension_by_service_plugin(plugin, plugin_inst) - # disable incompatible extensions in core plugin if any - utils.disable_extension_by_service_plugin(plugin, plugin_inst) - - LOG.debug("Successfully loaded %(type)s plugin. " - "Description: %(desc)s", - {"type": plugin_type, - "desc": plugin_inst.get_plugin_description()}) + LOG.debug("Successfully loaded %(type)s plugin. " + "Description: %(desc)s", + {"type": plugin_type, + "desc": plugin_inst.get_plugin_description()}) @classmethod @runtime.synchronized("manager") diff --git a/neutron/services/portforwarding/pf_plugin.py b/neutron/services/portforwarding/pf_plugin.py index cf67ba47152..19c408052c9 100644 --- a/neutron/services/portforwarding/pf_plugin.py +++ b/neutron/services/portforwarding/pf_plugin.py @@ -71,6 +71,8 @@ class PortForwardingPlugin(fip_pf.PortForwardingPluginBase): This class implements a Port Forwarding plugin. """ + required_service_plugins = ['router'] + supported_extension_aliases = ['floating-ip-port-forwarding', 'expose-port-forwarding-in-fip'] diff --git a/neutron/tests/unit/dummy_plugin.py b/neutron/tests/unit/dummy_plugin.py index 45617b84d86..bfe0bb8e781 100644 --- a/neutron/tests/unit/dummy_plugin.py +++ b/neutron/tests/unit/dummy_plugin.py @@ -28,6 +28,7 @@ from neutron import neutron_plugin_base_v2 RESOURCE_NAME = "dummy" COLLECTION_NAME = "%ss" % RESOURCE_NAME DUMMY_SERVICE_TYPE = "DUMMY" +DUMMY_SERVICE_WITH_REQUIRE_TYPE = "DUMMY_REQIURE" # Attribute Map for dummy resource RESOURCE_ATTRIBUTE_MAP = { @@ -132,6 +133,17 @@ class DummyServicePlugin(service_base.ServicePluginBase): raise exceptions.NotFound() +class DummyWithRequireServicePlugin(DummyServicePlugin): + required_service_plugins = ['dummy'] + + @classmethod + def get_plugin_type(cls): + return DUMMY_SERVICE_WITH_REQUIRE_TYPE + + def get_plugin_description(self): + return "Neutron Dummy Service Plugin with requirements" + + class DummyCorePluginWithoutDatastore( neutron_plugin_base_v2.NeutronPluginBaseV2): def create_subnet(self, context, subnet): diff --git a/neutron/tests/unit/extensions/test_expose_port_forwarding_in_fip.py b/neutron/tests/unit/extensions/test_expose_port_forwarding_in_fip.py index 41a2a9e6524..75cdc7bb6c5 100644 --- a/neutron/tests/unit/extensions/test_expose_port_forwarding_in_fip.py +++ b/neutron/tests/unit/extensions/test_expose_port_forwarding_in_fip.py @@ -73,7 +73,7 @@ class TestExtendFipPortForwardingExtension( def setUp(self): mock.patch('neutron.api.rpc.handlers.resources_rpc.' 'ResourcesPushRpcApi').start() - svc_plugins = (L3_PLUGIN, PF_PLUGIN_NAME, + svc_plugins = (PF_PLUGIN_NAME, L3_PLUGIN, 'neutron.services.qos.qos_plugin.QoSPlugin') ext_mgr = ExtendFipPortForwardingExtensionManager() super(TestExtendFipPortForwardingExtension, self).setUp( diff --git a/neutron/tests/unit/test_manager.py b/neutron/tests/unit/test_manager.py index ec56ecb2cbc..abb4a6b61c5 100644 --- a/neutron/tests/unit/test_manager.py +++ b/neutron/tests/unit/test_manager.py @@ -89,15 +89,20 @@ class NeutronManagerTestCase(base.BaseTestCase): "neutron.tests.unit.dummy_plugin." "DummyServicePlugin"]) cfg.CONF.set_override("core_plugin", DB_PLUGIN_KLASS) - e = self.assertRaises(ValueError, manager.NeutronManager.get_instance) - self.assertIn(dummy_plugin.DUMMY_SERVICE_TYPE, str(e)) + manager.NeutronManager.get_instance() + plugins = directory.get_plugins() + # CORE, DUMMY + self.assertEqual(2, len(plugins)) def test_multiple_plugins_by_name_specified_for_service_type(self): cfg.CONF.set_override("service_plugins", [dummy_plugin.Dummy.get_alias(), dummy_plugin.Dummy.get_alias()]) cfg.CONF.set_override("core_plugin", DB_PLUGIN_KLASS) - self.assertRaises(ValueError, manager.NeutronManager.get_instance) + manager.NeutronManager.get_instance() + plugins = directory.get_plugins() + # CORE, DUMMY + self.assertEqual(2, len(plugins)) def test_multiple_plugins_mixed_specified_for_service_type(self): cfg.CONF.set_override("service_plugins", @@ -105,7 +110,10 @@ class NeutronManagerTestCase(base.BaseTestCase): "DummyServicePlugin", dummy_plugin.Dummy.get_alias()]) cfg.CONF.set_override("core_plugin", DB_PLUGIN_KLASS) - self.assertRaises(ValueError, manager.NeutronManager.get_instance) + manager.NeutronManager.get_instance() + plugins = directory.get_plugins() + # CORE, DUMMY + self.assertEqual(2, len(plugins)) def test_service_plugin_conflicts_with_core_plugin(self): cfg.CONF.set_override("service_plugins", @@ -114,8 +122,45 @@ class NeutronManagerTestCase(base.BaseTestCase): cfg.CONF.set_override("core_plugin", "neutron.tests.unit.test_manager." "MultiServiceCorePlugin") - e = self.assertRaises(ValueError, manager.NeutronManager.get_instance) - self.assertIn(dummy_plugin.DUMMY_SERVICE_TYPE, str(e)) + manager.NeutronManager.get_instance() + plugins = directory.get_plugins() + # CORE, LOADBALANCER, DUMMY + self.assertEqual(3, len(plugins)) + + def test_load_plugins_with_requirements(self): + cfg.CONF.set_override("service_plugins", + ["neutron.tests.unit.dummy_plugin." + "DummyWithRequireServicePlugin"]) + cfg.CONF.set_override("core_plugin", DB_PLUGIN_KLASS) + manager.NeutronManager.get_instance() + plugins = directory.get_plugins() + # DUMMY will also be initialized since DUMMY_REQIURE needs it. + # CORE, DUMMY, DUMMY_REQIURE + self.assertEqual(3, len(plugins)) + + def test_load_plugins_with_requirements_with_parent(self): + cfg.CONF.set_override("service_plugins", + ["neutron.tests.unit.dummy_plugin." + "DummyServicePlugin", + "neutron.tests.unit.dummy_plugin." + "DummyWithRequireServicePlugin"]) + cfg.CONF.set_override("core_plugin", DB_PLUGIN_KLASS) + manager.NeutronManager.get_instance() + plugins = directory.get_plugins() + # CORE, DUMMY, DUMMY_REQIURE + self.assertEqual(3, len(plugins)) + + def test_load_plugins_with_requirements_child_first(self): + cfg.CONF.set_override("service_plugins", + ["neutron.tests.unit.dummy_plugin." + "DummyWithRequireServicePlugin", + "neutron.tests.unit.dummy_plugin." + "DummyServicePlugin"]) + cfg.CONF.set_override("core_plugin", DB_PLUGIN_KLASS) + manager.NeutronManager.get_instance() + plugins = directory.get_plugins() + # CORE, DUMMY, DUMMY_REQIURE + self.assertEqual(3, len(plugins)) def test_core_plugin_supports_services(self): cfg.CONF.set_override("core_plugin", diff --git a/releasenotes/notes/service-plugin-dependency-c8bf620b2526b869.yaml b/releasenotes/notes/service-plugin-dependency-c8bf620b2526b869.yaml new file mode 100644 index 00000000000..8d9bc6c5d57 --- /dev/null +++ b/releasenotes/notes/service-plugin-dependency-c8bf620b2526b869.yaml @@ -0,0 +1,22 @@ +--- +fixes: + - | + Adds the ``router`` service plugin to the ``port_forwarding`` service + plugin required list. For more info see + https://bugs.launchpad.net/neutron/+bug/1809238 +other: + - | + Neutron now supports having service plugins require other plugin(s) as + dependencies. For example, the ``port_forwarding`` service plugin + requires the ``router`` service plugin to achieve full functionality. A + new list, ``required_service_plugins``, was added to each service + plugin so the required dependencies of each service plugin can be + initialized. If one service plugin requires another, but the requirement + is not set in the config file, neutron will now initialize it to the + plugin directory. +upgrade: + - | + During the dependency resolution procedure, the code that loads service + plugins was refactored to not raise an exception if one plugin is + configured multiple times, with the last one taking effect. This is a + change from the previous behavior.