Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

fix: Fixes #13071 change order of ldd command and update ./docker/README.md #13072

Merged
merged 2 commits into from
Mar 7, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion docker/README.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# Substrate Builder Docker Image

The Docker image in this folder is a `builder` image. It is self contained and allow users to build the binaries themselves.
The Docker image in this folder is a `builder` image. It is self contained and allows users to build the binaries themselves.
There is no requirement on having Rust or any other toolchain installed but a working Docker environment.

Unlike the `parity/polkadot` image which contains a single binary (`polkadot`!) used by default, the image in this folder builds and contains several binaries and you need to provide the name of the binary to be called.
Expand All @@ -12,6 +12,13 @@ You should refer to the .Dockerfile for the actual list. At the time of editing,
- node-template
- chain-spec-builder

> Warning: Currently the pre-built image [`parity/substrate:latest`](https://hub.docker.com/layers/paritytech/substrate/latest/images/sha256-d1be27ff2a93d7de49a5ef9449b4e7aa5f479d9d03f808ec34bf2e8cea89cdc4?context=explore) is outdated and uses `substrate 3.0.0-dev-ea387c63471`. The entrypoint it uses is `ENTRYPOINT ["/usr/local/bin/substrate"]` so it only supports running that old Substrate binary and to use the image you need to provide options to it in the Docker run command but without passing the Substrate binary (i.e. `docker run --rm -it parity/substrate --version`).
Copy link
Member

Choose a reason for hiding this comment

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

What is that conveying?

I think we should remove this.

Copy link
Contributor Author

@ltfschoen ltfschoen Jan 5, 2023

Choose a reason for hiding this comment

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

yes if the user follows the instructions and runs ./build.sh before running the docker run ... commands then it'll build an image and a container with the latest substrate.

but if they forget to run the ./build.sh step (by not following the instructions, although that's unlikely..) then it'll fetch that old substrate image parity/substrate:latest from docker hub that is 3 years old, and they may not realise they're not using the latest version until it doesn't function as intended, which may lead to confusion.

do you think it would be worth perhaps changing it to > Note: If you do not first run the build.sh script to generate the parity/substrate image before using the docker run command to create a container based on that image name, then it will instead fetch a pre-generated version of that image that may not be the latest version.? or just remove it?

is there a reason why that parity/substrate:latest doesn't have the latest version like the image ci-linux:production?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i've pushed a commit removing the warning, i think it's sufficient to expect users to follow the instructions


To generate the latest parity/substrate image. Please first run:
```sh
./build.sh
```

The image can be used by passing the selected binary followed by the appropriate tags for this binary.

Your best guess to get started is to pass the `--help flag`. Here are a few examples:
Expand Down
4 changes: 2 additions & 2 deletions docker/substrate_builder.Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,10 @@ RUN useradd -m -u 1000 -U -s /bin/sh -d /substrate substrate && \
mkdir -p /data /substrate/.local/share/substrate && \
chown -R substrate:substrate /data && \
ln -s /data /substrate/.local/share/substrate && \
# unclutter and minimize the attack surface
rm -rf /usr/bin /usr/sbin && \
# Sanity checks
ldd /usr/local/bin/substrate && \
# unclutter and minimize the attack surface
rm -rf /usr/bin /usr/sbin && \
/usr/local/bin/substrate --version

USER substrate
Expand Down