Skip to content
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

extended_bin: Support 'inet_dist_use_interface' in vm.args #891

Merged
merged 1 commit into from
Dec 31, 2021

Conversation

weiss
Copy link
Contributor

@weiss weiss commented Nov 9, 2021

Support specifying the kernel option inet_dist_use_interface in the vm.args file. If ERL_DIST_PORT is set while using Erlang/OTP 23 or newer, make sure to hand over the inet_dist_use_interface address to erl_call's -address option.

My own use case is binding the VM to 127.0.0.1 (if not actually running a cluster) and still being able to use ERL_DIST_PORT.

@weiss weiss force-pushed the support-dist-addr branch 2 times, most recently from 7697a70 to c1722bc Compare November 9, 2021 11:05
@tsloughter
Copy link
Member

Thanks. But weird that the command line kernel option is a tuple and not just an ip string. I don't like having to parse that in shell script :( but I guess no way around it.

Support specifying the kernel option 'inet_dist_use_interface' in the
vm.args file. If ERL_DIST_PORT is set while using Erlang/OTP 23+, make
sure to hand over the 'inet_dist_use_interface' address to erl_call.
@weiss weiss force-pushed the support-dist-addr branch from c1722bc to 36137dd Compare November 12, 2021 09:15
@weiss
Copy link
Contributor Author

weiss commented Nov 12, 2021

I don't like having to parse that in shell script :(

Yes, ugliness 😒

I guess no way around it.

I see no way, except maybe giving up on the idea of specifying the address to use by the main node and by erl_call in a single place. I.e., just add some RPC_ADDRESS (or whetever) variable for erl_call to use, and let the user specify both inet_dist_user_interface and that new variable. (Would be good enough for me personally, and would allow for keeping the inet_dist_user_interface setting in sys.config, where it looks less ugly to me. But would be ugly in a different way.)

@weiss
Copy link
Contributor Author

weiss commented Dec 23, 2021

I see no way

Well I guess another way would be using Erlang instead, e.g. something along the following lines. But it's probably preferable not to spin up the VM for that job.

# Convert {127,0,0,1} to 127.0.0.1
addr_tuple_to_str() {
    addr="$1"
    code="catch io:format(\"~s\", [inet:ntoa($addr)]), halt()."
    result="$("$BINDIR/erl" -noinput -boot no_dot_erlang -eval "$code")"

    if [ -n "$result" ]; then
        printf '%s' "$result"
    else
        echo "Cannot parse IP address tuple: '$addr'" >&2
    fi
}

@tsloughter
Copy link
Member

Oh, duh, its using a tuple because its using -kernel ... so it is not an exposed erl arg like my original thinking.

@@ -674,8 +690,17 @@ EPMD_MODULE="$(grep '^-epmd_module' "$VMARGS_PATH" || true)"
if [ "$EPMD_MODULE" ]; then
DIST_ARGS="${DIST_ARGS} ${EPMD_MODULE}"
fi
INET_DIST_USE_INTERFACE="$(grep '^-kernel *inet_dist_use_interface' "$VMARGS_PATH" || true)"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why the *?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to cope with the user adding superflous whitespace in vm.args. I can totally live without that 😄

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean additional spaces between -kernel and inet_dist_use_interface.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd at least use it in quotes if it were possible, to clarify the intent.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry I probably need more coffee, you'd add quotes where exactly?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah nevermind, I misread the regex and nested argument. The quotes are already there. I had initially read it as 'grep ...' rather than grep '...'

@weiss
Copy link
Contributor Author

weiss commented Dec 31, 2021

Oh, duh, its using a tuple because its using -kernel ... so it is not an exposed erl arg like my original thinking.

Yes. That's the (only) way of telling the VM to dist-listen on localhost, I think? Which doesn't seem like an obscure use case to me? (I just want to use the extended start script for controlling my daemon, rather than running an actual cluster.)


if [ "$ERL_DIST_PORT" ]; then
if [ "$INET_DIST_USE_INTERFACE" ]; then
ADDRESS="$(addr_tuple_to_str "${INET_DIST_USE_INTERFACE#*inet_dist_use_interface }"):$ERL_DIST_PORT"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure what is going on here with ${INET_DIST_USE_INTERFACE#*inet_dist_use_interface }?

And have to make sure it is posix shell compliant? Not bash.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, looks like shellcheck caught something else so this must have passed and thus work ok with posix shell.

But I still don't know what it is doing :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

${INET_DIST_USE_INTERFACE#*inet_dist_use_interface }

At that point INET_DIST_USE_INTERFACE is e.g. -kernel inet_dist_use_interface {127,0,0,1}. The #*inet_dist_use_interface thing is stripping everything up to the actual address tuple.

And have to make sure it is posix shell compliant?

It is.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shall I try to come up with something less magic, more readable?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahh, no, this is good and makes sense. I was misreading (still early :) and thought it was like ${ADDRESS#*inet_dist_use_interface } so had no idea why inet_dist_use_interface was involved when I thought it was already just the address :)

@tsloughter
Copy link
Member

Not obscure at all. Something we should have supported for using the shell, rpc, etc stuff a long long time ago :).

I think this is good, just being cautious since tiny changes in this particular code has broken certain users before. It is a mess.

@weiss
Copy link
Contributor Author

weiss commented Dec 31, 2021

just being cautious since tiny changes in this particular code has broken certain users before. It is a mess.

Totally makes sense, I'm very happy with such detailed review! 👍

@tsloughter tsloughter merged commit 14f80c9 into erlware:master Dec 31, 2021
@weiss weiss deleted the support-dist-addr branch December 31, 2021 19:50
weiss added a commit to processone/eturnal that referenced this pull request Jan 2, 2022
Specify 'inet_dist_use_interface' in vm.args rather than sys.config.
This lets the extended start script parse the IP address, which allows
for specifying the environment variable ERL_DIST_PORT to avoid using
EPMD, which is now a documented use case.  This feature requires at
least Erlang/OTP 23.1 and Rebar3 3.18.0.  For details, see:

https://blog.erlware.org/epmdlessless/
erlware/relx#891
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants