diff --git a/neutron/db/l3_db.py b/neutron/db/l3_db.py index 30895dd8360..01a0cfc5b56 100644 --- a/neutron/db/l3_db.py +++ b/neutron/db/l3_db.py @@ -424,6 +424,8 @@ class L3_NAT_dbonly_mixin(l3.RouterPluginBase, self._delete_router_gw_port_db(context, router) self._core_plugin.delete_port( admin_ctx, gw_port_id, l3_port_check=False) + with context.session.begin(subtransactions=True): + context.session.refresh(router) registry.notify(resources.ROUTER_GATEWAY, events.AFTER_DELETE, self, router_id=router_id, @@ -531,7 +533,7 @@ class L3_NAT_dbonly_mixin(l3.RouterPluginBase, router = self._get_router(context, router_id) device_owner = self._get_device_owner(context, router) if any(rp.port_type == device_owner - for rp in router.attached_ports.all()): + for rp in router.attached_ports): raise l3.RouterInUse(router_id=router_id) return router @@ -543,13 +545,16 @@ class L3_NAT_dbonly_mixin(l3.RouterPluginBase, router = self._ensure_router_not_in_use(context, id) original = self._make_router_dict(router) self._delete_current_gw_port(context, id, router, None) + with context.session.begin(subtransactions=True): + context.session.refresh(router) - router_ports = router.attached_ports.all() + router_ports = router.attached_ports for rp in router_ports: self._core_plugin.delete_port(context.elevated(), rp.port.id, l3_port_check=False) with context.session.begin(subtransactions=True): + context.session.refresh(router) registry.notify(resources.ROUTER, events.PRECOMMIT_DELETE, self, context=context, router_db=router, router_id=id) @@ -887,6 +892,8 @@ class L3_NAT_dbonly_mixin(l3.RouterPluginBase, new_interface=new_router_intf, interface_info=interface_info) + with context.session.begin(subtransactions=True): + context.session.refresh(router) return self._make_router_interface_info( router.id, port['tenant_id'], port['id'], port['network_id'], subnets[-1]['id'], [subnet['id'] for subnet in subnets]) @@ -1013,6 +1020,8 @@ class L3_NAT_dbonly_mixin(l3.RouterPluginBase, port=port, router_id=router_id, interface_info=interface_info) + with context.session.begin(subtransactions=True): + context.session.refresh(router) return self._make_router_interface_info(router_id, port['tenant_id'], port['id'], port['network_id'], subnets[0]['id'], @@ -1922,6 +1931,7 @@ class L3_NAT_db_mixin(L3_NAT_dbonly_mixin, L3RpcNotifierMixin): def _migrate_router_ports( self, context, router_db, old_owner, new_owner): """Update the model to support the dvr case of a router.""" - for rp in router_db.attached_ports.filter_by(port_type=old_owner): - rp.port_type = new_owner - rp.port.device_owner = new_owner + for rp in router_db.attached_ports: + if rp.port_type == old_owner: + rp.port_type = new_owner + rp.port.device_owner = new_owner diff --git a/neutron/db/l3_dvr_db.py b/neutron/db/l3_dvr_db.py index d4cca3baa85..719a6f309b2 100644 --- a/neutron/db/l3_dvr_db.py +++ b/neutron/db/l3_dvr_db.py @@ -242,10 +242,8 @@ class DVRResourceOperationHandler(object): port_list = [] int_ports = ( - rp.port for rp in - router.attached_ports.filter_by( - port_type=const.DEVICE_OWNER_DVR_INTERFACE - ) + rp.port for rp in router.attached_ports + if rp.port_type == const.DEVICE_OWNER_DVR_INTERFACE ) LOG.info('SNAT interface port list does not exist,' ' so create one: %s', port_list) diff --git a/neutron/db/models/l3.py b/neutron/db/models/l3.py index 70d751ef78d..f1f7c446e36 100644 --- a/neutron/db/models/l3.py +++ b/neutron/db/models/l3.py @@ -58,7 +58,7 @@ class Router(standard_attr.HasStandardAttributes, model_base.BASEV2, attached_ports = orm.relationship( RouterPort, backref=orm.backref('router', load_on_pending=True), - lazy='dynamic') + lazy='subquery') l3_agents = orm.relationship( 'Agent', lazy='subquery', viewonly=True, secondary=rb_model.RouterL3AgentBinding.__table__) diff --git a/neutron/tests/unit/db/test_l3_dvr_db.py b/neutron/tests/unit/db/test_l3_dvr_db.py index a2977343242..699e976f636 100644 --- a/neutron/tests/unit/db/test_l3_dvr_db.py +++ b/neutron/tests/unit/db/test_l3_dvr_db.py @@ -29,6 +29,8 @@ from neutron.db import agents_db from neutron.db import common_db_mixin from neutron.db import l3_dvr_db from neutron.db import l3_dvrscheduler_db +from neutron.db.models import l3 as l3_models +from neutron.db import models_v2 from neutron.extensions import l3 from neutron.objects import router as router_obj from neutron.tests.unit.db import test_db_base_plugin_v2 @@ -274,26 +276,44 @@ class L3DvrTestCase(test_db_base_plugin_v2.NeutronDbPluginV2TestCase): def _setup_delete_current_gw_port_deletes_dvr_internal_ports( self, port=None, gw_port=True, new_network_id='ext_net_id_2'): - router = mock.MagicMock() - router.extra_attributes.distributed = True + router_db = { + 'name': 'foo_router', + 'admin_state_up': True, + 'distributed': True + } + router = self._create_router(router_db) if gw_port: - gw_port_db = { - 'id': 'my_gw_id', - 'network_id': 'ext_net_id', - 'device_owner': const.DEVICE_OWNER_ROUTER_GW, - 'fixed_ips': [{'ip_address': '1.2.3.4'}] - } - router.gw_port = gw_port_db + with self.subnet(cidr='10.10.10.0/24') as subnet: + port_dict = { + 'device_id': router.id, + 'device_owner': const.DEVICE_OWNER_ROUTER_GW, + 'admin_state_up': True, + 'fixed_ips': [{'subnet_id': subnet['subnet']['id'], + 'ip_address': '10.10.10.100'}] + } + net_id = subnet['subnet']['network_id'] + port_res = self.create_port(net_id, port_dict) + port_res_dict = self.deserialize(self.fmt, port_res) + with self.ctx.session.begin(subtransactions=True): + port_db = self.ctx.session.query(models_v2.Port).filter_by( + id=port_res_dict['port']['id']).one() + router.gw_port = port_db + router_port = l3_models.RouterPort( + router_id=router.id, + port_id=port_db.id, + port_type=const.DEVICE_OWNER_ROUTER_GW + ) + self.ctx.session.add(router) + self.ctx.session.add(router_port) + else: - router.gw_port = None + net_id = None plugin = mock.Mock() directory.add_plugin(plugin_constants.CORE, plugin) with mock.patch.object(l3_dvr_db.l3_db.L3_NAT_db_mixin, 'router_gw_port_has_floating_ips', return_value=False),\ - mock.patch.object(l3_dvr_db.l3_db.L3_NAT_db_mixin, - '_delete_router_gw_port_db'),\ mock.patch.object( self.mixin, '_get_router') as grtr,\ @@ -310,16 +330,17 @@ class L3DvrTestCase(test_db_base_plugin_v2.NeutronDbPluginV2TestCase): grtr.return_value = router self.mixin._delete_current_gw_port( self.ctx, router['id'], router, new_network_id) - return router, plugin, del_csnat_port, del_agent_gw_port, del_fip + return router, plugin, net_id, del_csnat_port,\ + del_agent_gw_port, del_fip def test_delete_current_gw_port_deletes_fip_agent_gw_port_and_fipnamespace( self): - rtr, plugin, d_csnat_port, d_agent_gw_port, del_fip = ( + rtr, plugin, ext_net_id, d_csnat_port, d_agent_gw_port, del_fip = ( self._setup_delete_current_gw_port_deletes_dvr_internal_ports()) self.assertFalse(d_csnat_port.called) self.assertTrue(d_agent_gw_port.called) - d_agent_gw_port.assert_called_once_with(mock.ANY, None, 'ext_net_id') - del_fip.assert_called_once_with(mock.ANY, 'ext_net_id') + d_agent_gw_port.assert_called_once_with(mock.ANY, None, ext_net_id) + del_fip.assert_called_once_with(self.ctx, ext_net_id) def test_delete_current_gw_port_never_calls_delete_fip_agent_gw_port(self): port = [{ @@ -332,30 +353,32 @@ class L3DvrTestCase(test_db_base_plugin_v2.NeutronDbPluginV2TestCase): 'network_id': 'ext_net_id', 'device_owner': const.DEVICE_OWNER_ROUTER_GW }] - rtr, plugin, d_csnat_port, d_agent_gw_port, del_fip = ( + rtr, plugin, ext_net_id, d_csnat_port, d_agent_gw_port, del_fip = ( self._setup_delete_current_gw_port_deletes_dvr_internal_ports( port=port)) self.assertFalse(d_csnat_port.called) self.assertFalse(d_agent_gw_port.called) self.assertFalse(del_fip.called) + self.assertIsNotNone(ext_net_id) def test_delete_current_gw_port_never_calls_delete_fipnamespace(self): - rtr, plugin, d_csnat_port, d_agent_gw_port, del_fip = ( + rtr, plugin, ext_net_id, d_csnat_port, d_agent_gw_port, del_fip = ( self._setup_delete_current_gw_port_deletes_dvr_internal_ports( gw_port=False)) self.assertFalse(d_csnat_port.called) self.assertFalse(d_agent_gw_port.called) self.assertFalse(del_fip.called) + self.assertIsNone(ext_net_id) def test_delete_current_gw_port_deletes_csnat_port(self): - rtr, plugin, d_csnat_port, d_agent_gw_port, del_fip = ( + rtr, plugin, ext_net_id, d_csnat_port, d_agent_gw_port, del_fip = ( self._setup_delete_current_gw_port_deletes_dvr_internal_ports( new_network_id=None)) self.assertTrue(d_csnat_port.called) self.assertTrue(d_agent_gw_port.called) d_csnat_port.assert_called_once_with(mock.ANY, rtr) - d_agent_gw_port.assert_called_once_with(mock.ANY, None, 'ext_net_id') - del_fip.assert_called_once_with(mock.ANY, 'ext_net_id') + d_agent_gw_port.assert_called_once_with(mock.ANY, None, ext_net_id) + del_fip.assert_called_once_with(mock.ANY, ext_net_id) def _floatingip_on_port_test_setup(self, hostid): router = {'id': 'foo_router_id', 'distributed': True} diff --git a/neutron/tests/unit/db/test_l3_hamode_db.py b/neutron/tests/unit/db/test_l3_hamode_db.py index 4cf79ecad9b..5535f855b25 100644 --- a/neutron/tests/unit/db/test_l3_hamode_db.py +++ b/neutron/tests/unit/db/test_l3_hamode_db.py @@ -1264,7 +1264,7 @@ class L3HAModeDbTestCase(L3HATestFramework): router = self._migrate_router(router['id'], False) self.assertFalse(router.extra_attributes['ha']) - for routerport in router.attached_ports.all(): + for routerport in router.attached_ports: self.assertEqual(constants.DEVICE_OWNER_ROUTER_INTF, routerport.port_type) self.assertEqual(constants.DEVICE_OWNER_ROUTER_INTF,