openstack/ovn-bgp-agent
Code Issues Proposed changes

Do not process external network that are not provider

Browse Source 
It seems it is posible to have external networks (with cr-lrp
associated to them) that are not provider networks (e.g., vxlan
type). In this case we are opting for not exposing through BGP
their IPs as there is no physical infra handling it. This patch
makes sure those networks are not considered when processing the
events associated to them.

Change-Id: Ib436894e2337248b4e37039a1d6690fef406753b
This commit is contained in:
Luis Tomas Bolivar 2023-03-16 16:03:03 +01:00
parent a298010044
commit d9c7de6465
2 changed files with 66 additions and 19 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -263,8 +263,9 @@ class OVNBGPDriver(driver_api.AgentDriverBase):
cr_lrp_port, self.chassis, self.sb_idl) cr_lrp_port, self.chassis, self.sb_idl)
if not ips: if not ips:
return return
self._expose_ip(ips, patch_port_row, associated_port=cr_lrp_port) ips_adv = self._expose_ip(ips, patch_port_row,
for ip in ips: associated_port=cr_lrp_port)
for ip in ips_adv:
if exposed_ips and ip in exposed_ips: if exposed_ips and ip in exposed_ips:
exposed_ips.remove(ip) exposed_ips.remove(ip)
if ovn_ip_rules: if ovn_ip_rules:
@ -560,21 +561,30 @@ class OVNBGPDriver(driver_api.AgentDriverBase):
# FIPs are only supported with IPv4 # FIPs are only supported with IPv4
fip_address, fip_datapath = self.sb_idl.get_fip_associated( fip_address, fip_datapath = self.sb_idl.get_fip_associated(
row.logical_port) row.logical_port)
if fip_address:
LOG.debug("Adding BGP route for FIP with ip %s", if not fip_address:
fip_address)
if self._expose_provider_port([fip_address], fip_datapath):
LOG.debug("Added BGP route for FIP with ip %s",
fip_address)
return [fip_address]
LOG.debug("Failure adding BGP route for FIP with ip %s",
fip_address)
return [] return []
if not self.sb_idl.is_provider_network(fip_datapath):
# Only exposing IPs if the associated network is a
# provider network
return []
LOG.debug("Adding BGP route for FIP with ip %s", fip_address)
if self._expose_provider_port([fip_address], fip_datapath):
LOG.debug("Added BGP route for FIP with ip %s",
fip_address)
return [fip_address]
LOG.debug("Failure adding BGP route for FIP with ip %s",
fip_address)
return []
# FIP association to VM # FIP association to VM
elif row.type == constants.OVN_PATCH_VIF_PORT_TYPE: elif row.type == constants.OVN_PATCH_VIF_PORT_TYPE:
if (associated_port and self.sb_idl.is_port_on_chassis( if (associated_port and self.sb_idl.is_port_on_chassis(
associated_port, self.chassis)): associated_port, self.chassis)):
if not self.ovn_local_cr_lrps.get(associated_port):
# Only exposing IPs if the associated network is a
# provider network
return []
LOG.debug("Adding BGP route for FIP with ip %s", ips) LOG.debug("Adding BGP route for FIP with ip %s", ips)
if self._expose_provider_port(ips, row.datapath): if self._expose_provider_port(ips, row.datapath):
LOG.debug("Added BGP route for FIP with ip %s", ips) LOG.debug("Added BGP route for FIP with ip %s", ips)
@ -589,6 +599,10 @@ class OVNBGPDriver(driver_api.AgentDriverBase):
row.logical_port) row.logical_port)
if not cr_lrp_datapath: if not cr_lrp_datapath:
return [] return []
if not self.sb_idl.is_provider_network(cr_lrp_datapath):
# Only exposing IPs if the associated network is a
# provider network
return []
bridge_device, bridge_vlan = self._get_bridge_for_datapath( bridge_device, bridge_vlan = self._get_bridge_for_datapath(
cr_lrp_datapath) cr_lrp_datapath)
@ -690,7 +704,10 @@ class OVNBGPDriver(driver_api.AgentDriverBase):
row.logical_port) row.logical_port)
if not fip_address: if not fip_address:
return return
if not self.sb_idl.is_provider_network(fip_datapath):
# Only exposing IPs if the associated network is a
# provider network
return
LOG.debug("Deleting BGP route for FIP with ip %s", fip_address) LOG.debug("Deleting BGP route for FIP with ip %s", fip_address)
if not self._withdraw_provider_port([fip_address], if not self._withdraw_provider_port([fip_address],
fip_datapath): fip_datapath):
@ -706,6 +723,10 @@ class OVNBGPDriver(driver_api.AgentDriverBase):
self.sb_idl.is_port_on_chassis( self.sb_idl.is_port_on_chassis(
associated_port, self.chassis) or associated_port, self.chassis) or
self.sb_idl.is_port_deleted(associated_port))): self.sb_idl.is_port_deleted(associated_port))):
if not self.ovn_local_cr_lrps.get(associated_port):
# Only exposing IPs if the associated network is a
# provider network
return
LOG.debug("Deleting BGP route for FIP with ip %s", ips) LOG.debug("Deleting BGP route for FIP with ip %s", ips)
if not self._withdraw_provider_port(ips, row.datapath): if not self._withdraw_provider_port(ips, row.datapath):
LOG.debug("Failure deleting BGP route for FIP with ip %s", LOG.debug("Failure deleting BGP route for FIP with ip %s",
@ -1036,7 +1057,7 @@ class OVNBGPDriver(driver_api.AgentDriverBase):
subnet_datapath = self.sb_idl.get_port_datapath( subnet_datapath = self.sb_idl.get_port_datapath(
row.options['peer']) row.options['peer'])
if not cr_lrp: if not cr_lrp or not self.ovn_local_cr_lrps.get(cr_lrp):
return return
if not self._address_scope_allowed(ip, row.options['peer']): if not self._address_scope_allowed(ip, row.options['peer']):
@ -1072,7 +1093,7 @@ class OVNBGPDriver(driver_api.AgentDriverBase):
"triggering a subnet exposure.", "triggering a subnet exposure.",
row.logical_port) row.logical_port)
return return
if not cr_lrp or cr_lrp not in self.ovn_local_cr_lrps.keys(): if not cr_lrp or not self.ovn_local_cr_lrps.get(cr_lrp):
# NOTE(ltomasbo) there is a chance the cr-lrp just got moved # NOTE(ltomasbo) there is a chance the cr-lrp just got moved
# to this node but was not yet processed. In that case there # to this node but was not yet processed. In that case there
# is no need to withdraw the network as it was not exposed here # is no need to withdraw the network as it was not exposed here

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

@ -188,6 +188,7 @@ class TestOVNBGPDriver(test_base.TestCase):
[self.ipv4, self.ipv6], patch_port_row) [self.ipv4, self.ipv6], patch_port_row)
exposed_ips = [self.ipv4, '192.168.1.20'] exposed_ips = [self.ipv4, '192.168.1.20']
mock_expose_ip.return_value = exposed_ips
ip_rules = {"{}/128".format(self.ipv6): 'fake-rules'} ip_rules = {"{}/128".format(self.ipv6): 'fake-rules'}
self.bgp_driver._ensure_cr_lrp_associated_ports_exposed( self.bgp_driver._ensure_cr_lrp_associated_ports_exposed(
'fake-cr-lrp', exposed_ips, ip_rules) 'fake-cr-lrp', exposed_ips, ip_rules)
@ -196,7 +197,6 @@ class TestOVNBGPDriver(test_base.TestCase):
[self.ipv4, self.ipv6], patch_port_row, [self.ipv4, self.ipv6], patch_port_row,
associated_port='fake-cr-lrp') associated_port='fake-cr-lrp')
self.assertEqual(['192.168.1.20'], exposed_ips) self.assertEqual(['192.168.1.20'], exposed_ips)
self.assertEqual({}, ip_rules)
def test__ensure_port_exposed(self): def test__ensure_port_exposed(self):
mock_expose_ip = mock.patch.object( mock_expose_ip = mock.patch.object(
@ -1161,7 +1161,7 @@ class TestOVNBGPDriver(test_base.TestCase):
@mock.patch.object(linux_net, 'add_ips_to_dev') @mock.patch.object(linux_net, 'add_ips_to_dev')
def test__expose_ip_vm_with_fip( def test__expose_ip_vm_with_fip(
self, mock_add_ip_dev, mock_add_rule, mock_add_route): self, mock_add_ip_dev, mock_add_rule, mock_add_route):
self.sb_idl.is_provider_network.return_value = False self.sb_idl.is_provider_network.side_effect = [False, True]
self.sb_idl.get_fip_associated.return_value = ( self.sb_idl.get_fip_associated.return_value = (
self.fip, 'fake-dp') self.fip, 'fake-dp')
mock_get_bridge = mock.patch.object( mock_get_bridge = mock.patch.object(
@ -1184,6 +1184,31 @@ class TestOVNBGPDriver(test_base.TestCase):
mock_add_route.assert_called_once_with( mock_add_route.assert_called_once_with(
mock.ANY, self.fip, 'fake-table', self.bridge, vlan=10) mock.ANY, self.fip, 'fake-table', self.bridge, vlan=10)
@mock.patch.object(linux_net, 'add_ip_route')
@mock.patch.object(linux_net, 'add_ip_rule')
@mock.patch.object(linux_net, 'add_ips_to_dev')
def test__expose_ip_vm_with_fip_no_provider(
self, mock_add_ip_dev, mock_add_rule, mock_add_route):
self.sb_idl.is_provider_network.side_effect = [False, False]
self.sb_idl.get_fip_associated.return_value = (
self.fip, 'fake-dp')
mock_get_bridge = mock.patch.object(
self.bgp_driver, '_get_bridge_for_datapath').start()
mock_get_bridge.return_value = (self.bridge, 10)
row = fakes.create_object({
'type': constants.OVN_VM_VIF_PORT_TYPE,
'logical_port': 'fake-logical-port',
'datapath': 'fake-dp'})
ips = [self.ipv4, self.ipv6]
ret = self.bgp_driver._expose_ip(ips, row)
# Assert that the add methods were called
self.assertEqual([], ret)
mock_add_ip_dev.assert_not_called()
mock_add_rule.assert_not_called()
mock_add_route.assert_not_called()
@mock.patch.object(linux_net, 'add_ip_route') @mock.patch.object(linux_net, 'add_ip_route')
@mock.patch.object(linux_net, 'add_ip_rule') @mock.patch.object(linux_net, 'add_ip_rule')
@mock.patch.object(linux_net, 'add_ips_to_dev') @mock.patch.object(linux_net, 'add_ips_to_dev')
@ -1221,7 +1246,8 @@ class TestOVNBGPDriver(test_base.TestCase):
'datapath': 'fake-dp'}) 'datapath': 'fake-dp'})
ips = [self.ipv4, self.ipv6] ips = [self.ipv4, self.ipv6]
ret = self.bgp_driver._expose_ip(ips, row, associated_port=True) ret = self.bgp_driver._expose_ip(ips, row,
associated_port=self.cr_lrp0)
# Assert that the add methods were called # Assert that the add methods were called
self.assertEqual(ips, ret) self.assertEqual(ips, ret)
@ -1417,7 +1443,7 @@ class TestOVNBGPDriver(test_base.TestCase):
@mock.patch.object(linux_net, 'del_ips_from_dev') @mock.patch.object(linux_net, 'del_ips_from_dev')
def test_withdraw_ip_vm_with_fip( def test_withdraw_ip_vm_with_fip(
self, mock_del_ip_dev, mock_del_rule, mock_del_route): self, mock_del_ip_dev, mock_del_rule, mock_del_route):
self.sb_idl.is_provider_network.return_value = False self.sb_idl.is_provider_network.side_effect = [False, True]
self.sb_idl.get_fip_associated.return_value = ( self.sb_idl.get_fip_associated.return_value = (
self.fip, 'fake-dp') self.fip, 'fake-dp')
mock_get_bridge = mock.patch.object( mock_get_bridge = mock.patch.object(
@ -1475,7 +1501,7 @@ class TestOVNBGPDriver(test_base.TestCase):
'datapath': 'fake-dp'}) 'datapath': 'fake-dp'})
ips = [self.ipv4, self.ipv6] ips = [self.ipv4, self.ipv6]
self.bgp_driver.withdraw_ip(ips, row, associated_port=True) self.bgp_driver.withdraw_ip(ips, row, associated_port=self.cr_lrp0)
# Assert that the del methods were called # Assert that the del methods were called
mock_del_ip_dev.assert_called_once_with( mock_del_ip_dev.assert_called_once_with(