-
Notifications
You must be signed in to change notification settings - Fork 166
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
Integrate beacond connected to bArtio via docker #1848
Conversation
WalkthroughThe recent changes enhance the Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1848 +/- ##
=======================================
Coverage 21.86% 21.86%
=======================================
Files 347 347
Lines 15759 15759
Branches 21 21
=======================================
Hits 3446 3446
Misses 12197 12197
Partials 116 116 |
5012d82
to
f4ae53f
Compare
Makefile
Outdated
@@ -11,6 +11,7 @@ include build/scripts/testing.mk | |||
include contracts/Makefile |
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.
Currently the pre-requisites steps are listed in the fullnode.mk
file. To mount these files such as config.toml and genesis.json, building the binary won't help.
So thinking to spin up beacond using docker containers to be able to mount volumes.
6ac7db3
to
d89de7c
Compare
652e87a
to
cbe7968
Compare
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.
Actionable comments posted: 2
Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Commits
Files that changed from the base of the PR and between 5813e7b and fb02e71148fa7b8c7ebd60adb35eff141018ba32.
Files selected for processing (1)
- build/scripts/testing.mk (1 hunks)
Additional comments not posted (5)
build/scripts/testing.mk (5)
50-50
: LGTM!The
RPC_URL
variable is correctly constructed using theIP_ADDRESS
.
51-52
: LGTM!The
IMAGE_NAME
andVERSION
variables are appropriately defined for Docker image management.
54-54
: LGTM!The
IMAGE_EXISTS
variable is correctly set up to check for the existence of the Docker image.
49-49
: Verify the IP address selection logic.The
IP_ADDRESS
variable useshostname -I
to get the first IP address. Ensure this is the correct IP address for your use case, as environments with multiple network interfaces might yield unexpected results.
56-62
: LGTM! But verifymake build-docker
is defined.The
build-beacond-image
target uses conditional logic effectively. Ensure that themake build-docker
command is defined and correctly implemented elsewhere in the Makefile.Verification successful
make build-docker
is definedThe
build-docker
target is defined in thebuild/scripts/build.mk
file, confirming that thebuild-beacond-image
target can successfully invoke it. No further action is needed.
build/scripts/build.mk
:build-docker: ## build a docker image containing beacond
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that `make build-docker` is defined in the Makefile. # Test: Search for the `build-docker` target definition. rg --type makefile '^build-docker:'Length of output: 70
Script:
#!/bin/bash # Find all Makefile-related files and search for the `build-docker` target definition. fd -e mk -e Makefile | xargs rg '^build-docker:'Length of output: 132
d8e28b6
to
1dab360
Compare
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.
Actionable comments posted: 0
Outside diff range comments (2)
build/scripts/testing.mk (2)
Line range hint
154-156
:
Remove redundant--rm
option.The
--rm
option is specified twice for different volume mounts in thestart-geth-bartio
target. It should be specified only once.- --rm -v $(PWD)/${TESTAPP_FILES_DIR}:/${TESTAPP_FILES_DIR} \ - --rm -v $(PWD)/${BARTIO_NETWORK_FILES_DIR}:/${BARTIO_NETWORK_FILES_DIR} \ + --rm -v $(PWD)/${TESTAPP_FILES_DIR}:/${TESTAPP_FILES_DIR} \ + -v $(PWD)/${BARTIO_NETWORK_FILES_DIR}:/${BARTIO_NETWORK_FILES_DIR} \
Line range hint
198-200
:
Remove redundant--rm
option.The
--rm
option is specified twice for different volume mounts in thestart-erigon
target. It should be specified only once.- --rm -v $(PWD)/${TESTAPP_FILES_DIR}:/${TESTAPP_FILES_DIR} \ - --rm -v $(PWD)/.tmp:/.tmp \ + --rm -v $(PWD)/${TESTAPP_FILES_DIR}:/${TESTAPP_FILES_DIR} \ + -v $(PWD)/.tmp:/.tmp \
Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Commits
Files that changed from the base of the PR and between fb02e71148fa7b8c7ebd60adb35eff141018ba32 and 1dab360f709bbccc84559eefdcb39dd2ac3fd414.
Files selected for processing (1)
- build/scripts/testing.mk (1 hunks)
Additional comments not posted (6)
build/scripts/testing.mk (6)
46-47
: Reminder: Address the TODO comment for snapshot restoration.The TODO comment indicates missing documentation or steps for restoring snapshots.
49-54
: Introduction of new environment variables is appropriate.The new variables are essential for configuring the Docker environment and managing the
beacond
image.
56-63
: Conditional Docker image building logic is efficient.The logic to check for the image's existence before building optimizes the build process.
64-83
: Comprehensive Docker run configuration is effective.The
start-bartio-docker
target includes necessary volume mounts and environment configurations, enhancing the service's operational capabilities.
85-85
: Comment on IPC support limitations is informative.The comment provides useful information about the current limitations of IPC support.
Line range hint
109-111
:
Remove redundant--rm
option.The
--rm
option is specified twice for different volume mounts in thestart-reth-bartio
target. It should be specified only once.
Signed-off-by: nidhi-singh02 <[email protected]>
Signed-off-by: nidhi-singh02 <[email protected]>
Signed-off-by: nidhi-singh02 <[email protected]>
Signed-off-by: nidhi-singh02 <[email protected]>
Signed-off-by: nidhi-singh02 <[email protected]>
Signed-off-by: nidhi-singh02 <[email protected]>
Signed-off-by: nidhi-singh02 <[email protected]>
Signed-off-by: nidhi-singh02 <[email protected]>
Signed-off-by: nidhi-singh02 <[email protected]>
c6aefb6
to
da99c67
Compare
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.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Commits
Files that changed from the base of the PR and between 1dab360f709bbccc84559eefdcb39dd2ac3fd414 and da99c67.
Files selected for processing (1)
- build/scripts/testing.mk (1 hunks)
Additional comments not posted (4)
build/scripts/testing.mk (4)
85-85
: Informative comment onstart-ipc
.The comment provides necessary context for the
start-ipc
target.
56-63
: Efficient Docker image management.The
build-beacond-image
target efficiently avoids unnecessary rebuilds usingIMAGE_EXISTS
.Ensure that the
build-docker
target is correctly defined and functional.Verification successful
Verified:
build-docker
target is correctly defined and used.The
build-docker
target is defined inbuild/scripts/build.mk
and is utilized in various Makefiles, indicating proper integration and functionality.
build/scripts/build.mk
:build-docker
target definition.kurtosis/Makefile
: Usage ofbuild-docker
.build/scripts/testing.mk
: Usage ofbuild-docker
.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the definition and functionality of `build-docker` target. # Test: Search for the `build-docker` target. Expect: Correct definition and usage. rg --type make 'build-docker'Length of output: 352
49-54
: LGTM! Verify the variable usage in the codebase.The introduction of these variables looks good and is essential for Docker integration.
However, ensure that these variables are used correctly throughout the codebase.
Verification successful
Variables are correctly utilized across the codebase.
The variables
IP_ADDRESS
,RPC_URL
,IMAGE_NAME
,VERSION
, andIMAGE_EXISTS
are consistently and correctly used in the codebase. Their integration into Docker image management and network configuration processes is appropriate.
IP_ADDRESS
andRPC_URL
are used for network configurations.IMAGE_NAME
andVERSION
are used for Docker image management.IMAGE_EXISTS
is used to check the existence of Docker images before building.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of new variables in the codebase. # Test: Search for the usage of each variable. Expect: Consistent and correct usage. rg --type make 'IP_ADDRESS|RPC_URL|IMAGE_NAME|VERSION|IMAGE_EXISTS'Length of output: 2301
64-83
: Comprehensive Docker run configuration.The
start-bartio-docker
target includes a well-structured Docker run command with necessary configurations.Verify that the Docker run command is executed correctly and all parameters are valid.
Verification successful
Docker run command parameters verified.
The parameters and environment variables used in the
start-bartio-docker
target are correctly defined and logically structured. This ensures that the Docker run command will execute as intended.
TESTAPP_FILES_DIR
is set totesting/files
.BARTIO_NETWORK_FILES_DIR
is set totesting/networks/80084
.IMAGE_NAME
isbeacond
.VERSION
iskurtosis-local
.JWT_PATH
istesting/files/jwt.hex
.RPC_URL
ishttp://${IP_ADDRESS}:8551
.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the execution and parameters of the Docker run command. # Test: Check for the Docker run command usage. Expect: Correct execution and parameter usage. rg --type make 'docker run'Length of output: 862
Script:
#!/bin/bash # Description: Search for the definitions of variables used in the Docker run command. # Search for TESTAPP_FILES_DIR definition rg 'TESTAPP_FILES_DIR\s*:?=' # Search for BARTIO_NETWORK_FILES_DIR definition rg 'BARTIO_NETWORK_FILES_DIR\s*:?=' # Search for IMAGE_NAME definition rg 'IMAGE_NAME\s*:?=' # Search for VERSION definition rg 'VERSION\s*:?=' # Search for JWT_PATH definition rg 'JWT_PATH\s*:?=' # Search for RPC_URL definition rg 'RPC_URL\s*:?='Length of output: 1358
This reverts commit d1ddc15.
This reverts commit d1ddc15.
Summary by CodeRabbit
beacond
service, facilitating easier builds and deployment.beacond
Docker image and starting the service with customizable parameters.