openstack/neutron
Code Issues Proposed changes

Log warning about port forwardings that won't work properly

Browse Source 
This is follow up patch to [1] in which was added warning about
incompatible configuration of the vlan/flat networks allowed as tenant
networks, distributed routing and port forwardings.

In this new patch similar warning is logged every time when port
forwarding is created using router which have actually connected vlan or
flat networks as "internal networks" (external gateway network is fine)
and when distributed routing is enabled in the Neutron config.

This patch additionally adds "neutron:is_ext_gw" flag to the
Logical_Router_Port's external_ids. With that it's easier to check if
network is used as gateway network (no checks needed) or not (checks are
perfomed and warning may be logged).

[1] https://review.opendev.org/c/openstack/neutron/+/892542

Related-Bug: #2028846
Change-Id: I101128bdb421ec83df5cdcb0d486cbafbbca2ce5
This commit is contained in:
Slawek Kaplonski 2024-02-01 11:36:29 +01:00
parent ce53fb55ad
commit a355d2a0d5
7 changed files with 200 additions and 4 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
neutron/common/ovn/utils.py
View File

@ -685,6 +685,15 @@ def is_gateway_chassis_invalid(chassis_name, gw_chassis,
def is_provider_network(network):
"""Check if given network is provider network
:param network: (str, dict) it can be given as network object or as string
with network ID only. In the latter case, network object
will be loaded from the database
"""
if isinstance(network, str):
ctx = n_context.get_admin_context()
plugin = directory.get_plugin()
network = plugin.get_network(ctx, network)
return network.get(provider_net.PHYSICAL_NETWORK, False)

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

@ -926,6 +926,41 @@ class DBInconsistenciesPeriodics(SchemaAwarePeriodicsBase):
raise periodics.NeverAgain()
# TODO(slaweq): Remove this in the E cycle (C+2 as it will be next SLURP)
@has_lock_periodic(spacing=600, run_immediately=True)
def add_gw_port_info_to_logical_router_port(self):
"""Add info if LRP is connecting internal subnet or ext gateway."""
cmds = []
context = n_context.get_admin_context()
for router in self._ovn_client._l3_plugin.get_routers(context):
ext_gw_networks = [
ext_gw['network_id'] for ext_gw in router['external_gateways']]
rtr_name = 'neutron-{}'.format(router['id'])
ovn_lr = self._nb_idl.get_lrouter(rtr_name)
for lrp in ovn_lr.ports:
if ovn_const.OVN_ROUTER_IS_EXT_GW in lrp.external_ids:
continue
ovn_network_name = lrp.external_ids.get(
ovn_const.OVN_NETWORK_NAME_EXT_ID_KEY)
if not ovn_network_name:
continue
network_id = ovn_network_name.replace('neutron-', '')
if not network_id:
continue
is_ext_gw = str(network_id in ext_gw_networks)
external_ids = lrp.external_ids
external_ids[ovn_const.OVN_ROUTER_IS_EXT_GW] = is_ext_gw
cmds.append(
self._nb_idl.update_lrouter_port(
name=lrp.name,
external_ids=external_ids))
if cmds:
with self._nb_idl.transaction(check_error=True) as txn:
for cmd in cmds:
txn.add(cmd)
raise periodics.NeverAgain()
@has_lock_periodic(spacing=600, run_immediately=True)
def check_router_default_route_empty_dst_ip(self):
"""Check routers with default route with empty dst-ip (LP: #2002993).

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

@ -1576,7 +1576,10 @@ class OVNClient(object):
ovn_const.OVN_SUBNET_EXT_IDS_KEY:
' '.join(utils.get_port_subnet_ids(port)),
ovn_const.OVN_NETWORK_NAME_EXT_ID_KEY:
utils.ovn_name(port['network_id'])}
utils.ovn_name(port['network_id']),
ovn_const.OVN_ROUTER_IS_EXT_GW:
str(const.DEVICE_OWNER_ROUTER_GW == port.get('device_owner')),
}
router_id = port.get('device_id')
if router_id:

36
neutron/services/portforwarding/drivers/ovn/driver.py
View File

@ -17,12 +17,14 @@ from neutron_lib import constants as const
from neutron_lib.plugins import constants as plugin_constants
from neutron_lib.plugins import directory
from oslo_log import log
from oslo_utils import strutils
from ovsdbapp.backend.ovs_idl import idlutils
from ovsdbapp import constants as ovsdbapp_const
from neutron.common.ovn import constants as ovn_const
from neutron.common.ovn import exceptions as ovn_exc
from neutron.common.ovn import utils as ovn_utils
from neutron.conf.plugins.ml2.drivers.ovn import ovn_conf
from neutron.db import ovn_revision_numbers_db as db_rev
from neutron import manager
from neutron.objects import port_forwarding as port_forwarding_obj
@ -129,7 +131,41 @@ class OVNPortForwardingHandler(object):
"Switch %s failed as it is not found",
lb_name, ls_name)
def _validate_router_networks(self, nb_ovn, router_id):
if not ovn_conf.is_ovn_distributed_floating_ip():
return
rtr_name = 'neutron-{}'.format(router_id)
ovn_lr = nb_ovn.get_lrouter(rtr_name)
if not ovn_lr:
return
for lrouter_port in ovn_lr.ports:
is_ext_gw = strutils.bool_from_string(
lrouter_port.external_ids.get(ovn_const.OVN_ROUTER_IS_EXT_GW))
if is_ext_gw:
# NOTE(slaweq): This is external gateway port of the router and
# this not needs to be checked
continue
ovn_network_name = lrouter_port.external_ids.get(
ovn_const.OVN_NETWORK_NAME_EXT_ID_KEY)
if not ovn_network_name:
continue
network_id = ovn_network_name.replace('neutron-', '')
if not network_id:
continue
if ovn_utils.is_provider_network(network_id):
LOG.warning("Port forwarding configured in the router "
"%(router_id)s will not work properly as "
"distributed floating IPs are enabled "
"and at least one provider network "
"(%(network_id)s) is connected to that router. "
"See bug https://launchpad.net/bugs/2028846 for "
"more details.", {
'router_id': router_id,
'network_id': network_id})
return
def port_forwarding_created(self, ovn_txn, nb_ovn, pf_obj):
self._validate_router_networks(nb_ovn, pf_obj.router_id)
pf_objs = pf_obj.unroll_port_ranges()
is_range = len(pf_objs) > 1
for pf_obj in pf_objs:

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

@ -946,6 +946,70 @@ class TestDBInconsistenciesPeriodics(testlib_api.SqlTestCaseLight,
('type', constants.LSP_TYPE_VIRTUAL))]
nb_idl.db_set.assert_has_calls(expected_calls)
def test_add_gw_port_info_to_logical_router_port(self):
nb_idl = self.fake_ovn_client._nb_idl
ext_net_id = uuidutils.generate_uuid()
internal_net_id = uuidutils.generate_uuid()
routers_db = [{
'id': uuidutils.generate_uuid(),
'external_gateways': [{'network_id': ext_net_id}]}]
ext_gw_lrp = fakes.FakeOvsdbRow.create_one_ovsdb_row(
attrs={'external_ids': {
constants.OVN_NETWORK_NAME_EXT_ID_KEY:
'neutron-{}'.format(ext_net_id)}})
internal_net_lrp = fakes.FakeOvsdbRow.create_one_ovsdb_row(
attrs={'external_ids': {
constants.OVN_NETWORK_NAME_EXT_ID_KEY:
'neutron-{}'.format(internal_net_id)}})
fake_router = fakes.FakeOvsdbRow.create_one_ovsdb_row(
attrs={
'ports': [ext_gw_lrp, internal_net_lrp]})
expected_new_ext_gw_lrp_ids = ext_gw_lrp.external_ids
expected_new_ext_gw_lrp_ids[constants.OVN_ROUTER_IS_EXT_GW] = 'True'
expected_new_internal_lrp_ids = internal_net_lrp.external_ids
expected_new_internal_lrp_ids[constants.OVN_ROUTER_IS_EXT_GW] = 'False'
self.fake_ovn_client._l3_plugin.get_routers.return_value = routers_db
nb_idl.get_lrouter.return_value = fake_router
self.assertRaises(
periodics.NeverAgain,
self.periodic.add_gw_port_info_to_logical_router_port)
nb_idl.update_lrouter_port.assert_has_calls([
mock.call(name=ext_gw_lrp.name,
external_ids=expected_new_ext_gw_lrp_ids),
mock.call(name=internal_net_lrp.name,
external_ids=expected_new_internal_lrp_ids)],
any_order=True)
def test_add_gw_port_info_to_logical_router_port_no_action_needed(self):
nb_idl = self.fake_ovn_client._nb_idl
ext_net_id = uuidutils.generate_uuid()
internal_net_id = uuidutils.generate_uuid()
routers_db = [{
'id': uuidutils.generate_uuid(),
'external_gateways': [{'network_id': ext_net_id}]}]
ext_gw_lrp = fakes.FakeOvsdbRow.create_one_ovsdb_row(
attrs={'external_ids': {
constants.OVN_NETWORK_NAME_EXT_ID_KEY:
'neutron-{}'.format(ext_net_id),
constants.OVN_ROUTER_IS_EXT_GW: 'True'}})
internal_net_lrp = fakes.FakeOvsdbRow.create_one_ovsdb_row(
attrs={'external_ids': {
constants.OVN_NETWORK_NAME_EXT_ID_KEY:
'neutron-{}'.format(internal_net_id),
constants.OVN_ROUTER_IS_EXT_GW: 'False'}})
fake_router = fakes.FakeOvsdbRow.create_one_ovsdb_row(
attrs={
'ports': [ext_gw_lrp, internal_net_lrp]})
self.fake_ovn_client._l3_plugin.get_routers.return_value = routers_db
nb_idl.get_lrouter.return_value = fake_router
self.assertRaises(
periodics.NeverAgain,
self.periodic.add_gw_port_info_to_logical_router_port)
nb_idl.update_lrouter_port.assert_not_called()
def test_check_router_default_route_empty_dst_ip(self):
nb_idl = self.fake_ovn_client._nb_idl
route0 = fakes.FakeOvsdbRow.create_one_ovsdb_row(

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

@ -100,7 +100,8 @@ class TestOVNL3RouterPlugin(test_mech_driver.Ml2PluginV2TestCase):
ovn_const.OVN_SUBNET_EXT_IDS_KEY: 'subnet-id',
ovn_const.OVN_REV_NUM_EXT_ID_KEY: '1',
ovn_const.OVN_NETWORK_NAME_EXT_ID_KEY:
utils.ovn_name(self.fake_network['id'])}}
utils.ovn_name(self.fake_network['id']),
ovn_const.OVN_ROUTER_IS_EXT_GW: 'False'}}
self.fake_router_ports = [self.fake_router_port]
self.fake_subnet = {'id': 'subnet-id',
'ip_version': 4,
@ -160,7 +161,8 @@ class TestOVNL3RouterPlugin(test_mech_driver.Ml2PluginV2TestCase):
ovn_const.OVN_SUBNET_EXT_IDS_KEY: 'ext-subnet-id',
ovn_const.OVN_REV_NUM_EXT_ID_KEY: '1',
ovn_const.OVN_NETWORK_NAME_EXT_ID_KEY:
utils.ovn_name('ext-network-id')},
utils.ovn_name('ext-network-id'),
ovn_const.OVN_ROUTER_IS_EXT_GW: 'True'},
'gateway_chassis': ['hv1'],
'options': {}}
self.fake_floating_ip_attrs = {'floating_ip_address': '192.168.0.10',
@ -450,7 +452,8 @@ class TestOVNL3RouterPlugin(test_mech_driver.Ml2PluginV2TestCase):
ovn_const.OVN_SUBNET_EXT_IDS_KEY: 'subnet-id',
ovn_const.OVN_REV_NUM_EXT_ID_KEY: '1',
ovn_const.OVN_NETWORK_NAME_EXT_ID_KEY:
utils.ovn_name(self.fake_network['id'])})
utils.ovn_name(self.fake_network['id']),
ovn_const.OVN_ROUTER_IS_EXT_GW: 'False'})
def test_remove_router_interface_router_not_found(self):
router_id = 'router-id'
@ -1951,6 +1954,8 @@ class TestOVNL3RouterPlugin(test_mech_driver.Ml2PluginV2TestCase):
fake_router_port_assert['options'] = {
ovn_const.OVN_ROUTER_PORT_GW_MTU_OPTION:
str(prov_net['mtu'])}
fake_router_port_assert['external_ids'][
ovn_const.OVN_ROUTER_IS_EXT_GW] = 'True'
self.l3_inst._nb_ovn.add_lrouter_port.assert_called_once_with(
**fake_router_port_assert)
@ -1998,6 +2003,8 @@ class TestOVNL3RouterPlugin(test_mech_driver.Ml2PluginV2TestCase):
fake_router_port_assert = self.fake_router_port_assert
fake_router_port_assert['gateway_chassis'] = mock.ANY
fake_router_port_assert['options'] = {}
fake_router_port_assert['external_ids'][
ovn_const.OVN_ROUTER_IS_EXT_GW] = 'True'
self.l3_inst._nb_ovn.add_lrouter_port.assert_called_once_with(
**fake_router_port_assert)

42
neutron/tests/unit/services/portforwarding/drivers/ovn/test_driver.py
View File

@ -450,6 +450,48 @@ class TestOVNPortForwardingHandler(TestOVNPortForwardingBase):
self.l3_plugin._nb_ovn.lb_del.assert_called_once_with(
exp_lb_name, exp_vip, if_exists=mock.ANY)
@mock.patch.object(port_forwarding.LOG, 'warning')
def test__validate_router_networks_provider_networks(self, mock_warning):
lr_ports = [
mock.MagicMock(external_ids={
ovn_const.OVN_NETWORK_NAME_EXT_ID_KEY: 'neutron-ext-net-id',
ovn_const.OVN_ROUTER_NAME_EXT_ID_KEY: 'True'}),
mock.MagicMock(external_ids={
ovn_const.OVN_NETWORK_NAME_EXT_ID_KEY: 'neutron-net-id',
ovn_const.OVN_ROUTER_NAME_EXT_ID_KEY: 'False'})]
lr_mock = mock.MagicMock(ports=lr_ports)
ovn_nb_mock = mock.Mock()
ovn_nb_mock.get_lrouter.return_value = lr_mock
with mock.patch.object(
ovn_conf, 'is_ovn_distributed_floating_ip',
return_value=True), mock.patch.object(
ovn_utils, 'is_provider_network',
return_value=True):
self.handler._validate_router_networks(
ovn_nb_mock, 'router-id')
self.assertEqual(1, mock_warning.call_count)
@mock.patch.object(port_forwarding.LOG, 'warning')
def test__validate_router_networks_tunnel_networks(self, mock_warning):
lr_ports = [
mock.MagicMock(external_ids={
ovn_const.OVN_NETWORK_NAME_EXT_ID_KEY: 'neutron-ext-net-id',
ovn_const.OVN_ROUTER_NAME_EXT_ID_KEY: 'True'}),
mock.MagicMock(external_ids={
ovn_const.OVN_NETWORK_NAME_EXT_ID_KEY: 'neutron-net-id',
ovn_const.OVN_ROUTER_NAME_EXT_ID_KEY: 'False'})]
lr_mock = mock.MagicMock(ports=lr_ports)
ovn_nb_mock = mock.Mock()
ovn_nb_mock.get_lrouter.return_value = lr_mock
with mock.patch.object(
ovn_conf, 'is_ovn_distributed_floating_ip',
return_value=True), mock.patch.object(
ovn_utils, 'is_provider_network',
return_value=False):
self.handler._validate_router_networks(
ovn_nb_mock, 'router-id')
mock_warning.assert_not_called()
class TestOVNPortForwarding(TestOVNPortForwardingBase):
def setUp(self):