Merge "Fix race condition retrieving logical router rows"

ovn_octavia_provider/helper.py

@@ -302,8 +302,8 @@ class OvnProviderHelper():
                     [])}
 
     def _find_lb_in_table(self, lb, table):
-        return [item for item in self.ovn_nbdb_api.tables[table].rows.values()
-                if lb in item.load_balancer]
+        return self.ovn_nbdb_api.find_lb_in_table(
+            lb, table).execute(check_error=True)
 
     def request_handler(self):
         while True:
@@ -636,7 +636,8 @@ class OvnProviderHelper():
         if not lrp_name:
             return
 
-        for lr in self.ovn_nbdb_api.tables['Logical_Router'].rows.values():
+        lrs = self.ovn_nbdb_api.get_lrs().execute(check_error=True)
+        for lr in lrs:
             for lrp in lr.ports:
                 if lrp.name == lrp_name:
                     return lr
@@ -991,12 +992,12 @@ class OvnProviderHelper():
                                             if_exists=True))
         lr_ref = ovn_lb.external_ids.get(ovn_const.LB_EXT_IDS_LR_REF_KEY, {})
         if lr_ref:
-            for lr in self.ovn_nbdb_api.tables[
-                    'Logical_Router'].rows.values():
-                if lr.name == lr_ref:
-                    commands.append(self.ovn_nbdb_api.lr_lb_del(
-                        lr.uuid, ovn_lb.uuid))
-                    break
+            try:
+                lr = self.ovn_nbdb_api.lookup('Logical_Router', lr_ref)
+                commands.append(self.ovn_nbdb_api.lr_lb_del(
+                    lr.uuid, ovn_lb.uuid))
+            except idlutils.RowNotFound:
+                pass
         # Delete LB from all Routers the LB is indirectly associated
         for lr in self._find_lb_in_table(ovn_lb, 'Logical_Router'):
             commands.append(

ovn_octavia_provider/ovsdb/impl_idl_ovn.py

@@ -16,8 +16,10 @@ import contextlib
 from neutron_lib import exceptions as n_exc
 from oslo_log import log
 from ovsdbapp.backend import ovs_idl
+from ovsdbapp.backend.ovs_idl import command
 from ovsdbapp.backend.ovs_idl import connection
 from ovsdbapp.backend.ovs_idl import idlutils
+from ovsdbapp.backend.ovs_idl import rowview
 from ovsdbapp.backend.ovs_idl import transaction as idl_trans
 from ovsdbapp.schema.ovn_northbound import impl_idl as nb_impl_idl
 from ovsdbapp.schema.ovn_southbound import impl_idl as sb_impl_idl
@@ -115,6 +117,26 @@ class OvsdbConnectionUnavailable(n_exc.ServiceUnavailable):
                 "'ovn_sb_connection' configuration options are correct.")
 
 
+class FindLbInTableCommand(command.ReadOnlyCommand):
+    def __init__(self, api, lb, table):
+        super().__init__(api)
+        self.lb = lb
+        self.table = table
+
+    def run_idl(self, txn):
+        self.result = [
+            rowview.RowView(item) for item in
+            self.api.tables[self.table].rows.values()
+            if self.lb in item.load_balancer]
+
+
+class GetLrsCommand(command.ReadOnlyCommand):
+    def run_idl(self, txn):
+        self.result = [
+            rowview.RowView(item) for item in
+            self.api.tables['Logical_Router'].rows.values()]
+
+
 class OvsdbNbOvnIdl(nb_impl_idl.OvnNbApiIdlImpl, Backend):
     def __init__(self, connection):
         super().__init__(connection)
@@ -144,6 +166,12 @@ class OvsdbNbOvnIdl(nb_impl_idl.OvnNbApiIdlImpl, Backend):
         except ovn_exc.RevisionConflict as e:
             LOG.info('Transaction aborted. Reason: %s', e)
 
+    def find_lb_in_table(self, lb, table):
+        return FindLbInTableCommand(self, lb, table)
+
+    def get_lrs(self):
+        return GetLrsCommand(self)
+
 
 class OvsdbSbOvnIdl(sb_impl_idl.OvnSbApiIdlImpl, Backend):
     def __init__(self, connection):

ovn_octavia_provider/ovsdb/ovsdb_monitor.py

@@ -66,6 +66,7 @@ class OvnIdl(BaseOvnIdl):
         # but not granted by the ovsdb-server.
         if self.is_lock_contended:
             return
+        row = idlutils.frozen_row(row)
         self.notify_handler.notify(event, row, updates)
 
     @abc.abstractmethod

ovn_octavia_provider/tests/unit/test_helper.py

@@ -558,8 +558,7 @@ class TestOvnProviderHelper(ovn_base.TestOvnOctaviaBase):
         net_cli.return_value.delete_port.return_value = None
         (self.helper.ovn_nbdb_api.ls_get.return_value.execute.
             return_value) = self.network
-        (self.helper.ovn_nbdb_api.tables['Logical_Router'].rows.
-            values.return_value) = [self.router]
+        self.helper.ovn_nbdb_api.lookup.return_value = self.router
         self.helper.lb_delete(self.ovn_lb)
         self.helper.ovn_nbdb_api.ls_lb_del.assert_called_once_with(
             self.network.uuid, self.ovn_lb.uuid)
@@ -1818,8 +1817,8 @@ class TestOvnProviderHelper(ovn_base.TestOvnOctaviaBase):
         ls = fakes.FakeOvsdbRow.create_one_ovsdb_row(
             attrs={'ports': [lsp]})
 
-        (self.helper.ovn_nbdb_api.tables['Logical_Router'].rows.
-            values.return_value) = [lr]
+        (self.helper.ovn_nbdb_api.get_lrs.return_value.
+            execute.return_value) = [lr]
         returned_lr = self.helper._find_lr_of_ls(ls)
         self.assertEqual(lr, returned_lr)
 
@@ -1841,8 +1840,8 @@ class TestOvnProviderHelper(ovn_base.TestOvnOctaviaBase):
         ls = fakes.FakeOvsdbRow.create_one_ovsdb_row(
             attrs={'ports': [lsp]})
 
-        (self.helper.ovn_nbdb_api.tables['Logical_Router'].rows.
-            values.return_value) = [lr]
+        (self.helper.ovn_nbdb_api.get_lrs.return_value.
+            execute.return_value) = [lr]
         returned_lr = self.helper._find_lr_of_ls(ls)
         self.assertEqual(lr, returned_lr)