Skip to content

ci: reuse the candidate binary for mirror warming - #7864

Merged
devantler merged 3 commits into
mainfrom
codex/warm-mirror-binary-artifact-7862
Oct 18, 2026
Merged

devantler merged 3 commits into
mainfrom
codex/warm-mirror-binary-artifact-7862

Conversation

@devantler

@devantler devantler commented Oct 18, 2026 •

Copy link
Copy Markdown
Contributor

🤖 Generated by the Agentic Engineer

Why

The image-preparation stage waits for a built program, then downloads its compiler inputs and looks up another copy anyway. The measured baseline spends 48 seconds on that redundant preparation and can rebuild again if the shared cache is evicted.

What

Reuse this run’s already-built, verified program while preserving image checks, test coverage and access limits. The replaced preparation measured 48 seconds → 18 seconds in the native run. Full-job timings were 9m56s → 6m31s, but different image-pull times prevent attributing that whole difference to this change.

Part of #7862

@devantler

Copy link
Copy Markdown
Contributor Author

🤖 Generated by the Agentic Engineer

Validation and bounded scope for the mirror-warmer artifact consumer:

  • RED: the regression initially failed because the actual warmer had no producer-artifact restore. The repair consumes the existing pinned, SHA-256-verifying installer with the producer's exact artifact ID and sealed binary digest. It removes compiler setup, the module download and the second binary-cache lookup, not any warming or validation.
  • The complete CI harness passed under the race detector after integrating merged main 83938d8a149a2cf1c4805007f9932b2aa2e07891 (56.433s). The final guards additionally require fail-closed job/step outcomes and the installer's default executable destination. Focused binary-artifact tests pass (3.991s); affected lint reports zero issues. The final complete harness rerun passed in 63.343s before publication at signed head 8363ebd655cc586e4b6c341e1c0bfdc99a0110ce.
  • Existing behavioral tests exercise the actual artifact installation and executable, rejecting absent/empty/symlinked/corrupt inputs, wrong IDs/digests and unprivileged or consumer-only reruns. The pinned installer matches the reviewed local action bytes. No application Go implementation changes are included; full application tests, build and all selected system tests are required in hosted CI.
  • Correctness/security and simplification self-review: this job already depends on the producer and required Go admission, and retains read-only permissions. Its disk cleanup, registry authentication, image validation, warming/publication and every other job remain unchanged. All 30 Docker scenarios, including all three Calico migrations, remain selected. No credential, runner-capacity, paid service or new cache writer is introduced.
  • The measured earlier warmer used 48s for the three removed preparation steps (1s setup, 43s module restore, 4s binary cache) within a 9m56s job. Native after-timings and full exact-head CI/review remain required. A per-step reduction is not automatically a whole-workflow improvement.

This ordinary consumer is also intended to verify #7860's main-populated immutable baseline cache and #645's maintained organization-required Go source. Those issues stay open until their native criteria are actually observed. Preserve #7348's historical failed budget windows.

@github-actions

Copy link
Copy Markdown
Contributor

@github-code-quality

Copy link
Copy Markdown
Contributor

Code coverage overview

Languages: Go

Go / code-coverage/go

main 83938d8 codex/warm-mirror-bi... 8363ebd +/-
72% 72% 0%

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

@devantler

Copy link
Copy Markdown
Contributor Author

🤖 Generated by the Agentic Engineer

Native component evidence at unchanged head 8363ebd655cc586e4b6c341e1c0bfdc99a0110ce:

  • Required Go run 39126460789 succeeded with all eight validation jobs selected and passing, plus change detection and the signed-fix jobs. Complete native log reads show cache disabled on every setup-go invocation, main-backed module restores and no Go archive saves. Ordinary test and race/coverage retain separate main compilation modes. This also completed .github [bug]: binary on macOS is throwing error when downloaded via GitHub releases. #645's independent source-adoption acceptance; it does not complete this PR's remaining system matrix.
  • Mirror warmer 117675042325 passed in 6m31s. The producer-artifact restore took 18s, compared with the earlier three preparation steps totaling 48s. This is a measured 30s / 62.5% reduction in the replaced preparation, in one native pair. The old whole job took 9m56s, but warming itself changed from 6m07s to 3m38s and disk cleanup also varied; do not attribute the full 3m25s difference to this patch.
  • The real missing-image refusal test still passed before warming. Binary restoration, warming and validated artifact publication all passed. Candidate artifact 12135624631 and mirror artifact 12136782096 are fresh run-local artifacts bound to this exact head; the latter is 2,810,517,353 bytes with digest sha256:7002599529986674702e1286bcbaaebc576fc18edf1584cb27572f459dcd6cbe. Artifact storage is not repository cache usage.
  • The independent Calico producer recorded a genuine exact main-cache hit and completed in 18s versus 9m28s cold; Reuse the immutable Calico baseline from a main-backed cache #7860 is closed with its full delivery/inventory receipts. Preserve Mirror caches are evicted before system tests restore them #7348's failed historical windows and managed-writer/sustained-budget requirements.

This remains draft until all selected native CI, including all 30 Docker scenarios and three migrations, finishes successfully, a fresh current-head semantic review is green, findings are resolved and the complete promotion/merge gates are rebound. No external review has been requested early. Local validation completed before the subsequent disk-floor hold; no additional local build/test or cluster work is being run under that hold. The body now reports measured component timings without claiming a controlled whole-CI speedup.

@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: fe5e8bc1-c30f-465f-9df5-f0bacccf222f
📥 Commits

Reviewing files that changed from the base of the PR and between 83938d8 and 8363ebd.

📒 Files selected for processing: 3
  • .github/workflows/ci.yaml
  • internal/ciharness/binary_artifact_test.go
  • internal/ciharness/go_module_cache_test.go

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: 3
.github/workflows/ci.yaml: 1

1866-1866: LGTM!

Also applies to: 1867-1867, 1868-1868, 1869-1869, 1871-1871, 1872-1872

internal/ciharness/binary_artifact_test.go: 1

87-138: LGTM!

internal/ciharness/go_module_cache_test.go: 1

30-32: LGTM!

📝 Walkthrough

The warm mirror-cache job now restores the binary artifact produced by build-artifact, using its artifact ID and SHA-256. It no longer sets up Go, restores Go modules, or restores the binary through the cache action. Workflow tests verify the artifact binding, job settings, and step order.

Priority: ⬇️ Low

Merge Risk: ⚪ Minimal · up to 8363e

This change makes mirror-cache warming reuse the verified binary built earlier in the run, which should shorten the job. No actionable merge-blocking risk is visible; hosted CI results remain the normal final check.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 2 files. (1 skipped: 1… 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 and concisely identifies the main change: reusing the candidate binary for mirror warming.
Description check Passed The description explains why the change is needed and what outcome it provides. It directly matches the changeset.
Full details: Docstring Coverage

Explanation

Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 2 files. (1 skipped: 1 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

Finishing evidence at 8363ebd655cc586e4b6c341e1c0bfdc99a0110ce:

  • The complete native run 39126461063 is SUCCESS at this exact head. All 30 Docker scenarios passed, including the three Calico migrations and the named Talos upgrade case. The complete PR rollup has 86 successful and six skipped contexts, with no failed or unfinished checks.
  • I verified the actual mirror-warmer job: the producer binary was restored successfully in 18 seconds, then warming and validated artifact publication succeeded. The missing-image refusal control also passed. This observes the changed runtime path; it is not just a workflow trace. The earlier 48-second preparation comparison remains a component measurement, not a controlled whole-workflow speedup.
  • The authenticated CodeRabbit summary was updated after this run's request, names this exact head, and the reviewed summary classifier returns GREEN. There are zero review objects, zero unresolved threads and no code findings. Correctness/security and simplification review of the complete diff found no additional issue: the job retains its producer/admission dependencies, read-only permission and fail-closed artifact verification.
  • The selected review's ancillary Docstring Coverage warning is assessed and refuted for this change: the diff adds one Go test declaration, TestKSailBinaryArtifactMirrorWarmerConsumer, whose descriptive name and assertions specify the contract, and updates an existing test's job inventory. It introduces or modifies no production Go declaration or public product API requiring a doc comment. The aggregate warning includes test declarations beyond the new test; no validation threshold, test or production control is changed or waived.
  • The existing PR body passes the effective template checker. Every readable required gate passes. The native code_quality rule is configured but remains UNVERIFIED because the helper has no per-head analysis surface; this is not reported as COMPLETE or waived. Per the consumer merge policy, GitHub's head-pinned merge API remains the authority for that native rule.

No provider trial or production mutation was performed.

@devantler
devantler marked this pull request as ready for review October 18, 2026 14:23
@devantler
devantler merged commit b9b42e8 into main Oct 18, 2026
92 checks passed
@devantler
devantler deleted the codex/warm-mirror-binary-artifact-7862 branch October 18, 2026 14:24
@github-project-automation github-project-automation Bot moved this from 🫴 Ready to ✅ Done in 🌊 Project Board Oct 18, 2026
@devantler

Copy link
Copy Markdown
Contributor Author

🤖 Generated by the Agentic Engineer

Post-merge verification is complete for b9b42e858dae2dbc887650a8ed9d8d4b2b660265:

  • Main CI, Code Quality, CodeQL, Release and TODOs all completed successfully at that exact commit. Main CI’s required-check rollup passed.
  • The merged and tested candidate commits independently resolve to the identical Git tree e48ee6a34cc7759b7b13e1eec47599a26393dda2; the delivered CI workflow blob is e96328e766fc52989fc7005a307ea05837d1ebcc. Candidate CI 39126461063 is also confirmed successful at 8363ebd655cc586e4b6c341e1c0bfdc99a0110ce.
  • Main CI skipped the mirror warmer and Docker system matrix. The actual warmer and 30-scenario evidence therefore remains the pre-merge candidate evidence above; these main results add no new timing measurement or whole-workflow speedup claim.

Issue #7862 remains closed with its existing acceptance criteria. The latest published release is still v7.203.24 from earlier today; a successful Release workflow is not evidence of a new published version for this CI-only change. No further verification action remains for this PR.

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