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 .pylintrc
Original file line number Diff line number Diff line change
Expand Up @@ -656,6 +656,7 @@ additional-builtins=__opts__,
__grains__,
__context__,
__runner__,
__matchers__,
__ret__,
__env__,
__low__,
Expand Down
1 change: 1 addition & 0 deletions changelog/64607.fixed.md
Original file line number Diff line number Diff line change
@@ -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.
1 change: 1 addition & 0 deletions salt/loader/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
)
Expand Down
18 changes: 2 additions & 16 deletions salt/matchers/compound_match.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@

import logging

import salt.loader
import salt.utils.minions

HAS_RANGE = False
Expand All @@ -18,22 +17,13 @@
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
"""
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")

Expand Down Expand Up @@ -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)
Expand Down
15 changes: 2 additions & 13 deletions salt/matchers/confirm_top.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,6 @@

import logging

import salt.loader

log = logging.getLogger(__file__)


Expand All @@ -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)
12 changes: 1 addition & 11 deletions salt/matchers/nodegroup_match.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
68 changes: 57 additions & 11 deletions tests/pytests/unit/matchers/test_confirm_top.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import pytest

import salt.config
import salt.loader
from tests.support.mock import patch

Expand All @@ -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)
1 change: 1 addition & 0 deletions tests/support/pytest/loader.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ class LoaderModuleMock:
"__grains__",
"__pillar__",
"__sdb__",
"__matchers__",
),
)
# These dunders might exist at the module global scope
Expand Down
Loading