From 6994ade726f4a33753a11b2b2c24454160c96d31 Mon Sep 17 00:00:00 2001 From: Swaminathan Vasudevan Date: Thu, 31 Jul 2014 14:57:08 -0700 Subject: [PATCH] Fix for floatingip-delete not removing fip_gw port fip_agent_gateway_port is created dynamically on a compute node where FIP namespace is created. When floatingip-disassociate is called this 'port' is deleted. But when we call floatingip-delete this port is not deleted. This patch fixes the issue mentioned above. Closes-Bug: #1351066 Change-Id: I1aa51680394547390546fe362abfae7a5d25425b --- neutron/db/l3_dvr_db.py | 39 +++++++++++----- neutron/tests/unit/db/test_l3_dvr_db.py | 61 +++++++++++++++++++++++++ 2 files changed, 89 insertions(+), 11 deletions(-) diff --git a/neutron/db/l3_dvr_db.py b/neutron/db/l3_dvr_db.py index b73ad8518..75b8ab356 100644 --- a/neutron/db/l3_dvr_db.py +++ b/neutron/db/l3_dvr_db.py @@ -144,24 +144,41 @@ class L3_NAT_with_dvr_db_mixin(l3_db.L3_NAT_db_mixin, previous_router_id = floatingip_db.router_id port_id, internal_ip_address, router_id = ( self._check_and_get_fip_assoc(context, fip, floatingip_db)) - agt_gw_port_check = False admin_ctx = context.elevated() if (not ('port_id' in fip and fip['port_id'])) and ( floatingip_db['fixed_port_id'] is not None): - port_db = self._core_plugin._get_port( - context, floatingip_db['fixed_port_id']) - LOG.debug("VM Port info: %s", port_db) - fip_hostid = self.get_vm_port_hostid(context, port_db['id']) - if fip_hostid: - agt_gw_port_check = self.check_fips_availability_on_host( - admin_ctx, fip['id'], fip_hostid) + self.clear_unused_fip_agent_gw_port( + admin_ctx, floatingip_db, fip['id']) floatingip_db.update({'fixed_ip_address': internal_ip_address, 'fixed_port_id': port_id, 'router_id': router_id, 'last_known_router_id': previous_router_id}) - if agt_gw_port_check: - LOG.debug('Deleting the Agent GW Port') - self.delete_floatingip_agent_gateway_port(admin_ctx, fip_hostid) + + def clear_unused_fip_agent_gw_port( + self, context, floatingip_db, fip_id): + """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( + context, fip_id, fip_hostid): + LOG.debug('Deleting the Agent GW Port on host: %s', fip_hostid) + self.delete_floatingip_agent_gateway_port(context, fip_hostid) + + 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, id) + super(L3_NAT_with_dvr_db_mixin, + self).delete_floatingip(context, id) def add_router_interface(self, context, router_id, interface_info): add_by_port, add_by_sub = self._validate_interface_info(interface_info) diff --git a/neutron/tests/unit/db/test_l3_dvr_db.py b/neutron/tests/unit/db/test_l3_dvr_db.py index 2bcfa01ec..9612aa7b2 100644 --- a/neutron/tests/unit/db/test_l3_dvr_db.py +++ b/neutron/tests/unit/db/test_l3_dvr_db.py @@ -20,9 +20,13 @@ from neutron.common import constants as l3_const from neutron import context from neutron.db import l3_dvr_db from neutron import manager +from neutron.openstack.common import uuidutils from neutron.tests.unit import testlib_api +_uuid = uuidutils.generate_uuid + + class L3DvrTestCase(testlib_api.SqlTestCase): def setUp(self): @@ -170,3 +174,60 @@ class L3DvrTestCase(testlib_api.SqlTestCase): gw_ports = {} 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() + } + fip_id = floatingip['id'] + with contextlib.nested( + mock.patch.object(l3_dvr_db.l3_db.L3_NAT_db_mixin, + '_get_floatingip'), + mock.patch.object(self.mixin, + 'get_vm_port_hostid'), + mock.patch.object(self.mixin, + 'check_fips_availability_on_host'), + mock.patch.object(self.mixin, + 'delete_floatingip_agent_gateway_port') + ) as (gfips, gvm, cfips, 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, fip_id) + self.assertTrue(dfips.called) + self.assertTrue(cfips.called) + self.assertTrue(gvm.called) + + def _delete_floatingip_test_setup(self, floatingip): + fip_id = floatingip['id'] + with contextlib.nested( + mock.patch.object(l3_dvr_db.l3_db.L3_NAT_db_mixin, + '_get_floatingip'), + mock.patch.object(self.mixin, + 'clear_unused_fip_agent_gw_port'), + mock.patch.object(l3_dvr_db.l3_db.L3_NAT_db_mixin, + 'delete_floatingip')) as (gf, vf, df): + gf.return_value = floatingip + self.mixin.delete_floatingip(self.ctx, fip_id) + return vf + + 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) -- 2.45.2