Skip to content

ci: start the longest Docker scenarios in the first wave - #7865

Merged
devantler merged 5 commits into
mainfrom
codex/ci-long-cases-first-7863
Oct 18, 2026
Merged

devantler merged 5 commits into
mainfrom
codex/ci-long-cases-first-7863

Conversation

@devantler

Copy link
Copy Markdown
Contributor

🤖 Generated by the Agentic Engineer

Why

KSail's full CI takes close to an hour, and its longest scenario waits more than ten minutes for a test slot. That delays feedback on every change needing full validation.

What

Start the long-running upgrade, nested-cluster and full-stack scenarios early, without increasing concurrency or removing any coverage. Measure the actual scheduling and total feedback time in hosted CI before calling this an improvement.

Part of #7863

Preserve all thirty effective cases and the fifteen-runner ceiling. Keep the named Talos upgrade on an original early combination, forward its effective args to cleanup, and verify the complete matrix against the frozen baseline.
@devantler

devantler commented Oct 18, 2026 •

Copy link
Copy Markdown
Contributor Author

🤖 Generated by the Agentic Engineer

Validation and measurement plan for 012f8a4147497746ace67f47aa6b022afc35bd9e:

  • RED/GREEN: the first regression failed with KWOK created first. A second refusal failed when distribution-only reordering left Vanilla's nested and 25m50s full-stack cases outside the first wave. The final axes create all default/nested hosts and all full-stack cases early.
  • The frozen matrix is from main 83938d8a149a2cf1c4805007f9932b2aa2e07891. The test proves a complete multiset bijection of all 30 effective scenarios and every option, including three Calico migrations, named Talos, exact upgrade versions, mirror/offline coverage, and equal effective test/cleanup arguments. Concurrency remains 15 and fail-fast remains false.
  • The five-file suite covering the affected matrix harness plus the merged binary-artifact and module-cache guards passes with the race detector (2.335s); lint reports zero issues. The expansion helper is separately characterized against GitHub's original-only overlay example and partial exclusion/reintroduction behavior. Diff and Prettier checks pass; the authored commits and merge are signed and verified.
  • A self-review mutation removed the explicit fail-fast: false setting. The strengthened presence guard correctly failed because GitHub defaults an omitted setting to true; the original workflow was restored byte-for-byte, and the race-enabled suite returned to green. No production setting was relaxed.
  • The first native attempt correctly caught an unintended compile-unit coupling: the dedicated two-file Bash 3.2 job does not compile the new matrix helper file. The upgrade test is self-contained again; the exact two-file command now passes locally with KSAIL_REQUIRE_BASH32=true (1.066s, no skip). The superseded attempt was automatically cancelled; it is not acceptance evidence.
  • Complete-package local race/lint attempts were stopped when disk headroom fell below the host's safety floor. They are incomplete, not passes. The installed actionlint rejects an unchanged code-quality permission already on main; it is not a green local workflow result. Hosted full Go validation and the complete selected CI matrix must pass before review/promotion.
  • Native baseline: successful ordinary PR run 39126461063 took 57m24s. The named Talos job started 12m43s after the first matrix jobs and ran for 25m29s. Vanilla full-stack ran for 25m50s. The earlier successful run 39118549615 had 10m26s Talos admission delay.
  • Intermediate native trial 39134340068, at superseded head fccbf4f2563c61e2f12655e343aa6fde6362ae0a, admitted the Talos default/upgrade job 117700282722 at 14:25:29 UTC, two seconds after the last predecessor finished. The earlier completed baselines waited 10m26s and 12m43s. This is an observed admission result, not a completed matrix or whole-workflow result: that trial was superseded after it exposed ambiguous long-argument-first job names.
  • Scenario names now keep the distribution first independently of axis order; executable rendering checks ensure all 30 remain distinguishable within an 80-character preview. The new check failed before the display-name fix and passes now. This preserves useful failure diagnostics rather than hiding distribution behind truncated arguments.
  • This head integrates merged main b9b42e858dae2dbc887650a8ed9d8d4b2b660265 (ci: reuse the candidate binary for mirror warming #7864), so the final trial also exercises the delivered binary-reuse path. Compare native predecessor-ready-to-start delay, selected case count/options, long-case durations, total feedback time and summed runner time in current ordinary PR run 39136489159. Earlier-head green checks are not acceptance here. Runtime/queue variance and combined-change confounders remain explicit. No CLI release is expected for this CI-only change.

Completed final-head native trial

Run 39136489159 completed successfully at 15:20:00 UTC on exact head 012f8a4147497746ace67f47aa6b022afc35bd9e. All 30 Docker scenarios passed, including all three migration cases. The full rollup is 86 passing checks and six intentional skips, with no failures or missing selected checks. The complete hosted workflow-contract and Bash 3.2 jobs passed; the separate required Go workflow 39136488877 passed in 24m16s.

Same native CI measurement Before: 39126461063 After: 39136489159
Full CI elapsed 57m24s 47m04s
Named Talos admission after the last prerequisite 12m46s 3s
Named Talos execution 25m29s 24m31s
Vanilla full-stack execution 25m50s 27m18s
Selected jobs / Docker cases 62 / 30 62 / 30
Sum of selected CI job elapsed time 447.93 runner-min 454.85 runner-min

The observed full feedback improvement is 10m20s (18.0%) in this pair. The admission comparison uses the same denominator in both runs: last prerequisite completion, not first matrix start. Before, the last prerequisite finished at 12:33:27 and Talos started at 12:46:13; after, those events were 14:52:02 and 14:52:05. All first-wave default/nested and full-stack cases actually started, rather than merely receiving queued-job placeholder timestamps.

This is a descriptive before/after pair, not a replicated causal estimate or a runner-cost reduction. Startup to prerequisite readiness was nearly unchanged (19m01s versus 19m06s); individual runtime and runner variance remain. Summed selected CI job time rose by 6m55s (1.5%), with matrix-only time up 65s (0.3%). Concurrency remains 15 and no additional capacity or weakened outcome is introduced. Both trials include the now-merged binary-reuse path; unrelated Go/analysis workflows are not included in this CI-only runner-time sum.

As a workflow user, I read the actual named Talos job: it executed the explicit Kubernetes v1.36.2 to v1.37.1 upgrade, confirmed the pinned upgraded version, exercised the four nested distributions' API readiness checks, and completed cleanup successfully. Native names remain distinct and distribution-first. This observes the real behavior, not only matrix expansion or a code-path trace.

Main's separate interrupted security scan was tracked and recovered in #7866. Its real scanner and required aggregate passed on unchanged main after one bounded failed-jobs rerun; enforcement was not relaxed. The original shutdown cause remains unknown.

Sources: GitHub matrix order and include semantics.

@github-actions

github-actions Bot commented Oct 18, 2026 •

Copy link
Copy Markdown
Contributor

@github-code-quality

github-code-quality Bot commented Oct 18, 2026 •

Copy link
Copy Markdown
Contributor

Code coverage overview

Languages: Go

Go / code-coverage/go

main b9b42e8 codex/ci-long-cases-... 012f8a4 +/-
72% 72% 0%
View coverage of the 2 most impacted files.
File main b9b42e8 codex/ci-long-cases-... 012f8a4 +/-
pkg/cli/cluster...i/distconfig.go 80% 79% -1%
pkg/svc/mirror/...unnelsession.go 97% 98% +1%

ℹ️ Calculations represent line coverage. Results may change as calculations are updated.


Updated October 18, 2026 14:57 UTC

@devantler

Copy link
Copy Markdown
Contributor Author

🤖 Generated by the Agentic Engineer

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Oct 18, 2026 •

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai

coderabbitai Bot commented Oct 18, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository YAML (base), Organization UI (inherited)
  • Review profile: ASSERTIVE
  • Plan: Advanced
  • Run ID: d89fc570-1fa1-4c08-b5c9-3ccfd87cecb2
📥 Commits

Reviewing files that changed from the base of the PR and between b9b42e8 and 012f8a4.

📒 Files selected for processing: 5
  • .github/workflows/ci.yaml
  • internal/ciharness/docker_matrix_test.go
  • internal/ciharness/kubernetes_update_test.go
  • internal/ciharness/system_test_harness_test.go
  • internal/ciharness/testdata/docker-matrix-baseline.yaml

Included review availability: This review used your included allowance. Your plan provides up to 2 included reviews per hour; 1 remain after this review.

📜 Recent review details
🔇 Additional comments: 6
.github/workflows/ci.yaml: 2

1940-1950: LGTM!

Also applies to: 1978-1981, 2110-2110, 2158-2158

1903-1903: 🩺 Stability & Availability

The concern is refuted. The repository requires only CI - Required Checks, not individual Docker matrix check names. That aggregate job depends on system-test-docker and reports its result, so the matrix name change does not leave the required check pending.

internal/ciharness/docker_matrix_test.go: 1

1-320: LGTM!

internal/ciharness/testdata/docker-matrix-baseline.yaml: 1

1-73: LGTM!

internal/ciharness/kubernetes_update_test.go: 1

278-278: LGTM!

Also applies to: 309-309, 325-325, 331-331

internal/ciharness/system_test_harness_test.go: 1

383-387: LGTM!

📝 Walkthrough

The Docker system-test workflow changes its matrix order, Talos upgrade arguments, job names, and arguments passed to test and cleanup actions. CI harness tests add matrix decoding and expansion checks. They compare the expanded scenarios with a frozen 30-scenario baseline and verify scenario ordering, arguments, coverage, and job-name uniqueness.

Priority: ⬇️ Low

Merge Risk: ⚪ Minimal · up to 012f8

No concrete merge-blocking issue is established. Confirm the intended timing benefit from hosted CI runs.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 35.71% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 14 functions across 3 files. (2 skipped: … Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Title check Passed The title clearly describes the main change: scheduling the longest Docker scenarios in the first wave.
Description check Passed The description directly explains the CI scheduling objective, preserved coverage, unchanged concurrency, and required hosted-CI validation.
Full details: Docstring Coverage

Explanation

Docstring coverage is 35.71% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 14 functions across 3 files. (2 skipped: 2 unsupported.)

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@devantler

Copy link
Copy Markdown
Contributor Author

🤖 Generated by the Agentic Engineer

Ready at exact head 012f8a4147497746ace67f47aa6b022afc35bd9e, against main b9b42e858dae2dbc887650a8ed9d8d4b2b660265.

  • Programmatic proof: all 86 passing checks, six intentional skips, complete selected hosted validation, all 30 Docker scenarios and three migrations, complete workflow-contract and Bash 3.2 checks. The local RED/GREEN, omission refusal, expansion-equivalence and naming checks are recorded in the validation comment. No skipped scanner is substituted for a completed verdict.
  • Independent review: CodeRabbit's completed summary, posted after authenticated request 6251628534, explicitly covers the full current head and all five files, with no actionable comments. The current-head classifier reports GREEN. There are zero review threads, unresolved findings, nitpicks or outside-diff findings.
  • Advisory assessment: the 35.71% docstring metric counts functions in the three changed test files. No production API changes; the YAML decoder method and non-obvious overlay/argument contracts are documented. The repository's existing .coderabbit.yaml explicitly retains this metric as an advisory warning because idiomatic Go test functions do not require doc comments. It reports no concrete missing production documentation. No threshold or review setting was changed for this PR. The individual matrix-check naming concern was separately refuted in the review: the stable required aggregate depends on the complete matrix, and native rules confirm the aggregate is the required status.
  • Tried as a workflow user: native run 39136489159 actually admitted Talos three seconds after the last prerequisite, executed its known version upgrade, four nested API-readiness trials and cleanup, and completed every scenario. Full feedback was 47m04s versus 57m24s in the comparable baseline; selected runner time was 1.5% higher, not a cost saving. This is one observed pair, not a guaranteed future speedup. Full measurements and caveats are in the validation record.
  • Final hygiene: exact signed head verified, PR-body contract passes on published readback, no conflicts, GitHub reports CLEAN, and main's interrupted scan has been recovered in Restore a complete main vulnerability scan after runner interruption #7866 with enforcement unchanged. The gate helper's previously recorded managed code-quality configuration remains UNVERIFIED; it is not being called complete or used to dismiss a BLOCKED state. GitHub's fresh CLEAN verdict is authoritative here.

Promoting and directly squash-merging with an exact-head pin. This is CI-only; no CLI release is expected. Issue #7863 stays open for merge/readback verification before completion.

@devantler
devantler marked this pull request as ready for review October 18, 2026 15:28
@devantler
devantler merged commit 477cd0c into main Oct 18, 2026
92 checks passed
@devantler
devantler deleted the codex/ci-long-cases-first-7863 branch October 18, 2026 15:28
@github-project-automation github-project-automation Bot moved this from 🫴 Ready to ✅ Done in 🌊 Project Board Oct 18, 2026
@devantler

devantler commented Oct 18, 2026 •

Copy link
Copy Markdown
Contributor Author

🤖 Generated by the Agentic Engineer

Post-merge readback for signed 477cd0c11be803c740263652fd743ace75980ded:

  • Its tree exactly matches the reviewed and fully tested candidate 012f8a4147497746ace67f47aa6b022afc35bd9e.
  • The affected five-file CI-contract suite passes on the actual merged commit with the race detector (2.605s).
  • Ordinary main CI 39140643041 succeeded at 15:48:41 UTC, including its real vulnerability scan, required aggregate, workflow contracts, Bash 3.2 check and home-isolation guard. The native watcher exited successfully; no watcher is left running.
  • Owned CodeQL and Release succeeded. Release explicitly reported no relevant changes and no version publication, as expected for this CI-only increment. Managed Code Quality was still running at the 15:49 UTC readback; the subsequent native read confirms run 39140642142 succeeded at 15:59:06 UTC. All five ordinary main workflows for this merge have now completed successfully.

#7863 is completed and its project item is Done. The ordinary full PR performance trial remains the measured before/after evidence (57m24s to 47m04s, all 30 cases preserved); the shorter main run has a different selected footprint and is not substituted into that comparison. The parent recovery epic remains open for its separate outcomes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: ✅ Done

Development

Successfully merging this pull request may close these issues.

1 participant