Add dependency for service plugin

Adds a required list 'required_service_plugins' to each service plugin,
then we can initialize the service plugin with required dependency.
And also adds the 'router' plugin to port forwarding service plugin
required list.

Conflicts:
    neutron/tests/unit/extensions/test_floating_ip_port_forwarding.py

Closes-Bug: #1809238
Change-Id: I53fdaee0cd96a5315a7abc39799657d613eb3a2e
(cherry picked from commit e8b7e768a2)

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 self._get_plugin_class(namespace, plugin_provider)()
-        return plugin_class()
 
     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',
+            plugin_class = self._get_plugin_class(
-                                                    provider)
+                '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
+    def _create_and_add_service_plugin(self, provider):
-            # specifying more than one plugin
+        plugin_inst = self._get_plugin_instance('neutron.service_plugins',
-            # for the same type is a fatal exception
+                                                provider)
-            # TODO(armax): simplify this by moving the conditional into the
+        plugin_type = plugin_inst.get_plugin_type()
-            # directory itself.
+        directory.add_plugin(plugin_type, plugin_inst)
-            plugin_type = plugin_inst.get_plugin_type()
-            if directory.get_plugin(plugin_type):
-                raise ValueError(_("Multiple plugins for service "
-                                   "%s were configured") % plugin_type)
 
-            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
+        # disable incompatible extensions in core plugin if any
-            # (needed by agent management extension)
+        utils.disable_extension_by_service_plugin(plugin, plugin_inst)
-            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
+        LOG.debug("Successfully loaded %(type)s plugin. "
-            utils.disable_extension_by_service_plugin(plugin, plugin_inst)
+                  "Description: %(desc)s",
-
+                  {"type": plugin_type,
-            LOG.debug("Successfully loaded %(type)s plugin. "
+                   "desc": plugin_inst.get_plugin_description()})
-                      "Description: %(desc)s",
-                      {"type": plugin_type,
-                       "desc": plugin_inst.get_plugin_description()})
 
     @classmethod
     @runtime.synchronized("manager")

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']
 

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):

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(

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)
+        manager.NeutronManager.get_instance()
-        self.assertIn(dummy_plugin.DUMMY_SERVICE_TYPE, str(e))
+        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)
+        manager.NeutronManager.get_instance()
-        self.assertIn(dummy_plugin.DUMMY_SERVICE_TYPE, str(e))
+        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",

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.