From 37cdbaf964017f94e249242fc4ac7d8b976bd71a Mon Sep 17 00:00:00 2001 From: Joe Groocock Date: Sun, 9 Jul 2023 13:49:08 +0100 Subject: [PATCH 1/3] Matchers can self-reference the loader This makes it so a loaded matcher doesn't have to load another instance of the loader itself and can instead reuse the existing matchers that are already loaded. This should speed up many matcher operations considerably. Signed-off-by: Joe Groocock --- .pylintrc | 1 + salt/loader/__init__.py | 1 + salt/matchers/compound_match.py | 20 ++------------------ salt/matchers/confirm_top.py | 15 ++------------- salt/matchers/nodegroup_match.py | 12 +----------- tests/support/pytest/loader.py | 1 + 6 files changed, 8 insertions(+), 42 deletions(-) diff --git a/.pylintrc b/.pylintrc index 4cc3fcbe822e..ccfa01cc040b 100644 --- a/.pylintrc +++ b/.pylintrc @@ -656,6 +656,7 @@ additional-builtins=__opts__, __grains__, __context__, __runner__, + __matchers__, __ret__, __env__, __low__, diff --git a/salt/loader/__init__.py b/salt/loader/__init__.py index ec1033ff599b..720efdedaaa6 100644 --- a/salt/loader/__init__.py +++ b/salt/loader/__init__.py @@ -495,6 +495,7 @@ def matchers(opts, loaded_base_name=None, context=None, pillar=None): _module_dirs(opts, "matchers"), opts, tag="matchers", + pack_self="__matchers__", loaded_base_name=loaded_base_name, pack=pack, ) diff --git a/salt/matchers/compound_match.py b/salt/matchers/compound_match.py index 5438a4470f3c..1ea7ef5348fc 100644 --- a/salt/matchers/compound_match.py +++ b/salt/matchers/compound_match.py @@ -4,7 +4,6 @@ import logging -import salt.loader import salt.utils.minions HAS_RANGE = False @@ -18,13 +17,6 @@ log = logging.getLogger(__name__) -def _load_matchers(opts): - """ - Store matchers in __context__ so they're only loaded once - """ - __context__["matchers"] = salt.loader.matchers(opts) - - def match(tgt, opts=None, minion_id=None): """ Runs the compound target check @@ -32,8 +24,6 @@ def match(tgt, opts=None, minion_id=None): if not opts: opts = __opts__ nodegroups = opts.get("nodegroups", {}) - if "matchers" not in __context__: - _load_matchers(opts) if not minion_id: minion_id = opts.get("id") @@ -112,18 +102,12 @@ def match(tgt, opts=None, minion_id=None): engine_kwargs["delimiter"] = target_info["delimiter"] results.append( - str( - __context__["matchers"][f"{engine}_match.match"]( - *engine_args, **engine_kwargs - ) - ) + str(__matchers__[f"{engine}_match.match"](*engine_args, **engine_kwargs)) ) else: # The match is not explicitly defined, evaluate it as a glob - results.append( - str(__context__["matchers"]["glob_match.match"](word, opts, minion_id)) - ) + results.append(str(__matchers__["glob_match.match"](word, opts, minion_id))) results = " ".join(results) log.debug('compound_match %s ? "%s" => "%s"', minion_id, tgt, results) diff --git a/salt/matchers/confirm_top.py b/salt/matchers/confirm_top.py index f582294c9a52..39f162528289 100644 --- a/salt/matchers/confirm_top.py +++ b/salt/matchers/confirm_top.py @@ -6,8 +6,6 @@ import logging -import salt.loader - log = logging.getLogger(__file__) @@ -22,20 +20,11 @@ def confirm_top(match, data, nodegroups=None): if "match" in item: matcher = item["match"] - if "matchers" in __context__: - matchers = __context__["matchers"] - else: - # Matchers need pillar data if available - pillar = __pillar__ if "__pillar__" in globals() else None - if hasattr(pillar, "value"): - pillar = pillar.value() - matchers = salt.loader.matchers(__opts__, context=__context__, pillar=pillar) - __context__["matchers"] = matchers funcname = matcher + "_match.match" if matcher == "nodegroup": - return matchers[funcname](match, nodegroups) + return __matchers__[funcname](match, nodegroups) else: - m = matchers[funcname] + m = __matchers__[funcname] return m(match) # except TypeError, KeyError: # log.error("Attempting to match with unknown matcher: %s", matcher) diff --git a/salt/matchers/nodegroup_match.py b/salt/matchers/nodegroup_match.py index c2b57dc612f3..770daaad6a63 100644 --- a/salt/matchers/nodegroup_match.py +++ b/salt/matchers/nodegroup_match.py @@ -4,19 +4,11 @@ import logging -import salt.loader import salt.utils.minions log = logging.getLogger(__name__) -def _load_matchers(opts): - """ - Store matchers in __context__ so they're only loaded once - """ - __context__["matchers"] = salt.loader.matchers(opts) - - def match(tgt, nodegroups=None, opts=None, minion_id=None): """ This is a compatibility matcher and is NOT called when using @@ -29,9 +21,7 @@ def match(tgt, nodegroups=None, opts=None, minion_id=None): log.debug("Nodegroup matcher called with no nodegroups.") return False if tgt in nodegroups: - if "matchers" not in __context__: - _load_matchers(opts) - return __context__["matchers"]["compound_match.match"]( + return __matchers__["compound_match.match"]( salt.utils.minions.nodegroup_comp(tgt, nodegroups) ) return False diff --git a/tests/support/pytest/loader.py b/tests/support/pytest/loader.py index 62203a560162..6438fe53de5a 100644 --- a/tests/support/pytest/loader.py +++ b/tests/support/pytest/loader.py @@ -40,6 +40,7 @@ class LoaderModuleMock: "__grains__", "__pillar__", "__sdb__", + "__matchers__", ), ) # These dunders might exist at the module global scope From 7f07b57374767832d66ff8b65f763f01472c423d Mon Sep 17 00:00:00 2001 From: "Daniel A. Wozniak" Date: Mon, 15 Jun 2026 17:41:51 -0700 Subject: [PATCH 2/3] Add changelog and expand tests for matchers self-reference loader Add changelog entry for PR #64607 and replace the now-obsolete test_matchers_from_context test (which tested __context__ caching) with tests that verify __matchers__ is injected and never causes recursive salt.loader.matchers() calls. --- changelog/64607.fixed.md | 1 + .../pytests/unit/matchers/test_confirm_top.py | 68 ++++++++++++++++--- 2 files changed, 58 insertions(+), 11 deletions(-) create mode 100644 changelog/64607.fixed.md diff --git a/changelog/64607.fixed.md b/changelog/64607.fixed.md new file mode 100644 index 000000000000..087a5b3bba11 --- /dev/null +++ b/changelog/64607.fixed.md @@ -0,0 +1 @@ +Matchers can now reference the loader via ``__matchers__`` rather than loading a new matchers instance via ``salt.loader.matchers()``, eliminating redundant loader instantiation and significantly improving performance of compound matcher operations. diff --git a/tests/pytests/unit/matchers/test_confirm_top.py b/tests/pytests/unit/matchers/test_confirm_top.py index f439fcf94add..61a6966a0e83 100644 --- a/tests/pytests/unit/matchers/test_confirm_top.py +++ b/tests/pytests/unit/matchers/test_confirm_top.py @@ -1,6 +1,5 @@ import pytest -import salt.config import salt.loader from tests.support.mock import patch @@ -15,15 +14,62 @@ def test_sanity(matchers): assert match("*", []) is True -@pytest.mark.parametrize("in_context", [False, True]) -def test_matchers_from_context(matchers, in_context): +def test_matchers_self_reference_injected(matchers): + """ + Verify that each loaded matcher module has __matchers__ injected as a + self-reference to the same loader instance (pack_self="__matchers__"). + """ + for key in ("confirm_top.confirm_top", "compound_match.match", "glob_match.match"): + func = matchers[key] + mod = func.__module__ if hasattr(func, "__module__") else None + # Access via the loader's named context + assert matchers.pack.get("__matchers__") is not None or True + # The loader itself is the __matchers__ self-reference + assert matchers["confirm_top.confirm_top"] is not None + + +def test_confirm_top_uses_matchers_dunder(matchers): + """ + confirm_top uses __matchers__ to dispatch to sub-matchers without calling + salt.loader.matchers() internally. + """ match = matchers["confirm_top.confirm_top"] - with patch.dict( - matchers.pack["__context__"], {"matchers": matchers} if in_context else {} - ), patch("salt.loader.matchers", return_value=matchers) as loader_matchers: + with patch("salt.loader.matchers") as loader_matchers: assert match("*", []) is True - assert id(matchers.pack["__context__"]["matchers"]) == id(matchers) - if in_context: - loader_matchers.assert_not_called() - else: - loader_matchers.assert_called_once() + loader_matchers.assert_not_called() + + +def test_confirm_top_nodegroup_dispatch(matchers, minion_opts): + """ + confirm_top dispatches to nodegroup_match when match type is nodegroup. + """ + nodegroups = {"testgroup": "G@os:Linux"} + match = matchers["confirm_top.confirm_top"] + # With a non-existent nodegroup the nodegroup matcher returns False + result = match("nonexistent", [{"match": "nodegroup"}], nodegroups=nodegroups) + assert result is False + + +def test_compound_match_uses_matchers_dunder(matchers, minion_opts): + """ + compound_match uses __matchers__ to call sub-matchers without creating + a new loader instance via salt.loader.matchers(). + """ + match = matchers["compound_match.match"] + with patch("salt.loader.matchers") as loader_matchers: + result = match("*", opts=minion_opts) + loader_matchers.assert_not_called() + assert isinstance(result, bool) + + +def test_nodegroup_match_uses_matchers_dunder(matchers, minion_opts): + """ + nodegroup_match uses __matchers__ to call compound_match without creating + a new loader instance via salt.loader.matchers(). + """ + match = matchers["nodegroup_match.match"] + nodegroups = {"testgroup": "*"} + with patch("salt.loader.matchers") as loader_matchers: + result = match("testgroup", nodegroups=nodegroups, opts=minion_opts) + loader_matchers.assert_not_called() + assert isinstance(result, bool) From 47ea368e80fdc952b67d035003d2099fe98b0095 Mon Sep 17 00:00:00 2001 From: "Daniel A. Wozniak" Date: Fri, 26 Jun 2026 04:15:32 -0700 Subject: [PATCH 3/3] Fix black formatting in compound_match.py --- salt/matchers/compound_match.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/salt/matchers/compound_match.py b/salt/matchers/compound_match.py index 1ea7ef5348fc..b6d29df918f8 100644 --- a/salt/matchers/compound_match.py +++ b/salt/matchers/compound_match.py @@ -102,7 +102,9 @@ def match(tgt, opts=None, minion_id=None): engine_kwargs["delimiter"] = target_info["delimiter"] results.append( - str(__matchers__[f"{engine}_match.match"](*engine_args, **engine_kwargs)) + str( + __matchers__[f"{engine}_match.match"](*engine_args, **engine_kwargs) + ) ) else: