[ML2/OVN] Add external_ids.neutron:is_static_route key for Static Routes

The Neutron owned Static Routes created in the OVN database are marked
with the external_ids key neutron:is_static_route. This mark will be used
by the OVN DB sync tool to distinguish the Neutron created Static Routes
from those added externally; the second ones should not be altered during
the sync process. The static route update task runs only once during the
maintenance task, and if all entries are already configured, no action is
performed.

Related-bug: #2027742
Change-Id: I4226024cb4cfd85bdf4f717d42d48150cab22442
This commit is contained in:
Roberto Bartzen Acosta
2024-02-01 16:20:04 -03:00
parent b847d89ac1
commit 89835e43e8
14 changed files with 299 additions and 17 deletions

View File

@@ -56,6 +56,7 @@ OVN_LIVENESS_CHECK_EXT_ID_KEY = 'neutron:liveness_check_at'
METADATA_LIVENESS_CHECK_EXT_ID_KEY = 'neutron:metadata_liveness_check_at'
OVN_PORT_BINDING_PROFILE = portbindings.PROFILE
OVN_HOST_ID_EXT_ID_KEY = 'neutron:host_id'
OVN_LRSR_EXT_ID_KEY = 'neutron:is_static_route'
MIGRATING_ATTR = 'migrating_to'
OVN_ROUTER_PORT_OPTION_KEYS = ['router-port', 'nat-addresses',

View File

@@ -274,6 +274,18 @@ class API(api.API, metaclass=abc.ABCMeta):
:returns: :class:`Command` with no result
"""
@abc.abstractmethod
def set_static_route(self, sroute, **columns):
"""Update static route columns in a logical router.
:param sroute: The unique identifier of the LRSR
:type sroute: string
:param columns: Dictionary of static columns
Supported columns: prefix, nexthop, external_ids
:type columns: dictionary
:returns: :class:`Command` with no result
"""
@abc.abstractmethod
def get_all_chassis_gateway_bindings(self,
chassis_candidate_list=None):

View File

@@ -704,6 +704,23 @@ class DelStaticRoutesCommand(command.BaseCommand):
route.delete()
class SetStaticRouteCommand(command.BaseCommand):
def __init__(self, api, sroute, **columns):
super().__init__(api)
self.sroute = sroute
self.columns = columns
def run_idl(self, txn):
try:
for col, val in self.columns.items():
setattr(self.sroute, col, val)
except idlutils.RowNotFound:
msg = (_('Logical Router Static Route %s does not exist')
% self.sroute)
raise RuntimeError(msg)
class UpdateObjectExtIdsCommand(command.BaseCommand):
table = None
field = 'name'

View File

@@ -369,6 +369,24 @@ class OvsdbNbOvnIdl(nb_impl_idl.OvnNbApiIdlImpl, Backend):
'dnat_and_snats': dnat_and_snats})
return result
def get_all_logical_routers_static_routes(self):
"""Get static routes associated with all logical Routers
@return: list of dict, each dict has key-value:
- 'name': string router_id in neutron.
- 'static_routes': list of static routes rows.
"""
result = []
for lrouter in self._tables['Logical_Router'].rows.values():
if (ovn_const.OVN_ROUTER_NAME_EXT_ID_KEY not in
lrouter.external_ids):
continue
result.append({'name': lrouter.name.replace('neutron-', ''),
'static_routes': getattr(lrouter, 'static_routes',
[])})
return result
def get_acl_by_id(self, acl_id):
try:
return self.lookup('ACL', uuid.UUID(acl_id))
@@ -477,6 +495,9 @@ class OvsdbNbOvnIdl(nb_impl_idl.OvnNbApiIdlImpl, Backend):
def delete_static_routes(self, lrouter, routes, if_exists=True):
return cmd.DelStaticRoutesCommand(self, lrouter, routes, if_exists)
def set_static_route(self, sroute, **columns):
return cmd.SetStaticRouteCommand(self, sroute, **columns)
def _get_logical_router_port_gateway_chassis(self, lrp, priorities=None):
"""Get the list of chassis hosting this gateway port.

View File

@@ -1282,6 +1282,49 @@ class DBInconsistenciesPeriodics(SchemaAwarePeriodicsBase):
txn.add(cmd)
raise periodics.NeverAgain()
# TODO(racosta): Remove this method in the E+2 cycle (SLURP release)
@has_lock_periodic(spacing=600, run_immediately=True)
def update_router_static_routes(self):
"""Set external_ids column to any Neutron's owned static route.
"""
context = n_context.get_admin_context()
sroute_update = []
lrouters = self._nb_idl.get_all_logical_routers_static_routes()
for router in lrouters:
sroutes = router['static_routes']
for sroute in sroutes:
# Skip Static Routes that are already configured with an
# external_id key
if (ovn_const.OVN_LRSR_EXT_ID_KEY not in
sroute.external_ids.keys()):
sroute_update.append({'sroute': sroute,
'name': router['name']})
routes_cache = {}
cmds = []
columns = {'external_ids': {ovn_const.OVN_LRSR_EXT_ID_KEY: 'true'}}
for sroute in sroute_update:
lrouter = utils.ovn_name(sroute['name'])
if lrouter not in routes_cache.keys():
router_db = self._ovn_client._l3_plugin.get_router(context,
sroute['name'], fields=['routes'])
routes_cache[lrouter] = router_db.get('routes')
ovn_route = sroute['sroute']
for db_route in routes_cache[lrouter]:
if (ovn_route.ip_prefix == db_route['destination'] and
ovn_route.nexthop == db_route['nexthop']):
cmds.append(self._nb_idl.set_static_route(sroute['sroute'],
**columns))
break
if cmds:
with self._nb_idl.transaction(check_error=True) as txn:
for cmd in cmds:
txn.add(cmd)
raise periodics.NeverAgain()
class HashRingHealthCheckPeriodics(object):

View File

@@ -1304,7 +1304,8 @@ class OVNClient(object):
continue
columns = {'external_ids': {
ovn_const.OVN_ROUTER_IS_EXT_GW: 'true',
ovn_const.OVN_SUBNET_EXT_ID_KEY: gw_info.subnet_id}}
ovn_const.OVN_SUBNET_EXT_ID_KEY: gw_info.subnet_id,
ovn_const.OVN_LRSR_EXT_ID_KEY: 'true'}}
if router_default_route_bfd_enabled:
columns.update({
'output_port': utils.ovn_lrouter_port_name(
@@ -1380,10 +1381,12 @@ class OVNClient(object):
lrouter_name = utils.ovn_name(router_id)
commands = []
for route in add:
columns = {'external_ids': {
ovn_const.OVN_LRSR_EXT_ID_KEY: 'true'}}
commands.append(
self._nb_idl.add_static_route(
lrouter_name, ip_prefix=route['destination'],
nexthop=route['nexthop']))
nexthop=route['nexthop'], **columns))
routes_to_delete = [
(r['destination'], r['nexthop'])
for r in remove

View File

@@ -787,7 +787,8 @@ class OvnNbSynchronizer(OvnDbSynchronizer):
LOG.warning("Add static routes %s to OVN NB DB",
sroute['add'])
for route in sroute['add']:
columns = {}
columns = {'external_ids': {
ovn_const.OVN_LRSR_EXT_ID_KEY: 'true'}}
if 'external_ids' in route:
columns['external_ids'] = route['external_ids']
txn.add(self.ovn_api.add_static_route(

View File

@@ -665,6 +665,36 @@ class TestNbApi(BaseOvnIdlTest):
self._assert_routes_exist(lr_name, 0)
def test_modify_static_route_external_ids(self):
lr_name = 'router_with_static_routes_and_external_ids'
columns = {
'bfd': [],
'external_ids': {'fake_eid_key': 'fake_eid_value'},
'ip_prefix': '0.0.0.0/0',
'nexthop': '192.0.2.1',
'options': {'fake_option_key': 'fake_option_value'},
'output_port': [],
'policy': ['dst-ip'],
'route_table': '',
}
with self.nbapi.transaction(check_error=True) as txn:
r = self._add_static_route(txn, lr_name, '', **columns)
# modify the external_ids
new_ids = {'external_ids': {'fake_eid_key': 'fake_eid_value',
ovn_const.OVN_LRSR_EXT_ID_KEY: 'true'}}
with self.nbapi.transaction(check_error=True) as txn:
txn.add(self.nbapi.set_static_route(r.result.static_routes[0],
**new_ids))
lr = self.nbapi.lookup('Logical_Router', lr_name)
external_ids = {'fake_eid_key': 'fake_eid_value',
ovn_const.OVN_LRSR_EXT_ID_KEY: 'true'}
self.assertEqual(external_ids, lr.static_routes[0].external_ids)
class TestIgnoreConnectionTimeout(BaseOvnIdlTest):
@classmethod

View File

@@ -1250,6 +1250,88 @@ class TestMaintenance(_TestMaintenanceHelper):
'false',
ls.other_config.get(ovn_const.LS_OPTIONS_BROADCAST_ARPS_ROUTERS))
def test_static_routes_with_external_ids(self):
ext_net = self._create_network('ext_networktest', external=True)
ext_subnet = self._create_subnet(
'ext_subnettest',
ext_net['id'],
**{'cidr': '100.0.0.0/24',
'gateway_ip': '100.0.0.254',
'allocation_pools': [
{'start': '100.0.0.2', 'end': '100.0.0.253'}],
'enable_dhcp': False})
net1 = self._create_network('network1test', external=False)
subnet1 = self._create_subnet('subnet1test', net1['id'])
external_gateway_info = {
'enable_snat': True,
'network_id': ext_net['id'],
'external_fixed_ips': [
{'ip_address': '100.0.0.2', 'subnet_id': ext_subnet['id']}]}
router = self._create_router(
'routertest', external_gateway_info=external_gateway_info)
self._add_router_interface(router['id'], subnet1['id'])
# Create static routes via Neutron
with mock.patch.object(self.nb_api,
'add_static_route', columns=None):
self.l3_plugin.update_router(
self.context, router['id'],
{'router': {'routes': [{'destination': '10.10.0.0/24',
'nexthop': '100.0.0.3'},
{'destination': '20.0.0.0/24',
'nexthop': '100.0.0.6'}]}})
# Create a Neutron owned static route with external_ids key
columns = {'external_ids': {ovn_const.OVN_LRSR_EXT_ID_KEY: 'true'}}
with self.nb_api.transaction(check_error=True) as txn:
txn.add(self.nb_api.add_static_route('neutron-' + router['id'],
ip_prefix='10.10.0.0/24',
nexthop='100.0.0.3',
**columns))
# Create a Neutron owned static route without external_ids key
with self.nb_api.transaction(check_error=True) as txn:
txn.add(self.nb_api.add_static_route('neutron-' + router['id'],
ip_prefix='20.0.0.0/24',
nexthop='100.0.0.6'))
# Create an OVN externally managed static route without external_ids
with self.nb_api.transaction(check_error=True) as txn:
txn.add(self.nb_api.add_static_route('neutron-' + router['id'],
ip_prefix='30.0.0.0/24',
nexthop='100.0.0.9'))
sroutes = self.nb_api.get_all_logical_routers_static_routes()[0]
sroute_info = sroutes['static_routes']
for route in sroute_info:
if route.ip_prefix == '10.10.0.0/24':
self.assertEqual(route.external_ids,
{ovn_const.OVN_LRSR_EXT_ID_KEY: 'true'})
if route.ip_prefix == '20.0.0.0/24':
self.assertEqual({}, route.external_ids)
if route.ip_prefix == '30.0.0.0/24':
self.assertEqual({}, route.external_ids)
# Call the maintenance task and check that the value has been
# updated in the external_ids.
self.assertRaises(periodics.NeverAgain,
self.maint.update_router_static_routes)
sroutes = self.nb_api.get_all_logical_routers_static_routes()[0]
sroute_info = sroutes['static_routes']
for route in sroute_info:
if route.ip_prefix == '10.10.0.0/24':
self.assertEqual(route.external_ids,
{ovn_const.OVN_LRSR_EXT_ID_KEY: 'true'})
# Check if the OVN static route was updated with the Neutron key
if route.ip_prefix == '20.0.0.0/24':
self.assertEqual(route.external_ids,
{ovn_const.OVN_LRSR_EXT_ID_KEY: 'true'})
# Check if the externally managed OVN static route remains
# without the Neutron key.
if route.ip_prefix == '30.0.0.0/24':
self.assertEqual({}, route.external_ids)
class TestLogMaintenance(_TestMaintenanceHelper,
test_log_driver.LogApiTestCaseBase):

View File

@@ -797,10 +797,12 @@ class TestOvnNbSync(base.TestOVNFunctionalBase):
txn.add(self.nb_api.delete_lrouter_port(lrport,
lrouter_name, True))
columns = {'external_ids': {ovn_const.OVN_LRSR_EXT_ID_KEY: 'true'}}
for lrouter_name, ip_prefix, nexthop in self.create_lrouter_routes:
txn.add(self.nb_api.add_static_route(lrouter_name,
ip_prefix=ip_prefix,
nexthop=nexthop))
nexthop=nexthop,
**columns))
routers = defaultdict(list)
for lrouter_name, ip_prefix, nexthop in self.delete_lrouter_routes:
routers[lrouter_name].append((ip_prefix, nexthop))

View File

@@ -1228,3 +1228,46 @@ class TestDBInconsistenciesPeriodics(testlib_api.SqlTestCaseLight,
# Assert there was no transactions because the value was already set
self.fake_ovn_client._nb_idl.db_set.assert_not_called()
def test_update_static_routes_with_external_ids(self):
_nb_idl = self.fake_ovn_client._nb_idl
sroute_a = fakes.FakeOvsdbRow.create_one_ovsdb_row(
attrs={'ip_prefix': '30.0.0.0/24', 'nexthop': '20.0.2.5',
'external_ids': {}})
sroute_b = fakes.FakeOvsdbRow.create_one_ovsdb_row(
attrs={'ip_prefix': '30.1.0.0/24', 'nexthop': '20.0.2.6',
'external_ids': {}})
self.fake_external_fixed_ips = {
'network_id': 'ext-network-id',
'external_fixed_ips': [{'ip_address': '20.0.2.1',
'subnet_id': 'ext-subnet-id'}]}
lrouter = {
'id': 'lr-test',
'routes': [{'nexthop': '20.0.2.5',
'destination': '30.0.0.0/24'},
{'nexthop': '20.0.2.6',
'destination': '30.1.0.0/24'}],
'name': 'lr-test',
'admin_state_up': True,
'external_gateway_info': self.fake_external_fixed_ips
}
self.fake_ovn_client._l3_plugin.get_router.return_value = lrouter
expected = [{'name': 'lr-test',
'static_routes': [sroute_a, sroute_b]}]
_nb_idl.get_all_logical_routers_static_routes.return_value = expected
# Call the maintenance task and check that the value has been
# updated in the external_ids
self.assertRaises(periodics.NeverAgain,
self.periodic.update_router_static_routes)
# Check static routes calls to verify if the maintenance task work
# as expected
external_ids = {constants.OVN_LRSR_EXT_ID_KEY: 'true'}
_nb_idl.set_static_route.assert_has_calls([
mock.call(sroute_a, external_ids=external_ids),
mock.call(sroute_b, external_ids=external_ids),
])

View File

@@ -88,7 +88,8 @@ class TestOVNClient(TestOVNClientBase):
maintain_bfd=False,
external_ids={
'neutron:is_ext_gw': 'true',
'neutron:subnet_id': subnet['id']})
'neutron:subnet_id': subnet['id'],
constants.OVN_LRSR_EXT_ID_KEY: 'true'})
def test__add_router_ext_gw_default_route_ecmp(self):
plugin = mock.MagicMock()
@@ -140,7 +141,8 @@ class TestOVNClient(TestOVNClientBase):
maintain_bfd=False,
external_ids={
'neutron:is_ext_gw': 'true',
'neutron:subnet_id': subnet1['id']},
'neutron:subnet_id': subnet1['id'],
constants.OVN_LRSR_EXT_ID_KEY: 'true'},
),
mock.call('neutron-' + router['id'],
ip_prefix='0.0.0.0/0',
@@ -148,7 +150,8 @@ class TestOVNClient(TestOVNClientBase):
maintain_bfd=False,
external_ids={
'neutron:is_ext_gw': 'true',
'neutron:subnet_id': subnet2['id']},
'neutron:subnet_id': subnet2['id'],
constants.OVN_LRSR_EXT_ID_KEY: 'true'},
),
])

