Docker build fix

[gruebel]

This should fix #4076

In general I could offer to clean up your Dockerfile :)

[CLAassistant]

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

[tjamet]

I would personally opt for adding Gopkg.* and running dep ensure -vendor-only instead of dep ensure.
Reason being that dep ensure takes a while (see https://github.com/golang/dep/blob/master/docs/FAQ.md#why-is-dep-slow) and that by nature, dep locks dependencies in a well-known file. We expect the dependencies lock to evolve less often than the code itself hence we can benefit docker build cache when re-building the image after hugo code update.

the following patch works for me:

diff --git i/Dockerfile w/Dockerfile
index 889584b6..119a972f 100644
--- i/Dockerfile
+++ w/Dockerfile
@@ -3,8 +3,9 @@ FROM golang:1.9.0-alpine3.6 AS build
 RUN apk add --no-cache --virtual git musl-dev
 RUN go get github.com/golang/dep/cmd/dep

+ADD Gopkg.* /go/src/github.com/gohugoio/hugo/
 WORKDIR /go/src/github.com/gohugoio/hugo
-RUN dep ensure
+RUN dep ensure -vendor-only
 ADD . /go/src/github.com/gohugoio/hugo/
 RUN go install -ldflags '-s -w'

[gruebel]

No problem, I added the -vendor-only flag. I also bumped up the build stage docker base image to 1.9.2.

In general I reformated the whole Dockerfile, while honoring the offical Dockerfile best practices.

• COPY instead of ADD (see https://docs.docker.com/engine/userguide/eng-image/dockerfile_best-practices/#add-or-copy)
• removed the --virtual flag from the apk add command, because there is no point in using it, when you are not planing to remove the dependencies afterwards

[tjamet]

Why using multiple lines and a single layer of cache here?

I find short single lines more readable personally

[gruebel]

I always use multi-line for package listing, it makes it much easier to read and also PR to review. In the offical Dockerfile best practice is a short passage about sorting multi-line, which points already in the right direction of using multi-line in general for package listing https://docs.docker.com/engine/userguide/eng-image/dockerfile_best-practices/#sort-multi-line-arguments

I definitley agree on using a oneliner, if you just install one package, but with two I prefer to use multi-line and starting with three there is no other choice, then going multi-line.

Regarding the merge of the two layers, it is just about saving a bit of storage, few kbs, https://docs.docker.com/engine/userguide/eng-image/dockerfile_best-practices/#minimize-the-number-of-layers and speed, because Docker doesn't have to save the intermediate layer.

68667e787a6e        24 hours ago        /bin/sh -c go get github.com/golang/dep/cm...   39MB
5eb1bd0ea57e        24 hours ago        /bin/sh -c apk add --no-cache     git     ...   31.6MB

7218153155d7        24 hours ago         /bin/sh -c apk add --no-cache     git     ...   70.5MB

And lastly it's about constistancy, if you look at the final stage of the Dockerfile, there is both used multi-line package listing and merging two layers ;)

hugo/Dockerfile

Lines 12 to 15 in 42fbf15

	RUN \
	adduser -h /site -s /sbin/nologin -u 1000 -D hugo && \
	apk add --no-cache \
	dumb-init

[tjamet]

I don't think the consistency nor the size are adding value here.

We are in the middle of the 'compilation' stage of a multi-stage build, both stages solves different problems.
At this stage, layers stays on the builder host and will never be pushed, compared to the latest stage where layers are actually pushed and where it is interesting to lower their number to gain on the image size.
At the same time, having more layers during this stage adds more steps for the user to use cache, interesting when it comes to re-building an image

However, I don't think that, in the current case, it has a significative impact, just a general thinking about guidelines and spirit of guidelines :)

[gruebel]

I think in the end it depends totally one personal taste.

The build stage is just garbage stage where you put all in, which you don't want to use in your final image, but need to make it.

And you are right, which ever approach we choose here it has almost no impact :D

So anyone else has an opinion on it? Then he/she could decide, which way to go.

[rmetzler]

The current master has a defunct Dockerfile. I have the same problem as #4076

I wonder why this PR hasn't merged yet and I guess it's because it does more than just fixing the bug.
Can we maybe have the bugfix and discuss the other topics later?

[ellerbrock]

@gruebel fully agree with your answers and docker best practices!
i just saw your pr after i run in the problem with the build myself yesterday night and made a fix for it: #4154

done here similar like you docker best practices, added multilines and changed ADD to COPY and updated alpine to the latest version.

after that i made a second pr with the production hugo image from scratch: #4155
and created a dockerhub and quay account to hopefully automate this process in the future.

hope that the maintainer (ping @anthonyfok) would see the need for having a automated official hugo image for the community.

anyway, just wanted to say that i'm totally on your side with the answers what you said about docker build and best practices.

cheers maik

[skoblenick]

Where is this PR at? I have seen the other optimization PR's but the immediate concern should be to fix the existing Dockerfile.

[github-actions]

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.