From cd45f16442b7c56c4876bef527c9c83ea0907c40 Mon Sep 17 00:00:00 2001 From: Swaminathan Vasudevan Date: Mon, 22 Jun 2015 17:17:15 -0700 Subject: [PATCH] Cleanup the fip agent gateway port delete routines Based on the parent patch, right now the Floatingip agent gateway ports will only be deleted when the last gateway port associated with the external network is deleted. The Floatingip agent gateway port will not be deleted for every floatingip dis-association and deletion. The Floatingip agent gateway port was created on all nodes as a substitute for the gateway port. So it makes sense to delete those ports only when the last gateway port on the external network is deleted. The agent should be able to delete the floatingip agent gateway port on a given external network when it is not required. This would substantially reduce the burden on the server to validate, read and delete the port form the DB. Change-Id: Ie561b19a2e58a2a563d79b75421e9e24c70f36f9 Closes-Bug: #1468007 Closes-Bug: #1408855 Closes-Bug: #1450982 --- neutron/db/l3_dvr_db.py | 77 +----------- neutron/tests/unit/db/test_l3_dvr_db.py | 149 +----------------------- 2 files changed, 7 insertions(+), 219 deletions(-) diff --git a/neutron/db/l3_dvr_db.py b/neutron/db/l3_dvr_db.py index e1619b245..3d947a7c1 100644 --- a/neutron/db/l3_dvr_db.py +++ b/neutron/db/l3_dvr_db.py @@ -206,25 +206,12 @@ class L3_NAT_with_dvr_db_mixin(l3_db.L3_NAT_db_mixin, ) def _update_fip_assoc(self, context, fip, floatingip_db, external_port): - """Override to create and delete floating agent gw port for DVR. + """Override to create floating agent gw port for DVR. Floating IP Agent gateway port will be created when a floatingIP association happens. - Floating IP Agent gateway port will be deleted when a - floatingIP disassociation happens. """ fip_port = fip.get('port_id') - unused_fip_agent_gw_port = ( - fip_port is None and floatingip_db['fixed_port_id']) - if unused_fip_agent_gw_port and floatingip_db.get('router_id'): - admin_ctx = context.elevated() - router_dict = self.get_router( - admin_ctx, floatingip_db['router_id']) - # Check if distributed router and then delete the - # FloatingIP agent gateway port - if router_dict.get('distributed'): - self._clear_unused_fip_agent_gw_port( - admin_ctx, floatingip_db) super(L3_NAT_with_dvr_db_mixin, self)._update_fip_assoc( context, fip, floatingip_db, external_port) associate_fip = fip_port and floatingip_db['id'] @@ -249,54 +236,12 @@ class L3_NAT_with_dvr_db_mixin(l3_db.L3_NAT_db_mixin, vm_hostid)) LOG.debug("FIP Agent gateway port: %s", fip_agent_port) - def _clear_unused_fip_agent_gw_port( - self, context, floatingip_db): - """Helper function to check for fip agent gw port and delete. - - This function checks on compute nodes to make sure if there - are any VMs using the FIP agent gateway port. If no VMs are - using the FIP agent gateway port, it will go ahead and delete - the FIP agent gateway port. If even a single VM is using the - port it will not delete. - """ - fip_hostid = self._get_vm_port_hostid( - context, floatingip_db['fixed_port_id']) - if fip_hostid and self._check_fips_availability_on_host_ext_net( - context, fip_hostid, floatingip_db['floating_network_id']): - LOG.debug('Deleting the Agent GW Port for ext-net: ' - '%s', floatingip_db['floating_network_id']) - self.delete_floatingip_agent_gateway_port( - context, fip_hostid, floatingip_db['floating_network_id']) - - def delete_floatingip(self, context, id): - floatingip = self._get_floatingip(context, id) - if floatingip['fixed_port_id']: - admin_ctx = context.elevated() - self._clear_unused_fip_agent_gw_port( - admin_ctx, floatingip) - super(L3_NAT_with_dvr_db_mixin, - self).delete_floatingip(context, id) - def _get_floatingip_on_port(self, context, port_id=None): """Helper function to retrieve the fip associated with port.""" fip_qry = context.session.query(l3_db.FloatingIP) floating_ip = fip_qry.filter_by(fixed_port_id=port_id) return floating_ip.first() - def disassociate_floatingips(self, context, port_id, do_notify=True): - """Override disassociate floatingips to delete fip agent gw port.""" - with context.session.begin(subtransactions=True): - fip = self._get_floatingip_on_port( - context, port_id=port_id) - if fip: - admin_ctx = context.elevated() - self._clear_unused_fip_agent_gw_port( - admin_ctx, fip) - return super(L3_NAT_with_dvr_db_mixin, - self).disassociate_floatingips(context, - port_id, - do_notify=do_notify) - def add_router_interface(self, context, router_id, interface_info): add_by_port, add_by_sub = self._validate_interface_info(interface_info) router = self._get_router(context, router_id) @@ -533,26 +478,6 @@ class L3_NAT_with_dvr_db_mixin(l3_db.L3_NAT_db_mixin, query = self._model_query(context, l3_db.Router.id) return [row[0] for row in query] - def _check_fips_availability_on_host_ext_net( - self, context, host_id, fip_ext_net_id): - """Query all floating_ips and filter on host and external net.""" - fip_count_on_host = 0 - with context.session.begin(subtransactions=True): - router_ids = self._get_router_ids(context) - floating_ips = self._get_sync_floating_ips(context, router_ids) - # Check for the active floatingip in the host - for fip in floating_ips: - f_host = self._get_vm_port_hostid(context, fip['port_id']) - if (f_host == host_id and - (fip['floating_network_id'] == fip_ext_net_id)): - fip_count_on_host += 1 - # If fip_count greater than 1 or equal to zero no action taken - # if the fip_count is equal to 1, then this would be last active - # fip in the host, so the agent gateway port can be deleted. - if fip_count_on_host == 1: - return True - return False - def delete_floatingip_agent_gateway_port( self, context, host_id, ext_net_id): """Function to delete FIP gateway port with given ext_net_id.""" diff --git a/neutron/tests/unit/db/test_l3_dvr_db.py b/neutron/tests/unit/db/test_l3_dvr_db.py index 0dc5330a4..00ea0d9e8 100644 --- a/neutron/tests/unit/db/test_l3_dvr_db.py +++ b/neutron/tests/unit/db/test_l3_dvr_db.py @@ -239,30 +239,6 @@ class L3DvrTestCase(test_db_base_plugin_v2.NeutronDbPluginV2TestCase): routers = self.mixin._build_routers_list(self.ctx, routers, gw_ports) self.assertIsNone(routers[0].get('gw_port')) - def test__clear_unused_fip_agent_gw_port(self): - floatingip = { - 'id': _uuid(), - 'fixed_port_id': _uuid(), - 'floating_network_id': _uuid() - } - with mock.patch.object(l3_dvr_db.l3_db.L3_NAT_db_mixin, - '_get_floatingip') as gfips,\ - mock.patch.object(self.mixin, '_get_vm_port_hostid') as gvm,\ - mock.patch.object( - self.mixin, - '_check_fips_availability_on_host_ext_net') as cfips,\ - mock.patch.object( - self.mixin, - 'delete_floatingip_agent_gateway_port') as dfips: - gfips.return_value = floatingip - gvm.return_value = 'my-host' - cfips.return_value = True - self.mixin._clear_unused_fip_agent_gw_port( - self.ctx, floatingip) - self.assertTrue(dfips.called) - self.assertTrue(cfips.called) - self.assertTrue(gvm.called) - def setup_port_has_ipv6_address(self, port): with mock.patch.object(l3_dvr_db.l3_db.L3_NAT_db_mixin, '_port_has_ipv6_address') as pv6: @@ -384,60 +360,6 @@ class L3DvrTestCase(test_db_base_plugin_v2.NeutronDbPluginV2TestCase): d_csnat_port.assert_called_once_with( mock.ANY, rtr) - def _delete_floatingip_test_setup(self, floatingip): - fip_id = floatingip['id'] - with mock.patch.object(l3_dvr_db.l3_db.L3_NAT_db_mixin, - '_get_floatingip') as gf,\ - mock.patch.object(self.mixin, - '_clear_unused_fip_agent_gw_port') as vf,\ - mock.patch.object(l3_dvr_db.l3_db.L3_NAT_db_mixin, - 'delete_floatingip'): - gf.return_value = floatingip - self.mixin.delete_floatingip(self.ctx, fip_id) - return vf - - def _disassociate_floatingip_setup(self, port_id=None, floatingip=None): - with mock.patch.object(self.mixin, '_get_floatingip_on_port') as gf,\ - mock.patch.object(self.mixin, - '_clear_unused_fip_agent_gw_port') as vf: - gf.return_value = floatingip - self.mixin.disassociate_floatingips( - self.ctx, port_id, do_notify=False) - return vf - - def test_disassociate_floatingip_with_vm_port(self): - port_id = '1234' - floatingip = { - 'id': _uuid(), - 'fixed_port_id': 1234, - 'floating_network_id': _uuid() - } - mock_disassociate_fip = self._disassociate_floatingip_setup( - port_id=port_id, floatingip=floatingip) - self.assertTrue(mock_disassociate_fip.called) - - def test_disassociate_floatingip_with_no_vm_port(self): - mock_disassociate_fip = self._disassociate_floatingip_setup() - self.assertFalse(mock_disassociate_fip.called) - - def test_delete_floatingip_without_internal_port(self): - floatingip = { - 'id': _uuid(), - 'fixed_port_id': None, - 'floating_network_id': _uuid() - } - mock_fip_clear = self._delete_floatingip_test_setup(floatingip) - self.assertFalse(mock_fip_clear.call_count) - - def test_delete_floatingip_with_internal_port(self): - floatingip = { - 'id': _uuid(), - 'fixed_port_id': _uuid(), - 'floating_network_id': _uuid() - } - mock_fip_clear = self._delete_floatingip_test_setup(floatingip) - self.assertTrue(mock_fip_clear.called) - def _floatingip_on_port_test_setup(self, hostid): router = {'id': 'foo_router_id', 'distributed': True} floatingip = { @@ -487,28 +409,7 @@ class L3DvrTestCase(test_db_base_plugin_v2.NeutronDbPluginV2TestCase): self.assertIn('fip_interface', router[l3_const.FLOATINGIP_AGENT_INTF_KEY]) - def test_delete_disassociated_floatingip_agent_port(self): - fip = { - 'id': _uuid(), - 'port_id': None - } - floatingip = { - 'id': _uuid(), - 'fixed_port_id': 1234, - 'router_id': 'foo_router_id' - } - router = {'id': 'foo_router_id', 'distributed': True} - with mock.patch.object(self.mixin, 'get_router') as grtr,\ - mock.patch.object(self.mixin, - '_clear_unused_fip_agent_gw_port') as vf,\ - mock.patch.object(l3_dvr_db.l3_db.L3_NAT_db_mixin, - '_update_fip_assoc'): - grtr.return_value = router - self.mixin._update_fip_assoc( - self.ctx, fip, floatingip, mock.ANY) - self.assertTrue(vf.called) - - def _setup_test_create_delete_floatingip( + def _setup_test_create_floatingip( self, fip, floatingip_db, router_db): port = { 'id': '1234', @@ -518,8 +419,6 @@ class L3DvrTestCase(test_db_base_plugin_v2.NeutronDbPluginV2TestCase): with mock.patch.object(self.mixin, 'get_router') as grtr,\ mock.patch.object(self.mixin, '_get_vm_port_hostid') as vmp,\ - mock.patch.object(self.mixin, - '_clear_unused_fip_agent_gw_port') as d_fip,\ mock.patch.object( self.mixin, 'create_fip_agent_gw_port_if_not_exists') as c_fip,\ @@ -529,7 +428,7 @@ class L3DvrTestCase(test_db_base_plugin_v2.NeutronDbPluginV2TestCase): vmp.return_value = 'my-host' self.mixin._update_fip_assoc( self.ctx, fip, floatingip_db, port) - return d_fip, c_fip + return c_fip def test_create_floatingip_agent_gw_port_with_dvr_router(self): floatingip = { @@ -541,11 +440,10 @@ class L3DvrTestCase(test_db_base_plugin_v2.NeutronDbPluginV2TestCase): 'id': _uuid(), 'port_id': _uuid() } - delete_fip, create_fip = ( - self._setup_test_create_delete_floatingip( + create_fip = ( + self._setup_test_create_floatingip( fip, floatingip, router)) self.assertTrue(create_fip.called) - self.assertFalse(delete_fip.called) def test_create_floatingip_agent_gw_port_with_non_dvr_router(self): floatingip = { @@ -557,45 +455,10 @@ class L3DvrTestCase(test_db_base_plugin_v2.NeutronDbPluginV2TestCase): 'id': _uuid(), 'port_id': _uuid() } - delete_fip, create_fip = ( - self._setup_test_create_delete_floatingip( - fip, floatingip, router)) - self.assertFalse(create_fip.called) - self.assertFalse(delete_fip.called) - - def test_delete_floatingip_agent_gw_port_with_dvr_router(self): - floatingip = { - 'id': _uuid(), - 'fixed_port_id': 1234, - 'router_id': 'foo_router_id' - } - router = {'id': 'foo_router_id', 'distributed': True} - fip = { - 'id': _uuid(), - 'port_id': None - } - delete_fip, create_fip = ( - self._setup_test_create_delete_floatingip( - fip, floatingip, router)) - self.assertTrue(delete_fip.called) - self.assertFalse(create_fip.called) - - def test_delete_floatingip_agent_gw_port_with_non_dvr_router(self): - floatingip = { - 'id': _uuid(), - 'fixed_port_id': 1234, - 'router_id': 'foo_router_id' - } - router = {'id': 'foo_router_id', 'distributed': False} - fip = { - 'id': _uuid(), - 'port_id': None - } - delete_fip, create_fip = ( - self._setup_test_create_delete_floatingip( + create_fip = ( + self._setup_test_create_floatingip( fip, floatingip, router)) self.assertFalse(create_fip.called) - self.assertFalse(delete_fip.called) def test_remove_router_interface_delete_router_l3agent_binding(self): interface_info = {'subnet_id': '123'} -- 2.45.2