From 1642bca4d9c4fee15129f74d93300c1eab1afd29 Mon Sep 17 00:00:00 2001 From: Eugene Nikanorov Date: Thu, 9 Apr 2015 01:16:18 +0300 Subject: [PATCH] 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 --- neutron/db/db_base_plugin_v2.py | 12 ++++-- neutron/plugins/ml2/plugin.py | 25 +++++++---- neutron/tests/unit/plugins/ml2/test_plugin.py | 41 ++++++++++++++++++- 3 files changed, 67 insertions(+), 11 deletions(-) diff --git a/neutron/db/db_base_plugin_v2.py b/neutron/db/db_base_plugin_v2.py index 01f431fa753..fb8c859c9aa 100644 --- 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 b9210377ffd..f3f8b2ab38c 100644 --- 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 e9cd744064d..1d0d1378f4a 100644 --- 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):