openstack/ovn-bgp-agent
Code Issues Proposed changes

Merge "Support for AddressScopes as an API of what to expose"

Browse Source
This commit is contained in:
Zuul 2023-01-30 16:27:15 +00:00 committed by Gerrit Code Review
commit b42a05ac30
6 changed files with 265 additions and 48 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

20
doc/source/contributor/bgp_mode_design.rst
View File

@ -64,6 +64,10 @@ A driver implements the support for BGP capabilities. It ensures both VMs and
LBs on providers networks or with Floating IPs associated can be
exposed throug BGP. In addition, VMs on tenant networks can be also exposed
if the ``expose_tenant_network`` configuration option is enabled.
To control what tenant networks are exposed another flag can be used:
``address_scopes``. If not set, all the tenant networks will be exposed, while
if it is configured with a (set of) address_scopes, only the tenant networks
whose address_scope matches will be exposed.
A common driver API is defined exposing the next methods:
@ -217,7 +221,8 @@ VMs and LBs on provider networks or with FIPs can be reached through BGP
VMs in tenant networks should be reachable too -- although instead of directly
in the node they are created, through one of the network gateway chassis nodes.
The same happens with ``expose_ipv6_gua_tenant_networks`` but only for IPv6
GUA ranges.
GUA ranges. In addition, if the config option ``address_scopes`` is set only
the tenant networks with matching corresponding address_scope will be exposed.
To accomplish this, it needs to ensure that:
@ -466,6 +471,7 @@ below:
expose_tenant_networks=True
# expose_ipv6_gua_tenant_networks=True
driver=osp_bgp_driver
address_scopes=2237917c7b12489a84de4ef384a2bcae
$ sudo bgp-agent --config-dir bgp-agent.conf
Starting BGP Agent...
@ -488,6 +494,13 @@ below:
instead.
.. note::
If you what to filter the tenant networks to be exposed by some specific
address scopes, add the list of address scopes to ``addresss_scope=XXX``
section. If no filtering should be applied, just remove the line.
Note that the OVN BGP Agent operates under the next assumptions:
- A dynamic routing solution, in this case FRR, is deployed and
@ -566,8 +579,9 @@ Limitations
The following limitations apply:
- There is no API to decide what to expose, all VMs/LBs on providers or with
Floating IPs associated to them will get exposed. And all the VMs in tenant
networks if the expose_tenant_network flag is enabled.
Floating IPs associated to them will get exposed. For the VMs in the tenant
networks, the flag ``address_scopes`` should be used for filtering what
subnets to expose -- which should be also used to ensure no overlapping IPs.
- There is no support for overlapping CIDRs, so this must be avoided, e.g., by
using address scopes and subnet pools.

78
ovn_bgp_agent/drivers/openstack/ovn_bgp_driver.py
View File

@ -45,6 +45,7 @@ class OVNBGPDriver(driver_api.AgentDriverBase):
def __init__(self):
self._expose_tenant_networks = (CONF.expose_tenant_networks or
CONF.expose_ipv6_gua_tenant_networks)
self.allowed_address_scopes = set(CONF.address_scopes or [])
self.ovn_routing_tables = {} # {'br-ex': 200}
self.ovn_bridge_mappings = {} # {'public': 'br-ex'}
self.ovn_local_cr_lrps = {}
@ -91,6 +92,9 @@ class OVNBGPDriver(driver_api.AgentDriverBase):
linux_net.delete_routes_from_table(CONF.bgp_vrf_table_id)
LOG.info("VRF configuration for advertising routes completed")
if self._expose_tenant_networks and self.allowed_address_scopes:
LOG.info("Configured allowed address scopes: %s",
", ".join(self.allowed_address_scopes))
events = ()
for event in self._get_events():
@ -621,20 +625,28 @@ class OVNBGPDriver(driver_api.AgentDriverBase):
return
if not CONF.expose_tenant_networks:
# This means CONF.expose_ipv6_gua_tenant_networks is enabled
ips_to_expose = []
gua_ips = []
for ip in ips:
if driver_utils.is_ipv6_gua(ip):
ips_to_expose.append(ip)
if not ips_to_expose:
gua_ips.append(ip)
if not gua_ips:
return
ips = ips_to_expose
ips = gua_ips
ips_to_expose = []
for ip in ips:
if self._address_scope_allowed(ip, None, row):
ips_to_expose.append(ip)
if not ips_to_expose:
return
port_lrp = self.sb_idl.get_lrp_port_for_datapath(row.datapath)
if port_lrp in self.ovn_local_lrps.keys():
LOG.debug("Adding BGP route for tenant IP %s on chassis %s",
ips, self.chassis)
linux_net.add_ips_to_dev(CONF.bgp_nic, ips)
ips_to_expose, self.chassis)
linux_net.add_ips_to_dev(CONF.bgp_nic, ips_to_expose)
LOG.debug("Added BGP route for tenant IP %s on chassis %s",
ips, self.chassis)
ips_to_expose, self.chassis)
@lockutils.synchronized('bgp')
def withdraw_remote_ip(self, ips, row, chassis=None):
@ -643,20 +655,27 @@ class OVNBGPDriver(driver_api.AgentDriverBase):
return
if not CONF.expose_tenant_networks:
# This means CONF.expose_ipv6_gua_tenant_networks is enabled
ips_to_withdraw = []
gua_ips = []
for ip in ips:
if driver_utils.is_ipv6_gua(ip):
ips_to_withdraw.append(ip)
if not ips_to_withdraw:
gua_ips.append(ip)
if not gua_ips:
return
ips = ips_to_withdraw
ips = gua_ips
ips_to_withdraw = []
for ip in ips:
if self._address_scope_allowed(ip, None, row):
ips_to_withdraw.append(ip)
if not ips_to_withdraw:
return
port_lrp = self.sb_idl.get_lrp_port_for_datapath(row.datapath)
if port_lrp in self.ovn_local_lrps.keys():
LOG.debug("Deleting BGP route for tenant IP %s on chassis %s",
ips, self.chassis)
linux_net.del_ips_from_dev(CONF.bgp_nic, ips)
ips_to_withdraw, self.chassis)
linux_net.del_ips_from_dev(CONF.bgp_nic, ips_to_withdraw)
LOG.debug("Deleted BGP route for tenant IP %s on chassis %s",
ips, self.chassis)
ips_to_withdraw, self.chassis)
def _process_cr_lrp_port(self, cr_lrp_port_name, provider_datapath,
router_port):
@ -697,6 +716,8 @@ class OVNBGPDriver(driver_api.AgentDriverBase):
# This should not happen: subnet without CIDR
return
if not self._address_scope_allowed(lrp_ip, lrp.options['peer']):
return
subnet_datapath = self.sb_idl.get_port_datapath(
lrp.options['peer'])
self._expose_lrp_port(lrp_ip, lrp.logical_port,
@ -875,16 +896,21 @@ class OVNBGPDriver(driver_api.AgentDriverBase):
LOG.debug("Deleting IP Rules for network %s on chassis %s", ip,
self.chassis)
exposed_lrp = False
if lrp:
if lrp in self.ovn_local_lrps.keys():
exposed_lrp = True
self.ovn_local_lrps.pop(lrp)
else:
for subnet_lp in cr_lrp_info['subnets_datapath'].keys():
if subnet_lp in self.ovn_local_lrps.keys():
exposed_lrp = True
self.ovn_local_lrps.pop(subnet_lp)
break
self.ovn_local_cr_lrps[associated_cr_lrp]['subnets_datapath'].pop(
lrp, None)
if not exposed_lrp:
return
cr_lrp_ips = [ip_address.split('/')[0]
for ip_address in cr_lrp_info.get('ips', [])]
@ -937,6 +963,9 @@ class OVNBGPDriver(driver_api.AgentDriverBase):
if not cr_lrp:
return
if not self._address_scope_allowed(ip, row.options['peer']):
return
self._expose_lrp_port(ip, row.logical_port, cr_lrp, subnet_datapath)
@lockutils.synchronized('bgp')
@ -971,3 +1000,24 @@ class OVNBGPDriver(driver_api.AgentDriverBase):
return
self._withdraw_lrp_port(ip, row.logical_port, cr_lrp)
def _address_scope_allowed(self, ip, port_name, sb_port=None):
if not self.allowed_address_scopes:
# No address scopes to filter on => announce everything
return True
if not sb_port:
sb_port = self.sb_idl.get_port_by_name(port_name)
if not sb_port:
LOG.error("Port %s missing, skipping.", port_name)
return False
address_scopes = driver_utils.get_addr_scopes(sb_port)
# if we should filter on address scopes and this port has no
# address scopes set we do not need to expose it
if not any(address_scopes.values()):
return False
# if address scope does not match, no need to expose it
ip_version = linux_net.get_ip_version(ip)
return address_scopes[ip_version] in self.allowed_address_scopes

17
ovn_bgp_agent/drivers/openstack/ovn_stretched_l2_bgp_driver.py
View File

@ -23,6 +23,7 @@ from oslo_log import log as logging
from ovn_bgp_agent import constants
from ovn_bgp_agent.drivers import driver_api
from ovn_bgp_agent.drivers.openstack.utils import driver_utils
from ovn_bgp_agent.drivers.openstack.utils import frr
from ovn_bgp_agent.drivers.openstack.utils import ovn
from ovn_bgp_agent.drivers.openstack.utils import ovs
@ -220,16 +221,6 @@ class OVNBGPStretchedL2Driver(driver_api.AgentDriverBase):
return True
def _get_addr_scopes(self, port):
return {
constants.IP_VERSION_4: port.external_ids.get(
constants.SUBNET_POOL_ADDR_SCOPE4
),
constants.IP_VERSION_6: port.external_ids.get(
constants.SUBNET_POOL_ADDR_SCOPE6
),
}
@lockutils.synchronized("bgp")
def expose_subnet(self, ip, row):
cr_lrp = self.sb_idl.is_router_gateway_on_any_chassis(row.datapath)
@ -358,7 +349,7 @@ class OVNBGPStretchedL2Driver(driver_api.AgentDriverBase):
LOG.error("Patchport %s for CR-LRP %s missing, skipping.",
patch_port, row.logical_port)
return
address_scopes = self._get_addr_scopes(port)
address_scopes = driver_utils.get_addr_scopes(port)
self.ovn_local_cr_lrps[row.logical_port][
"address_scopes"] = address_scopes
if not any([
@ -407,7 +398,7 @@ class OVNBGPStretchedL2Driver(driver_api.AgentDriverBase):
LOG.error("Patchport %s for CR-LRP %s missing, skipping.",
patch_port, gateway_port)
return
address_scopes = self._get_addr_scopes(port)
address_scopes = driver_utils.get_addr_scopes(port)
# if we should filter on address scopes and this port has no
# address scopes set we do not need to go further
if not any(address_scopes.values()):
@ -502,7 +493,7 @@ class OVNBGPStretchedL2Driver(driver_api.AgentDriverBase):
LOG.error("Patchport %s for CR-LRP %s missing, skipping.",
patch_port, gateway_port)
return
address_scopes = self._get_addr_scopes(port)
address_scopes = driver_utils.get_addr_scopes(port)
# if we have address scopes configured and none of them matches
# for this port, we can skip further processing
if not any(address_scopes.values()):

9
ovn_bgp_agent/drivers/openstack/utils/driver_utils.py
View File

@ -42,3 +42,12 @@ def is_ipv6_gua(ip):
if ipv6.is_global:
return True
return False
def get_addr_scopes(port):
return {
constants.IP_VERSION_4: port.external_ids.get(
constants.SUBNET_POOL_ADDR_SCOPE4),
constants.IP_VERSION_6: port.external_ids.get(
constants.SUBNET_POOL_ADDR_SCOPE6),
}

161
ovn_bgp_agent/tests/unit/drivers/openstack/test_ovn_bgp_driver.py
View File

@ -768,6 +768,37 @@ class TestOVNBGPDriver(test_base.TestCase):
# Assert that add_ip_route() was not called
mock_add_route.assert_not_called()
@mock.patch.object(linux_net, 'add_ips_to_dev')
@mock.patch.object(linux_net, 'add_ip_route')
@mock.patch.object(linux_net, 'add_ip_rule')
def test__process_lrp_port_address_scopes(
self, mock_add_rule, mock_add_route, mock_add_ips_dev):
gateway = {}
gateway['ips'] = ['{}/32'.format(self.fip),
'2003::1234:abcd:ffff:c0a8:102/128']
gateway['provider_datapath'] = 'bc6780f4-9510-4270-b4d2-b8d5c6802713'
gateway['subnets_datapath'] = {}
gateway['subnets_cidr'] = []
gateway['bridge_device'] = self.bridge
gateway['bridge_vlan'] = 10
self.bgp_driver.ovn_local_cr_lrps = {'gateway_port': gateway}
mock_address_scope_allowed = mock.patch.object(
self.bgp_driver, '_address_scope_allowed').start()
mock_address_scope_allowed.return_value = False
router_port = fakes.create_object({
'chassis': [],
'mac': ['{} {}/32'.format(self.mac, self.ipv4)],
'logical_port': 'lrp-fake-logical-port',
'options': {'peer': 'fake-peer'}})
self.bgp_driver._process_lrp_port(router_port, 'gateway_port')
# Assert that the add methods were called
mock_add_rule.assert_not_called()
mock_add_route.assert_not_called()
mock_add_ips_dev.assert_not_called()
def test__get_bridge_for_datapath(self):
self.sb_idl.get_network_name_and_tag.return_value = (
'fake-network', [10])
@ -1337,6 +1368,24 @@ class TestOVNBGPDriver(test_base.TestCase):
mock_add_ip_dev.assert_not_called()
@mock.patch.object(linux_net, 'add_ips_to_dev')
def test_expose_remote_ip_address_scope(self, mock_add_ip_dev):
self.sb_idl.is_provider_network.return_value = False
lrp = 'fake-lrp'
self.sb_idl.get_lrp_port_for_datapath.return_value = lrp
self.bgp_driver.ovn_local_lrps = {lrp: 'fake-cr-lrp'}
row = fakes.create_object({
'name': 'fake-row', 'datapath': 'fake-dp'})
mock_address_scope_allowed = mock.patch.object(
self.bgp_driver, '_address_scope_allowed').start()
mock_address_scope_allowed.side_effect = [False, True]
ips = [self.ipv4, self.ipv6]
self.bgp_driver.expose_remote_ip(ips, row)
mock_add_ip_dev.assert_called_once_with(CONF.bgp_nic, [self.ipv6])
@mock.patch.object(linux_net, 'del_ips_from_dev')
def test_withdraw_remote_ip(self, mock_del_ip_dev):
self.sb_idl.is_provider_network.return_value = False
@ -1416,6 +1465,24 @@ class TestOVNBGPDriver(test_base.TestCase):
mock_del_ip_dev.assert_not_called()
@mock.patch.object(linux_net, 'del_ips_from_dev')
def test_withdraw_remote_ip_address_scope(self, mock_del_ip_dev):
self.sb_idl.is_provider_network.return_value = False
lrp = 'fake-lrp'
self.sb_idl.get_lrp_port_for_datapath.return_value = lrp
self.bgp_driver.ovn_local_lrps = {lrp: 'fake-cr-lrp'}
row = fakes.create_object({
'name': 'fake-row', 'datapath': 'fake-dp'})
mock_address_scope_allowed = mock.patch.object(
self.bgp_driver, '_address_scope_allowed').start()
mock_address_scope_allowed.side_effect = [False, True]
ips = [self.ipv4, self.ipv6]
self.bgp_driver.withdraw_remote_ip(ips, row)
mock_del_ip_dev.assert_called_once_with(CONF.bgp_nic, [self.ipv6])
@mock.patch.object(linux_net, 'add_ndp_proxy')
@mock.patch.object(linux_net, 'get_ip_version')
def test__expose_cr_lrp_port(self, mock_ip_version, mock_ndp_proxy):
@ -1684,6 +1751,26 @@ class TestOVNBGPDriver(test_base.TestCase):
mock_expose_lrp_port.assert_not_called()
def test_expose_subnet_address_scope(self):
self.sb_idl.is_router_gateway_on_chassis.return_value = self.cr_lrp0
self.sb_idl.get_port_datapath.return_value = 'fake-port-dp'
row = fakes.create_object({
'name': 'fake-row',
'logical_port': 'subnet_port',
'datapath': 'fake-dp',
'options': {'peer': 'fake-peer'}})
mock_expose_lrp_port = mock.patch.object(
self.bgp_driver, '_expose_lrp_port').start()
mock_address_scope_allowed = mock.patch.object(
self.bgp_driver, '_address_scope_allowed').start()
mock_address_scope_allowed.return_value = False
self.bgp_driver.expose_subnet('fake-ip', row)
mock_expose_lrp_port.assert_not_called()
def test_withdraw_subnet(self):
row = fakes.create_object({
'name': 'fake-row',
@ -1799,3 +1886,77 @@ class TestOVNBGPDriver(test_base.TestCase):
mock_del_rule.assert_not_called()
mock_del_route.assert_not_called()
mock_del_exposed_ips.assert_not_called()
@mock.patch.object(driver_utils, 'get_addr_scopes')
def test__address_scope_allowed(self, m_addr_scopes):
self.bgp_driver.allowed_address_scopes = set(["fake_address_scope"])
port_ip = self.ipv4
port_name = "fake-port"
sb_port = "fake-sb-port"
self.sb_idl.get_port_by_name.return_value = sb_port
address_scopes = {
constants.IP_VERSION_4: "fake_address_scope",
constants.IP_VERSION_6: "fake_ipv6_address_scope"}
m_addr_scopes.return_value = address_scopes
ret = self.bgp_driver._address_scope_allowed(port_ip, port_name)
self.assertEqual(True, ret)
m_addr_scopes.assert_called_once_with(sb_port)
def test__address_scope_allowed_not_configured(self):
self.bgp_driver.allowed_address_scopes = set([])
port_ip = self.ipv4
port_name = "fake-port"
sb_port = "fake-sb-port"
ret = self.bgp_driver._address_scope_allowed(
port_ip, port_name, sb_port)
self.assertEqual(True, ret)
@mock.patch.object(driver_utils, 'get_addr_scopes')
def test__address_scope_allowed_no_match(self, m_addr_scopes):
self.bgp_driver.allowed_address_scopes = set(["fake_address_scope"])
port_ip = self.ipv4
port_name = "fake-port"
sb_port = "fake-sb-port"
self.sb_idl.get_port_by_name.return_value = sb_port
address_scopes = {
constants.IP_VERSION_4: "different_fake_address_scope",
constants.IP_VERSION_6: "fake_ipv6_address_scope"}
m_addr_scopes.return_value = address_scopes
ret = self.bgp_driver._address_scope_allowed(port_ip, port_name)
self.assertEqual(False, ret)
m_addr_scopes.assert_called_once_with(sb_port)
@mock.patch.object(driver_utils, 'get_addr_scopes')
def test__address_scope_allowed_no_port(self, m_addr_scopes):
self.bgp_driver.allowed_address_scopes = set(["fake_address_scope"])
port_ip = self.ipv4
port_name = "fake-port"
self.sb_idl.get_port_by_name.return_value = []
ret = self.bgp_driver._address_scope_allowed(port_ip, port_name)
self.assertEqual(False, ret)
m_addr_scopes.assert_not_called()
@mock.patch.object(driver_utils, 'get_addr_scopes')
def test__address_scope_allowed_no_address_scope(self, m_addr_scopes):
self.bgp_driver.allowed_address_scopes = set(["fake_address_scope"])
port_ip = self.ipv4
port_name = "fake-port"
sb_port = "fake-sb-port"
self.sb_idl.get_port_by_name.return_value = sb_port
address_scopes = {
constants.IP_VERSION_4: "",
constants.IP_VERSION_6: ""}
m_addr_scopes.return_value = address_scopes
ret = self.bgp_driver._address_scope_allowed(port_ip, port_name)
self.assertEqual(False, ret)
m_addr_scopes.assert_called_once_with(sb_port)

28
ovn_bgp_agent/tests/unit/drivers/openstack/test_ovn_stretched_l2_bgp_driver.py
View File

@ -20,6 +20,7 @@ from oslo_config import cfg
from ovn_bgp_agent import config
from ovn_bgp_agent import constants
from ovn_bgp_agent.drivers.openstack import ovn_stretched_l2_bgp_driver
from ovn_bgp_agent.drivers.openstack.utils import driver_utils
from ovn_bgp_agent.drivers.openstack.utils import frr
from ovn_bgp_agent.drivers.openstack.utils import ovn
from ovn_bgp_agent.drivers.openstack.utils import ovs
@ -226,10 +227,6 @@ class TestOVNBGPStretchedL2Driver(test_base.TestCase):
self.assertTrue(test_route not in self.bgp_driver.vrf_routes)
def test__get_addr_scopes(self):
addr_scopes = self.bgp_driver._get_addr_scopes(self.lp0)
self.assertEqual(self.addr_scope, addr_scopes)
def test__address_scope_allowed(self):
test_scope2 = {
constants.IP_VERSION_4: self.addr_scopev4,
@ -773,14 +770,11 @@ class TestOVNBGPStretchedL2Driver(test_base.TestCase):
{}
)
@mock.patch.object(driver_utils, "get_addr_scopes")
@mock.patch.object(linux_net, "add_ip_route")
def test__ensure_network_exposed_port_not_existing(
self,
mock_add_ip_route
):
mock__get_addr_scopes = mock.patch.object(
self.bgp_driver, "_get_addr_scopes"
).start()
def test__ensure_network_exposed_port_not_existing(self,
mock_add_ip_route,
mock_addr_scopes):
gateway = {}
gateway["ips"] = [
ipaddress.ip_interface(ip)
@ -793,7 +787,7 @@ class TestOVNBGPStretchedL2Driver(test_base.TestCase):
self.bgp_driver._ensure_network_exposed(
self.router_port, "gateway_port"
)
mock__get_addr_scopes.assert_not_called()
mock_addr_scopes.assert_not_called()
mock_add_ip_route.assert_not_called()
self.assertDictEqual(
self.bgp_driver.propagated_lrp_ports,
@ -1208,17 +1202,15 @@ class TestOVNBGPStretchedL2Driver(test_base.TestCase):
mock__ensure_network_exposed.assert_not_called()
self.sb_idl.get_port_by_name.assert_called_once_with("fake-port")
def test__expose_cr_lrp_no_addr_scope(self):
@mock.patch.object(driver_utils, "get_addr_scopes")
def test__expose_cr_lrp_no_addr_scope(self, mock_addr_scopes):
mock__ensure_network_exposed = mock.patch.object(
self.bgp_driver, "_ensure_network_exposed"
).start()
mock__get_addr_scopes = mock.patch.object(
self.bgp_driver, "_get_addr_scopes"
).start()
self.sb_idl.get_port_by_name.return_value = self.fake_patch_port
mock__get_addr_scopes.return_value = {
mock_addr_scopes.return_value = {
constants.IP_VERSION_4: "address_scope_v4",
constants.IP_VERSION_6: "address_scope_v6",
}
@ -1226,7 +1218,7 @@ class TestOVNBGPStretchedL2Driver(test_base.TestCase):
self.bgp_driver._expose_cr_lrp([], self.cr_lrp0)
self.sb_idl.get_port_by_name.assert_called_once_with("fake-port")
mock__get_addr_scopes.assert_called_once_with(self.fake_patch_port)
mock_addr_scopes.assert_called_once_with(self.fake_patch_port)
self.sb_idl.get_lrp_ports_for_router.assert_not_called()
mock__ensure_network_exposed.assert_not_called()