View File

@@ -119,7 +119,9 @@ class TestOVNL3RouterPlugin(test_mech_driver.Ml2PluginV2TestCase):
'admin_state_up': False,
'flavor_id': None,
'routes': [{'destination': '1.1.1.0/24',
'nexthop': '10.0.0.2'}]}
'nexthop': '10.0.0.2',
'external_ids':
{'neutron:is_static_route': 'true'}}]}
self.fake_router_interface_info = {
'port_id': 'router-port-id',
'device_id': '',
@@ -581,7 +583,9 @@ class TestOVNL3RouterPlugin(test_mech_driver.Ml2PluginV2TestCase):
mock_routes.return_value = self.fake_router['routes']
new_router = self.fake_router.copy()
updated_data = {'routes': [{'destination': '2.2.2.0/24',
'nexthop': '10.0.0.3'}]}
'nexthop': '10.0.0.3',
'external_ids':
{'neutron:is_static_route': 'true'}}]}
new_router.update(updated_data)
func.return_value = new_router
payload = self._create_payload_for_router_update(self.fake_router,
@@ -591,7 +595,8 @@ class TestOVNL3RouterPlugin(test_mech_driver.Ml2PluginV2TestCase):
self, payload)
self.l3_inst._nb_ovn.add_static_route.assert_called_once_with(
'neutron-router-id',
ip_prefix='2.2.2.0/24', nexthop='10.0.0.3')
ip_prefix='2.2.2.0/24', nexthop='10.0.0.3',
external_ids={'neutron:is_static_route': 'true'})
self.l3_inst._nb_ovn.delete_static_routes.assert_called_once_with(
'neutron-router-id', [('1.1.1.0/24', '10.0.0.2')])
@@ -655,7 +660,8 @@ class TestOVNL3RouterPlugin(test_mech_driver.Ml2PluginV2TestCase):
maintain_bfd=False,
external_ids={
ovn_const.OVN_ROUTER_IS_EXT_GW: 'true',
ovn_const.OVN_SUBNET_EXT_ID_KEY: 'ext-subnet-id'})]
ovn_const.OVN_SUBNET_EXT_ID_KEY: 'ext-subnet-id',
ovn_const.OVN_LRSR_EXT_ID_KEY: 'true'})]
self.l3_inst._nb_ovn.set_lrouter_port_in_lswitch_port.\
assert_called_once_with(
'gw-port-id', 'lrp-gw-port-id', is_gw_port=True,
@@ -835,7 +841,8 @@ class TestOVNL3RouterPlugin(test_mech_driver.Ml2PluginV2TestCase):
'neutron-router-id', ip_prefix='0.0.0.0/0',
maintain_bfd=False,
external_ids={ovn_const.OVN_ROUTER_IS_EXT_GW: 'true',
ovn_const.OVN_SUBNET_EXT_ID_KEY: 'ext-subnet-id'},
ovn_const.OVN_SUBNET_EXT_ID_KEY: 'ext-subnet-id',
ovn_const.OVN_LRSR_EXT_ID_KEY: 'true'},
nexthop='192.168.1.254')
self.l3_inst._nb_ovn.add_nat_rule_in_lrouter.assert_called_once_with(
'neutron-router-id', type='snat',
@@ -907,7 +914,8 @@ class TestOVNL3RouterPlugin(test_mech_driver.Ml2PluginV2TestCase):
nexthop='192.168.1.254',
maintain_bfd=False,
external_ids={ovn_const.OVN_ROUTER_IS_EXT_GW: 'true',
ovn_const.OVN_SUBNET_EXT_ID_KEY: 'ext-subnet-id'})
ovn_const.OVN_SUBNET_EXT_ID_KEY: 'ext-subnet-id',
ovn_const.OVN_LRSR_EXT_ID_KEY: 'true'})
self.l3_inst._nb_ovn.add_nat_rule_in_lrouter.assert_called_once_with(
'neutron-router-id', type='snat', logical_ip='10.0.0.0/24',
external_ip='192.168.1.1')
@@ -961,7 +969,8 @@ class TestOVNL3RouterPlugin(test_mech_driver.Ml2PluginV2TestCase):
nexthop='192.168.1.254',
maintain_bfd=False,
external_ids={ovn_const.OVN_ROUTER_IS_EXT_GW: 'true',
ovn_const.OVN_SUBNET_EXT_ID_KEY: 'ext-subnet-id'})
ovn_const.OVN_SUBNET_EXT_ID_KEY: 'ext-subnet-id',
ovn_const.OVN_LRSR_EXT_ID_KEY: 'true'})
self.l3_inst._nb_ovn.add_nat_rule_in_lrouter.assert_called_once_with(
'neutron-router-id', type='snat', logical_ip='10.0.0.0/24',
external_ip='192.168.1.1')
@@ -1017,7 +1026,8 @@ class TestOVNL3RouterPlugin(test_mech_driver.Ml2PluginV2TestCase):
'neutron-router-id', ip_prefix='0.0.0.0/0',
maintain_bfd=False,
external_ids={ovn_const.OVN_ROUTER_IS_EXT_GW: 'true',
ovn_const.OVN_SUBNET_EXT_ID_KEY: 'ext-subnet-id'},
ovn_const.OVN_SUBNET_EXT_ID_KEY: 'ext-subnet-id',
ovn_const.OVN_LRSR_EXT_ID_KEY: 'true'},
nexthop='192.168.1.254')
self.l3_inst._nb_ovn.add_nat_rule_in_lrouter.assert_not_called()
@@ -2163,7 +2173,8 @@ class OVNL3ExtrarouteTests(test_l3_gw.ExtGwModeIntTestCase,
super(OVNL3ExtrarouteTests, self). \
test_update_subnet_gateway_for_external_net()
self.l3_inst._nb_ovn.add_static_route.assert_called_once_with(
'neutron-fake_device', ip_prefix='0.0.0.0/0', nexthop='120.0.0.2')
'neutron-fake_device', ip_prefix='0.0.0.0/0', nexthop='120.0.0.2',
external_ids={'neutron:is_static_route': 'true'})
self.l3_inst._nb_ovn.delete_static_routes.assert_called_once_with(
'neutron-fake_device', [('0.0.0.0/0', '120.0.0.1')])
@@ -2172,7 +2183,8 @@ class OVNL3ExtrarouteTests(test_l3_gw.ExtGwModeIntTestCase,
test_router_update_gateway_upon_subnet_create_max_ips_ipv6()
expected_ext_ids = {
ovn_const.OVN_ROUTER_IS_EXT_GW: 'true',
ovn_const.OVN_SUBNET_EXT_ID_KEY: mock.ANY}
ovn_const.OVN_SUBNET_EXT_ID_KEY: mock.ANY,
ovn_const.OVN_LRSR_EXT_ID_KEY: 'true'}
add_static_route_calls = [
mock.call(mock.ANY, ip_prefix='0.0.0.0/0', nexthop='10.0.0.1',
maintain_bfd=False, external_ids=expected_ext_ids),

View File

@@ -0,0 +1,12 @@
---
features:
- |
The OVN ML2 mechanism driver for static routes will now include the key
``neutron:is_static_route`` in the external_ids register for external
gateway router ports. This is required for the OVN DB sync tool to
distinguish the Neutron created Static Routes from those added externally
in the OVN database. Previously created static route rules will be updated
only once during the maintenance task to include the
``neutron:is_static_route`` key in the external_ids register. In case all
static route entries are already configured using this key, no maintenance
action will be performed.