Skip to content
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

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

Closed
ShadowJonathan opened this issue Mar 20, 2024 · 6 comments · Fixed by #595
Labels
L2 Few Likelihood P3 Outrageous / Cannot continue to or start to use Priority level - includes missing documentation leading to same outrage etc T6 Crash Bug causes crash OR data loss

Comments

@ShadowJonathan
Copy link
Contributor

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

Gnuxie commented Mar 20, 2024

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

Gnuxie commented Mar 20, 2024

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

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

Gnuxie commented Mar 21, 2024

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

ShadowJonathan commented Mar 22, 2024

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 Gnuxie added L2 Few Likelihood P3 Outrageous / Cannot continue to or start to use Priority level - includes missing documentation leading to same outrage etc T6 Crash Bug causes crash OR data loss labels Apr 16, 2024
Gnuxie added a commit that referenced this issue Oct 4, 2024
@Gnuxie
Copy link
Member

Gnuxie commented Oct 4, 2024

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

Gnuxie added a commit that referenced this issue Oct 4, 2024
Gnuxie added a commit that referenced this issue Oct 10, 2024
Gnuxie added a commit that referenced this issue Oct 10, 2024
* Use multi-stage build in Dockerfile

#300.

* Move git describe and build into one stage.

Probably won't be a good idea to download an alpine image just to install git.

* Remove git describe step from CI.

* whoopsie, copy version from the build stage not the deleted stamp.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
L2 Few Likelihood P3 Outrageous / Cannot continue to or start to use Priority level - includes missing documentation leading to same outrage etc T6 Crash Bug causes crash OR data loss
Projects
Status: No status
Development

Successfully merging a pull request may close this issue.

2 participants