hack: update test dockerfiles to buildkit #666

tonistiigi · 2018-10-05T01:09:44Z

Signed-off-by: Tonis Tiigi [email protected]

tonistiigi · 2018-10-05T02:19:55Z

The main issue atm is that although the new dockerfile allows getting fast incremental builds, multi-platform releases of everything and integration test image very quickly there is no way to quickly run that image with privileges like the tests require. Only docker currently can do that, but docker does not have support for releasing the multi-arch release images needed in CI.

The options are:
a) load the integration test-integration image into Docker in CI. It is slow but ok for ci as it only happens once. For local development, we can use moby+buildkit by default.
b) Add more one-off execution features to buildkit. I think this is needed/useful for lots of things but don't have a specific plan yet.
c) Wait/carry #570 and execute in Dockerfile (or b)
d) Integrate with containerd in CI and try to use ctr. Probably complicated for local cases.
e) Build some stages twice in CI. Only release is done with buildctl.

Tend to think a is the best option atm.

@AkihiroSuda @tiborvass

AkihiroSuda · 2018-10-05T02:23:49Z

hack/dockerfiles/test.buildkit.Dockerfile

+
+# Rootless mode.
+# Still requires `--privileged`.
+FROM tonistiigi/buildkit:rootless-base AS rootless


what's this?

Its the rootless-base stage that built for all platforms so we can use cross-compilation on top of it. I can add a build arg to switch between this image and direct stage. eg. https://github.com/tonistiigi/copy/blob/master/Dockerfile#L3

btw, rootlesskit multi-platform is currently blocked with moby/datakit#638

wow, didn't notice rootlesskit was vendoring datakit

why do we need the copy binary for rootless?

Only if node can run apk an all platforms. What travis can't. The difference is that tonistiigi/buildkit:rootless-base is already multi-platform so any node can add the missing binaries on top of that.

Also, FROM --platform=$TARGETPLATFORM alpine AS rootless-base is the same as FROM alpine AS rootless-base.

# docker run --rm mplatform/mquery tonistiigi/buildkit:rootless-base Image: tonistiigi/buildkit:rootless-base * Manifest List: Yes * Supported platforms: - linux/amd64 - linux/arm/v6 - linux/arm/v7 - linux/arm64/undefined - linux/ppc64le - linux/s390x

What travis can't.

Doesn't qemu-user binfmt work on Travis?

Emulation is not that stable that we could trust it in here. Especially on power/s390x. Also, I'd expect this script to run anywhere, without requiring special setup or a new kernel.

(haven't really tested if emultaion works at all on travis)

Could you add the following stuff to the comment lines of the Dockerfile:

image digest for reproducibility

how you built the image

background (can be just a link to this GitHub issue thread)

AkihiroSuda · 2018-10-05T02:27:37Z

hack/dockerfiles/test.buildkit.Dockerfile

+RUN apk add --no-cache git
+
+# xgo is a helper for golang cross-compilation
+FROM --platform=$BUILDPLATFORM tonistiigi/xx:golang AS xgo


where's dockerfile for xx?

https://github.com/tonistiigi/xx/blob/master/golang/Dockerfile

AkihiroSuda · 2018-10-05T02:28:27Z

hack/dockerfiles/test.buildkit.Dockerfile

+# available targets: buildkitd, buildkitd.oci_only, buildkitd.containerd_only
+ARG BUILDKIT_TARGET=buildkitd
+ARG REGISTRY_VERSION=2.6
+ARG ROOTLESSKIT_VERSION=20b0fc24b305b031a61ef1a1ca456aadafaf5e77


d843aadf00d72082fd7a31572ef018d1e792535f

AkihiroSuda · 2018-10-05T02:30:23Z

hack/dockerfiles/test.buildkit.Dockerfile

+FROM git AS runc-src
+ARG RUNC_VERSION
+WORKDIR /usr/src
+RUN git clone git://github.com/opencontainers/runc.git runc \


Can we experiment ADD git:// source op?

Maybe FROM git:// ?

AkihiroSuda · 2018-10-05T02:33:42Z

a) load the integration test-integration image into Docker in CI. It is slow but ok for ci as it only happens once. For local development, we can use moby+buildkit by default.

does nightly dind work?
https://hub.docker.com/r/tianon/docker-master/
https://github.com/tianon/dockerfiles/tree/master/docker-master

tonistiigi · 2018-10-05T02:38:33Z

@AkihiroSuda With option a we can use any version of Docker. We would use buildctl to build everything and Docker only for running the test-integration image. For the opposite case, no version of moby currently supports building multi-platform images and it probably will not land before the containerd storage is integrated.

tonistiigi · 2018-10-05T04:51:54Z

hack/dockerfiles/test.buildkit.Dockerfile

+COPY --from=binaries / /usr/bin/
+COPY . .
+
+FROM rootless


delete this stage (duplicate)

tonistiigi · 2018-10-09T23:43:18Z

@AkihiroSuda Something really strange going on with rootless worker here that maybe you can look into. The new integration test image is based on stretch and it seems impossible to get it to pass before a timeout is reached. The oci-rootless tests are ~20x slower than before and I can reproduce locally with single tests. As soon as I switch the test stage to alpine the problem goes away and tests are fast again. Added a separate commit that patches alpine, to test the slow version just remove that commit.

data in https://gist.github.com/tonistiigi/a6f95f2527c8f284c3efa61bc8b4a972

AkihiroSuda · 2018-10-10T06:22:30Z

This is because of

buildkit/util/testutil/integration/util.go

Line 55 in e87d816

case <-time.After(20 * time.Second):

, but not sure why SIGTERM does not work here..

AkihiroSuda · 2018-10-10T06:30:41Z

Could be some weird difference across Alpine sudo and Debian sudo

AkihiroSuda

LGTM

AkihiroSuda · 2018-10-10T06:45:24Z

Mergeable?

tonistiigi · 2018-10-10T18:30:03Z

@AkihiroSuda No, I'll still try to get rid of the alpine commit so we don't need to have two stacks of go.

tonistiigi · 2018-10-10T23:16:48Z

@AkihiroSuda @tiborvass Ready now

tiborvass · 2018-10-11T00:42:35Z

hack/validate-generated-files

 case ${1:-} in
 '')
  . $(dirname $0)/util
  gogo_version=$(awk '$1 == "github.com/gogo/protobuf" { print $2 }' vendor.conf)
  case $buildmode in
  "buildkit")
-    buildctl build --frontend=dockerfile.v0 --local context=. --local dockerfile=. --frontend-opt build-arg:GOGO_VERSION=$gogo_version --frontend-opt filename=./hack/dockerfiles/generated-files.buildkit.Dockerfile
+    buildctl build $progressFlag --frontend=dockerfile.v0 --local context=. --local dockerfile=. --frontend-opt build-arg:GOGO_VERSION=$gogo_version --frontend-opt filename=./hack/dockerfiles/generated-files.buildkit.Dockerfile


$progressFlag should also go to docker build

CONTINUOUS_INTEGRATION is only set with buildctl

tiborvass · 2018-10-11T00:53:11Z

hack/test

+  rm $tmpfile
+  ;;
+*)
+  case $buildmode in


I don't understand this case $buildmode within the *) of a case $buildmode. Bad refactoring or am I missing something?

the * case is longer than inner switch

tiborvass · 2018-10-11T00:58:29Z

hack/dockerfiles/test.buildkit.Dockerfile

+ARG ROOTLESS_BASE_MODE=external
+
+# available targets: buildkitd, buildkitd.oci_only, buildkitd.containerd_only
+ARG BUILDKIT_TARGET=buildkitd


AkihiroSuda · 2018-10-11T05:54:53Z

hack/binaries

+if [ "$CONTINUOUS_INTEGRATION" == "true" ]; then progressFlag="--progress=plain"; fi
+
+binariesLegacy() {
+	mkdir -p bin


indentation seems broken.

I'm not sure whether there is gofmt-equivalent for bash, but emacs can indent shell scripts with C-x h M-x indent.

AkihiroSuda · 2018-10-11T05:56:10Z

hack/dockerfiles/test.Dockerfile

@@ -154,7 +155,8 @@ FROM buildkit-buildkitd AS rootless
 RUN apk add --no-cache shadow shadow-uidmap \
  && useradd --create-home --home-dir /home/user --uid 1000 user \
  && mkdir -p /run/user/1000 /home/user/.local/tmp /home/user/.local/share/buildkit \
-  && chown -R user /run/user/1000 /home/user
+  && chown -R user /run/user/1000 /home/user \
+  && rm /bin/su && ln -s /bin/busybox /bin/su


Probably, we can just lock the root with passwd -l root: https://github.com/genuinetools/img/blob/151b4506829bd7f52f5ba83aa99a3fd08709554a/Dockerfile#L53

Signed-off-by: Tonis Tiigi <[email protected]>

tonistiigi added the status/do-not-merge label Oct 5, 2018

AkihiroSuda reviewed Oct 5, 2018

View reviewed changes

tonistiigi commented Oct 5, 2018

View reviewed changes

tonistiigi force-pushed the dockerfile-update branch 5 times, most recently from 6186adb to 9487c77 Compare October 9, 2018 00:40

AkihiroSuda mentioned this pull request Oct 9, 2018

rootless: use Giuseppe's newuidmap which does not require CAP_SYS_ADMIN #671

Closed

tonistiigi force-pushed the dockerfile-update branch 3 times, most recently from 2e1ee93 to b59fa55 Compare October 9, 2018 06:33

tonistiigi removed the status/do-not-merge label Oct 9, 2018

tonistiigi force-pushed the dockerfile-update branch 4 times, most recently from ef186c1 to 714ca48 Compare October 9, 2018 23:13

tonistiigi added the status/do-not-merge label Oct 9, 2018

AkihiroSuda approved these changes Oct 10, 2018

View reviewed changes

tonistiigi force-pushed the dockerfile-update branch from 714ca48 to a962f8a Compare October 10, 2018 22:54

tonistiigi removed the status/do-not-merge label Oct 10, 2018

tiborvass reviewed Oct 11, 2018

View reviewed changes

tonistiigi force-pushed the dockerfile-update branch from a962f8a to e10477b Compare October 11, 2018 01:40

tonistiigi mentioned this pull request Oct 11, 2018

hack: add release script #676

Merged

tiborvass approved these changes Oct 11, 2018

View reviewed changes

AkihiroSuda reviewed Oct 11, 2018

View reviewed changes

tonistiigi added 2 commits October 10, 2018 23:26

hack: update test dockerfiles to buildkit

ddfb7f7

Signed-off-by: Tonis Tiigi <[email protected]>

integration: add setsid for sigterm

43bd294

Signed-off-by: Tonis Tiigi <[email protected]>

tonistiigi force-pushed the dockerfile-update branch from e10477b to 43bd294 Compare October 11, 2018 06:27

AkihiroSuda approved these changes Oct 11, 2018

View reviewed changes

AkihiroSuda merged commit 5d8be2c into moby:master Oct 11, 2018

crazy-max mentioned this pull request Oct 29, 2022

Dockerfile: buildkitd.oci_only and buildkitd.containerd_only targets #3225

Closed

hack: update test dockerfiles to buildkit #666

hack: update test dockerfiles to buildkit #666

Uh oh!

Conversation

tonistiigi commented Oct 5, 2018

Uh oh!

tonistiigi commented Oct 5, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AkihiroSuda commented Oct 5, 2018

Uh oh!

tonistiigi commented Oct 5, 2018

Uh oh!

tonistiigi Oct 5, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tonistiigi commented Oct 9, 2018

Uh oh!

AkihiroSuda commented Oct 10, 2018

Uh oh!

AkihiroSuda commented Oct 10, 2018

Uh oh!

AkihiroSuda left a comment

Choose a reason for hiding this comment

Uh oh!

AkihiroSuda commented Oct 10, 2018

Uh oh!

tonistiigi commented Oct 10, 2018

Uh oh!

tonistiigi commented Oct 10, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

tonistiigi Oct 5, 2018 •

edited

Loading