Skip to content

Accelerate CI image materialization with same-run snapshots - #74173

Open
zozo123 wants to merge 27 commits into
apache:mainfrom
zozo123:ci/ci-image-data-root-snapshot
Open

zozo123 wants to merge 27 commits into
apache:mainfrom
zozo123:ci/ci-image-data-root-snapshot

Conversation

@zozo123

@zozo123 zozo123 commented Oct 3, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Reduce repeated CI image materialization by publishing an optional Docker data-directory snapshot from the existing builder. Consumers in the same pull-request workflow run restore it when it fits their daemon; otherwise they use the existing image stash and breeze ci-image load. Image builds and test coverage are unchanged.

How it works

  • Create: after the image and mount cache are published, the builder runs breeze ci-image save --snapshot-file, which calls scripts/ci/docker_data_root_snapshot.sh create.
    • It keeps only the CI image and prunes the build cache.
    • It archives /var/lib/docker/{image,overlay2} with --xattrs --xattrs-include='*', so overlay2's trusted.overlay.opaque markers and file capabilities survive.
    • The archive goes to runner temporary storage, off the AMD Docker data disk.
  • Metadata: a separate 362-byte meta artifact records the daemon fingerprint, the image ID, the checkout revision, and a hash of the image's merged file tree.
    • The meta reads unavailable until the archive is complete, and it is uploaded even when the export fails.
    • Both artifacts are uploaded with overwrite: true, so a re-run never restores an earlier attempt's snapshot.
  • Restore: a shared load_image action, used by both prepare_breeze_and_image and prepare_single_ci_image, does this in order:
    1. It downloads the meta by pattern, so a missing snapshot adds no annotation.
    2. It runs a check of the revision, the daemon fingerprint and storage driver, and that the daemon holds no images.
    3. Only then does it download the snapshot and restore it through breeze ci-image load --snapshot-file, which keeps breeze's environment checks and diagnostics.
  • Verification:
    • After a restore, the image ID and the file-tree hash must match the meta. Otherwise the image store is wiped and the job falls back to the stash.
    • Composite steps cannot take timeout-minutes, so the script bounds each step itself: daemon stop and start at 60 s each, extraction at 300 s, and the check at 60 s.
  • Scope:
    • Snapshots run only for pull_request events on GitHub-hosted runners.
    • They are kept for one day and only read within the run that uploaded them.
    • An unsupported image store, such as the containerd store, is skipped before any pruning or upload.

Measurements and artifact budget

The measurements come from two full PR runs of the final design: 39190862270 on 6dabcd9 and 39199861265 on 6da2d0f. The baseline is 237 consumer jobs from three full-CI PR runs without the snapshot: 39141869252, 39143583838 and 39150120169. Durations are the composite-step duration_ms values from the job logs.

stash (baseline) snapshot, 39190862270 snapshot, 39199861265
acceptance – 79 / 79, 0 fallbacks 79 / 79, 0 fallbacks
download, median 53 s (2.38 GB) 86 s (2.55 GB) + 1–3 s for the meta and check 60 s (2.55 GB) + 1–3 s for the meta and check
load / unpack, median breeze ci-image load 103 s unpack via breeze 40 s unpack via breeze 47 s
total per consumer, median / p90 164 s / 210 s 135 s / 174 s 105 s / 141 s
extra builder time – 84 s 100 s

Net per consumer job: ready 29–59 s sooner at the median. Unpacking replaces the 103 s docker load with 40–47 s, but the snapshot download is slower than the stash download and gives back part of that.

Net per run: the builder finishes 84–100 s later, and every consumer starts that much later. Each consumer therefore finishes about 40–55 s later in wall-clock time. Across the 79 consumers, the run uses about 37–76 fewer runner-minutes. So this saves runner time, not wall-clock time.

Storage: both transports are kept: the snapshot is 2.55 GB and the stash 2.38 GB per Python × platform per run, about 4.9 GB together. The snapshot is kept for one day, compared with two for the stash, so it adds about 2.55 GB-days per combination per run. Matrix size and overlapping runs multiply this. Maintainers responsible for ASF artifact quota and CI capacity need to accept this trade-off before merging.

Scope and validation

  • The Dockerfile dependency-layer optimization moved to Reuse locked CI dependencies across workspace source changes #74466.
  • 40 snapshot tests run the real script through command shims. They cover create, check and restore, every failure and fallback path, the tree-hash and xattrs checks, and the workflow and action wiring.
  • Breeze tests cover ci-image save and ci-image load with --snapshot-file, including the exit codes when a snapshot cannot be created or restored. All 22 tests in test_ci_image_commands.py pass.
  • A Docker-in-Docker end-to-end run with Docker 28.5.2 and overlay2 restores a real snapshot. It also confirms that a restore made without --xattrs-include is rejected: the image ID stays the same, but the file tree differs.
  • An earlier revision hashed the file tree with sort inside the container. sort wrote temporary files into /tmp while find was still walking it, so every consumer rejected the snapshot (run 39159493825). The listing is now sorted on the host.
  • CodeQL reports no alerts. The snapshot step now calls breeze instead of running a repository script directly. It runs the same checked-out code as the job's other breeze steps and is limited to pull_request events.

Was generative AI tooling used to co-author this PR?
  • Yes — Claude Code (Opus 5.5) and Codex (GPT-6)

Generated-by: Claude Code (Opus 5.5) and Codex (GPT-6), following the Airflow guidelines.

Comment thread .github/workflows/ci-image-build.yml Fixed
@zozo123
zozo123 force-pushed the ci/ci-image-data-root-snapshot branch from f58c30f to 80d5cb7 Compare October 3, 2026 22:39
Comment thread .github/workflows/ci-image-build.yml Fixed
@zozo123
zozo123 force-pushed the ci/ci-image-data-root-snapshot branch from 1d49fda to bf067ab Compare October 4, 2026 04:49
Comment thread .github/workflows/ci-image-build.yml Fixed

@shahar1 shahar1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Measured on this PR's own CI run: each CI-image consumer saves ~87 s of image prep, but every consumer now starts ~4 min later, because snapshot-ci-images lives inside the reusable workflow those consumers wait on. Net wall-clock per PR goes up, while runner-minutes go down. One structural change (create the snapshot inside build-ci-images) should turn this into a pure win, so I'd like that resolved before merging.

Snapshot job extends the critical path of every consumer (.github/workflows/ci-image-build.yml:407)

Numbers from this PR's run 37206844037 and a baseline PR run from the same afternoon on the same runner pool, 37210552741, both ubuntu-22.04:

baseline this PR
Build CI linux/amd64 image 3.10 finished 14:54:13 13:55:59
first consumer job started 14:54:13 (+0 s) 13:59:55 (+3 min 56 s)
consumer CI-image prep, median 189 s (stash 67 s + load 106 s, n=29) 102 s (download 59 s + unpack 31 s, n=79)

Every job in ci-amd.yml that has needs: build-ci-images waits for the whole called workflow, and snapshot-ci-images is part of it; continue-on-error: true changes the conclusion, not the wait. The job spent 178 s of its 229 s restoring and docker image load-ing the stash that build-ci-images had exported seconds earlier, then 25 s snapshotting and 15 s uploading. So the ~87 s saved per consumer is paid for with ~236 s added before any consumer can start. For a PR the end-to-end duration gets roughly 2.5 min longer; what improves is total runner time (79 consumers × 87 s ≈ 115 runner-minutes in this run). The description lists "end-to-end workflow duration" among the metrics to evaluate but does not report it.

Suggestion: create the snapshot as the last step of build-ci-images instead of in a separate job. The image is already in that daemon, so the 178 s reload disappears and the extra critical path shrinks to roughly the 40 s of create + upload. move_docker_to_mnt.sh bind-mounts /mnt/var-lib-docker onto /var/lib/docker, so DockerRootDir still reads /var/lib/docker and check_supported_daemon passes there. Keep the same gate (github.event_name == 'pull_request' && inputs.upload-image-artifact == 'true' && inputs.image-stash-ref == '') and continue-on-error on the step, and place it after "Stash cache mount": create runs docker builder prune --all, which would otherwise wipe the mount cache before it is exported. If the separate job was chosen for isolation reasons, please spell them out in the job comment. In pull_request context the token is read-only either way, and build-ci-images already runs the PR's own sources.

