Merge "Fix for floatingip-delete not removing fip_gw port"

neutron/db/l3_dvr_db.py

@@ -144,24 +144,41 @@ def _update_fip_assoc(self, context, fip, floatingip_db, external_port):
         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)

neutron/tests/unit/db/test_l3_dvr_db.py

@@ -20,9 +20,13 @@
 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 @@ def test_build_routers_list_with_gw_port_mismatch(self):
         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)