Merge "Retry logical switch associations to load balancers"

ovn_octavia_provider/common/constants.py

@@ -15,6 +15,7 @@ from octavia_lib.common import constants
 
 # TODO(mjozefcz): Use those variables from neutron-lib once released.
 LRP_PREFIX = "lrp-"
+OVN_NAME_PREFIX = "neutron-"
 LB_VIP_PORT_PREFIX = "ovn-lb-vip-"
 OVN_PORT_NAME_EXT_ID_KEY = 'neutron:port_name'
 OVN_ROUTER_NAME_EXT_ID_KEY = 'neutron:router_name'

ovn_octavia_provider/common/utils.py

@@ -15,13 +15,22 @@ from oslo_utils import netutils
 from ovn_octavia_provider.common import constants
 
 
+def ovn_uuid(name):
+    # Get the UUID of a neutron OVN entry (neutron-<UUID>)
+    return name.replace(constants.OVN_NAME_PREFIX, '')
+
+
 def ovn_name(id):
     # The name of the OVN entry will be neutron-<UUID>
     # This is due to the fact that the OVN application checks if the name
     # is a UUID. If so then there will be no matches.
     # We prefix the UUID to enable us to use the Neutron UUID when
     # updating, deleting etc.
-    return 'neutron-%s' % id
+    # To be sure that just one prefix is used, we will check it before
+    # return concatenation.
+    if not id.startswith(constants.OVN_NAME_PREFIX):
+        return constants.OVN_NAME_PREFIX + '%s' % id
+    return id
 
 
 def ovn_lrouter_port_name(id):

ovn_octavia_provider/helper.py

@@ -29,6 +29,8 @@ from oslo_log import log as logging
 from oslo_serialization import jsonutils
 from ovs.stream import Stream
 from ovsdbapp.backend.ovs_idl import idlutils
+from ovsdbapp.schema.ovn_northbound import commands as cmd
+
 import tenacity
 
 from ovn_octavia_provider.common import clients
@@ -200,7 +202,6 @@ class OvnProviderHelper():
         # Network and Router references as pushed in the patch
         # https://github.com/openvswitch/ovs/commit
         # /612f80fa8ebf88dad2e204364c6c02b451dca36c
-        commands = []
         network = info['network']
         router = info['router']
 
@@ -210,14 +211,28 @@ class OvnProviderHelper():
         r_lb = set(router.load_balancer) - nw_lb
         # Delete all LB on N/W from Router
         for nlb in nw_lb:
-            commands.extend(self._update_lb_to_lr_association(nlb, router,
+            try:
-                                                              delete=True))
+                self._update_lb_to_lr_association(nlb, router, delete=True)
+            except idlutils.RowNotFound:
+                LOG.warning("The disassociation of loadbalancer %s to the "
+                            "logical router %s failed, trying step by step",
+                            nlb.uuid, router.uuid)
+                self._update_lb_to_lr_association_by_step(
+                    nlb, router, delete=True)
+
         # Delete all LB on Router from N/W
         for rlb in r_lb:
-            commands.append(self.ovn_nbdb_api.ls_lb_del(
+            try:
-                network.uuid, rlb.uuid))
+                self._update_lb_to_ls_association(
-        if commands:
+                    rlb,
-            self._execute_commands(commands)
+                    network_id=utils.ovn_uuid(network.name),
+                    associate=False,
+                    update_ls_ref=False)
+            except idlutils.RowNotFound:
+                LOG.warning("The disassociation of loadbalancer %s to the "
+                            "logical switch %s failed, just keep going on",
+                            rlb.uuid, utils.ovn_uuid(network.name))
+                pass
 
     def lb_create_lrp_assoc_handler(self, row):
         try:
@@ -231,21 +246,31 @@ class OvnProviderHelper():
                           'info': request_info})
 
     def lb_create_lrp_assoc(self, info):
-        commands = []
-
         router_lb = set(info['router'].load_balancer)
         network_lb = set(info['network'].load_balancer)
         # Add only those lb to routers which are unique to the network
         for lb in (network_lb - router_lb):
-            commands.extend(self._update_lb_to_lr_association(
+            try:
-                lb, info['router']))
+                self._update_lb_to_lr_association(lb, info['router'])
+            except idlutils.RowNotFound:
+                LOG.warning("The association of loadbalancer %s to the "
+                            "logical router %s failed, trying step by step",
+                            lb.uuid, info['router'].uuid)
+                self._update_lb_to_lr_association_by_step(lb, info['router'])
 
         # Add those lb to the network which are unique to the router
         for lb in (router_lb - network_lb):
-            commands.append(self.ovn_nbdb_api.ls_lb_add(
+            try:
-                            info['network'].uuid, lb.uuid, may_exist=True))
+                self._update_lb_to_ls_association(
-        if commands:
+                    lb,
-            self._execute_commands(commands)
+                    network_id=utils.ovn_uuid(info['network'].name),
+                    associate=True,
+                    update_ls_ref=False)
+            except idlutils.RowNotFound:
+                LOG.warning("The association of loadbalancer %s to the "
+                            "logical switch %s failed, just keep going on",
+                            lb.uuid, utils.ovn_uuid(info['network'].name))
+                pass
 
     def vip_port_update_handler(self, vip_lp):
         """Handler for VirtualIP port updates.
@@ -479,8 +504,24 @@ class OvnProviderHelper():
             for command in commands:
                 txn.add(command)
 
+    @tenacity.retry(
+        retry=tenacity.retry_if_exception_type(idlutils.RowNotFound),
+        wait=tenacity.wait_exponential(),
+        stop=tenacity.stop_after_attempt(3),
+        reraise=True)
     def _update_lb_to_ls_association(self, ovn_lb, network_id=None,
-                                     subnet_id=None, associate=True):
+                                     subnet_id=None, associate=True,
+                                     update_ls_ref=True):
+        # Note(froyo): Large topologies can change from the time we
+        # list the ls association commands and the execution, retry
+        # if this situation arises.
+        commands = self._get_lb_to_ls_association_commands(
+            ovn_lb, network_id, subnet_id, associate, update_ls_ref)
+        self._execute_commands(commands)
+
+    def _get_lb_to_ls_association_commands(self, ovn_lb, network_id=None,
+                                           subnet_id=None, associate=True,
+                                           update_ls_ref=True):
         """Update LB association with Logical Switch
 
            This function deals with updating the References of Logical Switch
@@ -549,10 +590,12 @@ class OvnProviderHelper():
             else:
                 ls_refs[ls_name] = ref_ct - 1
 
-        ls_refs = {ovn_const.LB_EXT_IDS_LS_REFS_KEY: jsonutils.dumps(ls_refs)}
+        if update_ls_ref:
-        commands.append(self.ovn_nbdb_api.db_set(
+            ls_refs = {
-            'Load_Balancer', ovn_lb.uuid,
+                ovn_const.LB_EXT_IDS_LS_REFS_KEY: jsonutils.dumps(ls_refs)}
-            ('external_ids', ls_refs)))
+            commands.append(self.ovn_nbdb_api.db_set(
+                'Load_Balancer', ovn_lb.uuid,
+                ('external_ids', ls_refs)))
 
         return commands
 
@@ -607,12 +650,44 @@ class OvnProviderHelper():
                                          ('external_ids', lr_rf)))
         return commands
 
+    @tenacity.retry(
+        retry=tenacity.retry_if_exception_type(idlutils.RowNotFound),
+        wait=tenacity.wait_exponential(),
+        stop=tenacity.stop_after_attempt(3),
+        reraise=True)
     def _update_lb_to_lr_association(self, ovn_lb, ovn_lr, delete=False):
+        # Note(froyo): Large topologies can change from the time we
+        # list the ls associated to lr until we execute the
+        # association command, retry if this situation arises.
+        commands = self._get_lb_to_lr_association_commands(
+            ovn_lb, ovn_lr, delete)
+        self._execute_commands(commands)
+
+    def _update_lb_to_lr_association_by_step(self, ovn_lb, ovn_lr,
+                                             delete=False):
+        # Note(froyo): just to make association commands step by
+        # step, in order to keep going on when LsLbAdd or LsLbDel
+        # happen.
+        commands = self._get_lb_to_lr_association_commands(
+            ovn_lb, ovn_lr, delete)
+        for command in commands:
+            try:
+                command.execute(check_error=True)
+            except idlutils.RowNotFound:
+                if isinstance(command, (cmd.LsLbAddCommand,
+                                        cmd.LsLbDelCommand)):
+                    LOG.warning('action lb to ls fail because ls '
+                                '%s is not found, keep going on...',
+                                getattr(command, 'switch', ''))
+                else:
+                    raise
+
+    def _get_lb_to_lr_association_commands(
+            self, ovn_lb, ovn_lr, delete=False):
         lr_ref = ovn_lb.external_ids.get(ovn_const.LB_EXT_IDS_LR_REF_KEY)
         if delete:
             return self._del_lb_to_lr_association(ovn_lb, ovn_lr, lr_ref)
-        else:
+        return self._add_lb_to_lr_association(ovn_lb, ovn_lr, lr_ref)
-            return self._add_lb_to_lr_association(ovn_lb, ovn_lr, lr_ref)
 
     def _find_ls_for_lr(self, router):
         neutron_client = clients.get_neutron_client()
@@ -868,16 +943,29 @@ class OvnProviderHelper():
                 loadbalancer[constants.ID],
                 protocol=protocol)
             ovn_lb = ovn_lb if protocol else ovn_lb[0]
-            commands = self._update_lb_to_ls_association(
+            # NOTE(froyo): This is the association of the lb to the VIP ls
+            # so this is executed right away
+            self._update_lb_to_ls_association(
                 ovn_lb, network_id=port['network_id'],
-                associate=True)
+                associate=True, update_ls_ref=True)
             ls_name = utils.ovn_name(port['network_id'])
             ovn_ls = self.ovn_nbdb_api.ls_get(ls_name).execute(
                 check_error=True)
             ovn_lr = self._find_lr_of_ls(ovn_ls, subnet.get('gateway_ip'))
             if ovn_lr:
-                commands.extend(self._update_lb_to_lr_association(
+                try:
-                    ovn_lb, ovn_lr))
+                    # NOTE(froyo): This is the association of the lb to the
+                    # router associated to VIP ls and all ls connected to that
+                    # router we try atomically, if it fails we will go step by
+                    # step, discarding the associations from lb to a
+                    # non-existent ls, but we will demand the association of
+                    # lb to lr
+                    self._update_lb_to_lr_association(ovn_lb, ovn_lr)
+                except idlutils.RowNotFound:
+                    LOG.warning("The association of loadbalancer %s to the "
+                                "logical router %s failed, trying step by "
+                                "step", ovn_lb.uuid, ovn_lr.uuid)
+                    self._update_lb_to_lr_association_by_step(ovn_lb, ovn_lr)
 
             # NOTE(mjozefcz): In case of LS references where passed -
             # apply LS to the new LB. That could happend in case we
@@ -893,11 +981,10 @@ class OvnProviderHelper():
                     # to duplicate.
                     if ls == ovn_ls.name:
                         continue
-                    commands.extend(self._update_lb_to_ls_association(
+                    self._update_lb_to_ls_association(
-                        ovn_lb, network_id=ls.replace('neutron-', ''),
+                        ovn_lb, network_id=utils.ovn_uuid(ls),
-                        associate=True))
+                        associate=True, update_ls_ref=True)
 
-            self._execute_commands(commands)
             operating_status = constants.ONLINE
             # The issue is that since OVN doesnt support any HMs,
             # we ideally should never put the status as 'ONLINE'
@@ -1024,6 +1111,9 @@ class OvnProviderHelper():
                 self.ovn_nbdb_api.lr_lb_del(lr.uuid, ovn_lb.uuid,
                                             if_exists=True))
         commands.append(self.ovn_nbdb_api.lb_del(ovn_lb.uuid))
+
+        # TODO(froyo): atomic process to execute all commands in a transaction
+        # if any of them fails the transaction is aborted
         self._execute_commands(commands)
         return status
 
@@ -1519,16 +1609,21 @@ class OvnProviderHelper():
 
         external_ids[pool_key] = pool_data[pool_key]
         commands.extend(self._refresh_lb_vips(ovn_lb.uuid, external_ids))
+        # Note (froyo): commands are now splitted to separate atomic process,
+        # leaving outside the not mandatory ones to allow add_member
+        # finish correctly
+        self._execute_commands(commands)
+
         subnet_id = member[constants.SUBNET_ID]
-        commands.extend(
+        self._update_lb_to_ls_association(
-            self._update_lb_to_ls_association(
+            ovn_lb, subnet_id=subnet_id, associate=True, update_ls_ref=True)
-                ovn_lb, subnet_id=subnet_id, associate=True))
 
         # Make sure that all logical switches related to logical router
         # are associated with the load balancer. This is needed to handle
         # potential race that happens when lrp and lb are created at the
         # same time.
         neutron_client = clients.get_neutron_client()
+        ovn_lr = None
         try:
             subnet = neutron_client.show_subnet(subnet_id)
             ls_name = utils.ovn_name(subnet['subnet']['network_id'])
@@ -1536,15 +1631,20 @@ class OvnProviderHelper():
                 check_error=True)
             ovn_lr = self._find_lr_of_ls(
                 ovn_ls, subnet['subnet'].get('gateway_ip'))
-            if ovn_lr:
-                commands.extend(self._update_lb_to_lr_association(
-                    ovn_lb, ovn_lr))
         except n_exc.NotFound:
             pass
         except idlutils.RowNotFound:
             pass
 
-        self._execute_commands(commands)
+        if ovn_lr:
+            try:
+                self._update_lb_to_lr_association(ovn_lb, ovn_lr)
+            except idlutils.RowNotFound:
+                LOG.warning("The association of loadbalancer %s to the "
+                            "logical router %s failed, trying step by step",
+                            ovn_lb.uuid, ovn_lr.uuid)
+                self._update_lb_to_lr_association_by_step(ovn_lb, ovn_lr)
+
         return member_info
 
     def member_create(self, member):
@@ -1614,11 +1714,10 @@ class OvnProviderHelper():
             external_ids[pool_key] = ",".join(existing_members)
             commands.extend(
                 self._refresh_lb_vips(ovn_lb.uuid, external_ids))
-            commands.extend(
-                self._update_lb_to_ls_association(
-                    ovn_lb, subnet_id=member.get(constants.SUBNET_ID),
-                    associate=False))
             self._execute_commands(commands)
+            self._update_lb_to_ls_association(
+                ovn_lb, subnet_id=member.get(constants.SUBNET_ID),
+                associate=False, update_ls_ref=True)
             return pool_status
         else:
             msg = f"Member {member[constants.ID]} not found in the pool"

ovn_octavia_provider/tests/functional/base.py

@@ -319,6 +319,77 @@ class TestOvnOctaviaBase(base.TestOVNFunctionalBase,
         expected_lbs = self._make_expected_lbs(lb_data)
         self._validate_loadbalancers(expected_lbs)
 
+    def _create_load_balancer_custom_lr_ls_and_validate(
+        self, admin_state_up=True, create_router=True,
+            force_retry_ls_to_lr_assoc=True):
+
+        self._o_driver_lib.update_loadbalancer_status.reset_mock()
+        r_id = self._create_router('r1') if create_router else None
+
+        net_info = []
+        net_info.append(self._create_net("n1", "10.0.1.0/24", r_id))
+        net_info.append(self._create_net("n2", "10.0.2.0/24", r_id))
+        net_info.append(self._create_net("n3", "10.0.3.0/24", r_id))
+
+        lb_data = {}
+        lb_data['model'] = self._create_lb_model(
+            vip=net_info[0][2],
+            vip_network_id=net_info[0][0],
+            vip_subnet_id=net_info[0][1],
+            vip_port_id=net_info[0][3],
+            admin_state_up=admin_state_up)
+
+        lb_data[ovn_const.LB_EXT_IDS_LR_REF_KEY] = \
+            (ovn_const.LR_REF_KEY_HEADER + r_id)
+        lb_data['vip_net_info'] = net_info[0]
+        lb_data[ovn_const.LB_EXT_IDS_LS_REFS_KEY] = {}
+        lb_data['listeners'] = []
+        lb_data['pools'] = []
+        self._update_ls_refs(lb_data, net_info[0][0])
+        ls = [ovn_const.LR_REF_KEY_HEADER + net[0] for net in net_info]
+
+        if force_retry_ls_to_lr_assoc:
+            ls_foo = copy.deepcopy(ls)
+            ls_foo.append('neutron-foo')
+            self.ovn_driver._ovn_helper._find_ls_for_lr = mock.MagicMock()
+            self.ovn_driver._ovn_helper._find_ls_for_lr.side_effect = \
+                [ls_foo, ls]
+
+        self.ovn_driver.loadbalancer_create(lb_data['model'])
+
+        name = '%s%s' % (ovn_const.LB_VIP_PORT_PREFIX,
+                         lb_data['model'].loadbalancer_id)
+        self.driver.update_port(
+            self.context, net_info[0][3], {'port': {'name': name}})
+
+        if lb_data['model'].admin_state_up:
+            expected_status = {
+                'loadbalancers': [{"id": lb_data['model'].loadbalancer_id,
+                                   "provisioning_status": "ACTIVE",
+                                   "operating_status": o_constants.ONLINE}]
+            }
+        else:
+            expected_status = {
+                'loadbalancers': [{"id": lb_data['model'].loadbalancer_id,
+                                   "provisioning_status": "ACTIVE",
+                                   "operating_status": o_constants.OFFLINE}]
+            }
+        self._wait_for_status_and_validate(lb_data, [expected_status])
+        self.assertTrue(
+            self._is_lb_associated_to_ls(
+                lb_data['model'].loadbalancer_id,
+                ovn_const.LR_REF_KEY_HEADER + net_info[0][0]))
+
+        # NOTE(froyo): Just to check all net connected to lr have a
+        # reference to lb
+        for net_id in ls:
+            self.assertTrue(
+                self._is_lb_associated_to_ls(
+                    lb_data['model'].loadbalancer_id,
+                    net_id))
+
+        return lb_data
+
     def _create_load_balancer_and_validate(self, lb_info,
                                            admin_state_up=True,
                                            only_model=False,

ovn_octavia_provider/tests/functional/test_driver.py

@@ -34,6 +34,16 @@ class TestOvnOctaviaProviderDriver(ovn_base.TestOvnOctaviaBase):
              'cidr': '10.0.0.0/24'}, admin_state_up=False)
         self._delete_load_balancer_and_validate(lb_data)
 
+    def test_create_lb_custom_network(self):
+        self._create_load_balancer_custom_lr_ls_and_validate(
+            admin_state_up=True, create_router=True,
+            force_retry_ls_to_lr_assoc=False)
+
+    def test_create_lb_custom_network_retry(self):
+        self._create_load_balancer_custom_lr_ls_and_validate(
+            admin_state_up=True, create_router=True,
+            force_retry_ls_to_lr_assoc=True)
+
     def test_delete_lb_on_nonexisting_lb(self):
         # LoadBalancer doesnt exist anymore, so just create a model and delete
         lb_data = self._create_load_balancer_and_validate(

ovn_octavia_provider/tests/unit/test_helper.py

@@ -143,11 +143,27 @@ class TestOvnProviderHelper(ovn_base.TestOvnOctaviaBase):
             '_update_lb_to_ls_association',
             return_value=[])
         self._update_lb_to_ls_association.start()
+        self._get_lb_to_ls_association_commands = mock.patch.object(
+            self.helper,
+            '_get_lb_to_ls_association_commands',
+            return_value=[])
+        self._get_lb_to_ls_association_commands.start()
         self._update_lb_to_lr_association = mock.patch.object(
             self.helper,
             '_update_lb_to_lr_association',
             return_value=[])
         self._update_lb_to_lr_association.start()
+        self._get_lb_to_lr_association_commands = mock.patch.object(
+            self.helper,
+            '_get_lb_to_lr_association_commands',
+            return_value=[])
+        self._get_lb_to_lr_association_commands.start()
+        self._update_lb_to_lr_association_by_step = \
+            mock.patch.object(
+                self.helper,
+                '_update_lb_to_lr_association_by_step',
+                return_value=[])
+        self._update_lb_to_lr_association_by_step.start()
 
         # NOTE(mjozefcz): Create foo router and network.
         net_id = uuidutils.generate_uuid()
@@ -491,8 +507,10 @@ class TestOvnProviderHelper(ovn_base.TestOvnOctaviaBase):
             selection_fields=['ip_src', 'ip_dst', 'tp_src', 'tp_dst'])
         self.helper._update_lb_to_ls_association.assert_has_calls([
             mock.call(self.ovn_lb, associate=True,
-                      network_id=self.lb['vip_network_id']),
+                      network_id=self.lb['vip_network_id'],
-            mock.call(self.ovn_lb, associate=True, network_id='foo')])
+                      update_ls_ref=True),
+            mock.call(self.ovn_lb, associate=True, network_id='foo',
+                      update_ls_ref=True)])
 
     def test_lb_create_on_multi_protocol_UDP(self):
         self._test_lb_create_on_multi_protocol('UDP')
@@ -1579,22 +1597,21 @@ class TestOvnProviderHelper(ovn_base.TestOvnOctaviaBase):
         self.helper._update_lb_to_lr_association.assert_not_called()
         mock_execute.assert_not_called()
 
-    @mock.patch.object(ovn_helper.OvnProviderHelper, '_execute_commands')
+    def test_lb_delete_lrp_assoc_no_net_lb_r_lb(self):
-    def test_lb_delete_lrp_assoc_no_net_lb_r_lb(self, mock_execute):
         info = {
             'network': self.network,
             'router': self.router,
         }
         self.network.load_balancer = []
         self.helper.lb_delete_lrp_assoc(info)
-        expected = [
+
-            self.helper.ovn_nbdb_api.ls_lb_del(
-                self.network.uuid,
-                self.router.load_balancer[0].uuid
-            ),
-        ]
         self.helper._update_lb_to_lr_association.assert_not_called()
-        mock_execute.assert_called_once_with(expected)
+        self.helper._update_lb_to_ls_association.assert_called_once_with(
+            self.router.load_balancer[0],
+            network_id=info['network'].uuid,
+            associate=False,
+            update_ls_ref=False
+        )
 
     @mock.patch.object(ovn_helper.OvnProviderHelper, '_execute_commands')
     def test_lb_delete_lrp_assoc_net_lb_no_r_lb(self, mock_execute):
@@ -1609,8 +1626,23 @@ class TestOvnProviderHelper(ovn_base.TestOvnOctaviaBase):
             self.network.load_balancer[0], self.router, delete=True
         )
 
-    @mock.patch.object(ovn_helper.OvnProviderHelper, '_execute_commands')
+    def test_lb_delete_lrp_assoc_r_lb_exception(self):
-    def test_lb_delete_lrp_assoc(self, mock_execute):
+        info = {
+            'network': self.network,
+            'router': self.router,
+        }
+        self.helper._update_lb_to_ls_association.side_effect = [
+            idlutils.RowNotFound]
+        with self.assertLogs(level='WARN') as cm:
+            self.helper.lb_delete_lrp_assoc(info)
+            self.assertEqual(
+                cm.output,
+                ['WARNING:ovn_octavia_provider.helper:'
+                 'The disassociation of loadbalancer '
+                 '%s to the logical switch %s failed, just keep going on'
+                 % (self.router.load_balancer[0].uuid, self.network.uuid)])
+
+    def test_lb_delete_lrp_assoc(self):
         info = {
             'network': self.network,
             'router': self.router,
@@ -1619,13 +1651,28 @@ class TestOvnProviderHelper(ovn_base.TestOvnOctaviaBase):
         self.helper._update_lb_to_lr_association.assert_called_once_with(
             self.network.load_balancer[0], self.router, delete=True
         )
-        expected = [
+        self.helper._update_lb_to_ls_association.assert_called_once_with(
-            self.helper.ovn_nbdb_api.ls_lb_del(
+            self.router.load_balancer[0],
-                self.network.uuid,
+            network_id=self.network.uuid,
-                self.router.load_balancer[0].uuid
+            associate=False, update_ls_ref=False
-            ),
+        )
-        ]
+
-        mock_execute.assert_called_once_with(expected)
+    def test_lb_delete_lrp_assoc_ls_by_step(self):
+        self._update_lb_to_ls_association.stop()
+        info = {
+            'network': self.network,
+            'router': self.router,
+        }
+        self.helper._update_lb_to_lr_association.side_effect = [
+            idlutils.RowNotFound]
+        self.helper.lb_delete_lrp_assoc(info)
+        self.helper._update_lb_to_lr_association.assert_called_once_with(
+            self.network.load_balancer[0], self.router, delete=True
+        )
+        self.helper._update_lb_to_lr_association_by_step \
+            .assert_called_once_with(
+                self.network.load_balancer[0],
+                self.router, delete=True)
 
     def test_lb_create_lrp_assoc_handler(self):
         lrp = fakes.FakeOvsdbRow.create_one_ovsdb_row()
@@ -1648,8 +1695,7 @@ class TestOvnProviderHelper(ovn_base.TestOvnOctaviaBase):
         self.helper.lb_create_lrp_assoc_handler(lrp)
         self.mock_add_request.assert_not_called()
 
-    @mock.patch.object(ovn_helper.OvnProviderHelper, '_execute_commands')
+    def test_lb_create_lrp_assoc(self):
-    def test_lb_create_lrp_assoc(self, mock_execute):
         info = {
             'network': self.network,
             'router': self.router,
@@ -1658,16 +1704,41 @@ class TestOvnProviderHelper(ovn_base.TestOvnOctaviaBase):
         self.helper._update_lb_to_lr_association.assert_called_once_with(
             self.network.load_balancer[0], self.router
         )
-        expected = [
-            self.helper.ovn_nbdb_api.ls_lb_add(
-                self.network.uuid,
-                self.router.load_balancer[0].uuid
-            ),
-        ]
-        mock_execute.assert_called_once_with(expected)
 
-    @mock.patch.object(ovn_helper.OvnProviderHelper, '_execute_commands')
+    def test_lb_create_lrp_assoc_r_lb_exception(self):
-    def test_lb_create_lrp_assoc_uniq_lb(self, mock_execute):
+        info = {
+            'network': self.network,
+            'router': self.router,
+        }
+        self.helper._update_lb_to_ls_association.side_effect = [
+            idlutils.RowNotFound]
+        with self.assertLogs(level='WARN') as cm:
+            self.helper.lb_create_lrp_assoc(info)
+            self.assertEqual(
+                cm.output,
+                ['WARNING:ovn_octavia_provider.helper:'
+                 'The association of loadbalancer '
+                 '%s to the logical switch %s failed, just keep going on'
+                 % (self.router.load_balancer[0].uuid, self.network.uuid)])
+
+    def test_lb_create_lrp_assoc_ls_by_step(self):
+        self._update_lb_to_ls_association.stop()
+        info = {
+            'network': self.network,
+            'router': self.router,
+        }
+        self.helper._update_lb_to_lr_association.side_effect = [
+            idlutils.RowNotFound]
+        self.helper.lb_create_lrp_assoc(info)
+        self.helper._update_lb_to_lr_association.assert_called_once_with(
+            self.network.load_balancer[0], self.router
+        )
+        self.helper._update_lb_to_lr_association_by_step \
+            .assert_called_once_with(
+                self.network.load_balancer[0],
+                self.router)
+
+    def test_lb_create_lrp_assoc_uniq_lb(self):
         info = {
             'network': self.network,
             'router': self.router,
@@ -1676,7 +1747,6 @@ class TestOvnProviderHelper(ovn_base.TestOvnOctaviaBase):
         self.network.load_balancer = self.router.load_balancer
         self.helper.lb_create_lrp_assoc(info)
         self.helper._update_lb_to_lr_association.assert_not_called()
-        mock_execute.assert_not_called()
 
     def test__find_lb_in_ls(self):
         net_lb = self.helper._find_lb_in_ls(self.network)
@@ -1744,9 +1814,10 @@ class TestOvnProviderHelper(ovn_base.TestOvnOctaviaBase):
         ovn_helper.OvnProviderHelper, '_del_lb_to_lr_association')
     @mock.patch.object(
         ovn_helper.OvnProviderHelper, '_add_lb_to_lr_association')
-    def test__update_lb_to_lr_association(self, add, delete):
+    def test__get_lb_to_lr_association_commands(self, add, delete):
-        self._update_lb_to_lr_association.stop()
+        self._get_lb_to_lr_association_commands.stop()
-        self.helper._update_lb_to_lr_association(self.ref_lb1, self.router)
+        self.helper._get_lb_to_lr_association_commands(
+            self.ref_lb1, self.router)
         lr_ref = self.ref_lb1.external_ids.get(
             ovn_const.LB_EXT_IDS_LR_REF_KEY)
         add.assert_called_once_with(self.ref_lb1, self.router, lr_ref)
@@ -1756,9 +1827,39 @@ class TestOvnProviderHelper(ovn_base.TestOvnOctaviaBase):
         ovn_helper.OvnProviderHelper, '_del_lb_to_lr_association')
     @mock.patch.object(
         ovn_helper.OvnProviderHelper, '_add_lb_to_lr_association')
-    def test__update_lb_to_lr_association_delete(self, add, delete):
+    def test__get_lb_to_lr_association_commands_delete(self, add, delete):
-        self._update_lb_to_lr_association.stop()
+        self._get_lb_to_lr_association_commands.stop()
-        self.helper._update_lb_to_lr_association(
+        self.helper._get_lb_to_lr_association_commands(
+            self.ref_lb1, self.router, delete=True)
+        lr_ref = self.ref_lb1.external_ids.get(
+            ovn_const.LB_EXT_IDS_LR_REF_KEY)
+        add.assert_not_called()
+        delete.assert_called_once_with(self.ref_lb1, self.router, lr_ref)
+
+    @mock.patch.object(
+        ovn_helper.OvnProviderHelper, '_del_lb_to_lr_association')
+    @mock.patch.object(
+        ovn_helper.OvnProviderHelper, '_add_lb_to_lr_association')
+    def test__get_lb_to_lr_association_commands_by_step(
+            self, add, delete):
+        self._update_lb_to_lr_association_by_step.stop()
+        self._get_lb_to_lr_association_commands.stop()
+        self.helper._update_lb_to_lr_association_by_step(
+            self.ref_lb1, self.router)
+        lr_ref = self.ref_lb1.external_ids.get(
+            ovn_const.LB_EXT_IDS_LR_REF_KEY)
+        add.assert_called_once_with(self.ref_lb1, self.router, lr_ref)
+        delete.assert_not_called()
+
+    @mock.patch.object(
+        ovn_helper.OvnProviderHelper, '_del_lb_to_lr_association')
+    @mock.patch.object(
+        ovn_helper.OvnProviderHelper, '_add_lb_to_lr_association')
+    def test__get_lb_to_lr_association_commands_by_step_delete(
+            self, add, delete):
+        self._update_lb_to_lr_association_by_step.stop()
+        self._get_lb_to_lr_association_commands.stop()
+        self.helper._update_lb_to_lr_association_by_step(
             self.ref_lb1, self.router, delete=True)
         lr_ref = self.ref_lb1.external_ids.get(
             ovn_const.LB_EXT_IDS_LR_REF_KEY)
@@ -1916,17 +2017,19 @@ class TestOvnProviderHelper(ovn_base.TestOvnOctaviaBase):
             values.assert_not_called())
         self.assertIsNone(returned_lr)
 
-    def test__update_lb_to_ls_association_empty_network_and_subnet(self):
+    def test__get_lb_to_ls_association_command_empty_network_and_subnet(self):
-        self._update_lb_to_ls_association.stop()
+        self._get_lb_to_ls_association_commands.stop()
-        returned_commands = self.helper._update_lb_to_ls_association(
+        returned_commands = self.helper._get_lb_to_ls_association_commands(
-            self.ref_lb1, associate=True)
+            self.ref_lb1, associate=True, update_ls_ref=True)
         self.assertListEqual(returned_commands, [])
 
     def test__update_lb_to_ls_association_network(self):
         self._update_lb_to_ls_association.stop()
+        self._get_lb_to_ls_association_commands.stop()
 
         self.helper._update_lb_to_ls_association(
-            self.ref_lb1, network_id=self.network.uuid, associate=True)
+            self.ref_lb1, network_id=self.network.uuid,
+            associate=True, update_ls_ref=True)
 
         self.helper.ovn_nbdb_api.ls_get.assert_called_once_with(
             self.network.name)
@@ -1934,9 +2037,22 @@ class TestOvnProviderHelper(ovn_base.TestOvnOctaviaBase):
         self.helper.ovn_nbdb_api.db_set.assert_called_once_with(
             'Load_Balancer', self.ref_lb1.uuid, ('external_ids', ls_refs))
 
+    def test__update_lb_to_ls_association_network_no_update_ls_ref(self):
+        self._update_lb_to_ls_association.stop()
+        self._get_lb_to_ls_association_commands.stop()
+
+        self.helper._update_lb_to_ls_association(
+            self.ref_lb1, network_id=self.network.uuid,
+            associate=True, update_ls_ref=False)
+
+        self.helper.ovn_nbdb_api.ls_get.assert_called_once_with(
+            self.network.name)
+        self.helper.ovn_nbdb_api.db_set.assert_not_called()
+
     @mock.patch('ovn_octavia_provider.common.clients.get_neutron_client')
     def test__update_lb_to_ls_association_subnet(self, net_cli):
         self._update_lb_to_ls_association.stop()
+        self._get_lb_to_ls_association_commands.stop()
         subnet = fakes.FakeSubnet.create_one_subnet(
             attrs={'id': 'foo_subnet_id',
                    'name': 'foo_subnet_name',
@@ -1944,18 +2060,21 @@ class TestOvnProviderHelper(ovn_base.TestOvnOctaviaBase):
         net_cli.return_value.show_subnet.return_value = {
             'subnet': subnet}
         self.helper._update_lb_to_ls_association(
-            self.ref_lb1, subnet_id=subnet.id, associate=True)
+            self.ref_lb1, subnet_id=subnet.id,
+            associate=True, update_ls_ref=True)
         self.helper.ovn_nbdb_api.ls_get.assert_called_once_with(
             'neutron-foo_network_id')
 
     def test__update_lb_to_ls_association_empty_ls_refs(self):
         self._update_lb_to_ls_association.stop()
+        self._get_lb_to_ls_association_commands.stop()
         (self.helper.ovn_nbdb_api.ls_get.return_value.execute.
             return_value) = self.network
         self.ref_lb1.external_ids.pop('ls_refs')
 
         self.helper._update_lb_to_ls_association(
-            self.ref_lb1, network_id=self.network.uuid)
+            self.ref_lb1, network_id=self.network.uuid,
+            update_ls_ref=True)
 
         self.helper.ovn_nbdb_api.ls_lb_add.assert_called_once_with(
             self.network.uuid, self.ref_lb1.uuid, may_exist=True)
@@ -1965,11 +2084,13 @@ class TestOvnProviderHelper(ovn_base.TestOvnOctaviaBase):
 
     def test__update_lb_to_ls_association_no_ls(self):
         self._update_lb_to_ls_association.stop()
+        self._get_lb_to_ls_association_commands.stop()
         (self.helper.ovn_nbdb_api.ls_get.return_value.execute.
             side_effect) = [idlutils.RowNotFound]
 
-        returned_commands = self.helper._update_lb_to_ls_association(
+        returned_commands = self.helper._get_lb_to_ls_association_commands(
-            self.ref_lb1, network_id=self.network.uuid)
+            self.ref_lb1, network_id=self.network.uuid,
+            update_ls_ref=True)
 
         self.helper.ovn_nbdb_api.ls_get.assert_called_once_with(
             self.network.name)
@@ -1977,11 +2098,13 @@ class TestOvnProviderHelper(ovn_base.TestOvnOctaviaBase):
 
     def test__update_lb_to_ls_association_network_disassociate(self):
         self._update_lb_to_ls_association.stop()
+        self._get_lb_to_ls_association_commands.stop()
         (self.helper.ovn_nbdb_api.ls_get.return_value.execute.
             return_value) = self.network
 
         self.helper._update_lb_to_ls_association(
-            self.ref_lb1, network_id=self.network.uuid, associate=False)
+            self.ref_lb1, network_id=self.network.uuid,
+            associate=False, update_ls_ref=True)
 
         self.helper.ovn_nbdb_api.ls_get.assert_called_once_with(
             self.network.name)
@@ -1991,13 +2114,65 @@ class TestOvnProviderHelper(ovn_base.TestOvnOctaviaBase):
         self.helper.ovn_nbdb_api.ls_lb_del.assert_called_once_with(
             self.network.uuid, self.ref_lb1.uuid, if_exists=True)
 
+    def test__update_lb_to_ls_association_net_disassoc_no_update_ls_ref(self):
+        self._update_lb_to_ls_association.stop()
+        self._get_lb_to_ls_association_commands.stop()
+        (self.helper.ovn_nbdb_api.ls_get.return_value.execute.
+            return_value) = self.network
+
+        self.helper._update_lb_to_ls_association(
+            self.ref_lb1, network_id=self.network.uuid,
+            associate=False, update_ls_ref=False)
+
+        self.helper.ovn_nbdb_api.ls_get.assert_called_once_with(
+            self.network.name)
+        self.helper.ovn_nbdb_api.db_set.assert_not_called()
+        self.helper.ovn_nbdb_api.ls_lb_del.assert_called_once_with(
+            self.network.uuid, self.ref_lb1.uuid, if_exists=True)
+
+    def test__update_lb_to_ls_association_dissasoc_net_not_assoc(self):
+        self._update_lb_to_ls_association.stop()
+        self._get_lb_to_ls_association_commands.stop()
+        (self.helper.ovn_nbdb_api.ls_get.return_value.execute.
+            return_value) = self.network
+
+        self.helper._update_lb_to_ls_association(
+            self.ref_lb1, network_id='foo',
+            associate=False, update_ls_ref=False)
+
+        self.helper.ovn_nbdb_api.ls_get.assert_called_once_with(
+            'neutron-foo')
+        self.helper.ovn_nbdb_api.db_set.assert_not_called()
+        self.helper.ovn_nbdb_api.ls_lb_del.assert_not_called()
+
+    def test__update_lb_to_ls_association_net_ls_ref_wrong_format(self):
+        self._update_lb_to_ls_association.stop()
+        self._get_lb_to_ls_association_commands.stop()
+
+        (self.helper.ovn_nbdb_api.ls_get.return_value.execute.
+            return_value) = self.network
+
+        self.ref_lb1.external_ids.update({
+            ovn_const.LB_EXT_IDS_LS_REFS_KEY:
+                '{\"neutron-%s\"}'})
+
+        self.helper._update_lb_to_ls_association(
+            self.ref_lb1, network_id=self.network.uuid,
+            associate=False, update_ls_ref=False)
+
+        self.helper.ovn_nbdb_api.ls_get.assert_called_once_with(
+            self.network.name)
+        self.helper.ovn_nbdb_api.db_set.assert_not_called()
+
     def test__update_lb_to_ls_association_network_dis_ls_not_found(self):
         self._update_lb_to_ls_association.stop()
+        self._get_lb_to_ls_association_commands.stop()
         (self.helper.ovn_nbdb_api.ls_get.return_value.execute.
             side_effect) = [idlutils.RowNotFound]
 
         self.helper._update_lb_to_ls_association(
-            self.ref_lb1, network_id=self.network.uuid, associate=False)
+            self.ref_lb1, network_id=self.network.uuid,
+            associate=False, update_ls_ref=True)
 
         self.helper.ovn_nbdb_api.ls_get.assert_called_once_with(
             self.network.name)
@@ -2011,10 +2186,12 @@ class TestOvnProviderHelper(ovn_base.TestOvnOctaviaBase):
             self, net_cli):
         net_cli.return_value.show_subnet.side_effect = n_exc.NotFound
         self._update_lb_to_ls_association.stop()
+        self._get_lb_to_ls_association_commands.stop()
         (self.helper.ovn_nbdb_api.ls_get.return_value.execute.
             return_value) = self.network
         self.helper._update_lb_to_ls_association(
-            self.ref_lb1, subnet_id='foo', associate=False)
+            self.ref_lb1, subnet_id='foo',
+            associate=False, update_ls_ref=True)
         self.helper.ovn_nbdb_api.ls_get.assert_not_called()
         self.helper.ovn_nbdb_api.db_set.assert_not_called()
         self.helper.ovn_nbdb_api.ls_lb_del.assert_not_called()
@@ -2026,13 +2203,15 @@ class TestOvnProviderHelper(ovn_base.TestOvnOctaviaBase):
         self.ref_lb1.external_ids.pop('ls_refs')
 
         self.helper._update_lb_to_ls_association(
-            self.ref_lb1, network_id=self.network.uuid, associate=False)
+            self.ref_lb1, network_id=self.network.uuid,
+            associate=False, update_ls_ref=True)
 
         self.helper.ovn_nbdb_api.ls_lb_del.assert_not_called()
         self.helper.ovn_nbdb_api.db_set.assert_not_called()
 
     def test__update_lb_to_ls_association_disassoc_multiple_refs(self):
         self._update_lb_to_ls_association.stop()
+        self._get_lb_to_ls_association_commands.stop()
         (self.helper.ovn_nbdb_api.ls_get.return_value.execute.
             return_value) = self.network
         # multiple refs
@@ -2040,7 +2219,8 @@ class TestOvnProviderHelper(ovn_base.TestOvnOctaviaBase):
         self.ref_lb1.external_ids.update(ls_refs)
 
         self.helper._update_lb_to_ls_association(
-            self.ref_lb1, network_id=self.network.uuid, associate=False)
+            self.ref_lb1, network_id=self.network.uuid,
+            associate=False, update_ls_ref=True)
 
         self.helper.ovn_nbdb_api.ls_get.assert_called_once_with(
             self.network.name)
@@ -2048,6 +2228,26 @@ class TestOvnProviderHelper(ovn_base.TestOvnOctaviaBase):
         self.helper.ovn_nbdb_api.db_set.assert_called_once_with(
             'Load_Balancer', self.ref_lb1.uuid, ('external_ids', exp_ls_refs))
 
+    @mock.patch.object(ovn_helper.OvnProviderHelper, '_execute_commands')
+    def test__update_lb_to_ls_association_retry(self, execute):
+        self._update_lb_to_ls_association.stop()
+        self._get_lb_to_ls_association_commands.stop()
+        self.helper._update_lb_to_ls_association(
+            self.ref_lb1, network_id=self.network.uuid)
+        expected = self.helper._get_lb_to_ls_association_commands(
+            self.ref_lb1, network_id=self.network.uuid)
+        execute.assert_called_once_with(expected)
+
+    @mock.patch.object(ovn_helper.OvnProviderHelper, '_execute_commands')
+    def test__update_lb_to_ls_association_retry_failed(self, execute):
+        execute.side_effect = [idlutils.RowNotFound for _ in range(4)]
+        self._update_lb_to_ls_association.stop()
+        self.assertRaises(
+            idlutils.RowNotFound,
+            self.helper._update_lb_to_ls_association,
+            self.ref_lb1,
+            network_id=self.network.uuid)
+
     def test_logical_switch_port_update_event_vip_port(self):
         self.switch_port_event = ovn_event.LogicalSwitchPortUpdateEvent(
             self.helper)
@@ -2117,6 +2317,46 @@ class TestOvnProviderHelper(ovn_base.TestOvnOctaviaBase):
         self.switch_port_event.run(mock.ANY, row, mock.ANY)
         self.mock_add_request.assert_not_called()
 
+    @mock.patch.object(ovn_helper.OvnProviderHelper, '_execute_commands')
+    def test__update_lb_to_lr_association_retry(self, execute):
+        self._update_lb_to_lr_association.stop()
+        self._get_lb_to_lr_association_commands.stop()
+        self.helper._update_lb_to_lr_association(self.ref_lb1, self.router)
+        expected = self.helper._get_lb_to_lr_association_commands(
+            self.ref_lb1, self.router)
+        execute.assert_called_once_with(expected)
+
+    @mock.patch.object(ovn_helper.OvnProviderHelper, '_execute_commands')
+    def test__update_lb_to_lr_association_retry_failed(self, execute):
+        execute.side_effect = [idlutils.RowNotFound for _ in range(4)]
+        self._update_lb_to_lr_association.stop()
+        self.assertRaises(
+            idlutils.RowNotFound,
+            self.helper._update_lb_to_lr_association,
+            self.ref_lb1,
+            self.router)
+
+    def test__update_lb_to_lr_association_by_step(self):
+        self._get_lb_to_lr_association_commands.stop()
+        self._update_lb_to_lr_association_by_step.stop()
+        self.helper._update_lb_to_lr_association_by_step(
+            self.network.load_balancer[0],
+            self.router)
+        self.helper.ovn_nbdb_api.db_set.assert_called()
+        self.helper.ovn_nbdb_api.lr_lb_add.assert_called()
+
+    def test__update_lb_to_lr_association_by_step_exception_raise(
+            self):
+        self._get_lb_to_lr_association_commands.stop()
+        self._update_lb_to_lr_association_by_step.stop()
+        (self.helper.ovn_nbdb_api.db_set.return_value.execute.
+            side_effect) = [idlutils.RowNotFound]
+        self.assertRaises(
+            idlutils.RowNotFound,
+            self.helper._update_lb_to_lr_association_by_step,
+            self.network.load_balancer[0],
+            self.router)
+
     @mock.patch('ovn_octavia_provider.helper.OvnProviderHelper.'
                 '_find_ovn_lbs')
     def test_vip_port_update_handler_multiple_lbs(self, lb):