Handle race condition on subnet-delete

This fix targets quite rare case of race condition between
port creation and subnet deletion. This usually happens
during API tests that do things quickly.
DHCP port is being created after delete_subnet checks for
DHCP ports, but before it checks for IPAllocations on subnet.
The solution is to apply retrying logic, which is really necessary
as we can't fetch new IPAllocations with the same query and within
the active transaction in mysql because of REPEATABLE READ
transaction isolation.

Change-Id: Ib9da018e654cdee3b64aa38de90f171c92ee28ee
Closes-Bug: 1357055

diff --git a/neutron/db/db_base_plugin_v2.py b/neutron/db/db_base_plugin_v2.py
index 01f431fa7531690682cb53453fac499b23f185d2..fb8c859c9aa32c0bdccf989fd1417e7c1c317e5c 100644 (file)
--- a/neutron/db/db_base_plugin_v2.py
+++ b/neutron/db/db_base_plugin_v2.py
@@ -1465,9 +1465,15 @@ class NeutronDbPluginV2(neutron_plugin_base_v2.NeutronPluginBaseV2,
         return result
 
     def _subnet_check_ip_allocations(self, context, subnet_id):
-        return context.session.query(
-            models_v2.IPAllocation).filter_by(
-                subnet_id=subnet_id).join(models_v2.Port).first()
+        return (context.session.query(models_v2.IPAllocation).
+                filter_by(subnet_id=subnet_id).join(models_v2.Port).first())
+
+    def _subnet_get_user_allocation(self, context, subnet_id):
+        """Check if there are any user ports on subnet and return first."""
+        return (context.session.query(models_v2.IPAllocation).
+                filter_by(subnet_id=subnet_id).join().
+                filter(~models_v2.Port.device_owner.
+                       in_(AUTO_DELETE_PORT_OWNERS)).first())
 
     def _subnet_check_ip_allocations_internal_router_ports(self, context,
                                                            subnet_id):

diff --git a/neutron/plugins/ml2/plugin.py b/neutron/plugins/ml2/plugin.py
index b9210377ffd9dd69554901da7e8a84d2b7219665..f3f8b2ab38c651d705e3819f6568fce013e2fe23 100644 (file)
--- a/neutron/plugins/ml2/plugin.py
+++ b/neutron/plugins/ml2/plugin.py
@@ -828,6 +828,8 @@ class Ml2Plugin(db_base_plugin_v2.NeutronDbPluginV2,
         self.mechanism_manager.update_subnet_postcommit(mech_context)
         return updated_subnet
 
+    @oslo_db_api.wrap_db_retry(max_retries=db_api.MAX_RETRIES,
+                               retry_on_request=True)
     def delete_subnet(self, context, id):
         # REVISIT(rkukura) The super(Ml2Plugin, self).delete_subnet()
         # function is not used because it deallocates the subnet's addresses
@@ -875,13 +877,22 @@ class Ml2Plugin(db_base_plugin_v2.NeutronDbPluginV2,
                 if not is_auto_addr_subnet:
                     alloc = self._subnet_check_ip_allocations(context, id)
                     if alloc:
-                        LOG.info(_LI("Found port (%(port_id)s, %(ip)s) "
-                                     "having IP allocation on subnet "
-                                     "%(subnet)s, cannot delete"),
-                                 {'ip': alloc.ip_address,
-                                  'port_id': alloc.port_id,
-                                  'subnet': id})
-                        raise exc.SubnetInUse(subnet_id=id)
+                        user_alloc = self._subnet_get_user_allocation(
+                            context, id)
+                        if user_alloc:
+                            LOG.info(_LI("Found port (%(port_id)s, %(ip)s) "
+                                         "having IP allocation on subnet "
+                                         "%(subnet)s, cannot delete"),
+                                     {'ip': user_alloc.ip_address,
+                                      'port_id': user_alloc.ports.id,
+                                      'subnet': id})
+                            raise exc.SubnetInUse(subnet_id=id)
+                        else:
+                            # allocation found and it was DHCP port
+                            # that appeared after autodelete ports were
+                            # removed - need to restart whole operation
+                            raise os_db_exception.RetryRequest(
+                                exc.SubnetInUse(subnet_id=id))
 
                 # If allocated is None, then all the IPAllocation were
                 # correctly deleted during the previous pass.

diff --git a/neutron/tests/unit/plugins/ml2/test_plugin.py b/neutron/tests/unit/plugins/ml2/test_plugin.py
index e9cd744064de93451143493120e153a45db200aa..1d0d1378f4af74dead4236ea339c448135d40217 100644 (file)
--- a/neutron/tests/unit/plugins/ml2/test_plugin.py
+++ b/neutron/tests/unit/plugins/ml2/test_plugin.py
@@ -32,6 +32,7 @@ from neutron import context
 from neutron.db import api as db_api
 from neutron.db import db_base_plugin_v2 as base_plugin
 from neutron.db import l3_db
+from neutron.db import models_v2
 from neutron.extensions import external_net as external_net
 from neutron.extensions import multiprovidernet as mpnet
 from neutron.extensions import portbindings
@@ -268,7 +269,45 @@ class TestMl2NetworksV2(test_plugin.TestNetworksV2,
 
 class TestMl2SubnetsV2(test_plugin.TestSubnetsV2,
                        Ml2PluginV2TestCase):
-    pass
+    def test_delete_subnet_race_with_dhcp_port_creation(self):
+        with self.network() as network:
+            with self.subnet(network=network) as subnet:
+                subnet_id = subnet['subnet']['id']
+                attempt = [0]
+
+                def check_and_create_ports(context, subnet_id):
+                    """A method to emulate race condition.
+
+                    Adds dhcp port in the middle of subnet delete
+                    """
+                    if attempt[0] > 0:
+                        return False
+                    attempt[0] += 1
+                    data = {'port': {'network_id': network['network']['id'],
+                                     'tenant_id':
+                                     network['network']['tenant_id'],
+                                     'name': 'port1',
+                                     'admin_state_up': 1,
+                                     'device_owner':
+                                     constants.DEVICE_OWNER_DHCP,
+                                     'fixed_ips': [{'subnet_id': subnet_id}]}}
+                    port_req = self.new_create_request('ports', data)
+                    port_res = port_req.get_response(self.api)
+                    self.assertEqual(201, port_res.status_int)
+                    return (context.session.query(models_v2.IPAllocation).
+                            filter_by(subnet_id=subnet_id).
+                            join(models_v2.Port).first())
+
+                plugin = manager.NeutronManager.get_plugin()
+                # we mock _subnet_check_ip_allocations with method
+                # that creates DHCP port 'in the middle' of subnet_delete
+                # causing retry this way subnet is deleted on the
+                # second attempt
+                with mock.patch.object(plugin, '_subnet_check_ip_allocations',
+                                       side_effect=check_and_create_ports):
+                    req = self.new_delete_request('subnets', subnet_id)
+                    res = req.get_response(self.api)
+                    self.assertEqual(204, res.status_int)
 
 
 class TestMl2PortsV2(test_plugin.TestPortsV2, Ml2PluginV2TestCase):