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

@@ -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

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()})
 

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

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

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)