Search for active port binding when updating LSP host info

When updating LSP host info for an UP port, the code
always uses the first port binding in the DB entry
(db_port.port_bindings[0].host).

Since there could be multiple port bindings present,
search for the active one when determining which host
value to use.

Closes-bug: #2134504
Change-Id: Ia07610a17afb55b802290c8910a17dace7794c4f
Signed-off-by: Brian Haley <haleyb.dev@gmail.com>

Author: brianphaley

neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_client.py

@@ -262,7 +262,7 @@ def _get_port_dhcp_options(self, port, ip_version):
                     wait=tenacity.wait_random(min=2, max=3),
                     stop=tenacity.stop_after_attempt(3),
                     reraise=True)
-    def _wait_for_port_bindings_host(self, context, port_id):
+    def _wait_for_active_port_bindings_host(self, context, port_id):
         db_port = ml2_db.get_port(context, port_id)
         # This is already checked previously but, just to stay on
         # the safe side in case the port is deleted mid-operation
@@ -275,11 +275,18 @@ def _wait_for_port_bindings_host(self, context, port_id):
                 _('No port bindings information found for  '
                   'port %s') % port_id)
 
-        if not db_port.port_bindings[0].host:
+        active_binding = p_utils.get_port_binding_by_status_and_host(
+            db_port.port_bindings, const.ACTIVE)
+        if not active_binding:
+            raise RuntimeError(
+                _('No active port bindings information found for '
+                  'port %s') % port_id)
+
+        if not active_binding.host:
             raise RuntimeError(
                 _('No hosting information found for port %s') % port_id)
 