Smaller observations

  • scripts/ci/docker_data_root_snapshot.sh:53 — check_supported_daemon accepts only overlay2. When GitHub's runner images move to the containerd image store, every snapshot will be rejected, the producer keeps spending its minutes, and nothing surfaces it because the create step is continue-on-error. An echo "::warning::..." on the fingerprint-mismatch and unsupported-store exits would make that visible in the run summary.
  • The description still says the key is ci-image-snapshot-v1-*; the code uses ci-image-snapshot-v2-.
  • The script tests drive the real bash through shimmed docker/sudo/zstd/git and cover every bail-out path. All 23 pass locally in under a second. Nice approach.

Worth a second look from

This change touches the CI image stash/restore flow; folks with the most context here:

  • @potiuk — authored 10 of the last 25 commits on ci-image-build.yml and prepare_breeze_and_image/action.yml (the stash and image-stash-ref design)
  • @jscheffl — 2 of the last 25 commits on the same files

None of them have been notified — asking any of them for an extra pass is the maintainer's call, and optional.


This review was drafted by an AI-assisted tool and
confirmed by an Apache Airflow maintainer. The findings
below are observations, not blockers; an Apache Airflow
maintainer — a real person — will take the next look at the
PR. If you think a finding is mis-applied, please reply on
the PR and a maintainer will weigh in.

More on how Apache Airflow handles maintainer review:
Contributing guide.

Comment thread .github/workflows/ci-image-build.yml Outdated
Comment thread .github/workflows/ci-image-build.yml Outdated
Comment thread scripts/ci/docker_data_root_snapshot.sh
@zozo123
zozo123 force-pushed the ci/ci-image-data-root-snapshot branch from d3c3f01 to 6e8fd38 Compare October 4, 2026 18:44
@zozo123 zozo123 changed the title Restore the CI image from a snapshot of Docker's data directory Restore CI images from Docker data-directory snapshots Oct 4, 2026
@zozo123 zozo123 changed the title Restore CI images from Docker data-directory snapshots Accelerate CI image materialization and dependency reuse Oct 4, 2026
@zozo123
zozo123 requested a review from shahar1 October 4, 2026 21:25
@zozo123

zozo123 commented Oct 5, 2026

Copy link
Copy Markdown
Contributor Author

@shahar1 — measured the current f53486a4 design against a successful baseline with the same active job names. Verdict: this run demonstrates useful CI savings, and supports keeping this optimization. Both total runner time and workflow wall time improved.

Metric Matched baseline This PR Improvement
Median CI-image preparation per consumer 210s 127s 83s faster (39.5%)
Total runner execution time 1,306.0 min 1,236.4 min 69.6 runner-minutes saved (5.3%)
AMD workflow wall time 51m 14s 49m 47s 87s faster (2.8%)
Median gap from CI builder completion to consumer start 3s 3s No increase

All 79 active CI-image consumers successfully restored the snapshot and skipped the legacy image-load action. Snapshot creation took 73s and upload took 23s; the total workflow numbers above already include that 96s overhead.

The critical-path tradeoff is also favorable for the median consumer in this comparison: consumers started 69s later relative to workflow creation, but their faster image preparation meant the median paired job reached the end of preparation 16s earlier. Across all 79 paired consumers, preparation used 110.6 fewer runner-minutes. The total workflow saving is 69.6 minutes after accounting for every executed job, including the builder.

Sources: current-head run, attempt 1 · matched baseline run, attempt 1. Measurements use GitHub Actions job/step timestamps and consumer action outcomes. Wall time is workflow creation to the last executed job completing; runner time is the sum of executed job durations.

This is one treatment run and one exact-matrix baseline, so the percentages describe this comparison rather than a guaranteed result for every PR. The dependency-layer cache saving is separate and not included as a demonstrated benefit here. Additional repetitions would establish consistency, but the measured result is already positive: faster preparation, less runner time, and a faster completed workflow.

@shahar1 shahar1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The serialization blocker from my first round is gone. I re-measured it, and your numbers in the comment above agree with mine. Since then the PR has also grown a second, independent optimization (the Dockerfile dependency layer), so this round covers both.

Snapshot: consumers no longer wait

Run 37226560360 (head c16d395b5f, AMD, 3.10):

  • Build CI linux/amd64 image 3.10 finished at 19:10:13; the first dependent job started at 19:10:14. That is a 1 s gap, against 3 min 56 s in the previous revision.
  • The snapshot steps cost the builder ~86 s: create 19:08:45 → 19:09:59, upload finalized 19:10:11 (2.52 GB).
  • Consumer image prep: download 72 s + unpack 48 s = 120 s, against stash 67 s + docker image load 106 s = 189 s in baseline run 37210552741.

Your run pair (127 s vs 210 s per consumer, −69.6 runner-minutes, 3 s builder-to-consumer gap) says the same thing. The −87 s wall-time figure compares two different PRs from one run each, so on a 51-minute workflow I would read it as "turnaround unchanged", not as a speed-up. The PR description now states this honestly ("no whole-workflow speedup is claimed"), thank you.

So the snapshot is a capacity win: roughly 70–90 runner-minutes per full AMD run, paid for with ~86 s of serial builder time and a second 2.5 GB artifact per Python version. That is a CI-budget call rather than a code question, so I would like @potiuk and/or @jscheffl to weigh in before this merges. The implementation itself I am happy with: the fallback is safe, failure handling brings back a running daemon, and test_docker_data_root_snapshot.py covers the interesting branches, including the "create after the cache publishers" ordering.

Dependency layer: please split it into its own PR

The dependency-manifests stage plus the --no-install-workspace preinstall is a separate change with a separate risk profile: it touches Dockerfile.ci, which every CI and local Breeze build uses, while the snapshot touches only PR workflows. It also cannot show its benefit in this PR. The layer can only be reused once main pushes it to the ghcr.io/.../cache-linux-amd64 registry cache. In this PR's build (job 111537879924) it missed, as expected: the preinstall took 28.7 s (789 packages) and the full sync took 19.3 s (only the 141 workspace packages). On a cache hit the builder therefore saves ~29 s. Unlike the snapshot, that saving is on the critical path, so it would win back part of the 86 s the snapshot costs.

That makes it worth having, but as its own PR with its own measurement: a PR that touches only sources, built after the layer has landed in the main cache. Then each change can be reviewed, reverted and backported on its own. The mechanism itself looks right to me: COPY --from keys on file contents, so the bind-mount stage re-running on every build does not invalidate it. The comment on install_airflow_when_building_images.sh ("usually incremental small set of packages") becomes true again. One inline comment on the || echo.

Nits

  • ci-image-snapshot-v2-...: there is no v1 of this artifact, so the -v2 suffix will puzzle the next reader (unless it avoids a name cached from an earlier revision of this branch).
  • test_ci_dependency_layer.py cuts the RUN bodies out of Dockerfile.ci by string splitting and runs them under bash. Two of its three tests check that find/cp failures propagate under errexit and that a || b exits 0, which is bash semantics rather than this change. If anyone reformats the Dockerfile, the tests break with an IndexError. The property worth testing is that a source-only change reuses the layer, and only a real build shows that. If this moves to its own PR, I would drop these tests and put the build measurement in the PR description.

This review was drafted by an AI-assisted tool and
confirmed by an Airflow maintainer. The findings
below are observations, not blockers; an Airflow
maintainer — a real person — will take the next look at the
PR. If you think a finding is mis-applied, please reply on
the PR and a maintainer will weigh in.

More on how Airflow handles maintainer review:
Contributing guide.

Comment thread .github/workflows/ci-image-build.yml Outdated
Comment thread .github/actions/prepare_breeze_and_image/action.yml Outdated
Comment thread .github/workflows/ci-image-build.yml Outdated
Comment thread Dockerfile.ci Outdated
zozo123 and others added 6 commits October 8, 2026 08:48
Every job that prepares the CI image runs `docker image load` on the stash,
which unpacks and checksums every layer of an 8 GB image again, about two
minutes per job. Extracting a copy of the image store into a stopped daemon
gives the same image in about half a minute on the same runners. The image
stash stays as it is, for other consumers and as the fallback when a
snapshot does not fit the daemon a job runs on.
Snapshot failures must not prevent the authoritative stash from loading, and an older branch snapshot must not substitute an image for the current checkout.
Raw daemon snapshots are job handoffs, so publishing them into branch caches unnecessarily exposes later runs to arbitrary checkout refs.
The snapshot script runs checkout code and prunes Docker state. Existing cache publications must finish before that optional execution.
The optional snapshot producer executes checkout code without publishing branch caches; its artifacts are available only to consumers in the same workflow run.
Snapshot producers must not accept arbitrary checkout refs in privileged workflow contexts, and a partially failed Docker stop must still trigger restart cleanup.
zozo123 and others added 6 commits October 8, 2026 08:48
The builder bind-mounts Docker from /mnt. Reading its image store and writing the compressed archive to that same disk competes for I/O; runner temp keeps the output on the root disk.
Ordinary source edits invalidate the dependency installation layer because it follows the full checkout copy. Reuse locked third-party dependencies while retaining the complete source installation and its resolution fallback.
Use portable path-preserving shell commands for the BuildKit manifest stage and cover nested paths containing spaces.\n\nCo-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Use the shell required by the manifest extraction stage so the BuildKit RUN command works with Docker's default shell.\n\nCo-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Restore the current UV and prek versions, propagate manifest discovery and copy failures, and cover failure injection in the extraction tests.\n\nCo-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@zozo123
zozo123 force-pushed the ci/ci-image-data-root-snapshot branch from f53486a to 3793fc0 Compare October 8, 2026 15:50
@zozo123 zozo123 changed the title Accelerate CI image materialization and dependency reuse Accelerate CI image materialization with same-run snapshots Oct 8, 2026
There has never been an earlier version of this artifact on main, so
the -v2 suffix left over from earlier branch revisions would puzzle the
next reader. Snapshot artifacts are scoped to the workflow run that
uploads them and kept for two days, so no consumer can see an artifact
under the old name and no compatibility shim is needed.

A test now checks that the upload and restore steps use the same
artifact name, so the two cannot drift apart silently.
A snapshot is only downloaded by consumer jobs of the run that uploaded
it. Once it expires, a re-run restores the image stash instead, which
keeps its two-day retention. One day halves the storage the snapshot
adds on top of the stash.
@zozo123

zozo123 commented Oct 9, 2026

Copy link
Copy Markdown
Contributor Author

@shahar1 ready for another look. Your round-2 points are addressed at 2fe6e1219d:

  • The dependency layer is split out into draft Reuse locked CI dependencies across workspace source changes #74466. It drops the || echo and the Bash-semantics tests.
  • The artifact is now ci-image-snapshot-v1-…, and a test checks that upload and restore use the same name.
  • New: the snapshot is kept for 1 day instead of 2. Only its own run reads it, and a re-run after it expires restores the 2-day stash instead. That halves the storage the snapshot adds.

@potiuk @jscheffl, shahar1 asked for your call on the CI budget before this merges. These figures are per built CI image (Python/platform) per PR run, from run 37804108397:

  • Cost: about 115 s of extra serial time in the builder, and one more 2.53 GB artifact (now kept 1 day) next to the existing 2.36 GB stash.
  • Saving: each of the 79 consumer jobs unpacks the snapshot in 38 s instead of running docker image load for 103–109 s. That is about 85–90 runner-minutes per AMD run. The download is the same size either way, and its time varies from run to run (45–211 s medians), so turnaround is unchanged.

Is that tradeoff acceptable?

CodeQL also reports one actions/cache-poisoning/poisonable-step alert on the snapshot step. It is a false positive: the step only runs on pull_request and writes no cache. The analysis and a reproduction are here. Someone with Security-tab access would need to dismiss it.


Drafted-by: Claude Code (Opus 5.5) (no human review before posting)

@shahar1 shahar1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All four round-3 points are fixed at 2fe6e121, and I found no further correctness issues in the script, the workflow wiring or the tests; what remains is not code: the CI-budget call that the PR body asks potiuk and jscheffl for, and dismissing CodeQL alert 670, which is the same actions/cache-poisoning/poisonable-step pattern as alerts 624–627 on this job's earlier steps that were already dismissed on main as false positives.

Smaller observations

  • .github/actions/prepare_breeze_and_image/action.yml — Heads-up, not a blocker: #74889 (mine) touches the same two files and, on self-hosted RunsOn runners, moves the image stash from GitHub artifacts to S3. The snapshot here always goes through actions/upload-artifact / download-artifact, so on that path it would trade an in-region S3 fetch for a 2.5 GB GitHub-artifact download per consumer. The script's fingerprint and DockerRootDir checks keep it safe there (it falls back to the stash), so the question is only whether it still pays off. Whichever PR lands second should either route the snapshot through the same stash action or limit it to GitHub-hosted runners; I am happy to do that on my side if this one lands first.
  • See inline comments on scripts/tests/ci/test_docker_data_root_snapshot.py:253.

This review was drafted by an AI-assisted tool and
confirmed by an Apache Airflow maintainer. The findings
below are observations, not blockers; an Apache Airflow
maintainer — a real person — will take the next look at the
PR. If you think a finding is mis-applied, please reply on
the PR and a maintainer will weigh in.

More on how Apache Airflow handles maintainer review:
Contributing guide.

Comment thread scripts/tests/ci/test_docker_data_root_snapshot.py Outdated
Co-authored-by: Yossi Eliaz <zozo123@users.noreply.github.com>
@zozo123

zozo123 commented Oct 17, 2026

Copy link
Copy Markdown
Contributor Author

@shahar1 thanks. Dropped that line in fe39ec5. On #74889, I'm glad to take you up on routing the snapshot through the stash action on your side if this lands first. @potiuk @jscheffl the only open items now are the CI budget call (figures in the PR description) and dismissing CodeQL alert 670.

@potiuk potiuk left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Restoring the Docker data-root instead of docker load is a nice idea. A few things need fixing first (inline), most importantly the dropped extended attributes, which silently change the image the tests run against.

Could you also share some measurements of the actual savings from real CI runs? For example: docker load time vs. snapshot restore time per consumer job, the extra time the builder spends on prune + create + upload, the snapshot size per image, and how often consumers actually accept the snapshot rather than falling back. Ideally from a few PR runs, so we can see the net effect across the ~30 consumer jobs against the extra upload and artifact storage.

One general point: the fast path only exists in prepare_breeze_and_image, while prepare_single_ci_image still loads from the stash. Putting the snapshot-or-stash decision into one shared loader would avoid making every later fix twice.


Drafted-by: Claude Code (Opus 5.5); reviewed by @potiuk before posting

Comment thread scripts/ci/docker_data_root_snapshot.sh Outdated
Comment thread scripts/ci/docker_data_root_snapshot.sh Outdated
Comment thread .github/workflows/ci-image-build.yml
Comment thread .github/workflows/ci-image-build.yml
Comment thread .github/actions/prepare_breeze_and_image/action.yml Outdated
Comment thread .github/actions/prepare_breeze_and_image/action.yml Outdated
Comment thread .github/actions/prepare_breeze_and_image/action.yml Outdated
Comment thread .github/actions/prepare_breeze_and_image/action.yml Outdated
Comment thread scripts/ci/docker_data_root_snapshot.sh
zozo123 and others added 2 commits October 18, 2026 20:03
…download

- Archive and extract xattrs in every namespace: GNU tar restores only
  user.* by default, which dropped overlay2's trusted.overlay.opaque and
  file capabilities, so a restored image could differ from the built one.
- Upload the snapshot with overwrite, plus a small metadata artifact that
  is uploaded even when the export fails; it reads "unavailable" until the
  archive is complete, so a rerun never restores an earlier attempt.
- Check the metadata against the checkout and daemon before downloading
  the snapshot, and bound extraction with a timeout that falls back to
  the stash after cleaning partial state.
- Load images for prepare_breeze_and_image and prepare_single_ci_image
  through one shared load_image action.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- Record the image ID and a hash of its merged file tree when creating the
  snapshot and compare both after restoring: a restore that lost an opaque
  directory marker keeps the image ID but changes the tree, so it now falls
  back to the stash instead of passing `bash -c true`.
- Restore through `breeze ci-image load --snapshot-file`, keeping breeze's
  environment checks, image check, diagnostics and rebuild marker.
- Bound the daemon stop and start as well as the extraction.
- Download the snapshot metadata by pattern, which adds no error annotation
  when the builder made no snapshot.
- Limit snapshots to GitHub-hosted runners, where self-hosted runners
  already restore the stash from their own storage.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…y the unavailable marker

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@zozo123

zozo123 commented Oct 18, 2026 •

Copy link
Copy Markdown
Contributor Author

