Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog/68103.fixed.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fixed `LocalClient.cmd_subset` raising `TypeError: argument of type 'bool' is not iterable` when one or more targeted minions failed to respond to the `sys.list_functions` probe. Failed minions are now skipped during subset selection.
8 changes: 7 additions & 1 deletion salt/client/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -536,7 +536,13 @@ def cmd_subset(
random.shuffle(minions)
f_tgt = []
for minion in minions:
if fun in minion_ret[minion]:
# Minions that failed to respond to ``sys.list_functions`` are
# represented as ``False`` (see the failed-minion handling in
# ``cmd``). Skip them rather than letting ``in`` raise.
functions = minion_ret[minion]
if not isinstance(functions, (list, tuple, set, dict)):
continue
if fun in functions:
f_tgt.append(minion)
if len(f_tgt) >= subset:
break
Expand Down
40 changes: 40 additions & 0 deletions tests/pytests/unit/test_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,46 @@ def test_cmd_subset(salt_master_factory):
)


def test_cmd_subset_skips_failed_minions_68103(salt_master_factory):
"""
Regression test for #68103.

When ``LocalClient.cmd`` cannot reach a minion it returns ``False`` for
that minion's entry in the result dict. ``cmd_subset`` previously did
``if fun in minion_ret[minion]`` without checking the value type and
raised ``TypeError: argument of type 'bool' is not iterable``. The
failed minion(s) should simply be skipped.
"""
salt_local_client = salt.client.get_local_client(mopts=salt_master_factory.config)

# ``cmd_subset`` calls ``random.shuffle`` on the minion list before
# iterating it. Replace shuffle with a no-op so the iteration order is
# deterministic and the failed minion is encountered first.
with patch("salt.client.random.shuffle", lambda x: None), patch(
"salt.client.LocalClient.cmd",
return_value={
# A minion that did not respond to ``sys.list_functions`` shows
# up in the return as ``False`` (see LocalClient.cmd's failed
# minion handling).
"minion1": False,
"minion2": ["first.func", "second.func"],
},
):
with patch("salt.client.LocalClient.cmd_cli") as cmd_cli_mock:
# Should not raise TypeError; failed minion must be skipped.
salt_local_client.cmd_subset("*", "first.func", subset=1, cli=True)
cmd_cli_mock.assert_called_with(
["minion2"],
"first.func",
(),
progress=False,
kwarg=None,
tgt_type="list",
full_return=False,
ret="",
)


@pytest.mark.skip_on_windows(reason="Not supported on Windows")
def test_pub(salt_master_factory):
"""
Expand Down
Loading