From: armando-migliaccio Date: Fri, 6 Feb 2015 20:06:31 +0000 (-0800) Subject: Decouple L3 and VPN service plugins during router operations X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=e1a9eb1f92169b8683364019c62efbafa3f63291;p=openstack-build%2Fneutron-build.git 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 3176d89bb..f3aa036b4 100644 --- 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 e44251a1d..7c280524d 100644 --- 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: