Allow building from source whilst still keeping container size down #10

lag-linaro · 2018-11-01T13:59:17Z

Fixes: #7
Fixes: docker-library/official-images#4718

Commit 3246cae (Notary 0.6.1, and rather than commit binaries,
build using multistage dockerfiles.) provided support for building
binaries from source, instead of introducing them into the repo. It
also introduced multi-stage builds into the project.

Multi-stage builds allow us to supply all of the build requirements
into disposable build-containers. Once finished with, only the
required build artifacts are copied out into a nice succinct run-
container. This results in a container with a much smaller
footprint.

The issue is; Docker's Official Images platform does not yet support
multi-stage builds. So until they do, we need to find a source of
compromise.

Here we start with a small base (Alpine), install only the build
requirements we need, build from source, then once complete clean-up
any unnecessary packages. This both allows for building from source
AND keeps the final run-container nice and small.

Signed-off-by: Lee Jones [email protected]

yosifkit · 2018-11-02T21:26:47Z

notary-server/Dockerfile

@@ -1,25 +1,35 @@
-FROM golang:1.10.3-alpine
+FROM alpine:latest


This should be a specific version of Alpine, like 3.8, to prevent possible incompatibilities (like when they switch ssl back to openssl).

Thanks. Will fix.

tianon · 2018-11-02T21:55:38Z

notary-server/Dockerfile

+		grep -A 5 ${TAG} | grep sha | \
+		awk '{print substr($2,2,8)}'` \
+		-X ${NOTARYPKG}/version.NotaryVersion=`cat NOTARY_VERSION`" \
+	${NOTARYPKG}/cmd/notary-server && \


This scraping via wget seems really hacky -- why are we going to all this trouble?

If it's important for version.GitCommit to be set to a value that isn't simple like $TAG (which is technically a valid Git ref, and is the ref we're building from), then why not instead install git, do a git clone --depth 1, and then use the existing Makefile to let the upstream build set the values appropriately itself?

Here's an example Dockerfile I've successfully tested (on both amd64 and arm64) that runs and generates a ~21MB image (~4MB for alpine:3.8 and ~17MB for notary-server), and provides the expected values in notary-server -debug:

FROM alpine:3.8 RUN adduser -D -H -g "" notary ENV PATH=$PATH:/notary/server WORKDIR /notary/server ENV NOTARYPKG github.com/theupdateframework/notary ENV TAG v0.6.1 RUN set -eux; \ \ apk add --no-cache --virtual .build-deps git go make musl-dev; \ \ export GOPATH=/go GOCACHE=/go/cache; \ mkdir -p "$GOPATH/src/$NOTARYPKG"; \ git clone -b "$TAG" --depth 1 "https://$NOTARYPKG" "$GOPATH/src/$NOTARYPKG"; \ \ make -C "$GOPATH/src/$NOTARYPKG" PREFIX=. ./bin/static/notary-server; \ cp -vL "$GOPATH/src/$NOTARYPKG/bin/static/notary-server" ./; \ rm -rf "$GOPATH"; \ \ apk del --no-network .build-deps EXPOSE 4443 COPY ./server-config.json . COPY ./entrypoint.sh . USER notary ENTRYPOINT [ "entrypoint.sh" ] CMD [ "notary-server", "--help" ]

I also don't understand why this and #11 are separate PRs (since it's effectively the same exact change), but the same approach will likely work in exactly the same way over on notary-signer.

... the same approach will likely work in exactly the same way over on notary-signer.

Confirmed -- copied, replaced all server with signer and everything appears to work.

One step at a time. This PR moves everything in the right direction (removes multi-stage builds) - it does not make things perfect. In the projects I normally work in, commits are expected to contain only one functional change.

They are separate PRs because I wanted to see if the change would be well received before spending even more effort on something that would have been NACKed. Ultimately I'm happy to merge them.

…size down Commit 3246cae (Notary 0.6.1, and rather than commit binaries, build using multistage dockerfiles.) provided support for building binaries from source, instead of introducing them into the repo. It also introduced multi-stage builds into the project. Multi-stage builds allow us to supply all of the build requirements into disposable build-containers. Once finished with, only the required build artifacts are copied out into a nice succinct run- container. This results in a container with a much smaller footprint. The issue is; Docker's Official Images platform does not yet support multi-stage builds. So until they do, we need to find a source of compromise. Here we start with a small base (Alpine), install only the build requirements we need, build from source, then once complete clean-up any unnecessary packages. This both allows for building from source AND keeps the final run-container nice and small. Signed-off-by: Lee Jones <[email protected]>

Signed-off-by: Lee Jones <[email protected]>

yosifkit · 2018-11-06T00:48:17Z

LGTM from the official-images side.

lag-linaro · 2018-11-06T07:27:36Z

Thanks for your time @yosifkit. Much appreciated.

lag-linaro · 2018-11-14T09:51:16Z

@justincormack now that the Official Images guys are happy, would you mind re-reviewing please?

justincormack · 2018-11-14T10:00:02Z

notary-server/Dockerfile

@@ -1,25 +1,28 @@
-FROM golang:1.10.3-alpine
+FROM alpine:3.8


I think its better to use eg golang/1.10.5-alpine3.8 so we pin the Go version and the Alpine version, rather than installing the Go that Alpine happens to have at present.

By doing so you will increase the size of the image from 21MB to 268MB.

We could do 'apk add go=1.10.5' if you'd prefer?

Ah ugh yes. I am just trying to get Alpine updated with the latest Go 1.10 patch release, as it is currently only on 1.10.1.

That would be a better solution.

Although it doesn't seem to affect Notary. It tested just fine with Apline 3.8.

So when installing this PR I see that Apline 3.8 installs Go version 1.10.5-r0.

(19/22) Installing go (1.10.5-r0)

Is that not adequate?

Yes, Its fixed now, I got an updated version pushed yesterday, all good...

lag-linaro · 2018-11-15T13:04:05Z

Thanks @justincormack, you're a star.

Do you want me to raise a PR on Official Images saying that we now support AArch64?

I'm happy to do it if you do not have time. Can tag you as a reviewer too.

lag-linaro · 2018-11-15T13:06:43Z

It's also required to bump to 0.6.1.

Official Images is still at 0.5.0.

justincormack · 2018-11-15T13:15:11Z

@lag-linaro I think I need to re-tag the v0.6.1 release that we did in this repo, as it has the previous version in. And not sure exactly what we need in the replacement for docker-library/official-images#4718 maybe @tianon or @yosifkit can help...

lag-linaro · 2018-11-15T13:20:33Z

@justincormack this and #11 PRs fix @tianon and @yosifkit reviews.

We can now re-submit docker-library/official-images#4718.

This is what I was proposing in #10 (comment).

justincormack · 2018-11-15T13:26:05Z

I think docker-library/official-images#4718 would at least need to list the architectures to build for. Let me re-tag this repo as well now.

lag-linaro · 2018-11-15T13:36:35Z

Absolutely. In order to provide additional arches, we need to supply an Architectures: tag.

lag-linaro · 2018-11-15T13:43:24Z

See HERE

Let me know if you'd like me to submit it.

justincormack · 2018-11-15T13:45:22Z

Ok, I pushed a new v0.6.1 tag to this repo and deleted the old one, so yes that should be good to submit I think.

lag-linaro · 2018-11-15T13:48:05Z

Great. That's sent.

It will help expedite the process if you could review and provide your LGTM.

justincormack approved these changes Nov 1, 2018

View reviewed changes

This was referenced Nov 2, 2018

0.6.0 is out! #4

Closed

more architectures in release builds notaryproject/notary#1380

Closed

Update Notary framework to use theupdateframework #9

Closed

Update notary server and signer images to 0.6.1 docker-library/official-images#4718

Closed

yosifkit reviewed Nov 2, 2018

View reviewed changes

tianon reviewed Nov 2, 2018

View reviewed changes

lag-linaro force-pushed the remove-multi-stage-builds branch from 464cabd to fe39657 Compare November 5, 2018 08:43

Lee Jones added 3 commits November 5, 2018 08:44

update.sh is superfluous now binaries are not pre-built

74297dc

Signed-off-by: Lee Jones <[email protected]>

lag-linaro force-pushed the remove-multi-stage-builds branch from fe39657 to 74297dc Compare November 5, 2018 08:44

This was referenced Nov 5, 2018

update.sh is superfluous now binaries are not pre-built #12

Closed

Allow building the signer from source whilst still keeping container size down #11

Closed

Lee Jones added 4 commits November 5, 2018 11:04

Use Git instead of Wget to pull down server source

6f69770

Signed-off-by: Lee Jones <[email protected]>

Use Git instead of Wget to pull down signer source

160d513

Signed-off-by: Lee Jones <[email protected]>

Use 'set -e' to stop on error instead of '&&'ing each subsequent command

dd6486a

Signed-off-by: Lee Jones <[email protected]>

Use APK's Virtual Packages functionality for easy clean-up

2c6f77a

Signed-off-by: Lee Jones <[email protected]>

lag-linaro mentioned this pull request Nov 6, 2018

Notary on AARCH64 #7

Closed

justincormack reviewed Nov 14, 2018

View reviewed changes

justincormack approved these changes Nov 15, 2018

View reviewed changes

justincormack merged commit 6db2078 into docker:master Nov 15, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow building from source whilst still keeping container size down #10

Allow building from source whilst still keeping container size down #10

lag-linaro commented Nov 1, 2018

yosifkit Nov 2, 2018

lag-linaro Nov 5, 2018

tianon Nov 2, 2018

tianon Nov 2, 2018

lag-linaro Nov 5, 2018

yosifkit commented Nov 6, 2018

lag-linaro commented Nov 6, 2018

lag-linaro commented Nov 14, 2018

justincormack Nov 14, 2018

lag-linaro Nov 14, 2018

lag-linaro Nov 14, 2018

justincormack Nov 14, 2018

lag-linaro Nov 14, 2018

lag-linaro Nov 15, 2018

justincormack Nov 15, 2018 •

edited

Loading

lag-linaro commented Nov 15, 2018 •

edited

Loading

lag-linaro commented Nov 15, 2018

justincormack commented Nov 15, 2018

lag-linaro commented Nov 15, 2018

justincormack commented Nov 15, 2018

lag-linaro commented Nov 15, 2018

lag-linaro commented Nov 15, 2018

justincormack commented Nov 15, 2018

lag-linaro commented Nov 15, 2018

		@@ -1,25 +1,35 @@
		FROM golang:1.10.3-alpine
		FROM alpine:latest

Allow building from source whilst still keeping container size down #10

Allow building from source whilst still keeping container size down #10

Conversation

lag-linaro commented Nov 1, 2018

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

yosifkit commented Nov 6, 2018

lag-linaro commented Nov 6, 2018

lag-linaro commented Nov 14, 2018

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

justincormack Nov 15, 2018 • edited Loading

Choose a reason for hiding this comment

lag-linaro commented Nov 15, 2018 • edited Loading

lag-linaro commented Nov 15, 2018

justincormack commented Nov 15, 2018

lag-linaro commented Nov 15, 2018

justincormack commented Nov 15, 2018

lag-linaro commented Nov 15, 2018

lag-linaro commented Nov 15, 2018

justincormack commented Nov 15, 2018

lag-linaro commented Nov 15, 2018

justincormack Nov 15, 2018 •

edited

Loading

lag-linaro commented Nov 15, 2018 •

edited

Loading