zozo123 commented on code in PR #74173:
URL: https://github.com/apache/airflow/pull/74173#discussion_r4221175135
##########
.github/workflows/ci-image-build.yml:
##########
@@ -401,3 +401,70 @@ jobs:
steps.stashed-image.outputs.reusable != 'true'
- name: "Check disk space after build"
run: df -H
+
+ # Snapshot only after cache publication: pruning removes the mount-cache
image.
+ # Keep creation in trusted workflow commands; the configurable checkout
is untrusted.
Review Comment:
Agreed: the builder already runs PR-authored Breeze code, so the inline
workflow commands did not provide an extra isolation boundary. Removed that
claim and moved creation into a `create` subcommand in
`docker_data_root_snapshot.sh`, sharing the fingerprint check, stop/start
helpers and EXIT recovery with restore. The workflow now calls the script after
cache publication, and the tests invoke both subcommands directly rather than
extracting the YAML run block. All 24 snapshot tests pass; the full scripts
suite on the rebased branch passed with 1,696 tests and one skip. Pushed in
head `3793fc0ad560`.
---
Drafted-by: Codex (GPT-6) (no human review before posting)
##########
.github/actions/prepare_breeze_and_image/action.yml:
##########
@@ -79,6 +79,33 @@ runs:
run: |
echo "Checking free space!"
df -H
+ - name: "Restore CI image snapshot ${{ inputs.platform }}:${{
inputs.python }}"
+ continue-on-error: true
+ uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c
# v8.0.1
+ with:
+ name: >-
+ ci-image-snapshot-v2-${{ inputs.python }}-${{
+ inputs.platform == 'linux/amd64' && 'amd64' || 'arm64' }}
+ path: "/mnt/"
+ id: restore-snapshot
+ if: >
+ github.event_name == 'pull_request' &&
+ inputs.image-type == 'ci' && inputs.image-stash-ref == ''
+ # Falls back to the image stash below whenever the snapshot does not fit
this runner's daemon.
+ - name: "Unpack CI image snapshot ${{ inputs.platform }}:${{ inputs.python
}}"
+ id: snapshot
+ env:
+ PLATFORM: ${{ inputs.platform }}
+ PYTHON: ${{ inputs.python }}
+ run: |
+ if ./scripts/ci/docker_data_root_snapshot.sh restore \
+ "/mnt/ci-image-snapshot-${PLATFORM//\//_}-${PYTHON}.tar.zst"; then
+ echo "restored=true" >> "${GITHUB_OUTPUT}"
+ else
+ echo "Falling back to loading the image stash"
Review Comment:
Fixed in head `3793fc0ad560`: restore registers the exact archive and
`.meta` paths with its EXIT cleanup before checking whether either file exists.
Every restore exit now removes both files, including missing/malformed
metadata, stale checkout, fingerprint/storage/inventory rejection, extraction
failure and failed image startup. This frees `/mnt` before the stash fallback
starts. The regression harness asserts archive and metadata removal for every
restore invocation, covering success and all tested rejection/recovery paths;
24 tests pass.
---
Drafted-by: Codex (GPT-6) (no human review before posting)
##########
.github/workflows/ci-image-build.yml:
##########
@@ -401,3 +401,70 @@ jobs:
steps.stashed-image.outputs.reusable != 'true'
- name: "Check disk space after build"
run: df -H
+
+ # Snapshot only after cache publication: pruning removes the mount-cache
image.
+ # Keep creation in trusted workflow commands; the configurable checkout
is untrusted.
+ - name: "Snapshot CI image ${{ inputs.platform }}:${{
env.PYTHON_MAJOR_MINOR_VERSION }}"
+ id: snapshot-export
+ continue-on-error: true
+ timeout-minutes: 5
+ env:
+ # Docker is bind-mounted from /mnt on AMD; write the archive on the
root disk.
+ SNAPSHOT_FILE: >-
+ ${{ runner.temp }}/ci-image-snapshot-${{
+ inputs.platform == 'linux/amd64' && 'linux_amd64' || 'linux_arm64'
+ }}-${{ env.PYTHON_MAJOR_MINOR_VERSION }}.tar.zst
+ run: |
+ daemon_stopped=false
+ function restart_docker_on_exit() {
+ local result=$?
+ trap - EXIT
+ if [[ "${daemon_stopped}" == true ]]; then
+ sudo systemctl start docker
+ fi
+ exit "${result}"
+ }
+ trap restart_docker_on_exit EXIT
+ daemon="$(docker info --format \
+ '{{.ServerVersion}} {{.Driver}} {{.Architecture}}
{{.DockerRootDir}}')"
+ read -r _ driver _ root <<< "${daemon}"
+ if [[ "${driver}" != overlay2 || "${root}" != /var/lib/docker ]];
then
+ echo "::warning::Unsupported Docker image store; skipping snapshot
creation: ${daemon}"
+ exit 3
+ fi
+ image_id="$(docker images --quiet --filter
'label=org.apache.airflow.image=airflow-ci' | sort -u)"
+ if [[ -z "${image_id}" || "${image_id}" == *$'\n'* ]]; then
+ echo "Expected exactly one CI image in the daemon, found:
'${image_id}'" >&2
+ exit 1
+ fi
+ docker ps --all --quiet | xargs --no-run-if-empty docker rm --force
>/dev/null
+ docker images --quiet | sort -u | {
+ grep --invert-match --fixed-strings --line-regexp "${image_id}" ||
true
+ } | xargs --no-run-if-empty docker rmi --force >/dev/null
+ docker builder prune --all --force >/dev/null
+ printf '%s %s %s\n' "${daemon}" "${image_id}" "$(git rev-parse
HEAD)" \
+ > "${SNAPSHOT_FILE}.meta"
+ daemon_stopped=true
+ sudo systemctl stop docker.socket docker
+ sudo tar --directory /var/lib/docker --xattrs --acls --numeric-owner
\
+ --create --file - image overlay2 \
+ | zstd -3 -T0 --quiet --force -o "${SNAPSHOT_FILE}"
+ sudo systemctl start docker
+ daemon_stopped=false
+ shell: bash
+ if: >
+ github.event_name == 'pull_request' &&
+ inputs.upload-image-artifact == 'true' && inputs.image-stash-ref ==
'' &&
+ (inputs.checkout-ref == '' || inputs.checkout-ref == github.sha)
+ - name: "Upload CI image snapshot ${{ inputs.platform }}:${{
env.PYTHON_MAJOR_MINOR_VERSION }}"
+ continue-on-error: true
+ uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a
# v7.0.1
+ with:
+ name: >-
+ ci-image-snapshot-v2-${{ env.PYTHON_MAJOR_MINOR_VERSION }}-${{
+ inputs.platform == 'linux/amd64' && 'amd64' || 'arm64' }}
+ path: "${{ runner.temp }}/ci-image-snapshot-*-${{
env.PYTHON_MAJOR_MINOR_VERSION }}.tar.zst*"
+ if-no-files-found: 'error'
+ compression-level: '0'
+ retention-days: '2'
Review Comment:
Agreed. The PR description now explicitly records 2.52 GB for the snapshot
plus 2.35 GB for the fallback stash: approximately 4.87 GB per built CI image /
Python-platform combination per run, with two-day retention for both
transports. Matrix size and overlapping retained runs multiply that amount; it
is not a 4.9 GB cap for the whole workflow. The description treats this as a
CI-capacity/artifact-quota tradeoff requiring maintainer acceptance before
merging, and claims no whole-workflow speedup. I have not assumed ASF budget
approval.
---
Drafted-by: Codex (GPT-6) (no human review before posting)
##########
Dockerfile.ci:
##########
@@ -1966,13 +1982,24 @@ RUN bash /scripts/docker/install_packaging_tools.sh
COPY --from=scripts install_airflow_when_building_images.sh /scripts/docker/
+COPY --from=dependency-manifests /dependency-manifests/ ${AIRFLOW_SOURCES}/
+ARG UPGRADE_RANDOM_INDICATOR_STRING=""
+# Only preinstall locked third-party dependencies. The full sync after COPY
still installs
+# workspace packages from the current checkout and retains its existing
resolution fallback.
+RUN
--mount=type=cache,id=ci-$TARGETARCH-$DEPENDENCY_CACHE_EPOCH,target=/root/.cache/
\
+ if [[ -z "${UPGRADE_RANDOM_INDICATOR_STRING}" ]]; then \
+ uv sync --all-packages --frozen --group ci-image
--no-install-workspace \
+ --no-binary-package lxml --no-binary-package xmlsec \
+ --no-python-downloads --no-managed-python || \
+ echo "Dependency preinstallation failed; deferring to the full
source installation"; \
Review Comment:
Agreed: a failed preinstall must not become a successful cached partial
layer. Removed the `|| echo` so installation failure now fails the build. Per
your review's split request, the entire dependency-layer change is removed from
#74173 and moved to draft #74466, which contains this correction. The
Dockerfile string-splitting/Bash-semantics tests were also dropped. That draft
records your earlier 28.7-second preinstall measurement and explicitly leaves
full-image equivalence and registry cache-hit performance unverified pending
separate validation. Regular and manual Dockerfile checks pass.
---
Drafted-by: Codex (GPT-6) (no human review before posting)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
