From e960a19f24426a7355aefbdc6b89e5f4fb89e4a3 Mon Sep 17 00:00:00 2001 From: Marian Buschsieweke Date: Sun, 29 Sep 2024 16:22:05 +0200 Subject: [PATCH] build system: simplify docker image pinning 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:` 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 https://github.com/RIOT-OS/RIOT/issues/20853 --- dist/tools/buildsystem_sanity_check/check.sh | 10 +----- .../get_dockerhub_digests.py | 4 +-- makefiles/docker.inc.mk | 32 +++---------------- 3 files changed, 7 insertions(+), 39 deletions(-) diff --git a/dist/tools/buildsystem_sanity_check/check.sh b/dist/tools/buildsystem_sanity_check/check.sh index 5a74658f1fa8..90de6e90d280 100755 --- a/dist/tools/buildsystem_sanity_check/check.sh +++ b/dist/tools/buildsystem_sanity_check/check.sh @@ -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" \ diff --git a/dist/tools/buildsystem_sanity_check/get_dockerhub_digests.py b/dist/tools/buildsystem_sanity_check/get_dockerhub_digests.py index 4ac75a191044..68aec7e49601 100755 --- a/dist/tools/buildsystem_sanity_check/get_dockerhub_digests.py +++ b/dist/tools/buildsystem_sanity_check/get_dockerhub_digests.py @@ -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]} ") - 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}") diff --git a/makefiles/docker.inc.mk b/makefiles/docker.inc.mk index c7414023996c..d4fe213ce22d 100644 --- a/makefiles/docker.inc.mk +++ b/makefiles/docker.inc.mk @@ -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 @@ -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 @@ -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 ?= @@ -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