tools: multi-arch support for the base alpine image #2244

arm64b · 2017-07-19T09:47:36Z

Alpine is the base docker image for the LinuxKit, but currently
it only support amd64 architecture. This patch try to refactor
the alpine docker image build process in order to support other
architectures, such as arm64, by using the new capability of 'ARG'
and 'FROM' in Dockerfile.
Also this patch splits the standalone packages into 2 parts:
one is common for all archs, another is arch-specific.

Signed-off-by: Dennis Chen dennis.chen@arm.com

- What I did
Support muti-arch build for tools/alpine docker image
- How I did it
Take use of the new feature of 'ARG' and 'FROM' supported by newer version of Moby in the Dockerfile, thus we can assign a specific BASE image with '--build-arg BASE=xxx' as the base image used by 'FROM'.

- How to verify it
Navigate into the tools/alpine folder, 'make ORG=your_org' on amd64 and arm64 platform, then we can get a your_org/alpine:xxx image on different platforms, using this image as the LinuxKit base image for other tools and pkgs.
$bin/moby build linuxkit.yml
$bin/linuxkit run ... linuxkit

- Description for the changelog

- A picture of a cute animal (not mandatory but encouraged)

arm64b · 2017-07-19T09:50:34Z

@justincormack @rn
please help to review those changes, especially those packages should be installed.

This RP is a key step for us to support ARM64 with LinuxKit mainline code. After that I will clone a brand new LinuxKit repos into different ARM64 platforms to test if we still have some gap to do that.

Thanks!

justincormack · 2017-07-19T09:55:31Z

tools/alpine/.detect-arch-var

@@ -0,0 +1,13 @@
+#!/usr/bin/env bash


please use sh not bash. Also having hidden files is kind of annoying, just use a normal one.

OK. I will wrap up all the changes and make a re-push

justincormack · 2017-07-19T10:08:53Z

I think there is a much simpler solution with a single Dockerfile as since Docker 17.06 you can use build args in the FROM line so the following works fine:

whale:arg justin$ cat Dockerfile 
ARG BASE=alpine:3.6
FROM $BASE
RUN cat /etc/alpine-release
RUN uname -a
whale:arg justin$ docker build --no-cache .
Sending build context to Docker daemon  2.048kB
Step 1/4 : ARG BASE=alpine:3.6
 ---> 
Step 2/4 : FROM $BASE
 ---> 7328f6f8b418
Step 3/4 : RUN cat /etc/alpine-release
 ---> Running in 288e5cbbe59c
3.6.2
 ---> 938b22c437cc
Removing intermediate container 288e5cbbe59c
Step 4/4 : RUN uname -a
 ---> Running in dda55f6687bd
Linux e1ede117fb1e 4.9.36-moby moby/moby#1 SMP Wed Jul 12 17:33:58 UTC 2017 x86_64 Linux
 ---> e1b9fa9cb8c1
Removing intermediate container dda55f6687bd
Successfully built e1b9fa9cb8c1
whale:arg justin$ docker build --no-cache --build-arg BASE=arm64v8/alpine:3.6 .
Sending build context to Docker daemon  2.048kB
Step 1/4 : ARG BASE=alpine:3.6
 ---> 
Step 2/4 : FROM $BASE
 ---> 77dabab369a1
Step 3/4 : RUN cat /etc/alpine-release
 ---> Running in e409bdf4f721
3.6.2
 ---> 2c52ad7e3a2e
Removing intermediate container e409bdf4f721
Step 4/4 : RUN uname -a
 ---> Running in 43acd0217e69
Linux bf9f9dc31cee 4.9.36-moby moby/moby#1 SMP Wed Jul 12 17:33:58 UTC 2017 aarch64 Linux
 ---> 79d388609c80
Removing intermediate container 43acd0217e69
Successfully built 79d388609c80

justincormack · 2017-07-19T10:17:52Z

(this would also allow us to override the base image for any architecture, which could be useful for testing alpine edge vs release)

arm64b · 2017-07-19T10:25:22Z

Ah, cool! let me try that, we can use only one Dockerfile if that works. My docker version is:
Client:
Version: unknown-version
API version: 1.31
Go version: go1.8.3
Git commit: 5dd3073
Built: Sun Jun 18 07:54:10 2017
OS/Arch: linux/arm64

Server:
Version: 17.06.0-dev
API version: 1.31 (minimum version 1.12)
Go version: go1.8.3
Git commit: b931c35
Built: Sun Jun 18 08:02:11 2017
OS/Arch: linux/arm64
Experimental: false

justincormack · 2017-07-19T10:33:11Z

That should be new enough. We are fine with requiring developers to have newest docker versions, we dd that before for multi stage builds too.

arm64b · 2017-07-19T10:36:57Z

@justincormack yes, it works. So that means we can use one Dockerfile. Now have another issue:
https://github.com/linuxkit/linuxkit/blob/master/tools/alpine/Dockerfile#L48
ovmf pkg is missed from arm64v8/alpine, does 'COPY' has some tricks like 'FROM'?

justincormack · 2017-07-19T10:41:17Z

I think the easiest thing to do is something like:

RUN mkdir -p /usr/share/ovmf
RUN [ $(uname -m) = "x86_64 ] && apk add -X http://dl-cdn.alpinelinux.org/alpine/edge/community ovmf 

...

COPY --from=mirror /usr/share/ovmf/ /usr/share/ovmf/

arm64b · 2017-07-19T10:45:51Z

Good point!
So how about the packages split? Since some pkgs in the 'packages' file is not available for arm64 such as open-vm-tools, syslinux...

justincormack · 2017-07-19T10:56:51Z

I think move those into packages.x86_64, add a (maybe empty) packages.aarch64 and then always install packages and packages.$(uname -r)

arm64b · 2017-07-19T11:03:02Z

like this?

COPY packages* /tmp/

RUN cat /tmp/packages.$(uname -m) >> /tmp/packages &&
mkdir -p /mirror/$(uname -m) &&
apk fetch --recursive -o /mirror/$(uname -m) $(apk info; cat /tmp/packages)

justincormack · 2017-07-19T11:05:51Z

yes thats what I was thinking.

arm64b · 2017-07-19T11:08:36Z

OK, I will wrap up those changes and re-push it again. Thanks for the good suggestion!

rn

Thanks for tackling this. I few more comments/suggestions but overall the shape looks good

rn · 2017-07-19T13:29:34Z

tools/alpine/Makefile

@@ -2,22 +2,24 @@

 ORG?=linuxkit
 IMAGE=alpine
-BASE=alpine:3.6
+
+BASE := $(shell bash -c 'source detect-arch-var && echo $${BASE}')


Could we not use bash here and in the next?

very good suggestion. I will drop the 'detect-arch-var' file and transfer its operation into Makefile, so I promise 'bash' will not show again ;-)

rn · 2017-07-19T13:31:22Z

tools/alpine/Makefile


 default: push

 show-tag:
 	@sed -n -e '1s/# \(.*\/.*:[0-9a-f]\{40\}\)/\1/p;q' versions

 hash: Dockerfile Makefile packages
-	DOCKER_CONTENT_TRUST=1 docker pull $(BASE)
-	docker build --no-cache -t $(IMAGE):build .
+	DOCKER_CONTENT_TRUST=$(DOCKER_CONT) docker pull $(BASE)


It would be better to copy the pattern used in pkg/package.mk for setting the content trust.

Of course. Actually I do think that's will be very elegant, at least looks like - as we'll move all the jobs in 'detect-*" into Makefile.

rn · 2017-07-19T13:34:17Z

tools/alpine/detect-arch-var

@@ -0,0 +1,11 @@
+#!/usr/bin/env sh


Can't this be moved into the Makefile, ie set a makefile variables for uname -m and then set other makefile variables based off that?

