Merge "Fix placement of lsp when external_ids not in sync"

ovn_bgp_agent/drivers/openstack/utils/driver_utils.py

@@ -40,6 +40,40 @@ def get_addr_scopes(port):
         }
 
 
+def get_port_chassis(port, chassis,
+                     default_port_type=constants.OVN_VM_VIF_PORT_TYPE):
+    # row.options['requested-chassis'] superseeds the id in external_ids.
+    # Since it is not used for virtual ports by ovn, this option will be
+    # ignored for virtual ports.
+
+    # since 'old' rows could be used, it will not hold the type information
+    # if that is the case, please supply a default in the arguments.
+
+    port_type = getattr(port, 'type', default_port_type)
+    if (port_type not in [constants.OVN_VIRTUAL_VIF_PORT_TYPE] and
+            hasattr(port, 'options') and
+            port.options.get(constants.OVN_REQUESTED_CHASSIS)):
+
+        # requested-chassis can be a comma separated list,
+        # so lets only return our chassis if it is a list, to be able
+        # to do a == equal comparison
+        req_chassis = port.options[constants.OVN_REQUESTED_CHASSIS]
+        if chassis in req_chassis.split(','):
+            req_chassis = chassis
+
+        return req_chassis.split(',')[0], constants.OVN_CHASSIS_AT_OPTIONS
+
+    elif (hasattr(port, 'external_ids') and
+            port.external_ids.get(constants.OVN_HOST_ID_EXT_ID_KEY)):
+
+        return (
+            port.external_ids[constants.OVN_HOST_ID_EXT_ID_KEY],
+            constants.OVN_CHASSIS_AT_EXT_IDS
+        )
+
+    return None, None
+
+
 def check_name_prefix(entity, prefix):
     try:
         return entity.name.startswith(prefix)

ovn_bgp_agent/drivers/openstack/utils/ovn.py

@@ -28,6 +28,7 @@ from ovsdbapp.schema.ovn_northbound import impl_idl as nb_impl_idl
 from ovsdbapp.schema.ovn_southbound import impl_idl as sb_impl_idl
 
 from ovn_bgp_agent import constants
+from ovn_bgp_agent.drivers.openstack.utils import driver_utils
 from ovn_bgp_agent import exceptions
 from ovn_bgp_agent.utils import helpers
 
@@ -414,14 +415,10 @@ class OvsdbNbOvnIdl(nb_impl_idl.OvnNbApiIdlImpl, Backend):
         ports = []
         cmd = self.db_find_rows('Logical_Switch_Port', ('up', '=', True))
         for row in cmd.execute(check_error=True):
-            if (row.external_ids and
+            port_chassis, _ = driver_utils.get_port_chassis(row, chassis)
-                    row.external_ids.get(
+            if port_chassis == chassis:
-                        constants.OVN_HOST_ID_EXT_ID_KEY) == chassis):
-                ports.append(row)
-            elif (row.options and
-                    row.options.get(
-                        constants.OVN_REQUESTED_CHASSIS) == chassis):
                 ports.append(row)
+
         return ports
 
     def get_active_cr_lrp_on_chassis(self, chassis):

ovn_bgp_agent/drivers/openstack/watchers/base_watcher.py

@@ -97,30 +97,8 @@ class LSPChassisEvent(Event):
         return len(mac.strip().split(' ')) > 1
 
     def _get_chassis(self, row, default_type=constants.OVN_VM_VIF_PORT_TYPE):
-        # row.options['requested-chassis'] superseeds the id in external_ids.
+        return driver_utils.get_port_chassis(row, self.agent.chassis,
-        # Since it is not used for virtual ports by ovn, this option will be
+                                             default_port_type=default_type)
-        # ignored for virtual ports.
-
-        # since 'old' rows could be used, it will not hold the type information
-        # if that is the case, please supply a default in the arguments.
-        row_type = getattr(row, 'type', default_type)
-        if (row_type not in [constants.OVN_VIRTUAL_VIF_PORT_TYPE] and
-                hasattr(row, 'options') and
-                row.options.get(constants.OVN_REQUESTED_CHASSIS)):
-
-            # requested-chassis can be a comma separated list,
-            # so lets only return our chassis if it is a list, to be able to
-            # do a == equal comparison
-            req_chassis = row.options[constants.OVN_REQUESTED_CHASSIS]
-            if self.agent.chassis in req_chassis.split(','):
-                req_chassis = self.agent.chassis
-
-            return (req_chassis, constants.OVN_CHASSIS_AT_OPTIONS)
-        elif (hasattr(row, 'external_ids') and
-                row.external_ids.get(constants.OVN_HOST_ID_EXT_ID_KEY)):
-            return (row.external_ids[constants.OVN_HOST_ID_EXT_ID_KEY],
-                    constants.OVN_CHASSIS_AT_EXT_IDS)
-        return None, None
 
     def _has_additional_binding(self, row):
         if (hasattr(row, 'options') and

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

@@ -68,6 +68,44 @@ class TestDriverUtils(test_base.TestCase):
             constants.IP_VERSION_6: None,
         })
 
+    def test_get_port_chassis_from_options(self):
+        my_host = 'foo-host'
+
+        # it is a VM port type, should use options field.
+        row = utils.create_row(
+            external_ids={constants.OVN_HOST_ID_EXT_ID_KEY: 'bar-host'},
+            options={constants.OVN_REQUESTED_CHASSIS: my_host})
+
+        self.assertEqual(driver_utils.get_port_chassis(row, chassis=my_host),
+                         (my_host, constants.OVN_CHASSIS_AT_OPTIONS))
+
+    def test_get_port_chassis_from_external_ids(self):
+        my_host = 'foo-host'
+
+        # it is a VM port type, should use options field.
+        row = utils.create_row(
+            external_ids={constants.OVN_HOST_ID_EXT_ID_KEY: my_host})
+
+        self.assertEqual(driver_utils.get_port_chassis(row, chassis=my_host),
+                         (my_host, constants.OVN_CHASSIS_AT_EXT_IDS))
+
+    def test_get_port_chassis_from_external_ids_virtual_port(self):
+        my_host = 'foo-host'
+
+        # it is a VM port type, should use options field.
+        row = utils.create_row(
+            external_ids={constants.OVN_HOST_ID_EXT_ID_KEY: my_host},
+            options={constants.OVN_REQUESTED_CHASSIS: 'bar-host'},
+            type=constants.OVN_VIRTUAL_VIF_PORT_TYPE)
+
+        self.assertEqual(driver_utils.get_port_chassis(row, chassis=my_host),
+                         (my_host, constants.OVN_CHASSIS_AT_EXT_IDS))
+
+    def test_get_port_chassis_no_information(self):
+        row = utils.create_row()
+        self.assertEqual(driver_utils.get_port_chassis(row, chassis='foo'),
+                         (None, None))
+
     def test_check_name_prefix(self):
         lb = utils.create_row(name='some-name')
         self.assertTrue(driver_utils.check_name_prefix(lb, 'some'))