-
Notifications
You must be signed in to change notification settings - Fork 141
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
feat: docker images for iroh #2404
Conversation
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.
Also to make this easy to connect to other services on the local host, best bet is to run in network mode host.
@Arqu once we merge this, can we automate it, to publish this to dockerhub on release? |
Dockerfile
Outdated
RUN cargo build --bin iroh --release --all-features | ||
|
||
### Final Image | ||
FROM ubuntu:latest as iroh |
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.
can we switch this to alpine?
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.
or sth else that is smaller
a6ff3e3
to
4b01a61
Compare
@Arqu we should have one minimal test in CI running the docker image and calling |
We got a node stats test now too. Also should we title it as breaking? Document the rpc changes? |
iroh-cli/src/commands.rs
Outdated
|
||
/// Address to serve RPC on. | ||
/// | ||
/// This overrides the address found in the IPFS_PATH. |
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.
What IPFS_PATH
?
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.
great question..
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.
fixed
iroh/src/node/builder.rs
Outdated
@@ -316,8 +320,10 @@ where | |||
} | |||
|
|||
/// Configure the default iroh rpc endpoint. | |||
pub async fn enable_rpc(self) -> Result<Builder<D>> { | |||
let (ep, actual_rpc_port) = make_rpc_endpoint(&self.secret_key, DEFAULT_RPC_PORT)?; | |||
pub async fn enable_rpc(self, rpc_addr: Option<SocketAddr>) -> Result<Builder<D>> { |
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.
Can we keep the default builder API simple? I would guess that 99% of all users will want to enable rpc with the default value here.
So either .enable_rpc()
or .enable_custom_rpc(AddrOpt)
or maybe enable_rpc_with_opts(RpcOpts)
in case we expect more options in the future?
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.
fixed
iroh/src/node/builder.rs
Outdated
let rpc_quinn_endpoint = match rpc_quinn_endpoint { | ||
Ok(ep) => ep, | ||
Err(err) => { | ||
if err.kind() == std::io::ErrorKind::AddrInUse { | ||
tracing::warn!( | ||
"RPC port {} already in use, switching to random port", | ||
rpc_port | ||
rpc_addr.port(), |
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 probably log the entire addr.
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.
fixed
build_docker.sh
Outdated
@@ -0,0 +1,2 @@ | |||
docker buildx build -f docker/Dockerfile --target iroh --platform linux/arm64/v8 --tag n0computer/iroh:latest . | |||
# docker buildx build -f docker/Dockerfile --target iroh --platform linux/amd64 --tag n0computer/iroh:latest . |
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.
what about this?
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 drop this, stuff is already there in the readme.
## Building | ||
|
||
- All commands are run from the root folder | ||
- If you're on macOS run `docker buildx build -f docker/Dockerfile --target iroh --platform linux/arm64/v8 --tag n0computer/iroh:latest .` |
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.
what does buildx
mean?
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.
Buildx is an official extension by docker for the build system for images. Gives you the ability to specify platforms and some other nifty stuff.
Description
Breaking Changes
iroh::node::Node::my_rpc_port
is nowmy_rpc_addr
and returnsOption<SocketAddr>
iroh::node::Builder::rpc_endpoint
takesOption<SocketAddr> instead of
Option`iroh::node::Builder::enable_rpc_with_addr
allowing to specify on which address + port to bind toNotes & open questions
Change checklist