-
Notifications
You must be signed in to change notification settings - Fork 54
Added docker image build script for k8s #206
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
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
Nice work!
One nit in the desc, not for you but for future devs: Core always routes (traffic) to private IP addresses, but the patch marks the Will take a proper look later. |
|
this needs a few changes:
|
| --build-arg BRANCH="v${VERSION}" \ | ||
| --build-arg BUILD_ARGS="${BUILD_ARGS}" \ | ||
| --tag "${IMAGE_FULL_NAME}" \ | ||
| . --push |
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.
This was not specifying --file Dockerfile_k8s so (for me) it was using the default Dockerfile.
This could be why @josibake observed networking not working? (perhaps patch not applied?)
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.
Could have been. I might have built and pushed the images and then renamed the Dockerfile but forgot to update it here. I might be misremembering but I thought I saw bitcoind compiling.
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.
We should re-push the images though as there is doubt.
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.
were the images re-pushed? if not no worries, was going to do it now as part of testing this PR
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.
I did not re-push anything
30ec94e to
1397282
Compare
|
Removed my commits for now, as we are not sure if the patch actually works? |
why not just keep the PR open for now with the commits? still works for docker, we just tell docker to assign IPs in a still need to do some thinking about k8s, but until then i dont see any point in remove commits from the PR |
|
The pr is open? |
"..with the commits" |
* remove erroneous whitespace * Reference correct dockerfile * Add Bitcoin Core v26.0
1397282 to
c191dee
Compare
|
LGTM. |
|
I am pushing v26.0 to docker hub too, so that main graph should work on k8s |
Kubernetes default networking prevents bitcoin core from routing traffic between nodes. Until we have a better workaround for that problem these images are of a modified bitcoin core that will route to private IP addresses.
Unlike warnets standard bitcoin-core images which download bitcoin core binary, these images are built from bitcoin core source so that we can apply a patch.