Skip to content

Commit

Permalink
SALT-669 Speed up matcher function calls at scale
Browse files Browse the repository at this point in the history
This PR changes match functions from loader access to direct imports of
the matchers in order to yield much faster results.

(cherry picked from commit ce20510)

Backported-from: saltstack#62295
Fixes: saltstack#62283
Signed-off-by: Joe Groocock <jgroocock@cloudflare.com>
  • Loading branch information
nicholasmhughes authored and frebib committed Oct 5, 2022
1 parent 53d98f8 commit 36ce8a0
Show file tree
Hide file tree
Showing 4 changed files with 105 additions and 25 deletions.
17 changes: 14 additions & 3 deletions salt/matchers/compound_match.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,23 @@
log = logging.getLogger(__name__)


def _load_matchers(opts):
"""
Store matchers in __context__ so they're only loaded once
"""
__context__["matchers"] = {}
__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", {})
matchers = salt.loader.matchers(opts)
if "matchers" not in __context__:
_load_matchers(opts)
if not minion_id:
minion_id = opts.get("id")

Expand Down Expand Up @@ -103,15 +112,17 @@ def match(tgt, opts=None, minion_id=None):

results.append(
str(
matchers["{}_match.match".format(engine)](
__context__["matchers"]["{}_match.match".format(engine)](
*engine_args, **engine_kwargs
)
)
)

else:
# The match is not explicitly defined, evaluate it as a glob
results.append(str(matchers["glob_match.match"](word, opts, minion_id)))
results.append(
str(__context__["matchers"]["glob_match.match"](word, opts, minion_id))
)

results = " ".join(results)
log.debug('compound_match %s ? "%s" => "%s"', minion_id, tgt, results)
Expand Down
13 changes: 11 additions & 2 deletions salt/matchers/nodegroup_match.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,14 @@
log = logging.getLogger(__name__)


def _load_matchers(opts):
"""
Store matchers in __context__ so they're only loaded once
"""
__context__["matchers"] = {}
__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 @@ -22,8 +30,9 @@ def match(tgt, nodegroups=None, opts=None, minion_id=None):
log.debug("Nodegroup matcher called with no nodegroups.")
return False
if tgt in nodegroups:
matchers = salt.loader.matchers(opts)
return matchers["compound_match.match"](
if "matchers" not in __context__:
_load_matchers(opts)
return __context__["matchers"]["compound_match.match"](
salt.utils.minions.nodegroup_comp(tgt, nodegroups)
)
return False
70 changes: 50 additions & 20 deletions salt/modules/match.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,14 @@
log = logging.getLogger(__name__)


def _load_matchers():
"""
Store matchers in __context__ so they're only loaded once
"""
__context__["matchers"] = {}
__context__["matchers"] = salt.loader.matchers(__opts__)


def compound(tgt, minion_id=None):
"""
Return True if the minion ID matches the given compound target
Expand All @@ -35,9 +43,12 @@ def compound(tgt, minion_id=None):
if minion_id is not None:
if not isinstance(minion_id, str):
minion_id = str(minion_id)
matchers = salt.loader.matchers(__opts__)
if "matchers" not in __context__:
_load_matchers()
try:
ret = matchers["compound_match.match"](tgt, opts=__opts__, minion_id=minion_id)
ret = __context__["matchers"]["compound_match.match"](
tgt, opts=__opts__, minion_id=minion_id
)
except Exception as exc: # pylint: disable=broad-except
log.exception(exc)
ret = False
Expand Down Expand Up @@ -65,9 +76,10 @@ def ipcidr(tgt):
- nodeclass: internal
"""
matchers = salt.loader.matchers(__opts__)
if "matchers" not in __context__:
_load_matchers()
try:
return matchers["ipcidr_match.match"](tgt, opts=__opts__)
return __context__["matchers"]["ipcidr_match.match"](tgt, opts=__opts__)
except Exception as exc: # pylint: disable=broad-except
log.exception(exc)
return False
Expand Down Expand Up @@ -96,9 +108,10 @@ def pillar_pcre(tgt, delimiter=DEFAULT_TARGET_DELIM):
.. versionadded:: 0.16.4
.. deprecated:: 2015.8.0
"""
matchers = salt.loader.matchers(__opts__)
if "matchers" not in __context__:
_load_matchers()
try:
return matchers["pillar_pcre_match.match"](
return __context__["matchers"]["pillar_pcre_match.match"](
tgt, delimiter=delimiter, opts=__opts__
)
except Exception as exc: # pylint: disable=broad-except
Expand Down Expand Up @@ -129,9 +142,12 @@ def pillar(tgt, delimiter=DEFAULT_TARGET_DELIM):
.. versionadded:: 0.16.4
.. deprecated:: 2015.8.0
"""
matchers = salt.loader.matchers(__opts__)
if "matchers" not in __context__:
_load_matchers()
try:
return matchers["pillar_match.match"](tgt, delimiter=delimiter, opts=__opts__)
return __context__["matchers"]["pillar_match.match"](
tgt, delimiter=delimiter, opts=__opts__
)
except Exception as exc: # pylint: disable=broad-except
log.exception(exc)
return False
Expand All @@ -147,9 +163,10 @@ def data(tgt):
salt '*' match.data 'spam:eggs'
"""
matchers = salt.loader.matchers(__opts__)
if "matchers" not in __context__:
_load_matchers()
try:
return matchers["data_match.match"](tgt, opts=__opts__)
return __context__["matchers"]["data_match.match"](tgt, opts=__opts__)
except Exception as exc: # pylint: disable=broad-except
log.exception(exc)
return False
Expand Down Expand Up @@ -178,9 +195,10 @@ def grain_pcre(tgt, delimiter=DEFAULT_TARGET_DELIM):
.. versionadded:: 0.16.4
.. deprecated:: 2015.8.0
"""
matchers = salt.loader.matchers(__opts__)
if "matchers" not in __context__:
_load_matchers()
try:
return matchers["grain_pcre_match.match"](
return __context__["matchers"]["grain_pcre_match.match"](
tgt, delimiter=delimiter, opts=__opts__
)
except Exception as exc: # pylint: disable=broad-except
Expand Down Expand Up @@ -211,9 +229,12 @@ def grain(tgt, delimiter=DEFAULT_TARGET_DELIM):
.. versionadded:: 0.16.4
.. deprecated:: 2015.8.0
"""
matchers = salt.loader.matchers(__opts__)
if "matchers" not in __context__:
_load_matchers()
try:
return matchers["grain_match.match"](tgt, delimiter=delimiter, opts=__opts__)
return __context__["matchers"]["grain_match.match"](
tgt, delimiter=delimiter, opts=__opts__
)
except Exception as exc: # pylint: disable=broad-except
log.exception(exc)
return False
Expand All @@ -237,9 +258,12 @@ def list_(tgt, minion_id=None):
if minion_id is not None:
if not isinstance(minion_id, str):
minion_id = str(minion_id)
matchers = salt.loader.matchers(__opts__)
if "matchers" not in __context__:
_load_matchers()
try:
return matchers["list_match.match"](tgt, opts=__opts__, minion_id=minion_id)
return __context__["matchers"]["list_match.match"](
tgt, opts=__opts__, minion_id=minion_id
)
except Exception as exc: # pylint: disable=broad-except
log.exception(exc)
return False
Expand All @@ -263,9 +287,12 @@ def pcre(tgt, minion_id=None):
if minion_id is not None:
if not isinstance(minion_id, str):
minion_id = str(minion_id)
matchers = salt.loader.matchers(__opts__)
if "matchers" not in __context__:
_load_matchers()
try:
return matchers["pcre_match.match"](tgt, opts=__opts__, minion_id=minion_id)
return __context__["matchers"]["pcre_match.match"](
tgt, opts=__opts__, minion_id=minion_id
)
except Exception as exc: # pylint: disable=broad-except
log.exception(exc)
return False
Expand All @@ -289,10 +316,13 @@ def glob(tgt, minion_id=None):
if minion_id is not None:
if not isinstance(minion_id, str):
minion_id = str(minion_id)
matchers = salt.loader.matchers(__opts__)
if "matchers" not in __context__:
_load_matchers()

try:
return matchers["glob_match.match"](tgt, opts=__opts__, minion_id=minion_id)
return __context__["matchers"]["glob_match.match"](
tgt, opts=__opts__, minion_id=minion_id
)
except Exception as exc: # pylint: disable=broad-except
log.exception(exc)
return False
Expand Down
30 changes: 30 additions & 0 deletions tests/pytests/unit/modules/test_match.py
Original file line number Diff line number Diff line change
Expand Up @@ -252,3 +252,33 @@ def test_list_match_different_minion_id():

# passing minion_id, should return True
assert match.list_("bar02,bar04", "bar04")


def test_matchers_loaded_only_once(minion_id):
"""
Test issue #62283
"""
mock_compound_match = MagicMock()
target = "bar04"

with patch.object(
salt.loader,
"matchers",
return_value={"compound_match.match": mock_compound_match},
) as matchers:
# do this 5 times
for run in range(0, 5):
match.compound(target)

# matcher loading was only performed once
matchers.assert_called_once()
# The matcher should get called with minion_id
assert len(matchers.call_args[0]) == 1
assert matchers.call_args[0][0].get("id") == minion_id

# compound match was called 5 times
assert mock_compound_match.call_count == 5
# The compound matcher should not get minion_id, no opts should be passed
mock_compound_match.assert_called_with(
target, minion_id=None, opts={"extension_modules": "", "id": minion_id}
)

0 comments on commit 36ce8a0

Please sign in to comment.