Просмотр исходного кода

Handles failed Keystone trust deletion during task finalization

_set_tasks_execution_status calls keystone.delete_trust() while
finalizing a task / execution inside task_completed. If that call raises
an exception, the exception propagates all the way back to the worker's RPC
call, which then misinterprets its own successful task_completed() call
as a failure and reports an error for a task that had already finished
successfully. Log a warning and continue instead.

Mocks out `coriolis.keystone.delete_trust` during integration tests, as
we do not use Keystone for them.
Claudiu Belu 1 месяц назад
Родитель
Сommit
972bb736f6
2 измененных файлов с 20 добавлено и 2 удалено
  1. 17 1
      coriolis/conductor/rpc/server.py
  2. 3 1
      coriolis/tests/integration/base.py

+ 17 - 1
coriolis/conductor/rpc/server.py

@@ -2766,7 +2766,23 @@ class ConductorServerEndpoint(object):
                     previous_execution_status,
                     new_execution_status,
                 )
-                keystone.delete_trust(ctxt)
+                try:
+                    keystone.delete_trust(ctxt)
+                except Exception:
+                    # NOTE: a failed trust cleanup should not be allowed to propagate
+                    # out of here. This also runs inside task_completed, whose RPC
+                    # caller treats any exception as its own task failure, which would
+                    # cause it to erroneously report a failure for a task that already
+                    # finalized successfully.
+                    LOG.warning(
+                        "Failed to delete Keystone trust '%s' for Execution '%s' "
+                        "(action '%s'). The trust may be leaked and require manual "
+                        "cleanup. Error was: %s",
+                        ctxt.trust_id,
+                        execution.id,
+                        execution.action_id,
+                        utils.get_exception_details(),
+                    )
         else:
             LOG.debug(
                 "Not deallocating minion machines for Execution '%s' "

+ 3 - 1
coriolis/tests/integration/base.py

@@ -82,6 +82,8 @@ class CoriolisIntegrationTestBase(test_base.CoriolisBaseTestCase):
             # Prevent the test runner from being killed by Coriolis sending
             # SIGINT to in-process workers.
             "psutil.Process.send_signal",
+            # We're not using keystone for tests. This is called in a few places.
+            "coriolis.keystone.delete_trust",
         ]
         for thing in to_patch:
             patcher = mock.patch(thing)
@@ -214,7 +216,7 @@ class CoriolisIntegrationTestBase(test_base.CoriolisBaseTestCase):
             cls._client.minion_pools.deallocate_minion_pool(pool_id, force=True)
             cls._wait_for_pool(pool_id, MINION_DEALLOCATED_TERMINAL)
 
-        with mock.patch("coriolis.keystone.delete_trust"):
+        with mock.patch("coriolis.keystone.delete_trust", autospec=False):
             # When removing minion pools, this is also called.
             # There is no Keystone, so it needs to be mocked.
             cls._client.minion_pools.delete(pool_id)