openstack
/
neutron
Code Issues Proposed changes

Update ml2 delete_subnet to deallocate via ipam 

Previosly during delete_subnet in ml2 plugin ip allocation were removed
directly from database. This way ipam driver was unaware of this
deallocation.

Updated workflow to skip removing ip allocations directly from database.
Now ips are deallocated during update_port workflow (L1008).

This resolves issue with dhcp ports left in allocated state on ipam
provides side. Now they are correctly deallocated via update_port.

But this patch has next limitation: currently SLAAC allocations can
not be delete via update_port workflow, so ipam driver is still unaware
of such deallocations.
This part of issue is expected to be fixed as separate patch.

Subnet_in_use check was reworked. Previously it assumed that
auto-allocated ip are already deleted by the time of this check, but
with new workflow auto allocated ips are deleted later (on update_port).
So now this check verifies if there are any user allocated ips instead
of checking all allocations.

Partial-Bug: #1564335
Change-Id: I08d66da8cb57ed88e11ec2b18c8345edfce37d37
This commit is contained in:
Pavel Bondar 2016-03-31 13:20:39 +03:00 committed by Akihiro Motoki
parent d8ae9cf475
commit 3a2e41bf70
2 changed files with 50 additions and 33 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -958,6 +958,7 @@ class Ml2Plugin(db_base_plugin_v2.NeutronDbPluginV2,
LOG.debug("Deleting subnet %s", id)
session = context.session
deallocated = set()
while True:
with session.begin(subtransactions=True):
record = self._get_subnet(context, id)
@ -976,43 +977,52 @@ class Ml2Plugin(db_base_plugin_v2.NeutronDbPluginV2,
qry_allocated = (
qry_allocated.filter(models_v2.Port.device_owner.
in_(db_base_plugin_v2.AUTO_DELETE_PORT_OWNERS)))
allocated = qry_allocated.all()
# Delete all the IPAllocation that can be auto-deleted
if allocated:
for x in allocated:
session.delete(x)
allocated = set(qry_allocated.all())
LOG.debug("Ports to auto-deallocate: %s", allocated)
# 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:
alloc = self._subnet_check_ip_allocations(context, id)
if alloc:
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.port_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))
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.port_id,
'subnet': id})
raise exc.SubnetInUse(subnet_id=id)
db_base_plugin_v2._check_subnet_not_used(context, id)
# If allocated is None, then all the IPAllocation were
# correctly deleted during the previous pass.
if not allocated:
# SLAAC allocations currently can not be removed using
# update_port workflow, and will persist in 'allocated'.
# So for now just make sure update_port is called once for
# them so MechanismDrivers is aware of the change.
# This way SLAAC allocation is deleted by FK on subnet deletion
# TODO(pbondar): rework update_port workflow to allow deletion
# of SLAAC allocation via update_port.
to_deallocate = allocated - deallocated
# If to_deallocate is blank, then all known IPAllocations
# (except SLAAC allocations) were correctly deleted
# during the previous pass.
# Check if there are more IP allocations, unless
# is_auto_address_subnet is True. If transaction isolation
# level is set to READ COMMITTED allocations made
# concurrently will be returned by this query and transaction
# will be restarted. It works for REPEATABLE READ isolation
# level too because this query is executed only once during
# transaction, and if concurrent allocations are detected
# transaction gets restarted. Executing this query second time
# in transaction would result in not seeing allocations
# committed by concurrent transactions.
if not to_deallocate:
if (not is_auto_addr_subnet and
self._subnet_check_ip_allocations(context, id)):
# 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))
network = self.get_network(context, subnet['network_id'])
mech_context = driver_context.SubnetContext(self, context,
subnet,
@ -1030,7 +1040,8 @@ class Ml2Plugin(db_base_plugin_v2.NeutronDbPluginV2,
LOG.debug("Committing transaction")
break
for a in allocated:
for a in to_deallocate:
deallocated.add(a)
if a.port:
# calling update_port() for each allocation to remove the
# IP from the port and call the MechanismDrivers

6
neutron/tests/unit/plugins/ml2/test_plugin.py
View File

@ -457,6 +457,12 @@ class TestMl2SubnetsV2(test_plugin.TestSubnetsV2,
req = self.new_delete_request('subnets', subnet_id)
res = req.get_response(self.api)
self.assertEqual(204, res.status_int)
# Validate chat check is called twice, i.e. after the
# first check transaction gets restarted.
calls = [mock.call(mock.ANY, subnet_id),
mock.call(mock.ANY, subnet_id)]
plugin._subnet_check_ip_allocations.assert_has_calls = (
calls)
class TestMl2DbOperationBounds(test_plugin.DbOperationBoundMixin,