diff --git a/neutron/plugins/ml2/plugin.py b/neutron/plugins/ml2/plugin.py index bc1a1beb81a..887a88a9dbb 100644 --- a/neutron/plugins/ml2/plugin.py +++ b/neutron/plugins/ml2/plugin.py @@ -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(ipal.port_id for ipal in qry_allocated) + 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( @@ -1143,20 +1143,14 @@ class Ml2Plugin(db_base_plugin_v2.NeutronDbPluginV2, LOG.debug("Committing transaction") break - 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: + 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 port['fixed_ips'] - if ip['subnet_id'] != id] + 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. @@ -1165,8 +1159,11 @@ 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 @@ -1179,7 +1176,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(port_id) + deallocated.add(a) kwargs = {'context': context, 'subnet': subnet} registry.notify(resources.SUBNET, events.AFTER_DELETE, self, **kwargs) diff --git a/neutron/tests/unit/plugins/ml2/test_plugin.py b/neutron/tests/unit/plugins/ml2/test_plugin.py index c1e6a4c2ba9..402e7c7349f 100644 --- a/neutron/tests/unit/plugins/ml2/test_plugin.py +++ b/neutron/tests/unit/plugins/ml2/test_plugin.py @@ -522,32 +522,6 @@ 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,\