Sure, of course! But interesting, for the 'ARG' and ‘FROM’ feature supported by the newer version of Docker, seems there's a potential bug for the DOCKER_CONTENT_TRUST setting:
$export DOCKER_CONTENT_TRUST=1
$docker build --no-cache --build-arg BASE=alpine:3.6 -t alpine:build .
You will get below message:
Sending build context to Docker daemon

error during connect: Post http://%2Fvar%2Frun%2Fdocker.sock/v1.31/build?buildargs=%7B%22BASE%22%3A%22alpine%3A3.6%22%7D&cachefrom=%5B%5D&cgroupparent=&cpuperiod=0&cpuquota=0&cpusetcpus=&cpusetmems=&cpushares=0&dockerfile=Dockerfile&labels=%7B%7D&memory=0&memswap=0&networkmode=default&nocache=1&rm=1&shmsize=0&t=alpine%3Abuild&target=&ulimits=null: invalid reference format: repository name must be lowercase

@justincormack @rn please help double check this, if it's done, my PR will be ready quickly to push...

hmm, yes I filed an issue https://github.com/moby/moby/issues/34199 will try to get it fixed ASAP.

As a temporary workaround, you could omit setting DOCKER_CONTENT_TRUST on the build step if the build arg is set.

Great! I will set the DOCKER_CONTENT_TRUST=0 as default in my PR, it will be easy to be tuned once this issue fixed.

arm64b · 2017-07-20T12:03:12Z

@justincormack @rn The gun is ready to shoot... ;-)

justincormack · 2017-07-20T12:44:07Z

tools/alpine/Makefile

-	DOCKER_CONTENT_TRUST=1 docker pull $(BASE)
-	docker build --no-cache -t $(IMAGE):build .
+hash: Dockerfile Makefile $(PKG_DEPS)
+	docker pull $(BASE)


you can keep the DOCKER_CONTENT_TRUST=1 here (unless the arm64v8 packages are not signed, in which case it needs to be conditional; it was only haveing it set on docker build that causes issues.

Ah, ok, the arm64 images are not signed, so I think you can just set NOTRUST for the arm64 case. But you must not disable for the x86_64 case. This Makefile does not set content trust for build as it already does a trusted pull, so should not run into the bug.

@riyazdf can probably help work out the best arrangement (I am on a train today)

agreed with @justincormack that we must keep x86_64 trust checking intact.

perhaps we can export DOCKER_CONTENT_TRUST=1 for this Makefile and set NOTRUST in the arm conditional where we override BASE?

@justincormack @riyazdf
Ah, thanks for the remind! I totally agree with that we need to keep x86_64 trust checking intact, I am missing that. So now I made below changes:

DOCKER_CONTENT_VAR=1 PKG_DEPS=packages ifeq ($(shell uname -m), x86_64) BASE=alpine:3.6 PKG_DEPS += packages.x86_64 endif ifeq ($(shell uname -m), aarch64) BASE=arm64v8/alpine:3.6 PKG_DEPS += packages.aarch64 DOCKER_CONTENT_VAR=0 endif default: push show-tag: @sed -n -e '1s/# $.*\/.*:[0-9a-f]\{40\}$/\1/p;q' versions hash: Dockerfile Makefile $(PKG_DEPS) DOCKER_CONTENT_TRUST=$(DOCKER_CONTENT_VAR) docker pull $(BASE) docker build --no-cache --build-arg BASE=$(BASE) -t $(IMAGE):build . docker run --rm $(IMAGE):build sh -c 'echo Dockerfile /lib/apk/db/installed $$(find /mirror -name '*.apk' -type f) $$(find /go/bin -type f) | xargs cat | sha1sum' | sed 's/ .*//' > $@ push: hash DOCKER_CONTENT_TRUST=$(DOCKER_CONTENT_VAR) docker pull $(ORG)/$(IMAGE):$(shell cat hash) || \ (docker tag $(IMAGE):build $(ORG)/$(IMAGE):$(shell cat hash) && \ DOCKER_CONTENT_TRUST=$(DOCKER_CONTENT_VAR) docker push $(ORG)/$(IMAGE):$(shell cat hash))

For x86_64, it willkeep the same behavior as it does before, for aarch64, the DOCKER_CONTENT_TRUST will be overrided. Does that make sense?

arm64b · 2017-07-21T10:22:43Z

@rn @justincormack @riyazdf

would you pls help to review the latest update?

rn

@arm64b I think you also need to have a per arch versions file as that get's checked in and the versions may differ on the architectures.

rn · 2017-07-21T12:02:28Z

tools/alpine/Dockerfile

@@ -25,7 +27,10 @@ RUN cp /mirror/$(uname -m)/APKINDEX.unsigned.tar.gz /mirror/$(uname -m)/APKINDEX
 RUN abuild-sign /mirror/$(uname -m)/APKINDEX.tar.gz

 # fetch OVMF for qemu EFI boot (this is not added as a package)
-RUN apk add -X http://dl-cdn.alpinelinux.org/alpine/edge/community ovmf
+RUN mkdir -p /usr/share/ovmf && \ 


This should probably be added the same way the wireguard tools were added a yesterday. The way this is currently added (and was in the past) does not record the version number in the versions file. I know it's not related directly to your PR, so maybe should be addressed separately as it also has implications elsewhere.

I am trying to generate a per-arch version file, will double check if the ovmf pkg has been recorded in that file like version.x86_64.

rn · 2017-07-21T12:03:31Z

tools/alpine/Makefile

 		(docker tag $(IMAGE):build $(ORG)/$(IMAGE):$(shell cat hash) && \
-		 DOCKER_CONTENT_TRUST=1 docker push $(ORG)/$(IMAGE):$(shell cat hash))
+		 DOCKER_CONTENT_TRUST=$(DOCKER_CONTENT_VAR) docker push $(ORG)/$(IMAGE):$(shell cat hash))
 	echo "# $(ORG)/$(IMAGE):$(shell cat hash)" > versions


