Build binaries with minimal deps and remove autogen code by cpuguy83 · Pull Request #40180 · moby/moby

cpuguy83 · 2019-11-05T22:09:58Z

More optimizations for the Dockerfile

Do not use the dev target for building binaries since we don't need all that stuff. This is much faster since we don't need to compile things like CRIU.
Do not use autoversion
Allow bind-mounting the build context to the binary target stages instead of copying.
Do not include Dockerfile or .dockerignore in build context - (maybe controversial)
i. Prevents rebuilding things just because there is some change in either of these files.
ii. Is not needed since we don't do nested build.

Dockerfile

thaJeztah · 2019-11-05T22:28:03Z

Dockerfile

Perhaps instead of copying these to /usr/local/bin we could just add the bundles directory to PATH, similar to what I did in docker/cli#419 (not sure if there's code that expects the binaries to be in /usr/local/bin though)

Here is the logic:

moby/hack/make/binary-daemon

Lines 4 to 31 in c36460c

copy_binaries() {

local dir="$1"

local hash="$2"

# Add nested executables to bundle dir so we have complete set of

# them available, but only if the native OS/ARCH is the same as the

# OS/ARCH of the build target

if [ "$(go env GOOS)/$(go env GOARCH)" != "$(go env GOHOSTOS)/$(go env GOHOSTARCH)" ]; then

return

fi

if [ ! -x /usr/local/bin/runc ]; then

return

fi

echo "Copying nested executables into $dir"

for file in containerd containerd-shim ctr runc docker-init docker-proxy rootlesskit rootlesskit-docker-proxy dockerd-rootless.sh; do

cp -f "$(command -v "$file")" "$dir/"

if [ "$hash" = "hash" ]; then

hash_files "$dir/$file"

fi

done

# vpnkit is amd64 only

if command -v "vpnkit.$(uname -m)" 2>&1 >/dev/null; then

cp -f "$(command -v "vpnkit.$(uname -m)")" "$dir/vpnkit"

if [ "$hash" = "hash" ]; then

hash_files "$dir/vpnkit"

fi

fi

}

So we can add them anywhere into PATH, or add anywhere to PATH, or just copy them manually.
I wanted to reduce redundant code as much as possible here.
The other thing that makes it kinda tricky is each build target builds in a subdir, like bundles/binary-daemon and bundles/dynbinary-daemon, so I'd have to do this copy twice.

I could bind-mount the files in and update PATH to include each one, but this would only reduce the amount of copying we are doing.

Basically my worry here is we have this list of commands that need to be copied as part of a build and different lists get out of sync.

cpuguy83 · 2019-11-25T19:30:44Z

Ping

thaJeztah

whoops, forgot about this one; left some comments 🤗

hack/make.sh

dockerversion/version_lib.go

.dockerignore

This makes the binary build targets use a minimal build env instead of having to build all the stuff needed for the full dev enviornment. Signed-off-by: Brian Goff <cpuguy83@gmail.com>

This eliminates the need to lay down an auto-generated file. IIRC this was originally hadded for gccgo which we no longer support. Signed-off-by: Brian Goff <cpuguy83@gmail.com>

Signed-off-by: Brian Goff <cpuguy83@gmail.com>

The Dockerfile is not needed in any of the build targets. The one exception may be the dev image, however in most cases the docker source will be bind mounted into the container anyway. Signed-off-by: Brian Goff <cpuguy83@gmail.com>

cpuguy83 · 2019-12-02T18:22:21Z

Fixed.

thaJeztah

LGTM

thaJeztah · 2019-12-03T12:26:37Z

Dockerfile


-FROM dev AS src
+FROM runtime-dev AS src
+# Make arg inheritable


nit: outdated comment?

thaJeztah · 2019-12-03T12:31:41Z

Dockerfile


-FROM src AS final
+FROM dev AS final
+COPY . /go/src/github.com/docker/docker


nit (no need to change); /go/src/github.com/docker/docker is the WORKDIR, is this could be

COPY . .

thaJeztah · 2020-01-17T11:42:43Z

hack/make/.go-autogen

-
-// Code generated by hack/make/.go-autogen. DO NOT EDIT.
-DVEOF
+LDFLAGS="${LDFALGS} \


Whoops; typo: (LDFALGS); fixing in #40389

Upstream moby/moby#40180 removed the auto-generated code, replacing it with build-time variables (-X). This patch updates the Dockerfiles to account for this change. Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

This was removed after refactoring the Dockerfile in moby#40180 Signed-off-by: Brian Goff <cpuguy83@gmail.com>

cpuguy83 requested review from AkihiroSuda and thaJeztah November 5, 2019 22:09

cpuguy83 requested a review from tianon as a code owner November 5, 2019 22:09

cpuguy83 force-pushed the no_more_dev_tools branch from f7af678 to 8f477f2 Compare November 5, 2019 22:11

thaJeztah reviewed Nov 5, 2019

View reviewed changes

Dockerfile Outdated Show resolved Hide resolved

thaJeztah reviewed Nov 5, 2019

View reviewed changes

cpuguy83 force-pushed the no_more_dev_tools branch from 8f477f2 to 1499b27 Compare November 5, 2019 22:44

cpuguy83 changed the title ~~No more dev tools~~ Build binaries with minimal deps Nov 6, 2019

thaJeztah requested changes Nov 27, 2019

View reviewed changes

hack/make.sh Outdated Show resolved Hide resolved

dockerversion/version_lib.go Show resolved Hide resolved

dockerversion/version_lib.go Outdated Show resolved Hide resolved

.dockerignore Outdated Show resolved Hide resolved

thaJeztah added area/project kind/refactor PR's that refactor, or clean-up code status/2-code-review labels Nov 27, 2019

binary targets do not need the dev environment

ccb0b0a

This makes the binary build targets use a minimal build env instead of having to build all the stuff needed for the full dev enviornment. Signed-off-by: Brian Goff <cpuguy83@gmail.com>

cpuguy83 force-pushed the no_more_dev_tools branch from 1499b27 to f2d171a Compare December 2, 2019 18:13

cpuguy83 added 4 commits December 2, 2019 10:21

Use -X ldflags to set dockerversion package vars

675b414

This eliminates the need to lay down an auto-generated file. IIRC this was originally hadded for gccgo which we no longer support. Signed-off-by: Brian Goff <cpuguy83@gmail.com>

Add support for outputing binaries to custom dir

79a52bb

Signed-off-by: Brian Goff <cpuguy83@gmail.com>

Bind-mount context for Dockerfile build targets

5f314c2

Signed-off-by: Brian Goff <cpuguy83@gmail.com>

Add Dockerfile to .dockerignore

e6d514d

The Dockerfile is not needed in any of the build targets. The one exception may be the dev image, however in most cases the docker source will be bind mounted into the container anyway. Signed-off-by: Brian Goff <cpuguy83@gmail.com>

cpuguy83 force-pushed the no_more_dev_tools branch from f2d171a to e6d514d Compare December 2, 2019 18:22

thaJeztah changed the title ~~Build binaries with minimal deps~~ Build binaries with minimal deps and remove autogen code Dec 3, 2019

thaJeztah approved these changes Dec 12, 2019

View reviewed changes

tonistiigi approved these changes Dec 12, 2019

View reviewed changes

tonistiigi merged commit 550d5f9 into moby:master Dec 12, 2019

This was referenced Jan 14, 2020

Include plugins in static build docker/docker-ce-packaging#415

Closed

[master] Add Support for Centos-8 docker/docker-ce-packaging#416

Merged

hack/make/.go-autogen: fix typo in LDFLAGS variable #40389

Merged

thaJeztah reviewed Jan 17, 2020

View reviewed changes

This was referenced Jan 17, 2020

Fix image based build to address for upstream changes docker/docker-ce-packaging#417

Closed

hack/make/.go-autogen: fix LDFLAGS to accept values with spaces #40393

Merged

AkihiroSuda mentioned this pull request Feb 6, 2020

make binary doesn't build bundles/binary-daemon/vpnkit #40459

Closed

cpuguy83 added a commit to cpuguy83/docker that referenced this pull request Feb 6, 2020

Add vpnkit back to binary targets

82879e9

This was removed after refactoring the Dockerfile in moby#40180 Signed-off-by: Brian Goff <cpuguy83@gmail.com>

cpuguy83 mentioned this pull request Feb 6, 2020

Add vpnkit back to binary targets #40465

Merged

thaJeztah added this to the 20.03.0 milestone Apr 2, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Build binaries with minimal deps and remove autogen code#40180

Build binaries with minimal deps and remove autogen code#40180
tonistiigi merged 5 commits intomoby:masterfrom
cpuguy83:no_more_dev_tools

cpuguy83 commented Nov 5, 2019 •

edited

Loading

Uh oh!

Uh oh!

thaJeztah Nov 5, 2019

Uh oh!

cpuguy83 Nov 5, 2019

Uh oh!

cpuguy83 Nov 5, 2019

Uh oh!

cpuguy83 commented Nov 25, 2019

Uh oh!

thaJeztah left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

cpuguy83 commented Dec 2, 2019

Uh oh!

thaJeztah left a comment

Uh oh!

thaJeztah Dec 3, 2019

Uh oh!

thaJeztah Dec 3, 2019

Uh oh!

thaJeztah Jan 17, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	copy_binaries() {
	local dir="$1"
	local hash="$2"
	# Add nested executables to bundle dir so we have complete set of
	# them available, but only if the native OS/ARCH is the same as the
	# OS/ARCH of the build target
	if [ "$(go env GOOS)/$(go env GOARCH)" != "$(go env GOHOSTOS)/$(go env GOHOSTARCH)" ]; then
	return
	fi
	if [ ! -x /usr/local/bin/runc ]; then
	return
	fi
	echo "Copying nested executables into $dir"
	for file in containerd containerd-shim ctr runc docker-init docker-proxy rootlesskit rootlesskit-docker-proxy dockerd-rootless.sh; do
	cp -f "$(command -v "$file")" "$dir/"
	if [ "$hash" = "hash" ]; then
	hash_files "$dir/$file"
	fi
	done

	# vpnkit is amd64 only
	if command -v "vpnkit.$(uname -m)" 2>&1 >/dev/null; then
	cp -f "$(command -v "vpnkit.$(uname -m)")" "$dir/vpnkit"
	if [ "$hash" = "hash" ]; then
	hash_files "$dir/vpnkit"
	fi
	fi
	}

Conversation

cpuguy83 commented Nov 5, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

thaJeztah Nov 5, 2019

Choose a reason for hiding this comment

Uh oh!

cpuguy83 Nov 5, 2019

Choose a reason for hiding this comment

Uh oh!

cpuguy83 Nov 5, 2019

Choose a reason for hiding this comment

Uh oh!

cpuguy83 commented Nov 25, 2019

Uh oh!

thaJeztah left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

cpuguy83 commented Dec 2, 2019

Uh oh!

thaJeztah left a comment

Choose a reason for hiding this comment

Uh oh!

thaJeztah Dec 3, 2019

Choose a reason for hiding this comment

Uh oh!

thaJeztah Dec 3, 2019

Choose a reason for hiding this comment

Uh oh!

thaJeztah Jan 17, 2020

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

cpuguy83 commented Nov 5, 2019 •

edited

Loading