-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
feat: refactor dockerfile to add entrypoint script #993
Conversation
b391837
to
63009be
Compare
Signed-off-by: Jason Yi <jasonhk.yi@consensys.net>
63009be
to
28815f7
Compare
COPY --from=builder /go-ethereum/build/bin/geth /usr/local/bin/ | ||
|
||
COPY docker-entrypoint.sh ./ | ||
|
||
RUN curl -LO https://github.com/bnb-chain/bsc/releases/download/${BSC_VERSION}/mainnet.zip \ |
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.
the URL can be replaced by .github/release.env
.
the mainnet and testnet config of the new release is obtained from here
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.
release.env
still pointing to v1.1.8
and that is why I have a ARG to build on.
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.
Actually every time a new version is released the additional files come from here, so it will be the same if releas.env
doesn't update
The reason I want you to use release.env
is that developers will most likely forget to update the Dockerfile when they release a new version
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.
when each release, no need to update the BSC_VERSION
in Dockerfile
, the build process should pass in --build-arg
for the new version to be used, similar to the new target docker
in makefile.
BSC_VERSION=v1.1.10 make docker
Additionally, this won't cause confusion for developer between the regular release in the github repo and configuration version being used in Dockerfile.
docker-compose.yaml
Outdated
command: ["--http.addr", "0.0.0.0", "--ws.addr", "0.0.0.0"] | ||
environment: | ||
- LOG_LEVEL=3 | ||
- DIFF_SYNC=true |
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.
Most of the flags do not yet support the reading of env...
Must use an additional script to convert to flag, or we need to add support
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.
the reason which I suggest to only support 2 BSC unique env: BSC_NETWORK
& BSC_DATA_DIR
is that geth
has quite a few command line options and is quite familiar by the developers. to keep the compatibility and minimise the maintenance for the script. BSC_NETWORK
is used to boot the node to different network. default /data
will be convenient to configure docker image, specially when hosting in k8s when mounting volume. config.toml
& genesis.json
can also be mounted by configmap
in k8s into the BSC_HOME/mainnet/
orBSC_HOME/testnet/
before bootstrap script kicks in.
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.
another thought on building network specific image is to use another ARG: <mainnet|testnet>
to build image for mainnet
& testnet
separately, this will end up with two image tags, e.g. ghcr.io/bnb-chain/bsc:v1.1.11-mainnet
, ghcr.io/bnb-chain/bsc:v1.1.11-testnet
. the config in the docker image will only host the config from the network..this will give much clean image but extra image tag.
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.
- I agree with you, but I just want to say that the
DIFF_SYNC=true
environment variable has no effect here - I think building it as an image is enough, the program is the same, just the configuration is different
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.
sample docker-compose file just updated to reflect the usage with command line.
in k8s, it will look like
containers:
- name: client
image: bnb-chain/bsc:v1.1.11
args: ["--http.addr", "0.0.0.0", "--http.port", "8545", "--http.vhosts", "*", "--verbosity", "3"]
env:
- name: BSC_NETWORK
value: mainnet
Signed-off-by: Jason Yi <jasonhk.yi@consensys.net>
Can you write a simple document on how to use your docker image? It can be placed in |
and please change the target branch to |
Signed-off-by: Jason Yi <jasonhk.yi@consensys.net>
Signed-off-by: Jason Yi <jasonhk.yi@consensys.net>
Signed-off-by: Jason Yi <jasonhk.yi@consensys.net>
done. please review |
Signed-off-by: Jason Yi <jasonhk.yi@consensys.net>
Signed-off-by: Jason Yi <jasonhk.yi@consensys.net>
Signed-off-by: Jason Yi <jasonhk.yi@consensys.net>
close this PR and replace with PR #998 |
Description
dockerfile
to add defaultconfig.toml
&genesis
file frommainnet
&testnet
based on the binary release version.BSC_NETWORK
&BSC_DATA_DIR
to provide the flexibility to boot node in different network.Rationale
testnet
ormainnet
with default configuration.Example
docker run -d ghcr.io/bnb-chain/bsc
should init with the genesis file and start a full node intestnet
docker run -d -e BSC_NETWORK=mainnet ghcr.io/bnb-chain/bsc
should start a full node inmainnet
docker run -d -e BSC_NETWORK=mainnet ghcr.io/bnb-chain/bsc --datadir /data2
should start a full node inmainnet
with custom data directory.Changes
Notable changes:
tini
for better process handlingFeature Request: #991