Updates the Dockerfile to use multi-stage by cpuguy83 · Pull Request #35100 · moby/moby

cpuguy83 · 2017-10-05T18:24:23Z

Posting this up here for others to contribute to.
IIRC the CI dockerd instances don't support multi-stage builds yet.

I'm not sure if this broke CRIU or not (I haven't tried to use CRIU in dind before and it definitely doesn't work with this PR).
Everything else seems to work.
I find this to be a little slower (haven't measured, not much but if you are paying attention you'd notice) to run the build from a full cache than the monolithic Dockerfile, but it makes it MUCH faster for individual changes to the dockerfile not requiring to rebuild basically the whole thing.

AkihiroSuda · 2017-10-06T00:40:27Z

Dockerfile

seems unneeded

Looks like a bad rebase or so; I see && apt-get install -y curl at line 12 below, which probably had to be here

AkihiroSuda · 2017-10-06T00:41:33Z

Dockerfile

Can we make this Dockerfile to support other archs

Can we do it separately? I'd love to but I'm sure there must be other things that need to be done.
Also I'm not sure if the base images have multi-arch even though they provide manifest lists now?

AkihiroSuda · 2017-10-06T02:42:33Z

Dockerfile

How does it relate to golang?

The frozen image script is using go env.
This could be worked around, but I'd rather leave it as is for now.

AkihiroSuda · 2017-10-06T02:43:56Z

Dockerfile

Can we just use docker.io/library/golang? (cc @thaJeztah @justincormack )

I don't know why we aren't using the golang image... it's come up before, just can't remember the details.

AkihiroSuda · 2017-10-06T02:45:52Z

Dockerfile

unrelated to golang?

AkihiroSuda · 2017-11-29T02:42:34Z

ping @cpuguy83 ^^

cpuguy83 · 2017-12-11T20:40:01Z

I updated this... but of course our builders are still out of date.

seemethere · 2017-12-11T21:08:02Z

hack/dockerfile/install/gometalinter.installer

Why not just use git -C here instead of doing a cd and then a git command?

These are copied from the old install-binaries.sh file. I'd prefer to optimize them separately.

seemethere · 2017-12-11T21:08:24Z

hack/dockerfile/install/proxy.installer

Same note here about git -C

thaJeztah · 2017-12-12T03:56:37Z

Dockerfile

&& apt-get install -y curl

bad rebase?

AkihiroSuda · 2018-02-06T05:23:28Z

ping ^^ @cpuguy83

thaJeztah · 2018-02-06T05:31:04Z

I was also wondering if we should pin the Go version for the various upstream binaries (e.g., what if upstream RunC has only been tested/verified with Go 1.8?)

cpuguy83 · 2018-02-06T09:26:08Z

We are also still waiting on Jenkins to be updated. There are a few issues on the nodes being tracked.

@psftw

thaJeztah · 2018-02-16T07:48:00Z

Hmf looks like this Jenkins isn’t update yet 😞

thaJeztah · 2018-02-19T16:18:49Z

Binary installs were split off in #36336

tonistiigi

parallelization preview:

> docker build .
...
real	9m15.516s
user	0m2.052s
sys	0m0.880s

> buildctl build --frontend dockerfile.v0  --local context=. --local dockerfile=.

[+] Building 268.2s (61/61) FINISHED
...
real	4m28.229s
user	0m6.208s
sys	0m3.696s

Rebuild with cache for docker build --pull . is 9-10sec vs ~1-2 sec. in buildkit (almost all that time is spent verifying that the base images have not changed in registry as there is no tag cache).

tonistiigi · 2018-02-23T02:25:37Z

Dockerfile

I'd love if these versions would be defined in the top of the Dockerfile now so you could do things like docker build --build-arg RUNC_COMMIT=master .

^^ Good suggestion

I agree. Can we get this change in and then start adding in new features?

vdemeester · 2018-02-25T22:18:38Z

janky tests are ok, the binary cross got "cut" because the build ran for too long

Signed-off-by: Brian Goff <cpuguy83@gmail.com> Signed-off-by: Vincent Demeester <vincent@sbr.pm>

thaJeztah · 2018-02-26T14:16:00Z

Created follow-up issues for tracking:

Unify Dockerfiles for all architectures #36411 Unify Dockerfiles for all architectures
Dockerfile: use build-args to allow overriding binary-commits (and more) #36412 Dockerfile: use build-args to allow overriding binary-commits (and more)
Dockerfile: skip COPY if BIND_DIR is used #36413 Dockerfile: skip COPY if BIND_DIR is used
Dockerfile: standardize artefact paths for intermediate stages #36414 Dockerfile: standardize artefact paths for intermediate stages
Dockerfile: split CI parts to separate Dockerfile / stage #36415 Dockerfile: split CI parts to separate Dockerfile / stage

kolyshkin · 2018-02-26T20:38:26Z

Dockerfile


-FROM debian:stretch
-
+FROM buildpack-deps:stretch AS base


Can we start with golang image (which is essentially stretch with golang installed)?

I think we could, cc @cpuguy83 👼

cc @tianon

I know this was attempted in the past, I don't recall why we didn't do it.

If I recall correctly, at the time we were still using utilities that Ubuntu had better packaging for than Debian, so given we're on Debian now, it's likely OK. 👍

See #36425 👼

cpuguy83 requested review from dnephin and tianon as code owners October 5, 2017 18:24

GordonTheTurtle added the status/0-triage label Oct 5, 2017

AkihiroSuda reviewed Oct 6, 2017

View reviewed changes

Dockerfile Outdated

Copy link
Copy Markdown

Member

AkihiroSuda Oct 6, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unrelated to golang?

GordonTheTurtle assigned thaJeztah Oct 20, 2017

cpuguy83 force-pushed the multistage_dockerfile branch from 793d9be to 250c99a Compare December 11, 2017 16:39

seemethere reviewed Dec 11, 2017

View reviewed changes

thaJeztah reviewed Dec 12, 2017

View reviewed changes

Dockerfile Outdated

Copy link
Copy Markdown

Member

thaJeztah Dec 12, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

&& apt-get install -y curl

bad rebase?

vdemeester added status/2-code-review and removed status/0-triage labels Jan 26, 2018

cpuguy83 force-pushed the multistage_dockerfile branch from 250c99a to d017f2a Compare February 15, 2018 17:55

thaJeztah mentioned this pull request Feb 19, 2018

Split binary installers/commit scripts #36336

Merged

cpuguy83 force-pushed the multistage_dockerfile branch 3 times, most recently from 63aaf8f to 2a0786c Compare February 22, 2018 19:13

tonistiigi reviewed Feb 23, 2018

View reviewed changes

vdemeester added rebuild/janky labels Feb 23, 2018

GordonTheTurtle removed rebuild/experimental labels Feb 23, 2018

vdemeester added the rebuild/janky label Feb 25, 2018

GordonTheTurtle removed the rebuild/janky label Feb 25, 2018

vdemeester added rebuild/janky labels Feb 25, 2018

GordonTheTurtle removed rebuild/janky labels Feb 25, 2018

vdemeester added status/2-code-review and removed status/4-merge labels Feb 26, 2018

Updates the Dockerfile to use multi-stage

d539038

Signed-off-by: Brian Goff <cpuguy83@gmail.com> Signed-off-by: Vincent Demeester <vincent@sbr.pm>

vdemeester force-pushed the multistage_dockerfile branch from fe9d0ab to d539038 Compare February 26, 2018 08:14

vdemeester added status/4-merge rebuild/z and removed status/2-code-review labels Feb 26, 2018

GordonTheTurtle removed the rebuild/z label Feb 26, 2018

vdemeester added the rebuild/z label Feb 26, 2018

GordonTheTurtle removed the rebuild/z label Feb 26, 2018

thaJeztah added the area/project label Feb 26, 2018

vdemeester merged commit 00c1c60 into moby:master Feb 26, 2018

vdemeester mentioned this pull request Feb 26, 2018

CI enhancement and spring cleaning #36416

Open

17 tasks

kolyshkin reviewed Feb 26, 2018

View reviewed changes

cpuguy83 deleted the multistage_dockerfile branch February 26, 2018 20:52

seemethere mentioned this pull request Mar 7, 2018

Build containerd, runc, and proxy statically #36518

Merged

albers mentioned this pull request Jun 29, 2018

Re-add support for a custom .bashrc file in build env #37370

Merged

Conversation

cpuguy83 commented Oct 5, 2017

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 Nov 29, 2017

Uh oh!

cpuguy83 commented Dec 11, 2017

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 Feb 6, 2018

Uh oh!

thaJeztah commented Feb 6, 2018

Uh oh!

cpuguy83 commented Feb 6, 2018

Uh oh!

thaJeztah commented Feb 16, 2018

Uh oh!

thaJeztah commented Feb 19, 2018

Uh oh!

tonistiigi left a comment

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!

vdemeester commented Feb 25, 2018

Uh oh!

thaJeztah commented Feb 26, 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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

10 participants