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/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/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..b6d29df918f8 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") @@ -113,17 +103,13 @@ def match(tgt, opts=None, minion_id=None): results.append( str( - __context__["matchers"][f"{engine}_match.match"]( - *engine_args, **engine_kwargs - ) + __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/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) 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