packaging: refactor docker image build #2566

AlexanderYastrebov · 2023-09-01T15:32:27Z

Makefile changes

Build local architecture binaries into build/ folder and nest other architecture-dependent binaries into build/ subfolders (e.g. build/linux/amd64).

Add help and make it a default target.

Cleanup tar packages.

Remove unused variables.

Dockerfile changes

Remove BUILD_FOLDER build arg, see Makefile changes.

Use COPY instead of ADD following https://docs.docker.com/develop/develop-images/dockerfile_best-practices/#add-or-copy

Remove redundant commands.

Use one Docker file for all architectures.

Use alpine:latest in Dockerfile and configure trusted base images for amd64 and multiarch builds in Makefile.

Testing

Using https://github.com/multiarch/qemu-user-static it is possible to test that all make targets produce images and binaries for proper architectures, see make test

Links

AlexanderYastrebov · 2023-09-01T15:37:41Z

@linki you did a great job adding multiarch build within #2019 - could you have a look maybe and share your opinion?

AlexanderYastrebov · 2023-09-01T15:40:32Z

packaging/Makefile

 VERSION            ?= $(shell git rev-parse HEAD)
 REGISTRY           ?= registry-write.opensource.zalan.do/teapot
 BINARIES           ?= skipper webhook eskip routesrv
 IMAGE              ?= $(REGISTRY)/skipper:$(VERSION)
 ARM64_IMAGE        ?= $(REGISTRY)/skipper-arm64:$(VERSION)
 ARM_IMAGE          ?= $(REGISTRY)/skipper-armv7:$(VERSION)
-PACKAGE            ?= github.com/zalando/skipper
+MULTIARCH_IMAGE    ?= container-registry-test.zalando.net/teapot/skipper:$(VERSION)


Added this for consistency with other *_IMAGE variables.

RomanZavodskikh · 2023-09-01T16:29:37Z

👍

szuecs · 2023-09-01T18:01:09Z

packaging/Dockerfile

@@ -1,16 +1,18 @@
 ARG BASE_IMAGE=default
-FROM registry.opensource.zalan.do/library/alpine-3:latest AS default
+FROM alpine:latest AS default


Why this change?

I thought it would be more consistent:

we override multiarch base image via build arg

this Docker file by itself now depends on the stock alpine

otherwise we would need to override base image for arm64 and armv7 (non-multiarch)

If you switch the base image from a single-arch to a multi-arch it should just work (if the requested arch is supported, but alpine supports all we need). So you should be able to use alpine:latest for "arm64 and armv7 (non-multiarch)" as well. (or the equivalent Zalando variant container-registry.zalando.net/library/alpine-3:latest, note that this is not a public image)

linki · 2023-09-08T16:05:22Z

packaging/Makefile

+.PHONY: docker.build.amd64
+docker.build.amd64: clean build.linux.amd64 docker.build.enable ## build linux/amd64 image using a trusted amd64 alpine base image
+	docker buildx build -t $(IMAGE) --platform linux/amd64 --load . \
+		--build-arg BASE_IMAGE=registry.opensource.zalan.do/library/alpine-3:latest


Why use a custom base image here? Is it for compliance with Zalando? The default alpine:latest would also work.

container-registry.zalando.net/library/alpine-3:latest would also work as it's just the compliant version of alpine:latest (more correctly alpine:3).

Is it for compliance with Zalando?

Yes, so the idea is that Docker file uses opensource image and Makefile configured base image we need for us

Then best use alpine:latest (or with a version) by default for open source and overwrite it in Makefile with container-registry.zalando.net/library/alpine-3:latest (for us).

Latest will break OpenSSF Scorecard because they want sha based image versions. I think it's also better being explicit of versions and have automations that change it. In the future we can likely merge these pr automatically as discussed in an internal meeting.

linki · 2023-09-08T16:07:39Z

packaging/Makefile

 	docker buildx build --rm -t $(MULTIARCH_IMAGE) --platform linux/amd64,linux/arm64 --push \
-	  --build-arg BASE_IMAGE=container-registry.zalando.net/library/alpine-3:latest .
+		--build-arg BASE_IMAGE=container-registry.zalando.net/library/alpine-3:latest .


Similar to https://github.com/zalando/skipper/pull/2566/files#r1320057345. This is the right approach for targeting Zalando clusters but for the open source images (do you still publish to public registries?) alpine:latest would also work here.

But like described above, it's perfectly fine to use a multi-arch base image (either alpine:latest or container-registry.zalando.net/library/alpine-3:latest) to build a single-arch image (i.e. --platform linux/amd64 or --platform linux/arm64).

I'm assuming you're using docker buildx build. For the places where you're still using docker build (if any, I didn't check) the outcome is different. Better to stick with buildx everywhere.

linki · 2023-09-08T16:26:45Z

packaging/Makefile

+	IMAGE=skipper-packaging-test:arm64 TARGETPLATFORM=linux/arm64 make test-run
+
+	ARM_IMAGE=skipper-packaging-test:armv7 make docker.build.armv7
+	IMAGE=skipper-packaging-test:armv7 TARGETPLATFORM=linux/arm/v7 make test-run


Should/Can we also test the multi-arch image? Maybe even by running it on different (emulated) archs?

I omitted multiarch here because it is not possible to build it without pushing docker/buildx#59. I guess to test it we'd need to push it first.

Makefile changes == Build local architecture binaries into build/ folder and nest other architecture-dependent binaries into build/<arch> subfolders (e.g. build/linux/amd64). Add help and make it a default target. Cleanup tar packages. Remove unused variables. Dockerfile changes == Remove BUILD_FOLDER build arg, see Makefile changes. Use COPY instead of ADD following https://docs.docker.com/develop/develop-images/dockerfile_best-practices/#add-or-copy Remove redundant commands. Use one Docker file for all architectures. Use alpine:latest in Dockerfile and configure trusted base images for amd64 and multiarch builds in Makefile. Testing == Using https://github.com/multiarch/qemu-user-static it is possible to test that all make targets produce images and binaries for proper architectures, see `make test` Links == * https://www.docker.com/blog/multi-arch-images/ * https://www.thorsten-hans.com/how-to-build-multi-arch-docker-images-with-ease/ * https://www.stereolabs.com/docs/docker/building-arm-container-on-x86/ * https://github.com/multiarch/qemu-user-static * https://stackoverflow.com/questions/72444103/what-does-running-the-multiarch-qemu-user-static-does-before-building-a-containe Signed-off-by: Alexander Yastrebov <[email protected]>

szuecs · 2023-12-22T10:27:50Z

👎

Add [opencontainer image labels](https://github.com/opencontainers/image-spec/blob/main/annotations.md) (except org.opencontainers.image.created) to multiarch image. Decided not touch Dockerfile.arm* because it is better to unify them with Dockerfile (see e.g. #2566). Github action already adds opencontainer image labels: ```console docker inspect ghcr.io/zalando/skipper:v0.21.209 | jq .[0].Config.Labels { "maintainer": "Team Gateway&Proxy @ Zalando SE <[email protected]>", "org.opencontainers.image.created": "2024-09-26T09:40:09.452Z", "org.opencontainers.image.description": "An HTTP router and reverse proxy for service composition, including use cases like Kubernetes Ingress", "org.opencontainers.image.licenses": "NOASSERTION", "org.opencontainers.image.revision": "08a295f2cbf443d5d87f680d6ba3335fba57daef", "org.opencontainers.image.source": "https://github.com/zalando/skipper", "org.opencontainers.image.title": "skipper", "org.opencontainers.image.url": "https://github.com/zalando/skipper", "org.opencontainers.image.version": "v0.21.209" } ``` This change also fixes licenses and adds vendor label. Signed-off-by: Alexander Yastrebov <[email protected]>

AlexanderYastrebov commented Sep 1, 2023

View reviewed changes

AlexanderYastrebov force-pushed the packaging/refactor-docker-build branch 2 times, most recently from 6924001 to e7ea94b Compare September 1, 2023 15:49

szuecs reviewed Sep 1, 2023

View reviewed changes

linki reviewed Sep 8, 2023

View reviewed changes

AlexanderYastrebov force-pushed the packaging/refactor-docker-build branch from e7ea94b to 785678b Compare September 11, 2023 10:44

AlexanderYastrebov mentioned this pull request Sep 27, 2024

packaging: add opencontainers image labels #3251

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

packaging: refactor docker image build #2566

packaging: refactor docker image build #2566

AlexanderYastrebov commented Sep 1, 2023

AlexanderYastrebov commented Sep 1, 2023

AlexanderYastrebov Sep 1, 2023

RomanZavodskikh commented Sep 1, 2023

szuecs Sep 1, 2023

AlexanderYastrebov Sep 1, 2023 •

edited

Loading

linki Sep 8, 2023 •

edited

Loading

linki Sep 8, 2023 •

edited

Loading

linki Sep 8, 2023

AlexanderYastrebov Sep 11, 2023

linki Dec 22, 2023

szuecs Dec 22, 2023

linki Sep 8, 2023 •

edited

Loading

linki Sep 8, 2023

linki Sep 8, 2023

linki Sep 8, 2023

AlexanderYastrebov Sep 8, 2023 •

edited

Loading

szuecs commented Dec 22, 2023

packaging: refactor docker image build #2566

Are you sure you want to change the base?

packaging: refactor docker image build #2566

Conversation

AlexanderYastrebov commented Sep 1, 2023

Makefile changes

Dockerfile changes

Testing

Links

AlexanderYastrebov commented Sep 1, 2023

Choose a reason for hiding this comment

RomanZavodskikh commented Sep 1, 2023

Choose a reason for hiding this comment

AlexanderYastrebov Sep 1, 2023 • edited Loading

Choose a reason for hiding this comment

linki Sep 8, 2023 • edited Loading

Choose a reason for hiding this comment

linki Sep 8, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

linki Sep 8, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AlexanderYastrebov Sep 8, 2023 • edited Loading

Choose a reason for hiding this comment

szuecs commented Dec 22, 2023

AlexanderYastrebov Sep 1, 2023 •

edited

Loading

linki Sep 8, 2023 •

edited

Loading

linki Sep 8, 2023 •

edited

Loading

linki Sep 8, 2023 •

edited

Loading

AlexanderYastrebov Sep 8, 2023 •

edited

Loading