Skip to content

Commit

Permalink
fixes saltstack#62283 match function calls are slow at scale
Browse files Browse the repository at this point in the history
  • Loading branch information
nicholasmhughes committed Sep 19, 2022
1 parent a55a8bc commit ce20510
Show file tree
Hide file tree
Showing 5 changed files with 106 additions and 25 deletions.
1 change: 1 addition & 0 deletions changelog/62283.fixed
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix matcher slowness due to loader invocation
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 @@ -253,3 +253,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 ce20510

Please sign in to comment.