diff --git a/.github/workflows/README.md b/.github/workflows/README.md index 55102904ec..8f1b298af1 100644 --- a/.github/workflows/README.md +++ b/.github/workflows/README.md @@ -33,6 +33,7 @@ flowchart TB chunk0["test-deploy-chunk-0"] chunk0 --> chunk1["test-deploy-chunk-1"] chunk1 --> chunk2["test-deploy-chunk-2"] + chunk2 --> chunk3["test-deploy-chunk-3"] end lintwf --> chain @@ -99,8 +100,11 @@ the whole list instead of re-testing the same head forever. GitHub Actions cannot generate a variable number of jobs, so the chunk blocks are written out in the orchestrator. `INFINITO_CI_MAX_CHUNKS` must equal how many exist there — `slots` plans a sweep against that key, and a sweep planned -larger than the chain silently drops its tail. Blocks whose slice comes back -empty skip themselves and cost nothing. +larger than the chain silently drops its tail. Declare one block more than +`available` fills (`ceil(available / chunk size) + 1`): the split receives +every declared block, so after a short priority chunk the regular rows still +fill the rest of the budget. Blocks whose slice comes back empty skip +themselves and cost nothing. Run `python -m cli.meta.ci.slots --matrix` to see the whole budget. diff --git a/.github/workflows/call-orchestrator.yml b/.github/workflows/call-orchestrator.yml index 81c0b61b07..276f93029a 100644 --- a/.github/workflows/call-orchestrator.yml +++ b/.github/workflows/call-orchestrator.yml @@ -253,6 +253,42 @@ jobs: filesystem: ${{ inputs.filesystem }} tor: ${{ inputs.tor }} + test-deploy-chunk-3: + name: 🧱 Chunk 3 + if: >- + ${{ always() + && inputs.resume_from_chunk <= 3 + && needs.lint.result == 'success' + && needs.test.result == 'success' + && needs.build-ci-images.result == 'success' + && needs.images-mirror-missing.result == 'success' + && needs.test-dns.result == 'success' + && (!inputs.chunk_gate + || (contains(fromJSON('["success","skipped"]'), needs.test-deploy-chunk-0.result) + && contains(fromJSON('["success","skipped"]'), needs.test-deploy-chunk-1.result) + && contains(fromJSON('["success","skipped"]'), needs.test-deploy-chunk-2.result))) }} + needs: + - lint + - test + - build-ci-images + - images-mirror-missing + - test-dns + - test-deploy-chunk-0 + - test-deploy-chunk-1 + - test-deploy-chunk-2 + uses: ./.github/workflows/call-test-deploy.yml + with: + distros: ${{ inputs.distros }} + index: "3" + sweep: ${{ inputs.sweep != '' && inputs.sweep || github.run_number }} + offset: ${{ inputs.offset }} + whitelist: ${{ inputs.whitelist }} + priority: ${{ inputs.priority }} + lifecycles: ${{ inputs.lifecycles }} + modes: ${{ inputs.mode }} + filesystem: ${{ inputs.filesystem }} + tor: ${{ inputs.tor }} + test-runner-smoke: needs: - test-deploy-chunk-0 @@ -286,6 +322,7 @@ jobs: - test-deploy-chunk-0 - test-deploy-chunk-1 - test-deploy-chunk-2 + - test-deploy-chunk-3 if: ${{ always() && github.ref == 'refs/heads/main' && (contains(needs.*.result, 'failure') || contains(needs.*.result, 'cancelled')) }} runs-on: ubuntu-latest permissions: @@ -311,6 +348,7 @@ jobs: - test-deploy-chunk-0 - test-deploy-chunk-1 - test-deploy-chunk-2 + - test-deploy-chunk-3 - test-runner-smoke - test-workspace - test diff --git a/cli/meta/ci/chunks.py b/cli/meta/ci/chunks.py index 498a5004b1..81a34e63ea 100644 --- a/cli/meta/ci/chunks.py +++ b/cli/meta/ci/chunks.py @@ -58,7 +58,7 @@ def plan( priority: rows of the priority line, in query order. regular: rows of the regular line, in query order. size: rows one chunk may hold (``slots.chunk_size``). - blocks: chunk blocks the workflow declares (``slots.chunk_count``). + blocks: chunk blocks the workflow declares (``slots.chunk_blocks``). budget: rows the run job cap allows in total (``slots.available``). offset: how many regular rows to skip before filling the chunks. ``0`` (the default) starts at the head of the ranking. A value past the diff --git a/cli/meta/ci/matrix.py b/cli/meta/ci/matrix.py index 554a60cdf3..3beb7ad6a7 100644 --- a/cli/meta/ci/matrix.py +++ b/cli/meta/ci/matrix.py @@ -169,7 +169,7 @@ def chunks_of( [entry for entry in entries if entry["priority"] == "true"], regular, size=slots.chunk_size(), - blocks=slots.chunk_count(), + blocks=slots.chunk_blocks(), budget=slots.available(), offset=offset_index(offset, regular), ) diff --git a/cli/meta/ci/slots.py b/cli/meta/ci/slots.py index ed4e3cb795..2a59db7fd0 100644 --- a/cli/meta/ci/slots.py +++ b/cli/meta/ci/slots.py @@ -34,8 +34,9 @@ What is left is :func:`available`, the rows one sweep can deploy. It is spent by :func:`chunk_count` blocks of :func:`chunk_size` rows. Because the orchestrator cannot generate a variable number of jobs, the block count is also capped by ``INFINITO_CI_MAX_CHUNKS``, the number of chunk blocks the -workflow YAML actually declares; a sweep that would need more rows than those -blocks hold leaves the rest to the next sweep's offset. +workflow YAML actually declares (:func:`chunk_blocks`); a sweep that would +need more rows than those blocks hold leaves the rest to the next sweep's +offset. """ from __future__ import annotations @@ -190,10 +191,15 @@ def available(repo_root: Path | None = None) -> int: return max(limit - reserved_slots(repo_root) - entry_overhead(repo_root), 1) +def chunk_blocks() -> int: + """Chunk blocks call-orchestrator.yml declares, the most a sweep may plan.""" + return setting("INFINITO_CI_MAX_CHUNKS") + + def chunk_count(repo_root: Path | None = None) -> int: """Chunk blocks a sweep fills, capped by the blocks the YAML declares.""" needed = math.ceil(available(repo_root) / chunk_size(repo_root)) - return max(min(needed, setting("INFINITO_CI_MAX_CHUNKS")), 1) + return max(min(needed, chunk_blocks()), 1) def rows_per_sweep(repo_root: Path | None = None) -> int: @@ -230,7 +236,7 @@ def render_matrix() -> str: ("job timeout (minutes)", job_timeout_minutes()), ("waves", waves()), ("chunk size", chunk_size()), - ("chunk blocks (INFINITO_CI_MAX_CHUNKS)", setting("INFINITO_CI_MAX_CHUNKS")), + ("chunk blocks (INFINITO_CI_MAX_CHUNKS)", chunk_blocks()), ("chunks filled", chunk_count()), ("rows per sweep", rows_per_sweep()), ] diff --git a/default.env b/default.env index 93cc0f4c87..6f4233db7a 100644 --- a/default.env +++ b/default.env @@ -103,8 +103,8 @@ INFINITO_CI_DISTRO_BUDGET_SECONDS=20400 # Job budget per workflow run cli.meta.ci.slots spends on the deploy chunks (GitHub matrix cap: 256). INFINITO_CI_JOB_LIMIT=256 -# Chunk job blocks call-orchestrator.yml declares; cli.meta.ci.slots must never plan more than these exist in the YAML. -INFINITO_CI_MAX_CHUNKS=3 +# Chunk job blocks call-orchestrator.yml declares; one above what the budget fills, so a short priority chunk strands no row. +INFINITO_CI_MAX_CHUNKS=4 # Regular rows a run skips before filling its chunks; a count or a selection token, 0 starts at the head of the ranking. INFINITO_CI_OFFSET=0 diff --git a/tests/integration/meta/workflows/test_chunk_blocks_cover_the_budget.py b/tests/integration/meta/workflows/test_chunk_blocks_cover_the_budget.py new file mode 100644 index 0000000000..0c9659d8d9 --- /dev/null +++ b/tests/integration/meta/workflows/test_chunk_blocks_cover_the_budget.py @@ -0,0 +1,39 @@ +from __future__ import annotations + +import math +import re +import unittest + +from cli.meta.ci import slots +from utils.cache.files import PROJECT_ROOT +from utils.cache.yaml import load_yaml_any + +WORKFLOW = PROJECT_ROOT / ".github/workflows/call-orchestrator.yml" + + +class TestChunkBlocksCoverTheBudget(unittest.TestCase): + def test_the_declared_block_count_matches_the_orchestrator(self) -> None: + jobs = load_yaml_any(str(WORKFLOW))["jobs"] + declared = [ + name for name in jobs if re.fullmatch(r"test-deploy-chunk-\d+", name) + ] + self.assertEqual( + len(declared), + slots.chunk_blocks(), + "INFINITO_CI_MAX_CHUNKS must equal the chunk jobs call-orchestrator.yml " + "declares; a plan larger than the chain silently drops its tail", + ) + + def test_a_short_priority_chunk_strands_no_budget(self) -> None: + filled = math.ceil(slots.available() / slots.chunk_size()) + self.assertGreaterEqual( + slots.chunk_blocks(), + filled + 1, + "a priority chunk shorter than a block still occupies one, so the " + "regular rows need a block beyond what the budget fills to spend " + "the rest of it", + ) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/unit/python/cli/meta/ci/test_chunks.py b/tests/unit/python/cli/meta/ci/test_chunks.py index 1e96de9443..e35b73192b 100644 --- a/tests/unit/python/cli/meta/ci/test_chunks.py +++ b/tests/unit/python/cli/meta/ci/test_chunks.py @@ -53,6 +53,12 @@ class TestPlan(unittest.TestCase): self.assertEqual([len(c) for c in plan], [5, 80, 80]) self.assertEqual(plan[0], list(range(5))) + def test_a_spare_block_spends_what_a_short_priority_chunk_leaves(self) -> None: + plan = chunks.plan( + list(range(9)), list(range(100, 400)), size=80, blocks=4, budget=210 + ) + self.assertEqual([len(c) for c in plan], [9, 80, 80, 41]) + def test_the_seam_chunk_is_never_topped_up_with_regular_rows(self) -> None: plan = chunks.plan( list(range(5)), diff --git a/tests/unit/python/cli/meta/ci/test_matrix.py b/tests/unit/python/cli/meta/ci/test_matrix.py index 0dab0a2a5a..2eb5b3b780 100644 --- a/tests/unit/python/cli/meta/ci/test_matrix.py +++ b/tests/unit/python/cli/meta/ci/test_matrix.py @@ -31,7 +31,7 @@ class TestChunksOf(unittest.TestCase): def _chunks(self, entries: list[dict], *, size: int) -> list[list[dict]]: with ( mock.patch.object(matrix.slots, "chunk_size", return_value=size), - mock.patch.object(matrix.slots, "chunk_count", return_value=4), + mock.patch.object(matrix.slots, "chunk_blocks", return_value=4), mock.patch.object(matrix.slots, "available", return_value=99), ): return matrix.chunks_of(entries, 0) @@ -58,6 +58,26 @@ class TestChunksOf(unittest.TestCase): [chunk[0]["apps"] for chunk in chunks], ["web-app-z", "web-app-a"] ) + def test_a_short_priority_chunk_leaves_the_regular_rows_a_spare_block( + self, + ) -> None: + entries = [_entry("web-app-p", "0", "compose", priority=True)] + [ + _entry(f"web-app-{index:02d}", "0", "compose") for index in range(20) + ] + with ( + mock.patch.object(matrix.slots, "chunk_size", return_value=10), + mock.patch.object(matrix.slots, "chunk_count", return_value=2), + mock.patch.object(matrix.slots, "chunk_blocks", return_value=3), + mock.patch.object(matrix.slots, "available", return_value=21), + ): + chunks = matrix.chunks_of(entries, 0) + self.assertEqual( + [len(chunk) for chunk in chunks], + [1, 10, 10], + "the split must get every declared block, not only the ones the " + "budget fills, or the rows a short priority chunk leaves go unplanned", + ) + def test_priority_chunks_are_sorted_on_their_own(self) -> None: entries = [ _entry("web-app-z", "0", "compose", priority=True), @@ -80,7 +100,7 @@ class TestRedundant(unittest.TestCase): def _chunked(self, entries: list[dict]) -> list[dict]: with ( mock.patch.object(matrix.slots, "chunk_size", return_value=10), - mock.patch.object(matrix.slots, "chunk_count", return_value=4), + mock.patch.object(matrix.slots, "chunk_blocks", return_value=4), mock.patch.object(matrix.slots, "available", return_value=99), ): return [row for chunk in matrix.chunks_of(entries, 0) for row in chunk]