openstack
/
neutron
Code Issues Proposed changes

Implement extension: extraroute-atomic 

I avoided changing the pre-existing extraroute implementation to
also avoid any breakage of legacy behavior. Alternatively we can
refactor the extraroute code to better fit both APIs:
  * the old: updates to the 'routes' attribute and
  * the new: actions 'add_extraroutes' / 'remove_extraroutes'.

Change-Id: Ia78e0e02de4a42a056e3ff654d8b6976a4e52616
Closes-Bug: #1826396 (rfe)
Depends-On: https://review.opendev.org/679259
Related-Change: https://review.opendev.org/655680 (spec)
This commit is contained in:
Bence Romsics 2019-07-15 15:18:46 +02:00
parent 2cb24490c0
commit ab07b91b1a
3 changed files with 166 additions and 0 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

82
neutron/db/extraroute_db.py
View File

@ -151,6 +151,88 @@ class ExtraRoute_dbonly_mixin(l3_db.L3_NAT_dbonly_mixin):
raise xroute_exc.RouterInterfaceInUseByRoute(
router_id=router_id, subnet_id=subnet_id)
@staticmethod
def _add_extra_routes(old, add):
"""Add two lists of extra routes.
Exact duplicates (both destination and nexthop) in old and add are
merged into one item.
Same destinations with different nexthops are accepted and all of
them are returned.
Overlapping destinations are accepted and all of them are returned.
"""
routes_dict = {} # its values are sets of nexthops
for r in old + add:
dst = r['destination']
nexthop = r['nexthop']
if dst not in routes_dict:
routes_dict[dst] = set()
routes_dict[dst].add(nexthop)
routes_list = []
for dst, nexthops in routes_dict.items():
for nexthop in nexthops:
routes_list.append({'destination': dst, 'nexthop': nexthop})
return routes_list
@staticmethod
def _remove_extra_routes(old, remove):
"""Remove the 2nd list of extra routes from the first.
Since we care about the end state if an extra route to be removed
is already missing from old, that's not an error, but accepted.
"""
routes_dict = {} # its values are sets of nexthops
for r in old:
dst = r['destination']
nexthop = r['nexthop']
if dst not in routes_dict:
routes_dict[dst] = set()
routes_dict[dst].add(nexthop)
for r in remove:
dst = r['destination']
nexthop = r['nexthop']
if dst in routes_dict:
routes_dict[dst].discard(nexthop)
routes_list = []
for dst, nexthops in routes_dict.items():
for nexthop in nexthops:
routes_list.append({'destination': dst, 'nexthop': nexthop})
return routes_list
@db_api.retry_if_session_inactive()
def add_extraroutes(self, context, router_id, body=None):
# NOTE(bence romsics): The input validation is delayed until
# update_router() validates the whole set of routes. Until then
# do not trust 'routes'.
routes = body['router']['routes']
with db_api.CONTEXT_WRITER.using(context):
old_routes = self._get_extra_routes_by_router_id(
context, router_id)
router = self.update_router(
context,
router_id,
{'router':
{'routes':
self._add_extra_routes(old_routes, routes)}})
return {'router': router}
@db_api.retry_if_session_inactive()
def remove_extraroutes(self, context, router_id, body=None):
# NOTE(bence romsics): The input validation is delayed until
# update_router() validates the whole set of routes. Until then
# do not trust 'routes'.
routes = body['router']['routes']
with db_api.CONTEXT_WRITER.using(context):
old_routes = self._get_extra_routes_by_router_id(
context, router_id)
router = self.update_router(
context,
router_id,
{'router':
{'routes':
self._remove_extra_routes(old_routes, routes)}})
return {'router': router}
class ExtraRoute_db_mixin(ExtraRoute_dbonly_mixin, l3_db.L3_NAT_db_mixin):
"""Mixin class to support extra route configuration on router with rpc."""

2
neutron/services/l3_router/l3_router_plugin.py
View File

