-
Notifications
You must be signed in to change notification settings - Fork 5
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
Update dockerfile for JIMM image #1033
Conversation
Makefile
Outdated
@@ -8,6 +8,7 @@ PROJECT := github.com/canonical/jimm | |||
|
|||
GIT_COMMIT := $(shell git rev-parse --verify HEAD) | |||
GIT_VERSION := $(shell git describe --abbrev=0 --dirty) | |||
GO_VERSION := $(shell cat go.mod | sed -n "/^go/p" | cut -d ' ' -f 2) |
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.
Alternatively, we can use this to avoid manual file parsing:
go list -f {{.GoVersion}} -m
# 1.20
1910506
to
ddd36e7
Compare
ARG GIT_COMMIT | ||
ARG VERSION | ||
ARG GO_VERSION |
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.
Can read this from gomod, why explicitly pass as arg?
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're reading from go.mod in the Makefile then passing that here, I guess I was just following the same ideas as version and git commit. Should I change it to read from the go.mod inside the dockerfile?
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 don't mind but it's an extra dependency on the makefile when it can be handled in dockerfile, up to you really (personally I'd just do it in docker file)
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'll just leave it like this, easy tweak if needed in the future.
RUN apt update && apt install wget gcc -y | ||
RUN wget -L "https://golang.org/dl/go${GO_VERSION}.linux-amd64.tar.gz" | ||
RUN tar -C /usr/local -xzf "go${GO_VERSION}.linux-amd64.tar.gz" | ||
ENV PATH="${PATH}:/usr/local/go/bin" |
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.
Aren't there more envs we need to setup? Or is this just it?
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.
That's it
RUN echo "${GIT_COMMIT}" | tee ./version/commit.txt | ||
RUN echo "${VERSION}" | tee ./version/version.txt | ||
RUN --mount=type=ssh source /root/.gvm/scripts/gvm && go mod vendor | ||
RUN --mount=type=ssh source /root/.gvm/scripts/gvm && go build -tags version -o jimmsrv -v -a -mod vendor ./cmd/jimmsrv | ||
RUN go build -tags version -o jimmsrv -v ./cmd/jimmsrv |
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.
Wasthinking about if we need to vendor and tbh glad you removed it, modules kinda make it redundant
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.
lgtm bar some questions
Description
The current dockerfile for JIMM is not building see - https://github.com/canonical/jimm/actions/runs/5949764032/job/16138514650
Which looks to be due to the GVM (Go Version Manager) tool failing. To simplify things, I've opted to remove that dependency and just install the desired version of Go.
One note, while we could use the golang: docker image for the build phase, we run into an issue if we want to then run the binary on an Ubuntu image. Because JIMM is not a statically linked binary (there are some sqlite CGO dependencies among potentially others) one has to take care that the build image and run image are similar, for example building the server on golang:1.20 and then running the server on Ubuntu 20.04 fails due to glibc errors, it does however work on Ubuntu 22.04, but if the golang version were to be bumped we'd run into issues that we don't test for. So to keep things consistent, build and run on the same environment.
This needs to be forward merged to
feature-rebac
as well.