Fix local_cache in multi_returner - #67880
Conversation
|
Hi there! Welcome to the Salt Community! Thank you for making your first contribution. We have a lengthy process for issues and PRs. Someone from the Core Team will follow up as soon as possible. In the meantime, here’s some information that may help as you continue your Salt journey. There are lots of ways to get involved in our community. Every month, there are around a dozen opportunities to meet with other contributors and the Salt Core team and collaborate in real time. The best way to keep track is by subscribing to the Salt Community Events Calendar. |
twangboy
left a comment
twangboy
left a comment
There was a problem hiding this comment.
Please add a changelog and some tests
twangboy
left a comment
twangboy
left a comment
There was a problem hiding this comment.
Please write a test for this
twangboy
added
needs-testcase
|
The ``get_jid_filter`` (singular) name introduced earlier in this PR did not match Salt's canonical ``<returner>.get_jids_filter`` (plural) that the ``salt/runners/jobs.py`` runner looks up. Both the function name and the ``fstr`` lookup are renamed to ``get_jids_filter`` so the runner integration works end-to-end. ``save_reg`` was ``return``-ing inside the loop, writing to only the first configured returner that implemented ``save_reg`` and silently skipping the rest. The early ``return`` is dropped so every configured returner is written, matching the write-to-all contract that ``returner()``, ``save_load()``, ``save_minions()``, ``update_endtime()`` and friends already follow. Regression test added at ``tests/pytests/unit/returners/test_multi_returner.py`` covering: * ``save_load`` skips ``local_cache`` for ``cmd == '_return'`` and dispatches normally otherwise (the original fix in this PR); * ``save_load`` forwards the ``minions`` argument (previously dropped); * ``get_jids_filter`` dispatches and merges under the canonical plural name, and the misnamed singular attribute does not exist; * ``save_reg`` writes to every configured returner that implements it. All seven new tests pass on this branch and fail on ``origin/master``. Co-authored-by: a76yyyy <q981331502@163.com>
twangboy
left a comment
twangboy
left a comment
There was a problem hiding this comment.
Looks great. Please create against the earliest branch where this bug exists.
3 participants
What does this PR do?
Fixes incorrect storage of
_return-type job results in local_cache when usingmulti_returnerconfiguration. Now strictly follows the logic insalt.utils.job.store_jobby only executing multi-returner storage whenload['cmd']is not equal to'_return'.What issues does this PR fix or reference?
None
Previous Behavior
With multi-returner configuration:
All types of return data (including
cmd: _returnjob results) were stored inlocal_cache, violating the expected behavior defined in store_job logic:salt/salt/utils/job.py
Lines 127 to 130 in f906ca5
New Behavior
Now only save_load when
returner_ == "local_cache" and load['cmd'] != '_return'Merge requirements satisfied?
[NOTICE] Bug fixes or features added to Salt require tests.
Commits signed with GPG?
No