openstack/ovn-bgp-agent
Code Issues Proposed changes

Ensure exposed ovn lb IPs are not removed upon re-sync

Browse Source 
After change [1] we forgot to consider the ovn lb IPs exposed
on the re-syncs. This leads to the exposed IPs to be withdrawn.
This patch fixes this, ensuring the exposed IPs are not removed
on re-syncs.

[1] https://review.opendev.org/c/x/ovn-bgp-agent/+/873073

Change-Id: Ie1fb190d671b4957633cd528e1371ad409cf07c2
This commit is contained in:
Luis Tomas Bolivar 2023-02-17 14:28:01 +01:00
parent 60e896d313
commit 05de18942d
2 changed files with 51 additions and 35 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -233,7 +233,9 @@ class OVNBGPDriver(driver_api.AgentDriverBase):
for ovn_lb_port, ovn_lb_ip in ovn_lb_vips.items(): for ovn_lb_port, ovn_lb_ip in ovn_lb_vips.items():
self._expose_ovn_lb_on_provider(ovn_lb_ip, self._expose_ovn_lb_on_provider(ovn_lb_ip,
ovn_lb_port, ovn_lb_port,
cr_lrp_port) cr_lrp_port,
exposed_ips,
ovn_ip_rules)
# remove extra routes/ips # remove extra routes/ips
# remove all the leftovers on the list of current ips on dev OVN # remove all the leftovers on the list of current ips on dev OVN
@ -255,14 +257,15 @@ class OVNBGPDriver(driver_api.AgentDriverBase):
return return
self._expose_ip(ips, patch_port_row, associated_port=cr_lrp_port) self._expose_ip(ips, patch_port_row, associated_port=cr_lrp_port)
for ip in ips: for ip in ips:
ip_version = linux_net.get_ip_version(ip) if exposed_ips and ip in exposed_ips:
if ip_version == constants.IP_VERSION_6:
ip_dst = "{}/128".format(ip)
else:
ip_dst = "{}/32".format(ip)
if ip in exposed_ips:
exposed_ips.remove(ip) exposed_ips.remove(ip)
ovn_ip_rules.pop(ip_dst, None) if ovn_ip_rules:
ip_version = linux_net.get_ip_version(ip)
if ip_version == constants.IP_VERSION_6:
ip_dst = "{}/128".format(ip)
else:
ip_dst = "{}/32".format(ip)
ovn_ip_rules.pop(ip_dst, None)
def _ensure_port_exposed(self, port, exposed_ips, ovn_ip_rules): def _ensure_port_exposed(self, port, exposed_ips, ovn_ip_rules):
if port.type not in constants.OVN_VIF_PORT_TYPES or not port.mac: if port.type not in constants.OVN_VIF_PORT_TYPES or not port.mac:
@ -293,15 +296,16 @@ class OVNBGPDriver(driver_api.AgentDriverBase):
for port_ip in ips_adv: for port_ip in ips_adv:
ip_address = port_ip.split("/")[0] ip_address = port_ip.split("/")[0]
ip_version = linux_net.get_ip_version(port_ip) if exposed_ips and ip_address in exposed_ips:
if ip_version == constants.IP_VERSION_6:
ip_dst = "{}/128".format(ip_address)
else:
ip_dst = "{}/32".format(ip_address)
if ip_address in exposed_ips:
# remove each ip to add from the list of current ips on dev OVN # remove each ip to add from the list of current ips on dev OVN
exposed_ips.remove(ip_address) exposed_ips.remove(ip_address)
ovn_ip_rules.pop(ip_dst, None) if ovn_ip_rules:
ip_version = linux_net.get_ip_version(port_ip)
if ip_version == constants.IP_VERSION_6:
ip_dst = "{}/128".format(ip_address)
else:
ip_dst = "{}/32".format(ip_address)
ovn_ip_rules.pop(ip_dst, None)
def _expose_provider_port(self, port_ips, provider_datapath, def _expose_provider_port(self, port_ips, provider_datapath,
bridge_device=None, bridge_vlan=None, bridge_device=None, bridge_vlan=None,
@ -330,8 +334,8 @@ class OVNBGPDriver(driver_api.AgentDriverBase):
self.ovn_routing_tables[bridge_device], bridge_device, self.ovn_routing_tables[bridge_device], bridge_device,
vlan=bridge_vlan) vlan=bridge_vlan)
def _expose_tenant_port(self, port, ip_version, exposed_ips=[], def _expose_tenant_port(self, port, ip_version, exposed_ips=None,
ovn_ip_rules={}): ovn_ip_rules=None):
# specific case for ovn-lb vips on tenant networks # specific case for ovn-lb vips on tenant networks
if not port.mac and not port.chassis and not port.up[0]: if not port.mac and not port.chassis and not port.up[0]:
ext_n_cidr = port.external_ids.get( ext_n_cidr = port.external_ids.get(
@ -340,9 +344,10 @@ class OVNBGPDriver(driver_api.AgentDriverBase):
ovn_lb_ip = ext_n_cidr.split(" ")[0].split("/")[0] ovn_lb_ip = ext_n_cidr.split(" ")[0].split("/")[0]
linux_net.add_ips_to_dev( linux_net.add_ips_to_dev(
CONF.bgp_nic, [ovn_lb_ip]) CONF.bgp_nic, [ovn_lb_ip])
if ovn_lb_ip in exposed_ips: if exposed_ips and ovn_lb_ip in exposed_ips:
exposed_ips.remove(ovn_lb_ip) exposed_ips.remove(ovn_lb_ip)
ovn_ip_rules.pop(ext_n_cidr.split(" ")[0], None) if ovn_ip_rules:
ovn_ip_rules.pop(ext_n_cidr.split(" ")[0], None)
return return
elif (not port.mac or elif (not port.mac or
port.type not in ( port.type not in (
@ -370,13 +375,14 @@ class OVNBGPDriver(driver_api.AgentDriverBase):
if port_ip_version == ip_version: if port_ip_version == ip_version:
linux_net.add_ips_to_dev( linux_net.add_ips_to_dev(
CONF.bgp_nic, [port_ip]) CONF.bgp_nic, [port_ip])
if port_ip in exposed_ips: if exposed_ips and port_ip in exposed_ips:
exposed_ips.remove(port_ip) exposed_ips.remove(port_ip)
if port_ip_version == constants.IP_VERSION_6: if ovn_ip_rules:
ip_dst = "{}/128".format(port_ip) if port_ip_version == constants.IP_VERSION_6:
else: ip_dst = "{}/128".format(port_ip)
ip_dst = "{}/32".format(port_ip) else:
ovn_ip_rules.pop(ip_dst, None) ip_dst = "{}/32".format(port_ip)
ovn_ip_rules.pop(ip_dst, None)
def _withdraw_provider_port(self, port_ips, provider_datapath, def _withdraw_provider_port(self, port_ips, provider_datapath,
bridge_device=None, bridge_vlan=None, bridge_device=None, bridge_vlan=None,
@ -452,7 +458,8 @@ class OVNBGPDriver(driver_api.AgentDriverBase):
if action == constants.WITHDRAW: if action == constants.WITHDRAW:
self._withdraw_ovn_lb_on_provider(vip_port, local_lb_cr_lrp) self._withdraw_ovn_lb_on_provider(vip_port, local_lb_cr_lrp)
def _expose_ovn_lb_on_provider(self, ip, ovn_lb_vip_port, cr_lrp): def _expose_ovn_lb_on_provider(self, ip, ovn_lb_vip_port, cr_lrp,
exposed_ips=None, ovn_ip_rules=None):
self.ovn_local_cr_lrps[cr_lrp]['ovn_lb_vips'].append(ovn_lb_vip_port) self.ovn_local_cr_lrps[cr_lrp]['ovn_lb_vips'].append(ovn_lb_vip_port)
self.ovn_lb_vips.setdefault(ovn_lb_vip_port, []).append(ip) self.ovn_lb_vips.setdefault(ovn_lb_vip_port, []).append(ip)
bridge_device = self.ovn_local_cr_lrps[cr_lrp]['bridge_device'] bridge_device = self.ovn_local_cr_lrps[cr_lrp]['bridge_device']
@ -462,6 +469,15 @@ class OVNBGPDriver(driver_api.AgentDriverBase):
self._expose_provider_port([ip], None, bridge_device=bridge_device, self._expose_provider_port([ip], None, bridge_device=bridge_device,
bridge_vlan=bridge_vlan) bridge_vlan=bridge_vlan)
LOG.debug("Added BGP route for loadbalancer VIP %s", ip) LOG.debug("Added BGP route for loadbalancer VIP %s", ip)
if exposed_ips and ip in exposed_ips:
exposed_ips.remove(ip)
if ovn_ip_rules:
ip_version = linux_net.get_ip_version(ip)
if ip_version == constants.IP_VERSION_6:
ip_dst = "{}/128".format(ip)
else:
ip_dst = "{}/32".format(ip)
ovn_ip_rules.pop(ip_dst, None)
def _withdraw_ovn_lb_on_provider(self, ovn_lb_vip_port, cr_lrp): def _withdraw_ovn_lb_on_provider(self, ovn_lb_vip_port, cr_lrp):
bridge_device = self.ovn_local_cr_lrps[cr_lrp]['bridge_device'] bridge_device = self.ovn_local_cr_lrps[cr_lrp]['bridge_device']
@ -765,8 +781,8 @@ class OVNBGPDriver(driver_api.AgentDriverBase):
LOG.debug("Deleted BGP route for tenant IP %s on chassis %s", LOG.debug("Deleted BGP route for tenant IP %s on chassis %s",
ips_to_withdraw, self.chassis) ips_to_withdraw, self.chassis)
def _process_lrp_port(self, lrp, associated_cr_lrp, exposed_ips=[], def _process_lrp_port(self, lrp, associated_cr_lrp, exposed_ips=None,
ovn_ip_rules={}): ovn_ip_rules=None):
if (lrp.chassis or if (lrp.chassis or
not lrp.logical_port.startswith('lrp-') or not lrp.logical_port.startswith('lrp-') or
"chassis-redirect-port" in lrp.options.keys() or "chassis-redirect-port" in lrp.options.keys() or
@ -863,7 +879,7 @@ class OVNBGPDriver(driver_api.AgentDriverBase):
cr_lrp_port) cr_lrp_port)
def _expose_lrp_port(self, ip, lrp, associated_cr_lrp, subnet_datapath, def _expose_lrp_port(self, ip, lrp, associated_cr_lrp, subnet_datapath,
exposed_ips=[], ovn_ip_rules={}): exposed_ips=None, ovn_ip_rules=None):
if not self._expose_tenant_networks: if not self._expose_tenant_networks:
return return
if not CONF.expose_tenant_networks: if not CONF.expose_tenant_networks:

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

@ -1646,11 +1646,11 @@ class TestOVNBGPDriver(test_base.TestCase):
mask='32', via=self.fip) mask='32', via=self.fip)
expected_calls = [ expected_calls = [
mock.call(dp_port0, ip_version=constants.IP_VERSION_4, mock.call(dp_port0, ip_version=constants.IP_VERSION_4,
exposed_ips=[], ovn_ip_rules={}), exposed_ips=None, ovn_ip_rules=None),
mock.call(dp_port1, ip_version=constants.IP_VERSION_4, mock.call(dp_port1, ip_version=constants.IP_VERSION_4,
exposed_ips=[], ovn_ip_rules={}), exposed_ips=None, ovn_ip_rules=None),
mock.call(dp_port2, ip_version=constants.IP_VERSION_4, mock.call(dp_port2, ip_version=constants.IP_VERSION_4,
exposed_ips=[], ovn_ip_rules={})] exposed_ips=None, ovn_ip_rules=None)]
mock_expose_tenant_port.assert_has_calls(expected_calls) mock_expose_tenant_port.assert_has_calls(expected_calls)
@mock.patch.object(linux_net, 'add_ip_route') @mock.patch.object(linux_net, 'add_ip_route')
@ -1732,11 +1732,11 @@ class TestOVNBGPDriver(test_base.TestCase):
mask='128', via=self.fip) mask='128', via=self.fip)
expected_calls = [ expected_calls = [
mock.call(dp_port0, ip_version=constants.IP_VERSION_6, mock.call(dp_port0, ip_version=constants.IP_VERSION_6,
exposed_ips=[], ovn_ip_rules={}), exposed_ips=None, ovn_ip_rules=None),
mock.call(dp_port1, ip_version=constants.IP_VERSION_6, mock.call(dp_port1, ip_version=constants.IP_VERSION_6,
exposed_ips=[], ovn_ip_rules={}), exposed_ips=None, ovn_ip_rules=None),
mock.call(dp_port2, ip_version=constants.IP_VERSION_6, mock.call(dp_port2, ip_version=constants.IP_VERSION_6,
exposed_ips=[], ovn_ip_rules={})] exposed_ips=None, ovn_ip_rules=None)]
mock_expose_tenant_port.assert_has_calls(expected_calls) mock_expose_tenant_port.assert_has_calls(expected_calls)
@mock.patch.object(linux_net, 'add_ip_route') @mock.patch.object(linux_net, 'add_ip_route')