openstack
/
neutron
Code Issues Proposed changes

[OVN] Make mandatory the router name in the LRP.external_ids 

The router name will be always defined in the "Logical_Router_Port"
external_ids field.

Related-Bug: #2052821
Change-Id: Ia2f70363963dca9f035eff8d1ff0c399dc8b9239
This commit is contained in:
Rodolfo Alonso Hernandez 2024-02-23 04:41:15 +00:00 committed by Rodolfo Alonso
parent 310a96a302
commit b5aecfeff8
4 changed files with 49 additions and 31 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -1606,8 +1606,8 @@ class OVNClient(object):
const.TYPE_VLAN]:
return network.get(pnet.PHYSICAL_NETWORK)
def _gen_router_port_ext_ids(self, port):
ext_ids = {
def _gen_router_port_ext_ids(self, port, router_id):
return {
ovn_const.OVN_REV_NUM_EXT_ID_KEY: str(utils.get_revision_number(
port, ovn_const.TYPE_ROUTER_PORTS)),
ovn_const.OVN_SUBNET_EXT_IDS_KEY:
@ -1616,14 +1616,9 @@ class OVNClient(object):
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,
}
router_id = port.get('device_id')
if router_id:
ext_ids[ovn_const.OVN_ROUTER_NAME_EXT_ID_KEY] = router_id
return ext_ids
def _get_reside_redir_for_gateway_port(self, device_id):
admin_context = n_context.get_admin_context()
reside_redir_ch = 'true'
@ -1730,7 +1725,7 @@ class OVNClient(object):
mac=port['mac_address'],
networks=networks,
may_exist=True,
external_ids=self._gen_router_port_ext_ids(port),
external_ids=self._gen_router_port_ext_ids(port, router['id']),
**columns)
]
@ -1808,10 +1803,14 @@ class OVNClient(object):
update = {'networks': networks, 'ipv6_ra_configs': ipv6_ra_configs}
is_gw_port = const.DEVICE_OWNER_ROUTER_GW == port.get(
'device_owner')
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]
commands = [
self._nb_idl.update_lrouter_port(
name=lrp_name,
external_ids=self._gen_router_port_ext_ids(port),
external_ids=self._gen_router_port_ext_ids(port, router_id),
options=self._gen_router_port_options(port),
if_exists=if_exists,
**update),
@ -1854,17 +1853,15 @@ class OVNClient(object):
]
return utils.OvsdbClientTransactCommand.run(cmd)
def _delete_lrouter_port(self, context, port_id, router_id=None, txn=None):
def _delete_lrouter_port(self, context, port_id, router_id, txn=None):
"""Delete a logical router port."""
commands = [self._nb_idl.lrp_del(
utils.ovn_lrouter_port_name(port_id),
utils.ovn_name(router_id) if router_id else None,
if_exists=True)]
utils.ovn_name(router_id), if_exists=True)]
self._transaction(commands, txn=txn)
db_rev.delete_revision(context, port_id, ovn_const.TYPE_ROUTER_PORTS)
def delete_router_port(self, context, port_id, router_id=None,
subnet_ids=None):
def delete_router_port(self, context, port_id, subnet_ids=None):
try:
ovn_port = self._nb_idl.lookup(
'Logical_Router_Port', utils.ovn_lrouter_port_name(port_id))
@ -1887,20 +1884,16 @@ class OVNClient(object):
# rules in the router itself if we have to
port_removed = True
router_id = router_id or ovn_port.external_ids.get(
ovn_const.OVN_ROUTER_NAME_EXT_ID_KEY)
if port and not router_id:
router_id = port.get('device_id')
router_id = ovn_port.external_ids[
ovn_const.OVN_ROUTER_NAME_EXT_ID_KEY]
router = None
gw_ports = []
if router_id:
try:
router = self._l3_plugin.get_router(context, router_id)
gw_ports = self._get_router_gw_ports(context, router_id)
except l3_exc.RouterNotFound:
# If the router is gone, the router port is also gone
port_removed = True
try:
router = self._l3_plugin.get_router(context, router_id)
gw_ports = self._get_router_gw_ports(context, router_id)
except l3_exc.RouterNotFound:
# If the router is gone, the router port is also gone
port_removed = True
if not router or not gw_ports:
if port_removed:

1
neutron/services/ovn_l3/service_providers/ovn.py
View File

@ -143,7 +143,6 @@ class OvnDriver(base.L3ServiceProvider):
subnet_ids[0], subnet_ids)
try:
self.l3plugin._ovn_client.delete_router_port(context, port['id'],
router_id=router.id,
subnet_ids=subnet_ids)
except Exception:
with excutils.save_and_reraise_exception():

1
neutron/tests/unit/fake_resources.py
View File

@ -140,6 +140,7 @@ class FakeOvsdbNbOvnIdl(object):
self.lsp_list = mock.MagicMock()
self.db_find = mock.Mock()
self.db_find_rows = mock.Mock()
self.db_get = mock.Mock()
self.db_set = mock.Mock()
self.db_clear = mock.Mock()
self.db_remove = mock.Mock()

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

@ -101,7 +101,10 @@ class TestOVNL3RouterPlugin(test_mech_driver.Ml2PluginV2TestCase):
ovn_const.OVN_REV_NUM_EXT_ID_KEY: '1',
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_IS_EXT_GW: 'False',
ovn_const.OVN_ROUTER_NAME_EXT_ID_KEY: 'router-id',
}
}
self.fake_router_ports = [self.fake_router_port]
self.fake_subnet = {'id': 'subnet-id',
'ip_version': 4,
@ -162,7 +165,9 @@ class TestOVNL3RouterPlugin(test_mech_driver.Ml2PluginV2TestCase):
ovn_const.OVN_REV_NUM_EXT_ID_KEY: '1',
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_IS_EXT_GW: 'True',
ovn_const.OVN_ROUTER_NAME_EXT_ID_KEY: 'router-id',
},
'options': {}}
self.fake_floating_ip_attrs = {'floating_ip_address': '192.168.0.10',
'fixed_ip_address': '10.0.0.10'}
@ -405,6 +410,8 @@ class TestOVNL3RouterPlugin(test_mech_driver.Ml2PluginV2TestCase):
'network_id': 'network-id1'}
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}
self.ovn_drv._process_add_router_interface(resources.ROUTER_INTERFACE,
events.AFTER_CREATE,
self, payload)
@ -422,6 +429,8 @@ class TestOVNL3RouterPlugin(test_mech_driver.Ml2PluginV2TestCase):
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})
self.get_port.side_effect = n_exc.PortNotFound(
port_id='router-port-id')
@ -437,6 +446,8 @@ class TestOVNL3RouterPlugin(test_mech_driver.Ml2PluginV2TestCase):
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}
payload = self._create_payload_for_router_interface(router_id,
pass_subnet=False)
self.ovn_drv._process_remove_router_interface(
@ -452,10 +463,14 @@ class TestOVNL3RouterPlugin(test_mech_driver.Ml2PluginV2TestCase):
ovn_const.OVN_REV_NUM_EXT_ID_KEY: '1',
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_IS_EXT_GW: 'False',
ovn_const.OVN_ROUTER_NAME_EXT_ID_KEY: 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})
self.get_port.side_effect = n_exc.PortNotFound(
port_id='router-port-id')
self.get_router.side_effect = l3_exc.RouterNotFound(
@ -757,6 +772,8 @@ class TestOVNL3RouterPlugin(test_mech_driver.Ml2PluginV2TestCase):
def test_remove_router_interface_with_gateway_set(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})
self.get_router.return_value = self.fake_router_with_ext_gw
self.get_port.side_effect = n_exc.PortNotFound(
port_id='router-port-id')
@ -2098,6 +2115,10 @@ 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'}
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)
# Note(dongj): According to bug #1657693, status of an unassociated
# floating IP is set to DOWN. Revise expected_status to DOWN for related
@ -2141,3 +2162,7 @@ class OVNL3ExtrarouteTests(test_l3_gw.ExtGwModeIntTestCase,
maintain_bfd=False, external_ids=expected_ext_ids)]
self.l3_inst._nb_ovn.add_static_route.assert_has_calls(
add_static_route_calls, any_order=True)
def test_create_floatingip_with_assoc(self, **kwargs):
self.l3_inst._nb_ovn.lookup.return_value = mock.Mock(load_balancer=[])
super().test_create_floatingip_with_assoc(**kwargs)