support floonet, separate entrypoint from cmd, add docker volume by kim0 · Pull Request #2265 · mimblewimble/grin

kim0 · 2018-12-31T18:41:15Z

Support floonet as a buildtime ARG
Separate entrypoint from cmd, allowing the use of different grin sub-commands on docker run
Adding a docker volume to hold state

kim0 · 2018-12-31T18:45:10Z

@Kargakis .. This is a continuation of #2257 which it seems I closed .. Anyway, this is a bigger revamp.

ignopeverell · 2018-12-31T18:46:49Z

This doesn't look like it's going to run Floonet (no --floonet option) but exposes future mainnet ports?

kim0 · 2018-12-31T18:50:10Z

@ignopeverell .. I default to mainnet .. To build --floonet use the below

docker build -t grin -f etc/Dockerfile . --build-arg grinnet=--floonet

Ofc, grinnet could be anything else supported by grin. If not mentioned (default), it should default to mainnet

0xmichalis · 2018-12-31T19:20:38Z

etc/Dockerfile

+RUN grin ${grinnet} server config && \
    sed -i -e 's/run_tui = true/run_tui = false/' grin-server.toml

+VOLUME ["/root/.grin"]


The volume cmd is unnecessary?

Nope, it's useful .. When it's not marked as a volume .. docker does not copy data from inside the image to that location .. so the grin-server.toml is not copied to that external location. While when marked as a volume, it is. When it's marked as a volume, we should pass a volume name like docker run ... -v dotgrin:/root/.grin .. and see the volume with docker volume ls

AFAICT, this is how docker should be used! And it works well. When bind mounting a volume -v .:/root/.grin .. docker does NOT copy the grin-server.toml file.

0xmichalis · 2018-12-31T19:21:35Z

etc/Dockerfile

 EXPOSE 13413 13414 13415 13416

-ENTRYPOINT ["grin", "server", "run"]
+EXPOSE 3413 3414 3415 3416


Add a comment that these are the mainnet ports.

0xmichalis · 2018-12-31T19:21:54Z

etc/Dockerfile

-ENTRYPOINT ["grin", "server", "run"]
+EXPOSE 3413 3414 3415 3416
+
+ENTRYPOINT ["grin"]


Shouldn't you be using grinnet here?

Not really .. since ARG is only available at buildtime. In run time, passing --floonet should just be regarded as extra cli args, which are now supported. So for running a floonet container, I use:
docker run --rm -it -p 13413-13416:13413-13416 -v dotgrin:/root/.grin --name grin grin --floonet server run

0xmichalis · 2018-12-31T19:24:10Z

docker build -t grin -f etc/Dockerfile . --build-arg grinnet=--floonet

@kim0 can you update the build docs with this?

https://github.com/mimblewimble/grin/blob/master/doc/build.md#docker

0xmichalis · 2018-12-31T19:25:32Z

etc/Dockerfile

+ARG grinnet=""
+
+RUN grin ${grinnet} server config && \
    sed -i -e 's/run_tui = true/run_tui = false/' grin-server.toml


This command is going to fail now since each network gets its own directory. Are you able to build the image correctly?

Hrm, maybe grin server config just spits out the config w/o creating the extra directory..

That was my impression too .. but when I tested, it worked .. I see the below inside the container

root@27fa945a58d6:~/.grin# ls chain_data floo grin-server.log grin-server.toml root@27fa945a58d6:~/.grin# ls floo/ root@27fa945a58d6:~/.grin# ls -la floo/ total 10 drwxr-xr-x 2 root root 3 Dec 31 19:31 . drwxr-xr-x 4 root root 6 Dec 31 19:31 .. -rw-r--r-- 1 root root 20 Dec 31 19:31 .api_secret

kim0 · 2018-12-31T19:50:26Z

@Kargakis .. I see 2 paths ... either the one above .. and I simply add a couple of lines of docs on how to build and run in both cases (floonet and mainnet) .. or we simply create 2 dockerfiles (Dockerfile for mainnet, and Dockerfile.floonet for floonet) and we basically avoid the 2 extra pieces of config when using floonet (--build-arg when building, and --floonet arg when running), at the expense of maintaining 2 separate files

Let me know which path makes more sense to you, and I'll adjust and update the readme
thanks!

kim0 · 2018-12-31T19:52:37Z

In my opinion, I think the 2 files solution would be slightly better. Also, ideally grin would accept some configs from environment variables export GRINNET=floonet or so! Like all buzzword compliant apps (cloud-native ..etc) should :)

0xmichalis · 2018-12-31T20:04:33Z

I think I prefer the separate Dockerfile approach too, it keeps the Dockerfile simpler. So maybe rename the present Dockerfile to Dockerfile.floonet? Regarding the VOLUME I don't feel strong with adding it since I don't expect most people are going to use Docker directly to deploy Grin but will use something like Kubernetes which handles volume management separately. Maybe it's fine to have it for local development?

…

On Mon, Dec 31, 2018, 20:52 Ahmed Kamal ***@***.*** wrote: In my opinion, I think the 2 files solution would be slightly better. Also, ideally grin would accept some configs from environment variables export GRINNET=floonet or so! Like all buzzword compliant apps (cloud-native ..etc) should :) — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#2265 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ADuFfwZ-DcKNGincFOM5blv8AVTUqSlzks5u-msIgaJpZM4ZlmRD> .

kim0 · 2018-12-31T20:52:58Z

@Kargakis .. Does that look better? I'm still awaiting the build on my box, which takes 24 mins :) Is there any interest to try improving performance by caching some docker layers? Let me know, and I can work on it .. it will add a few lines to the dockerfile though

0xmichalis · 2018-12-31T21:06:43Z

doc/build.md

 ```sh
 docker build -t grin -f etc/Dockerfile .
 ```
+for floonet, use `etc/Dockerfile.floonet` instead


nit: "For floonet, use etc/Dockerfile.floonet instead."

0xmichalis · 2018-12-31T21:07:00Z

doc/build.md

 ```sh
 docker run -it -d -v $HOME/.grin:/root/.grin grin
 ```
+if you prefer to use a docker named volume, you can pass `-v dotgrin:/root/.grin` instead.


0xmichalis · 2018-12-31T21:08:08Z

doc/build.md

 docker run -it -d -v $HOME/.grin:/root/.grin grin
 ```
+if you prefer to use a docker named volume, you can pass `-v dotgrin:/root/.grin` instead.
+Using a named volume copies a default grin-server.toml


The grin cache does not contain only the server config. Also, is this true for named volumes that already contain cached data?

Yeah, I know it contains other data. When docker creates a new volume .. it will copy everything in the image under /root/.grin to that volume. But it will not for a bind mounted directory. When using a named volume -v dotgrin:/root/.grin .. if the volume contains data, it will not be overwritten

0xmichalis · 2018-12-31T21:15:26Z

@Kargakis .. Does that look better? I'm still awaiting the build on my box, which takes 24 mins :) Is there any interest to try improving performance by caching some docker layers? Let me know, and I can work on it .. it will add a few lines to the dockerfile though

Yes, it looks good to me, had just a couple of comments in the docs. Regarding speeding up the docker build, we could tear down the whole build step process and bind-mount the binary from the host, so the flow for building the image would be something like this:

cargo build --release
docker build -t grin -f etc/Dockerfile

and inside the container we would

COPY target/release/grin /usr/bin/

The downside would be that we would have to keep the dependencies on the host but I think most folks who build it have the necessary deps installed already.

kim0 · 2018-12-31T21:56:20Z

@Kargakis .. I fixed the typos .. Let's get this merged if you like :)

I don't really like having the build deps on the host for reproducibility at least .. let's look at improving build performance in a separate PR .. Thanks!

0xmichalis · 2018-12-31T22:00:24Z

Thanks, lgtm

support floonet, separate entrypoint from cmd, add docker volume

1a052fe

0xmichalis reviewed Dec 31, 2018

View reviewed changes

Separate Dockerfile for floonet

16b1e48

0xmichalis reviewed Dec 31, 2018

View reviewed changes

Spelling and docs

70e96fe

0xmichalis approved these changes Dec 31, 2018

View reviewed changes

ignopeverell merged commit daa712a into mimblewimble:master Jan 2, 2019

Conversation

kim0 commented Dec 31, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kim0 commented Dec 31, 2018

Uh oh!

ignopeverell commented Dec 31, 2018

Uh oh!

kim0 commented Dec 31, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

0xmichalis commented Dec 31, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

0xmichalis Dec 31, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kim0 commented Dec 31, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kim0 commented Dec 31, 2018

Uh oh!

0xmichalis commented Dec 31, 2018 via email

Uh oh!

kim0 commented Dec 31, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

0xmichalis commented Dec 31, 2018

Uh oh!

kim0 commented Dec 31, 2018

Uh oh!

0xmichalis commented Dec 31, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

kim0 commented Dec 31, 2018 •

edited

Loading

0xmichalis Dec 31, 2018 •

edited

Loading

kim0 commented Dec 31, 2018 •

edited

Loading

0xmichalis commented Dec 31, 2018 •

edited

Loading