Skip to content
This repository has been archived by the owner on Jun 20, 2024. It is now read-only.

Use alpine docker file #1

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Conversation

justgage
Copy link

I'm no docker expert by any means but it seems this works locally, this should reduce the download size by a great amount.

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
Copy link

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.

Copy link
Author

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
Copy link

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 RUNs into one layer like the existing image?

Copy link
Author

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.

Copy link
Author

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

Copy link
Author

@justgage justgage Feb 27, 2018

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

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.

@justgage
Copy link
Author

justgage commented Mar 1, 2018

I'm sure you're busy but when you have time I think this is ready for another code review @westy92

@justgage
Copy link
Author

justgage commented Mar 2, 2018

@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.

@westy92
Copy link

westy92 commented Mar 2, 2018

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

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

Copy link
Author

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)?

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

Copy link
Author

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

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants