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

Add docker image #375

Merged
merged 6 commits into from
Jul 27, 2018
Merged

Add docker image #375

merged 6 commits into from
Jul 27, 2018

Conversation

chevdor
Copy link
Contributor

@chevdor chevdor commented Jul 19, 2018

Status

Pretty much ready beside the fact that the automatic build on docker hub may timeout due to the time required to make the image.

asciicast

Usage

  • see readme to build your own image. Beware, that takes a while...
  • or docker run -d -p 30333:30333 -p 9933:9933 chevdor/polkadot:latest polkadot

Todos

  • set version based on Cargo.toml
  • rebuild as --release
  • install polkadot so we don´t need to pass the full path
  • set / document volumes
  • set / document ports
  • reduce image size (no one wants to download 8GB images)

FROM phusion/baseimage:0.10.1
LABEL maintainer "[email protected]"

RUN apt-get update && \
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not into docker stuff, but maybe it would be better to split this into separate steps? I found it more useful since if you change some later steps, previous steps could be reused.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That makes it more readable and I would do that also in a regular script but for docker this is different. Having a 'single' command packs all in a single layer.

That could be split however but I have very bad internet atm and testing the build of docker images is a pain :)

Copy link
Contributor

Choose a reason for hiding this comment

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

If you split apt-get update from upgrade and install, then docker will cache the update step and then when you try to build the rest of the image it may fail because the outcome of the update is outdated.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep, very good point, that´s what I did.

docker/build.sh Outdated
@@ -0,0 +1,3 @@
#!/bin/sh
Copy link
Contributor

Choose a reason for hiding this comment

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

Better to use #!/usr/bin/env bash shebang since we already use it in other scripts.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok no problem, I will change that.

@ltfschoen
Copy link
Contributor

very pleased to see this

RUN apt-get update && \
apt-get upgrade -y && \
apt-get install -y cmake pkg-config libssl-dev && \
curl https://sh.rustup.rs -sSf | sh && \
Copy link
Contributor

@ltfschoen ltfschoen Jul 19, 2018

Choose a reason for hiding this comment

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

i get the following error when it gets up to running the curl command using Docker Community Edition Version 18.03.1-ce-mac65 (24312) on macOS 10.13.4:

info: downloading installer
rustup: Unable to run interactively. Run with -y to accept defaults, --help for additional options
The command '/bin/sh -c curl https://sh.rustup.rs -sSf | sh' returned a non-zero code: 1

Copy link
Contributor

Choose a reason for hiding this comment

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

i fixed the error by changing it to curl https://sh.rustup.rs -sSf | sh -s -- -y (i.e. appending -s -- -y), as recommended in this thread rust-lang/rustup#1031

@chevdor
Copy link
Contributor Author

chevdor commented Jul 19, 2018 via email

apt-get upgrade -y && \
apt-get install -y cmake pkg-config libssl-dev && \
curl https://sh.rustup.rs -sSf | sh && \
rustup update nightly && \
Copy link
Contributor

@ltfschoen ltfschoen Jul 19, 2018

Choose a reason for hiding this comment

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

i was getting error the following error when trying to run rustup as it wasn't added to the PATH.

i fixed it by adding the following lines of code after the curl command:

RUN echo 'PATH="$/root/.cargo/bin:$PATH";' >> ~/.bash_profile && . ~/.bash_profile && . /root/.cargo/env
RUN which rustup && which cargo

rustup target add wasm32-unknown-unknown --toolchain nightly && \
rustup update stable && \
cargo install --git https://github.com/alexcrichton/wasm-gc && \
git clone https://github.com/paritytech/polkadot.git && \
Copy link
Contributor

Choose a reason for hiding this comment

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

i get error /bin/sh: 1: git: not found because you need to install git before you can use it, by adding the following beforehand:

apt-get install -y git && \

ltfschoen added a commit that referenced this pull request Jul 20, 2018
@chevdor
Copy link
Contributor Author

chevdor commented Jul 20, 2018 via email

chevdor added a commit to chevdor/substrate that referenced this pull request Jul 20, 2018
@chevdor chevdor changed the title WIP: Add Dockerfile WIP: Add docker image Jul 20, 2018
Add documentation

Ref paritytech#375
chevdor added a commit to chevdor/substrate that referenced this pull request Jul 20, 2018
@chevdor chevdor changed the title WIP: Add docker image Add docker image Jul 20, 2018
@chevdor
Copy link
Contributor Author

chevdor commented Jul 20, 2018

So, as I feared, the build on docker hub takes more than 2 hours and times out.
I removed the WIP since the current solution is working and usable, users can either:

  • build their own image, depending on the perf of their system and docker settings it may take ~30 minutes
  • use a pre built image

The good news with the last option is that the current (compressed) size of the image is around 300MB. We can probably skim a few MB here and there but the time to do so is probably not worth it.

I will investigate an option to get the automatic docker builds to pass. Maybe someone has an idea/hint beside splitting the image? Most of the time is spent on building rust components, so I put little hope in making a base rust image and if it helps, it may not work very long as polkadot´s complexity increases.

@gavofyork gavofyork added the A0-please_review Pull request needs code review. label Jul 21, 2018
Copy link
Member

@gavofyork gavofyork left a comment

Choose a reason for hiding this comment

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

Generally prefer the docker instructions to go later since they're not the first-principles but rather more like a binary installer.

README.md Outdated
To get up and running with the smallest footprint on your system, you may use the Polkadot Docker image.
You can either build it yourself (it takes a while...):


Copy link
Member

Choose a reason for hiding this comment

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

some information missing 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.

Yes, hard to split the docker part.
I agree the building part should probably come later.

I moved the whole docker to the end in my last commit, including the (indeed) missing part.

The docker part describing how to 'just run' polkadot may be however interesting for many as it allows starting a node with the least amount of effort and dependencies beside a valid docker install. This is also much much quicker than to build the whole project.

@ltfschoen
Copy link
Contributor

ltfschoen commented Jul 21, 2018

@chevdor I tried implementing Docker on Polkadot just after your initial commit. I approached it the way I usually do on projects. I've pushed my proposed approach to the following link. I have poor internet this weekend, but I hope you can take a look at what I've done and choose anything you find useful into your PR https://github.com/paritytech/polkadot/compare/master...ltfschoen:docker?expand=1

@chevdor
Copy link
Contributor Author

chevdor commented Jul 21, 2018

@ltfschoen cool thanks. I am pushing a commit to add the missing info as @gavofyork mentioned and I will check your link tomorrow to see if I missed anything.

@chevdor chevdor changed the title Add docker image WIP: Add docker image Jul 22, 2018
@chevdor
Copy link
Contributor Author

chevdor commented Jul 22, 2018

@ltfschoen here is a little update. I am still working on improving the dockerfile, mainly to improve build speed and final size.

Here is some feedback regarding diff with your branch:

I added back a WIP on this issue. I will remove it once done.

@chevdor
Copy link
Contributor Author

chevdor commented Jul 22, 2018

I did some experiment trying to increase the number of layers to speed up building the image.
This unfortunately has a very negative impact on the size of the image (but makes indeed the build much faster since we can skip the apt updates, rust updates, etc...).

By splitting into more layers, the image went from 337MB to more than 3.3GB 🤢. The reason is mainly that /root/.cargo is huge and I cannot delete it between the layers since it is usually required by the next layer.

I also considered using the official rust slim image but the base image is already bigger than the best image I get today.

The current 2 layers options seem to be the best so far but I still have an idea I want to test.

chevdor added a commit to chevdor/substrate that referenced this pull request Jul 23, 2018
@chevdor chevdor changed the title WIP: Add docker image Add docker image Jul 23, 2018
@chevdor
Copy link
Contributor Author

chevdor commented Jul 26, 2018

@gavofyork I moved the docker part further down.

@gavofyork gavofyork merged commit 8f414ab into paritytech:master Jul 27, 2018
dvdplm added a commit that referenced this pull request Jul 30, 2018
* master: (86 commits)
  Make contract a separate runtime module (#345)
  Version bump (#450)
  DB-based blockchain data cache for light nodes (#251)
  Update libp2p again (#445)
  Update version on git head change (#444)
  Fix the public key of bootnode 3 (#441)
  Update libp2p (#442)
  Switch to the master branch of libp2p (#427)
  Export ws port 9944 and add doc (#440)
  Iterate over overlay to decide which keys to purge (#436)
  Exit signal gets its own trait (#433)
  Add docker image (#375)
  Reset peers.json if the content is not loadable (#405)
  Limit number of incoming connections (#391)
  Fix memory leaks in libp2p (#432)
  Do not queue empty blocks set for import (#431)
  5 random fixes (#1) (#435)
  Chore: fix typo (#434)
  Prevent building invalid blocks (#430)
  Better logging for public key mismatch (#429)
  ...
lamafab pushed a commit to lamafab/substrate that referenced this pull request Jun 16, 2020
liuchengxu added a commit to chainx-org/substrate that referenced this pull request Aug 23, 2021
* Simplify the logger

* Fix test

* Docs

* .

* .

* .

* No line in target
liuchengxu pushed a commit to autonomys/substrate that referenced this pull request Jun 3, 2022
Check bounds in a few places in archiver and document some panics
helin6 pushed a commit to boolnetwork/substrate that referenced this pull request Jul 25, 2023
* WIP update dependencies to crates versions

* Use released versions of sp-version and sp-arithmetic

* Attempt to align version of sp-keyring used to correspond to 4.0.0 releases. No luck but closer

* sp-keyring 4.0.0

* also sp-keyring 4.0.0 for macro crate

* simplify cargo.tomls

Co-authored-by: James Wilson <[email protected]>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A0-please_review Pull request needs code review.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants