From 6a0277af9c6d2ecf942ee358313c7e56edab003f Mon Sep 17 00:00:00 2001 From: Pujitha Paladugu <10557236+pujitha24@users.noreply.github.com> Date: Sat, 17 Oct 2026 19:33:52 -0700 Subject: [PATCH] Don't mask await_lock body exceptions as FileLockError Motivation: salt.utils.files.await_lock yields inside a try whose broad `except Exception` re-raised everything as FileLockError. Any exception raised by the body of `async with await_lock(...)` was therefore reported by the minion as "Failed to acquire job_queue lock ... queuing anyway", with the real traceback lost. The minion's handler also logged without the exception. Approach: track whether control was handed to the with-body and re-raise body exceptions unchanged; lock-acquire failures still become FileLockError. The minion's warning now logs with exc_info=True. Adds a changelog entry. Not addressed: the issue's third point (retry limit / max age for the disk job queue) is a larger design change and is left out. The sync wait_lock has the same pattern and is left unchanged here. Validation: added test_await_lock_does_not_mask_body_exceptions in tests/pytests/unit/utils/test_files.py. It fails before the change (the ValueError is converted to FileLockError) and passes after; the whole test_files.py module passes (18 passed, 2 skipped). tests/pytests/unit/test_minion.py shows the same 2 failures with and without this change (49 passed). Not run: a live minion reproduction of the log flood. Report: https://github.com/saltstack/salt/issues/70229 Signed-off-by: Pujitha Paladugu <10557236+pujitha24@users.noreply.github.com> --- changelog/70229.fixed.md | 1 + salt/minion.py | 1 + salt/utils/files.py | 5 +++++ tests/pytests/unit/utils/test_files.py | 12 ++++++++++++ 4 files changed, 19 insertions(+) create mode 100644 changelog/70229.fixed.md diff --git a/changelog/70229.fixed.md b/changelog/70229.fixed.md new file mode 100644 index 000000000000..52d17792a103 --- /dev/null +++ b/changelog/70229.fixed.md @@ -0,0 +1 @@ +Fixed `salt.utils.files.await_lock` so exceptions raised inside its `async with` body are no longer converted into `FileLockError`, and the minion now logs the traceback when a job_queue lock failure occurs. diff --git a/salt/minion.py b/salt/minion.py index b1162a813814..9dcd1003846e 100644 --- a/salt/minion.py +++ b/salt/minion.py @@ -2537,6 +2537,7 @@ async def _handle_decoded_payload_impl(self, data): log.warning( "Failed to acquire job_queue lock for jid %s, queuing anyway.", data["jid"], + exc_info=True, ) # If we can't get the lock, we assume high contention and queue it to be safe. # Or we could just proceed (unsafe). Queuing is safer for stability. diff --git a/salt/utils/files.py b/salt/utils/files.py index ee8aeea1d07a..6071862290d0 100644 --- a/salt/utils/files.py +++ b/salt/utils/files.py @@ -411,6 +411,7 @@ async def await_lock(path, lock_fn=None, timeout=5, sleep=0.1, time_start=None): if time_start is None: time_start = time.time() obtained_lock = False + in_body = False def _raise_error(msg, race=False): """ @@ -459,6 +460,7 @@ def _raise_error(msg, race=False): log.trace("Write lock %s obtained", lock_fn) obtained_lock = True # Transfer control back to the code inside the with block + in_body = True yield # Exit the loop break @@ -473,6 +475,9 @@ def _raise_error(msg, race=False): raise except Exception as exc: # pylint: disable=broad-except + if in_body: + # Not a locking failure; let the caller see the real exception + raise _raise_error(f"Error encountered obtaining file lock {lock_fn}: {exc}") finally: diff --git a/tests/pytests/unit/utils/test_files.py b/tests/pytests/unit/utils/test_files.py index 6c8e96ee9c58..02cb4c41a203 100644 --- a/tests/pytests/unit/utils/test_files.py +++ b/tests/pytests/unit/utils/test_files.py @@ -474,3 +474,15 @@ def test_set_umask_nests_in_a_single_thread(): with salt.utils.files.set_umask(0o022): pass assert salt.utils.files.get_umask() == orig + + +async def test_await_lock_does_not_mask_body_exceptions(tmp_path): + """ + Exceptions raised inside the ``async with`` body must propagate unchanged + and must not be reported as a FileLockError. The lock must be released. + """ + lock_fn = str(tmp_path / "test.lock") + with pytest.raises(ValueError, match="boom"): + async with salt.utils.files.await_lock(lock_fn, lock_fn=lock_fn, timeout=1): + raise ValueError("boom") + assert not os.path.exists(lock_fn)