openstack
/
neutron
Code Issues Proposed changes

[ML2/OVN] Add gateway_port support for FIP 

The OVN changed support for NAT rules including a new column and auto discovery logic (which may not work in some cases) [1][2].
If the OVN backend supports this column in the Northbound DB Schema, set gateway port uuid to any floating IP to prevent North/South traffic issues for floating IPs.

This patch updates the method for creating FIP NAT rules in OVN backend and updates previously created FIP rules to include the gateway_port reference. This NAT rule update task runs only once during the maintenance task, and if all entries are already configured no action is performed.

[1] 15348b7b80
[2] 2d942be7db

Closes-Bug: 2035281
Change-Id: I802b6bd8c281cb6dacdee2e9c15285f069d4e04c
This commit is contained in:
Roberto Bartzen Acosta 2023-09-15 08:44:52 -03:00
parent 6514e37e47
commit 78b5fe2ff4
11 changed files with 399 additions and 11 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -1216,3 +1216,7 @@ 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 is_nat_gateway_port_supported(idl):
return idl.is_col_present('NAT', 'gateway_port')

4
neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/impl_idl_ovn.py
View File

@ -361,6 +361,10 @@ class OvsdbNbOvnIdl(nb_impl_idl.OvnNbApiIdlImpl, Backend):
columns['external_mac'] = nat.external_mac[0]
if nat.logical_port:
columns['logical_port'] = nat.logical_port[0]
columns['external_ids'] = nat.external_ids
columns['uuid'] = nat.uuid
if utils.is_nat_gateway_port_supported(self):
columns['gateway_port'] = nat.gateway_port
dnat_and_snats.append(columns)
elif nat.type == 'snat':
snat.append(columns)

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

@ -1148,6 +1148,52 @@ class DBInconsistenciesPeriodics(SchemaAwarePeriodicsBase):
raise periodics.NeverAgain()
@has_lock_periodic(spacing=600, run_immediately=True)
def update_nat_floating_ip_with_gateway_port_reference(self):
"""Set NAT rule gateway_port column to any floating IP without
router gateway port uuid reference - LP#2035281.
"""
if not utils.is_nat_gateway_port_supported(self._nb_idl):
raise periodics.NeverAgain()
context = n_context.get_admin_context()
fip_update = []
lrouters = self._nb_idl.get_all_logical_routers_with_rports()
for router in lrouters:
ovn_fips = router['dnat_and_snats']
for ovn_fip in ovn_fips:
# Skip FIPs that are already configured with gateway_port
if ovn_fip['gateway_port']:
continue
fip_id = ovn_fip['external_ids'].get(
ovn_const.OVN_FIP_EXT_ID_KEY)
if fip_id:
fip_update.append({'uuid': ovn_fip['uuid'],
'router_id': router['name']})
# Simple caching mechanism to avoid unnecessary DB calls
gw_port_id_cache = {}
lrp_cache = {}
cmds = []
for fip in fip_update:
lrouter = utils.ovn_name(fip['router_id'])
if lrouter not in gw_port_id_cache.keys():
router_db = self._ovn_client._l3_plugin.get_router(context,
fip['router_id'], fields=['gw_port_id'])
gw_port_id_cache[lrouter] = router_db.get('gw_port_id')
lrp_cache[lrouter] = self._nb_idl.get_lrouter_port(
gw_port_id_cache[lrouter])
columns = {'gateway_port': lrp_cache[lrouter].uuid}
cmds.append(self._nb_idl.set_nat_rule_in_lrouter(lrouter,
fip['uuid'], **columns))
if cmds:
with self._nb_idl.transaction(check_error=True) as txn:
for cmd in cmds:
txn.add(cmd)
raise periodics.NeverAgain()
class HashRingHealthCheckPeriodics(object):

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

@ -847,6 +847,14 @@ class OVNClient(object):
'logical_port': floatingip['port_id'],
'external_ids': ext_ids}
# If OVN supports gateway_port column for NAT rules set gateway port
# uuid to any floating IP without gw port reference - LP#2035281.
if utils.is_nat_gateway_port_supported(self._nb_idl):
router_db = self._l3_plugin.get_router(admin_context, router_id)
gw_port_id = router_db.get('gw_port_id')
lrp = self._nb_idl.get_lrouter_port(gw_port_id)
columns['gateway_port'] = lrp.uuid
if ovn_conf.is_ovn_distributed_floating_ip():
if self._nb_idl.lsp_get_up(floatingip['port_id']).execute():
columns['external_mac'] = port_db['mac_address']

59
neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_maintenance.py
View File

@ -1092,6 +1092,65 @@ class TestMaintenance(_TestMaintenanceHelper):
check_error=True, log_errors=True)
self.assertTrue(options_intersect(new_nb_global_options, nb_options))
def test_floating_ip_with_gateway_port(self):
ext_net = self._create_network('ext_networktest', external=True)
ext_subnet = self._create_subnet(
'ext_subnettest',
ext_net['id'],
**{'cidr': '100.0.0.0/24',
'gateway_ip': '100.0.0.254',
'allocation_pools': [
{'start': '100.0.0.2', 'end': '100.0.0.253'}],
'enable_dhcp': False})
net1 = self._create_network('network1test', external=False)
subnet1 = self._create_subnet('subnet1test', net1['id'])
external_gateway_info = {
'enable_snat': True,
'network_id': ext_net['id'],
'external_fixed_ips': [
{'ip_address': '100.0.0.2', 'subnet_id': ext_subnet['id']}]}
router = self._create_router(
'routertest', external_gateway_info=external_gateway_info)
self._add_router_interface(router['id'], subnet1['id'])
p1 = self._create_port('testp1', net1['id'])
logical_ip = p1['fixed_ips'][0]['ip_address']
fip_info = {'floatingip': {
'tenant_id': self._tenant_id,
'description': 'test_fip',
'floating_network_id': ext_net['id'],
'port_id': p1['id'],
'fixed_ip_address': logical_ip}}
# Create floating IP without gateway_port
with mock.patch.object(utils,
'is_nat_gateway_port_supported', return_value=False):
fip = self.l3_plugin.create_floatingip(self.context, fip_info)
self.assertEqual(router['id'], fip['router_id'])
self.assertEqual('testp1', fip['port_details']['name'])
self.assertIsNotNone(self.nb_api.get_lswitch_port(fip['port_id']))
rules = self.nb_api.get_all_logical_routers_with_rports()[0]
fip_rule = rules['dnat_and_snats'][0]
if utils.is_nat_gateway_port_supported(self.nb_api):
self.assertEqual([], fip_rule['gateway_port'])
else:
self.assertNotIn('gateway_port', fip_rule)
# Call the maintenance task and check that the value has been
# updated in the NAT rule
self.assertRaises(periodics.NeverAgain,
self.maint.update_nat_floating_ip_with_gateway_port_reference)
rules = self.nb_api.get_all_logical_routers_with_rports()[0]
fip_rule = rules['dnat_and_snats'][0]
if utils.is_nat_gateway_port_supported(self.nb_api):
self.assertNotEqual([], fip_rule['gateway_port'])
else:
self.assertNotIn('gateway_port', fip_rule)
class TestLogMaintenance(_TestMaintenanceHelper,
test_log_driver.LogApiTestCaseBase):

102
neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/test_mech_driver.py
View File

@ -1254,3 +1254,105 @@ class TestAgentApi(base.TestOVNFunctionalBase):
self.plugin.delete_agent(self.context, metadata_id)
self.assertRaises(agent_exc.AgentNotFound, self.plugin.get_agent,
self.context, metadata_id)
class TestNATRuleGatewayPort(base.TestOVNFunctionalBase):
def setUp(self):
super().setUp()
self._ovn_client = self.mech_driver._ovn_client
def deserialize(self, content_type, response):
ctype = 'application/%s' % content_type
data = self._deserializers[ctype].deserialize(response.body)['body']
return data
def _create_router(self, name, external_gateway_info=None):
data = {'router': {'name': name, 'tenant_id': self._tenant_id,
'external_gateway_info': external_gateway_info}}
as_admin = bool(external_gateway_info.get('enable_snat'))
req = self.new_create_request('routers', data, self.fmt,
as_admin=as_admin)
res = req.get_response(self.api)
return self.deserialize(self.fmt, res)['router']
def _process_router_interface(self, action, router_id, subnet_id):
req = self.new_action_request(
'routers', {'subnet_id': subnet_id}, router_id,
'%s_router_interface' % action)
res = req.get_response(self.api)
return self.deserialize(self.fmt, res)
def _add_router_interface(self, router_id, subnet_id):
return self._process_router_interface('add', router_id, subnet_id)
def _create_port(self, name, net_id, security_groups=None,
device_owner=None):
data = {'port': {'name': name,
'tenant_id': self._tenant_id,
'network_id': net_id}}
if security_groups is not None:
data['port']['security_groups'] = security_groups
if device_owner is not None:
data['port']['device_owner'] = device_owner
req = self.new_create_request('ports', data, self.fmt)
res = req.get_response(self.api)
return self.deserialize(self.fmt, res)['port']
def test_create_floatingip(self):
ext_net = self._make_network(
self.fmt, 'ext_networktest', True, as_admin=True,
arg_list=('router:external',
'provider:network_type',
'provider:physical_network'),
**{'router:external': True,
'provider:network_type': 'flat',
'provider:physical_network': 'public'})['network']
res = self._create_subnet(self.fmt, ext_net['id'],
'100.0.0.0/24', gateway_ip='100.0.0.254',
allocation_pools=[{'start': '100.0.0.2',
'end': '100.0.0.253'}],
enable_dhcp=False)
ext_subnet = self.deserialize(self.fmt, res)['subnet']
net1 = self._make_network(
self.fmt, 'network1test', True)['network']
res = self._create_subnet(self.fmt, net1['id'],
'192.168.0.0/24', gateway_ip='192.168.0.1',
allocation_pools=[{'start': '192.168.0.2',
'end': '192.168.0.253'}],
enable_dhcp=False)
subnet1 = self.deserialize(self.fmt, res)['subnet']
external_gateway_info = {
'enable_snat': True,
'network_id': ext_net['id'],
'external_fixed_ips': [
{'ip_address': '100.0.0.2', 'subnet_id': ext_subnet['id']}]}
router = self._create_router(
'routertest', external_gateway_info=external_gateway_info)
self._add_router_interface(router['id'], subnet1['id'])
p1 = self._create_port('testp1', net1['id'])
logical_ip = p1['fixed_ips'][0]['ip_address']
fip_info = {'floatingip': {
'tenant_id': self._tenant_id,
'description': 'test_fip',
'floating_network_id': ext_net['id'],
'port_id': p1['id'],
'fixed_ip_address': logical_ip}}
fip = self.l3_plugin.create_floatingip(self.context, fip_info)
self.assertEqual(router['id'], fip['router_id'])
self.assertEqual('testp1', fip['port_details']['name'])
self.assertIsNotNone(self.nb_api.get_lswitch_port(fip['port_id']))
rules = self.nb_api.get_all_logical_routers_with_rports()[0]
fip_rule = rules['dnat_and_snats'][0]
if utils.is_nat_gateway_port_supported(self.nb_api):
self.assertNotEqual([], fip_rule['gateway_port'])
else:
self.assertNotIn('gateway_port', fip_rule)

1
neutron/tests/unit/fake_resources.py
View File

@ -166,6 +166,7 @@ class FakeOvsdbNbOvnIdl(object):
self.get_lrouter_gw_ports = mock.Mock()
self.lrp_get = mock.Mock()
self.get_schema_version = mock.Mock(return_value='3.6.0')
self.get_lrouter_port = mock.Mock()
class FakeOvsdbSbOvnIdl(object):

52
neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_impl_idl_ovn.py
View File

@ -15,6 +15,7 @@ import collections
import copy
from unittest import mock
from oslo_utils import uuidutils
from ovsdbapp.backend import ovs_idl
from neutron.common.ovn import constants as ovn_const
@ -208,11 +209,15 @@ class TestNBImplIdlOvn(TestDBImplIdlOvn):
'type': 'snat'},
{'external_ip': '20.0.2.4', 'logical_ip': '10.0.0.4',
'type': 'dnat_and_snat', 'external_mac': [],
'logical_port': []},
'logical_port': [],
'external_ids': {ovn_const.OVN_FIP_EXT_ID_KEY: 'fip_id_a'},
'gateway_port': uuidutils.generate_uuid()},
{'external_ip': '20.0.2.5', 'logical_ip': '10.0.0.5',
'type': 'dnat_and_snat',
'external_mac': ['00:01:02:03:04:05'],
'logical_port': ['lsp-id-001']}],
'logical_port': ['lsp-id-001'],
'external_ids': {ovn_const.OVN_FIP_EXT_ID_KEY: 'fip_id_b'},
'gateway_port': []}],
'acls': [
{'unit_test_id': 1,
'action': 'allow-related', 'direction': 'from-lport',
@ -475,13 +480,39 @@ class TestNBImplIdlOvn(TestDBImplIdlOvn):
'provnet_ports': []}]
self.assertCountEqual(mapping, expected)
def test_get_all_logical_routers_with_rports(self):
def _test_get_all_logical_routers_with_rports(self, is_gw_port):
# Test empty
mapping = self.nb_ovn_idl.get_all_logical_switches_with_ports()
self.assertCountEqual(mapping, {})
# Test loaded values
self._load_nb_db()
# Test with gateway_port_support enabled
utils.is_nat_gateway_port_supported = mock.Mock()
utils.is_nat_gateway_port_supported.return_value = is_gw_port
mapping = self.nb_ovn_idl.get_all_logical_routers_with_rports()
lra_nat = self._find_ovsdb_fake_row(self.nat_table,
'external_ip', '20.0.2.4')
lrb_nat = self._find_ovsdb_fake_row(self.nat_table,
'external_ip', '20.0.2.5')
lra_fip = {'external_ip': '20.0.2.4',
'logical_ip': '10.0.0.4',
'type': 'dnat_and_snat',
'external_ids': {ovn_const.OVN_FIP_EXT_ID_KEY: 'fip_id_a'},
'uuid': lra_nat.uuid}
lrb_fip = {'external_ip': '20.0.2.5',
'logical_ip': '10.0.0.5',
'type': 'dnat_and_snat',
'external_mac': '00:01:02:03:04:05',
'logical_port': 'lsp-id-001',
'external_ids': {ovn_const.OVN_FIP_EXT_ID_KEY: 'fip_id_b'},
'uuid': lrb_nat.uuid}
if is_gw_port:
lra_fip['gateway_port'] = lra_nat.gateway_port
lrb_fip['gateway_port'] = lrb_nat.gateway_port
expected = [{'name': 'lr-id-a',
'ports': {'orp-id-a1': ['10.0.1.0/24'],
'orp-id-a2': ['10.0.2.0/24'],
@ -500,14 +531,7 @@ class TestNBImplIdlOvn(TestDBImplIdlOvn):
'snats': [{'external_ip': '20.0.2.1',
'logical_ip': '10.0.0.0/24',
'type': 'snat'}],
'dnat_and_snats': [{'external_ip': '20.0.2.4',
'logical_ip': '10.0.0.4',
'type': 'dnat_and_snat'},
{'external_ip': '20.0.2.5',
'logical_ip': '10.0.0.5',
'type': 'dnat_and_snat',
'external_mac': '00:01:02:03:04:05',
'logical_port': 'lsp-id-001'}]},
'dnat_and_snats': [lra_fip, lrb_fip]},
{'name': 'lr-id-c', 'ports': {}, 'static_routes': [],
'snats': [], 'dnat_and_snats': []},
{'name': 'lr-id-d', 'ports': {}, 'static_routes': [],
@ -519,6 +543,12 @@ class TestNBImplIdlOvn(TestDBImplIdlOvn):
'snats': [], 'dnat_and_snats': []}]
self.assertCountEqual(mapping, expected)
def test_get_all_logical_routers_with_rports(self):
self._test_get_all_logical_routers_with_rports(True)
def test_get_all_logical_routers_with_rports_without_nat_gw_port(self):
self._test_get_all_logical_routers_with_rports(False)
def test_get_acls_for_lswitches(self):
self._load_nb_db()
# Test neutron switches

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

