diff --git a/changelog/62283.fixed b/changelog/62283.fixed new file mode 100644 index 00000000000..2a5d60d658b --- /dev/null +++ b/changelog/62283.fixed @@ -0,0 +1 @@ +Fix matcher slowness due to loader invocation diff --git a/salt/matchers/compound_match.py b/salt/matchers/compound_match.py index 40ef2b03fa5..538d2f92a37 100644 --- a/salt/matchers/compound_match.py +++ b/salt/matchers/compound_match.py @@ -18,6 +18,14 @@ except ImportError: 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 @@ -25,7 +33,8 @@ def match(tgt, opts=None, minion_id=None): 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") @@ -103,7 +112,7 @@ 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 ) ) @@ -111,7 +120,9 @@ def match(tgt, opts=None, minion_id=None): 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) diff --git a/salt/matchers/nodegroup_match.py b/salt/matchers/nodegroup_match.py index f3dea0ce96b..1ce621510fb 100644 --- a/salt/matchers/nodegroup_match.py +++ b/salt/matchers/nodegroup_match.py @@ -10,6 +10,14 @@ import salt.utils.minions 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 @@ -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 diff --git a/salt/modules/match.py b/salt/modules/match.py index 878ab354365..25d9de51a76 100644 --- a/salt/modules/match.py +++ b/salt/modules/match.py @@ -17,6 +17,14 @@ __func_alias__ = {"list_": "list"} 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 @@ -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 @@ -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 @@ -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 @@ -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 @@ -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 @@ -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 @@ -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 @@ -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 @@ -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 @@ -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 diff --git a/tests/pytests/unit/modules/test_match.py b/tests/pytests/unit/modules/test_match.py index c5f0fc180dd..848bbdd1452 100644 --- a/tests/pytests/unit/modules/test_match.py +++ b/tests/pytests/unit/modules/test_match.py @@ -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} + )