Decouple L3 and VPN service plugins during router operations

This change leverages the event registry to decouple L3 and VPN
when checking whether certain router operations can be permitted
or not. Follow-up changes will clean up the rest of the coupling.

This clean-up is required because the existing code couples the two
service plugins together, as L3 needs to know about a higher-level
service, like VPN. A similar coupling would be needed for any other
service that might be using or interested in knowning about router
resources; in a world where advanced services have a life of their
own this hardly works or it leads to the tight coupling
experienced so far, which is not the right way to go about it.

Depends-on: I6ac366b329466df6c79a3683e6374feca76773a0
Related-blueprint: services-split
Related-blueprint: plugin-interface-perestroika

Change-Id: Iada03606982670111ae911549a3aad85ed358bea

diff --git a/neutron/db/l3_db.py b/neutron/db/l3_db.py
index 3176d89bb4dc4eb022df316d2e26ad807c9a1958..f3aa036b4be3ff5aa08235baf753999188067970 100644 (file)
--- a/neutron/db/l3_db.py
+++ b/neutron/db/l3_db.py
@@ -17,8 +17,14 @@ import sqlalchemy as sa
 from sqlalchemy import orm
 from sqlalchemy.orm import exc
 
+from oslo_utils import excutils
+
 from neutron.api.rpc.agentnotifiers import l3_rpc_agent_api
 from neutron.api.v2 import attributes
+from neutron.callbacks import events
+from neutron.callbacks import exceptions
+from neutron.callbacks import registry
+from neutron.callbacks import resources
 from neutron.common import constants as l3_constants
 from neutron.common import exceptions as n_exc
 from neutron.common import rpc as n_rpc
@@ -338,13 +344,22 @@ class L3_NAT_dbonly_mixin(l3.RouterPluginBase):
             router.gw_port = None
             context.session.add(router)
             context.session.expire(gw_port)
-            vpnservice = manager.NeutronManager.get_service_plugins().get(
-                constants.VPN)
-            if vpnservice:
-                vpnservice.check_router_in_use(context, router_id)
+            self._check_router_gw_port_in_use(context, router_id)
         self._core_plugin.delete_port(
             admin_ctx, gw_port['id'], l3_port_check=False)
 
+    def _check_router_gw_port_in_use(self, context, router_id):
+        try:
+            kwargs = {'context': context, 'router_id': router_id}
+            registry.notify(
+                resources.ROUTER_GATEWAY, events.BEFORE_DELETE, self, **kwargs)
+        except exceptions.CallbackFailure as e:
+            with excutils.save_and_reraise_exception():
+                # NOTE(armax): preserve old check's behavior
+                if len(e.errors) == 1:
+                    raise e.errors[0].error
+                raise l3.RouterInUse(router_id=router_id, reason=e)
+
     def _create_gw_port(self, context, router_id, router, new_network,
                         ext_ips, ext_ip_change):
         new_valid_gw_port_attachment = (
@@ -573,10 +588,17 @@ class L3_NAT_dbonly_mixin(l3.RouterPluginBase):
         subnet_db = self._core_plugin._get_subnet(context, subnet_id)
         subnet_cidr = netaddr.IPNetwork(subnet_db['cidr'])
         fip_qry = context.session.query(FloatingIP)
-        vpnservice = manager.NeutronManager.get_service_plugins().get(
-            constants.VPN)
-        if vpnservice:
-            vpnservice.check_subnet_in_use(context, subnet_id)
+        try:
+            kwargs = {'context': context, 'subnet_id': subnet_id}
+            registry.notify(
+                resources.ROUTER_INTERFACE,
+                events.BEFORE_DELETE, self, **kwargs)
+        except exceptions.CallbackFailure as e:
+            with excutils.save_and_reraise_exception():
+                # NOTE(armax): preserve old check's behavior
+                if len(e.errors) == 1:
+                    raise e.errors[0].error
+                raise l3.RouterInUse(router_id=router_id, reason=e)
         for fip_db in fip_qry.filter_by(router_id=router_id):
             if netaddr.IPAddress(fip_db['fixed_ip_address']) in subnet_cidr:
                 raise l3.RouterInterfaceInUseByFloatingIP(

diff --git a/neutron/tests/unit/test_l3_plugin.py b/neutron/tests/unit/test_l3_plugin.py
index e44251a1d7b57d00efc1bde114efe765df55f823..7c280524d355834c655730e2d45b5e2f93c60d41 100644 (file)
--- a/neutron/tests/unit/test_l3_plugin.py
+++ b/neutron/tests/unit/test_l3_plugin.py
@@ -26,6 +26,8 @@ from webob import exc
 from neutron.api.rpc.agentnotifiers import l3_rpc_agent_api
 from neutron.api.rpc.handlers import l3_rpc
 from neutron.api.v2 import attributes
+from neutron.callbacks import exceptions
+from neutron.callbacks import registry
 from neutron.common import constants as l3_constants
 from neutron.common import exceptions as n_exc
 from neutron import context
@@ -1352,6 +1354,57 @@ class L3NatTestCaseBase(L3NatTestCaseMixin):
                                               s['subnet']['id'],
                                               None)
 
+    def test_router_remove_interface_callback_failure_returns_409(self):
+        with contextlib.nested(
+            self.router(),
+            self.subnet(),
+            mock.patch.object(registry, 'notify')) as (r, s, notify):
+                errors = [
+                    exceptions.NotificationError(
+                        'foo_callback_id', n_exc.InUse()),
+                ]
+                # we fail the first time, but not the second, when
+                # the clean-up takes place
+                notify.side_effect = [
+                    exceptions.CallbackFailure(errors=errors), None
+                ]
+                self._router_interface_action('add',
+                                              r['router']['id'],
+                                              s['subnet']['id'],
+                                              None)
+                self._router_interface_action(
+                    'remove',
+                    r['router']['id'],
+                    s['subnet']['id'],
+                    None,
+                    exc.HTTPConflict.code)
+                # remove properly to clean-up
+                self._router_interface_action(
+                    'remove',
+                    r['router']['id'],
+                    s['subnet']['id'],
+                    None)
+
+    def test_router_clear_gateway_callback_failure_returns_409(self):
+        with contextlib.nested(
+            self.router(),
+            self.subnet(),
+            mock.patch.object(registry, 'notify')) as (r, s, notify):
+                errors = [
+                    exceptions.NotificationError(
+                        'foo_callback_id', n_exc.InUse()),
+                ]
+                notify.side_effect = exceptions.CallbackFailure(errors=errors)
+                self._set_net_external(s['subnet']['network_id'])
+                self._add_external_gateway_to_router(
+                       r['router']['id'],
+                       s['subnet']['network_id'])
+                self._remove_external_gateway_from_router(
+                    r['router']['id'],
+                    s['subnet']['network_id'],
+                    external_gw_info={},
+                    expected_code=exc.HTTPConflict.code)
+
     def test_router_remove_interface_wrong_subnet_returns_400(self):
         with self.router() as r:
             with self.subnet() as s: