From c406f7db5457d81e0f864ce82f232c21420fec5d Mon Sep 17 00:00:00 2001 From: Fernando Royo Date: Wed, 14 Sep 2022 15:52:43 +0200 Subject: [PATCH] Avoid LB ERROR status on delete when LS/LR are already deleted The delete operation of an LB lists all references to it (LS and LR) and in a single transaction deletes them together with the LB itself. If any of those deletions raise a RowNotFound exception, the execution will report ERROR status. This patch will reduce those situations, rechecking the command that triggers the RowNotFound exception and passing in case a LS or a LR was the culprit, deleting finally the LB and reporting as a DELETED status. In other case, the exception is raised and finally the LB will be reported as ERROR status. Closes-Bug:#1990129 Change-Id: I91b29add2a5d2ca3d4eff4b8d6a88be16c42be51 (cherry picked from commit f9ccb4c48627a200ef27dde60b01bccdccac4e45) --- ovn_octavia_provider/helper.py | 27 ++++++++++++++++--- .../tests/unit/test_helper.py | 24 +++++++++++++++++ 2 files changed, 48 insertions(+), 3 deletions(-) diff --git a/ovn_octavia_provider/helper.py b/ovn_octavia_provider/helper.py index e53ba94c..7d19dd22 100644 --- a/ovn_octavia_provider/helper.py +++ b/ovn_octavia_provider/helper.py @@ -1151,9 +1151,30 @@ class OvnProviderHelper(object): 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) + try: + self._execute_commands(commands) + except idlutils.RowNotFound: + # NOTE(froyo): If any of the Ls or Lr had been deleted between + # time to list and time to execute txn, we will received a + # RowNotFound exception, if this case we will run every command + # one by one passing exception in case the command is related to + # deletion of Ls or Lr already deleted. Any other case will raise + # exception and upper function will report the LB in ERROR status + for command in commands: + try: + command.execute(check_error=True) + except idlutils.RowNotFound: + if isinstance(command, (cmd.LsLbDelCommand)): + LOG.warning('delete lb from ls fail because ls ' + '%s is not found, keep going on...', + getattr(command, 'switch', '')) + elif isinstance(command, (cmd.LrLbDelCommand)): + LOG.warning('delete lb to lr fail because lr ' + '%s is not found, keep going on...', + getattr(command, 'router', '')) + else: + raise + return status def lb_update(self, loadbalancer): diff --git a/ovn_octavia_provider/tests/unit/test_helper.py b/ovn_octavia_provider/tests/unit/test_helper.py index bd4b65e6..98bfa8e9 100644 --- a/ovn_octavia_provider/tests/unit/test_helper.py +++ b/ovn_octavia_provider/tests/unit/test_helper.py @@ -685,6 +685,30 @@ class TestOvnProviderHelper(ovn_base.TestOvnOctaviaBase): self.ovn_lb.uuid) del_port.assert_not_called() + @mock.patch.object(ovn_helper.OvnProviderHelper, 'delete_vip_port') + def test_lb_delete_step_by_step(self, del_port): + self.helper.ovn_nbdb_api.lr_lb_del.side_effect = [idlutils.RowNotFound] + status = self.helper.lb_delete(self.lb) + self.assertEqual(status['loadbalancers'][0]['provisioning_status'], + constants.DELETED) + self.assertEqual(status['loadbalancers'][0]['operating_status'], + constants.OFFLINE) + self.helper.ovn_nbdb_api.lb_del.assert_called_once_with( + self.ovn_lb.uuid) + del_port.assert_called_once_with('foo_port') + + @mock.patch.object(ovn_helper.OvnProviderHelper, 'delete_vip_port') + def test_lb_delete_step_by_step_exception(self, del_port): + self.helper.ovn_nbdb_api.lb_del.side_effect = [idlutils.RowNotFound] + status = self.helper.lb_delete(self.lb) + self.assertEqual(status['loadbalancers'][0]['provisioning_status'], + constants.ERROR) + self.assertEqual(status['loadbalancers'][0]['operating_status'], + constants.ERROR) + self.helper.ovn_nbdb_api.lb_del.assert_called_once_with( + self.ovn_lb.uuid) + del_port.assert_not_called() + @mock.patch('ovn_octavia_provider.common.clients.get_neutron_client') @mock.patch.object(ovn_helper.OvnProviderHelper, 'delete_vip_port') def test_lb_delete_port_not_found(self, del_port, net_cli):