-
Notifications
You must be signed in to change notification settings - Fork 201
Use alpine docker file #1
base: master
Are you sure you want to change the base?
Conversation
Dockerfile
Outdated
useradd -ms /bin/bash freegeoip | ||
WORKDIR /go/src/github.com/apilayer/freegeoip/cmd/freegeoip | ||
RUN apk update | ||
RUN apk add git libcap shadow |
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.
RUN apk add --no-cache git libcap shadow
Removes the need to run apk update
and to remove package lists after.
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.
Nice! Didn't know that.
Dockerfile
Outdated
WORKDIR /go/src/github.com/apilayer/freegeoip/cmd/freegeoip | ||
RUN apk update | ||
RUN apk add git libcap shadow | ||
RUN go get -d |
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 not group all of these RUN
s into one layer like the existing image?
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.
Splitting it up (from my limited knowlege) allows if you change the lower ones the upper ones will be cached, meaning you won't have to run the higher ones nearly as often. Perhaps I missunderstood that though.
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.
In my most recent PR I've organized it so that it should cache installing the depenecies (changes infrequently). I found what helped the most was moving the COPY down. However It seems from this discussion that there's tradofs of caching for file size differences: https://stackoverflow.com/questions/39223249/multiple-run-vs-single-chained-run-in-dockerfile-what-is-better
|
||
EXPOSE 8080 | ||
|
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 deleted these comments because I don't think they were bringing much value, however I can put them back due to that being outside the scope of this PR
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.
@justgage there is a great Dockerfile best practices article here. I thought I would pass it along.
https://docs.docker.com/develop/develop-images/dockerfile_best-practices/
Following these recommendations, specifically for RUN, it will create a more readable Dockerfile:
FROM golang:1.9-alpine
RUN apk add --no-cache \
git \
libcap \
shadow \
&& addgroup -g 1000 -S freegeoip \
&& adduser -u 1000 -S freegeoip -G freegeoip
COPY cmd/freegeoip/public /var/www
COPY . /go/src/github.com/apilayer/freegeoip
WORKDIR /go/src/github.com/apilayer/freegeoip/cmd/freegeoip
RUN go get -d \
&& go install \
&& setcap cap_net_bind_service=+ep /go/bin/freegeoip
ENTRYPOINT ["/go/bin/freegeoip"]
USER freegeoip
EXPOSE 8080
Notice that the ADD was changed to a COPY as per the recommendation here on transparency.
I'm sure you're busy but when you have time I think this is ready for another code review @westy92 |
@westy92 thanks for approving it. I don't have write access to hit merge but perhaps you're waiting to do that anyway, not sure. |
I have no authority here, merely a user trying to provide valuable input. |
Dockerfile
Outdated
WORKDIR /go/src/github.com/apilayer/freegeoip/cmd/freegeoip | ||
RUN apk update | ||
RUN apk add git libcap shadow | ||
RUN go get -d |
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.
A rebuild produced this error
Step 10/13 : RUN useradd -ms /bin/bash freegeoip
---> Running in afcee3ec8484
Creating mailbox file: No such file or directory
but otherwise image rebuilt and installed OK fully functioning on :80
still outstanding issue on :443 #3
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.
Yeah, I looked up that error hoping I could fix it, however after my reserach it seemed like there wasn't a great way to fix it, and it doesn't cause issues. I could create the directory that it's trying to access but I was afraid that would only add to the image size (not sure of that though)?
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.
According to this ticket, mhart/alpine-node#48, the correct way to add a user in Apline 3.7 is:
RUN \
addgroup -g 1000 -S freegeoip && \
adduser -u 1000 -S freegeoip -G freegeoip
The base image golang:1.9-alpine
is currently running 3.6.2, but this still seems to correctly create the home directory:
echo $HOME
/home/freegeoip
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.
followed @dsperling advice, seems the errors are gone @OlagStegan
I'm no docker expert by any means but it seems this works locally, this should reduce the download size by a great amount.