Docker multi-stage #2927

sapk · 2017-11-16T23:06:05Z

Permit to do directly docker build . in repo root. since binary is build at first stage and COPY in the exported image. Resolve #2879

In addition :

permit to chose the version of the binary build via --build-arg GITEA_VERSION=v1.2.3
permit to chose the tags of the binary build via --build-arg TAGS="bindata sqlite"

I kept make docker-build if some people use docker to build the binary.

codecov-io · 2017-11-16T23:13:44Z

Codecov Report

Merging #2927 into master will increase coverage by 0.01%.
The diff coverage is n/a.

@@            Coverage Diff            @@
##           master   #2927      +/-   ##
=========================================
+ Coverage   35.89%   35.9%   +0.01%     
=========================================
  Files         286     286              
  Lines       41312   41312              
=========================================
+ Hits        14830   14835       +5     
+ Misses      24300   24297       -3     
+ Partials     2182    2180       -2

Impacted Files	Coverage Δ
models/repo_indexer.go	`49.57% <0%> (+1.27%)`	⬆️
models/repo_list.go	`67.18% <0%> (+1.56%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b333e71...25d4d4f. Read the comment docs.

appleboy · 2017-11-17T00:56:37Z

docker/Makefile

+
+.PHONY: docker-build
+docker-build:
+	docker run -ti --rm -v $(CURDIR):/srv/app/src/code.gitea.io/gitea -w /srv/app/src/code.gitea.io/gitea -e TAGS="bindata $(TAGS)" webhippie/golang:edge make clean generate build


remove -ti?

make generate can use go get that can ask the user for git access maybe ? Don't really know, I kept it as before only if some people use it for building the binary inside docker (if go is not available on host). But looking more at it we could totally remove docker-build target ?

And even if this target will stay in Makefile, I think it would be reasonable to switch it to golang:1.9-alpine3.6 with content trust enabled as well.

@0rzech for docker-build it is not possible golang:1.9-alpine3.6 is missing some build tools. I get them in the build stage of the Dockerfile.

@sapk You're right. I forgot it misses gcc, git, make and musl-dev.

lunny · 2017-11-17T03:18:33Z

@appleboy maybe this should be in v1.4.0?

appleboy · 2017-11-17T03:21:57Z

@lunny OK. changed.

0rzech

The .dockerignore file should not be removed, but repurposed to exclude anything not needed to compile the binary. Otherwise everything will be unnecessarily sent to Docker build context, thus will reduce build performance.

sapk · 2017-12-02T02:13:03Z

@0rzech I know this is intended to be able to build any version/tag/commit via build arg.

0rzech

AFAIU this and this, netgo build tag is no longer necessary and is also redundant due to ENV GODEBUG=netdns=go presence in Dockerfile (which seems no longer necessary either, btw).

sapk · 2017-12-04T00:23:38Z

@0rzech good catch, I will clean it up and rebase on your PR(that has been merge) for content trust.

sapk · 2017-12-06T23:01:08Z

@0rzech netgo tag removed.

0rzech · 2017-12-07T02:27:51Z

Dockerfile

+ARG TAGS="bindata sqlite"
+
+#Build deps
+RUN apk --no-cache add git build-base


I think it is always better to sort packages alphabetically. Not a blocker.

0rzech · 2017-12-07T02:31:49Z

Dockerfile

+WORKDIR ${GOPATH}/src/code.gitea.io/gitea
+
+#Checkout version if set
+RUN if [ -n "${GITEA_VERSION}" ]; then git checkout ${GITEA_VERSION}; fi \


AFAIR, when you don't write RUN set -eu; <some other commands>, each run will exit without error. The build will fail anyways of course, but stdout will be polluted with more errors due to each subsequent command printing fails instead of first failed only. Not a blocker.

0rzech · 2017-12-07T02:34:25Z

@sapk Thanks. I've made two more suggestions, but it is your choice whether to apply them or not.

0rzech

LGTM

0rzech · 2017-12-07T02:43:21Z

Dockerfile


 VOLUME ["/data"]

 ENTRYPOINT ["/usr/bin/entrypoint"]
 CMD ["/bin/s6-svscan", "/etc/s6"]

 COPY docker /
-COPY gitea /app/gitea/gitea
+COPY --from=build-env /go/src/code.gitea.io/gitea/gitea /app/gitea/gitea


This way gitea binary will become writable for user git. This is out of scope for this PR - I'm just writing it for future reference. Not a blocker.

Are you sure ? I haven't tested but that should be root the owner and permissions should be the same as go build result.

I'm sure. It's because of this.

Ok that should be fix but it was already the case before and not introduce by this PR.

I agree. This is exactly why I wrote:

This is out of scope for this PR - I'm just writing it for future reference. Not a blocker.

;)

Sorry misread, and thinking that specific line introduce that XD.

No problem. ;)

lafriks · 2017-12-09T16:19:49Z

LGTM

0rzech · 2017-12-09T16:23:27Z

Dockerfile

+
+###################################
+#Build stage
+FROM golang:alpine AS build-env


It is a bad practice not to fix on specific version number. Besides, golang:alpine is the same as golang:1.9-alpine3.6. You can change it to alpine:3.7 and install go package with apk from Alpine's community repository.

I know, I hesitate on that part and thinking that we it's not that bad since it is only for build.
You are rigth, I will fix to 3.7 and use in go alpine packages (1.9.2) and when golang:1.10-alpine-3.7 is release I will update. It's too bad that official image doesn't offer a golang:1.9-alpine-3.7 tag.

A golang:1.9-alpine-3.7 tag should be release soon. ^^

0rzech

LGTM

0rzech · 2017-12-10T01:06:17Z

Dockerfile


 ARG GITEA_VERSION
 ARG TAGS="sqlite"
 ENV TAGS "bindata $TAGS"

+#Temporary in wait of golang:X.XX-alpine-3.7


We wait for something, not wait of. ;) Not a blocker.

Should be release monday on docker hub. Could be merge before and fix after. ^^

lafriks · 2017-12-10T10:55:09Z

@sapk please resolve conflicts

sapk · 2017-12-26T00:02:12Z

Codacy is complaining about maintainers because doesn't contains email. but I don't think we have a email address for it.

lafriks · 2018-01-16T21:26:57Z

@sapk please use maintainers@gitea.io for that

sapk · 2018-01-16T23:42:05Z

@lafriks Done + squashed and rebased

lafriks · 2018-01-18T05:15:54Z

@sapk still fails

Codacy don't seems to follow https://docs.docker.com/engine/reference/builder/#maintainer-deprecated

sapk · 2018-01-18T13:39:06Z

@lafriks I can't figure out what codacy wants but it doesn't validate the format documented here : https://docs.docker.com/engine/reference/builder/#maintainer-deprecated.
I follow the docker documentation please ignore Codacy error.

lafriks · 2018-01-18T14:05:43Z

Maybe it does not understand new format

jagu-sayan · 2018-02-28T10:45:04Z

@sapk @lafriks We can ignore this error. Fixed on hadolint (Dockerfile linter used by codacy) in v1.2.3. But it seems that Codacy use an older version.

tboerger added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Nov 16, 2017

appleboy reviewed Nov 17, 2017

View reviewed changes

appleboy added this to the 1.3.0 milestone Nov 17, 2017

appleboy added the topic/build PR changes how Gitea is built, i.e. regarding Docker or the Makefile label Nov 17, 2017

appleboy modified the milestones: 1.3.0, 1.4.0 Nov 17, 2017

0rzech suggested changes Dec 2, 2017

View reviewed changes

0rzech approved these changes Dec 2, 2017

View reviewed changes

0rzech suggested changes Dec 2, 2017

View reviewed changes

0rzech mentioned this pull request Dec 3, 2017

Gitea 1.3.0 fails to start #3052

Closed

sapk force-pushed the docker-multi-stage branch from 4b009e9 to 2b2a0e4 Compare December 6, 2017 22:57

0rzech reviewed Dec 7, 2017

View reviewed changes

0rzech approved these changes Dec 7, 2017

View reviewed changes

0rzech reviewed Dec 7, 2017

View reviewed changes

0rzech mentioned this pull request Dec 8, 2017

Update alpine to 3.7 #3121

Merged

sapk force-pushed the docker-multi-stage branch from 379830c to f13f14f Compare December 9, 2017 16:10

tboerger added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Dec 9, 2017

0rzech suggested changes Dec 9, 2017

View reviewed changes

0rzech approved these changes Dec 10, 2017

View reviewed changes

sapk force-pushed the docker-multi-stage branch from 3fa0761 to 4fddeff Compare December 25, 2017 23:01

sapk force-pushed the docker-multi-stage branch 5 times, most recently from 10ee7df to fb817e3 Compare January 16, 2018 23:30

Setup docker multi-stage and little sugar at it

94ca7e0

sapk force-pushed the docker-multi-stage branch from fb817e3 to 94ca7e0 Compare January 16, 2018 23:46

sapk added 2 commits January 18, 2018 12:17

Make codacy happy ?

1fa605e

Revert back to what the official docker documentation suggest

1dcd41e

Codacy don't seems to follow https://docs.docker.com/engine/reference/builder/#maintainer-deprecated

lafriks modified the milestones: 1.4.0, 1.5.0 Jan 22, 2018

sapk mentioned this pull request Feb 1, 2018

Official arm/rpi docker image #2607

Closed

whatever4711 mentioned this pull request Mar 6, 2018

Enable Multiarchitecture build for Docker Container #3633

Closed

sapk added 2 commits March 12, 2018 10:14

Merge branch 'master' into docker-multi-stage

3825a13

Update golang version

25d4d4f

lafriks approved these changes Mar 12, 2018

View reviewed changes

appleboy approved these changes Mar 12, 2018

View reviewed changes

tboerger added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Mar 12, 2018

lafriks merged commit 0e26db8 into go-gitea:master Mar 12, 2018

sapk deleted the docker-multi-stage branch March 12, 2018 10:00

go-gitea locked and limited conversation to collaborators Nov 24, 2020

Docker multi-stage #2927

Docker multi-stage #2927

Conversation

sapk commented Nov 16, 2017 • edited

codecov-io commented Nov 16, 2017 • edited

Codecov Report

Choose a reason for hiding this comment

sapk Nov 17, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

0rzech Dec 2, 2017 • edited

Choose a reason for hiding this comment

lunny commented Nov 17, 2017

appleboy commented Nov 17, 2017

0rzech left a comment • edited

Choose a reason for hiding this comment

sapk commented Dec 2, 2017 • edited

0rzech left a comment • edited

Choose a reason for hiding this comment

sapk commented Dec 4, 2017

sapk commented Dec 6, 2017

0rzech Dec 7, 2017 • edited

Choose a reason for hiding this comment

0rzech Dec 7, 2017 • edited

Choose a reason for hiding this comment

0rzech commented Dec 7, 2017

0rzech left a comment

Choose a reason for hiding this comment

0rzech Dec 7, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

0rzech Dec 7, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

0rzech Dec 7, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lafriks commented Dec 9, 2017

0rzech Dec 9, 2017 • edited

Choose a reason for hiding this comment

sapk Dec 9, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

0rzech left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lafriks commented Dec 10, 2017

sapk commented Dec 26, 2017

lafriks commented Jan 16, 2018

sapk commented Jan 16, 2018 • edited

lafriks commented Jan 18, 2018

sapk commented Jan 18, 2018

lafriks commented Jan 18, 2018

jagu-sayan commented Feb 28, 2018

sapk commented Nov 16, 2017 •

edited

codecov-io commented Nov 16, 2017 •

edited

sapk Nov 17, 2017 •

edited

0rzech Dec 2, 2017 •

edited

0rzech left a comment •

edited

sapk commented Dec 2, 2017 •

edited

0rzech left a comment •

edited

0rzech Dec 7, 2017 •

edited

0rzech Dec 7, 2017 •

edited

0rzech Dec 7, 2017 •

edited

0rzech Dec 7, 2017 •

edited

0rzech Dec 7, 2017 •

edited

0rzech Dec 9, 2017 •

edited

sapk Dec 9, 2017 •

edited

sapk commented Jan 16, 2018 •

edited