diff --git a/doc/source/ovn/gaps.rst b/doc/source/ovn/gaps.rst index 6a27429fc4b..57b90075e7c 100644 --- a/doc/source/ovn/gaps.rst +++ b/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 diff --git a/neutron/cmd/upgrade_checks/checks.py b/neutron/cmd/upgrade_checks/checks.py index 28b2f03c1e4..127f02726ae 100644 --- a/neutron/cmd/upgrade_checks/checks.py +++ b/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.')) 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/extensions.py b/neutron/common/ovn/extensions.py index 3151d7862ff..7bb510ed361 100644 --- a/neutron/common/ovn/extensions.py +++ b/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, diff --git a/neutron/common/ovn/utils.py b/neutron/common/ovn/utils.py index 8a6959377e3..d90655da457 100644 --- a/neutron/common/ovn/utils.py +++ b/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() diff --git a/neutron/db/models/securitygroup.py b/neutron/db/models/securitygroup.py index c7b516c2634..9d777d87780 100644 --- a/neutron/db/models/securitygroup.py +++ b/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, diff --git a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/maintenance.py b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/maintenance.py index 6088d44e1fc..56455d44c5e 100644 --- a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/maintenance.py +++ b/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). diff --git a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_client.py b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_client.py index c5b932a6bb6..20b7ad3b2c0 100644 --- a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_client.py +++ b/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: diff --git a/neutron/services/portforwarding/drivers/ovn/driver.py b/neutron/services/portforwarding/drivers/ovn/driver.py index 64ac97d92be..02c4cca5cb9 100644 --- a/neutron/services/portforwarding/drivers/ovn/driver.py +++ b/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: diff --git a/neutron/tests/unit/cmd/upgrade_checks/test_checks.py b/neutron/tests/unit/cmd/upgrade_checks/test_checks.py index 69dc4a1d9c2..6fa46c86060 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_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() diff --git a/neutron/tests/unit/common/ovn/test_utils.py b/neutron/tests/unit/common/ovn/test_utils.py index 03dbe576f55..3ccb04fe61d 100644 --- a/neutron/tests/unit/common/ovn/test_utils.py +++ b/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) diff --git a/neutron/tests/unit/db/test_securitygroups_db.py b/neutron/tests/unit/db/test_securitygroups_db.py index 593272027b7..50b11287d41 100644 --- a/neutron/tests/unit/db/test_securitygroups_db.py +++ b/neutron/tests/unit/db/test_securitygroups_db.py @@ -347,6 +347,10 @@ def _test_security_group_precommit_create_event(self, # Especially we want to check the revision number here. sg_dict_got = self.mixin.get_security_group( self.ctx, sg_dict['id']) + # Order the SG rules to avoid issues in the assertion. + for _sg_dict in (sg_dict, sg_dict_got): + _sg_dict['security_group_rules'] = sorted( + _sg_dict['security_group_rules'], key=lambda d: d['id']) self.assertEqual(sg_dict, sg_dict_got) def test_security_group_precommit_create_event_with_revisions(self): diff --git a/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_maintenance.py b/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_maintenance.py index 73a1d87f7d0..3b307d81250 100644 --- a/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_maintenance.py +++ b/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_maintenance.py @@ -937,6 +937,70 @@ def test_update_port_virtual_type(self, *args): ('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( diff --git a/neutron/tests/unit/services/ovn_l3/test_plugin.py b/neutron/tests/unit/services/ovn_l3/test_plugin.py index ac99441f5b8..0fe02caddca 100644 --- a/neutron/tests/unit/services/ovn_l3/test_plugin.py +++ b/neutron/tests/unit/services/ovn_l3/test_plugin.py @@ -93,7 +93,8 @@ def setUp(self): 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, @@ -149,7 +150,8 @@ def setUp(self): 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(self.fake_network['id'])}, + utils.ovn_name(self.fake_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', @@ -404,7 +406,8 @@ def test_remove_router_interface_update_lrouter_port(self): 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' @@ -1631,6 +1634,8 @@ def test_add_router_interface_need_to_frag_enabled( 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) @@ -1678,6 +1683,8 @@ def test_add_router_interface_need_to_frag_enabled_then_remove( 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) 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..ed936c63977 100644 --- a/neutron/tests/unit/services/portforwarding/drivers/ovn/test_driver.py +++ b/neutron/tests/unit/services/portforwarding/drivers/ovn/test_driver.py @@ -14,7 +14,18 @@ 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 @@ -22,13 +33,6 @@ 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): @@ -446,10 +450,53 @@ def test_port_forwarding_deleted(self, m_info): 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): 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 +522,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()) 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 + `_. diff --git a/releasenotes/notes/ovn-add-extension-subnetpool-prefix-ops-9b2e4dbdcc174ede.yaml b/releasenotes/notes/ovn-add-extension-subnetpool-prefix-ops-9b2e4dbdcc174ede.yaml new file mode 100644 index 00000000000..fc61ee893bc --- /dev/null +++ b/releasenotes/notes/ovn-add-extension-subnetpool-prefix-ops-9b2e4dbdcc174ede.yaml @@ -0,0 +1,3 @@ +other: + - | + Added extension ``subnetpool-prefix-ops`` to the ML2/OVN mechanism driver. diff --git a/zuul.d/grenade.yaml b/zuul.d/grenade.yaml index b3ec14418d8..9a30c984734 100644 --- a/zuul.d/grenade.yaml +++ b/zuul.d/grenade.yaml @@ -374,7 +374,7 @@ # Move this forward when master changes to a new skip-level-allowed # target release. Right now, this is xena because master is zed. # When master is A, this should become yoga, and so forth. - grenade_from_branch: stable/yoga + grenade_from_branch: unmaintained/yoga grenade_localrc: NOVA_ENABLE_UPGRADE_WORKAROUND: True @@ -388,6 +388,6 @@ # Move this forward when master changes to a new skip-level-allowed # target release. Right now, this is xena because master is zed. # When master is A, this should become yoga, and so forth. - grenade_from_branch: stable/yoga + grenade_from_branch: unmaintained/yoga grenade_localrc: NOVA_ENABLE_UPGRADE_WORKAROUND: True