Make dora cli connect to remote coordinator

[Gege-Wang]

To make CLI coordinator connection configurable, such as
dora start dataflow.yaml --coordinator-addr IP
Allow users to pass a parameter when dora up, dora start, and dora list --coordinator-address to specify the address or the default address control_socket_addr().

To test this PR:

# In a remote terminal
dora up
# In a local terminal
dora list --coordinator-addr REMOTE_IP
dora start dataflow.yaml --coordinator-addr REMOTE_IP
dora logs [DATAFLOW] <NODE> --coordinator-addr REMOTE_IP
dora destroy --coordinator-addr REMOTE_IP
dora check --coordinator-addr REMOTE_IP

now, there are no CLI CI test to test it.

[phil-opp]

Thanks! Looks good to me overall, just one small suggestion.

@XxChang Thanks for reviewing!

[haixuanTao]

Before merging, can we add some CI test so that we know that this features works.

Something in the likes of adding a bash script within https://github.com/dora-rs/dora/blob/ea47a556a77f6c64f29ab07939d747edc8573671/.github/workflows/ci.yml with something like:

export IP="$(curl ipinfo.io/ip)"     
dora coordinator --addr $IP

dora list --coordinator-addr $IP

# ...

Thanks!

[haixuanTao]

Also, there is some Clippy warnings worth checking: https://github.com/dora-rs/dora/pull/513/files

[Gege-Wang]

Before merging, can we add some CI test so that we know that this features works.

Something in the likes of adding a bash script within https://github.com/dora-rs/dora/blob/ea47a556a77f6c64f29ab07939d747edc8573671/.github/workflows/ci.yml with something like:

export IP="$(curl ipinfo.io/ip)"     
dora coordinator --addr $IP

dora list --coordinator-addr $IP

# ...

Thanks!

ok, I will add it!

[haixuanTao]

Could we try to find what need to be changed for the CI to work with external IP?

[XxChang]

Could we try to find what need to be changed for the CI to work with external IP?

How about creating a docker image for testing coordinator services?

[haixuanTao]

I think that before trying docker, we need to be able to run with the same machine, same environment, just with the external IP instead of localhost.

We can have docker container testing as a follow up PR.

Does it work on your local machine?

You can try with a docker container but it's probably not going to work.

[phil-opp]

Looks good to me, thanks a lot for your work!

Could we try to find what need to be changed for the CI to work with external IP?

@haixuanTao You mean substituting the 127.0.0.1 in the CI job with the external IP of the runner? We might be able to do that using the third-party haythem/public-ip action.

[haixuanTao]

If i try:

export IP="0.0.0.0:6013"                                                                                                          (base)  ✘ 130 remotes/gege/multiple
dora coordinator --addr $IP &

dora list --coordinator-addr $IP

I'm having this warning issue:

  2024-05-29T14:20:56.596707Z  WARN dora_coordinator::listener: failed to deserialize node message

Caused by:
    invalid type: string "List", expected struct Timestamped at line 1 column 6

Location:
    /home/peter/Documents/work/dora/binaries/coordinator/src/listener.rs:38:48
    at binaries/coordinator/src/listener.rs:41

[phil-opp]

Could we try to find what need to be changed for the CI to work with external IP?

@haixuanTao You mean substituting the 127.0.0.1 in the CI job with the external IP of the runner? We might be able to do that using the third-party haythem/public-ip action.

The public IP is probably not reachable this way because of firewalls, NATs, etc. So we decided to postpone the creation of a proper CI test for now.

[Gege-Wang]

  2024-05-29T14:20:56.596707Z  WARN dora_coordinator::listener: failed to deserialize node message

Caused by:
    invalid type: string "List", expected struct Timestamped at line 1 column 6

Location:
    /home/peter/Documents/work/dora/binaries/coordinator/src/listener.rs:38:48
    at binaries/coordinator/src/listener.rs:41

dora coordinator --addr $IP1 &
dora list --coordinator-addr $IP2
I think $IP1 is not same as $IP2, $IP1 is the coordinator port, which is default DORA_COORDINATOR_PORT_DEFAULT. IP2 is the coordinator control port(default control_socket_addr()), almost all of the cli command create session using control port, but the coordinator task using the hardcode control_socket_addr(), we haven't make it configurable.

dora/binaries/coordinator/src/lib.rs
    let control_events = control::control_events(control_socket_addr(), tasks)
        .await
        .wrap_err("failed to create control events")?;

[haixuanTao]

I see. So, basically the port is not configurable yet. In that case, can we only specify IP address in the cli configuration. To avoid people using the PORT and realising it can´t be used.

[Gege-Wang]

I have tried in EC2 as the following:

#  EC2
ubuntu@ip-172-31-30-172:~/dora/test_rust_project$ dora up
started dora coordinator
started dora daemon

and local

# local
# check the EC2.   (right)
dora check --coordinator-addr 54.234.111.197:6012
Dora Coordinator: ok
Dora Daemon: ok

# check the default coordinator-adds.   (right)
$ dora check                                              1 ↵
Dora Coordinator: not running
Dora Daemon: not running

Environment check failed.

now, the control_socket_addr() is still 127.0.0.1:6012, but we can still connect to remote coordinator. I cannot understand, but it works, I think it should be 0.0.0.0:6012, but not.

dora/binaries/coordinator/src/lib.rs
    let control_events = control::control_events(control_socket_addr(), tasks)
        .await
        .wrap_err("failed to create control events")?;

@XxChang , yes, there are also some problem about dora start.

dora start dataflow.yaml --coordinator-addr 54.234.111.197:6012
Failed to read yaml dataflow: failed to open given file: No such file or directory (os error 2)

[Gege-Wang]

I see. So, basically the port is not configurable yet. In that case, can we only specify IP address in the cli configuration. To avoid people using the PORT and realising it can´t be used.

ok, I have done this in ce2b02f

[phil-opp]

dora coordinator --addr $IP1 & dora list --coordinator-addr $IP2 I think $IP1 is not same as $IP2, $IP1 is the coordinator port, which is default DORA_COORDINATOR_PORT_DEFAULT. IP2 is the coordinator control port(default control_socket_addr()), almost all of the cli command create session using control port, but the coordinator task using the hardcode control_socket_addr(), we haven't make it configurable.

Yes, exactly. The coordinator listens on two different ports:

• On DORA_COORDINATOR_PORT_DEFAULT (0xD02A = 53290) for daemon<->coordinator connections.
  • This port be configured using dora coordinator --addr 0.0.0.0:<port>
  • The daemon needs to be started with the remote IP and port of the coordinator: dora list --coordinator-addr <ip:port>
• On control_socket_addr (6012) for cli<->coordinator connections.
  • This port is not configurable yet on the coordinator side.
  • This PR adds a argument to the CLI commands to specify how the CLI can reach the coordinator.

[phil-opp]

I cannot understand, but it works, I think it should be 0.0.0.0:6012, but not.

Yes, the coordinator should listen on 0.0.0.0:6012 instead of 127.0.0.1:6012. The daemon should still connect to 127.0.0.1:6012 by default.

[haixuanTao]

Thanks! Great work!

[phil-opp]

Yes, the coordinator should listen on 0.0.0.0:6012 instead of 127.0.0.1:6012. The daemon should still connect to 127.0.0.1:6012 by default.

I implemented this in #520, among some other improvements.