zozo123 commented on code in PR #74173: URL: https://github.com/apache/airflow/pull/74173#discussion_r4308439313
########## scripts/ci/docker_data_root_snapshot.sh: ########## @@ -0,0 +1,161 @@ +#!/usr/bin/env bash +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +# Moves the CI image between jobs as a copy of the Docker data directory that holds it, so a job +# restores it with one extraction instead of `docker image load` unpacking and checksumming every +# layer again. +# +# docker_data_root_snapshot.sh create SNAPSHOT_FILE +# Creates the snapshot after the builder publishes its caches. +# docker_data_root_snapshot.sh restore SNAPSHOT_FILE +# Restores the image. Incompatible snapshots leave the daemon untouched. Failed +# materialization cleans partial image state and restarts Docker for the normal stash path. +set -euo pipefail + +DATA_ROOT="/var/lib/docker" +DAEMON_STOPPED=false +RESTORE_IN_PROGRESS=false +RESTORE_FILE="" + +function cleanup_daemon() { + local result=$? + trap - EXIT + if [[ -n "${RESTORE_FILE}" ]]; then + rm -f "${RESTORE_FILE}" "${RESTORE_FILE}.meta" + fi + if [[ "${RESTORE_IN_PROGRESS}" == true ]]; then + stop_daemon + remove_image_store + start_daemon + elif [[ "${DAEMON_STOPPED}" == true ]]; then + start_daemon + fi + exit "${result}" +} +trap cleanup_daemon EXIT + +function check_supported_daemon() { + local daemon=$1 + local _ driver root + read -r _ driver _ root <<< "${daemon}" + if [[ "${driver}" != overlay2 || "${root}" != "${DATA_ROOT}" ]]; then + echo "::warning::Unsupported Docker image store; skipping snapshot: ${daemon}" + exit 3 + fi +} + +# A data directory is only readable by the daemon version and storage driver that wrote it. +function daemon_fingerprint() { + docker info --format '{{.ServerVersion}} {{.Driver}} {{.Architecture}} {{.DockerRootDir}}' +} + +function stop_daemon() { + DAEMON_STOPPED=true + sudo systemctl stop docker.socket docker +} + +function start_daemon() { + sudo systemctl start docker + DAEMON_STOPPED=false +} + +function remove_image_store() { + sudo rm -rf "${DATA_ROOT}/image" "${DATA_ROOT}/overlay2" +} + +function create_snapshot() { + local snapshot_file="${1}" + local daemon image_id + daemon="$(daemon_fingerprint)" + check_supported_daemon "${daemon}" + 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" + stop_daemon + sudo tar --directory "${DATA_ROOT}" --xattrs --acls --numeric-owner \ + --create --file - image overlay2 \ + | zstd -3 -T0 --quiet --force -o "${snapshot_file}" + start_daemon +} + +function restore_snapshot() { + local snapshot_file="${1}" + RESTORE_FILE="${snapshot_file}" + local -a meta + local image_id snapshot_fingerprint daemon existing_images existing_containers + if [[ ! -f "${snapshot_file}" || ! -f "${snapshot_file}.meta" ]]; then + echo "No snapshot at ${snapshot_file}" + exit 2 + fi + read -r -a meta < "${snapshot_file}.meta" + if [[ ${#meta[@]} != 6 ]]; then + echo "Invalid snapshot metadata" + exit 3 + fi + if [[ "${meta[5]}" != "$(git rev-parse HEAD)" ]]; then + echo "Snapshot belongs to a different checkout revision" + exit 3 + fi + image_id="${meta[4]}" + snapshot_fingerprint="${meta[*]:0:4}" + daemon="$(daemon_fingerprint)" + check_supported_daemon "${daemon}" + if [[ "${snapshot_fingerprint}" != "${daemon}" ]]; then + echo "::warning::Snapshot daemon fingerprint mismatch: '${snapshot_fingerprint}' != '${daemon}'" + exit 3 + fi + existing_images="$(docker images --all --quiet)" + existing_containers="$(docker ps --all --quiet)" + if [[ -n "${existing_images}" || -n "${existing_containers}" ]]; then + echo "The daemon already holds images, which restoring the snapshot would drop" + exit 3 + fi + stop_daemon + RESTORE_IN_PROGRESS=true + remove_image_store + zstd -d -T0 --quiet --stdout "${snapshot_file}" \ + | sudo tar --directory "${DATA_ROOT}" --xattrs --acls --numeric-owner --extract --file - Review Comment: Fixed in 7c367ee0e2: create and extract now both use `--xattrs --xattrs-include='*'`. I reproduced the problem with GNU tar 1.34 (ubuntu:22.04). Without the include, `trusted.overlay.opaque` and `security.capability` are lost while `user.*` survives; with it, all three come back. Tests assert the flag on both sides. ########## scripts/ci/docker_data_root_snapshot.sh: ########## @@ -0,0 +1,161 @@ +#!/usr/bin/env bash +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +# Moves the CI image between jobs as a copy of the Docker data directory that holds it, so a job +# restores it with one extraction instead of `docker image load` unpacking and checksumming every +# layer again. +# +# docker_data_root_snapshot.sh create SNAPSHOT_FILE +# Creates the snapshot after the builder publishes its caches. +# docker_data_root_snapshot.sh restore SNAPSHOT_FILE +# Restores the image. Incompatible snapshots leave the daemon untouched. Failed +# materialization cleans partial image state and restarts Docker for the normal stash path. +set -euo pipefail + +DATA_ROOT="/var/lib/docker" +DAEMON_STOPPED=false +RESTORE_IN_PROGRESS=false +RESTORE_FILE="" + +function cleanup_daemon() { + local result=$? + trap - EXIT + if [[ -n "${RESTORE_FILE}" ]]; then + rm -f "${RESTORE_FILE}" "${RESTORE_FILE}.meta" + fi + if [[ "${RESTORE_IN_PROGRESS}" == true ]]; then + stop_daemon + remove_image_store + start_daemon + elif [[ "${DAEMON_STOPPED}" == true ]]; then + start_daemon + fi + exit "${result}" +} +trap cleanup_daemon EXIT + +function check_supported_daemon() { + local daemon=$1 + local _ driver root + read -r _ driver _ root <<< "${daemon}" + if [[ "${driver}" != overlay2 || "${root}" != "${DATA_ROOT}" ]]; then + echo "::warning::Unsupported Docker image store; skipping snapshot: ${daemon}" + exit 3 + fi +} + +# A data directory is only readable by the daemon version and storage driver that wrote it. +function daemon_fingerprint() { + docker info --format '{{.ServerVersion}} {{.Driver}} {{.Architecture}} {{.DockerRootDir}}' +} + +function stop_daemon() { + DAEMON_STOPPED=true + sudo systemctl stop docker.socket docker +} + +function start_daemon() { + sudo systemctl start docker + DAEMON_STOPPED=false +} + +function remove_image_store() { + sudo rm -rf "${DATA_ROOT}/image" "${DATA_ROOT}/overlay2" +} + +function create_snapshot() { + local snapshot_file="${1}" + local daemon image_id + daemon="$(daemon_fingerprint)" + check_supported_daemon "${daemon}" + 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" + stop_daemon + sudo tar --directory "${DATA_ROOT}" --xattrs --acls --numeric-owner \ + --create --file - image overlay2 \ + | zstd -3 -T0 --quiet --force -o "${snapshot_file}" + start_daemon +} + +function restore_snapshot() { + local snapshot_file="${1}" + RESTORE_FILE="${snapshot_file}" + local -a meta + local image_id snapshot_fingerprint daemon existing_images existing_containers + if [[ ! -f "${snapshot_file}" || ! -f "${snapshot_file}.meta" ]]; then + echo "No snapshot at ${snapshot_file}" + exit 2 + fi + read -r -a meta < "${snapshot_file}.meta" + if [[ ${#meta[@]} != 6 ]]; then + echo "Invalid snapshot metadata" + exit 3 + fi + if [[ "${meta[5]}" != "$(git rev-parse HEAD)" ]]; then + echo "Snapshot belongs to a different checkout revision" + exit 3 + fi + image_id="${meta[4]}" + snapshot_fingerprint="${meta[*]:0:4}" + daemon="$(daemon_fingerprint)" + check_supported_daemon "${daemon}" + if [[ "${snapshot_fingerprint}" != "${daemon}" ]]; then + echo "::warning::Snapshot daemon fingerprint mismatch: '${snapshot_fingerprint}' != '${daemon}'" + exit 3 + fi + existing_images="$(docker images --all --quiet)" + existing_containers="$(docker ps --all --quiet)" + if [[ -n "${existing_images}" || -n "${existing_containers}" ]]; then + echo "The daemon already holds images, which restoring the snapshot would drop" + exit 3 + fi + stop_daemon + RESTORE_IN_PROGRESS=true + remove_image_store + zstd -d -T0 --quiet --stdout "${snapshot_file}" \ + | sudo tar --directory "${DATA_ROOT}" --xattrs --acls --numeric-owner --extract --file - + start_daemon + if ! docker run --rm --entrypoint /bin/bash "${image_id}" -c true; then Review Comment: Done in 2d3dec6b5c. - When the snapshot is created, the meta now also records the image ID from `docker image inspect` and a hash of the image's merged file tree: `find / -xdev -printf '%p %y %m'`, sorted, then sha256. - After a restore, both are recomputed and must match. If they don't, the store is wiped and the job falls back to the stash. - I checked this end to end on Docker 28.5.2 with overlay2 in DinD. The test image has a later layer that deletes and recreates a directory and deletes a file. - Restoring without `--xattrs-include` keeps the same image ID but changes the tree hash. The restore is rejected (exit 4) and leaves an empty daemon behind. With the fix, the restore passes and the deleted paths stay deleted. ########## .github/workflows/ci-image-build.yml: ########## @@ -401,3 +401,35 @@ 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. + - 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: | + ./scripts/ci/docker_data_root_snapshot.sh create "${SNAPSHOT_FILE}" + 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-v1-${{ 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' Review Comment: Added `overwrite: true`. In addition: - The `.meta` is now its own artifact. It is uploaded whenever the export step ran, also with `overwrite: true`. - `create` writes `unavailable` into it before it starts. So a re-run whose export fails also retires attempt 1's snapshot instead of leaving it in place. ########## .github/workflows/ci-image-build.yml: ########## @@ -401,3 +401,35 @@ 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. + - 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: | + ./scripts/ci/docker_data_root_snapshot.sh create "${SNAPSHOT_FILE}" + 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 }}" Review Comment: Measured in the PR comment: 2.55 GB per Python × platform per PR run, kept for 1 day, on top of the 2.38 GB stash. ########## .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 Review Comment: Done. - The `.meta` is a separate artifact. - Consumers download it first and run `docker_data_root_snapshot.sh check`, which looks at the revision, the daemon fingerprint, the driver and root, and whether the daemon is empty. - They download the snapshot only if the check passes. ########## .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-v1-${{ 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 == '' Review Comment: The metadata download now uses `pattern:` instead of `name:`. download-artifact only throws "Artifact not found" for `name`; a pattern that matches nothing downloads 0 artifacts and adds no annotation. The large download only runs after the check passes. This avoids threading a builder output through every consumer workflow. ########## .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-v1-${{ 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 Review Comment: Composite actions don't accept `timeout-minutes`: the runner's action schema allows only `continue-on-error`, `env`, `id`, `if`, `name`, `run`, `shell` and `working-directory` on composite run steps. So the bounds live inside the script: - `systemctl stop` and `systemctl start`: 60 s each - `zstd` and `tar`: 300 s - the check after the restore: 60 s In the worst case the step gives up after about 10 minutes. Any timeout wipes the partial store and restarts Docker, and then the stash path runs. -- 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]
