From c7c9c398dbf74456c2a04ebc634e7eaddd5e0ec7 Mon Sep 17 00:00:00 2001 From: Richard Theis Date: Fri, 29 Jul 2016 10:30:14 -0500 Subject: [PATCH] Support callbacks for L3 plugins without an agent Agentless L3 plugins, such as used by networking-ovn, were required to inherit from L3_NAT_db_mixin instead of L3_NAT_dbonly_mixin due to an L3 NAT DB signature mismatch which was fixed by [1]. With [1] fix, agentless L3 plugins would still implicitly pick up callbacks intended for use by L3 agents. Such callbacks will fail unless the L3 plugin inherits from L3_NAT_db_mixin. This patch supports callbacks for L3 plugins without an agent. The callbacks have been refactored and are now done implicitly during object creation. As a result, the l3_db subscribe() method has been deprecated. [1] https://review.openstack.org/#/c/348558/ Change-Id: Id5dd012ffd274314f7d1b39a28525893e0675500 Partial-Bug: #1597898 --- neutron/db/l3_db.py | 62 +++++++++++++------ .../services/l3_router/l3_router_plugin.py | 1 - neutron/tests/unit/db/test_l3_db.py | 2 +- neutron/tests/unit/extensions/test_l3.py | 2 +- neutron/tests/unit/plugins/ml2/test_plugin.py | 8 ++- 5 files changed, 51 insertions(+), 24 deletions(-) diff --git a/neutron/db/l3_db.py b/neutron/db/l3_db.py index 3e7b2935e16..bfadcbe4cec 100644 --- a/neutron/db/l3_db.py +++ b/neutron/db/l3_db.py @@ -15,6 +15,7 @@ import functools import itertools +from debtcollector import removals import netaddr from neutron_lib.api import validators from neutron_lib import constants as l3_constants @@ -153,6 +154,21 @@ class L3_NAT_dbonly_mixin(l3.RouterPluginBase, _dns_integration = None + # NOTE(armax): multiple l3 service plugins (potentially out of tree) + # inherit from l3_db and may need the callbacks to be processed. Having + # an implicit subscription (through the __new__ method) preserves the + # existing behavior, and at the same time it avoids fixing it manually + # in each and every l3 plugin out there. + def __new__(cls): + L3_NAT_dbonly_mixin._subscribe_callbacks() + return super(L3_NAT_dbonly_mixin, cls).__new__(cls) + + @staticmethod + def _subscribe_callbacks(): + registry.subscribe( + _prevent_l3_port_delete_callback, resources.PORT, + events.BEFORE_DELETE) + @property def _is_dns_integration_supported(self): if self._dns_integration is None: @@ -1645,6 +1661,27 @@ class L3_NAT_dbonly_mixin(l3.RouterPluginBase, class L3RpcNotifierMixin(object): """Mixin class to add rpc notifier attribute to db_base_plugin_v2.""" + # NOTE(armax): multiple l3 service plugins (potentially out of tree) + # inherit from l3_db and may need the callbacks to be processed. Having + # an implicit subscription (through the __new__ method) preserves the + # existing behavior, and at the same time it avoids fixing it manually + # in each and every l3 plugin out there. + def __new__(cls): + L3RpcNotifierMixin._subscribe_callbacks() + return object.__new__(cls) + + @staticmethod + def _subscribe_callbacks(): + registry.subscribe( + _notify_routers_callback, resources.PORT, events.AFTER_DELETE) + registry.subscribe( + _notify_subnet_gateway_ip_update, resources.SUBNET_GATEWAY, + events.AFTER_UPDATE) + registry.subscribe( + _notify_subnetpool_address_scope_update, + resources.SUBNETPOOL_ADDRESS_SCOPE, + events.AFTER_UPDATE) + @property def l3_rpc_notifier(self): if not hasattr(self, '_l3_rpc_notifier'): @@ -1822,24 +1859,9 @@ def _notify_subnetpool_address_scope_update(resource, event, l3plugin.notify_routers_updated(context, router_ids) +@removals.remove( + message="This will be removed in the P cycle. " + "Subscriptions are now registered during object creation." +) def subscribe(): - registry.subscribe( - _prevent_l3_port_delete_callback, resources.PORT, events.BEFORE_DELETE) - registry.subscribe( - _notify_routers_callback, resources.PORT, events.AFTER_DELETE) - registry.subscribe( - _notify_subnet_gateway_ip_update, resources.SUBNET_GATEWAY, - events.AFTER_UPDATE) - registry.subscribe( - _notify_subnetpool_address_scope_update, - resources.SUBNETPOOL_ADDRESS_SCOPE, - events.AFTER_UPDATE) - -# NOTE(armax): multiple l3 service plugins (potentially out of tree) inherit -# from l3_db and may need the callbacks to be processed. Having an implicit -# subscription (through the module import) preserves the existing behavior, -# and at the same time it avoids fixing it manually in each and every l3 plugin -# out there. That said, The subscription is also made explicit in the -# reference l3 plugin. The subscription operation is idempotent so there is no -# harm in registering the same callback multiple times. -subscribe() + pass diff --git a/neutron/services/l3_router/l3_router_plugin.py b/neutron/services/l3_router/l3_router_plugin.py index dd7c8223c9e..45651b71f08 100644 --- a/neutron/services/l3_router/l3_router_plugin.py +++ b/neutron/services/l3_router/l3_router_plugin.py @@ -72,7 +72,6 @@ class L3RouterPlugin(service_base.ServicePluginBase, super(L3RouterPlugin, self).__init__() if 'dvr' in self.supported_extension_aliases: l3_dvrscheduler_db.subscribe() - l3_db.subscribe() self.agent_notifiers.update( {n_const.AGENT_TYPE_L3: l3_rpc_agent_api.L3AgentNotifyAPI()}) diff --git a/neutron/tests/unit/db/test_l3_db.py b/neutron/tests/unit/db/test_l3_db.py index 585e49b4368..4124f2d9b4b 100644 --- a/neutron/tests/unit/db/test_l3_db.py +++ b/neutron/tests/unit/db/test_l3_db.py @@ -200,7 +200,7 @@ class TestL3_NAT_dbonly_mixin(base.BaseTestCase): @mock.patch.object(l3_db, '_notify_subnetpool_address_scope_update') def test_subscribe_address_scope_of_subnetpool(self, notify): - l3_db.subscribe() + l3_db.L3RpcNotifierMixin._subscribe_callbacks() registry.notify(resources.SUBNETPOOL_ADDRESS_SCOPE, events.AFTER_UPDATE, mock.ANY, context=mock.ANY, subnetpool_id='fake_id') diff --git a/neutron/tests/unit/extensions/test_l3.py b/neutron/tests/unit/extensions/test_l3.py index b7617384b6b..21ee0fee309 100644 --- a/neutron/tests/unit/extensions/test_l3.py +++ b/neutron/tests/unit/extensions/test_l3.py @@ -1982,7 +1982,7 @@ class L3NatTestCaseBase(L3NatTestCaseMixin): def test_floatingip_with_assoc_fails(self): self._test_floatingip_with_assoc_fails( - 'neutron.db.l3_db.L3_NAT_db_mixin._check_and_get_fip_assoc') + 'neutron.db.l3_db.L3_NAT_dbonly_mixin._check_and_get_fip_assoc') def test_create_floatingip_with_assoc( self, expected_status=l3_constants.FLOATINGIP_STATUS_ACTIVE): diff --git a/neutron/tests/unit/plugins/ml2/test_plugin.py b/neutron/tests/unit/plugins/ml2/test_plugin.py index b3b2a3b9ff7..5012218f478 100644 --- a/neutron/tests/unit/plugins/ml2/test_plugin.py +++ b/neutron/tests/unit/plugins/ml2/test_plugin.py @@ -1565,12 +1565,18 @@ class TestMl2PortBinding(Ml2PluginV2TestCase, def test_update_distributed_port_binding_on_concurrent_port_delete(self): plugin = manager.NeutronManager.get_plugin() + l3plugin = manager.NeutronManager.get_service_plugins().get( + p_const.L3_ROUTER_NAT) with self.port() as port: port = { 'id': port['port']['id'], portbindings.HOST_ID: 'foo_host', } - with mock.patch.object(plugin, 'get_port', new=plugin.delete_port): + # NOTE(rtheis): Mock L3 service prevent_l3_port_deletion() to + # prevent recursive loop introduced by the plugin get_port mock. + with mock.patch.object(plugin, 'get_port', + new=plugin.delete_port), \ + mock.patch.object(l3plugin, 'prevent_l3_port_deletion'): res = plugin.update_distributed_port_binding( self.context, 'foo_port_id', {'port': port}) self.assertIsNone(res)