openstack/neutron
Code Issues Proposed changes

Merge "Fix LRP.external_ids router name prefix"

Browse Source
This commit is contained in:
Zuul
2025-04-09 10:42:29 +00:00
committed by Gerrit Code Review
parent a85a28e85c 2996f9ff53
commit 763818b1d9
8 changed files with 130 additions and 26 deletions
Download Patch File Download Diff File Expand all files Collapse all files

7
neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/impl_idl_ovn.py
View File

@@ -560,10 +560,9 @@ class OvsdbNbOvnIdl(nb_impl_idl.OvnNbApiIdlImpl, Backend):
default=None)
if not lrp:
return []
router_id = lrp.external_ids.get(
router_name = lrp.external_ids.get(
ovn_const.OVN_ROUTER_NAME_EXT_ID_KEY, "")
lrouter = self.lookup('Logical_Router', utils.ovn_name(router_id),
default=None)
lrouter = self.lookup('Logical_Router', router_name, default=None)
if not lrouter:
return []
az_string = lrouter.external_ids.get(
@@ -819,7 +818,7 @@ class OvsdbNbOvnIdl(nb_impl_idl.OvnNbApiIdlImpl, Backend):
for lrp in getattr(lr, 'ports', []):
lrp_ext_ids = getattr(lrp, 'external_ids', {})
if (r_name not in lrp_ext_ids or
utils.ovn_name(lrp_ext_ids[r_name]) != lr.name or
lrp_ext_ids[r_name] != lr.name or
not strutils.bool_from_string(lrp_ext_ids.get(is_gw))):
continue

40
neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/maintenance.py
View File

@@ -485,6 +485,46 @@ class DBInconsistenciesPeriodics(SchemaAwarePeriodicsBase):
context, fip_id)
self._ovn_client.delete_floatingip(context, fip_id)
# A static spacing value is used here, but this method will only run
# once per lock due to the use of periodics.NeverAgain().
@has_lock_periodic(
periodic_run_limit=ovn_const.MAINTENANCE_TASK_RETRY_LIMIT,
spacing=ovn_const.MAINTENANCE_ONE_RUN_TASK_SPACING,
run_immediately=True)
def update_lrouter_ports_ext_ids_name_prefix(self):
"""Update OVN logical router ports if missing external ids
"neutron-" prefix for router name.
"""
LOG.debug(
'Maintenance task: Check missing prefix router_name in '
'external_ids of LRPs.')
self._sync_timer.restart()
ports_to_prefix = []
rports = self._nb_idl.db_find(
'Logical_Router_Port').execute(check_error=True)
for port in rports:
ext_id_rname = port['external_ids'].get(
ovn_const.OVN_ROUTER_NAME_EXT_ID_KEY)
if ext_id_rname and not ext_id_rname.startswith(
ovn_const.OVN_NAME_PREFIX):
ports_to_prefix.append(port)
if ports_to_prefix:
LOG.debug('Update missing prefix for router_name in %d LRPs:\n%s',
len(ports_to_prefix),
[str(p['_uuid']) for p in ports_to_prefix])
with self._nb_idl.transaction(check_error=True) as txn:
for port in ports_to_prefix:
router_id = port['external_ids'].get(
ovn_const.OVN_ROUTER_NAME_EXT_ID_KEY)
txn.add(self._nb_idl.update_lrouter_port(
name=port.name,
external_ids=self._ovn_client._gen_router_port_ext_ids(
port, router_id)))
self._sync_timer.stop()
LOG.info('Maintenance task: Check missing prefix router_name in LRPs '
'(took %.2f seconds)', self._sync_timer.elapsed())
raise periodics.NeverAgain()
# A static spacing value is used here, but this method will only run
# once per lock due to the use of periodics.NeverAgain().
@has_lock_periodic(

9
neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_client.py
View File

@@ -1672,7 +1672,8 @@ class OVNClient:
utils.ovn_name(port['network_id']),
ovn_const.OVN_ROUTER_IS_EXT_GW:
str(const.DEVICE_OWNER_ROUTER_GW == port.get('device_owner')),
ovn_const.OVN_ROUTER_NAME_EXT_ID_KEY: router_id,
ovn_const.OVN_ROUTER_NAME_EXT_ID_KEY:
utils.ovn_name(router_id),
}
def _get_reside_redir_for_gateway_port(self, device_id):
@@ -1877,7 +1878,8 @@ class OVNClient:
external_ids = self._nb_idl.db_get(
'Logical_Router_Port', lrp_name,
'external_ids').execute(check_error=True)
router_id = external_ids[ovn_const.OVN_ROUTER_NAME_EXT_ID_KEY]
router_id = external_ids[ovn_const.OVN_ROUTER_NAME_EXT_ID_KEY].replace(
ovn_const.OVN_NAME_PREFIX, '')
commands = [
self._nb_idl.update_lrouter_port(
name=lrp_name,
@@ -1956,7 +1958,8 @@ class OVNClient:
port_removed = True
router_id = ovn_port.external_ids[
ovn_const.OVN_ROUTER_NAME_EXT_ID_KEY]
ovn_const.OVN_ROUTER_NAME_EXT_ID_KEY].replace(
ovn_const.OVN_NAME_PREFIX, '')
router = None
gw_ports = []
try:

19
neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_maintenance.py
View File

@@ -197,6 +197,10 @@ class _TestMaintenanceHelper(testlib_api.MySQLTestCaseMixin,
ovn_const.OVN_ROUTER_NAME_EXT_ID_KEY) == name):
return row
def _get_lrp_ext_ids_router_name(self, port_id):
row = self._find_router_port_row_by_port_id(port_id)
return row.external_ids.get(ovn_const.OVN_ROUTER_NAME_EXT_ID_KEY)
def _create_security_group(self):
data = {'security_group': {'name': 'sgtest',
'description': 'SpongeBob Rocks!'}}
@@ -730,6 +734,21 @@ class TestMaintenance(_TestMaintenanceHelper):
self.assertIsNotNone(
self._find_router_port_row_by_port_id(neutron_obj['port_id']))
# Assert router port has "neutron-" prefix and correct router uuid
# for external_ids neutron:router_name value, when created and after
# maintenance/updated (LP#2055045)
self.assertEqual(
'%s%s' % (ovn_const.OVN_NAME_PREFIX, neutron_router['id']),
self._get_lrp_ext_ids_router_name(neutron_obj['port_id']))
self.assertRaises(periodics.NeverAgain,
self.maint.update_lrouter_ports_ext_ids_name_prefix)
self.assertEqual(
'%s%s' % (ovn_const.OVN_NAME_PREFIX, neutron_router['id']),
self._get_lrp_ext_ids_router_name(neutron_obj['port_id']))
# > Delete
with mock.patch.object(self._l3_ovn_client, 'delete_router_port'):

5
neutron/tests/functional/services/ovn_l3/test_plugin.py
View File

@@ -346,8 +346,9 @@ class TestRouter(base.TestOVNFunctionalBase):
if (ovn_const.OVN_ROUTER_NAME_EXT_ID_KEY
not in row.external_ids):
continue
if row.external_ids[
ovn_const.OVN_ROUTER_NAME_EXT_ID_KEY] == router['id']:
ext_ids_rtr_name = row.external_ids[
ovn_const.OVN_ROUTER_NAME_EXT_ID_KEY]
if ext_ids_rtr_name == ovn_utils.ovn_name(router['id']):
chassis = {}
for gwc in row.gateway_chassis:
chassis[gwc.priority] = gwc.chassis_name

24
neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_impl_idl_ovn.py
View File

@@ -168,18 +168,24 @@ class TestNBImplIdlOvn(TestDBImplIdlOvn):
'lr-name-f'}}],
'lrouter_ports': [
{'name': utils.ovn_lrouter_port_name('orp-id-a1'),
'external_ids': {ovn_const.OVN_ROUTER_NAME_EXT_ID_KEY: 'lr-id-a',
ovn_const.OVN_ROUTER_IS_EXT_GW: str(True)},
'external_ids': {
ovn_const.OVN_ROUTER_NAME_EXT_ID_KEY:
utils.ovn_name('lr-id-a'),
ovn_const.OVN_ROUTER_IS_EXT_GW: str(True)},
'networks': ['10.0.1.0/24'],
'options': {ovn_const.OVN_GATEWAY_CHASSIS_KEY: 'host-1'}},
{'name': utils.ovn_lrouter_port_name('orp-id-a2'),
'external_ids': {ovn_const.OVN_ROUTER_NAME_EXT_ID_KEY: 'lr-id-a',
ovn_const.OVN_ROUTER_IS_EXT_GW: str(True)},
'external_ids': {
ovn_const.OVN_ROUTER_NAME_EXT_ID_KEY:
utils.ovn_name('lr-id-a'),
ovn_const.OVN_ROUTER_IS_EXT_GW: str(True)},
'networks': ['10.0.2.0/24'],
'options': {ovn_const.OVN_GATEWAY_CHASSIS_KEY: 'host-1'}},
{'name': utils.ovn_lrouter_port_name('orp-id-a3'),
'external_ids': {ovn_const.OVN_ROUTER_NAME_EXT_ID_KEY: 'lr-id-a',
ovn_const.OVN_ROUTER_IS_EXT_GW: str(True)},
'external_ids': {
ovn_const.OVN_ROUTER_NAME_EXT_ID_KEY:
utils.ovn_name('lr-id-a'),
ovn_const.OVN_ROUTER_IS_EXT_GW: str(True)},
'networks': ['10.0.3.0/24'],
'options': {ovn_const.OVN_GATEWAY_CHASSIS_KEY: None}},
{'name': 'xrp-id-b1',
@@ -191,8 +197,10 @@ class TestNBImplIdlOvn(TestDBImplIdlOvn):
'external_ids': {}, 'networks': ['20.0.3.0/24'],
'options': {}},
{'name': utils.ovn_lrouter_port_name('gwc'),
'external_ids': {ovn_const.OVN_ROUTER_NAME_EXT_ID_KEY: 'lr-id-f',
ovn_const.OVN_ROUTER_IS_EXT_GW: str(True)},
'external_ids': {
ovn_const.OVN_ROUTER_NAME_EXT_ID_KEY:
utils.ovn_name('lr-id-f'),
ovn_const.OVN_ROUTER_IS_EXT_GW: str(True)},
'networks': ['10.0.4.0/24'],
'options': {}}],
'gateway_chassis': [

27
neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_maintenance.py
View File

@@ -394,6 +394,33 @@ class TestDBInconsistenciesPeriodics(testlib_api.SqlTestCaseLight,
self.periodic._log_maintenance_inconsistencies(incst, [])
self.assertFalse(mock_log.called)
def test_update_lrouter_ports_ext_ids_name_prefix(self):
lrp1_no_prefix = fakes.FakeOvsdbRow.create_one_ovsdb_row(
attrs={
'_uuid': 'port-id1',
'name': 'lrp-id1',
'external_ids': {
constants.OVN_ROUTER_NAME_EXT_ID_KEY: 'rtr-id1'}})
lrp2_with_prefix = fakes.FakeOvsdbRow.create_one_ovsdb_row(
attrs={
'_uuid': 'port-id2',
'name': 'lrp-id2',
'external_ids': {
constants.OVN_ROUTER_NAME_EXT_ID_KEY:
'%srtr-id2' % constants.OVN_NAME_PREFIX}})
nb_idl = self.fake_ovn_client._nb_idl
nb_idl.db_find.return_value.execute.return_value = [
lrp1_no_prefix,
lrp2_with_prefix]
self.assertRaises(
periodics.NeverAgain,
self.periodic.update_lrouter_ports_ext_ids_name_prefix)
nb_idl.update_lrouter_port.assert_called_once_with(
name='lrp-id1', external_ids=mock.ANY)
def test_check_for_igmp_snoop_support(self):
cfg.CONF.set_override('igmp_snooping_enable', True, group='OVS')
nb_idl = self.fake_ovn_client._nb_idl

25
neutron/tests/unit/services/ovn_l3/test_plugin.py
View File

@@ -106,7 +106,8 @@ class BaseTestOVNL3RouterPluginMixin():
ovn_const.OVN_NETWORK_NAME_EXT_ID_KEY:
utils.ovn_name(self.fake_network['id']),
ovn_const.OVN_ROUTER_IS_EXT_GW: 'False',
ovn_const.OVN_ROUTER_NAME_EXT_ID_KEY: 'router-id',
ovn_const.OVN_ROUTER_NAME_EXT_ID_KEY:
utils.ovn_name('router-id'),
}
}
self.fake_router_ports = [self.fake_router_port]
@@ -172,7 +173,8 @@ class BaseTestOVNL3RouterPluginMixin():
ovn_const.OVN_NETWORK_NAME_EXT_ID_KEY:
utils.ovn_name('ext-network-id'),
ovn_const.OVN_ROUTER_IS_EXT_GW: 'True',
ovn_const.OVN_ROUTER_NAME_EXT_ID_KEY: 'router-id',
ovn_const.OVN_ROUTER_NAME_EXT_ID_KEY:
utils.ovn_name('router-id'),
},
'options': {
ovn_const.OVN_ROUTER_PORT_GW_MTU_OPTION: str(default_mtu)
@@ -429,7 +431,8 @@ class BaseTestOVNL3RouterPluginMixin():
fake_rtr_intf_networks = ['2001:db8::1/24', '2001:dba::1/24']
payload = self._create_payload_for_router_interface(router_id)
self.l3_inst._nb_ovn.db_get.return_value.execute.return_value = {
ovn_const.OVN_ROUTER_NAME_EXT_ID_KEY: router_id}
ovn_const.OVN_ROUTER_NAME_EXT_ID_KEY:
utils.ovn_name('router-id')}
self.ovn_drv._process_add_router_interface(resources.ROUTER_INTERFACE,
events.AFTER_CREATE,
self, payload)
@@ -448,7 +451,8 @@ class BaseTestOVNL3RouterPluginMixin():
def test_remove_router_interface(self):
router_id = 'router-id'
self.l3_inst._nb_ovn.lookup.return_value = mock.Mock(
external_ids={ovn_const.OVN_ROUTER_NAME_EXT_ID_KEY: router_id})
external_ids={ovn_const.OVN_ROUTER_NAME_EXT_ID_KEY:
utils.ovn_name(router_id)})
self.get_port.side_effect = n_exc.PortNotFound(
port_id='router-port-id')
@@ -465,7 +469,7 @@ class BaseTestOVNL3RouterPluginMixin():
def test_remove_router_interface_update_lrouter_port(self):
router_id = 'router-id'
self.l3_inst._nb_ovn.db_get.return_value.execute.return_value = {
ovn_const.OVN_ROUTER_NAME_EXT_ID_KEY: router_id}
ovn_const.OVN_ROUTER_NAME_EXT_ID_KEY: utils.ovn_name(router_id)}
payload = self._create_payload_for_router_interface(router_id,
pass_subnet=False)
self.ovn_drv._process_remove_router_interface(
@@ -482,17 +486,19 @@ class BaseTestOVNL3RouterPluginMixin():
ovn_const.OVN_NETWORK_NAME_EXT_ID_KEY:
utils.ovn_name(self.fake_network['id']),
ovn_const.OVN_ROUTER_IS_EXT_GW: 'False',
ovn_const.OVN_ROUTER_NAME_EXT_ID_KEY: router_id,
ovn_const.OVN_ROUTER_NAME_EXT_ID_KEY:
utils.ovn_name(router_id)
})
def test_remove_router_interface_router_not_found(self):
router_id = 'router-id'
self.l3_inst._nb_ovn.lookup.return_value = mock.Mock(
external_ids={ovn_const.OVN_ROUTER_NAME_EXT_ID_KEY: router_id})
external_ids={ovn_const.OVN_ROUTER_NAME_EXT_ID_KEY:
utils.ovn_name(router_id)})
self.get_port.side_effect = n_exc.PortNotFound(
port_id='router-port-id')
self.get_router.side_effect = l3_exc.RouterNotFound(
router_id='router-id')
router_id='neutron-router-id')
payload = self._create_payload_for_router_interface(router_id,
pass_subnet=False)
@@ -2177,7 +2183,8 @@ class OVNL3ExtrarouteTests(test_l3_gw.ExtGwModeIntTestCase,
'OVNClient.delete_mac_binding_entries_by_mac',
return_value=1)
self.setup_notification_driver()
ext_ids = {ovn_const.OVN_ROUTER_NAME_EXT_ID_KEY: 'router-id'}
ext_ids = {ovn_const.OVN_ROUTER_NAME_EXT_ID_KEY:
utils.ovn_name('router-id')}
self.l3_inst._nb_ovn.db_get.return_value.execute.return_value = ext_ids
self.l3_inst._nb_ovn.lookup.return_value = mock.Mock(
external_ids=ext_ids)