diff --git a/neutron/db/db_base_plugin_v2.py b/neutron/db/db_base_plugin_v2.py index 59ace79773..7cfda75fa7 100644 --- a/neutron/db/db_base_plugin_v2.py +++ b/neutron/db/db_base_plugin_v2.py @@ -14,6 +14,7 @@ # under the License. import random +import weakref import netaddr from oslo.config import cfg @@ -90,6 +91,16 @@ class CommonDbMixin(object): model_hooks[name] = {'query': query_hook, 'filter': filter_hook, 'result_filters': result_filters} + @property + def safe_reference(self): + """Return a weakref to the instance. + + Minimize the potential for the instance persisting + unnecessarily in memory by returning a weakref proxy that + won't prevent deallocation. + """ + return weakref.proxy(self) + def _model_query(self, context, model): query = context.session.query(model) # define basic filter condition for model query diff --git a/neutron/manager.py b/neutron/manager.py index 43aac09703..10e4941144 100644 --- a/neutron/manager.py +++ b/neutron/manager.py @@ -13,6 +13,8 @@ # License for the specific language governing permissions and limitations # under the License. +import weakref + from oslo.config import cfg from neutron.common import utils @@ -193,20 +195,31 @@ class NeutronManager(object): @classmethod @utils.synchronized("manager") def _create_instance(cls): - if cls._instance is None: + if not cls.has_instance(): cls._instance = cls() + @classmethod + def has_instance(cls): + return cls._instance is not None + + @classmethod + def clear_instance(cls): + cls._instance = None + @classmethod def get_instance(cls): # double checked locking - if cls._instance is None: + if not cls.has_instance(): cls._create_instance() return cls._instance @classmethod def get_plugin(cls): - return cls.get_instance().plugin + # Return a weakref to minimize gc-preventing references. + return weakref.proxy(cls.get_instance().plugin) @classmethod def get_service_plugins(cls): - return cls.get_instance().service_plugins + # Return weakrefs to minimize gc-preventing references. + return dict((x, weakref.proxy(y)) + for x, y in cls.get_instance().service_plugins.iteritems()) diff --git a/neutron/plugins/nec/nec_plugin.py b/neutron/plugins/nec/nec_plugin.py index d0f45608c0..753b893284 100644 --- a/neutron/plugins/nec/nec_plugin.py +++ b/neutron/plugins/nec/nec_plugin.py @@ -104,7 +104,7 @@ class NECPluginV2(db_base_plugin_v2.NeutronDbPluginV2, def __init__(self): super(NECPluginV2, self).__init__() - self.ofc = ofc_manager.OFCManager(self) + self.ofc = ofc_manager.OFCManager(self.safe_reference) self.base_binding_dict = self._get_base_binding_dict() portbindings_base.register_port_dict_function() @@ -120,7 +120,7 @@ class NECPluginV2(db_base_plugin_v2.NeutronDbPluginV2, config.CONF.router_scheduler_driver ) - nec_router.load_driver(self, self.ofc) + nec_router.load_driver(self.safe_reference, self.ofc) self.port_handlers = { 'create': { const.DEVICE_OWNER_ROUTER_GW: self.create_router_port, @@ -148,7 +148,7 @@ class NECPluginV2(db_base_plugin_v2.NeutronDbPluginV2, # NOTE: callback_sg is referred to from the sg unit test. self.callback_sg = SecurityGroupServerRpcCallback() - callbacks = [NECPluginV2RPCCallbacks(self), + callbacks = [NECPluginV2RPCCallbacks(self.safe_reference), DhcpRpcCallback(), L3RpcCallback(), self.callback_sg, diff --git a/neutron/plugins/ryu/ryu_neutron_plugin.py b/neutron/plugins/ryu/ryu_neutron_plugin.py index 2279abbc42..67220a5435 100644 --- a/neutron/plugins/ryu/ryu_neutron_plugin.py +++ b/neutron/plugins/ryu/ryu_neutron_plugin.py @@ -163,10 +163,18 @@ class RyuNeutronPluginV2(db_base_plugin_v2.NeutronDbPluginV2, self.tun_client.create_tunnel_key(net_id, tunnel_key) def _client_delete_network(self, net_id): + RyuNeutronPluginV2._safe_client_delete_network(self.safe_reference, + net_id) + + @staticmethod + def _safe_client_delete_network(safe_reference, net_id): + # Avoid handing naked plugin references to the client. When + # the client is mocked for testing, such references can + # prevent the plugin from being deallocated. client.ignore_http_not_found( - lambda: self.client.delete_network(net_id)) + lambda: safe_reference.client.delete_network(net_id)) client.ignore_http_not_found( - lambda: self.tun_client.delete_tunnel_key(net_id)) + lambda: safe_reference.tun_client.delete_tunnel_key(net_id)) def create_network(self, context, network): session = context.session diff --git a/neutron/tests/base.py b/neutron/tests/base.py index 7eb566cba9..75a629e5f3 100644 --- a/neutron/tests/base.py +++ b/neutron/tests/base.py @@ -18,10 +18,12 @@ """Base Test Case for all Unit Tests""" import contextlib +import gc import logging import os import os.path import sys +import weakref import eventlet.timeout import fixtures @@ -30,7 +32,7 @@ from oslo.config import cfg import testtools from neutron.common import config -from neutron.common import constants as const +from neutron.db import agentschedulers_db from neutron import manager from neutron.openstack.common.notifier import api as notifier_api from neutron.openstack.common.notifier import test_notifier @@ -58,19 +60,24 @@ def fake_use_fatal_exceptions(*args): class BaseTestCase(testtools.TestCase): - def _cleanup_coreplugin(self): - if manager.NeutronManager._instance: - agent_notifiers = getattr(manager.NeutronManager._instance.plugin, - 'agent_notifiers', {}) - dhcp_agent_notifier = agent_notifiers.get(const.AGENT_TYPE_DHCP) - if dhcp_agent_notifier: - dhcp_agent_notifier._plugin = None - manager.NeutronManager._instance = self._saved_instance + def cleanup_core_plugin(self): + """Ensure that the core plugin is deallocated.""" + nm = manager.NeutronManager + if not nm.has_instance(): + return + + #TODO(marun) Fix plugins that do not properly initialize notifiers + agentschedulers_db.AgentSchedulerDbMixin.agent_notifiers = {} + + plugin = weakref.ref(nm._instance.plugin) + nm.clear_instance() + gc.collect() + + #TODO(marun) Ensure that mocks are deallocated? + if plugin() and not isinstance(plugin(), mock.Base): + self.fail('The plugin for this test was not deallocated.') def setup_coreplugin(self, core_plugin=None): - self._saved_instance = manager.NeutronManager._instance - self.addCleanup(self._cleanup_coreplugin) - manager.NeutronManager._instance = None if core_plugin is not None: cfg.CONF.set_override('core_plugin', core_plugin) @@ -103,6 +110,11 @@ class BaseTestCase(testtools.TestCase): def setUp(self): super(BaseTestCase, self).setUp() + + # Ensure plugin cleanup is triggered last so that + # test-specific cleanup has a chance to release references. + self.addCleanup(self.cleanup_core_plugin) + self.addCleanup(self._cleanup_rpc_backend) # Configure this first to ensure pm debugging support for setUp() diff --git a/neutron/tests/unit/ml2/test_mechanism_odl.py b/neutron/tests/unit/ml2/test_mechanism_odl.py index 8034d6496b..5b58c4f6de 100644 --- a/neutron/tests/unit/ml2/test_mechanism_odl.py +++ b/neutron/tests/unit/ml2/test_mechanism_odl.py @@ -18,6 +18,8 @@ from neutron.plugins.common import constants from neutron.plugins.ml2 import config as config from neutron.plugins.ml2 import driver_api as api from neutron.plugins.ml2.drivers import mechanism_odl +from neutron.plugins.ml2 import plugin +from neutron.tests import base from neutron.tests.unit import test_db_plugin as test_plugin PLUGIN_NAME = 'neutron.plugins.ml2.plugin.Ml2Plugin' @@ -65,30 +67,34 @@ class OpenDaylightTestCase(test_plugin.NeutronDbPluginV2TestCase): self.assertFalse(self.mech.check_segment(self.segment)) -class OpenDayLightMechanismConfigTests(test_plugin.NeutronDbPluginV2TestCase): +class OpenDayLightMechanismConfigTests(base.BaseTestCase): - def _setUp(self): + def _set_config(self, url='http://127.0.0.1:9999', username='someuser', + password='somepass'): config.cfg.CONF.set_override('mechanism_drivers', ['logger', 'opendaylight'], 'ml2') - config.cfg.CONF.set_override('url', 'http://127.0.0.1:9999', 'ml2_odl') - config.cfg.CONF.set_override('username', 'someuser', 'ml2_odl') - config.cfg.CONF.set_override('password', 'somepass', 'ml2_odl') + config.cfg.CONF.set_override('url', url, 'ml2_odl') + config.cfg.CONF.set_override('username', username, 'ml2_odl') + config.cfg.CONF.set_override('password', password, 'ml2_odl') - def test_url_required(self): - self._setUp() - config.cfg.CONF.set_override('url', None, 'ml2_odl') - self.assertRaises(config.cfg.RequiredOptError, self.setUp, PLUGIN_NAME) + def _test_missing_config(self, **kwargs): + self._set_config(**kwargs) + self.assertRaises(config.cfg.RequiredOptError, + plugin.Ml2Plugin) - def test_username_required(self): - self._setUp() - config.cfg.CONF.set_override('username', None, 'ml2_odl') - self.assertRaises(config.cfg.RequiredOptError, self.setUp, PLUGIN_NAME) + def test_valid_config(self): + self._set_config() + plugin.Ml2Plugin() - def test_password_required(self): - self._setUp() - config.cfg.CONF.set_override('password', None, 'ml2_odl') - self.assertRaises(config.cfg.RequiredOptError, self.setUp, PLUGIN_NAME) + def test_missing_url_raises_exception(self): + self._test_missing_config(url=None) + + def test_missing_username_raises_exception(self): + self._test_missing_config(username=None) + + def test_missing_password_raises_exception(self): + self._test_missing_config(password=None) class OpenDaylightMechanismTestBasicGet(test_plugin.TestBasicGet, diff --git a/neutron/tests/unit/ml2/test_ml2_plugin.py b/neutron/tests/unit/ml2/test_ml2_plugin.py index fd592a7792..20613a5850 100644 --- a/neutron/tests/unit/ml2/test_ml2_plugin.py +++ b/neutron/tests/unit/ml2/test_ml2_plugin.py @@ -72,18 +72,19 @@ class Ml2PluginV2TestCase(test_plugin.NeutronDbPluginV2TestCase): self.context = context.get_admin_context() -class TestMl2BulkToggle(Ml2PluginV2TestCase): +class TestMl2BulkToggleWithBulkless(Ml2PluginV2TestCase): + + _mechanism_drivers = ['logger', 'test', 'bulkless'] def test_bulk_disable_with_bulkless_driver(self): - self.tearDown() - self._mechanism_drivers = ['logger', 'test', 'bulkless'] - self.setUp() self.assertTrue(self._skip_native_bulk) + +class TestMl2BulkToggleWithoutBulkless(Ml2PluginV2TestCase): + + _mechanism_drivers = ['logger', 'test'] + def test_bulk_enabled_with_bulk_drivers(self): - self.tearDown() - self._mechanism_drivers = ['logger', 'test'] - self.setUp() self.assertFalse(self._skip_native_bulk) diff --git a/neutron/tests/unit/test_db_plugin.py b/neutron/tests/unit/test_db_plugin.py index 255b0ff6e2..7f764e332c 100644 --- a/neutron/tests/unit/test_db_plugin.py +++ b/neutron/tests/unit/test_db_plugin.py @@ -911,9 +911,9 @@ class TestPortsV2(NeutronDbPluginV2TestCase): self.skipTest("Plugin does not support native bulk port create") ctx = context.get_admin_context() with self.network() as net: - orig = manager.NeutronManager._instance.plugin.create_port - with mock.patch.object(manager.NeutronManager._instance.plugin, - 'create_port') as patched_plugin: + plugin = manager.NeutronManager.get_plugin() + orig = plugin.create_port + with mock.patch.object(plugin, 'create_port') as patched_plugin: def side_effect(*args, **kwargs): return self._fail_second_call(patched_plugin, orig, @@ -2405,9 +2405,9 @@ class TestSubnetsV2(NeutronDbPluginV2TestCase): def test_create_subnets_bulk_native_plugin_failure(self): if self._skip_native_bulk: self.skipTest("Plugin does not support native bulk subnet create") - orig = manager.NeutronManager._instance.plugin.create_subnet - with mock.patch.object(manager.NeutronManager._instance.plugin, - 'create_subnet') as patched_plugin: + plugin = manager.NeutronManager.get_plugin() + orig = plugin.create_subnet + with mock.patch.object(plugin, 'create_subnet') as patched_plugin: def side_effect(*args, **kwargs): return self._fail_second_call(patched_plugin, orig, *args, **kwargs)