Merge pull request #123 from stackhpc/upstream/2023.1-2024-02-19

Synchronise 2023.1 with upstream

Author: mnasiadka

doc/source/ovn/gaps.rst

@@ -56,6 +56,17 @@ at [1]_.
 
   The NDP proxy functionality for IPv6 addresses is not supported by OVN.
 
+* Floating IP Port Forwarding in provider networks and with distributed routing
+
+  Currently, when provider network types like ``vlan`` or ``flat`` are plugged
+  to a router as internal networks while the ``enable_distributed_floating_ip``
+  configuration option is enabled, Floating IP port forwardings
+  which are using such router will not work properly.
+  Due to an incompatible setting of the router to make traffic in the vlan/flat
+  networks to be distributed but port forwardings are always centralized in
+  ML2/OVN backend.
+  This is being reported in [9]_.
+
 References
 ----------
 
@@ -67,3 +78,4 @@ References
 .. [6] https://bugs.launchpad.net/neutron/+bug/1926515
 .. [7] https://review.opendev.org/c/openstack/neutron/+/788594
 .. [8] https://docs.openstack.org/neutron/latest/admin/config-dns-res.html
+.. [9] https://bugs.launchpad.net/neutron/+bug/2028846

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/extensions.py

@@ -73,6 +73,7 @@
 from neutron_lib.api.definitions import stateful_security_group
 from neutron_lib.api.definitions import subnet_dns_publish_fixed_ip
 from neutron_lib.api.definitions import subnet_service_types
+from neutron_lib.api.definitions import subnetpool_prefix_ops
 from neutron_lib.api.definitions import trunk
 from neutron_lib.api.definitions import uplink_status_propagation
 from neutron_lib.api.definitions import vlantransparent
@@ -156,6 +157,7 @@
     constants.SUBNET_ALLOCATION_EXT_ALIAS,
     'standard-attr-tag',
     'standard-attr-timestamp',
+    subnetpool_prefix_ops.ALIAS,
     subnet_service_types.ALIAS,
     trunk.ALIAS,
     seg_def.ALIAS,

neutron/common/ovn/utils.py

@@ -679,6 +679,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)
 
 
@@ -1211,3 +1220,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/db/models/securitygroup.py

@@ -101,9 +101,13 @@ class SecurityGroupRule(standard_attr.HasStandardAttributes, model_base.BASEV2,
     port_range_min = sa.Column(sa.Integer)
     port_range_max = sa.Column(sa.Integer)
     remote_ip_prefix = sa.Column(sa.String(255))
+    # NOTE(ralonsoh): loading method is temporarily changed to "joined" until
+    # a proper way to only load the security groups "shared" field, without
+    # loading the rest of the synthetic fields, is implemented. LP#2052419
+    # description for more information and context.
     security_group = orm.relationship(
         SecurityGroup, load_on_pending=True,
-        backref=orm.backref('rules', cascade='all,delete', lazy='dynamic'),
+        backref=orm.backref('rules', cascade='all,delete', lazy='joined'),
         primaryjoin="SecurityGroup.id==SecurityGroupRule.security_group_id")
     source_group = orm.relationship(
         SecurityGroup,

neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/maintenance.py

@@ -971,6 +971,41 @@ def create_router_extra_attributes_registers(self):
 
         raise periodics.NeverAgain()
 
+    # TODO(slaweq): Remove this in the E cycle (C+2 as it will be next SLURP)
+    @periodics.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()
+
     @periodics.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).

neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_client.py

@@ -1537,7 +1537,10 @@ def _gen_router_port_ext_ids(self, port):
             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:

neutron/services/portforwarding/drivers/ovn/driver.py

@@ -10,20 +10,21 @@
 #    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 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
@@ -130,7 +131,41 @@ def _port_forwarding_created(self, ovn_txn, nb_ovn, pf_obj,
                                 "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:
@@ -192,10 +227,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()