Skip to content

rust: support and default to --host localhost #4804

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

Merged
merged 1 commit into from
Mar 20, 2021

Conversation

wchargin
Copy link
Contributor

@wchargin wchargin commented Mar 19, 2021

Summary:
We now interpret --host as a string that can be getaddrinfod rather
than requiring it to be a literal IP address. Thus, we can support
hostnames like localhost that may have multiple resolutions:

[src/main.rs:3] ("localhost", 6006).to_socket_addrs().unwrap().collect::<Vec<_>>() = [
    127.0.0.1:6006,
    [::1]:6006,
]

Thus, we change the default --host from ::1 to localhost to
support systems where ::1 may not be available. Should fix #4801.

We also improve the error message on bind failure: it’s pretty-printed
instead of Debug-formatted, and it includes the problematic address.

Test Plan:
From tensorboard/data/server/, run cargo build, then reproduce the
failure in Docker:

$ img=tensorflow/tensorflow@sha256:31775f136e5fe2d50ae075100dff335e9ae0954b8f9b529791a8bf739f3dd415
$ v="$PWD/target/debug:/rustboard-bin:ro"
$ docker run --rm -it -v "$v" "$img" /rustboard-bin/rustboard --logdir /tmp --host ::1
fatal: failed to bind to ("::1", 6806): Cannot assign requested address (os error 99)

…then try again with default --host and note that it works:

$ docker run --rm -it -v "$v" "$img" /rustboard-bin/rustboard --logdir /tmp
listening on 127.0.0.1:6806

(You may need to docker kill $(docker ps -q) the thing afterward.)

wchargin-branch: rust-host-getaddrinfo

Summary:
We now interpret `--host` as a string that can be `getaddrinfo`d rather
than requiring it to be a literal IP address. Thus, we can support
addresses like `localhost` that [may have multiple resolutions][play]:

```
[src/main.rs:3] "localhost:6006".to_socket_addrs().unwrap().collect::<Vec<_>>() = [
    127.0.0.1:6006,
    [::1]:6006,
]
```

[play]: https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=922073bea0c690fd3d249ff3d822cde6

Thus, we change the default `--host` from `::1` to `localhost` to
support systems where `::1` may not be available. Should fix #4801.

We also improve the error message on bind failure: it’s pretty-printed
instead of `Debug`-formatted, and it includes the problematic address.

Test Plan:
From `tensorboard/data/server/`, run `cargo build`, then reproduce the
failure in Docker:

```
$ img=tensorflow/tensorflow@sha256:31775f136e5fe2d50ae075100dff335e9ae0954b8f9b529791a8bf739f3dd415
$ v="$PWD/target/debug:/rustboard-bin:ro"
$ docker run --rm -it -v "$v" "$img" /rustboard-bin/rustboard --logdir /tmp --host ::1
fatal: failed to bind to ("::1", 6806): Cannot assign requested address (os error 99)
```

…then try again with default `--host` and note that it works:

```
$ docker run --rm -it -v "$v" "$img" /rustboard-bin/rustboard --logdir /tmp
listening on 127.0.0.1:6806
```

(You may need to `docker kill $(docker ps -q)` the thing afterward.)

wchargin-branch: rust-host-getaddrinfo
wchargin-source: 0b612771320a6f14203137fbed1b6fc627b6d1c1
@wchargin wchargin added the core:rustboard //tensorboard/data/server/... label Mar 19, 2021
@google-cla google-cla bot added the cla: yes label Mar 19, 2021
@wchargin wchargin requested a review from stephanwlee March 19, 2021 20:15
@wchargin
Copy link
Contributor Author

If it’s not too much trouble, would you mind giving this a quick test on
macOS? No Docker tomfoolery required, just:

bazel run --define=link_data_server=true //tensorboard -- --logdir ~/tensorboard_data/ --bind_all --load_fast=auto --verbosity 0

and make sure that you can see some data in TensorBoard. It should all
be fine, but with network-y things it can be helpful to double-check.

@wchargin wchargin merged commit fb40958 into master Mar 20, 2021
@wchargin wchargin deleted the wchargin-rust-host-getaddrinfo branch March 20, 2021 03:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes core:rustboard //tensorboard/data/server/...
Projects
None yet
Development

Successfully merging this pull request may close these issues.

In Docker, RustBoard can't bind to ::1
2 participants