From: Swaminathan Vasudevan <swaminathan.vasudevan@hp.com>
Date: Thu, 8 Jan 2015 17:44:01 +0000 (-0800)
Subject: Fix FIP agent gw port delete based on external net
X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=579da4988084bc412f5477492f1311c0022a0322;p=openstack-build%2Fneutron-build.git

Fix FIP agent gw port delete based on external net

Today the FIP agent gateway port for DVR is deleted
based on the host. When there are multiple external
networks, then the port deletion for the second
external network may fail.

So the current fix checks for the valid host and
external network id and then deletes the FIP agent
gw port if it is the last one to be deleted.

Closes-Bug: #1421886

Change-Id: Ic5dfb88409c39d06d912263b338d0c5f7ebd3bcb
---

diff --git a/neutron/db/l3_dvr_db.py b/neutron/db/l3_dvr_db.py
index 47972ba75..226b30e34 100644
--- a/neutron/db/l3_dvr_db.py
+++ b/neutron/db/l3_dvr_db.py
@@ -232,10 +232,12 @@ class L3_NAT_with_dvr_db_mixin(l3_db.L3_NAT_db_mixin,
         """
         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_hostid):
-            LOG.debug('Deleting the Agent GW Port on host: %s', fip_hostid)
-            self.delete_floatingip_agent_gateway_port(context, fip_hostid)
+        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)
@@ -457,8 +459,9 @@ class L3_NAT_with_dvr_db_mixin(l3_db.L3_NAT_db_mixin,
         if ports:
             return ports[0]
 
-    def check_fips_availability_on_host(self, context, host_id):
-        """Query all floating_ips and filter by particular host."""
+    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):
             routers = self._get_sync_routers(context, router_ids=None)
@@ -467,7 +470,8 @@ class L3_NAT_with_dvr_db_mixin(l3_db.L3_NAT_db_mixin,
             # 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:
+                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
@@ -476,10 +480,12 @@ class L3_NAT_with_dvr_db_mixin(l3_db.L3_NAT_db_mixin,
                 return True
             return False
 
-    def delete_floatingip_agent_gateway_port(self, context, host_id):
-        """Function to delete the FIP agent gateway port on host."""
+    def delete_floatingip_agent_gateway_port(
+        self, context, host_id, ext_net_id):
+        """Function to delete FIP gateway port with given ext_net_id."""
         # delete any fip agent gw port
-        device_filter = {'device_owner': [DEVICE_OWNER_AGENT_GW]}
+        device_filter = {'device_owner': [DEVICE_OWNER_AGENT_GW],
+                         'network_id': [ext_net_id]}
         ports = self._core_plugin.get_ports(context,
                                             filters=device_filter)
         for p in ports:
diff --git a/neutron/tests/unit/db/test_l3_dvr_db.py b/neutron/tests/unit/db/test_l3_dvr_db.py
index 2e7316463..08cef8f6f 100644
--- a/neutron/tests/unit/db/test_l3_dvr_db.py
+++ b/neutron/tests/unit/db/test_l3_dvr_db.py
@@ -193,7 +193,7 @@ class L3DvrTestCase(testlib_api.SqlTestCase):
             mock.patch.object(self.mixin,
                               'get_vm_port_hostid'),
             mock.patch.object(self.mixin,
-                              'check_fips_availability_on_host'),
+                              'check_fips_availability_on_host_ext_net'),
             mock.patch.object(self.mixin,
                               'delete_floatingip_agent_gateway_port')
                              ) as (gfips, gvm, cfips, dfips):
@@ -206,6 +206,28 @@ class L3DvrTestCase(testlib_api.SqlTestCase):
             self.assertTrue(cfips.called)
             self.assertTrue(gvm.called)
 
+    def test_delete_floatingip_agent_gateway_port(self):
+        port = {
+            'id': 'my_port_id',
+            'binding:host_id': 'foo_host',
+            'network_id': 'ext_network_id',
+            'device_owner': l3_const.DEVICE_OWNER_AGENT_GW
+        }
+        with contextlib.nested(
+            mock.patch.object(manager.NeutronManager, 'get_plugin'),
+            mock.patch.object(self.mixin,
+                              'get_vm_port_hostid')) as (gp, vm_host):
+            plugin = mock.Mock()
+            gp.return_value = plugin
+            plugin.get_ports.return_value = [port]
+            vm_host.return_value = 'foo_host'
+            self.mixin.delete_floatingip_agent_gateway_port(
+                self.ctx, 'foo_host', 'network_id')
+        plugin.get_ports.assert_called_with(self.ctx, filters={
+            'network_id': ['network_id'],
+            'device_owner': [l3_const.DEVICE_OWNER_AGENT_GW]})
+        plugin._delete_port.assert_called_with(self.ctx, 'my_port_id')
+
     def _delete_floatingip_test_setup(self, floatingip):
         fip_id = floatingip['id']
         with contextlib.nested(