Create a Dockerfile #193

ynamiki · 2020-06-04T05:09:06Z

A Dockerfile for the example described in README.md in the project root. By using this, a user can try our project without environment setup (i.e. installing SGX SDK, build, etc.). Note that, in this container, we use the SGX simulation mode so an Intel SGX-equipped processor is not required to run.

This is a part of #163. I'll try to create another Dockerfile for the HW mode later in another PR.

nhoriguchi

I confirmed that the sample application properly ran in Docker container, that's nice.

One concern is that you generate copies of YAML files for docker, which might be a burden from maintenance viewpoint (imagine that if we add some new config parameters, then we might have to update two places). I think that we can use viper's function to override configs from files with environment variables, and my brief check showed that keys in peer.yaml can be overridden by environment variable PEER_KEYS. Unfortunately, peers in consensus.yaml is loaded separately from peer.yaml so we need change application code to override it with environment variable. So I don't think this should be done in this PR.

README.md

sample/docker/README.md

nhoriguchi · 2020-06-08T06:41:36Z

sample/docker/README.md

@@ -0,0 +1,29 @@
+# Docker Image #


Maybe the title can be more descriptive like "Running sample application as Docker containers".

It could also be a concise title followed by some short description.

nhoriguchi · 2020-06-08T07:32:32Z

sample/docker/README.md

+## Prerequisites ##
+
+To try this, be sure [Docker](https://www.docker.com) (Docker Engine
+and Docker Compose) are installed on your system.


This is the first time we mention docker in this project, so it would be kind to add some basic notes on docker (I mean for example that "you need to run this as root with sudo", or "you have to check that docker service is running" or something like that) or to point to some references (Hyperledger Fabric does this). With that, we can avoid questions about docker.

sample/docker/docker-compose.yml

sergefdrv

Thanks for great work. I particularly like the way the docker file is structured with multi-stage build. I think we could also consider adding top-level make targets to build docker images and use those in the example as future work.

README.md

sergefdrv · 2020-06-08T15:19:57Z

README.md

@@ -161,6 +164,9 @@ export SGX_MODE=SIM

 ## Getting Started ##

+Note: We have [a Docker image](sample/docker/README.md) to try this
+project quickly.
+


Maybe just mention that the following steps apply to native build, and suggest trying out Docker to get started easily? Maybe we could even make it the default option at once?

sample/docker/Dockerfile

sergefdrv · 2020-06-08T16:16:36Z

sample/docker/README.md

@@ -0,0 +1,29 @@
+# Docker Image #


It could also be a concise title followed by some short description.

sample/docker/README.md

sample/docker/docker-entrypoint.sh

ynamiki · 2020-06-11T05:43:44Z

@Naoya-Horiguchi @sergefdrv Thanks for your many suggestions. I have update the code as well as the document. I the latest commit, I deleted sample/docker/README.md and move the contents to an new section "Quick Start" of README.md at the root with revising based on your comments.

ynamiki · 2020-06-11T05:54:27Z

@Naoya-Horiguchi I understand your concern. In the last commit, I tried avoiding the duplication of the config files by creating consensus.yaml dynamically; rewriting by sed based on the original sample/config/consensus.yaml.

One concern is that you generate copies of YAML files for docker, which might be a burden from maintenance viewpoint (imagine that if we add some new config parameters, then we might have to update two places). I think that we can use viper's function to override configs from files with environment variables, and my brief check showed that keys in peer.yaml can be overridden by environment variable PEER_KEYS. Unfortunately, peers in consensus.yaml is loaded separately from peer.yaml so we need change application code to override it with environment variable. So I don't think this should be done in this PR.

README.md

nhoriguchi · 2020-06-15T04:49:02Z

README.md

+
+This quick start shows you how to runnning this project using Docker
+containers. This is the easiest way to try out this project with minimal
+setup. If you want to build and run without Docker, skip this section.


(EDITED) Actually I like putting this new section subtree just before "Roadmap" section and inserting short notes as you did in the previous version. But this might conflict with Sergey's suggestion, so I'm fine if you decide to go with this.

README.md

sergefdrv

Thank you, this PR looks to me much better now! I just left some minor comments.

README.md

Makefile

ynamiki · 2020-06-19T04:18:34Z

@Naoya-Horiguchi @sergefdrv Thanks for comments. I have updated.

Changes from the previous revision (diffs):

Fix some errors in English grammar in README.
Add docker-clean target in Makefile to delete the image.
Add User option to docker-compose.yml to run the process in the container as not root but a user who has the same UID as the current user in the host. By this, the owner of keys.yaml, which is created in the container in the directory shared with host, is now the current user in the host. So we don't need sudo for deleting the file (as described in the section Tear Down). Note that, $UID is not an environment variable but a shell variable of bash, we have to pass it to docker-compose by UID=$UID docker-compose .... I have set the default value, 1000, which is the common first UID for non-root, non-system user in Ubuntu.

