the-draupnir-project / Draupnir

A Matrix moderation bot
https://the-draupnir-project.github.io/draupnir-documentation/
91 stars 14 forks source link

Running `docker build` without supplying `version.txt` in the repo will fail because `git` isnt in the image #300

Closed ShadowJonathan closed 3 weeks ago

ShadowJonathan commented 7 months ago

I encountered this while doing some development and building the image on my VM; Doing a bare clone and then running docker build immediately will fail with something like this;

root@mjolnir:~# docker-compose build mjol
Building mjol
Step 1/8 : FROM node:20-slim
 ---> 8c3d9f745295
Step 2/8 : COPY . /tmp/src
 ---> c17271863704
Step 3/8 : RUN cd /tmp/src     && yarn install --network-timeout 100000     && yarn build     && mv lib/ /mjolnir/     && mv node_modules /     && mv mjolnir-entrypoint.sh /     && mv package.json /     && mv version.txt /     && cd /     && rm -rf /tmp/*
 ---> Running in 259083d3408e
yarn install v1.22.19
[1/5] Validating package.json...
[2/5] Resolving packages...
[3/5] Fetching packages...
[4/5] Linking dependencies...
[5/5] Building fresh packages...
Done in 9.73s.
yarn run v1.22.19
$ tsc
$ yarn remove-tests-from-lib && yarn describe-version
$ rm -rf lib/test/ && cp -r lib/src/* lib/ && rm -rf lib/src/
$ (git describe > version.txt.tmp && mv version.txt.tmp version.txt) || true && rm -f version.txt.tmp
/bin/sh: 1: git: not found
Done in 7.71s.
mv: cannot stat 'version.txt': No such file or directory
ERROR: Service 'mjol' failed to build: The command '/bin/sh -c cd /tmp/src     && yarn install --network-timeout 100000     && yarn build     && mv lib/ /mjolnir/     && mv node_modules /     && mv mjolnir-entrypoint.sh /     && mv package.json /     && mv version.txt /     && cd /     && rm -rf /tmp/*' returned a non-zero code: 1
root@mjolnir:~#

This isn't noticed in the Github Actions Flow because there the file is always given, and the step in yarn build where it attempts to get it is skipped.

Gnuxie commented 7 months ago

This has always been known, but clearly not documented anywhere. You can't just build the docker image without providing version.txt. And honestly I didn't know this was something that people were trying to do?

Gnuxie commented 7 months ago

I'm not a system admin so i would be interested to know what you recommend. Since we want the version to be provided, but also don't want to install git in the image just to be able to describe the version (although it could be removed as part of the build process? idk)

ShadowJonathan commented 7 months ago

You could possibly attach another build step which downloads a minimal git image, grabs and outputs the version, and then passes it on to the next stage?

The way the dockerfile is currently constructed, adding it to the last stage would add it to the final image, but having a 'builder' pre-step like that would not have it stay in the final image.

And honestly I didn't know this was something that people were trying to do?

With any and all projects supplying Dockerfile, it is expected that "Just Running docker build" will always give it a usable image.

Gnuxie commented 7 months ago

We never supported doing so, but I can understand how it might be seen that we have entered an implicit contract.

I assume that by another build step and stage you are referring to what is described in the documentation here? That's pretty cool, I don't know why we didn't use this before or seemingly not realise that this is something that we can do.

ShadowJonathan commented 7 months ago

Yes, multi-stage builds are awesome. You can use a fully-fledged build environment to compile your code, and then just copy the binary to a minimal execution environment, and trim down on the final image size like that.

Here, it can be done to inspect the current git repository from one stage, and then hand the result to the next :3

In any case, clarifying the error and/or possibly halting when it sees that the required environment variable is missing would go a long way in explaining why the build failed.

Gnuxie commented 4 weeks ago

@ShadowJonathan I've given this a go in #595 if you'd like to take a look <3