From c23b3e46b4fc25b24afed818f44797a90f4a3a31 Mon Sep 17 00:00:00 2001 From: bdrx312 Date: Wed, 25 Dec 2024 13:33:33 -0500 Subject: [PATCH] Fix order chunks not handling a state with both require and order first or last --- changelog/67120.fixed.md | 1 + salt/utils/requisite.py | 10 +++ .../modules/state/requisites/test_require.py | 82 +++++++++++++++++++ .../utils/requisite/test_dependency_graph.py | 50 +++++++++++ 4 files changed, 143 insertions(+) create mode 100644 changelog/67120.fixed.md diff --git a/changelog/67120.fixed.md b/changelog/67120.fixed.md new file mode 100644 index 000000000000..14cd36a5d7fc --- /dev/null +++ b/changelog/67120.fixed.md @@ -0,0 +1 @@ +Fixed order chunks not handling a state with both require and order first or last diff --git a/salt/utils/requisite.py b/salt/utils/requisite.py index 188188311e73..35d6d6e76e1d 100644 --- a/salt/utils/requisite.py +++ b/salt/utils/requisite.py @@ -202,6 +202,16 @@ def _get_chunk_order(self, cap: int, node: str) -> tuple[int | float, int | floa .get("chunk", {}) .get("order", float("inf")) ) + if child_order is None or not isinstance( + child_order, (int, float) + ): + if child_order == "last": + child_order = cap + 1000000 + elif child_order == "first": + child_order = 0 + else: + child_order = cap + dag.nodes[child]["chunk"]["order"] = child_order child_min = min(child_min, child_order) node_data["child_min"] = child_min if order > child_min: diff --git a/tests/pytests/functional/modules/state/requisites/test_require.py b/tests/pytests/functional/modules/state/requisites/test_require.py index ce2a3fd0109d..4e684e24806c 100644 --- a/tests/pytests/functional/modules/state/requisites/test_require.py +++ b/tests/pytests/functional/modules/state/requisites/test_require.py @@ -360,6 +360,88 @@ def test_requisites_require_ordering_and_errors_5(state, state_tree): assert ret.errors == [errmsg] +def test_requisites_require_with_order_first_last(state, state_tree): + """ + Call sls file containing a state with require_in order first + and require and order last. + + Ensure that the order is right. + """ + sls_contents = """ + # Complex require/require_in graph + # + # Relative order of A > D is given by the definition order + # + # B (1) <------+ + # | + # A (2) + + # | + # D (3) <--+ --| + # | + # E (4) | + # | + # C (5) ---+ + # + + A: + test.succeed_with_changes + + B: + test.succeed_with_changes: + - order: first + - require_in: + - D + + C: + test.succeed_with_changes: + - order: last + - require: + - D + + D: + test.succeed_with_changes + + E: + test.succeed_with_changes + """ + expected_result = { + "test_|-B_|-B_|-succeed_with_changes": { + "__run_num__": 0, + "result": True, + "changes": True, + "comment": "Success!", + }, + "test_|-A_|-A_|-succeed_with_changes": { + "__run_num__": 1, + "result": True, + "changes": True, + "comment": "Success!", + }, + "test_|-D_|-D_|-succeed_with_changes": { + "__run_num__": 2, + "result": True, + "changes": True, + "comment": "Success!", + }, + "test_|-E_|-E_|-succeed_with_changes": { + "__run_num__": 3, + "result": True, + "changes": True, + "comment": "Success!", + }, + "test_|-C_|-C_|-succeed_with_changes": { + "__run_num__": 4, + "result": True, + "changes": True, + "comment": "Success!", + }, + } + with pytest.helpers.temp_file("requisite.sls", sls_contents, state_tree): + ret = state.sls("requisite") + result = normalize_ret(ret.raw) + assert result == expected_result + + def test_requisites_require_any(state, state_tree): """ Call sls file containing require_any. diff --git a/tests/pytests/unit/utils/requisite/test_dependency_graph.py b/tests/pytests/unit/utils/requisite/test_dependency_graph.py index dc4bbbc1d0f9..37d3ee62dc59 100644 --- a/tests/pytests/unit/utils/requisite/test_dependency_graph.py +++ b/tests/pytests/unit/utils/requisite/test_dependency_graph.py @@ -19,12 +19,56 @@ def test_ordering(): sls = "test" env = "base" chunks = [ + { + "__id__": "success-last-1", + "name": "success-last-1", + "state": "test", + "fun": "succeed_with_changes", + "order": "last", + }, + { + "__id__": "success-last-3", + "name": "success-last-3", + "state": "test", + "fun": "succeed_with_changes", + "require": [{"test": "success-last-2"}], + "order": "last", + }, + { + "__id__": "success-last-2", + "name": "success-last-2", + "state": "test", + "fun": "succeed_with_changes", + "order": "last", + }, { "__id__": "success-6", "name": "success-6", "state": "test", "fun": "succeed_with_changes", }, + { + "__id__": "success-first-1", + "name": "success-first-1", + "state": "test", + "fun": "succeed_with_changes", + "order": "first", + }, + { + "__id__": "success-first-3", + "name": "success-first-3", + "state": "test", + "fun": "succeed_with_changes", + "require": [{"test": "success-first-2"}], + "order": "first", + }, + { + "__id__": "success-first-2", + "name": "success-first-2", + "state": "test", + "fun": "succeed_with_changes", + "order": "first", + }, { "__id__": "fail-0", "name": "fail-0", @@ -123,6 +167,9 @@ def test_ordering(): chunk["__id__"] for chunk in depend_graph.aggregate_and_order_chunks(100) ] expected_order = [ + "success-first-1", + "success-first-2", + "success-first-3", "success-1", "success-2", "success-a", @@ -136,6 +183,9 @@ def test_ordering(): "success-5", "success-6", "success-d", + "success-last-1", + "success-last-2", + "success-last-3", ] assert expected_order == ordered_chunk_ids