nhoriguchi

Looks good to me, thank you 👍

sergefdrv

Sorry, I just left a couple of tiny comments.

Makefile

README.md

This patch includes a Dockerfile for the example described in README.md. By using this, a user can try our project without environment setup. Signed-off-by: Yuta Namiki <[email protected]>

ynamiki · 2020-06-19T11:44:33Z

Thanks for suggestions! I have updated again (diffs).

sergefdrv

👍 Thanks!

ynamiki self-assigned this Jun 4, 2020

ynamiki added this to the v0.1.0 milestone Jun 4, 2020

ynamiki marked this pull request as ready for review June 4, 2020 23:26

ynamiki requested review from nhoriguchi and sergefdrv as code owners June 4, 2020 23:26

nhoriguchi reviewed Jun 8, 2020

View reviewed changes

sergefdrv reviewed Jun 8, 2020

View reviewed changes

nhoriguchi reviewed Jun 15, 2020

View reviewed changes

ynamiki requested a review from sergefdrv June 17, 2020 22:26

sergefdrv reviewed Jun 18, 2020

View reviewed changes

README.md Outdated Show resolved Hide resolved

README.md Outdated Show resolved Hide resolved

README.md Outdated Show resolved Hide resolved

Makefile Show resolved Hide resolved

ynamiki requested review from nhoriguchi and sergefdrv June 19, 2020 04:18

nhoriguchi previously approved these changes Jun 19, 2020

View reviewed changes

sergefdrv reviewed Jun 19, 2020

View reviewed changes

Makefile Outdated Show resolved Hide resolved

README.md Outdated Show resolved Hide resolved

ynamiki dismissed nhoriguchi’s stale review via 70c8562 June 19, 2020 11:15

Create a Dockerfile

487679f

This patch includes a Dockerfile for the example described in README.md. By using this, a user can try our project without environment setup. Signed-off-by: Yuta Namiki <[email protected]>

ynamiki requested review from sergefdrv and nhoriguchi June 19, 2020 11:44

sergefdrv approved these changes Jun 19, 2020

View reviewed changes

nhoriguchi approved these changes Jun 20, 2020

View reviewed changes

ynamiki merged commit c76eb0b into hyperledger-labs:master Jun 20, 2020

ynamiki deleted the docker branch June 20, 2020 06:35

ynamiki mentioned this pull request Jun 20, 2020

Create Docker container images of example applications #163

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Create a Dockerfile #193

Create a Dockerfile #193

ynamiki commented Jun 4, 2020 •

edited

Loading

nhoriguchi left a comment

nhoriguchi Jun 8, 2020

sergefdrv Jun 8, 2020

nhoriguchi Jun 8, 2020

sergefdrv left a comment

sergefdrv Jun 8, 2020

sergefdrv Jun 8, 2020

ynamiki commented Jun 11, 2020

ynamiki commented Jun 11, 2020

nhoriguchi Jun 15, 2020 •

edited

Loading

sergefdrv left a comment

ynamiki commented Jun 19, 2020

nhoriguchi left a comment

sergefdrv left a comment

ynamiki commented Jun 19, 2020

sergefdrv left a comment

Create a Dockerfile #193

Create a Dockerfile #193

Conversation

ynamiki commented Jun 4, 2020 • edited Loading

nhoriguchi left a comment

Choose a reason for hiding this comment

nhoriguchi Jun 8, 2020

Choose a reason for hiding this comment

sergefdrv Jun 8, 2020

Choose a reason for hiding this comment

nhoriguchi Jun 8, 2020

Choose a reason for hiding this comment

sergefdrv left a comment

Choose a reason for hiding this comment

sergefdrv Jun 8, 2020

Choose a reason for hiding this comment

sergefdrv Jun 8, 2020

Choose a reason for hiding this comment

ynamiki commented Jun 11, 2020

ynamiki commented Jun 11, 2020

nhoriguchi Jun 15, 2020 • edited Loading

Choose a reason for hiding this comment

sergefdrv left a comment

Choose a reason for hiding this comment

ynamiki commented Jun 19, 2020

nhoriguchi left a comment

Choose a reason for hiding this comment

sergefdrv left a comment

Choose a reason for hiding this comment

ynamiki commented Jun 19, 2020

sergefdrv left a comment

Choose a reason for hiding this comment

ynamiki commented Jun 4, 2020 •

edited

Loading

nhoriguchi Jun 15, 2020 •

edited

Loading