Merge "Introduce LSP address column parsing functions" into stable/2024.1

ovn_bgp_agent/drivers/openstack/nb_ovn_bgp_driver.py

@@ -26,6 +26,7 @@ from ovn_bgp_agent.drivers.openstack.utils import bgp as bgp_utils
 from ovn_bgp_agent.drivers.openstack.utils import driver_utils
 from ovn_bgp_agent.drivers.openstack.utils import ovn
 from ovn_bgp_agent.drivers.openstack.utils import ovs
+from ovn_bgp_agent.drivers.openstack.utils import port as port_utils
 from ovn_bgp_agent.drivers.openstack.utils import wire as wire_utils
 from ovn_bgp_agent.drivers.openstack.watchers import nb_bgp_watcher as watcher
 from ovn_bgp_agent import exceptions
@@ -244,8 +245,11 @@ class NBOVNBGPDriver(driver_api.AgentDriverBase):
         _, bridge_device, bridge_vlan = self._get_provider_ls_info(
             logical_switch)
 
-        ips = port.addresses[0].strip().split(' ')[1:]
+        try:
-        mac = port.addresses[0].strip().split(' ')[0]
+            ips = port_utils.get_ips_from_lsp(port)
+        except exceptions.IpAddressNotFound:
+            ips = []
+        mac = port_utils.get_mac_from_lsp(port)
         self._expose_ip(ips, mac, logical_switch, bridge_device, bridge_vlan,
                         port.type, port.external_ids.get(
                             constants.OVN_CIDRS_EXT_ID_KEY, "").split())
@@ -777,13 +781,18 @@ class NBOVNBGPDriver(driver_api.AgentDriverBase):
             # Check if the ip's on this port match the address scope. As the
             # port can be dual-stack, it could be that v4 is not allowed, but
             # v6 is allowed, so then only v6 address should be exposed.
-            ips = self._ips_in_address_scope(port.addresses[0].split(' ')[1:],
+            try:
-                                             subnet_info['address_scopes'])
+                ips = self._ips_in_address_scope(
+                    port_utils.get_ips_from_lsp(port),
+                    subnet_info['address_scopes'])
+            except exceptions.IpAddressNotFound:
+                continue
+
             if not ips:
                 # All ip's have been removed due to address scope requirement
                 continue
 
