-
Notifications
You must be signed in to change notification settings - Fork 39
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
Consensus node Dockerfile for test framework (BFT-398) #57
Changes from 22 commits
6bd6170
b2fb9ab
e768bc6
e6b6487
4ca7fdc
28df58b
3e4b881
febcafc
b2e7e9c
5e27e6e
6e2236a
810a4b5
4088c0a
4475489
689b6d5
99dc691
a3b270d
f93890f
f3893cf
1c721c8
8cac863
d24c8bc
9520c4c
54c72ce
13e61e1
3b8d299
2d09c1d
c287c56
e66cec9
987add2
222fca2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
/node/target |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,22 @@ | ||
# Build Stage | ||
FROM rust:1.72.1 as build | ||
COPY /node/ /node/ | ||
COPY Makefile . | ||
WORKDIR /node | ||
RUN apt-get update && apt-get install -y libclang-dev | ||
RUN cargo build --release | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Following the discussion you had about caching: As for line 7, as you already mentioned, it should be possible to avoid the reinstalling of dependencies, by only copying This is not critical for now, but might worth trying. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Cargo.toml is a workspace manifest, so it would need to be converted to a crate manifest. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Exactly, that was my initial approach for caching external dependencies but since it works as a workspace manifest all the local code would be necessary to build along them. |
||
RUN cd .. && make docker_node_configs | ||
|
||
# Runtime Stage | ||
FROM debian:stable-slim as runtime | ||
|
||
COPY --from=build /node/target/release/executor /node/ | ||
COPY --from=build /node/tools/docker-config/node-configs /node/ | ||
COPY entrypoint.sh /node/ | ||
|
||
WORKDIR /node | ||
RUN chmod +x entrypoint.sh | ||
|
||
ENTRYPOINT ["./entrypoint.sh"] | ||
|
||
EXPOSE 3054 |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,41 @@ | ||
.PHONY: node node_configs docker_node_configs node_docker consensus_docker_example clean clean_docker | ||
IP?=127.0.0.1:3054 | ||
DOCKER_IP=172.12.0.10 | ||
EXECUTABLE_NODE_DIR=node/tools | ||
|
||
# Locally run commands | ||
|
||
node: | ||
export RUST_LOG=INFO && cd ${EXECUTABLE_NODE_DIR}/node-configs/${IP} && cargo run -- --database ../../database/${IP} | ||
|
||
node_configs: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sounds good, I also changed the generated directory where the configs will be. |
||
cd ${EXECUTABLE_NODE_DIR} && cargo run --bin localnet_config -- --input-addrs addresses.txt --output-dir node-configs | ||
|
||
# Docker commands | ||
|
||
docker_node_configs: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This command doesn't work from outside the container, as well as being undocumented (also in the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This command shouldn't be important for the user. It is used by Docker in the initial phase of the image building process to generate the configuration. However, I added a comment above the command with this disclaimer. |
||
cd ${EXECUTABLE_NODE_DIR} && cargo run --release --bin localnet_config -- --input-addrs docker-config/addresses.txt --output-dir docker-config/node-configs | ||
|
||
node_docker: | ||
mkdir -p ${EXECUTABLE_NODE_DIR}/docker-config | ||
cd ${EXECUTABLE_NODE_DIR}/docker-config && rm -rf addresses.txt && echo ${DOCKER_IP}:3054 >> addresses.txt | ||
docker-compose up -d node-1 | ||
|
||
consensus_docker_example: | ||
mkdir -p ${EXECUTABLE_NODE_DIR}/docker-config | ||
cd ${EXECUTABLE_NODE_DIR}/docker-config && rm -rf addresses.txt && touch addresses.txt && echo 172.12.0.10:3054 >> addresses.txt && echo 172.12.0.11:3054 >> addresses.txt | ||
docker-compose up -d | ||
|
||
# Clean commands | ||
|
||
clean: clean_docker | ||
rm -rf ${EXECUTABLE_NODE_DIR}/node-configs | ||
rm -rf ${EXECUTABLE_NODE_DIR}/database | ||
|
||
clean_docker: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What about a command for stopping the container(s), without a full cleanup (which currently also removes the image build)? |
||
rm -rf ${EXECUTABLE_NODE_DIR}/docker-config | ||
docker rm -f consensus-node | ||
docker rm -f consensus-node-1 | ||
docker rm -f consensus-node-2 | ||
docker network rm -f node-net | ||
docker image rm -f consensus-node |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,28 @@ | ||
version: "3.9" | ||
|
||
services: | ||
node-1: | ||
build: . | ||
image: consensus-node | ||
container_name: consensus-node-1 | ||
networks: | ||
node_net: | ||
# This allow us to know the ip of the node-1 container to fill the address in the config file | ||
# Only for test purposes, may be removed in the future | ||
ipv4_address: 172.12.0.10 | ||
node-2: | ||
image: consensus-node | ||
container_name: consensus-node-2 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Did you check the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I checked it, and it was my initial choice. The thing about that feature is that it duplicates the exact same container, and I needed to manually assign IPs to the containers to run the example in a single command where I can write the IPs in the config file, knowing them before running the container. Perhaps I overlooked something, and it's still feasible to use it. |
||
networks: | ||
node_net: | ||
# This allow us to know the ip of the node-2 container to fill the address in the config file | ||
# Only for test purposes, may be removed in the future | ||
ipv4_address: 172.12.0.11 | ||
|
||
networks: | ||
node_net: | ||
name: node-net | ||
ipam: | ||
config: | ||
- subnet: "172.12.0.0/24" | ||
gateway: "172.12.0.1" |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
#!/bin/bash | ||
|
||
cd $(hostname -i):3054 | ||
export RUST_LOG=INFO | ||
../executor | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Seeing this "../" path is a bit confusing, considering it's located at the top-level folder. Perhaps add some comments that this is to be used within the docker container, or put it within the Dockerfile altogether. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I renamed the file to |
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.
Maybe
rust:latest
?