@ -16,6 +16,7 @@
from neutron_lib.agent import topics
from neutron_lib.api.definitions import dvr
from neutron_lib.api.definitions import extraroute
from neutron_lib.api.definitions import extraroute_atomic
from neutron_lib.api.definitions import fip_port_details
from neutron_lib.api.definitions import floatingip_pools
from neutron_lib.api.definitions import l3 as l3_apidef
@ -94,6 +95,7 @@ class L3RouterPlugin(service_base.ServicePluginBase,
_supported_extension_aliases = [dvr.ALIAS, l3_apidef.ALIAS,
l3_ext_gw_mode.ALIAS,
extraroute.ALIAS,
extraroute_atomic.ALIAS,
n_const.L3_AGENT_SCHEDULER_EXT_ALIAS,
l3_ext_ha_mode.ALIAS,
router_availability_zone.ALIAS,

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

@ -73,3 +73,85 @@ class TestExtraRouteDb(testlib_api.SqlTestCase):
self.assertItemsEqual(updated_router['routes'], routes)
got_router = self._plugin.get_router(ctx, router_id)
self.assertItemsEqual(got_router['routes'], routes)
def assertEqualRoutes(self, a, b):
"""Compare a list of routes without caring for the list order."""
return self.assertSetEqual(
set(frozenset(r.items()) for r in a),
set(frozenset(r.items()) for r in b))
def test_add_extra_routes(self):
self.assertEqual(
[],
self._plugin._add_extra_routes([], []),
)
old = [{"destination": "10.0.10.0/24", "nexthop": "10.0.0.10"}]
add = []
self.assertEqual(old, self._plugin._add_extra_routes(old, add))
old = []
add = [{"destination": "10.0.10.0/24", "nexthop": "10.0.0.10"}]
self.assertEqual(add, self._plugin._add_extra_routes(old, add))
old = [{"destination": "10.0.10.0/24", "nexthop": "10.0.0.10"}]
add = [{"destination": "10.0.10.0/24", "nexthop": "10.0.0.10"}]
self.assertEqual(old, self._plugin._add_extra_routes(old, add))
old = [{"destination": "10.0.10.0/24", "nexthop": "10.0.0.10"}]
add = [{"destination": "10.0.10.0/24", "nexthop": "10.0.0.11"}]
self.assertEqualRoutes(
old + add, self._plugin._add_extra_routes(old, add))
def test_remove_extra_routes(self):
old = []
remove = []
self.assertEqual(old, self._plugin._remove_extra_routes(old, remove))
old = [{"destination": "10.0.10.0/24", "nexthop": "10.0.0.10"}]
remove = []
self.assertEqual(old, self._plugin._remove_extra_routes(old, remove))
old = []
remove = [{"destination": "10.0.10.0/24", "nexthop": "10.0.0.10"}]
self.assertEqual(old, self._plugin._remove_extra_routes(old, remove))
old = [{"destination": "10.0.10.0/24", "nexthop": "10.0.0.10"}]
remove = [{"destination": "10.0.10.0/24", "nexthop": "10.0.0.11"}]
self.assertEqual(old, self._plugin._remove_extra_routes(old, remove))
old = [{"destination": "10.0.10.0/24", "nexthop": "10.0.0.10"}]
remove = old
self.assertEqual([], self._plugin._remove_extra_routes(old, remove))
old = [
{"destination": "10.0.10.0/24", "nexthop": "10.0.0.10"},
{"destination": "10.0.11.0/24", "nexthop": "10.0.0.11"},
]
remove = old[1:]
self.assertEqual(
old[:1], self._plugin._remove_extra_routes(old, remove))
old = [
{"destination": "10.0.10.0/24", "nexthop": "10.0.0.10"},
{"destination": "10.0.10.0/24", "nexthop": "10.0.0.11"},
]
remove = old[1:]
self.assertEqual(
old[:1], self._plugin._remove_extra_routes(old, remove))
old = []
remove = [
{"destination": "10.0.10.0/24", "nexthop": "10.0.0.10"},
{"destination": "10.0.10.0/24", "nexthop": "10.0.0.10"},
]
self.assertEqual([], self._plugin._remove_extra_routes(old, remove))
old = [
{"destination": "10.0.10.0/24", "nexthop": "10.0.0.10"},
]
remove = [
{"destination": "10.0.10.0/24", "nexthop": "10.0.0.10"},
{"destination": "10.0.10.0/24", "nexthop": "10.0.0.10"},
]
self.assertEqual([], self._plugin._remove_extra_routes(old, remove))