-            mac = port.addresses[0].strip().split(' ')[0]
+            mac = port_utils.get_mac_from_lsp(port)
             ips_info = {
                 'mac': mac,
                 'cidrs': port.external_ids.get(constants.OVN_CIDRS_EXT_ID_KEY,
@@ -824,8 +833,11 @@ class NBOVNBGPDriver(driver_api.AgentDriverBase):
 
         ports = self.nb_idl.get_active_lsp(subnet_info['network'])
         for port in ports:
-            ips = port.addresses[0].split(' ')[1:]
+            try:
-            mac = port.addresses[0].strip().split(' ')[0]
+                ips = port_utils.get_ips_from_lsp(port)
+            except exceptions.IpAddressNotFound:
+                continue
+            mac = port_utils.get_mac_from_lsp(port)
             ips_info = {
                 'mac': mac,
                 'cidrs': port.external_ids.get(constants.OVN_CIDRS_EXT_ID_KEY,

ovn_bgp_agent/drivers/openstack/utils/port.py

@@ -14,6 +14,7 @@
 
 from ovn_bgp_agent import constants
 from ovn_bgp_agent.drivers.openstack.utils import common
+from ovn_bgp_agent import exceptions
 
 
 def has_ip_address_defined(address):
@@ -31,10 +32,37 @@ def has_additional_binding(row):
         constants.OVN_REQUESTED_CHASSIS, '')
 
 
+def get_address_list(lsp):
+    try:
+        addrs = lsp.addresses[0].strip().split(' ')
+        # Check the first element for an empty string
+        if not addrs[0]:
+            return []
+    except (AttributeError, IndexError):
+        return []
+
+    return addrs
+
+
+def get_mac_from_lsp(lsp):
+    try:
+        return get_address_list(lsp)[0]
+    except IndexError:
+        raise exceptions.MacAddressNotFound(lsp=lsp)
+
+
+def get_ips_from_lsp(lsp):
+    addresses = get_address_list(lsp)[1:]
+    if not addresses:
+        raise exceptions.IpAddressNotFound(lsp=lsp)
+
+    return addresses
+
+
 def make_lsp_dict(row):
     # TODO(jlibosva): Stop passing around dynamic maps
     return {
-        'mac': row.addresses[0].strip().split(' ')[0],
+        'mac': get_mac_from_lsp(row),
         'cidrs': row.external_ids.get(constants.OVN_CIDRS_EXT_ID_KEY,
                                       "").split(),
         'type': row.type,

ovn_bgp_agent/drivers/openstack/watchers/nb_bgp_watcher.py

@@ -22,6 +22,7 @@ from ovn_bgp_agent.drivers.openstack.utils import loadbalancer as lb_utils
 from ovn_bgp_agent.drivers.openstack.utils import port as port_utils
 from ovn_bgp_agent.drivers.openstack.utils import router as router_utils
 from ovn_bgp_agent.drivers.openstack.watchers import base_watcher
+from ovn_bgp_agent import exceptions
 
 
 LOG = logging.getLogger(__name__)
@@ -74,7 +75,11 @@ class LogicalSwitchPortProviderCreateEvent(base_watcher.LSPChassisEvent):
             # At this point, the port is bound on this host, it is up and
             # the logical switch is exposable by the agent.
             # Only create the ips if not already exposed.
-            ips = row.addresses[0].split(' ')[1:]
+            try:
+                ips = port_utils.get_ips_from_lsp(row)
+            except exceptions.IpAddressNotFound:
+                return False
+
             return not self.agent.is_ip_exposed(logical_switch, ips)
 
         except (IndexError, AttributeError):
@@ -82,7 +87,7 @@ class LogicalSwitchPortProviderCreateEvent(base_watcher.LSPChassisEvent):
 
     def _run(self, event, row, old):
         with _SYNC_STATE_LOCK.read_lock():
-            ips = row.addresses[0].split(' ')[1:]
+            ips = port_utils.get_ips_from_lsp(row)
             ips_info = port_utils.make_lsp_dict(row)
             self.agent.expose_ip(ips, ips_info)
 
@@ -108,7 +113,11 @@ class LogicalSwitchPortProviderDeleteEvent(base_watcher.LSPChassisEvent):
             if not port_utils.has_ip_address_defined(row.addresses[0]):
                 return False
 
-            ips = row.addresses[0].split(' ')[1:]
+            try:
+                ips = port_utils.get_ips_from_lsp(row)
+            except exceptions.IpAddressNotFound:
+                return False
+
             logical_switch = common_utils.get_from_external_ids(
                 row, constants.OVN_LS_NAME_EXT_ID_KEY)
 
@@ -141,7 +150,7 @@ class LogicalSwitchPortProviderDeleteEvent(base_watcher.LSPChassisEvent):
 
     def _run(self, event, row, old):
         with _SYNC_STATE_LOCK.read_lock():
-            ips = row.addresses[0].split(' ')[1:]
+            ips = port_utils.get_ips_from_lsp(row)
             ips_info = port_utils.make_lsp_dict(row)
             self.agent.withdraw_ip(ips, ips_info)
 
@@ -603,7 +612,9 @@ class LogicalSwitchPortTenantCreateEvent(base_watcher.LSPChassisEvent):
     def match_fn(self, event, row, old):
         try:
             # single and dual-stack format
-            if not port_utils.has_ip_address_defined(row.addresses[0]):
+            try:
+                port_utils.get_ips_from_lsp(row)
+            except exceptions.IpAddressNotFound:
                 return False
 
             if not bool(row.up[0]):
@@ -632,8 +643,8 @@ class LogicalSwitchPortTenantCreateEvent(base_watcher.LSPChassisEvent):
                             constants.OVN_VIRTUAL_VIF_PORT_TYPE]:
             return
         with _SYNC_STATE_LOCK.read_lock():
-            ips = row.addresses[0].split(' ')[1:]
+            ips = port_utils.get_ips_from_lsp(row)
-            mac = row.addresses[0].strip().split(' ')[0]
+            mac = port_utils.get_mac_from_lsp(row)
             ips_info = {
                 'mac': mac,
                 'cidrs': row.external_ids.get(constants.OVN_CIDRS_EXT_ID_KEY,
@@ -678,8 +689,11 @@ class LogicalSwitchPortTenantDeleteEvent(base_watcher.LSPChassisEvent):
                             constants.OVN_VIRTUAL_VIF_PORT_TYPE]:
             return
         with _SYNC_STATE_LOCK.read_lock():
-            ips = row.addresses[0].split(' ')[1:]
+            # The address is present because of the
-            mac = row.addresses[0].strip().split(' ')[0]
+            # has_ip_address_defined() check in match_fn(), therefore
+            # there is no need for the try-except block.
+            ips = port_utils.get_ips_from_lsp(row)
+            mac = port_utils.get_mac_from_lsp(row)
             ips_info = {
                 'mac': mac,
                 'cidrs': row.external_ids.get(constants.OVN_CIDRS_EXT_ID_KEY,

ovn_bgp_agent/exceptions.py

@@ -158,3 +158,12 @@ class InvalidArgument(RuntimeError):
     def __init__(self, message=None, device=None):
         message = message or self.message % {'device': device}
         super(InvalidArgument, self).__init__(message)
+
+
+class MacAddressNotFound(OVNBGPAgentException):
+    message = _("Addresses column is empty or not set for port %(lsp)s")
+
+
+class IpAddressNotFound(OVNBGPAgentException):
+    message = _("Addresses column has no IP addresses or is not set for port"
+                " %(lsp)s")

ovn_bgp_agent/tests/unit/drivers/openstack/utils/test_port.py

@@ -15,6 +15,7 @@
 
 from ovn_bgp_agent import constants
 from ovn_bgp_agent.drivers.openstack.utils import port
+from ovn_bgp_agent import exceptions
 from ovn_bgp_agent.tests import base as test_base
 from ovn_bgp_agent.tests import utils as test_utils
 
@@ -82,3 +83,71 @@ class TestHasAdditionalBinding(test_base.TestCase):
             options={constants.OVN_REQUESTED_CHASSIS: ""})
 
         self.assertFalse(port.has_additional_binding(lsp))
+
+
+class TestGetAddressList(test_base.TestCase):
+    def test_get_list(self):
+        expected_list = ["mac", "ip1", "ip2"]
+        lsp = test_utils.create_row(
+            addresses=["mac ip1 ip2"])
+        observed = port.get_address_list(lsp)
+
+        self.assertListEqual(expected_list, observed)
+
+    def test_get_list_strip(self):
+        expected_list = ["mac", "ip1", "ip2"]
+        lsp = test_utils.create_row(
+            addresses=["  mac ip1 ip2 "])
+        observed = port.get_address_list(lsp)
+
+        self.assertListEqual(expected_list, observed)
+
+    def test_get_list_no_addresses(self):
+        lsp = test_utils.create_row()
+        observed = port.get_address_list(lsp)
+
+        self.assertListEqual([], observed)
+
+    def test_get_list_empty(self):
+        lsp = test_utils.create_row(addresses=[])
+        observed = port.get_address_list(lsp)
+
+        self.assertListEqual([], observed)
+
+    def test_get_list_empty_string(self):
+        lsp = test_utils.create_row(addresses=[""])
+        observed = port.get_address_list(lsp)
+
+        self.assertListEqual([], observed)
+
+
+class TestGetMacFromLsp(test_base.TestCase):
+    def test_get_mac(self):
+        mac = 'mac'
+        lsp = test_utils.create_row(
+            addresses=["%s ip1 ip2" % mac])
+        observed_mac = port.get_mac_from_lsp(lsp)
+
+        self.assertEqual(mac, observed_mac)
+
+    def test_get_mac_empty_list(self):
+        lsp = test_utils.create_row(
+            addresses=[])
+        self.assertRaises(
+            exceptions.MacAddressNotFound, port.get_mac_from_lsp, lsp)
+
+
+class TestGetIpsFromLsp(test_base.TestCase):
+    def test_get_ips(self):
+        ips = ['ip1', 'ip2']
+        lsp = test_utils.create_row(
+            addresses=["mac " + ' '.join(ips)])
+        observed_ips = port.get_ips_from_lsp(lsp)
+
+        self.assertEqual(ips, observed_ips)
+
+    def test_get_ips_empty_list(self):
+        lsp = test_utils.create_row(
+            addresses=[])
+        self.assertRaises(
+            exceptions.IpAddressNotFound, port.get_ips_from_lsp, lsp)