Merge "Get rid of delete_subnet method in ML2" into stable/newton
This commit is contained in:
commit
a85d94dc35
|
@ -28,6 +28,7 @@ from oslo_utils import excutils
|
||||||
from oslo_utils import uuidutils
|
from oslo_utils import uuidutils
|
||||||
from sqlalchemy import and_
|
from sqlalchemy import and_
|
||||||
from sqlalchemy import event
|
from sqlalchemy import event
|
||||||
|
from sqlalchemy import exc as sql_exc
|
||||||
from sqlalchemy import not_
|
from sqlalchemy import not_
|
||||||
|
|
||||||
from neutron._i18n import _, _LE, _LI
|
from neutron._i18n import _, _LE, _LI
|
||||||
|
@ -835,10 +836,6 @@ class NeutronDbPluginV2(db_base_plugin_common.DbBasePluginCommon,
|
||||||
|
|
||||||
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 _subnet_get_user_allocation(self, context, subnet_id):
|
def _subnet_get_user_allocation(self, context, subnet_id):
|
||||||
"""Check if there are any user ports on subnet and return first."""
|
"""Check if there are any user ports on subnet and return first."""
|
||||||
# need to join with ports table as IPAllocation's port
|
# 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)
|
"cannot delete", subnet_id)
|
||||||
raise exc.SubnetInUse(subnet_id=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()
|
@db_api.retry_if_session_inactive()
|
||||||
def delete_subnet(self, context, id):
|
def delete_subnet(self, context, id):
|
||||||
with context.session.begin(subtransactions=True):
|
LOG.debug("Deleting subnet %s", id)
|
||||||
subnet = self._get_subnet(context, id)
|
# Make sure the subnet isn't used by other resources
|
||||||
|
_check_subnet_not_used(context, id)
|
||||||
# Make sure the subnet isn't used by other resources
|
self._remove_subnet_ip_allocations_from_ports(context, id)
|
||||||
_check_subnet_not_used(context, id)
|
# retry integrity errors to catch ip allocation races
|
||||||
|
with db_api.exc_to_retry(sql_exc.IntegrityError), \
|
||||||
# Delete all network owned ports
|
context.session.begin(subtransactions=True):
|
||||||
qry_network_ports = (
|
subnet_db = self._get_subnet(context, id)
|
||||||
context.session.query(models_v2.IPAllocation).
|
subnet = self._make_subnet_dict(subnet_db, context=context)
|
||||||
filter_by(subnet_id=subnet['id']).
|
registry.notify(resources.SUBNET, events.PRECOMMIT_DELETE,
|
||||||
join(models_v2.Port))
|
self, context=context, subnet_id=id)
|
||||||
# Remove network owned ports, and delete IP allocations
|
context.session.delete(subnet_db)
|
||||||
# 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)
|
|
||||||
# Delete related ipam subnet manually,
|
# Delete related ipam subnet manually,
|
||||||
# since there is no FK relationship
|
# since there is no FK relationship
|
||||||
self.ipam.delete_subnet(context, id)
|
self.ipam.delete_subnet(context, id)
|
||||||
|
registry.notify(resources.SUBNET, events.AFTER_DELETE,
|
||||||
|
self, context=context, subnet=subnet)
|
||||||
|
|
||||||
@db_api.retry_if_session_inactive()
|
@db_api.retry_if_session_inactive()
|
||||||
def get_subnet(self, context, id, fields=None):
|
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 registry
|
||||||
from neutron.callbacks import resources
|
from neutron.callbacks import resources
|
||||||
from neutron.common import constants as n_const
|
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 rpc as n_rpc
|
||||||
from neutron.common import topics
|
from neutron.common import topics
|
||||||
from neutron.common import utils
|
from neutron.common import utils
|
||||||
|
@ -171,6 +170,10 @@ class Ml2Plugin(db_base_plugin_v2.NeutronDbPluginV2,
|
||||||
events.AFTER_CREATE)
|
events.AFTER_CREATE)
|
||||||
registry.subscribe(self._handle_segment_change, resources.SEGMENT,
|
registry.subscribe(self._handle_segment_change, resources.SEGMENT,
|
||||||
events.AFTER_DELETE)
|
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._setup_dhcp()
|
||||||
self._start_rpc_notifiers()
|
self._start_rpc_notifiers()
|
||||||
self.add_agent_status_check_worker(self.agent_health_check)
|
self.add_agent_status_check_worker(self.agent_health_check)
|
||||||
|
@ -1046,142 +1049,28 @@ class Ml2Plugin(db_base_plugin_v2.NeutronDbPluginV2,
|
||||||
return updated_subnet
|
return updated_subnet
|
||||||
|
|
||||||
@utils.transaction_guard
|
@utils.transaction_guard
|
||||||
@db_api.retry_if_session_inactive()
|
|
||||||
def delete_subnet(self, context, id):
|
def delete_subnet(self, context, id):
|
||||||
# REVISIT(rkukura) The super(Ml2Plugin, self).delete_subnet()
|
# the only purpose of this override is to protect this from being
|
||||||
# function is not used because it deallocates the subnet's addresses
|
# called inside of a transaction.
|
||||||
# from ports in the DB without invoking the derived class's
|
return super(Ml2Plugin, self).delete_subnet(context, id)
|
||||||
# update_port(), preventing mechanism drivers from being called.
|
|
||||||
# This approach should be revisited when the API layer is reworked
|
|
||||||
# during icehouse.
|
|
||||||
|
|
||||||
LOG.debug("Deleting subnet %s", id)
|
def _subnet_delete_precommit_handler(self, rtype, event, trigger,
|
||||||
session = context.session
|
context, subnet_id, **kwargs):
|
||||||
deallocated = set()
|
record = self._get_subnet(context, subnet_id)
|
||||||
while True:
|
subnet = self._make_subnet_dict(record, context=context)
|
||||||
# NOTE(kevinbenton): this loop keeps db objects in scope
|
network = self.get_network(context, subnet['network_id'])
|
||||||
# so we must expire them or risk stale reads.
|
mech_context = driver_context.SubnetContext(self, context,
|
||||||
# see bug/1623990
|
subnet, network)
|
||||||
session.expire_all()
|
# TODO(kevinbenton): move this mech context into something like
|
||||||
with session.begin(subtransactions=True):
|
# a 'delete context' so it's not polluting the real context object
|
||||||
record = self._get_subnet(context, id)
|
setattr(context, '_mech_context', mech_context)
|
||||||
subnet = self._make_subnet_dict(record, None, context=context)
|
self.mechanism_manager.delete_subnet_precommit(mech_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)
|
|
||||||
|
|
||||||
db_base_plugin_v2._check_subnet_not_used(context, id)
|
def _subnet_delete_after_delete_handler(self, rtype, event, trigger,
|
||||||
|
context, subnet, **kwargs):
|
||||||
# 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)
|
|
||||||
try:
|
try:
|
||||||
self.mechanism_manager.delete_subnet_postcommit(mech_context)
|
self.mechanism_manager.delete_subnet_postcommit(
|
||||||
|
context._mech_context)
|
||||||
except ml2_exc.MechanismDriverError:
|
except ml2_exc.MechanismDriverError:
|
||||||
# TODO(apech) - One or more mechanism driver failed to
|
# TODO(apech) - One or more mechanism driver failed to
|
||||||
# delete the subnet. Ideally we'd notify the caller of
|
# delete the subnet. Ideally we'd notify the caller of
|
||||||
|
|
|
@ -586,7 +586,7 @@ class TestMl2SubnetsV2(test_plugin.TestSubnetsV2,
|
||||||
subnet_id = subnet['subnet']['id']
|
subnet_id = subnet['subnet']['id']
|
||||||
attempt = [0]
|
attempt = [0]
|
||||||
|
|
||||||
def check_and_create_ports(context, subnet_id):
|
def create_dhcp_port(*args, **kwargs):
|
||||||
"""A method to emulate race condition.
|
"""A method to emulate race condition.
|
||||||
|
|
||||||
Adds dhcp port in the middle of subnet delete
|
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_req = self.new_create_request('ports', data)
|
||||||
port_res = port_req.get_response(self.api)
|
port_res = port_req.get_response(self.api)
|
||||||
self.assertEqual(201, port_res.status_int)
|
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
|
# we mock _subnet_check_ip_allocations with method
|
||||||
# that creates DHCP port 'in the middle' of subnet_delete
|
# that creates DHCP port 'in the middle' of subnet_delete
|
||||||
# causing retry this way subnet is deleted on the
|
# causing retry this way subnet is deleted on the
|
||||||
# second attempt
|
# second attempt
|
||||||
with mock.patch.object(plugin, '_subnet_check_ip_allocations',
|
registry.subscribe(create_dhcp_port, resources.SUBNET,
|
||||||
side_effect=check_and_create_ports):
|
events.PRECOMMIT_DELETE)
|
||||||
req = self.new_delete_request('subnets', subnet_id)
|
req = self.new_delete_request('subnets', subnet_id)
|
||||||
res = req.get_response(self.api)
|
res = req.get_response(self.api)
|
||||||
self.assertEqual(204, res.status_int)
|
self.assertEqual(204, res.status_int)
|
||||||
# Validate chat check is called twice, i.e. after the
|
self.assertEqual(1, attempt[0])
|
||||||
# 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,
|
class TestMl2DbOperationBounds(test_plugin.DbOperationBoundMixin,
|
||||||
|
|
Loading…
Reference in New Issue