Thanks for the review. Every point is addressed in the commits below, with details in the inline replies.

  • xattrs dropped on restore (7c367ee): create and extract both use --xattrs --xattrs-include='*', so trusted.overlay.opaque and security.capability survive.
  • bash -c true is too weak (2d3dec6, 6dabcd9): the meta records the image ID and a hash of the merged file tree. After a restore, both must match. If they don't, the store is wiped and the job loads the stash.
  • Stale snapshot on reruns (7c367ee): the snapshot and its meta are uploaded with overwrite: true. The meta reads unavailable until the archive is complete, and it is uploaded even when the export fails.
  • Second multi-GB artifact: measured below.
  • Big download before cheap checks (7c367ee): consumers first download the 362-byte meta and run check (revision, daemon fingerprint, driver, root, empty daemon). Only then do they download the snapshot.
  • Error annotations when there is no snapshot (2d3dec6): the meta is downloaded with pattern:, which adds no annotation when nothing matches. None of the 123 jobs I checked in run 39190862270 has an annotation.
  • No unpack timeout (7c367ee, 2d3dec6): composite steps don't accept timeout-minutes, so the script sets the limits itself: 60 s each for daemon stop and start, 300 s for zstd and tar, 60 s for the check after the restore. On a timeout it wipes the partial store and falls back to the stash.
  • Restore skips breeze's checks (2d3dec6, 7c367ee, 6da2d0f): the restore runs through breeze ci-image load --snapshot-file. Both prepare actions now share one load_image action. The builder now creates the snapshot through breeze ci-image save --snapshot-file as well, so both sides go through breeze.
  • containerd image store (08c9943): create exits before pruning on an unsupported store and publishes only the unavailable meta. Tests cover this.

The first version of the file-tree check was broken. On 08c9943 (run 39159493825), all 79 eligible consumers rejected the snapshot and loaded the stash. The run passed, but it saved nothing. The hash sorted the 265k-entry listing inside the container, and sort wrote temporary files into /tmp while find was still walking it, so the hash was different on every run. 6dabcd9 sorts on the host instead. A DinD check with the same script (Docker 28.5.2, overlay2) still rejects a restore made without --xattrs-include.

Measurements
These come from two full PR runs of the final design: 39190862270 on 6dabcd9 and 39199861265 on 6da2d0f. The baseline is 237 consumer jobs from three full-CI PR runs without the snapshot: 39141869252, 39143583838 and 39150120169. Durations are the composite-step duration_ms values from the job logs.

stash (baseline) snapshot, 39190862270 snapshot, 39199861265
acceptance – 79 / 79, 0 fallbacks 79 / 79, 0 fallbacks
download, median 53 s (2.38 GB) 86 s (2.55 GB) + 1–3 s meta and check 60 s (2.55 GB) + 1–3 s meta and check
load / unpack, median breeze ci-image load 103 s unpack via breeze 40 s unpack via breeze 47 s
total per consumer, median / p90 164 s / 210 s 135 s / 174 s 105 s / 141 s
extra builder time – 84 s 100 s
extra storage per PR run – 2.55 GB per Python × platform, 1-day retention same

The 26 PROD-image jobs in each run are not eligible.

Net

  • Per consumer job: ready 29–59 s sooner at the median and 36–69 s sooner at p90. Unpacking replaces a 103 s docker load with 40–47 s, but the snapshot download is slower than the stash download and gives back part of that. Download speed varies a lot between runs.
  • Per run: the builder finishes 84–100 s later, so every consumer starts that much later, and each consumer finishes about 40–55 s later in wall-clock time. Across 79 consumers, the run uses about 37–76 fewer runner-minutes after the builder's extra time.
  • So this saves runner time, not wall-clock time, and costs 2.55 GB of 1-day artifact storage per PR run. If that trade isn't worth it for the project, I'm fine closing this.

CI on 6da2d0f: all checks are green (135 passed, none failing), and CodeQL reports no alerts. It no longer reports the poisonable-step alert on the snapshot step: since 6da2d0f it calls breeze instead of running a repository script directly. The step runs the same checked-out code as the other breeze steps in this job. It has always been limited to pull_request events, so this is a design change, not a security fix.

Needs a maintainer: only a decision on the runner-time vs. wall-clock and storage trade above.

@shahar1: snapshots are limited to GitHub-hosted runners (runner.environment == 'github-hosted') on both the producer and the consumer side. Self-hosted runners keep loading the stash, so #74889 can land in either order.

zozo123 and others added 2 commits October 19, 2026 05:37
Sorting the CI image's 265k-entry listing inside the container spilled
to temporary files in /tmp while find was still walking it, so every run
listed different files and every consumer rejected the snapshot.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The builder now calls `breeze ci-image save --snapshot-file`, mirroring the
`breeze ci-image load --snapshot-file` that consumers restore with, so both
sides of the snapshot go through breeze and its environment checks.
@zozo123
zozo123 requested review from potiuk and shahar1 October 19, 2026 05:44

@shahar1 shahar1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@potiuk - WDYT?

This branch has not been deployed

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants