-
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 17 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,21 @@ | ||
# Build Stage | ||
FROM rust:1.72.1 as build | ||
COPY /node/ Makefile /node/ | ||
pompon0 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
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 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 consenus_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 tools && 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 | ||
|
||
consenus_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 |
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,27 @@ | ||
# Running a test node | ||
# Running a test consensus node | ||
|
||
## Local | ||
|
||
1. Generate a file named `addresses.txt` in the root directory of the tools crate, containing node addresses in the format `IP:PORT`, with each address on a separate line. | ||
2. Run `make node-configs`. This command will establish a directory named `node-configs` and create a folder for each address listed in the `.txt` file, providing the necessary configuration files for the respective node. | ||
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 think it would be beneficial to start with a template file which can be seen and just modified. |
||
3. Execute `make run-node IP=<NODE_IP>`. The default value for this command would be `127.0.0.1:3054`. Note that running this command will take control of the terminal. | ||
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. It is not totally clear that the Makefile is located in a parent directory, and running make commands from this location would fail. 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. You're right, with new changes in the directories and overall structure I forgot to update the README. Now it should be updated including the last changes added from this comments. Let me know if it's more clear now. |
||
2. Run `make node_configs`. This command will establish a directory named `node-configs` and create a folder for each address listed in the `.txt` file, providing the necessary configuration files for the respective node. | ||
3. Execute `make node IP=<NODE_IP>`. The default value for this command would be `127.0.0.1:3054`. Note that running this command will take control of the terminal. | ||
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 found this a bit confusing given the header "Running a test consensus node", while both this step and the previous one are intended for running multiple nodes. Perhaps add some clarification about that. |
||
|
||
## Dockerized | ||
|
||
To get up a standalone consensus node running in a docker container just run the following command inside the tools crate: | ||
|
||
`make node_docker` | ||
|
||
This will create a container running a single node advancing views and finalizing blocks. | ||
|
||
To set up a simple example with two different nodes communicating with each other in running in different containers run the following command: | ||
|
||
`make consenus_docker_example` | ||
|
||
IAvecilla marked this conversation as resolved.
Show resolved
Hide resolved
|
||
This will set up two distinct containers, each hosting a consensus node. These nodes will be interlinked, progressing through views and finalizing blocks achieving consensus between them. | ||
|
||
To clean all the state after running these commands use: | ||
|
||
`make clean_docker` | ||
|
||
> This will delete the generated images and containers, requiring them to be regenerated. |
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
?