Fix DetachedInstanceError on subnet delete
While trying to delete subnets, some failed with DetachedInstanceError,
because 'port' is not part of 'IPAllocation' object when accessed
outside the DB session(in which 'IPAllocation' object was created).
To avoid this error, we call get_port explicitly before using the port.
This issue is not seen on master branch as recent code refactor[1] is
also calling get_port there.
[1] https://review.openstack.org/#/c/428774/
Closes-bug: #1672701
Change-Id: I924fa7e36ea9e45bf0ef3480972341a851bda86c
(cherry picked from commit b9242c348c)
This commit is contained in:
venkata anil
Ihar Hrachyshka
parent
d662c101fb
commit
20b9b89343
|
|
@ -1059,11 +1059,11 @@ class Ml2Plugin(db_base_plugin_v2.NeutronDbPluginV2,
|
|||
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):
|
||||
# NOTE(kevinbenton): this loop keeps db objects in scope
|
||||
# so we must expire them or risk stale reads.
|
||||
# see bug/1623990
|
||||
session.expire_all()
|
||||
record = self._get_subnet(context, id)
|
||||
subnet = self._make_subnet_dict(record, None, context=context)
|
||||
qry_allocated = (session.query(models_v2.IPAllocation).
|
||||
|
|
@ -1080,7 +1080,7 @@ 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 = set(qry_allocated.all())
|
||||
allocated = set(ipal.port_id for ipal in qry_allocated)
|
||||
LOG.debug("Ports to auto-deallocate: %s", allocated)
|
||||
if not is_auto_addr_subnet:
|
||||
user_alloc = self._subnet_get_user_allocation(
|
||||
|
|
@ -1143,14 +1143,20 @@ class Ml2Plugin(db_base_plugin_v2.NeutronDbPluginV2,
|
|||
LOG.debug("Committing transaction")
|
||||
break
|
||||
|
||||
for a in to_deallocate:
|
||||
if a.port:
|
||||
for port_id in to_deallocate:
|
||||
try:
|
||||
port = self.get_port(context, port_id)
|
||||
except exc.PortNotFound:
|
||||
LOG.debug("Port %s deleted concurrently", port_id)
|
||||
deallocated.add(port_id)
|
||||
continue
|
||||
if 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]
|
||||
fixed_ips = [{'subnet_id': ip['subnet_id'],
|
||||
'ip_address': ip['ip_address']}
|
||||
for ip in 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.
|
||||
|
|
@ -1159,11 +1165,8 @@ class Ml2Plugin(db_base_plugin_v2.NeutronDbPluginV2,
|
|||
'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
|
||||
|
|
@ -1176,7 +1179,7 @@ class Ml2Plugin(db_base_plugin_v2.NeutronDbPluginV2,
|
|||
utils.attach_exc_details(
|
||||
e, _LE("Exception deleting fixed_ip from "
|
||||
"port %s"), port_id)
|
||||
deallocated.add(a)
|
||||
deallocated.add(port_id)
|
||||
|
||||
kwargs = {'context': context, 'subnet': subnet}
|
||||
registry.notify(resources.SUBNET, events.AFTER_DELETE, self, **kwargs)
|
||||
|
|
|
|||
|
|
@ -522,6 +522,32 @@ class TestMl2SubnetsV2(test_plugin.TestSubnetsV2,
|
|||
kwargs = after_create.mock_calls[0][2]
|
||||
self.assertEqual(s['subnet']['id'], kwargs['subnet']['id'])
|
||||
|
||||
def test_subnet_delete_no_DetachedInstanceError_for_port_update(self):
|
||||
mock_check_subnet_not_used = mock.patch.object(
|
||||
base_plugin, '_check_subnet_not_used').start()
|
||||
with self.network() as n:
|
||||
with self.subnet(network=n, cidr='1.1.1.0/24') as s1,\
|
||||
self.subnet(network=n, cidr='1.1.2.0/24') as s2:
|
||||
fixed_ips = [{'subnet_id': s1['subnet']['id']},
|
||||
{'subnet_id': s2['subnet']['id']}]
|
||||
with self.port(subnet=s1, fixed_ips=fixed_ips,
|
||||
device_owner=constants.DEVICE_OWNER_DHCP) as p:
|
||||
|
||||
def subnet_not_used(ctx, subnet_id):
|
||||
# Make the session invalid after the query.
|
||||
ctx.session.expunge_all()
|
||||
mock_check_subnet_not_used.side_effect = None
|
||||
mock_check_subnet_not_used.side_effect = subnet_not_used
|
||||
req = self.new_delete_request('subnets',
|
||||
s1['subnet']['id'])
|
||||
res = req.get_response(self.api)
|
||||
self.assertEqual(204, res.status_int)
|
||||
# ensure port only has 1 IP on s2
|
||||
port = self._show('ports', p['port']['id'])['port']
|
||||
self.assertEqual(1, len(port['fixed_ips']))
|
||||
self.assertEqual(s2['subnet']['id'],
|
||||
port['fixed_ips'][0]['subnet_id'])
|
||||
|
||||
def test_port_update_subnetnotfound(self):
|
||||
with self.network() as n:
|
||||
with self.subnet(network=n, cidr='1.1.1.0/24') as s1,\
|
||||
|
|
|
|||
Loading…
Reference in New Issue