erlware / relx

Sane, simple release creation for Erlang
http://erlware.github.io/relx
Apache License 2.0
696 stars 233 forks source link

Force use of nodetool if proto_dist set #924

Closed GlenWalker closed 1 year ago

GlenWalker commented 2 years ago

When using a custom proto_dist (in our case a custom TLS wrapper) for distribution erl_call doesn't work as it doesn't support proto_dist, and so the extended_bin script doesn't work correctly.

Forcing use of nodetool instead of erl_call in this situation fixes the issue.

If necessary we could also check for specific values of proto_dist to avoid forcing use of nodetool when not strictly required (e.g. still use erl_call if proto_dist is inet_tcp)

tsloughter commented 2 years ago

That shouldn't be an issue. It was why we switched to erl_call. What version of OTP are you using? It should be changing to nodetool if on an older version of OTP.

GlenWalker commented 2 years ago

This is with OTP 23.2, which is new enough that relx switches to erl_call, which doesn't work with our proto_dist. I can't see anywhere in extended_bin that tries to pass proto_dist to erl_call, or anything in the erl_call docs that indicates that it supports proto_dist anywhere, but maybe I am misunderstanding something.

If I manually set USE_NODETOOL=1, or apply this patch, everything works, but otherwise it seems to fail.

tsloughter commented 2 years ago

Oh, hm, guessing I was focused on removing the need to start a node that listens on a port and has to talk to epmd that I wasn't thinking about the case of a custom protocol.

And you aren't using inet_tls but a custom TLS proto dist implementation?

GlenWalker commented 2 years ago

Yeah, it’s a thin wrapper over inet_tls that helps manage certificates. It might make sense to only fallback to nodetool if proto_dist is not one known to work with erl_call (e.g. inet_tls can still use erl_call)

tsloughter commented 1 year ago

Thanks for this, sorry it took me so long to merge. I'll be making a relx, and then there will be a rebar3 release, soon.

Do you mind opening an issue on the OTP repo about erl_call not supporting proto_dist?