Skip to content
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

Closed
wants to merge 13 commits into from

Conversation

jasonyic
Copy link
Contributor

@jasonyic jasonyic commented Jul 14, 2022

Description

  • refactor dockerfile to add default config.toml & genesis file from mainnet & testnet based on the binary release version.
  • add entry point script to init genesis file and start node, with additional command options
  • add support on 2 env variables BSC_NETWORK & BSC_DATA_DIR to provide the flexibility to boot node in different network.

Rationale

  • can easily start a full node in testnet or mainnet with default configuration.
  • also support the command line options to overwrite default configuration

Example

docker run -d ghcr.io/bnb-chain/bsc should init with the genesis file and start a full node in testnet
docker run -d -e BSC_NETWORK=mainnet ghcr.io/bnb-chain/bsc should start a full node in mainnet
docker run -d -e BSC_NETWORK=mainnet ghcr.io/bnb-chain/bsc --datadir /data2 should start a full node in mainnet with custom data directory.

Changes

Notable changes:

  • refactor dockefile to add default configuration,
  • refactor dockerfile to use non-root user
  • refactor dockerfile to use tini for better process handling
  • add entry point script to init genesis file and boot node, also support command line options
  • a docker-composer file provided for local testing

Feature Request: #991

@jasonyic jasonyic force-pushed the feat/refactor_dockerfile branch 3 times, most recently from b391837 to 63009be Compare July 14, 2022 12:44
Signed-off-by: Jason Yi <jasonhk.yi@consensys.net>
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 \
Copy link
Contributor

@j75689 j75689 Jul 15, 2022

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

Copy link
Contributor Author

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.

Copy link
Contributor

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

Copy link
Contributor Author

@jasonyic jasonyic Jul 15, 2022

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.

command: ["--http.addr", "0.0.0.0", "--ws.addr", "0.0.0.0"]
environment:
- LOG_LEVEL=3
- DIFF_SYNC=true
Copy link
Contributor

@j75689 j75689 Jul 15, 2022

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

Copy link
Contributor Author

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.

Copy link
Contributor Author

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. I agree with you, but I just want to say that the DIFF_SYNC=true environment variable has no effect here
  2. I think building it as an image is enough, the program is the same, just the configuration is different

Copy link
Contributor Author

@jasonyic jasonyic Jul 15, 2022

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>
@jasonyic jasonyic requested a review from j75689 July 15, 2022 03:57
@j75689
Copy link
Contributor

j75689 commented Jul 15, 2022

Can you write a simple document on how to use your docker image? It can be placed in docs/docker/readme.md

@j75689
Copy link
Contributor

j75689 commented Jul 15, 2022

and please change the target branch to develop, thanks

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>
Signed-off-by: Jason Yi <jasonhk.yi@consensys.net>
@jasonyic jasonyic changed the base branch from master to develop July 15, 2022 08:56
Signed-off-by: Jason Yi <jasonhk.yi@consensys.net>
@jasonyic
Copy link
Contributor Author

Can you write a simple document on how to use your docker image? It can be placed in docs/docker/readme.md

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>
@jasonyic jasonyic marked this pull request as ready for review July 15, 2022 09:57
@jasonyic
Copy link
Contributor Author

close this PR and replace with PR #998

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants