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

Update docker configuration #376

Closed
wants to merge 6 commits into from

Conversation

beokuu
Copy link

@beokuu beokuu commented Aug 25, 2017

[EDIT] If you want to use bitshares with docker go here https://github.com/crazybits/docker-compose-files

What changed

  • rebased the image on ubuntu 16.04 and rewrote the Dockerfile following the instructions in README.md
  • running and customizing the image is now done with docker-compose if it is supported
  • removed the docker/ directory
  • added a .dockerignore to prevent some useless compilation

Usage with docker-compose

  • Install docker and docker-compose
  • Clone the repository, get on the revision you want and update the submodules:
git clone https://github.com/beokuu/bitshares-core.git
cd bitshares-core
git checkout update-docker-configuration
git submodule update --init --recursive
  • Do docker-compose up -d to build bitshare and start the node in the background
  • Grab a tea while bitshares compiles
  • When the compilation is done, do docker-compose down to stop the node
  • Customize witness_node_data_dir/config.ini, for example set
rpc-endpoint = 127.0.0.1:8090

(You might need to bind to 0.0.0.0 if you want to expose your node since knowing the container ip there is not trivial)

  • Add the port(s) needed in docker-compose.yml, for example:
    volumes:
      - ...
    ports:
      - 8090:8090
      - 9090:9090
  • Start the node again with docker-compose up -d (see compose documentation, you can do a lot of stuff with it)

Access the cli wallet

  • Do docker-compose exec witness bash to enter the container
  • In the container, do ./programs/cli_wallet/cli_wallet

Change the data directory location

To change the data directory location on the host you need to modify the docker-compose.yml file like so

    volumes:
      - /PATH/TO/DATA/DIR:/bitshares/witness_node_data_dir

Logs

To see the logs either do

  • docker-compose logs while the node is running

or

  • docker-compose up without the -d option to run the node

Update the image

  • docker-compose build will recompile bitshares if you updated your repository

Other

  • If the PR gets merged we should put the Usage part of this comment in some kind of doc
  • I'll try to maintain the docker stuff if this gets merged
  • I know this is not easily customizable for now but it should build and run on everything that supports docker (even with just docker build docker run)

@beokuu beokuu changed the base branch from fix-20170710 to master August 25, 2017 21:36
@beokuu beokuu changed the base branch from master to fix-20170710 August 25, 2017 21:36
@beokuu
Copy link
Author

beokuu commented Aug 25, 2017

I'm confused about what base I should be using for the PR..
Choosing master adds a commit that is not from me.

@pmconrad
Copy link
Contributor

Please base on develop.

@pmconrad pmconrad self-requested a review August 26, 2017 09:34
@oxarbitrage
Copy link
Member

oxarbitrage commented Aug 26, 2017

this is great, thank you very much for making it. i am going to test it as well. as @pmconrad mentions you need to push to the develop branch.

@oxarbitrage oxarbitrage self-requested a review August 26, 2017 15:10
Copy link
Contributor

@pmconrad pmconrad left a comment

Choose a reason for hiding this comment

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

First of all - I'd never really used docker until this morning, and with your instruction I finally managed to create my first container. Thanks for that! :-)

Along the way I ran into a few issues:

  • The docker-compose on my system is too old for the "version: '3'" in your file. I think you should specify the minimum version required to get a working result. "version 2" did it for me just fine.
  • The build failed for me until I removed ".git" from the dockerignore file. .git is needed for finding the git version and timestamp during build. Did you really test that?
  • My build ran out of diskspace. It worked after I moved to CMAKE_BUILD_TYPE=Release and instructed make to only build witness_node and cli_wallet.

Like I said, I haven't worked with docker before, and therefore don't know about best practices etc.. I'd expected that the resulting image is optimized for production use, i. e. kept as small as possible. To me, that'd mean installing only the resulting binaries in /bin or something and then getting rid of everything used only for building.

CMake complains that it can't find readline, curses and bzip2 headers. Since cli_wallet is being built you should add at least the readline development files.

@beokuu
Copy link
Author

beokuu commented Aug 28, 2017

Thanks for the review, sorry for the mistakes :P


should specify the minimum version required to get a working result

I chose version 3 because there is features in version 2 that are not supported in version 3 and I wanted to prevent locking an old version of docker but you're probably right.


removed ".git" (...) Did you really test that?

Nope :/
I added that because the COPY . /bitshares would have caused the image to rebuild every-time a file that is not in the .dockerignore would change and if I was committing a change to dockerfile it would recompile everything from scratch even if the change was after the compilation step.
But that is clearly not a good solution.
I should probably make another Dockerfile for dev with the repo as a volume.


My build ran out of diskspace

I wanted to do a size reduction pass but forgot.


I'd expected that the resulting image is optimized for production use

I was making the image so I could develop for bitshares but a production image makes a lot more sense for docker. I'll try to make an env file for docker that allow for a dev./prod. toggle with prod. by default.

  • What binaries (or other things) should I keep in the production image?
  • Is CMAKE_BUILD_TYPE the only flag I need to care about?

CMake complains that it can't find readline, curses and bzip2 headers. Since cli_wallet is being built you should add at least the readline development files.

Will do.

@beokuu beokuu changed the base branch from fix-20170710 to develop August 28, 2017 18:49
@beokuu beokuu force-pushed the update-docker-configuration branch from 2c20332 to 1711392 Compare August 28, 2017 19:01
@beokuu
Copy link
Author

beokuu commented Aug 28, 2017

The image is now 1.12GB instead of the previous 7+GB.
Still too much I guess.
If the executables are self contained (only depend on linux kernel/all libs statically linked (not 100% about this part)) we could base the container from a scratch image, copy the binaries and be done while having an extremely small image. I have never done microcontainers, could be interesting.

@Zapata
Copy link
Member

Zapata commented Aug 30, 2017

Did you look at crazybits images?
They are already "production" ready, modular and smaller. (@beokuu I think the compîled code it what makes your container bigger).

May be production images could be put in a dedicated official repo (forked from the one of crazybits ), and just keep a dev image in this repo.

Also it might be interesting to upgrade the Vagrant file to use the same docker "dev" container.

@beokuu
Copy link
Author

beokuu commented Aug 30, 2017

Thanks @Zapata!
If you don't fork the repo at least put a link in the README and remove the Dockerfile in this repo, this would prevent useless work.

@beokuu beokuu force-pushed the update-docker-configuration branch from 738da99 to 7ba346b Compare August 30, 2017 19:25
@xeroc
Copy link
Member

xeroc commented Sep 1, 2017

Clean an simple solution. Nice!

@beokuu beokuu force-pushed the update-docker-configuration branch from 7ba346b to 895c6ed Compare September 2, 2017 09:42
@beokuu
Copy link
Author

beokuu commented Sep 2, 2017

I've replaced the code with a development oriented image, that's what I'm using to dev on arch linux.
With:

./docker/builder/run.sh

(if your platform don't support the dirname command use
docker-compose -f ./docker/builder/docker-compose.yml -p bitshares run --rm builder)

You access an Ubuntu 16.04 container with all development dependencies installed and an unprivileged user which home is a volume of the repository. The default shell is fish, you can use git, cmake, make and other tools in the container.

I also removed the original Dockerfile and put info in the README about docker.

Copy link
Member

@oxarbitrage oxarbitrage left a comment

Choose a reason for hiding this comment

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

bumping pull, please excuse the delay, i am going to be back on this tomorrow as i think we need an updated docker working configuration. looking to merge some of this.

@oxarbitrage oxarbitrage mentioned this pull request Nov 10, 2017
@xeroc
Copy link
Member

xeroc commented Nov 13, 2017

Just to be clear on this .. the purpose of this Dockerfile is to offer a console that has the sources of bitshares and all the required dependencies installed in it? I am asking because I don't even see code that compiles bitshares in this PR.

@oxarbitrage
Copy link
Member

closing as @xeroc new docker configurations replace this and no further comment was made by author.

@oxarbitrage oxarbitrage closed this Dec 1, 2017
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.

5 participants