-
-
Notifications
You must be signed in to change notification settings - Fork 7
make image (much) smaller #34
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,11 +1,14 @@ | ||
| FROM golang:1.17.6 | ||
| FROM golang:1.17.6 as build | ||
| ARG COMMIT="latest" | ||
|
|
||
| RUN mkdir -p /heartbeat/config | ||
| ADD . /heartbeat | ||
| WORKDIR /heartbeat | ||
| RUN mkdir -p /heartbeat/config | ||
| COPY . . | ||
|
|
||
| ENV PATH "${PATH}:${GOPATH}/bin" | ||
| RUN make deps build | ||
|
|
||
| CMD /heartbeat/heartbeat | ||
| FROM gcr.io/distroless/base-debian11 | ||
| COPY --from=build /heartbeat/heartbeat /heartbeat/heartbeat | ||
| COPY --from=build /heartbeat/www /heartbeat/www | ||
| WORKDIR /heartbeat | ||
| ENTRYPOINT [ "/heartbeat/heartbeat" ] | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -16,14 +16,7 @@ build: generate | |
|
|
||
| deps: | ||
| go install github.com/valyala/quicktemplate/qtc | ||
| go get -u github.com/ferluci/fast-realip | ||
| go get -u github.com/go-redis/redis/v8 | ||
| go get -u github.com/joho/godotenv | ||
| go get -u github.com/nitishm/go-rejson/v4 | ||
| go get -u github.com/valyala/fasthttp | ||
| go get -u github.com/valyala/quicktemplate | ||
| go get -u golang.org/x/text/language | ||
| go get -u golang.org/x/text/message | ||
| go get -u ./... | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Out of curiosity, is there documentation on what this actually does? I assume it just pulls everything needed as specified in the
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. from https://go.dev/ref/mod#go-get
it's just easier to do this than keep the list up to date here as well.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ohh okay. I will resolve this specific change once I test locally to see if it does do |
||
|
|
||
| docker-build: | ||
| @docker build --build-arg COMMIT=${TAG} -t ${IMG} . | ||
|
|
||
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.
Does this still create the heartbeat folder inside of
/dataas needed? Including the empty folders?Mainly, we need the contents of
/heartbeat/configinside of/heartbeatso when it is mounted, the config files reside inside of/data/heartbeatThere 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 data path is deliberately not created because it's intended to be a volume mount. at the time of writing, i did not know that the
VOLUMEinstruction exists. making the config directory avoids permission issues when mounting files alone. it's not the best solution, admittedly, but hardening is probably for another 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.
Ah okay. I'm also unfamiliar with
VOLUME, we do need to make changes to the internal server in order for it to work with the configuration that you have here? As far as I can tell?Please let me know if I'm mistaken or if there's something I'm missing.
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 do remember testing this at the time, and it didn't seem to require any further modification. I'll test this again as soon as possible, but I don't think I'll have time until February. I see that there is a merge conflict so I might have failed to notice changes in the meantime.