@ -22,6 +22,7 @@ from neutron_lib import context
from neutron_lib.db import api as db_api
from oslo_config import cfg
from oslo_serialization import jsonutils
from oslo_utils import uuidutils
from neutron.common.ovn import constants
from neutron.common.ovn import utils
@ -1095,3 +1096,66 @@ class TestDBInconsistenciesPeriodics(testlib_api.SqlTestCaseLight,
external_ids=external_ids)]
nb_idl.set_lswitch_port.assert_has_calls(expected_calls)
self.assertEqual(2, nb_idl.set_lswitch_port.call_count)
def test_update_nat_floating_ip_with_gateway_port(self):
_nb_idl = self.fake_ovn_client._nb_idl
utils.is_nat_gateway_port_supported.return_value = True
lrp = fakes.FakeOvsdbRow.create_one_ovsdb_row(attrs={'options': {}})
_nb_idl.get_lrouter_port.return_value = lrp
self.fake_external_fixed_ips = {
'network_id': 'ext-network-id',
'external_fixed_ips': [{'ip_address': '20.0.2.1',
'subnet_id': 'ext-subnet-id'}]}
lrouter = {
'id': 'lr-id-b',
'name': utils.ovn_name('lr-id-b'),
'admin_state_up': True,
'external_gateway_info': self.fake_external_fixed_ips,
'gw_port_id': 'gw-port-id'
}
_nb_idl._l3_plugin.get_router.return_value = lrouter
lra_nat = {'external_ip': '20.0.2.4', 'logical_ip': '10.0.0.4',
'type': 'dnat_and_snat', 'external_mac': [],
'logical_port': [],
'external_ids': {constants.OVN_FIP_EXT_ID_KEY: 'fip_id_1'},
'gateway_port': uuidutils.generate_uuid(),
'uuid': uuidutils.generate_uuid()}
lrb_nat = {'external_ip': '20.0.2.5', 'logical_ip': '10.0.0.5',
'type': 'dnat_and_snat',
'external_mac': ['00:01:02:03:04:05'],
'logical_port': ['lsp-id-001'],
'external_ids': {constants.OVN_FIP_EXT_ID_KEY: 'fip_id_2'},
'gateway_port': [],
'uuid': uuidutils.generate_uuid()}
expected = [{'name': 'lr-id-a',
'ports': {'orp-id-a1': ['10.0.1.0/24'],
'orp-id-a2': ['10.0.2.0/24'],
'orp-id-a3': ['10.0.3.0/24']},
'static_routes': [{'destination': '20.0.0.0/16',
'nexthop': '10.0.3.253'}],
'snats': [{'external_ip': '10.0.3.1',
'logical_ip': '20.0.0.0/16',
'type': 'snat'}],
'dnat_and_snats': []},
{'name': 'lr-id-b',
'ports': {'xrp-id-b1': ['20.0.1.0/24'],
'orp-id-b2': ['20.0.2.0/24']},
'static_routes': [{'destination': '10.0.0.0/16',
'nexthop': '20.0.2.253'}],
'snats': [{'external_ip': '20.0.2.1',
'logical_ip': '10.0.0.0/24',
'type': 'snat'}],
'dnat_and_snats': [lra_nat, lrb_nat]}]
_nb_idl.get_all_logical_routers_with_rports.return_value = expected
self.assertRaises(periodics.NeverAgain,
self.periodic.update_nat_floating_ip_with_gateway_port_reference)
_nb_idl.set_nat_rule_in_lrouter.assert_called_once_with(
utils.ovn_name('lr-id-b'),
lrb_nat['uuid'],
gateway_port=lrp.uuid)

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

@ -305,6 +305,9 @@ class TestOVNL3RouterPlugin(test_mech_driver.Ml2PluginV2TestCase):
'neutron.plugins.ml2.drivers.ovn.mech_driver.ovsdb.ovn_client.'
'OVNClient.delete_mac_binding_entries_by_mac',
return_value=1)
self._start_mock(
'neutron.common.ovn.utils.is_nat_gateway_port_supported',
return_value=False)
def test__plugin_driver(self):
# No valid mech drivers should raise an exception.
@ -1140,6 +1143,58 @@ class TestOVNL3RouterPlugin(test_mech_driver.Ml2PluginV2TestCase):
mock.call('NAT', self.fake_ovn_nat_rule.uuid, 'external_mac'),
mock.call('NAT', self.fake_ovn_nat_rule.uuid, 'logical_port')])
def _test_create_floatingip_gateway_port_option(self, is_gw_port):
_nb_ovn = self.l3_inst._nb_ovn
_nb_ovn.is_col_present.return_value = True
self._get_floatingip.return_value = {'floating_port_id': 'fip-port-id'}
_nb_ovn.get_lrouter_nat_rules.return_value = [
{'external_ip': '192.168.0.10', 'logical_ip': '10.0.0.0/24',
'type': 'snat', 'uuid': 'uuid1'}]
utils.is_nat_gateway_port_supported.return_value = is_gw_port
lrp = fake_resources.FakeOvsdbRow.create_one_ovsdb_row(
attrs={'options': {}})
_nb_ovn.get_lrouter_port.return_value = lrp
self.l3_inst.get_router.return_value = self.fake_router_with_ext_gw
self.l3_inst.create_floatingip(self.context, 'floatingip')
_nb_ovn.set_nat_rule_in_lrouter.assert_not_called()
expected_ext_ids = {
ovn_const.OVN_FIP_EXT_ID_KEY: self.fake_floating_ip['id'],
ovn_const.OVN_REV_NUM_EXT_ID_KEY: '1',
ovn_const.OVN_FIP_PORT_EXT_ID_KEY:
self.fake_floating_ip['port_id'],
ovn_const.OVN_ROUTER_NAME_EXT_ID_KEY: utils.ovn_name(
self.fake_floating_ip['router_id']),
ovn_const.OVN_FIP_EXT_MAC_KEY: 'aa:aa:aa:aa:aa:aa',
ovn_const.OVN_FIP_NET_ID:
self.fake_floating_ip['floating_network_id']}
if is_gw_port:
_nb_ovn.add_nat_rule_in_lrouter.assert_called_once_with(
'neutron-router-id',
type='dnat_and_snat',
logical_ip='10.0.0.10',
external_ip='192.168.0.10',
logical_port='port_id',
external_ids=expected_ext_ids,
gateway_port=lrp.uuid)
else:
_nb_ovn.add_nat_rule_in_lrouter.assert_called_once_with(
'neutron-router-id',
type='dnat_and_snat',
logical_ip='10.0.0.10',
external_ip='192.168.0.10',
logical_port='port_id',
external_ids=expected_ext_ids)
def test_create_floatingip_with_gateway_port(self):
self._test_create_floatingip_gateway_port_option(True)
def test_create_floatingip_without_gateway_port(self):
self._test_create_floatingip_gateway_port_option(False)
@mock.patch('neutron.db.l3_db.L3_NAT_dbonly_mixin.delete_floatingip')
def test_delete_floatingip(self, df):
nb_ovn = self.l3_inst._nb_ovn

15
releasenotes/notes/add-gw-port-support-for-FIP-fb97b85f5928740b.yaml Normal file
View File

@ -0,0 +1,15 @@
---
prelude: >
The OVN changed support for NAT rules including a new column and
auto-discovery logic to know about logical router gateway ports for NAT on
a Logical Router.
features:
- |
A new OVN driver Northbound DB column has been added to allow configuring
gateway port for NAT rule. If the OVN backend supports the `gateway_port`
column in the Northbound DB NAT table, the gateway port uuid will be
configured to any floating IP to prevent North/South traffic issues.
Previously created FIP rules will be updated only once during the
maintenance task to include the gateway_port reference (if OVN backend
supports it). In case all FIP entries are already configured no maintenance
action will be performed.