Updated Dockerfile in preparation for public Docker image#826
Updated Dockerfile in preparation for public Docker image#826maciejdudko merged 2 commits intotemporalio:mainfrom
Conversation
9cc8491 to
ce05c0e
Compare
| RUN set -x && apt-get update && DEBIAN_FRONTEND=noninteractive apt-get install -y \ | ||
| ca-certificates && \ | ||
| rm -rf /var/lib/apt/lists/* |
There was a problem hiding this comment.
Can you confirm that this container has the alpine equivalent of ca-certificates?
There was a problem hiding this comment.
ca-certificates exists in Alpine repo but is missing in the image. I will push a new revision that has this fixed.
| FROM --platform=$BUILDARCH scratch AS dist | ||
| COPY ./dist/nix_linux_amd64_v1/temporal /dist/amd64/temporal | ||
| COPY ./dist/nix_linux_arm64/temporal /dist/arm64/temporal | ||
|
|
||
| WORKDIR /app | ||
| FROM alpine:3.22 | ||
| ARG TARGETARCH | ||
| COPY --from=dist /dist/$TARGETARCH/temporal /usr/local/bin/temporal |
There was a problem hiding this comment.
Can't we do this in one step? Why do we need a separate diststep?
There was a problem hiding this comment.
The dist step acts as a manual per-architecture mapping to the right binary. Docker doesn't support conditional COPY so due to the differences in Go and Docker target naming, it would be complicated to do in a single step.
There was a problem hiding this comment.
Not sure I follow, can't this step be COPY ./dist/$TARGETARCH/temporal /usr/local/bin/temporal?
There was a problem hiding this comment.
It won't work because the directory name is in the wrong format (arch is amd64_v1 in Goreleaser but amd64 in Docker).
There was a problem hiding this comment.
So use a build arg to say where to copy from instead?
There was a problem hiding this comment.
It wouldn't work for building multiplatform image with a single docker build command as all platforms would receive the same argument. I guess we could have a Docker bake file or a shell script wrapper or something but I think we should wait with that kind of stuff until we have proper release automation so that we don't do any throwaway work, and keep this PR as simple as possible. Especially since the extra step doesn't affect the final container image.
| COPY ./dist/nix_linux_arm64/temporal /dist/arm64/temporal | ||
|
|
||
| WORKDIR /app | ||
| FROM alpine:3.22 |
There was a problem hiding this comment.
Doesn't this also need a --platform flag?
There was a problem hiding this comment.
No. --platform defaults to $TARGETPLATFORM, which is what we want here. dist uses $BUILDPLATFORM so it can be potentially cached for multiplatform builds.
| COPY ./dist/nix_linux_arm64/temporal /dist/arm64/temporal | ||
|
|
||
| WORKDIR /app | ||
| FROM alpine:3.22 |
There was a problem hiding this comment.
So we don't test against this OS very well FWIW, though Go is self-contained and doesn't use libc/musl so it should be fine. But just curious why alpine instead of e.g. debian:bookworm-slim or maybe even gcr.io/distroless/static-debian12 or others often seen in the wild? It seems the latter (https://github.com/GoogleContainerTools/distroless) is quite common for this use case.
There was a problem hiding this comment.
I was basing the image on our admin-tools image which uses Alpine. I'm not very well versed in Docker conventions, but it works, and Alpine is only 4MB compared to debian:bookworm-slim's 28MB. But I don't have a strong opinion here.
There was a problem hiding this comment.
👍 Curious about the distroless one since it's fairly common these days (e.g. it's what Docker uses in their guide sample at https://docs.docker.com/guides/golang/build-images/#multi-stage-builds). But Alpine is fine too. Not a blocker.
There was a problem hiding this comment.
I was thinking of recommending distroless too here. Alpine is a bit nicer if you ever want a shell to inspect the image, we may want that if we add support for config files later. Agree it's not a blocker.
| Docker image build requires [Goreleaser](https://goreleaser.com/) to build the binaries first, although it doesn't use | ||
| Goreleaser for the Docker image itself. |
There was a problem hiding this comment.
although it doesn't use Goreleaser for the Docker image itself.
So was looking at https://goreleaser.com/customization/docker/ and it seems they won't publish for you, is that correct? If so, it makes sense we don't want to rely on it.
There was a problem hiding this comment.
It's not documented but it can in fact push to Dockerhub. But since it's undocumented I don't want to rely on it. The bigger reason is that it can't build a true multiplatform image, it needs to build them separately for each platform and then combine them with a manifest, and it can only do one tag at a time it seems (and we need two - the version and the latest). It too is very poorly documented, and also we're using Goreleaser v1 which they don't publish docs for anymore (I had to use Web Archive). Overall, it feels very not worth it.
There was a problem hiding this comment.
Wondering why, with this conversation seemingly open-ended, the MR was merged.
| COPY ./dist/nix_linux_arm64/temporal /dist/arm64/temporal | ||
|
|
||
| WORKDIR /app | ||
| FROM alpine:3.22 |
There was a problem hiding this comment.
👍 Curious about the distroless one since it's fairly common these days (e.g. it's what Docker uses in their guide sample at https://docs.docker.com/guides/golang/build-images/#multi-stage-builds). But Alpine is fine too. Not a blocker.
What was changed
Updated Dockerfile to be based on Alpine and to package binaries produced by Goreleaser. Added instructions to CONTRIBUTORS.md on how to build Docker image.
Note that this PR does NOT contain any automation for Docker image publishing. Release 1.4.0 will be published manually after this PR is merged.
Why?
Feature request: #316
Checklist
Internal release instructions will need to be updated.