Skip to content

Commit

Permalink
build system: simplify docker image pinning
Browse files Browse the repository at this point in the history
It turns out that the ID mechanics of docker are even more crazy than
realized before: On Linux (x86_64) they use a different SHA256 when
referring to a locally installed image than when referring to the
same image at dockerhub. On Mac OS (Apple Silicon), the use the repo
SHA256 also when referring to the local image.

Instead of increasing the complexity of the current solution even more
by covering both cases, we now use
`docker.io/riot/riotbuild@sha256:<SHA256_OF_DOCKERHUB_IMAGE>` to refer
to a specific docker image, which hopefully works across systems.

Instead of pulling the image explicitly, we now can rely on docker
to do so automatically if the pinned image is not found locally. As
a result, the knob to disable automatic pulling has been dropped.

Fixes RIOT-OS#20853
  • Loading branch information
maribu committed Oct 9, 2024
1 parent 85172ed commit e960a19
Show file tree
Hide file tree
Showing 3 changed files with 7 additions and 39 deletions.
10 changes: 1 addition & 9 deletions dist/tools/buildsystem_sanity_check/check.sh
Original file line number Diff line number Diff line change
Expand Up @@ -382,20 +382,12 @@ check_tests_application_path() {
}

check_pinned_docker_version_is_up_to_date() {
local pinned_digest
local pinned_repo_digest
local upstream_digest
local upstream_repo_digest
pinned_digest="$(awk '/^DOCKER_TESTED_IMAGE_ID := (.*)$/ { print substr($0, index($0, $3)); exit }' "$RIOTMAKE/docker.inc.mk")"
pinned_repo_digest="$(awk '/^DOCKER_TESTED_IMAGE_REPO_DIGEST := (.*)$/ { print substr($0, index($0, $3)); exit }' "$RIOTMAKE/docker.inc.mk")"
# not using docker and jq here but a python script to not have to install
# more stuff for the static test docker image
IFS=' ' read -r upstream_digest upstream_repo_digest <<< "$("$RIOTTOOLS/buildsystem_sanity_check/get_dockerhub_digests.py" "riot/riotbuild")"

if [ "$pinned_digest" != "$upstream_digest" ]; then
git -C "${RIOTBASE}" grep -n '^DOCKER_TESTED_IMAGE_ID :=' "$RIOTMAKE/docker.inc.mk" \
| error_with_message "Update docker image SHA256 to ${upstream_digest}"
fi
IFS=' ' read -r upstream_repo_digest <<< "$("$RIOTTOOLS/buildsystem_sanity_check/get_dockerhub_digests.py" "riot/riotbuild")"

if [ "$pinned_repo_digest" != "$upstream_repo_digest" ]; then
git -C "${RIOTBASE}" grep -n '^DOCKER_TESTED_IMAGE_REPO_DIGEST :=' "$RIOTMAKE/docker.inc.mk" \
Expand Down
4 changes: 2 additions & 2 deletions dist/tools/buildsystem_sanity_check/get_dockerhub_digests.py
Original file line number Diff line number Diff line change
Expand Up @@ -87,5 +87,5 @@ def get_upstream_digests(repo, tag="latest", token=None):
if len(sys.argv) != 2:
sys.exit(f"Usage {sys.argv[0]} <REPO_NAME>")

digest, repo_digest = get_upstream_digests(sys.argv[1])
print(f"{digest} {repo_digest}")
_, repo_digest = get_upstream_digests(sys.argv[1])
print(f"{repo_digest}")
32 changes: 4 additions & 28 deletions makefiles/docker.inc.mk
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,10 @@
# When the docker image is updated, checks at
# dist/tools/buildsystem_sanity_check/check.sh start complaining in CI, and
# provide the latest values to verify and fill in.
DOCKER_TESTED_IMAGE_ID := 1329f419ec1a045a5830361f288536a56a0671a3b0db216e469369b00719cdff
DOCKER_TESTED_IMAGE_REPO_DIGEST := d5a70c06703731ddfebb98e9227eb03a69f02c393d9e89bbbcd65d71f3ef056e

DOCKER_PULL_IDENTIFIER := docker.io/riot/riotbuild@sha256:$(DOCKER_TESTED_IMAGE_REPO_DIGEST)
DOCKER_IMAGE_DEFAULT := sha256:$(DOCKER_TESTED_IMAGE_ID)
DOCKER_AUTO_PULL ?= 1
export DOCKER_IMAGE ?= $(DOCKER_IMAGE_DEFAULT)
export DOCKER_IMAGE ?= $(DOCKER_PULL_IDENTIFIER)
export DOCKER_BUILD_ROOT ?= /data/riotbuild
DOCKER_RIOTBASE ?= $(DOCKER_BUILD_ROOT)/riotbase

Expand Down Expand Up @@ -39,25 +36,6 @@ else
export INSIDE_DOCKER := 0
endif

ifeq (0:1,$(INSIDE_DOCKER):$(BUILD_IN_DOCKER))
ifeq ($(DOCKER_IMAGE),$(DOCKER_IMAGE_DEFAULT))
IMAGE_PRESENT:=$(shell $(DOCKER) image inspect $(DOCKER_IMAGE) 2>/dev/null >/dev/null && echo 1 || echo 0)
ifeq (0,$(IMAGE_PRESENT))
$(warning Required docker image $(DOCKER_IMAGE) not installed)
ifeq (1,$(DOCKER_AUTO_PULL))
$(info Pulling required image automatically. You can disable this with DOCKER_AUTO_PULL=0)
DEPS_FOR_RUNNING_DOCKER += docker-pull
else
$(info Building with latest available riotbuild image. You can pull the correct image automatically with DOCKER_AUTO_PULL=1)
# The currently set DOCKER_IMAGE is not locally available, and the
# user opted out to automatically pull it. Fall back to the
# latest (locally) available riot/riotbuild image instead.
export DOCKER_IMAGE := docker.io/riot/riotbuild:latest
endif
endif
endif
endif

# Default target for building inside a Docker container if nothing was given
export DOCKER_MAKECMDGOALS ?= all
# List of all exported environment variables that shall be passed on to the
Expand Down Expand Up @@ -164,6 +142,9 @@ DOCKER_USER ?= $$(id -u)
DOCKER_USER_OPT = $(if $(_docker_is_podman),--userns keep-id,--user $(DOCKER_USER))
DOCKER_RUN_FLAGS ?= --rm --tty $(DOCKER_USER_OPT)

# Explicitly set the platform to what the image is expecting
DOCKER_RUN_FLAGS += --platform linux/amd64

# allow setting make args from command line like '-j'
DOCKER_MAKE_ARGS ?=

Expand Down Expand Up @@ -378,11 +359,6 @@ docker_run_make = \
-w '$(DOCKER_APPDIR)' '$2' \
$(MAKE) $(DOCKER_OVERRIDE_CMDLINE) $4 $1

# This target pulls the docker image required for BUILD_IN_DOCKER
.PHONY: docker-pull
docker-pull:
$(DOCKER) pull '$(DOCKER_PULL_IDENTIFIER)'

# This will execute `make $(DOCKER_MAKECMDGOALS)` inside a Docker container.
# We do not push the regular $(MAKECMDGOALS) to the container's make command in
# order to only perform building inside the container and defer executing any
Expand Down

0 comments on commit e960a19

Please sign in to comment.