ci: shared per-run build + nightly-warmed builder image for PR CI - #418
Conversation
PR CI compiles the same head commit twice: the standalone pessimistic job builds natively with -cover and the compose proxy job repeats the whole build (thirdparties + full Go graph) inside docker. Add a build-mo.yaml reusable workflow that builds the -cover mo-service once and uploads the self-contained runtime tree (mo-service, lib/, dict/, cgo .so) as a run-scoped artifact. Both BVT workflows gain an optional build_artifact input: when set they download the artifact, verify its head sha and build flavor, and skip their own build; on any miss or mismatch they build from source exactly as before, so this lands safely before the matrixone entrypoint passes the input. The compose job assembles its runtime image from the artifact via optools/images/Dockerfile.prebuilt when the head ships it. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Every PR build recompiled all PR-invariant inputs from scratch: the Go module graph, the full Go build cache, and the C thirdparties. Add a ci-builder job to the nightly image workflow that bakes those into matrixorigin/matrixone:ci-builder (Dockerfile.ci-builder in matrixone), and teach build-mo.yaml to mount the PR checkout into that image and run make build there: only packages the PR changed recompile, and go build incrementally downloads modules newer than the baked cache, so a stale image only costs speed, never correctness. Measured locally: unchanged tree ~20s, worst-case exported-API change to pkg/container/types ~54s, versus ~10 minutes cold. Building inside the ubuntu-22.04 toolchain container also makes the binary match the matrixorigin/ubuntu:22.04 runtime image glibc by construction. When the image pull fails (not yet published, registry outage) the job falls back to the existing from-source build on the runner, so this is safe to land before the first nightly image exists. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The UT job (ci.yaml) and coverage-UT job compile the full tree from scratch every run: ~10 minutes of go-test compilation plus a C thirdparties build. The nightly ci-builder image now warms exactly those compile flavors (-race -tags matrixone_test, and covermode=set with the verbatim coverpkg list), and the Go build cache is location-independent for pure-Go packages, so both jobs gain a best-effort seed step: pull the image, extract the build/module caches into the runner's cache paths, and stash the prebuilt thirdparties for restoration after make clean (which wipes them; go clean -testcache only invalidates cached test results, never compiled objects). prepare_coverage_cgo.sh honors the same MO_PREBUILT_THIRDPARTIES seed so make cgo stops rebuilding the C thirdparties. The module cache extracts via a tar stream with --no-same-permissions because its read-only directory layout breaks plain docker cp. Every seed path is guarded and continue-on-error: no docker, no image, or a failed pull just leaves the job building cold as today. Also give the shared build job environment: ci so the optional Docker Hub credentials resolve, and extend the ci-builder nightly timeout for the two extra warm flavors. Verified locally against a checkout at a different path with the host toolchain: full ./... race-flavor test compile in 3m18s warm (1m11s rerun floor), make cgo in ~11s with seeded thirdparties. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Extend the nightly ci-builder to both CI architectures via a build matrix, published per-arch and combined under one multi-arch manifest tag so every consumer pulls matrixorigin/matrixone:ci-builder and gets its own architecture. The arm64 leg warms what the SCA job compiles (plain build + the full static-check pass), and the SCA job gains the same guarded seed step as the UT jobs, additionally restoring golangci-lint's analysis cache. Mirror all ci-builder tags to ACR (registry.cn-shanghai.aliyuncs.com) and make every pull site prefer the ACR mirror: the SCA and coverage-UT runners are self-hosted in Shanghai/Guangzhou where Docker Hub is slow or unreliable. The ACR attempt is time-bounded (300s) so a slow cross-region pull from GitHub-hosted runners cannot eat the savings, with Docker Hub as the fallback and a cold build as the final fallback. Both registry logins are optional secrets and best-effort. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
XuPeng-SH
left a comment
There was a problem hiding this comment.
Requesting changes at head 176f2059. The shared-once build direction is sound, but the producer/cache owner still has three correctness and security blockers.
-
[P0] Keep untrusted PR compilation outside the privileged credential boundary. The current matrixone caller is a
pull_request_targetworkflow usingsecrets: inherit. This reusable job addsenvironment: ci, checks out the fork head with the supplied token and default persisted credentials, logs into registries, and then executes the PR-controlledMakefile. The host cold path can directly read runner credential state, and the shared build does not need privileged secrets to validate a public PR. Please run head compilation in an unprivilegedpull_requestcontext with no inherited/environment secrets andpersist-credentials: false, or split trusted cache preparation from untrusted compilation so credentials and untrusted code never share a runner. The CI workflow should make that trust contract explicit rather than relying on every caller to remember it. -
[P1] Do not restore
thirdparties/installwithout exact input compatibility.build-mo.yaml,ci.yaml, andcoverage-ut.yamlcopy the nightly native tree into the PR checkout unconditionally. The thirdparty file targets do not depend on all source archives, Makefiles, flags, compiler/base-image identity, target OS/arch, and branch state, so a PR changing native inputs can test the nightly main implementation and go falsely green. The smallest safe first rollout is to seed only Go module/build caches. If native reuse is retained, require an exact manifest/fingerprint covering every native input and toolchain dimension and rebuild on any mismatch. -
[P1] Close fallback inside the shared-build producer. The caller makes both BVT consumers hard
needsdependents. If the cacheddocker runor builder image itself is stale/broken, this job fails and both consumers are skipped; their missing-artifact fallback is unreachable. Treat the cached/container build as best effort, remove all partial outputs after failure, set up the checkout Go version, and run the existing clean host build. Publish only after one path succeeds; a real cold compile failure should still fail once and block both consumers.
Before calling the speedup proven, please also report end-to-end measurements including compressed builder-image size and pull time on both GitHub ubuntu-22.04 westus and the China self-hosted pools. Bound both ACR and Docker Hub pulls; today ACR may consume 300 seconds and the Docker Hub fallback has no explicit bound, so a cache outage can make the optimized path slower than the baseline.
Suggested acceptance closure: fork PR cannot access inherited/environment/checkout/registry credentials; native-input change and divergent release branch force a native rebuild; forced cached-build failure completes through a clean cold build; produced artifact is SHA/flavor/platform-validated and smoke-tested by both standalone and compose consumers.
aptend
left a comment
There was a problem hiding this comment.
Requesting changes at head 176f205.
-
[P0] Keep untrusted PR code outside the privileged credential boundary. The caller is pull_request_target with secrets: inherit; this workflow checks out the PR head, logs into registries, and then executes the PR-controlled Makefile. Move PR compilation to an unprivileged pull_request workflow with no inherited/environment secrets and persist-credentials: false, or strictly separate trusted cache preparation from untrusted compilation.
-
[P1] Do not restore thirdparties/install without exact compatibility validation. The native targets lack source/config/toolchain prerequisites, so a PR changing native archives, headers, flags, or a release branch diverging from main can test the nightly main artifacts and falsely pass. Seed only Go caches, or require an exact fingerprint covering native inputs, branch, toolchain, OS/arch, base image, and build flags.
-
[P2] Close fallback inside the shared-build producer. MatrixOne#27077 makes both BVT consumers hard needs dependents, so a producer failure skips them before their cold-build fallback can run. Treat cached/container failures as best effort, clean partial outputs, and retry once with a clean host build while preserving genuine PR compile failures.
Please also isolate and clean Docker credentials; raw docker login persists credentials in runner Docker state that later PR-controlled steps can use.
…ache, contained fallback Review fixes for #418 / matrixone#27077 (XuPeng-SH, aptend, LeftHandCold): [P0] No credential may share a runner with PR-controlled code. The shared build job drops environment: ci and every registry secret; all ci-builder pulls (build-mo and the three seed steps) are now anonymous and the checkout no longer persists its token. If anonymous ACR pulls are not permitted, provision a pull-only credential at the runner/node level via infrastructure — never via workflow-level docker login, which persists credentials in runner docker state that later PR-controlled steps can read. [P1] The prebuilt thirdparties/install tree now restores only on an exact fingerprint match (schema, os, arch, git rev-parse HEAD:thirdparties — the tree hash covers every archive, header, the Makefile with its flags, and the download script) at all four consumers; any mismatch clears the seed and builds native deps from source. Go module/build caches remain best-effort acceleration, which is always correctness-safe. [P2] Fallback now lives inside the producer: the cached container build is best-effort (continue-on-error) with ownership repair and a loader smoke test; on any failure the job sets up the checkout's Go toolchain and runs the existing make clean cold build, and packaging (with a final env -u LD_LIBRARY_PATH smoke test) runs after either path. A consumer skip via needs now only happens on a genuine compile failure, which is the intended behavior. Also per review: every runtime DSO ships under lib/ ($ORIGIN/lib), the prebuilt-cgo/ contract is gone, and the Docker Hub fallback pull is time-bounded like the ACR attempt so a registry outage cannot make the optimized path slower than the cold baseline (layers are content- addressed, so a timed-out ACR attempt still counts toward the Docker Hub attempt). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Thanks @XuPeng-SH @aptend — all points addressed at head 8b18024 (with matrixone-side e417582532): [P0] Credential boundary. The shared build job now holds no credentials at all: [P1] Native cache compatibility. [P2] Fallback containment. The cached container build is now best-effort inside the producer: Pull bounds. Both the ACR and Docker Hub pulls are bounded at 300s each. Layers are content-addressed, so a timed-out ACR attempt still counts toward the Docker Hub attempt of the same image; worst case a full registry outage adds a bounded ~10 min before the cold path, typical is 1–3 min. Artifact contract. All runtime DSOs consolidated under Measurements. Local: incremental 🤖 Generated with Claude Code |
aptend
left a comment
There was a problem hiding this comment.
Requesting changes at head 8b18024 after re-review.
[P1] Preserve the cached build's failure status before running the loader smoke test. The step starts with set -uo pipefail (without -e), captures docker run in status, and evaluates [ "${status}" -eq 0 ], but a false test does not stop the script. If make build produces mo-service and then fails a later validation, the subsequent ./mo-service -h can succeed, making the entire step successful. The host cold-build fallback is then skipped and the artifact is published despite the failed build target.
Please explicitly exit on nonzero status after the ownership repair, for example:
if [ "${status}" -ne 0 ]; then
exit "${status}"
fiThe previous credential and native-cache blockers are otherwise addressed at this head: registry pulls are anonymous with non-persisted checkout credentials, and native restoration is fingerprint-gated.
…ge producer, locality-routed pulls, Go-caches-only host seeding Re-review fixes for #418 / matrixone#27077: [P1, aptend] The cached-build step ran without set -e, so its bare status test could not stop the script: a failed docker run whose leftover mo-service still passed the loader smoke test would mark the step successful, skip the cold fallback, and publish an artifact from a failed build. Explicitly exit with the docker run status right after the ownership repair. [P1, XuPeng-SH] The producer no longer accepts any caller secret: the TOKEN_ACTION declaration and checkout token are gone (public anonymous checkout with the job's read-only default token), and the workflow caps permissions to contents: read so the trust contract is enforced here rather than remembered by callers. The matrixone caller drops secrets: inherit in tandem. [P1, XuPeng-SH] Route registry pulls by runner locality via RUNNER_ENVIRONMENT: GitHub-hosted runners try Docker Hub first and the Shanghai ACR second, self-hosted pools the reverse; each attempt stays bounded at 300s and completed layers carry across attempts. When both fail, prune partially pulled layers so they cannot starve the cold build of disk. [P2, XuPeng-SH] Host jobs (UT, SCA, coverage-UT) now seed Go module/build caches ONLY — the smaller safe first rollout. Reusing prebuilt native libraries outside the builder container would need a producer/consumer toolchain-ABI fingerprint (CC, cmake, base-image identity), which schema/os/arch/tree-hash does not capture across different runner images. The container path in this workflow keeps fingerprint-gated native reuse because producer and consumer toolchains are the same image by construction. prepare_coverage_cgo.sh reverts to its original form. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Re-review round addressed at head 2c39853 (matrixone side ad96d0c882): [P1] Cached-build failure propagation (@aptend, head 8b18024). Confirmed — without [P1] Producer credential boundary (@XuPeng-SH #2). The producer now accepts no caller secrets at all: the [P1] Registry routing & disk (@XuPeng-SH #3). Pulls now route by [P2] Native-cache toolchain dimensions (@XuPeng-SH #4). Adopted your smaller safe first rollout: UT/SCA/coverage-UT now seed Go module/build caches only — the content-addressed Go cache is always correctness-safe — and native deps keep building from source on those hosts; Process/canary (@XuPeng-SH #1). Agreed: #27077 stays draft; after this PR merges I'll dispatch the image workflow, then run a canary on the exact #27077 head — including a forced cached-build failure — and post per-pool compressed-size/pull/build timings plus cache-hit/miss/registry-unavailable outcomes here before it leaves draft. 🤖 Generated with Claude Code |
|
@XuPeng-SH @aptend — friendly status note: aptend's deep re-review on matrixone#27077 (2026-08-14) examined this PR's exact head 2c39853 jointly and confirmed the previously raised blockers are fixed (producer permissions cap, no inherited secrets, cached-build status propagation, host seeding scaled to Go caches only). The remaining item over there was purely the readiness state, now restored to draft. This PR is the head of the merge sequence — when you get a chance to re-review, everything downstream (first image publish, canary with forced cached-failure, per-pool measurements) is queued behind it. 🤖 Generated with Claude Code |
…cs, atomic cache seeding Findings from a systematic pre-publish self-review (multi-agent, all findings verified against primary sources), each reproduced in local simulation before and after the fix: - The cached-build step now clears the Actions default errexit (set -u +e -o pipefail): previously the status capture and ownership repair were dead code — bash -e exited at the failed docker run, so the exact failure path the fallback was written for left root-owned partial outputs and failed the job. Ownership repair additionally runs inside the container via an EXIT trap (chown to the host UID/GID), removing the dependency on host passwordless sudo. - Both consumers guard artifact extraction: a truncated or corrupt mo-build.tar.gz now falls through to the from-source / full-docker build instead of hard-failing the step. The standalone consumer also loader-smoke-tests the staged binary (rpath resolves against the staged lib/) before committing to it, closing the glibc/toolchain-skew arc, and extracts the tar exactly once (BUILD_INFO is packed first). - Seed steps are rewritten around failure atomicity: early-exit before any pull when the runner's cache is already warm (persistent runners), a free-disk guard (<30GB skips seeding — cold build is strictly safer than mid-job ENOSPC), extraction into same-filesystem staging dirs renamed into place only when complete (a mid-stream failure can never leave a half-populated cache that a later run mistakes for warm), the nightly warm-status ledger surfaced in the log, and the image freed with docker rmi before the job's real work needs the disk. - The nightly ci-builder and manifest jobs are continue-on-error so a builder-image problem cannot turn the product image pipeline red; consumers just build cold until the next successful publish. Verified end-to-end locally: five consumer degradation cases (good/mismatch/corrupt/smoke-fail/missing), three cached-step status cases, staged-seeding happy path against the real image (11G+1.8G installed atomically in 32s, no residue) and mid-stream failure (both caches left empty, no residue). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… compose-based pessimistic BVT Upstream (#419-#423) landed two things this branch must integrate with: - The SCA job gained its own trusted actions/cache pipeline (module cache from matrixorigin/matrixone's sca-go-module-cache.yaml producer, native prerequisites keyed on compiler identity plus full native-input hashes, cached static-check tools). That mechanism subsumes this branch's SCA seed step — including the toolchain-ABI fingerprint this branch deliberately deferred — so the SCA seed is dropped, the nightly ci-builder publishes amd64 only (the arm64 leg existed solely for SCA), and the multi-arch manifest job goes away. - The pessimistic standalone BVT was rebuilt on the compose topology (its native launch build kept exceeding the job timeout). The old native-binary artifact consumer no longer applies; both BVT jobs now consume the shared build identically, assembling the runtime image from the artifact via Dockerfile.prebuilt with the same guarded fallback to the full in-docker build. The UT job keeps both the upstream Shanghai-proxy canary and this branch's cache seed; they are independent. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Merged latest main into this branch (head cd102e2) and re-validated end to end. Two upstream changes required real integration decisions:
Re-verified on the merged tree: 🤖 Generated with Claude Code |
## What type of PR is this? - [x] Test and CI ## Which issue(s) this PR fixes: issue #27076 ## What this PR does / why we need it: MatrixOne side of the PR-CI speedup (workflows side: matrixorigin/CI#418). 1. **`entrypoint.yaml`**: add a `matrixone-shared-build` job (calls `matrixorigin/CI/build-mo.yaml@main`) that builds the `-cover` mo-service once per run; the standalone and compose BVT workflows consume the artifact instead of each spending ~10 minutes on identical builds. A compile-broken PR now fails once, in ~4 minutes. 2. **`optools/images/Dockerfile.prebuilt`**: assembles the compose runtime image from the prebuilt artifact in seconds instead of a full in-docker rebuild; its `/mo-service -h` smoke check guards the glibc contract and any failure falls back to the full docker build. 3. **`optools/images/Dockerfile.ci-builder`**: the nightly-refreshed builder image (published by the existing nightly image workflow as `matrixorigin/matrixone:ci-builder`, amd64+arm64, Docker Hub + ACR mirror). It bakes every PR-invariant build input — Go module cache, Go build caches warmed in exactly the flavors PR CI compiles (`-cover` build; `-race -tags matrixone_test`; `-covermode=set` + coverpkg; arm64 static-check), and the compiled C thirdparties. A stale image only costs speed, never correctness: `go build` recompiles and downloads incrementally. Measured locally: incremental `-cover` build 20s (unchanged) to ~1min (worst-case exported-API change to `pkg/container/types`) vs ~10min cold; full race-flavor test compile ~3min warm; `make cgo` ~11s with seeded thirdparties. **⚠️ MERGE ORDER: matrixorigin/CI#418 must merge first.** This PR references `build-mo.yaml@main` and passes a `build_artifact` input — merging it before the CI PR breaks every PR's CI at workflow-parse time. Draft until then. After both merge, manually dispatch the "Build and Push Images" workflow once to publish the first `ci-builder` image (until then, all consumers fall back to today's cold builds). 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
## What type of PR is this? - [x] Test and CI ## Which issue(s) this PR fixes: issue #27076 ## What this PR does / why we need it: Publish-prerequisite split from #27077, per the reviewer-required rollout order there. The nightly `ci-builder` job (already on `matrixorigin/CI@main` via matrixorigin/CI#418) checks out **main** and skips gracefully while `optools/images/Dockerfile.ci-builder` is absent — so the first builder image cannot publish until this file lands. **Zero behavior change for any existing job**: nothing on main references either file until the entrypoint wiring in #27077 merges. What this unblocks, in order: 1. Dispatch/nightly of "Build and Push Images" publishes `matrixorigin/matrixone:ci-builder` (amd64, Docker Hub + ACR mirror). 2. The UT and coverage-UT cache seeds already live in `matrixorigin/CI ci.yaml` go from no-op to active (guarded, `continue-on-error`, cold-build fallback — validated in matrixorigin/CI#418's review rounds). 3. The canary #27077's reviewers require (shared-build path incl. forced cached-build failure) becomes runnable, after which #27077 leaves draft. Both Dockerfiles passed `docker build --check`, and a full local image build on current main + fingerprint/warm-status verification is documented in matrixorigin/CI#418. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
What this PR does / why we need it:
Cuts the repeated cold compilation out of MatrixOne PR CI. Tracking issue: matrixorigin/matrixone#27076.
build-mo.yaml(new reusable workflow) — builds the-covermo-service once per run and uploads the self-contained runtime tree (~99MB artifact). It builds inside the nightlyci-builderimage when reachable (only PR-changed packages recompile; the binary links against the ubuntu-22.04 toolchain, matching the runtime image glibc by construction), and falls back to a from-source runner build otherwise.e2e-standalone-parallelande2e-compose-parallelaccept an optionalbuild_artifactinput: download, verify head sha + flavor, and skip their own ~10-minute builds; any mismatch falls back to building from source.optools/images/Dockerfile.prebuilt(matrixone side) assembles the compose runtime image from the artifact in seconds.-race -tags matrixone_test;-covermode=setwith the verbatim coverpkg list; the full static-check pass on arm64). Guarded,continue-on-errorseed steps extract the Go build/module caches (and golangci-lint's cache for SCA) onto the runner; the Go build cache is location-independent for pure-Go packages so tests keep running on the host exactly as today.prepare_coverage_cgo.shhonors a seeded thirdparties dir somake cgostops rebuilding the C thirdparties.ci-builderimages — added toimage-build.yaml: amd64+arm64 matrix, combined under one manifest tag, pushed to Docker Hub and mirrored to ACR; every pull site prefers the ACR mirror (time-bounded) because the SCA/coverage runners are in Shanghai/Guangzhou. Skipped on release events and on branches without the Dockerfile.Merge order: this PR is a safe no-op on its own (all new inputs optional, all seeds degrade to cold builds) and MUST merge before the matrixone-side PR that wires
entrypoint.yamltobuild-mo.yaml.Measured locally: incremental
-coverbuild 20s–1min vs ~10min cold; full race-flavor test compile ~3min warm;make cgo~11s with seeded thirdparties.🤖 Generated with Claude Code