-
-
Notifications
You must be signed in to change notification settings - Fork 5.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[WIP] Multi-stage (& multi-arch) dockerfile #1615
Conversation
This will need at least docker/moby@v17.05.0-ce-rc2 for multi-stage to work. So this PR is a little in advance. This will dependency is only for building multi-stage. The image resulting will be able to run even with older docker/moby version. |
Dockerfile
Outdated
@@ -1,33 +1,38 @@ | |||
FROM alpine:3.4 | |||
MAINTAINER Thomas Boerger <thomas@webhippie.de> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why remove the Maintainer? Add yourself instead
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's only needed for the published image (last FROM)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And I use the LABEL instruction since MAINTAINER is depraced.
Dockerfile
Outdated
#FROM webhippie/golang:${GO_VERSION} AS build-env | ||
FROM sapk/alpine-multi AS build-env | ||
|
||
ARG TAGS |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe default to sqlite
?
Dockerfile
Outdated
ENV PATH "${PATH}:${GOPATH}/bin" | ||
|
||
RUN apk -U --no-cache add go git build-base \ | ||
&& go get -u github.com/jteeuwen/go-bindata/... |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No need to go get go-bindata
since it will be fetched by make generate
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have re-tested and you are right. I remember that one time it ask for it but don't remember why.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that it was because git
wasn't installed so it couldn't be installed and failed after.
Dockerfile
Outdated
ADD . ${GOPATH}/src/code.gitea.io/gitea | ||
WORKDIR ${GOPATH}/src/code.gitea.io/gitea | ||
|
||
RUN pwd && ls -lah && make clean generate build |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why pwd && ls -lah
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Debug ^^
.dockerignore
Outdated
@@ -1,5 +1,16 @@ | |||
* | |||
!gitea |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this should def. be here if we're building it inside the container now
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Heck, all there things you've added in here should still go into the first build...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At startup docker "clone" local folder to build folder. This limit what is copied into workspace.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After re-thinking if we add all the repo (remove .dockerignore) we could be able to ask to compile a specific version (tag) at build stage via git.
Dockerfile
Outdated
su-exec ca-certificates sqlite bash git linux-pam s6 curl openssh tzdata \ | ||
&& addgroup -S -g ${GID} git \ | ||
&& adduser -S -H -D -h /data/git -s /bin/bash -u ${UID} -G git git \ | ||
&& echo "git:$(date +%s | sha256sum | base64 | head -c 32)" | chpasswd |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
echo "git:$(dd if=/dev/urandom bs=24 count=1 | base64)" | chpasswd
(24 byte => base64 => 32 byte)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I haven't touch that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
right, it was more of a "since you're already screwing with it, might as well fix this"
Dockerfile
Outdated
openssh \ | ||
tzdata && \ | ||
rm -rf \ | ||
/var/cache/apk/* && \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should still remove the cache...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can but with --no-cache it shouldn't be needed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Erm no, completely different things... Don't remove this line...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
https://github.com/gliderlabs/docker-alpine/blob/master/docs/usage.md#disabling-cache It seem, it doesn't even need -U also.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have checked after apk --no-cache -> /var/cache/apk is empty.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ooh, apk --no-cache
, I thought you meant docker build --no-cache
😂
Then it's fine 👍
Makefile
Outdated
@@ -99,11 +99,14 @@ build: $(EXECUTABLE) | |||
|
|||
$(EXECUTABLE): $(SOURCES) | |||
go build $(GOFLAGS) $(EXTRA_GOFLAGS) -tags '$(TAGS)' -ldflags '-s -w $(LDFLAGS)' -o $@ | |||
|
|||
.PHONY: docker-build | |||
docker-build: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this stage still required?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have separate it but still keep it if someone want to build binaries via docker.
I added |
To test against docker version in master : http://blog.alexellis.io/mutli-stage-docker-builds/#howcanitryitout |
Makefile
Outdated
@@ -105,8 +105,9 @@ 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 | |||
|
|||
.PHONY: docker | |||
docker: TAGS=sqlite |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no, ARG TAGS="sqlite"
in the Dockerfile...
At this stage, Dockerfile should be buildable on platform amd64, arm, aarch64 simply by tapping docker build . I am figuring out how to build them in drone and do multi-arch image. |
su-exec ca-certificates sqlite bash git linux-pam s6 curl openssh tzdata \ | ||
&& addgroup -S -g ${GID} git \ | ||
&& adduser -S -H -D -h /data/git -s /bin/bash -u ${UID} -G git git \ | ||
&& echo "git:$(dd if=/dev/urandom bs=24 count=1 | base64)" | chpasswd | ||
|
||
ENV USER git |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reminder to myself : Check if this is usefull since this still run with root after.
@sapk There's no way to build this on the drone server until it's upgraded to 17.05 🙂 |
@bkcsoft we could by using the method explain in http://blog.alexellis.io/mutli-stage-docker-builds/#howcanitryitout but this would make use build last version of docker and start a container with this version in it and build this Dockerfile in it. Not really efficient. We could also do the same without docker building step and re-use release tar.gz |
@sapk I know it's not matter of this PR, but it'd be great if you make a HOW-TO on making a multi arch image, i'm interested in doing so as well. |
@klud1 a multi-arch image is just a fake image linking to various image corresponding for arch (documented in files like https://github.com/sapk/dockerfiles/tree/master/alpine-multi) that you push with github.com/estesp/manifest-tool (you can see command-line example in https://github.com/sapk/dockerfiles/blob/master/.travis.yml that update the refs). And docker pull (like https://hub.docker.com/r/sapk/alpine-multi/tags/) it will choose the corresponding hash/image to pull for his running arch. So you build a image for each arch and reference them in a .yml For the multi-arch part, I use https://github.com/portainer/docker-manifest as a reference and use multiarch/alpine for a good base to have image on most arch. So with my alpine base multi-arch use as base, if you build this Dockerfile, it will/should result in a working on you current arch image. |
@sapk amazing, thank you so much. |
I tested to do a I think that it would be more clean to push this change under Dockerfile.multi and keep the olds ones around until the needed version for docker is ready if is is ok with you. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
otherwise LG-TM
docker/manifest/gitea-1-0-0.yml
Outdated
@@ -0,0 +1,14 @@ | |||
image: sapk/gitea:1.0.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What are all these manifests for?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For generating multi-arch image that reference the image to corresponding arch.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The only part missing is generation of image on arch. This could be done via qemu like https://github.com/multiarch/alpine does.
ae0f493
to
599056b
Compare
We need to waiting for this PR drone-plugins/drone-docker#124 for Drone to support multi-stage build. |
Ok so for a little update. I remove olds Dockerfile. The task The tasks |
It's also possible to choose the version gitea we want in the container via variable |
I will break this into multiple PR to ease the process of validation.
|
Implement multi-stage dockerfile. This will permit cleaner build with the ability to configure build params and maybe if FROM are multi-arch to have only one dockerfile.
After that someone could simply do
docker build .
to build the image locally on every platform. (Not yet ready)