From fb0c062899032288091df50a18f7f30a594bcc14 Mon Sep 17 00:00:00 2001 From: Slawek Kaplonski Date: Thu, 21 Feb 2019 11:16:03 +0100 Subject: [PATCH] Avoid loading same service plugin more than once In patch [1] requirement that only each service plugin can be loaded only once was removed. Unfortunatelly it is not possible that same service plugin will be instantiate more than once because it may reqister some callbacks or other things which can't be duplicated. So this patch adds mechanism which will ensure that each service plugin class is instantiate only once and reused if necessary. [1] https://review.openstack.org/#/c/626561/ Closes-Bug: #1816771 Change-Id: Ie6e6cc1bbbe50ff7cfad4e8033e48711569ea020 (cherry picked from commit d802fad8a92625005597ebda4931b0bbe13418e9) --- neutron/manager.py | 10 ++++++- neutron/tests/unit/test_manager.py | 46 ++++++++++++++++++++++++++---- 2 files changed, 49 insertions(+), 7 deletions(-) diff --git a/neutron/manager.py b/neutron/manager.py index c42557ec797..e6ff94ffd22 100644 --- a/neutron/manager.py +++ b/neutron/manager.py @@ -113,6 +113,9 @@ class NeutronManager(object): __trace_args__ = {"name": "rpc"} def __init__(self, options=None, config_file=None): + # Store instances of already loaded plugins to avoid instantiate same + # plugin more than once + self._loaded_plugins = {} # If no options have been provided, create an empty dict if not options: options = {} @@ -166,7 +169,12 @@ class NeutronManager(object): return self.load_class_for_provider(namespace, plugin_provider) def _get_plugin_instance(self, namespace, plugin_provider): - return self._get_plugin_class(namespace, plugin_provider)() + plugin_class = self._get_plugin_class(namespace, plugin_provider) + plugin_inst = self._loaded_plugins.get(plugin_class) + if not plugin_inst: + plugin_inst = plugin_class() + self._loaded_plugins[plugin_class] = plugin_inst + return plugin_inst def _load_services_from_core_plugin(self, plugin): """Puts core plugin in service_plugins for supported services.""" diff --git a/neutron/tests/unit/test_manager.py b/neutron/tests/unit/test_manager.py index abb4a6b61c5..efe2f486ffb 100644 --- a/neutron/tests/unit/test_manager.py +++ b/neutron/tests/unit/test_manager.py @@ -16,6 +16,7 @@ import weakref import fixtures +import mock from neutron_lib.plugins import constants as lib_const from neutron_lib.plugins import directory from oslo_config import cfg @@ -132,11 +133,22 @@ class NeutronManagerTestCase(base.BaseTestCase): ["neutron.tests.unit.dummy_plugin." "DummyWithRequireServicePlugin"]) cfg.CONF.set_override("core_plugin", DB_PLUGIN_KLASS) - manager.NeutronManager.get_instance() - plugins = directory.get_plugins() + with mock.patch( + "neutron.tests.unit.dummy_plugin.DummyServicePlugin.__init__", + return_value=None + ) as dummy_init_mock, mock.patch( + "neutron.tests.unit.dummy_plugin." + "DummyWithRequireServicePlugin.__init__", + return_value=None + ) as dummy_with_require_init_mock: + 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)) + # ensure that DUMMY and DUMMY_REQIURE was instantiate only once: + self.assertEqual(1, dummy_init_mock.call_count) + self.assertEqual(1, dummy_with_require_init_mock.call_count) def test_load_plugins_with_requirements_with_parent(self): cfg.CONF.set_override("service_plugins", @@ -145,10 +157,21 @@ class NeutronManagerTestCase(base.BaseTestCase): "neutron.tests.unit.dummy_plugin." "DummyWithRequireServicePlugin"]) cfg.CONF.set_override("core_plugin", DB_PLUGIN_KLASS) - manager.NeutronManager.get_instance() - plugins = directory.get_plugins() + with mock.patch( + "neutron.tests.unit.dummy_plugin.DummyServicePlugin.__init__", + return_value=None + ) as dummy_init_mock, mock.patch( + "neutron.tests.unit.dummy_plugin." + "DummyWithRequireServicePlugin.__init__", + return_value=None + ) as dummy_with_require_init_mock: + manager.NeutronManager.get_instance() + plugins = directory.get_plugins() # CORE, DUMMY, DUMMY_REQIURE self.assertEqual(3, len(plugins)) + # ensure that DUMMY and DUMMY_REQIURE was instantiate only once: + self.assertEqual(1, dummy_init_mock.call_count) + self.assertEqual(1, dummy_with_require_init_mock.call_count) def test_load_plugins_with_requirements_child_first(self): cfg.CONF.set_override("service_plugins", @@ -157,10 +180,21 @@ class NeutronManagerTestCase(base.BaseTestCase): "neutron.tests.unit.dummy_plugin." "DummyServicePlugin"]) cfg.CONF.set_override("core_plugin", DB_PLUGIN_KLASS) - manager.NeutronManager.get_instance() - plugins = directory.get_plugins() + with mock.patch( + "neutron.tests.unit.dummy_plugin.DummyServicePlugin.__init__", + return_value=None + ) as dummy_init_mock, mock.patch( + "neutron.tests.unit.dummy_plugin." + "DummyWithRequireServicePlugin.__init__", + return_value=None + ) as dummy_with_require_init_mock: + manager.NeutronManager.get_instance() + plugins = directory.get_plugins() # CORE, DUMMY, DUMMY_REQIURE self.assertEqual(3, len(plugins)) + # ensure that DUMMY and DUMMY_REQIURE was instantiate only once: + self.assertEqual(1, dummy_init_mock.call_count) + self.assertEqual(1, dummy_with_require_init_mock.call_count) def test_core_plugin_supports_services(self): cfg.CONF.set_override("core_plugin",