-
Notifications
You must be signed in to change notification settings - Fork 87
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
ref(docker): create a standalone docker artifact to run lightwalletd
#476
base: master
Are you sure you want to change the base?
Conversation
lightwalletd
lightwalletd
Also, as this is now using a multistage Dockerfile, this can be easily extended to add a |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
utACK with suggestion.
I'd like to follow-up with some extra changes on top of this, @daira @LarryRuane are you aware if there's something missing here to get it merged? |
I don't understand how to run this (to test it). I'm sure part of the problem is that I'm unfamiliar with docker. For example, am I expected to start a Also I see that this PR needs to be rebased onto the latest master branch. (I did that here locally, it wasn't difficult, but if you'd like I can push my branch to GitHub.) |
I see this has been approved now, thank you! Is there anything in particular that is preventing this PR from merging? |
@LarryRuane I'll be adding the process and documentation so it's clearer and easier to test this changes. |
No, we can merge it, but I just wanted to do some testing myself as an extra safety margin. But since this PR makes only docker-related changes, it's safe for anyone not using docker. Now that @gustavovalverde will be adding some documentation, we could wait for this and have me test it. Or would you rather have me merge it right away? I'm okay with either plan, I don't want to hold anyone up. |
Let's wait for the extra documentation to be on the safe side. |
This also includes documentation on how to run it
Hi @LarryRuane! I've added extra testing steps into the PR body, and also in the documentation. I added an extra |
Please ensure this checklist is followed for any pull requests for this repo. This checklist must be checked by both the PR creator and by anyone who reviews the PR.
As a note, all CI tests need to be passing and all appropriate code reviews need to be done before this PR can be merged
Motivation
This PR is meant to align lightwalletd docker execution to what we previously had working in ZcashFoundation/zebra#8323 (but we no longer need in our repo). Allowing users to use an standalone lightwalletd docker image without further configurations, allowing users to just do:
And have a working lightwalletd server out-of-the-box (at least for testing/development purposes)
Changes
golang:1.19
as 1002f42 made this a requirement, considering some dependencies no longer work withgolang:1.17
go
instead ofmake
to build inside the image (this is the expected approach with Docker)docker_img_run_zcashd
as we don't havezcashd nor
zcash-cli` inside the container.dockerignore
file to just have the required file inside the containerlightwalletd
, and with a considerably reduced image sizeARGs
, so people can change their values if they'd like to build the image locally.zcash.conf
to use this image as a standalone artifact without further configurationscert.key
andcert.pem
from the repo to avoid running it with--no-tls-very-insecure
by defaultTesting
You can test this using:
docker build --target runtime --tag local/lightwalletd . docker run -p 9067:9067 -p 9068:9068 local/lightwalletd
If you'd like to test this running alongside
Zebra
, you can test with: