openstack
/
neutron
Code Issues Proposed changes

Dont raise RouterInterfaceNotFound on overlap check router ports 

A corner case of the fix done in [1] could happend if, as a race
scenario, parallel requests evaluate other ports that could be deleted
during the process if they had already determined a overlapping, in that
case a RouterInterfaceNotFound exception was raised and the request
finished with that exception and a 404 status code.

This patch removes the exception due to a port not found, because if the
port is not found, the related subnet should not participate in the
overlap evaluation, so it makes no sense to break the process for a port
that no longer exists. It also improves the previous validation to
perform the overlapping check, being performed only when we have
at least more than one subnet, as the overlapping check with only one
subnet did not make sense.

Closes-Bug: #1998226

[1] https://review.opendev.org/c/openstack/neutron/+/859143

Change-Id: If4afe6f525c46f9cf7f02d8aae27dfc56144fd62
(cherry picked from commit 92efd8e45b)
This commit is contained in:
Fernando Royo 2022-12-01 11:47:58 +01:00
parent 25dc4c0166
commit 7672b0e76a
3 changed files with 50 additions and 23 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

11
neutron/db/l3_db.py
View File

@ -1025,10 +1025,15 @@ class L3_NAT_dbonly_mixin(l3.RouterPluginBase,
subnets_id.extend([fixed_ip['subnet_id']
for fixed_ip in port['fixed_ips']])
else:
raise l3_exc.RouterInterfaceNotFound(
router_id=router.id, port_id=rp.port_id)
# due to race conditions maybe the port under analysis is
# deleted, so instead returning a RouterInterfaceNotFound
# we continue the analysis avoiding that port
LOG.debug("Port %s could not be found, it might have been "
"deleted concurrently. Will not be checked for "
"an overlapping router interface.",
rp.port_id)
if subnets_id:
if len(subnets_id) > 1:
id_filter = {'id': subnets_id}
subnets = self._core_plugin.get_subnets(context.elevated(),
filters=id_filter)

12
neutron/tests/base.py
View File

@ -492,7 +492,11 @@ class BaseTestCase(DietTestCase):
root_helper_daemon=get_rootwrap_daemon_cmd())
def _simulate_concurrent_requests_process_and_raise(self, calls, args):
self._simulate_concurrent_requests_process(calls, args,
raise_on_exception=True)
def _simulate_concurrent_requests_process(self, calls, args,
raise_on_exception=False):
class SimpleThread(threading.Thread):
def __init__(self, q):
super(SimpleThread, self).__init__()
@ -529,10 +533,16 @@ class BaseTestCase(DietTestCase):
t.start()
q.join()
threads_exceptions = []
for t in threads:
e = t.get_exception()
if e:
raise e
if raise_on_exception:
raise e
else:
threads_exceptions.append(e)
return threads_exceptions
class PluginFixture(fixtures.Fixture):

50
neutron/tests/fullstack/test_l3_agent.py
View File

@ -270,25 +270,37 @@ class TestL3Agent(base.BaseFullStackTestCase):
def _test_concurrent_router_subnet_attachment_overlapping_cidr(self,
ha=False):
tenant_id = uuidutils.generate_uuid()
subnet_cidr = '10.100.0.0/24'
network1 = self.safe_client.create_network(
tenant_id, name='foo-network1')
subnet1 = self.safe_client.create_subnet(
tenant_id, network1['id'], subnet_cidr)
network2 = self.safe_client.create_network(
tenant_id, name='foo-network2')
subnet2 = self.safe_client.create_subnet(
tenant_id, network2['id'], subnet_cidr)
subnet_cidr = '10.200.0.0/24'
# to have many port interactions where race conditions would happen
# deleting ports meanwhile find operations to evaluate the overlapping
subnets = 10
funcs = []
args = []
router = self.safe_client.create_router(tenant_id, ha=ha)
funcs = [self.safe_client.add_router_interface,
self.safe_client.add_router_interface]
args = [(router['id'], subnet1['id']), (router['id'], subnet2['id'])]
self.assertRaises(
exceptions.BadRequest,
self._simulate_concurrent_requests_process_and_raise,
funcs,
args)
for i in range(subnets):
network_tmp = self.safe_client.create_network(
tenant_id, name='foo-network' + str(i))
subnet_tmp = self.safe_client.create_subnet(
tenant_id, network_tmp['id'], subnet_cidr)
funcs.append(self.safe_client.add_router_interface)
args.append((router['id'], subnet_tmp['id']))
exception_requests = self._simulate_concurrent_requests_process(
funcs, args)
if not all(type(e) == exceptions.BadRequest
for e in exception_requests):
self.fail('Unexpected exception adding interfaces to router from '
'different subnets overlapping')
if not len(exception_requests) >= (subnets - 1):
self.fail('If we have tried to associate %s subnets overlapping '
'cidr to the router, we should have received at least '
'%s or %s rejected requests, but we have only received '
'%s', (str(subnets), str(subnets - 1), str(subnets),
str(len(exception_requests))))
class TestLegacyL3Agent(TestL3Agent):
@ -441,7 +453,7 @@ class TestLegacyL3Agent(TestL3Agent):
def test_router_fip_qos_after_admin_state_down_up(self):
self._router_fip_qos_after_admin_state_down_up()
def test_concurrent_router_subnet_attachment_overlapping_cidr_(self):
def test_concurrent_router_subnet_attachment_overlapping_cidr(self):
self._test_concurrent_router_subnet_attachment_overlapping_cidr()
@ -601,6 +613,6 @@ class TestHAL3Agent(TestL3Agent):
def test_router_fip_qos_after_admin_state_down_up(self):
self._router_fip_qos_after_admin_state_down_up(ha=True)
def test_concurrent_router_subnet_attachment_overlapping_cidr_(self):
def test_concurrent_router_subnet_attachment_overlapping_cidr(self):
self._test_concurrent_router_subnet_attachment_overlapping_cidr(
ha=True)