-        return db_port
+        return active_binding
 
     def update_lsp_host_info(self, context, db_port, up=True):
         """Update the binding hosting information for the LSP.
@@ -308,18 +315,23 @@ def update_lsp_host_info(self, context, db_port, up=True):
             if not db_port.port_bindings:
                 return
 
-            if not db_port.port_bindings[0].host:
+            # There could be more than one port binding present, we need
+            # to find the active one
+            active_binding = p_utils.get_port_binding_by_status_and_host(
+                db_port.port_bindings, const.ACTIVE)
+
+            if not active_binding or not active_binding.host:
                 # NOTE(lucasgomes): There might be a sync issue between
                 # the moment that this port was fetched from the database
                 # and the hosting information being set, retry a few times
                 try:
-                    db_port = self._wait_for_port_bindings_host(
+                    active_binding = self._wait_for_active_port_bindings_host(
                         context, db_port.id)
                 except RuntimeError as e:
                     LOG.warning(e)
                     return
 
-            host = db_port.port_bindings[0].host
+            host = active_binding.host
             ext_ids = ('external_ids',
                        {ovn_const.OVN_HOST_ID_EXT_ID_KEY: host})
             cmd.append(

neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_ovn_client.py

@@ -95,9 +95,13 @@ def setUp(self):
         super().setUp()
         self.get_plugin = mock.patch(
             'neutron_lib.plugins.directory.get_plugin').start()
+        self.get_pb_bsah = mock.patch(
+            'neutron_lib.plugins.utils.'
+            'get_port_binding_by_status_and_host').start()
 
         # Disable tenacity wait for UT
-        self.ovn_client._wait_for_port_bindings_host.retry.wait = wait_none()
+        self.ovn_client._wait_for_active_port_bindings_host.retry.wait = (
+            wait_none())
 
     def test__add_router_ext_gw_default_route(self):
         plugin = mock.MagicMock()
@@ -245,8 +249,9 @@ def test_update_lsp_host_info_up(self):
         context = mock.MagicMock()
         host_id = 'fake-binding-host-id'
         port_id = 'fake-port-id'
-        db_port = mock.Mock(
-            id=port_id, port_bindings=[mock.Mock(host=host_id)])
+        port_binding = mock.Mock(host=host_id)
+        db_port = mock.Mock(id=port_id, port_bindings=[port_binding])
+        self.get_pb_bsah.return_value = port_binding
 
         self.ovn_client.update_lsp_host_info(context, db_port)
 
@@ -258,14 +263,16 @@ def test_update_lsp_host_info_up_retry(self):
         context = mock.MagicMock()
         host_id = 'fake-binding-host-id'
         port_id = 'fake-port-id'
+        port_binding = mock.Mock(host=host_id)
+        port_binding_no_host = mock.Mock(host="")
         db_port_no_host = mock.Mock(
-            id=port_id, port_bindings=[mock.Mock(host="")])
-        db_port = mock.Mock(
-            id=port_id, port_bindings=[mock.Mock(host=host_id)])
+            id=port_id, port_bindings=[port_binding_no_host])
+        self.get_pb_bsah.return_value = None
 
         with mock.patch.object(
-                self.ovn_client, '_wait_for_port_bindings_host') as mock_wait:
-            mock_wait.return_value = db_port
+                self.ovn_client,
+                '_wait_for_active_port_bindings_host') as mock_wait:
+            mock_wait.return_value = port_binding
             self.ovn_client.update_lsp_host_info(context, db_port_no_host)
 
             # Assert _wait_for_port_bindings_host was called
@@ -281,9 +288,11 @@ def test_update_lsp_host_info_up_retry_fail(self):
         port_id = 'fake-port-id'
         db_port_no_host = mock.Mock(
             id=port_id, port_bindings=[mock.Mock(host="")])
+        self.get_pb_bsah.return_value = None
 
         with mock.patch.object(
-                self.ovn_client, '_wait_for_port_bindings_host') as mock_wait:
+                self.ovn_client,
+                '_wait_for_active_port_bindings_host') as mock_wait:
             mock_wait.side_effect = RuntimeError("boom")
             self.ovn_client.update_lsp_host_info(context, db_port_no_host)
 
@@ -315,39 +324,45 @@ def test_update_lsp_host_info_trunk_subport(self):
         self.nb_idl.db_set.assert_not_called()
 
     @mock.patch.object(ml2_db, 'get_port')
-    def test__wait_for_port_bindings_host(self, mock_get_port):
+    def test__wait_for_active_port_bindings_host(self, mock_get_port):
         context = mock.MagicMock()
         host_id = 'fake-binding-host-id'
         port_id = 'fake-port-id'
+        port_binding = mock.Mock(host=host_id)
+        port_binding_no_host = mock.Mock(host="")
         db_port_no_host = mock.Mock(
-            id=port_id, port_bindings=[mock.Mock(host="")])
+            id=port_id, port_bindings=[port_binding_no_host])
         db_port = mock.Mock(
-            id=port_id, port_bindings=[mock.Mock(host=host_id)])
+            id=port_id, port_bindings=[port_binding])
+        # no active binding, no binding with host, binding with host
+        self.get_pb_bsah.side_effect = (None, port_binding_no_host,
+                                        port_binding)
 
-        mock_get_port.side_effect = (db_port_no_host, db_port)
+        mock_get_port.side_effect = (db_port_no_host, db_port_no_host, db_port)
 
-        ret = self.ovn_client._wait_for_port_bindings_host(
+        ret = self.ovn_client._wait_for_active_port_bindings_host(
             context, port_id)
 
-        self.assertEqual(ret, db_port)
+        self.assertEqual(ret, port_binding)
 
         expected_calls = [mock.call(context, port_id),
                           mock.call(context, port_id)]
         mock_get_port.assert_has_calls(expected_calls)
 
     @mock.patch.object(ml2_db, 'get_port')
-    def test__wait_for_port_bindings_host_fail(self, mock_get_port):
+    def test__wait_for_active_port_bindings_host_fail(self, mock_get_port):
         context = mock.MagicMock()
         port_id = 'fake-port-id'
         db_port_no_pb = mock.Mock(id=port_id, port_bindings=[])
         db_port_no_host = mock.Mock(
             id=port_id, port_bindings=[mock.Mock(host="")])
+        self.get_pb_bsah.return_value = None
 
         mock_get_port.side_effect = (
             db_port_no_pb, db_port_no_host, db_port_no_host)
 
         self.assertRaises(
-            RuntimeError, self.ovn_client._wait_for_port_bindings_host,
+            RuntimeError, self.ovn_client._wait_for_active_port_bindings_host,
             context, port_id)
 
         expected_calls = [mock.call(context, port_id),