Merge "[OVN] Warn about invalid OVN and FIP PF config during start of Neutron" into stable/2023.1

Author: Zuul

neutron/cmd/upgrade_checks/checks.py

@@ -27,6 +27,10 @@
 
 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
 from neutron.db.extra_dhcp_opt import models as extra_dhcp_opt_models
 from neutron.db.models import agent as agent_model
@@ -179,6 +183,8 @@ def get_checks(self):
              self.floatingip_inherit_qos_from_network),
             (_('Port extra DHCP options check'),
              self.extra_dhcp_options_check),
+            (_('Floating IP Port forwarding and OVN L3 plugin configuration'),
+             self.ovn_port_forwarding_configuration_check),
         ]
 
     @staticmethod
@@ -515,3 +521,28 @@ def extra_dhcp_options_check(checker):
             upgradecheck.Code.SUCCESS,
             _('There are no extra_dhcp_opts with the newline character '
               'in the option name or option value.'))
+
+    @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.'))

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.')

neutron/common/ovn/utils.py

@@ -1211,3 +1211,20 @@ def get_requested_chassis(requested_chassis):
 # becomes the norm and older versions of OVN are no longer supported
 def is_additional_chassis_supported(idl):
     return idl.is_col_present('Port_Binding', 'additional_chassis')
+
+
+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()

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 @@ def port_forwarding_deleted(self, ovn_txn, nb_ovn, pf_obj):
 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:

neutron/tests/unit/cmd/upgrade_checks/test_checks.py

@@ -18,6 +18,8 @@
 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
 
 
@@ -278,3 +280,25 @@ def test_extra_dhcp_options_check_bad_value(self):
                           opt_value='bar\nbar')]
             result = checks.CoreChecks.extra_dhcp_options_check(mock.ANY)
             self.assertEqual(Code.WARNING, result.code)
+
+    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()

neutron/tests/unit/common/ovn/test_utils.py

@@ -29,6 +29,7 @@
 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
@@ -1172,3 +1173,44 @@ def test_vnic_remote_managed_port_context(self):
             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)

neutron/tests/unit/services/portforwarding/drivers/ovn/test_driver.py

@@ -14,21 +14,25 @@
 
 from unittest import mock
 
+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 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
-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 oslo_utils import uuidutils
-from ovsdbapp import constants as ovsdbapp_const
 
 
 class TestOVNPortForwardingBase(base.BaseTestCase):
@@ -450,6 +454,7 @@ def test_port_forwarding_deleted(self, m_info):
 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 @@ def test_init(self):
         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())

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
+    <https://bugs.launchpad.net/neutron/+bug/2028846>`_.