Skip to content

Commit

Permalink
Fix order chunks not handling a state with both require and order fir…
Browse files Browse the repository at this point in the history
…st or last
  • Loading branch information
bdrx312 committed Dec 25, 2024
1 parent 9233e1c commit c23b3e4
Show file tree
Hide file tree
Showing 4 changed files with 143 additions and 0 deletions.
1 change: 1 addition & 0 deletions changelog/67120.fixed.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fixed order chunks not handling a state with both require and order first or last
10 changes: 10 additions & 0 deletions salt/utils/requisite.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
82 changes: 82 additions & 0 deletions tests/pytests/functional/modules/state/requisites/test_require.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
50 changes: 50 additions & 0 deletions tests/pytests/unit/utils/requisite/test_dependency_graph.py
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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",
Expand All @@ -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

Expand Down

0 comments on commit c23b3e4

Please sign in to comment.