diff --git a/changelog/68408.fixed.md b/changelog/68408.fixed.md new file mode 100644 index 000000000000..23a852bdd1c4 --- /dev/null +++ b/changelog/68408.fixed.md @@ -0,0 +1 @@ +Fixed issue with check_requisite checking several times the same state diff --git a/salt/state.py b/salt/state.py index d1f897439ae5..d298448ff522 100644 --- a/salt/state.py +++ b/salt/state.py @@ -1903,6 +1903,43 @@ def apply_exclude(self, high): high.pop(id_) return high + @staticmethod + def _add_to_extend( + extend, + id_to_extend, + state_mod_name, + req_type, + req_id, + req_state_mod_name, + ): + """ + Add req_id as a requisite for id_to_extend + """ + + if id_to_extend not in extend: + # The state does not exist yet: create it + extend[id_to_extend] = HashableOrderedDict() + + if (state_args := extend[id_to_extend].get(state_mod_name)) is None: + # The state module is not present yet, create it and initialize it + extend[id_to_extend][state_mod_name] = [ + {req_type: [{req_state_mod_name: req_id}]} + ] + return + + # Lookup req_type in state_args + for state_arg in state_args: + if (req_items := state_arg.get(req_type)) is not None: + for req_item in req_items: + if req_item.get(req_state_mod_name) == req_id: + # (req_state_mode_name, req_id) is already defined as a requisiste + return + # Extending again + state_arg[req_type].append({req_state_mod_name: req_id}) + return + # req_type does not exist already for this state module + state_args.append({req_type: [{req_state_mod_name: req_id}]}) + def requisite_in(self, high): """ Extend the data reference with requisite_in arguments @@ -1953,11 +1990,7 @@ def requisite_in(self, high): if isinstance(items, dict): # Formatted as a single req_in for _state, name in items.items(): - # Not a use requisite_in - found = False - if name not in extend: - extend[name] = HashableOrderedDict() if "." in _state: errors.append( "Invalid requisite in {}: {} for " @@ -1971,23 +2004,12 @@ def requisite_in(self, high): ) ) _state = _state.split(".")[0] - if _state not in extend[name]: - extend[name][_state] = [] + self._add_to_extend( + extend, name, _state, rkey, id_, state + ) extend[name]["__env__"] = body["__env__"] extend[name]["__sls__"] = body["__sls__"] - for ind in range(len(extend[name][_state])): - if next(iter(extend[name][_state][ind])) == rkey: - # Extending again - extend[name][_state][ind][rkey].append( - {state: id_} - ) - found = True - if found: - continue - # The rkey is not present yet, create it - extend[name][_state].append({rkey: [{state: id_}]}) - - if isinstance(items, list): + elif isinstance(items, list): # Formed as a list of requisite additions hinges = [] for ind in items: @@ -2123,27 +2145,13 @@ def requisite_in(self, high): continue extend[id_][state].append(arg) continue - found = False - if name not in extend: - extend[name] = HashableOrderedDict() - if _state not in extend[name]: - extend[name][_state] = [] + self._add_to_extend( + extend, name, _state, rkey, id_, state + ) + extend[name]["__env__"] = body["__env__"] extend[name]["__sls__"] = body["__sls__"] - for ind in range(len(extend[name][_state])): - if ( - next(iter(extend[name][_state][ind])) - == rkey - ): - # Extending again - extend[name][_state][ind][rkey].append( - {state: id_} - ) - found = True - if found: - continue - # The rkey is not present yet, create it - extend[name][_state].append({rkey: [{state: id_}]}) + high["__extend__"] = [] for key, val in extend.items(): high["__extend__"].append({key: val}) diff --git a/tests/pytests/unit/state/test_add_to_extend.py b/tests/pytests/unit/state/test_add_to_extend.py new file mode 100644 index 000000000000..8db03ec15240 --- /dev/null +++ b/tests/pytests/unit/state/test_add_to_extend.py @@ -0,0 +1,222 @@ +from copy import deepcopy + +import pytest + +import salt.state +from salt.utils.odict import HashableOrderedDict + +pytestmark = [ + pytest.mark.core_test, +] + + +simple_extend_test_cases = [ + ( + # Simple addition + # + # add_to_extend: + # foo: + # file.managed: + # - required_in: + # - file: bar + {}, # extend + "bar", # id_to_extend + "file", # state_mod_name + "require", # req_type + "foo", # req_id + "file", # req_state_mod_name + {"bar": {"file": [{"require": [{"file": "foo"}]}]}}, # expected + ), + ( + # Requisite already exists + # + # bar: + # file.managed: + # require: + # - file: foo + # baz: + # file.managed: + # - require: + # - file: foo + # + # add_to_extend: + # foo: + # file.managed: + # - required_in: + # - file: baz + { # extend + "bar": { + "file": [{"require": [{"file": "foo"}]}], + }, + "baz": { + "file": [{"require": [{"file": "foo"}]}], + }, + }, + "baz", # id_to_extend + "file", # state_mod_name + "require", # req_type + "foo", # req_id + "file", # req_state_mod_name + { # expected + "bar": { + "file": [{"require": [{"file": "foo"}]}], + }, + "baz": { + "file": [{"require": [{"file": "foo"}]}], + }, + }, + ), + ( + # Append requisite + # + # bar: + # file.managed: + # require: + # - file: foo + # baz: + # file.managed: + # - require: + # - file: foo + # + # add_to_extend: + # bar: + # file.managed: + # - require_in: + # - file: baz + { # extend + "bar": HashableOrderedDict( + [ + ("file", [{"require": [{"file": "foo"}]}]), + ] + ), + "baz": HashableOrderedDict( + [ + ("file", [{"require": [{"file": "foo"}]}]), + ] + ), + }, + "baz", # id_to_extend + "file", # state_mod_name + "require", # req_type + "bar", # req_id + "file", # req_state_mod_name + { # expected + "bar": HashableOrderedDict( + [ + ("file", [{"require": [{"file": "foo"}]}]), + ] + ), + "baz": HashableOrderedDict( + [ + ("file", [{"require": [{"file": "foo"}, {"file": "bar"}]}]), + ] + ), + }, + ), + ( + # Append with different requisite type + # + # bar: + # file.managed: + # require: + # - file: foo + # baz: + # file.managed: + # - require: + # - file: foo + # + # add_to_extend: + # bar: + # file.managed: + # - prereq_in: + # - file: baz + { # extend + "bar": { + "file": [{"require": [{"file": "foo"}]}], + }, + "baz": { + "file": [{"require": [{"file": "foo"}]}], + }, + }, + "baz", # id_to_extend + "file", # state_mod_name + "prereq", # req_type + "bar", # req_id + "file", # req_state_mod_name + { # expected + "bar": { + "file": [ + { + "require": [ + {"file": "foo"}, + ] + }, + ], + }, + "baz": { + "file": [ + { + "require": [ + {"file": "foo"}, + ] + }, + { + "prereq": [ + {"file": "bar"}, + ] + }, + ], + }, + }, + ), +] + + +def _flatten_extend(a): + return { + (k1, k2, k4, k6, v6) + for k1, v1 in a.items() + for k2, v2 in v1.items() + for v3 in v2 + for k4, v4 in v3.items() + for v5 in v4 + for k6, v6 in v5.items() + } + + +def test_flatten(): + a = { + "bar": {"file": [{"require": [{"file": "foo"}]}]}, + "baz": { + "file": [ + {"require": [{"file": "foo"}]}, + {"prereq": [{"file": "bar"}]}, + ] + }, + } + b = { + "baz": { + "file": [ + {"prereq": [{"file": "bar"}]}, + {"require": [{"file": "foo"}]}, + ] + }, + "bar": {"file": [{"require": [{"file": "foo"}]}]}, + } + assert _flatten_extend(a) == _flatten_extend(b) + + +@pytest.mark.parametrize( + "extend,id_to_extend,state_mod_name,req_type,req_id,req_state_mod_name,expected", + simple_extend_test_cases, +) +def test_simple_extend( + extend, id_to_extend, state_mod_name, req_type, req_id, req_state_mod_name, expected +): + # local copy of extend, as it is modified by _add_to_extend + _extend = deepcopy(extend) + + salt.state.State._add_to_extend( + _extend, id_to_extend, state_mod_name, req_type, req_id, req_state_mod_name + ) + assert _flatten_extend(_extend) == _flatten_extend(expected)