From 8cda2174db4c6a02259923fa542edf4d364a89fc Mon Sep 17 00:00:00 2001 From: Jakub Libosvar Date: Wed, 24 Apr 2024 14:51:01 +0000 Subject: [PATCH] Fix sync for OVN LB VIPs There was a bad comparison of datapath types, we compared router datapath is equal to loadbalancer datapaths instead of if it is included there. Closes-bug: #2064922 Change-Id: I4f92d75f512ca906ad884e9da21ee7048b1dc5d6 Signed-off-by: Jakub Libosvar --- ovn_bgp_agent/drivers/openstack/utils/ovn.py | 12 +++++------ .../drivers/openstack/watchers/bgp_watcher.py | 12 +++++------ .../tests/unit/utils/test_helpers.py | 20 +++++++++---------- ovn_bgp_agent/utils/helpers.py | 18 ++++++++--------- 4 files changed, 31 insertions(+), 31 deletions(-) diff --git a/ovn_bgp_agent/drivers/openstack/utils/ovn.py b/ovn_bgp_agent/drivers/openstack/utils/ovn.py index 75dc7313..99e14180 100644 --- a/ovn_bgp_agent/drivers/openstack/utils/ovn.py +++ b/ovn_bgp_agent/drivers/openstack/utils/ovn.py @@ -777,21 +777,21 @@ class OvsdbSbOvnIdl(sb_impl_idl.OvnSbApiIdlImpl, Backend): lb = self.get_ovn_lb(lb_name) if not lb: continue - lb_dp, lr_dp = helpers.get_lb_datapath_groups(lb) - if not lb_dp: - lb_dp = lb.datapaths + lb_dps, lr_dps = helpers.get_lb_datapaths(lb) + if not lb_dps: + lb_dps = lb.datapaths - if not lr_dp: + if not lr_dps: # assume all the members are connected through the same router # so only one datapath needs to be checked - router_lrps = self.get_lrps_for_datapath(lb_dp[0]) + router_lrps = self.get_lrps_for_datapath(lb_dps[0]) for lrp in router_lrps: router_lrp_dp = self.get_port_datapath(lrp) if router_lrp_dp == router_dp: lb_ip = ip_info.split(" ")[0].split("/")[0] lbs[lb.name] = lb_ip break - elif lr_dp == router_dp: + elif router_dp in lr_dps: lb_ip = ip_info.split(" ")[0].split("/")[0] lbs[lb.name] = lb_ip diff --git a/ovn_bgp_agent/drivers/openstack/watchers/bgp_watcher.py b/ovn_bgp_agent/drivers/openstack/watchers/bgp_watcher.py index e4e9f4c6..de197a4d 100644 --- a/ovn_bgp_agent/drivers/openstack/watchers/bgp_watcher.py +++ b/ovn_bgp_agent/drivers/openstack/watchers/bgp_watcher.py @@ -397,12 +397,12 @@ class OVNLBMemberCreateEvent(base_watcher.OVNLBEvent): if not self.agent.ovn_local_cr_lrps: return try: - row_dp = row.datapaths + row_dps = row.datapaths except AttributeError: - row_dp = [] + row_dps = [] - row_dp, router_dps = helpers.get_lb_datapath_groups(row) - if not row_dp: + row_dps, router_dps = helpers.get_lb_datapaths(row) + if not row_dps: # No need to continue. There is no need to expose it as there is # no datapaths (aka members). return @@ -419,7 +419,7 @@ class OVNLBMemberCreateEvent(base_watcher.OVNLBEvent): CONF.expose_ipv6_gua_tenant_networks): # assume all the members are connected through the same router # so only one member needs to be checked - member_dp = row_dp[0] + member_dp = row_dps[0] # get lrps on that dp (patch ports) router_lrps = ( self.agent.sb_idl.get_lrps_for_datapath(member_dp)) @@ -430,7 +430,7 @@ class OVNLBMemberCreateEvent(base_watcher.OVNLBEvent): if vip_port.datapath != cr_lrp_info.get('provider_datapath'): continue if cr_lrp_info.get('subnets_datapath'): - if set(row_dp).intersection(set( + if set(row_dps).intersection(set( cr_lrp_info.get('subnets_datapath').values())): associated_cr_lrp_port = cr_lrp_port break diff --git a/ovn_bgp_agent/tests/unit/utils/test_helpers.py b/ovn_bgp_agent/tests/unit/utils/test_helpers.py index e5daf76a..434ae4a3 100644 --- a/ovn_bgp_agent/tests/unit/utils/test_helpers.py +++ b/ovn_bgp_agent/tests/unit/utils/test_helpers.py @@ -44,32 +44,32 @@ class TestHelpers(test_base.TestCase): self.assertEqual(ret_bridge, None) -class TestHelperGetLBDatapathGroup(test_base.TestCase): +class TestHelperGetLBDatapaths(test_base.TestCase): def setUp(self): - super(TestHelperGetLBDatapathGroup, self).setUp() + super().setUp() self.dp_group = utils.create_row(_uuid='fake_dp_group', datapaths=['dp']) self.dp_group1 = utils.create_row(_uuid='fake_dp_group1', datapaths=['dp1']) - def test_get_lb_datapath_group(self): + def test_get_lb_datapaths(self): lb = utils.create_row(name='ovn-lb', datapath_group=[self.dp_group]) - self.assertEqual((['dp'], []), helpers.get_lb_datapath_groups(lb)) + self.assertEqual((['dp'], []), helpers.get_lb_datapaths(lb)) - def test_get_lb_datapath_group_ls_datapath(self): + def test_get_lb_datapaths_ls_datapath(self): lb = utils.create_row(name='ovn-lb', ls_datapath_group=[self.dp_group]) - self.assertEqual((['dp'], []), helpers.get_lb_datapath_groups(lb)) + self.assertEqual((['dp'], []), helpers.get_lb_datapaths(lb)) - def test_get_lb_datapath_group_lr_datapath(self): + def test_get_lb_datapaths_lr_datapath(self): lb = utils.create_row(name='ovn-lb', lr_datapath_group=[self.dp_group]) - self.assertEqual(([], ['dp']), helpers.get_lb_datapath_groups(lb)) + self.assertEqual(([], ['dp']), helpers.get_lb_datapaths(lb)) - def test_get_lb_datapath_group_ls_and_lr_datapath(self): + def test_get_lb_datapaths_ls_and_lr_datapath(self): lb = utils.create_row(name='ovn-lb', ls_datapath_group=[self.dp_group], lr_datapath_group=[self.dp_group1]) - self.assertEqual((['dp'], ['dp1']), helpers.get_lb_datapath_groups(lb)) + self.assertEqual((['dp'], ['dp1']), helpers.get_lb_datapaths(lb)) diff --git a/ovn_bgp_agent/utils/helpers.py b/ovn_bgp_agent/utils/helpers.py index 61242aba..0a929967 100644 --- a/ovn_bgp_agent/utils/helpers.py +++ b/ovn_bgp_agent/utils/helpers.py @@ -27,21 +27,21 @@ def parse_bridge_mapping(bridge_mapping): return network, bridge -def _get_lb_datapath_group(lb, attr): +def _get_lb_datapaths_from_group(lb, attr): try: - dp = getattr(lb, attr)[0].datapaths - if dp: - return dp + dps = getattr(lb, attr)[0].datapaths + if dps: + return dps except (AttributeError, IndexError): pass return [] -def get_lb_datapath_groups(lb): +def get_lb_datapaths(lb): for attr in ('ls_datapath_group', 'datapath_group'): - ls_dp = _get_lb_datapath_group(lb, attr) - if ls_dp: + ls_dps = _get_lb_datapaths_from_group(lb, attr) + if ls_dps: break - lr_dp = _get_lb_datapath_group(lb, 'lr_datapath_group') - return (ls_dp, lr_dp) + lr_dps = _get_lb_datapaths_from_group(lb, 'lr_datapath_group') + return (ls_dps, lr_dps)