openstack
/
neutron
Code Issues Proposed changes

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
This commit is contained in:
Salvatore Orlando 2015-01-23 13:51:15 -08:00
parent 4143763913
commit 939f558226
2 changed files with 27 additions and 15 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

23
neutron/db/db_base_plugin_v2.py
View File

@ -1217,6 +1217,11 @@ class NeutronDbPluginV2(neutron_plugin_base_v2.NeutronPluginBaseV2,
result['allocation_pools'] = new_pools result['allocation_pools'] = new_pools
return result 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): def delete_subnet(self, context, id):
with context.session.begin(subtransactions=True): with context.session.begin(subtransactions=True):
subnet = self._get_subnet(context, id) subnet = self._get_subnet(context, id)
@ -1236,13 +1241,17 @@ class NeutronDbPluginV2(neutron_plugin_base_v2.NeutronPluginBaseV2,
network_ports = qry_network_ports.all() network_ports = qry_network_ports.all()
if network_ports: if network_ports:
map(context.session.delete, network_ports) map(context.session.delete, network_ports)
# Check if there are tenant owned ports # Check if there are more IP allocations, unless
tenant_ports = (context.session.query(models_v2.IPAllocation). # is_auto_address_subnet is True. In that case the check is
filter_by(subnet_id=subnet['id']). # unnecessary. This additional check not only would be wasteful
join(models_v2.Port). # for this class of subnet, but is also error-prone since when
filter_by(network_id=subnet['network_id']).first()) # the isolation level is set to READ COMMITTED allocations made
if tenant_ports: # concurrently will be returned by this query
raise n_exc.SubnetInUse(subnet_id=id) 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) context.session.delete(subnet)

19
neutron/plugins/ml2/plugin.py
View File

@ -807,14 +807,17 @@ class Ml2Plugin(db_base_plugin_v2.NeutronDbPluginV2,
if allocated: if allocated:
map(session.delete, allocated) map(session.delete, allocated)
LOG.debug("Ports to auto-deallocate: %s", allocated) LOG.debug("Ports to auto-deallocate: %s", allocated)
# Check if there are tenant owned ports # Check if there are more IP allocations, unless
tenant_port = (session.query(models_v2.IPAllocation). # is_auto_address_subnet is True. In that case the check is
filter_by(subnet_id=id). # unnecessary. This additional check not only would be wasteful
join(models_v2.Port). # for this class of subnet, but is also error-prone since when
first()) # the isolation level is set to READ COMMITTED allocations made
if tenant_port: # concurrently will be returned by this query
LOG.debug("Tenant-owned ports exist") if not is_auto_addr_subnet:
raise exc.SubnetInUse(subnet_id=id) 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 # If allocated is None, then all the IPAllocation were
# correctly deleted during the previous pass. # correctly deleted during the previous pass.