refactor: update docker images to use cargo-chef #1619
Conversation
.circleci/config.yml
Outdated
| @@ -66,7 +66,7 @@ jobs: | |||
| - run: | |||
| name: Build Docker image | |||
| no_output_timeout: 30m | |||
| command: docker build -f ./docker/Dockerfile -t $IMAGE_NAME:latest --build-arg GIT_HASH=$CIRCLE_SHA1 . | |||
| command: docker build -f ./docker/Dockerfile -t $IMAGE_NAME:latest | |||
There was a problem hiding this comment.
Maybe I'm missing where this happens, but are you sure we have git initialized at this point? Otherwise there's no .git folder for docker to pick up
There was a problem hiding this comment.
This PR won't work without #1615 if you are testing
I plan to merge them at the same time so it shouldn't be a issue.
Yes the .git folder is initalized
https://circleci.com/docs/configuration-reference/#checkout
^ CircleCI just does git commands to pull changes and such
There was a problem hiding this comment.
CircleCI does a git clone or git fetch depending on circumstances so the .git would be initialized if that was the case
docker/Dockerfile.bridge
Outdated
| COPY --from=builder /trin/target/release/portal-bridge /usr/bin/ | ||
| COPY --from=builder /trin/target/release/sample_range /usr/bin/ | ||
| COPY --from=builder /trin/target/release/poll_latest /usr/bin/ | ||
|
|
There was a problem hiding this comment.
nit remove this newline
|
|
||
| # final base | ||
| FROM ubuntu:22.04 | ||
| # Build application |
There was a problem hiding this comment.
nit maybe leave a comment here referencing the .dockerignore so that people know where to go and include a folder if a new workspace is created...
eg. Copy over all project folders specified in the .dockerignore or something like that
What was wrong?
@njgheorghita brought up a concern that #1615 (comment) with my PR to update how we get the client info, to use vergen instead of shadow-rs #1615 that our docker images would no longer get the right git commitment.
He brought up a very good point! The way our dockerfiles were setup vergen wouldn't work properly.
How was it fixed?
Remove the requirement to pass
GITHASHto the docker build, by copying the whole working directory, since then the working directory has access to the .git folder, we no longer need to pass it in.Another thing is now we don't need to update our dockerfile everytime we add a new crate, which will improve maintainability.
Because we are now copying the whole workspace, to not lose caching performance I added cargo-chef (which reth uses) which will changes more info can be found at https://github.com/LukeMathWalker/cargo-chef
I tested these changes locally with #1615 and it works