the versions file probably needs to be per arch

Not very sure about the exact usage purpose of this file, if it really needs to generate a per arch, then we can do like this:
... > versions.$(shell uname -m)
But IMO, if you work on a platform, no matter amd64 or arm64, it's easy for you to know that the version file is the list of packages installed for that platform.
I am OK for both.

This file is checked in and is expected to contain the exact versions present in the image hash in the header. Given that this package is built with network access it might be quite hard to ensure that there is no skew due to arches being built at different times (or even atthe same time if there is skew in the upstream alpine repo).

IMHO it would be best to find a way to build a single arch agnostic base mirror image with a multiarch mirrror in it, but for now I think per arch version files are the minimum we need.

Ah, I am making the per-arch version file building test, will generate versions.x86_64 on x86_64 platform, versions.aarch64 on AArch64.

arm64b · 2017-07-21T13:10:13Z

@rn @justincormack @ijc @riyazdf
The latest update to address the 'versions' file, pls help to resolve the conflict if possible. Thanks!

rn · 2017-07-21T13:21:01Z

tools/alpine/Makefile

+BASE=arm64v8/alpine:3.6
+PKG_DEPS += packages.aarch64
+DOCKER_CONTENT_VAR=0
+endif


I lost track of this part, but is there a reason, this is not following the pattern we have elsewhere (pkg/package.mk) and simply just sets NOTRUST when on arm?

The reason is, if we export DOCKER_CONTENT_TRUST=1 for x86, then we will encounter the issue filed by https://github.com/moby/moby/issues/34199 no matter we use:
$docker build --no-cache --build-arg BASE=alpine:3.6 -t alpine:build .
or
$docker build --no-cache -t alpine:build .

That's why we still use the original Makefile content trust mode when building the image.

Thanks for reminding me

justincormack · 2017-07-21T14:28:13Z

Needs a rebase (versions changed).

arm64b · 2017-07-24T02:42:48Z

Will wrap up and edit the commit message in the 1st PR submitted since we have a little big change for now @justincormack

arm64b · 2017-07-24T06:27:19Z

@justincormack @riyazdf @rn
The latest commit is 516320b

rn · 2017-07-24T09:16:49Z

@arm64b apologies, but this needs another rebase. I accidentally merged a later PR which introduced conflicts with this one. Otherwise this looks good to me and I'll merge after the rebase

Alpine is the base docker image for the LinuxKit, but currently it only supports amd64 architecture. This patch is try to unify the alpine tool docker image build process order to suport other architectures, such as AArch64, by using '--build-arg' to override the alpine base image specified by 'FROM' in the Dockerfile. Also this patch splits the standalone packages into 2 parts: one is common for all archs, another is arch-specific. Signed-off-by: Dennis Chen <dennis.chen@arm.com>

arm64b · 2017-07-24T10:14:52Z

@rn rebase to the latest code base. it's not a straightforward rebase since we need to take care about the hash value of the versions.* in case installed pkgs changed

rn

thanks for the rebase. will merge this as soon as CI finishes to avoid more hassle

GordonTheTurtle added the status/0-triage label Jul 19, 2017

justincormack reviewed Jul 19, 2017

View reviewed changes

arm64b force-pushed the tool-alpine-refactor branch from d9491a6 to 7c121e1 Compare July 19, 2017 12:17

rn reviewed Jul 19, 2017

View reviewed changes

justincormack mentioned this pull request Jul 20, 2017

build args in FROM do not work with content trust moby/buildkit#4255

Open

arm64b force-pushed the tool-alpine-refactor branch from 7c121e1 to 5bf86e6 Compare July 20, 2017 08:03

justincormack reviewed Jul 20, 2017

View reviewed changes

arm64b force-pushed the tool-alpine-refactor branch from 5bf86e6 to 79bcfb2 Compare July 21, 2017 03:04

rn reviewed Jul 21, 2017

View reviewed changes

arm64b force-pushed the tool-alpine-refactor branch from 79bcfb2 to 9f3890f Compare July 21, 2017 13:03

rn reviewed Jul 21, 2017

View reviewed changes

arm64b force-pushed the tool-alpine-refactor branch from 9f3890f to 516320b Compare July 24, 2017 05:35

arm64b force-pushed the tool-alpine-refactor branch from 516320b to cc14a74 Compare July 24, 2017 10:11

justincormack approved these changes Jul 24, 2017

View reviewed changes

rn approved these changes Jul 24, 2017

View reviewed changes

ijc mentioned this pull request Jul 24, 2017

alpine: Use docker build --iidfile #2260

Merged

rn merged commit 53bd58e into linuxkit:master Jul 24, 2017

arm64b deleted the tool-alpine-refactor branch July 24, 2017 11:06

tools: multi-arch support for the base alpine image #2244

tools: multi-arch support for the base alpine image #2244

Conversation

arm64b commented Jul 19, 2017 • edited

arm64b commented Jul 19, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

justincormack commented Jul 19, 2017

justincormack commented Jul 19, 2017

arm64b commented Jul 19, 2017

justincormack commented Jul 19, 2017

arm64b commented Jul 19, 2017

justincormack commented Jul 19, 2017

arm64b commented Jul 19, 2017

justincormack commented Jul 19, 2017

arm64b commented Jul 19, 2017 • edited

justincormack commented Jul 19, 2017

arm64b commented Jul 19, 2017

rn left a 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

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

arm64b commented Jul 20, 2017

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

arm64b Jul 21, 2017 • edited

Choose a reason for hiding this comment

arm64b commented Jul 21, 2017

rn left a 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

arm64b Jul 21, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

arm64b commented Jul 21, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

justincormack commented Jul 21, 2017

arm64b commented Jul 24, 2017

arm64b commented Jul 24, 2017

rn commented Jul 24, 2017

arm64b commented Jul 24, 2017

rn left a comment

Choose a reason for hiding this comment

arm64b commented Jul 19, 2017 •

edited

arm64b commented Jul 19, 2017 •

edited

arm64b Jul 21, 2017 •

edited

arm64b Jul 21, 2017 •

edited