Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 20 additions & 12 deletions buildroot-external/package/hassio/create-data-partition.sh
Original file line number Diff line number Diff line change
Expand Up @@ -7,30 +7,38 @@ channel=$3
docker_version=$4

data_img="${dst_dir}/data.ext4"
data_dir="${build_dir}/data"

APPARMOR_URL="https://version.home-assistant.io/apparmor.txt"

# Make image
rm -f "${data_img}"
truncate --size="1280M" "${data_img}"
mkfs.ext4 -L "hassos-data" -E lazy_itable_init=0,lazy_journal_init=0 "${data_img}"

# Mount / init file structs
mkdir -p "${build_dir}/data/"
sudo mount -o loop,discard "${data_img}" "${build_dir}/data/"
mkdir -p "${data_dir}"
sudo mount -o loop,discard "${data_img}" "${data_dir}"

trap 'docker rm -f ${container} > /dev/null; sudo umount ${data_dir} || true' ERR EXIT
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Critical: Trap uses single quotes, preventing variable expansion—variables won't be cleaned up.

The trap on line 23 uses single quotes, which prevent bash variable expansion. When the trap executes, it will try to run literal commands like docker rm -f ${container} (not expanded), which fails silently. This defeats the PR's goal of ensuring cleanup on failure.

The earlier review discussion already identified this issue and suggested using a cleanup function where variables are evaluated at execution time.

Apply this fix:

# Cleanup function to ensure resources are freed
cleanup() {
    if [[ -n "${container:-}" ]]; then
        docker rm -f "$container"
    fi
    sudo umount "${data_dir}"
}

# Set trap early to ensure cleanup on any failure
trap cleanup ERR EXIT

# Use official Docker in Docker images
# We use the same version as Buildroot is using to ensure best compatibility
container=$(docker run --privileged -e DOCKER_TLS_CERTDIR="" \
    -v "${data_dir}":/mnt/data \
    -v "${build_dir}":/build \
    -d "docker:${docker_version}-dind" --feature containerd-snapshotter --data-root /mnt/data/docker)

This ensures:

  • The trap is set before docker run, so cleanup runs even if the command fails
  • Variables are evaluated when cleanup() executes, not when the trap is set
  • If docker run fails, $container will be empty and only umount runs
  • If it succeeds, both container removal and umount run

Also applies to: 27-30

🤖 Prompt for AI Agents
In buildroot-external/package/hassio/create-data-partition.sh around lines 23
and 27-30, the trap currently uses single quotes so variables are not expanded
at execution time causing cleanup to fail; replace the inline single-quoted trap
with a cleanup() function defined before the docker run that checks if container
is non-empty and removes it (docker rm -f "$container") and always attempts sudo
umount "$data_dir", then set trap to call cleanup on ERR and EXIT (trap cleanup
ERR EXIT) before running docker so variables are evaluated when cleanup runs;
ensure variable expansions are quoted and the trap is set early enough to catch
docker run failures.


# Use official Docker in Docker images
# We use the same version as Buildroot is using to ensure best compatibility
container=$(docker run --privileged -e DOCKER_TLS_CERTDIR="" \
-v "${build_dir}/data/":/data \
-v "${build_dir}/data/docker/":/var/lib/docker \
-v "${build_dir}":/build \
-d "docker:${docker_version}-dind" --feature containerd-snapshotter)
-v "${data_dir}":/mnt/data \
-v "${build_dir}":/build \
-d "docker:${docker_version}-dind" --feature containerd-snapshotter --data-root /mnt/data/docker)

docker exec "${container}" sh /build/dind-import-containers.sh "${channel}"

docker stop "${container}"
docker exec "${container}" sh /build/dind-import-containers.sh

sudo bash -ex <<EOF
# Indicator for docker-prepare.service to use the containerd snapshotter
sudo touch "${build_dir}/data/.docker-use-containerd-snapshotter"
touch "${data_dir}/.docker-use-containerd-snapshotter"

# Setup AppArmor
mkdir -p "${data_dir}/supervisor/apparmor"
curl -fsL -o "${data_dir}/supervisor/apparmor/hassio-supervisor" "${APPARMOR_URL}"

# Unmount data image
sudo umount "${build_dir}/data/"
# Persist build-time updater channel
jq -n --arg channel "${channel}" '{"channel": \$channel}' > "${data_dir}/supervisor/updater.json"
EOF
10 changes: 0 additions & 10 deletions buildroot-external/package/hassio/dind-import-containers.sh
Original file line number Diff line number Diff line change
@@ -1,10 +1,6 @@
#!/bin/sh
set -e

channel=$1

APPARMOR_URL="https://version.home-assistant.io/apparmor.txt"

# Make sure we can talk to the Docker daemon
echo "Waiting for Docker daemon..."
while ! docker version 2> /dev/null > /dev/null; do
Expand All @@ -25,9 +21,3 @@ done
supervisor=$(docker images --filter "label=io.hass.type=supervisor" --quiet)
arch=$(docker inspect --format '{{ index .Config.Labels "io.hass.arch" }}' "${supervisor}")
docker tag "${supervisor}" "ghcr.io/home-assistant/${arch}-hassio-supervisor:latest"

# Setup AppArmor
mkdir -p "/data/supervisor/apparmor"
wget -O "/data/supervisor/apparmor/hassio-supervisor" "${APPARMOR_URL}"

echo "{ \"channel\": \"${channel}\" }" > /data/supervisor/updater.json