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

Docs4Doge: Dockerfile implementation #2337

Closed
wants to merge 16 commits into from
Closed

Docs4Doge: Dockerfile implementation #2337

wants to merge 16 commits into from

Conversation

AbcSxyZ
Copy link
Contributor

@AbcSxyZ AbcSxyZ commented Jun 23, 2021

As the next Docs4Doge step, to continue #2285, I worked on a Dockerfile + docker-entrypoint.py.

Tried to make something flexible, with configuration and binary. I let you try it out & make review, bug report, improvement, it can be a good base for docker installations.

Test Dockerfile

#Go into a new dir
mkdir test_docker

#Get Dockerfile & script
wget https://raw.githubusercontent.com/AbcSxyZ/dogecoin/docker/contrib/docker/docker-entrypoint.py
wget https://raw.githubusercontent.com/AbcSxyZ/dogecoin/docker/contrib/docker/Dockerfile

#Build image & run
docker build -t dogecoin .
docker run --name doge-container dogecoin

#Use dogecoin-cli within container
docker exec doge-container dogecoin-cli help

Test some configured container :

# Change rpc credentials
# With env
docker run -e RPCUSER=test -e RPCPASSWORD=123456 dogecoin
# With arguments
docker run dogecoin -rpcuser=test -rpcpassword=123456

# Volumes (use it for custom dogecoin.conf)
docker run -v  $(pwd)/data:/dogecoin/.dogecoin dogecoin

# Run from VPS
docker run -p 22555:22555 -p 22556:22556 -e RPCUSER=test -e RPCPASSWORD=123456 -e RPCALLOWIP=0.0.0.0/0 dogecoin
# Use local cli with remote node
docker run dogecoin dogecoin-cli -rpcuser=test -rpcpassword=123456 -rpcconnect=[DOMAIN|IP] help

Usage

The container syntax is the following:

docker run  [docker_options] image [dogecoin-binary] [binary-arguments]

docker_options : Set environment variables, ports, volumes and other docker settings
image : dockerfile builded image
dogecoin-binary : You can choose between dogecoind, dogecoin-cli, dogecoin-tx or dogecoin-qt. Default is dogecoind.
binary-arguments : Pass arguments directly to binary.

You can use both environment variables or arguments to set configuration. All options are available in both format.
You can also use dogecoin.conf within a volume.

Designs to discuss

Use of GitHub releases instead of sources

I do not compile from source but use Dogecoin releases.

Pro :

  • Download few resources
  • Save compilation time
  • Easy if you just want to run node

Cons :

  • Not cool for development, can't edit sources/configs.

Should we have something easy & fast to setup to deploy a node, or also docker setup for development ?

Environment variables using man options.

To get all options available as environment variable, it parses the man page of used executable, searching for variable with the same value.
Probably some limits, need feedback !

P.S.: dogecoin-qt not available, need to connect to remote X host, didn't try, possible improvement + Documentation not done, on to do list.

Related to #2367 & #2366

@AbcSxyZ
Copy link
Contributor Author

AbcSxyZ commented Jun 23, 2021

I'm actually wondering if compiling from source is better or not. As standalone, I think it's better using release because it does not need to build, but for a repository Dockerfile, using sources is perhaps more interesting, like in #2285.

I need to check with other repo, how Docker Hub images are built, if it uses sources or not. Thought ?

docker-entrypoint.py Outdated Show resolved Hide resolved
@rnicoll rnicoll added this to the 1.14.4 milestone Jun 27, 2021
dgs2019
dgs2019 previously approved these changes Jun 28, 2021
@AbcSxyZ AbcSxyZ marked this pull request as draft July 22, 2021 12:46
@patricklodder patricklodder modified the milestones: 1.14.4, 1.14.5 Aug 20, 2021
@patricklodder patricklodder changed the base branch from 1.14.4-dev to 1.14.5-dev August 20, 2021 21:24
@patricklodder patricklodder dismissed dgs2019’s stale review August 20, 2021 21:24

The base branch was changed.

@AbcSxyZ AbcSxyZ mentioned this pull request Sep 5, 2021
@AbcSxyZ AbcSxyZ marked this pull request as ready for review September 10, 2021 09:56
"""
# Use dogecoind --help menu to find available options
man_path = container_executable_man()
raw_options = subprocess.check_output(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Use regex instead of subprocess

Copy link
Contributor Author

Choose a reason for hiding this comment

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

To verify if it's convenient with raw man page syntax

dgs2019
dgs2019 previously approved these changes Sep 11, 2021
@AbcSxyZ
Copy link
Contributor Author

AbcSxyZ commented Sep 14, 2021

I added the Dockerfile documentation inside INSTALL.md. For me, it can be integrated, it will need a first stage of reviews. I need feedback about the development situation, to see if we use releases or source code.

It's related to other PRs about installation instruction refresh (#2465) and getting started tutorial(#2568), to facilitate install and usage of Dogecoin Core.

@evaluationcopy
Copy link

I suggest updating the documentation to urge users of dogecoind to create a volume for their data directory.

Each time a user casually performs a docker run, their data directory will be lost. This will initiate a new download of the blockchain data which can take days or weeks, and unnecessarily pulls resources from the nodes on the network.

I see two perspectives to consider in the documentation:

  1. A user that casually decides to run a node. They may not realize the significance of the dogecoin data directory or that each time they run the docker image they are downloading the entire blockchain.
  2. A user that knows about the dogecoin data directory but doesn't know how to point a volume to it or where the image expects it.

My method of urging users to use a volume in #2578 was to only give an example of running the image with a volume.

@AbcSxyZ
Copy link
Contributor Author

AbcSxyZ commented Sep 20, 2021

Nice suggestion, it's certain someone will to do the mistake !

I updated the documentation to reflect it, what do you think about the following : c39b2a3 ?

I decided to make it really visible by adding a warning at the top and to use it inside the example.
If someone copy/paste instruction, he will have a volume, and in any case he should see this information.

@AbcSxyZ
Copy link
Contributor Author

AbcSxyZ commented Sep 25, 2021

@chromatic comment in Discord:

I'm curious at the use of sudo with docker when launching these containers though, but the entire approach seems sound.

About use of sudo to run commands, idk, I usually see it like that within documentation for docker. But it's possible to add users to docker group to avoid use of sudo, maybe we can suggest doing it that way.

In github repositories at least, but if you take a look to official mysql images on Docker hub, they do not use sudo.

I will change it, use of sudo is convenience, restrict it is security.

@patricklodder
Copy link
Member

@AbcSxyZ first off, thanks for all the work you did on this, it's much improved now!!!

could you please give me a ping when you think this is ready so I can do the review?

@AbcSxyZ
Copy link
Contributor Author

AbcSxyZ commented Sep 27, 2021

I'm checking bitcoin related issues/PRs about docker, I didn't think about it, not the reflex. Look like it's still a WIP situation since years.

I'm currently comparing my Dockerfile with other bitcoin docker repos from the following comment and also verifying Dockerfile best practices to see possible improvements.

After that, it should be ready :)

@AbcSxyZ
Copy link
Contributor Author

AbcSxyZ commented Sep 28, 2021

Whenever you want @patricklodder you can do a first review, I'm good :)

There is some field to improve (hash verification at least), but I'm a bit limited by the current configuration (no signature files directly with releases).
Let's see what you think about this current Dockerfile.

@@ -32,7 +32,10 @@ $ wget https://raw.githubusercontent.com/dogecoin/dogecoin/master/contrib/docker
$ wget https://raw.githubusercontent.com/dogecoin/dogecoin/master/contrib/docker/Dockerfile

# Build the image & start a node
# Use `--build-arg VERSION=x.y.z` to specify a version
Copy link
Member

Choose a reason for hiding this comment

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

Does this default to the VERSION defined in Dockerfile if not provided? That might be worth mentioning here.

I thought I saw a release script or release commit that Ross (?) had to update the version string right before a release.

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, by default the variable of Dockerfile, otherwise it overrides it. But actually, it won't work, hash is hardcoded.

I didn't know how I should check signature during the download, hashes seem ton be only available on gitian.sigs and not with released. I wasn't sure if I should use sigs folder, or if signatures have to be with tar files of the release.

In my mind, it's a possible improvement area, and it may improve security in the same time. I commented the Dockerfile about it.

Copy link
Member

@patricklodder patricklodder left a comment

Choose a reason for hiding this comment

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

I've given a LOT of thoughts to the version pointer and I think that one of the following has to happen:

  1. Put this in a separate dogecoin/docker repo, or
  2. The branching strategy needs to be changed to make master the leading branch for development (but then the question arises: which base branch? We have more than one at a time 😕)

My preference would be option 1.

@rnicoll @michilumin @langerhans does any of you have an objection to creating dogecoin/docker to solve version pointer issues for once and for all? Or do you have any alternatives?

(Other comments inline)

contrib/docker/Dockerfile Show resolved Hide resolved
contrib/docker/README.md Show resolved Hide resolved
contrib/docker/Dockerfile Show resolved Hide resolved
contrib/docker/Dockerfile Show resolved Hide resolved
contrib/docker/README.md Show resolved Hide resolved
contrib/docker/README.md Show resolved Hide resolved
contrib/docker/README.md Show resolved Hide resolved
Comment on lines +30 to +32
# Alternatively, download Dockerfile & entrypoint script
$ wget https://raw.githubusercontent.com/dogecoin/dogecoin/master/contrib/docker/docker-entrypoint.py
$ wget https://raw.githubusercontent.com/dogecoin/dogecoin/master/contrib/docker/Dockerfile
Copy link
Member

Choose a reason for hiding this comment

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

Let's leave this out for now? It gives a hard path to master, and master is slow to get updated. So this would introduce some risk.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The benefit is to be able to run your daemon by downloading only 2 files.

Wouldn't be equal to latest release ? 1.14.4-maint & master are at the same commit

Copy link
Member

Choose a reason for hiding this comment

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

Well yes but it won't point there because you don't know the hash of the release until it's moved to master, and adding a commit changes the hash. So that would create a problem. You will always run the previous version on that Dockerfile then... hence my request for dogecoin/docker.

Copy link
Contributor Author

@AbcSxyZ AbcSxyZ Oct 11, 2021

Choose a reason for hiding this comment

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

Did you see #2337 (comment) ?

Actually, hard coded hash may be removable if we use an external sigs files. Is it interesting to add this file to the release ?
See the following comment in Dockerfile :

# Security: more secure way than check hash for download,

# Security: more secure way than check hash for download,
# see https://github.com/docker-library/official-images#security

Hash may not be an issue, but good point I missed in the current state !

Copy link
Member

Choose a reason for hiding this comment

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

Did you see #2337 (comment) ?

Yes.

Actually, hard coded hash may be removable if we use an external sigs files. Is it interesting to add this file to the release ?

Hmm that would centralize things a bit because you'd need a single authority to sign it. Anyone can verify the authenticity of the hash through gitian right now, so that's both secure and decentralized. I don't like making the Dogecoin Core process worse because of an external implementation, so instead we could list the hashes in a separate file on release though, that would not be disruptive, just need a process that checks the published hashes and files against gitian both. (It's somewhere deep down on my todo, once there's no more crises) Either way, even requiring a hash list is something that tightly couples this with Dogecoin Core processes and it shouldn't really. It's better to be independent.

Also, per my original, broader, comment, please note that the link into a branch can still cause issues in case you would need to solve a bug or upgrade the image source outside of the Dogecoin Core release process... and perhaps more. It's not something we'll solve by keeping it here, it's much better to segregate this. All I need is one more waiver of objections from a maintainer and I'll set the separate repo up with you.

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'm not able to understand the implication it creates to add this signature file & how a central authority is needed, because at least I do not understand the flow used to check hashes in the https://github.com/docker-library/official-images#security examples.

Gitian is the decentralized way to check hashes, you can check gitian.sigs, but are people doing it & wouldn't it be a way to have more people checking hashes to add the list to releases ? It should give more incentive.

@rnicoll
Copy link
Contributor

rnicoll commented Oct 11, 2021

I've given a LOT of thoughts to the version pointer and I think that one of the following has to happen:

1. Put this in a separate dogecoin/docker repo, or

2. The branching strategy needs to be changed to make `master` the leading branch for development (but then the question arises: which base branch? We have more than one at a time 😕)

My preference would be option 1.

@rnicoll @michilumin @langerhans does any of you have an objection to creating dogecoin/docker to solve version pointer issues for once and for all? Or do you have any alternatives?

(Other comments inline)

I think dogecoin/docker makes sense, and in particular then leads neatly to that potentially having its own CI process which pushes to Dockerhub (in the distant future).

Also, separately, I'd quite like main to be the current release-grade development (in this case, 1.14.x until 1.21's first release, and from there we should maintain confidence that the tests on 1.21 are sufficient that what's in main is good enough for anyone who can compile it, to have an acceptable experience). However that discussion shouldn't hinder this.

@patricklodder
Copy link
Member

then leads neatly to that potentially having its own CI process which pushes to Dockerhub (in the distant future).

Great idea! IIRC one of the rosetta-dogecoin devs maintains the dogecoin org on docker hub and has in the past offered to do a handover. Once we have a repo and a working image, I'll reach out.

Also, separately, I'd quite like main to be the current release-grade development (in this case, 1.14.x until 1.21's first release, and from there we should maintain confidence that the tests on 1.21 are sufficient that what's in main is good enough for anyone who can compile it, to have an acceptable experience). However that discussion shouldn't hinder this.

I have many questions for you on this one, so once you get a moment, would you mind proposing all of that to discussions?

@AbcSxyZ
Copy link
Contributor Author

AbcSxyZ commented Oct 11, 2021

I definitively agree to organize it with Dockerhub in mind.

I never did it myself so no experience, I'm really wondering what is the best way to achieve this. I think about it because there is 2 ways for Dockerhub to get an image [Dockerhub use builded image if i'm right] :

  1. Build & send the image yourself (free)
  2. Dockerhub pull an image & build it from a remote Dockerfile (paid user & open source organization).

The 2nd way is restricted to enterprise or for open source organization who should apply somewhere on the web to get the solution for free.
I asked if they can do it for us, no answer 😅

I don't know if it's possible to do everything from a single Dockerfile, inside the repo, matching his branch/release ? Or multiples Dockerfiles are needed, to deal with versions for example ?
If it's a single file, who may not change that much, idk if keeping it internally could make sense. If multiple Dockerfiles are needed, or something more complex (compose, whatever), for sure it would be great to split it. Also, what about a Dockerfile for development too, would it be interesting & what to do in this case ?

Without more answer, idk what would be the best between a single repo or a separate repo. Depend on how Dockerhub works, what is possible to get (foundation would apply?), how a Dockerfile need to be maintained...

@xanimo
Copy link
Member

xanimo commented Oct 11, 2021

I definitively agree to organize it with Dockerhub in mind.

I never did it myself so no experience, I'm really wondering what is the best way to achieve this. I think about it because there is 2 ways for Dockerhub to get an image [Dockerhub use builded image if i'm right] :

  1. Build & send the image yourself (free)
  2. Dockerhub pull an image & build it from a remote Dockerfile (paid user & open source organization).

The 2nd way is restricted to enterprise or for open source organization who should apply somewhere on the web to get the solution for free. I asked if they can do it for us, no answer 😅

I don't know if it's possible to do everything from a single Dockerfile, inside the repo, matching his branch/release ? Or multiples Dockerfiles are needed, to deal with versions for example ? If it's a single file, who may not change that much, idk if keeping it internally could make sense. If multiple Dockerfiles are needed, or something more complex (compose, whatever), for sure it would be great to split it. Also, what about a Dockerfile for development too, would it be interesting & what to do in this case ?

Without more answer, idk what would be the best between a single repo or a separate repo. Depend on how Dockerhub works, what is possible to get (foundation would apply?), how a Dockerfile need to be maintained...

you can setup ci between github repo/branches and docker hub so any pushes trigger image build automatically and without paid subscription unless they changed something since last i looked.

my vote is for a separate repo.

@patricklodder
Copy link
Member

@michilumin @langerhans does either of you have any objections to creating dogecoin/docker?

@patricklodder
Copy link
Member

Taking this out of the 1.14.5 milestone in anticipation of a separate repository (@AbcSxyZ I sent you an email on Oct 17th about this, to smoothly coordinate getting this source into a new repo)

@patricklodder patricklodder removed this from the 1.14.5 milestone Nov 2, 2021
@patricklodder patricklodder added the external Requests which are outside our control label Nov 2, 2021
@patricklodder patricklodder self-assigned this Nov 2, 2021
@AbcSxyZ
Copy link
Contributor Author

AbcSxyZ commented Nov 6, 2021

Yes, let's do it. I wasn't sure it was you, it's better to work publicly :) Fine, let's create a repository and work there. You tell me to reach you on Discord, if you want to chat about organization, let's talk on discord in #core-dev-discussion, I visit regularly the chat.

@patricklodder patricklodder changed the base branch from 1.14.5-dev to 1.14.6-dev November 8, 2021 14:15
@patricklodder
Copy link
Member

I have cherry-picked this into https://github.com/dogecoin/docker, minus the readme for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
external Requests which are outside our control
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants