From 939f55822615bb352d86d575fbdf50337be6c8a8 Mon Sep 17 00:00:00 2001 From: Salvatore Orlando Date: Fri, 23 Jan 2015 13:51:15 -0800 Subject: [PATCH] Do not check twice IP allocations for auto-address subnets For auto-address subnets such as those with SLAAC and DHCP_STATELESS address modes it is ok to delete them even when there are active IP allocations. The current logic might trigger unexpected 409 errors if IP allocations are made on these subnets concurrently with their deletion. This patch simply ensures the final check for active IP allocations is not performed for this class of subnets; since all IP allocations will be removed anyway, it does not make sense to check whether there are allocations at all. In fact, doing this check might cause a failure of the delete operation if an IP allocation is made concurrently. This patch also factors out the logic for checking whether there are IP allocations on the subnet to avoid code duplication. Closes-Bug: #1414199 Change-Id: I1c4ca6f677845f6e2e8d88f3629c0e91ca73b5d0 --- neutron/db/db_base_plugin_v2.py | 23 ++++++++++++++++------- neutron/plugins/ml2/plugin.py | 19 +++++++++++-------- 2 files changed, 27 insertions(+), 15 deletions(-) diff --git a/neutron/db/db_base_plugin_v2.py b/neutron/db/db_base_plugin_v2.py index af241d0db..9eb281bfd 100644 --- a/neutron/db/db_base_plugin_v2.py +++ b/neutron/db/db_base_plugin_v2.py @@ -1217,6 +1217,11 @@ class NeutronDbPluginV2(neutron_plugin_base_v2.NeutronPluginBaseV2, result['allocation_pools'] = new_pools 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() + def delete_subnet(self, context, id): with context.session.begin(subtransactions=True): subnet = self._get_subnet(context, id) @@ -1236,13 +1241,17 @@ class NeutronDbPluginV2(neutron_plugin_base_v2.NeutronPluginBaseV2, network_ports = qry_network_ports.all() if network_ports: map(context.session.delete, network_ports) - # Check if there are tenant owned ports - tenant_ports = (context.session.query(models_v2.IPAllocation). - filter_by(subnet_id=subnet['id']). - join(models_v2.Port). - filter_by(network_id=subnet['network_id']).first()) - if tenant_ports: - raise n_exc.SubnetInUse(subnet_id=id) + # Check if there are more IP allocations, unless + # is_auto_address_subnet is True. In that case the check is + # unnecessary. This additional check not only would be wasteful + # for this class of subnet, but is also error-prone since when + # the isolation level is set to READ COMMITTED allocations made + # concurrently will be returned by this query + if not is_auto_addr_subnet: + if self._subnet_check_ip_allocations(context, id): + LOG.debug("Found IP allocations on subnet %s, " + "cannot delete", id) + raise n_exc.SubnetInUse(subnet_id=id) context.session.delete(subnet) diff --git a/neutron/plugins/ml2/plugin.py b/neutron/plugins/ml2/plugin.py index 2520f9cc6..475f97d28 100644 --- a/neutron/plugins/ml2/plugin.py +++ b/neutron/plugins/ml2/plugin.py @@ -807,14 +807,17 @@ class Ml2Plugin(db_base_plugin_v2.NeutronDbPluginV2, if allocated: map(session.delete, allocated) LOG.debug("Ports to auto-deallocate: %s", allocated) - # Check if there are tenant owned ports - tenant_port = (session.query(models_v2.IPAllocation). - filter_by(subnet_id=id). - join(models_v2.Port). - first()) - if tenant_port: - LOG.debug("Tenant-owned ports exist") - raise exc.SubnetInUse(subnet_id=id) + # Check if there are more IP allocations, unless + # is_auto_address_subnet is True. In that case the check is + # unnecessary. This additional check not only would be wasteful + # for this class of subnet, but is also error-prone since when + # the isolation level is set to READ COMMITTED allocations made + # concurrently will be returned by this query + if not is_auto_addr_subnet: + if self._subnet_check_ip_allocations(context, id): + LOG.debug("Found IP allocations on subnet %s, " + "cannot delete", id) + raise exc.SubnetInUse(subnet_id=id) # If allocated is None, then all the IPAllocation were # correctly deleted during the previous pass. -- 2.45.2