From a1f8a6c71121affe6a163646888ff71b3a2955a1 Mon Sep 17 00:00:00 2001 From: Lucas Alvares Gomes Date: Wed, 20 Dec 2023 14:49:03 +0000 Subject: [PATCH] Address the Load_Balancer's datapath_group column deprecation OVN deprecated the datapath_group column from Load_Balancer table. Now this column was split in two: ls_datapath_group and lr_datapath_group. This patch changes the code to also look for the datapath group in these new columns. This change is backward compatible and will work with a newer or older version of OVN. Change-Id: Ia3a95b77fccaa056aa9169114d102258f015002f Signed-off-by: Lucas Alvares Gomes (cherry picked from commit 5ffe0ce083a53f64500da064afca30f3bf70083e) --- ovn_bgp_agent/drivers/openstack/utils/ovn.py | 29 ++++++++++------- .../drivers/openstack/watchers/bgp_watcher.py | 12 +++---- .../openstack/watchers/test_bgp_watcher.py | 8 ++--- .../tests/unit/utils/test_helpers.py | 32 +++++++++++++++++++ ovn_bgp_agent/utils/helpers.py | 20 ++++++++++++ 5 files changed, 77 insertions(+), 24 deletions(-) diff --git a/ovn_bgp_agent/drivers/openstack/utils/ovn.py b/ovn_bgp_agent/drivers/openstack/utils/ovn.py index 504794ea..40a6e1e3 100644 --- a/ovn_bgp_agent/drivers/openstack/utils/ovn.py +++ b/ovn_bgp_agent/drivers/openstack/utils/ovn.py @@ -29,6 +29,7 @@ from ovsdbapp.schema.ovn_southbound import impl_idl as sb_impl_idl from ovn_bgp_agent import constants from ovn_bgp_agent import exceptions +from ovn_bgp_agent.utils import helpers CONF = cfg.CONF LOG = logging.getLogger(__name__) @@ -775,20 +776,24 @@ class OvsdbSbOvnIdl(sb_impl_idl.OvnSbApiIdlImpl, Backend): lb = self.get_ovn_lb(lb_name) if not lb: continue - if hasattr(lb, 'datapath_group'): - lb_dp = lb.datapath_group[0].datapaths - else: + lb_dp, lr_dp = helpers.get_lb_datapath_groups(lb) + if not lb_dp: lb_dp = lb.datapaths - # 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]) - 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 + if not lr_dp: + # 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]) + 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: + lb_ip = ip_info.split(" ")[0].split("/")[0] + lbs[lb.name] = lb_ip + return lbs def get_ovn_vip_port(self, name): diff --git a/ovn_bgp_agent/drivers/openstack/watchers/bgp_watcher.py b/ovn_bgp_agent/drivers/openstack/watchers/bgp_watcher.py index 2452f93b..e4e9f4c6 100644 --- a/ovn_bgp_agent/drivers/openstack/watchers/bgp_watcher.py +++ b/ovn_bgp_agent/drivers/openstack/watchers/bgp_watcher.py @@ -18,6 +18,7 @@ from oslo_log import log as logging from ovn_bgp_agent import constants from ovn_bgp_agent.drivers.openstack.watchers import base_watcher +from ovn_bgp_agent.utils import helpers CONF = cfg.CONF LOG = logging.getLogger(__name__) @@ -399,12 +400,8 @@ class OVNLBMemberCreateEvent(base_watcher.OVNLBEvent): row_dp = row.datapaths except AttributeError: row_dp = [] - if hasattr(row, 'datapath_group'): - if row.datapath_group: - dp_datapaths = row.datapath_group[0].datapaths - if dp_datapaths: - row_dp = dp_datapaths + row_dp, router_dps = helpers.get_lb_datapath_groups(row) if not row_dp: # No need to continue. There is no need to expose it as there is # no datapaths (aka members). @@ -418,9 +415,8 @@ class OVNLBMemberCreateEvent(base_watcher.OVNLBEvent): vip_ip = vip_ip.strip().split(" ")[0].split("/")[0] associated_cr_lrp_port = None - router_dps = [] - if not (CONF.expose_tenant_networks or - CONF.expose_ipv6_gua_tenant_networks): + if not router_dps and not (CONF.expose_tenant_networks or + 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] diff --git a/ovn_bgp_agent/tests/unit/drivers/openstack/watchers/test_bgp_watcher.py b/ovn_bgp_agent/tests/unit/drivers/openstack/watchers/test_bgp_watcher.py index d9d5d7a9..82ffaca2 100644 --- a/ovn_bgp_agent/tests/unit/drivers/openstack/watchers/test_bgp_watcher.py +++ b/ovn_bgp_agent/tests/unit/drivers/openstack/watchers/test_bgp_watcher.py @@ -922,7 +922,7 @@ class TestOVNLBMemberCreateEvent(test_base.TestCase): dpg1 = utils.create_row(_uuid='fake_dp_group', datapaths=['s_dp1']) row = utils.create_row(name='ovn-lb1', - datapath_group=[dpg1], + ls_datapath_group=[dpg1], vips={'172.24.100.66:80': '10.0.0.5:8080'}) self.agent.ovn_local_cr_lrps = { 'cr-lrp1': {'provider_datapath': 'dp1', @@ -945,7 +945,7 @@ class TestOVNLBMemberCreateEvent(test_base.TestCase): dpg1 = utils.create_row(_uuid='fake_dp_group', datapaths=['s_dp1']) row = utils.create_row(name='ovn-lb1', - datapath_group=[dpg1], + lr_datapath_group=[dpg1], vips={'172.24.100.66:80': '10.0.0.5:8080'}) self.agent.sb_idl.get_ovn_vip_port.return_value = [] self.event.run(self.event.ROW_CREATE, row, mock.Mock()) @@ -973,7 +973,7 @@ class TestOVNLBMemberCreateEvent(test_base.TestCase): dpg1 = utils.create_row(_uuid='fake_dp_group', datapaths=['s_dp2']) row = utils.create_row(name='ovn-lb1', - datapath_group=[dpg1], + ls_datapath_group=[dpg1], vips={'172.24.100.66:80': '10.0.0.5:8080'}) vip_port = utils.create_row( datapath='dp1', @@ -990,7 +990,7 @@ class TestOVNLBMemberCreateEvent(test_base.TestCase): dpg1 = utils.create_row(_uuid='fake_dp_group', datapaths=['s_dp1']) row = utils.create_row(name='ovn-lb1', - datapath_group=[dpg1], + lr_datapath_group=[dpg1], vips={'172.24.100.66:80': '10.0.0.5:8080'}) vip_port = utils.create_row( datapath='dp1', diff --git a/ovn_bgp_agent/tests/unit/utils/test_helpers.py b/ovn_bgp_agent/tests/unit/utils/test_helpers.py index 6b927683..e5daf76a 100644 --- a/ovn_bgp_agent/tests/unit/utils/test_helpers.py +++ b/ovn_bgp_agent/tests/unit/utils/test_helpers.py @@ -13,6 +13,7 @@ # limitations under the License. from ovn_bgp_agent.tests import base as test_base +from ovn_bgp_agent.tests import utils from ovn_bgp_agent.utils import helpers @@ -41,3 +42,34 @@ class TestHelpers(test_base.TestCase): self.assertEqual(ret_net, None) self.assertEqual(ret_bridge, None) + + +class TestHelperGetLBDatapathGroup(test_base.TestCase): + + def setUp(self): + super(TestHelperGetLBDatapathGroup, self).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): + lb = utils.create_row(name='ovn-lb', + datapath_group=[self.dp_group]) + self.assertEqual((['dp'], []), helpers.get_lb_datapath_groups(lb)) + + def test_get_lb_datapath_group_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)) + + def test_get_lb_datapath_group_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)) + + def test_get_lb_datapath_group_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)) diff --git a/ovn_bgp_agent/utils/helpers.py b/ovn_bgp_agent/utils/helpers.py index 9e7ec205..61242aba 100644 --- a/ovn_bgp_agent/utils/helpers.py +++ b/ovn_bgp_agent/utils/helpers.py @@ -25,3 +25,23 @@ def parse_bridge_mapping(bridge_mapping): bridge_mapping) return None, None return network, bridge + + +def _get_lb_datapath_group(lb, attr): + try: + dp = getattr(lb, attr)[0].datapaths + if dp: + return dp + except (AttributeError, IndexError): + pass + return [] + + +def get_lb_datapath_groups(lb): + for attr in ('ls_datapath_group', 'datapath_group'): + ls_dp = _get_lb_datapath_group(lb, attr) + if ls_dp: + break + + lr_dp = _get_lb_datapath_group(lb, 'lr_datapath_group') + return (ls_dp, lr_dp)