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

diff --git a/neutron/db/db_base_plugin_v2.py b/neutron/db/db_base_plugin_v2.py
index af241d0db83472a1badf88ef4e017c336db25627..9eb281bfdce816ddc24ff3c95f66e63efbc9df34 100644 (file)
--- 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 2520f9cc6170c0df99e2ba58e467d58613cd3b8c..475f97d28cb4351bbb0f903509c5974401b53dbe 100644 (file)
--- 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.