Merge "Get rid of delete_subnet method in ML2" into stable/newton
This commit is contained in:
Jenkins
Gerrit Code Review
commit
a85d94dc35
|
|
@ -28,6 +28,7 @@ from oslo_utils import excutils
|
|||
from oslo_utils import uuidutils
|
||||
from sqlalchemy import and_
|
||||
from sqlalchemy import event
|
||||
from sqlalchemy import exc as sql_exc
|
||||
from sqlalchemy import not_
|
||||
|
||||
from neutron._i18n import _, _LE, _LI
|
||||
|
|
@ -835,10 +836,6 @@ class NeutronDbPluginV2(db_base_plugin_common.DbBasePluginCommon,
|
|||
|
||||
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 _subnet_get_user_allocation(self, context, subnet_id):
|
||||
"""Check if there are any user ports on subnet and return first."""
|
||||
# need to join with ports table as IPAllocation's port
|
||||
|
|
@ -863,55 +860,71 @@ class NeutronDbPluginV2(db_base_plugin_common.DbBasePluginCommon,
|
|||
"cannot delete", subnet_id)
|
||||
raise exc.SubnetInUse(subnet_id=subnet_id)
|
||||
|
||||
@db_api.retry_if_session_inactive()
|
||||
def _remove_subnet_from_port(self, context, sub_id, port_id, auto_subnet):
|
||||
try:
|
||||
fixed = [f for f in self.get_port(context, port_id)['fixed_ips']
|
||||
if f['subnet_id'] != sub_id]
|
||||
if auto_subnet:
|
||||
# special flag to avoid re-allocation on auto subnets
|
||||
fixed.append({'subnet_id': sub_id, 'delete_subnet': True})
|
||||
data = {attributes.PORT: {'fixed_ips': fixed}}
|
||||
self.update_port(context, port_id, data)
|
||||
except exc.PortNotFound:
|
||||
# port is gone
|
||||
return
|
||||
except exc.SubnetNotFound as e:
|
||||
# another subnet in the fixed ips was concurrently removed. retry
|
||||
raise os_db_exc.RetryRequest(e)
|
||||
|
||||
def _ensure_no_user_ports_on_subnet(self, context, id):
|
||||
alloc = self._subnet_get_user_allocation(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)
|
||||
|
||||
@db_api.retry_if_session_inactive()
|
||||
def _remove_subnet_ip_allocations_from_ports(self, context, id):
|
||||
# Do not allow a subnet to be deleted if a router is attached to it
|
||||
self._subnet_check_ip_allocations_internal_router_ports(
|
||||
context, id)
|
||||
subnet = self._get_subnet(context, id)
|
||||
is_auto_addr_subnet = ipv6_utils.is_auto_address_subnet(subnet)
|
||||
if not is_auto_addr_subnet:
|
||||
# we only automatically remove IP addresses from user ports if
|
||||
# the IPs come from auto allocation subnets.
|
||||
self._ensure_no_user_ports_on_subnet(context, id)
|
||||
net_allocs = (context.session.query(models_v2.IPAllocation.port_id).
|
||||
filter_by(subnet_id=id))
|
||||
port_ids_on_net = [ipal.port_id for ipal in net_allocs]
|
||||
for port_id in port_ids_on_net:
|
||||
self._remove_subnet_from_port(context, id, port_id,
|
||||
auto_subnet=is_auto_addr_subnet)
|
||||
|
||||
@db_api.retry_if_session_inactive()
|
||||
def delete_subnet(self, context, id):
|
||||
with context.session.begin(subtransactions=True):
|
||||
subnet = self._get_subnet(context, id)
|
||||
|
||||
# Make sure the subnet isn't used by other resources
|
||||
_check_subnet_not_used(context, id)
|
||||
|
||||
# Delete all network owned ports
|
||||
qry_network_ports = (
|
||||
context.session.query(models_v2.IPAllocation).
|
||||
filter_by(subnet_id=subnet['id']).
|
||||
join(models_v2.Port))
|
||||
# Remove network owned ports, and delete IP allocations
|
||||
# for IPv6 addresses which were automatically generated
|
||||
# via SLAAC
|
||||
is_auto_addr_subnet = ipv6_utils.is_auto_address_subnet(subnet)
|
||||
if is_auto_addr_subnet:
|
||||
self._subnet_check_ip_allocations_internal_router_ports(
|
||||
context, id)
|
||||
else:
|
||||
qry_network_ports = (
|
||||
qry_network_ports.filter(models_v2.Port.device_owner.
|
||||
in_(AUTO_DELETE_PORT_OWNERS)))
|
||||
network_ports = qry_network_ports.all()
|
||||
if network_ports:
|
||||
for port in network_ports:
|
||||
context.session.delete(port)
|
||||
# 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:
|
||||
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)
|
||||
|
||||
context.session.delete(subnet)
|
||||
LOG.debug("Deleting subnet %s", id)
|
||||
# Make sure the subnet isn't used by other resources
|
||||
_check_subnet_not_used(context, id)
|
||||
self._remove_subnet_ip_allocations_from_ports(context, id)
|
||||
# retry integrity errors to catch ip allocation races
|
||||
with db_api.exc_to_retry(sql_exc.IntegrityError), \
|
||||
context.session.begin(subtransactions=True):
|
||||
subnet_db = self._get_subnet(context, id)
|
||||
subnet = self._make_subnet_dict(subnet_db, context=context)
|
||||
registry.notify(resources.SUBNET, events.PRECOMMIT_DELETE,
|
||||
self, context=context, subnet_id=id)
|
||||
context.session.delete(subnet_db)
|
||||
# Delete related ipam subnet manually,
|
||||
# since there is no FK relationship
|
||||
self.ipam.delete_subnet(context, id)
|
||||
registry.notify(resources.SUBNET, events.AFTER_DELETE,
|
||||
self, context=context, subnet=subnet)
|
||||
|
||||
@db_api.retry_if_session_inactive()
|
||||
def get_subnet(self, context, id, fields=None):
|
||||
|
|
|
|||
|
|
@ -41,7 +41,6 @@ from neutron.callbacks import exceptions
|
|||
from neutron.callbacks import registry
|
||||
from neutron.callbacks import resources
|
||||
from neutron.common import constants as n_const
|
||||
from neutron.common import ipv6_utils
|
||||
from neutron.common import rpc as n_rpc
|
||||
from neutron.common import topics
|
||||
from neutron.common import utils
|
||||
|
|
@ -171,6 +170,10 @@ class Ml2Plugin(db_base_plugin_v2.NeutronDbPluginV2,
|
|||
events.AFTER_CREATE)
|
||||
registry.subscribe(self._handle_segment_change, resources.SEGMENT,
|
||||
events.AFTER_DELETE)
|
||||
registry.subscribe(self._subnet_delete_precommit_handler,
|
||||
resources.SUBNET, events.PRECOMMIT_DELETE)
|
||||
registry.subscribe(self._subnet_delete_after_delete_handler,
|
||||
resources.SUBNET, events.AFTER_DELETE)
|
||||
self._setup_dhcp()
|
||||
self._start_rpc_notifiers()
|
||||
self.add_agent_status_check_worker(self.agent_health_check)
|
||||
|
|
@ -1046,142 +1049,28 @@ class Ml2Plugin(db_base_plugin_v2.NeutronDbPluginV2,
|
|||
return updated_subnet
|
||||
|
||||
@utils.transaction_guard
|
||||
@db_api.retry_if_session_inactive()
|
||||
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
|
||||
# from ports in the DB without invoking the derived class's
|
||||
# update_port(), preventing mechanism drivers from being called.
|
||||
# This approach should be revisited when the API layer is reworked
|
||||
# during icehouse.
|
||||
# the only purpose of this override is to protect this from being
|
||||
# called inside of a transaction.
|
||||
return super(Ml2Plugin, self).delete_subnet(context, id)
|
||||
|
||||
LOG.debug("Deleting subnet %s", id)
|
||||
session = context.session
|
||||
deallocated = set()
|
||||
while True:
|
||||
# NOTE(kevinbenton): this loop keeps db objects in scope
|
||||
# so we must expire them or risk stale reads.
|
||||
# see bug/1623990
|
||||
session.expire_all()
|
||||
with session.begin(subtransactions=True):
|
||||
record = self._get_subnet(context, id)
|
||||
subnet = self._make_subnet_dict(record, None, context=context)
|
||||
qry_allocated = (session.query(models_v2.IPAllocation).
|
||||
filter_by(subnet_id=id).
|
||||
join(models_v2.Port))
|
||||
is_auto_addr_subnet = ipv6_utils.is_auto_address_subnet(subnet)
|
||||
# Remove network owned ports, and delete IP allocations
|
||||
# for IPv6 addresses which were automatically generated
|
||||
# via SLAAC
|
||||
if is_auto_addr_subnet:
|
||||
self._subnet_check_ip_allocations_internal_router_ports(
|
||||
context, id)
|
||||
else:
|
||||
qry_allocated = (
|
||||
qry_allocated.filter(models_v2.Port.device_owner.
|
||||
in_(db_base_plugin_v2.AUTO_DELETE_PORT_OWNERS)))
|
||||
allocated = set(qry_allocated.all())
|
||||
LOG.debug("Ports to auto-deallocate: %s", allocated)
|
||||
if not is_auto_addr_subnet:
|
||||
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)
|
||||
def _subnet_delete_precommit_handler(self, rtype, event, trigger,
|
||||
context, subnet_id, **kwargs):
|
||||
record = self._get_subnet(context, subnet_id)
|
||||
subnet = self._make_subnet_dict(record, context=context)
|
||||
network = self.get_network(context, subnet['network_id'])
|
||||
mech_context = driver_context.SubnetContext(self, context,
|
||||
subnet, network)
|
||||
# TODO(kevinbenton): move this mech context into something like
|
||||
# a 'delete context' so it's not polluting the real context object
|
||||
setattr(context, '_mech_context', mech_context)
|
||||
self.mechanism_manager.delete_subnet_precommit(mech_context)
|
||||
|
||||
db_base_plugin_v2._check_subnet_not_used(context, id)
|
||||
|
||||
# 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,
|
||||
network)
|
||||
self.mechanism_manager.delete_subnet_precommit(
|
||||
mech_context)
|
||||
|
||||
LOG.debug("Deleting subnet record")
|
||||
session.delete(record)
|
||||
|
||||
# The super(Ml2Plugin, self).delete_subnet() is not called,
|
||||
# so need to manually call delete_subnet for pluggable ipam
|
||||
self.ipam.delete_subnet(context, id)
|
||||
|
||||
LOG.debug("Committing transaction")
|
||||
break
|
||||
|
||||
for a in to_deallocate:
|
||||
if a.port:
|
||||
# calling update_port() for each allocation to remove the
|
||||
# IP from the port and call the MechanismDrivers
|
||||
fixed_ips = [{'subnet_id': ip.subnet_id,
|
||||
'ip_address': ip.ip_address}
|
||||
for ip in a.port.fixed_ips
|
||||
if ip.subnet_id != id]
|
||||
# By default auto-addressed ips are not removed from port
|
||||
# on port update, so mark subnet with 'delete_subnet' flag
|
||||
# to force ip deallocation on port update.
|
||||
if is_auto_addr_subnet:
|
||||
fixed_ips.append({'subnet_id': id,
|
||||
'delete_subnet': True})
|
||||
data = {attributes.PORT: {'fixed_ips': fixed_ips}}
|
||||
try:
|
||||
# NOTE Don't inline port_id; needed for PortNotFound.
|
||||
port_id = a.port_id
|
||||
self.update_port(context, port_id, data)
|
||||
except exc.PortNotFound:
|
||||
# NOTE Attempting to access a.port_id here is an error.
|
||||
LOG.debug("Port %s deleted concurrently", port_id)
|
||||
except exc.SubnetNotFound:
|
||||
# NOTE we hit here if another subnet was concurrently
|
||||
# removed that the port has a fixed_ip on. we just
|
||||
# continue so the loop re-iterates and the IPs are
|
||||
# looked up again
|
||||
continue
|
||||
except Exception as e:
|
||||
with excutils.save_and_reraise_exception():
|
||||
utils.attach_exc_details(
|
||||
e, _LE("Exception deleting fixed_ip from "
|
||||
"port %s"), port_id)
|
||||
deallocated.add(a)
|
||||
|
||||
kwargs = {'context': context, 'subnet': subnet}
|
||||
registry.notify(resources.SUBNET, events.AFTER_DELETE, self, **kwargs)
|
||||
def _subnet_delete_after_delete_handler(self, rtype, event, trigger,
|
||||
context, subnet, **kwargs):
|
||||
try:
|
||||
self.mechanism_manager.delete_subnet_postcommit(mech_context)
|
||||
self.mechanism_manager.delete_subnet_postcommit(
|
||||
context._mech_context)
|
||||
except ml2_exc.MechanismDriverError:
|
||||
# TODO(apech) - One or more mechanism driver failed to
|
||||
# delete the subnet. Ideally we'd notify the caller of
|
||||
|
|
|
|||
|
|
@ -586,7 +586,7 @@ class TestMl2SubnetsV2(test_plugin.TestSubnetsV2,
|
|||
subnet_id = subnet['subnet']['id']
|
||||
attempt = [0]
|
||||
|
||||
def check_and_create_ports(context, subnet_id):
|
||||
def create_dhcp_port(*args, **kwargs):
|
||||
"""A method to emulate race condition.
|
||||
|
||||
Adds dhcp port in the middle of subnet delete
|
||||
|
|
@ -605,26 +605,17 @@ class TestMl2SubnetsV2(test_plugin.TestSubnetsV2,
|
|||
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)
|
||||
# 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)
|
||||
registry.subscribe(create_dhcp_port, resources.SUBNET,
|
||||
events.PRECOMMIT_DELETE)
|
||||
req = self.new_delete_request('subnets', subnet_id)
|
||||
res = req.get_response(self.api)
|
||||
self.assertEqual(204, res.status_int)
|
||||
self.assertEqual(1, attempt[0])
|
||||
|
||||
|
||||
class TestMl2DbOperationBounds(test_plugin.DbOperationBoundMixin,
|
||||
|
|
|
|||
Loading…
Reference in New Issue