]> review.fuel-infra Code Review - openstack-build/neutron-build.git/commitdiff
Fix FIP agent gw port delete based on external net
authorSwaminathan Vasudevan <swaminathan.vasudevan@hp.com>
Thu, 8 Jan 2015 17:44:01 +0000 (09:44 -0800)
committerSwaminathan Vasudevan <swaminathan.vasudevan@hp.com>
Tue, 17 Feb 2015 18:41:19 +0000 (18:41 +0000)
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

neutron/db/l3_dvr_db.py
neutron/tests/unit/db/test_l3_dvr_db.py

index 47972ba75785887b3ab91bf5a0c7615df3ccba40..226b30e34f0c3b06cde257a8049e7d311db8f6fc 100644 (file)
@@ -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:
index 2e7316463f3863f311634b3491560fb26a750d27..08cef8f6f585db7f454e9e55a0bc206d63be29e2 100644 (file)
@@ -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(