openstack/neutron
Code Issues Proposed changes

Don't delete already deleted extra router routes

Browse Source 
When handling the deletion of extra routes we need to handle the case
that the route is already deleted by another call in the time we have
fetched the extra routes and try to delete it. This is a classic race
condition when two calls try to update the routes of a router at the
same time. The default MariaDB/MySQL transaction isolation level does
not suffice to prevent this scenario. Directly deleting the route
without fetching it solves this problem.

Change-Id: Ie8238310569eb7c1c53296195800bef5c9cb92a3
Closes-Bug: #2057698
This commit is contained in:
Sebastian Lohff 2024-03-12 19:34:17 +01:00
parent 2a196fefd4
commit 27b2f22df1
2 changed files with 48 additions and 2 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
neutron/db/extraroute_db.py
View File

@ -133,11 +133,11 @@ class ExtraRoute_dbonly_mixin(l3_db.L3_NAT_dbonly_mixin):
LOG.debug('Removed routes are %s', removed)
for route in removed:
l3_obj.RouterRoute.get_object(
l3_obj.RouterRoute.delete_objects(
context,
router_id=router['id'],
destination=route['destination'],
nexthop=route['nexthop']).delete()
nexthop=route['nexthop'])
return added, removed
@staticmethod

46
neutron/tests/unit/db/test_extraroute_db.py
View File

@ -22,6 +22,7 @@ from neutron_lib.plugins import constants
from neutron_lib.plugins import directory
from neutron.db import extraroute_db
from neutron.objects import router as l3_obj
from neutron.tests.unit import testlib_api
@ -156,3 +157,48 @@ class TestExtraRouteDb(testlib_api.SqlTestCase):
{"destination": "10.0.10.0/24", "nexthop": "10.0.0.10"},
]
self.assertEqual([], self._plugin._remove_extra_routes(old, remove))
def test_update_routes_where_route_vanishes_while_on_delete(self):
ctx = context.get_admin_context()
create_request = {
'router': {
'name': 'my router',
'tenant_id': 'my tenant',
'admin_state_up': True,
}
}
router = self._plugin.create_router(ctx, create_request)
self.assertCountEqual(router['routes'], [])
router_id = router['id']
routes = [
{'destination': '10.0.0.0/24', 'nexthop': '1.1.1.4'},
{'destination': '10.1.0.0/24', 'nexthop': '1.1.1.3'},
{'destination': '10.2.0.0/24', 'nexthop': '1.1.1.2'},
]
self._test_update_routes(ctx, router_id, router, routes)
routes = [
{'destination': '10.0.0.0/24', 'nexthop': '1.1.1.4'},
{'destination': '10.1.0.0/24', 'nexthop': '1.1.1.3'},
]
def _remove_last_route(orig_func):
def _wrapper(ctx, router_id):
routes = orig_func(ctx, router_id)
# forcefully delete route to 10.2.0.0/24
ctx2 = context.get_admin_context()
l3_obj.RouterRoute.get_object(
ctx2,
router_id=router_id,
destination="10.2.0.0/24",
nexthop="1.1.1.2").delete()
return routes
return _wrapper
with mock.patch.object(self._plugin, '_get_extra_routes_by_router_id',
wraps=_remove_last_route(
self._plugin._get_extra_routes_by_router_id)) \
as mock_get_routes:
self._test_update_routes(ctx, router_id, router, routes)
mock_get_routes.assert_called_once()