diff --git a/neutron/cmd/upgrade_checks/checks.py b/neutron/cmd/upgrade_checks/checks.py index 140eea86539..4da9d89ca69 100644 --- a/neutron/cmd/upgrade_checks/checks.py +++ b/neutron/cmd/upgrade_checks/checks.py @@ -28,6 +28,8 @@ from sqlalchemy import or_ from neutron._i18n import _ from neutron.cmd.upgrade_checks import base +from neutron.common.ovn import exceptions as ovn_exc +from neutron.common.ovn import utils as ovn_utils from neutron.conf.plugins.ml2 import config as ml2_conf from neutron.conf.plugins.ml2.drivers.ovn import ovn_conf from neutron.conf import service as conf_service @@ -231,6 +233,8 @@ class CoreChecks(base.BaseChecks): self.ovn_for_bm_provisioning_over_ipv6_check), (_('ML2/OVS IGMP Flood check'), self.ml2_ovs_igmp_flood_check), + (_('Floating IP Port forwarding and OVN L3 plugin configuration'), + self.ovn_port_forwarding_configuration_check), ] @staticmethod @@ -669,3 +673,28 @@ class CoreChecks(base.BaseChecks): return upgradecheck.Result( upgradecheck.Code.SUCCESS, _('IGMP related traffic configuration is not affected.')) + + @staticmethod + def ovn_port_forwarding_configuration_check(checker): + ovn_l3_plugin_names = [ + 'ovn-router', + 'neutron.services.ovn_l3.plugin.OVNL3RouterPlugin'] + if not any(plugin in ovn_l3_plugin_names + for plugin in cfg.CONF.service_plugins): + return upgradecheck.Result( + upgradecheck.Code.SUCCESS, _('No OVN L3 plugin enabled.')) + + ml2_conf.register_ml2_plugin_opts() + ovn_conf.register_opts() + try: + ovn_utils.validate_port_forwarding_configuration() + return upgradecheck.Result( + upgradecheck.Code.SUCCESS, + _('OVN L3 plugin and Port Forwarding configuration are fine.')) + except ovn_exc.InvalidPortForwardingConfiguration: + return upgradecheck.Result( + upgradecheck.Code.WARNING, + _('Neutron configuration is invalid. Port forwardings ' + 'can not be used with ML2/OVN backend, distributed ' + 'floating IPs and provider network type(s) used as ' + 'tenant networks.')) diff --git a/neutron/common/ovn/exceptions.py b/neutron/common/ovn/exceptions.py index b17d0748cc4..9ad078f4a49 100644 --- a/neutron/common/ovn/exceptions.py +++ b/neutron/common/ovn/exceptions.py @@ -37,3 +37,10 @@ class HashRingIsEmpty(n_exc.NeutronException): '%(node_count)d nodes were found offline. This should never ' 'happen in a normal situation, please check the status ' 'of your cluster') + + +class InvalidPortForwardingConfiguration(n_exc.NeutronException): + message = _('Neutron configuration is invalid. Port forwardings ' + 'can not be used with ML2/OVN backend, distributed ' + 'floating IPs and provider network type(s) used as ' + 'tenant networks.') diff --git a/neutron/common/ovn/utils.py b/neutron/common/ovn/utils.py index 034aa18987d..74f1b9a6111 100644 --- a/neutron/common/ovn/utils.py +++ b/neutron/common/ovn/utils.py @@ -1246,3 +1246,20 @@ def is_nat_gateway_port_supported(idl): def is_ovn_provider_router(router): flavor_id = router.get('flavor_id') return flavor_id is None or flavor_id is const.ATTR_NOT_SPECIFIED + + +def validate_port_forwarding_configuration(): + if not ovn_conf.is_ovn_distributed_floating_ip(): + return + + pf_plugin_names = [ + 'port_forwarding', + 'neutron.services.portforwarding.pf_plugin.PortForwardingPlugin'] + if not any(plugin in pf_plugin_names + for plugin in cfg.CONF.service_plugins): + return + + provider_network_types = ['vlan', 'flat'] + if any(net_type in provider_network_types + for net_type in cfg.CONF.ml2.tenant_network_types): + raise ovn_exc.InvalidPortForwardingConfiguration() diff --git a/neutron/services/portforwarding/drivers/ovn/driver.py b/neutron/services/portforwarding/drivers/ovn/driver.py index 64ac97d92be..e5942148bd8 100644 --- a/neutron/services/portforwarding/drivers/ovn/driver.py +++ b/neutron/services/portforwarding/drivers/ovn/driver.py @@ -10,19 +10,18 @@ # License for the specific language governing permissions and limitations # under the License. -from oslo_log import log - -from ovsdbapp.backend.ovs_idl import idlutils -from ovsdbapp import constants as ovsdbapp_const - from neutron_lib.callbacks import events from neutron_lib.callbacks import registry from neutron_lib.callbacks import resources 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 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.db import ovn_revision_numbers_db as db_rev from neutron import manager @@ -192,10 +191,27 @@ class OVNPortForwardingHandler(object): class OVNPortForwarding(object): def __init__(self, l3_plugin): + self._validate_configuration() self._l3_plugin = l3_plugin self._pf_plugin_property = None self._handler = OVNPortForwardingHandler() + def _validate_configuration(self): + """This method checks if Neutron config is compatible with OVN and PFs. + + It stops process in case when provider network types (vlan/flat) + are enabled as tenant networks AND distributed floating IPs are enabled + as this configuration is not working fine with FIP PFs in ML2/OVN case. + """ + try: + ovn_utils.validate_port_forwarding_configuration() + except ovn_exc.InvalidPortForwardingConfiguration: + LOG.warning("Neutron configuration is invalid for port " + "forwardings and ML2/OVN backend. " + "It is not valid to use together provider network " + "types (vlan/flat) as tenant networks, distributed " + "floating IPs and port forwardings.") + @property def _pf_plugin(self): if self._pf_plugin_property is None: diff --git a/neutron/tests/unit/cmd/upgrade_checks/test_checks.py b/neutron/tests/unit/cmd/upgrade_checks/test_checks.py index 1be1529144d..c769bad88f9 100644 --- a/neutron/tests/unit/cmd/upgrade_checks/test_checks.py +++ b/neutron/tests/unit/cmd/upgrade_checks/test_checks.py @@ -18,6 +18,8 @@ from oslo_config import cfg from oslo_upgradecheck.upgradecheck import Code from neutron.cmd.upgrade_checks import checks +from neutron.common.ovn import exceptions as ovn_exc +from neutron.common.ovn import utils as ovn_utils from neutron.tests import base @@ -350,3 +352,35 @@ class TestChecks(base.BaseTestCase): mock.ANY) self.assertEqual(Code.WARNING, result.code) mock_get_ovn_client.assert_called_once_with() + + def test_ovn_port_forwarding_configuration_check_no_ovn_l3_router(self): + cfg.CONF.set_override("service_plugins", 'router,some-other-plugin') + with mock.patch.object( + ovn_utils, + 'validate_port_forwarding_configuration') as validate_mock: + result = checks.CoreChecks.ovn_port_forwarding_configuration_check( + mock.ANY) + self.assertEqual(Code.SUCCESS, result.code) + validate_mock.assert_not_called() + + def test_ovn_port_forwarding_configuration_check_ovn_l3_success(self): + cfg.CONF.set_override("service_plugins", 'ovn-router') + with mock.patch.object( + ovn_utils, + 'validate_port_forwarding_configuration') as validate_mock: + result = checks.CoreChecks.ovn_port_forwarding_configuration_check( + mock.ANY) + self.assertEqual(Code.SUCCESS, result.code) + validate_mock.assert_called_once_with() + + def test_ovn_port_forwarding_configuration_check_ovn_l3_failure(self): + cfg.CONF.set_override("service_plugins", 'ovn-router') + with mock.patch.object( + ovn_utils, + 'validate_port_forwarding_configuration', + side_effect=ovn_exc.InvalidPortForwardingConfiguration + ) as validate_mock: + result = checks.CoreChecks.ovn_port_forwarding_configuration_check( + mock.ANY) + self.assertEqual(Code.WARNING, result.code) + validate_mock.assert_called_once_with() diff --git a/neutron/tests/unit/common/ovn/test_utils.py b/neutron/tests/unit/common/ovn/test_utils.py index aa18b8dbe63..b38e177ebe0 100644 --- a/neutron/tests/unit/common/ovn/test_utils.py +++ b/neutron/tests/unit/common/ovn/test_utils.py @@ -29,6 +29,7 @@ from oslo_config import cfg import testtools from neutron.common.ovn import constants +from neutron.common.ovn import exceptions as ovn_exc from neutron.common.ovn import utils from neutron.conf.plugins.ml2.drivers.ovn import ovn_conf from neutron.tests import base @@ -1217,3 +1218,44 @@ class DetermineBindHostTestCase(base.BaseTestCase): self.fake_smartnic_hostname, utils.determine_bind_host(self.mock_sb_idl, {}, port_context=context)) + + +class ValidatePortForwardingConfigurationTestCase(base.BaseTestCase): + + def setUp(self): + super().setUp() + ovn_conf.register_opts() + + def test_validation_when_distributed_fip_disabled(self): + cfg.CONF.set_override( + 'enable_distributed_floating_ip', False, group='ovn') + cfg.CONF.set_override('service_plugins', 'some_plugin,port_forwarding') + cfg.CONF.set_override('tenant_network_types', 'geneve,vlan', + group='ml2') + utils.validate_port_forwarding_configuration() + + def test_validation_when_no_pf_plugin_enabled(self): + cfg.CONF.set_override( + 'enable_distributed_floating_ip', True, group='ovn') + cfg.CONF.set_override('service_plugins', 'some_plugin') + cfg.CONF.set_override('tenant_network_types', 'geneve,vlan', + group='ml2') + utils.validate_port_forwarding_configuration() + + def test_validation_when_no_provider_net_configured(self): + cfg.CONF.set_override( + 'enable_distributed_floating_ip', True, group='ovn') + cfg.CONF.set_override('service_plugins', 'some_plugin,port_forwarding') + cfg.CONF.set_override('tenant_network_types', 'geneve,vxlan', + group='ml2') + utils.validate_port_forwarding_configuration() + + def test_validation_when_pf_and_provider_net_enabled(self): + cfg.CONF.set_override( + 'enable_distributed_floating_ip', True, group='ovn') + cfg.CONF.set_override('service_plugins', 'some_plugin,port_forwarding') + cfg.CONF.set_override('tenant_network_types', 'geneve,vlan', + group='ml2') + self.assertRaises( + ovn_exc.InvalidPortForwardingConfiguration, + utils.validate_port_forwarding_configuration) diff --git a/neutron/tests/unit/services/portforwarding/drivers/ovn/test_driver.py b/neutron/tests/unit/services/portforwarding/drivers/ovn/test_driver.py index 178475d0212..c770b868583 100644 --- a/neutron/tests/unit/services/portforwarding/drivers/ovn/test_driver.py +++ b/neutron/tests/unit/services/portforwarding/drivers/ovn/test_driver.py @@ -14,14 +14,6 @@ from unittest import mock -from neutron.common.ovn import constants as ovn_const -from neutron.objects import port_forwarding as port_forwarding_obj -from neutron.services.portforwarding.constants import PORT_FORWARDING -from neutron.services.portforwarding.constants import PORT_FORWARDING_PLUGIN -from neutron.services.portforwarding.drivers.ovn import driver \ - as port_forwarding -from neutron.tests import base -from neutron.tests.unit import fake_resources from neutron_lib.callbacks import events from neutron_lib.callbacks import registry from neutron_lib.callbacks import resources @@ -30,6 +22,18 @@ from neutron_lib.plugins import constants as plugin_constants from oslo_utils import uuidutils 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.objects import port_forwarding as port_forwarding_obj +from neutron.services.portforwarding.constants import PORT_FORWARDING +from neutron.services.portforwarding.constants import PORT_FORWARDING_PLUGIN +from neutron.services.portforwarding.drivers.ovn import driver \ + as port_forwarding +from neutron.tests import base +from neutron.tests.unit import fake_resources + class TestOVNPortForwardingBase(base.BaseTestCase): def setUp(self): @@ -450,6 +454,7 @@ class TestOVNPortForwardingHandler(TestOVNPortForwardingBase): class TestOVNPortForwarding(TestOVNPortForwardingBase): def setUp(self): super(TestOVNPortForwarding, self).setUp() + ovn_conf.register_opts() self.pf_plugin = mock.Mock() self.handler = mock.Mock() get_mock_pf_plugin = lambda alias: self.pf_plugin if ( @@ -475,6 +480,25 @@ class TestOVNPortForwarding(TestOVNPortForwardingBase): self.assertEqual(self._ovn_pf._handler, self.handler) self.assertEqual(self._ovn_pf._pf_plugin, self.pf_plugin) + def test__validate_configuration_ok(self): + with mock.patch.object( + port_forwarding.LOG, "warning") as mock_warning, \ + mock.patch.object(ovn_utils, + "validate_port_forwarding_configuration"): + + self._ovn_pf._validate_configuration() + mock_warning.assert_not_called() + + def test__validate_configuration_wrong(self): + with mock.patch.object( + port_forwarding.LOG, "warning") as mock_warning, \ + mock.patch.object( + ovn_utils, + "validate_port_forwarding_configuration", + side_effect=ovn_exc.InvalidPortForwardingConfiguration): + self._ovn_pf._validate_configuration() + mock_warning.assert_called_once_with(mock.ANY) + def test_register(self): with mock.patch.object(registry, 'subscribe') as mock_subscribe: self._ovn_pf.register(mock.ANY, mock.ANY, mock.Mock()) diff --git a/releasenotes/notes/OVN-L3-with-FIP-PF-require-centralized-FIPs-65864dfeb3edc9b1.yaml b/releasenotes/notes/OVN-L3-with-FIP-PF-require-centralized-FIPs-65864dfeb3edc9b1.yaml new file mode 100644 index 00000000000..152e5ad143f --- /dev/null +++ b/releasenotes/notes/OVN-L3-with-FIP-PF-require-centralized-FIPs-65864dfeb3edc9b1.yaml @@ -0,0 +1,17 @@ +--- +other: + - | + When the following configuration is enabled at the same time: + + * OVN L3 service plugin (``ovn-router``) + * Port forwarding service plugin (``port_forwarding``) + * "vlan" or "flat" network types configured in the ML2 configuration + variable ``tenant_network_types`` + * The OVN floating IP traffic is distributed + (``enable_distributed_floating_ip`` = ``True``) + + the Neutron server will report a warning during plugin initialization + because this is an invalid configuration matrix. Floating IPs need to + always be centralized in such a case. + For more details see `bug report + `_.