Skip to content

Ensure that elixir escript does not read from stdin #10108

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
Dec 14, 2023

Conversation

lukebakken
Copy link
Collaborator

@lukebakken lukebakken commented Dec 12, 2023

This change ensures that you do not have to redirect stdin from /dev/null to use rabbitmqctl and related utilities in a while / read shell loop.

References:

TODO: test with inter-node TLS.

@lukebakken lukebakken self-assigned this Dec 12, 2023
@lukebakken lukebakken force-pushed the lukebakken/figure-out-rabbitmqctl-stdin branch 2 times, most recently from 9ab2e63 to a73031c Compare December 12, 2023 16:56
@lukebakken lukebakken changed the base branch from v3.12.x to main December 12, 2023 16:56
@lukebakken lukebakken force-pushed the lukebakken/figure-out-rabbitmqctl-stdin branch from a73031c to 910e0df Compare December 12, 2023 16:56
lukebakken added a commit to lukebakken/misc that referenced this pull request Dec 12, 2023
@lukebakken
Copy link
Collaborator Author

To test, use this shell script:

https://github.com/lukebakken/misc/blob/main/sh/add_users.sh

You'll have to adjust the path to rabbitmqctl.

Without the changes in this PR, only the first user will be added from users.txt. After applying this PR, the loop will complete correctly.

This is the reason why - https://stackoverflow.com/a/13800476/1466825

It took quite a bit of trial-and-error to figure out how to correctly pass -noinput to an Elixir escript, because theoretically we should have already been passing that argument with the original code in run_escript.

@lukebakken lukebakken force-pushed the lukebakken/figure-out-rabbitmqctl-stdin branch 5 times, most recently from dc5026f to 761d592 Compare December 13, 2023 18:44
@lukebakken
Copy link
Collaborator Author

lukebakken commented Dec 13, 2023

I opened this PR against v3.12.x:

#10131

...because I can't seem to form a cluster using the latest main code.

Update: this PR fixes that issue.

@lukebakken lukebakken force-pushed the lukebakken/figure-out-rabbitmqctl-stdin branch from 6e690f3 to 12bb6a1 Compare December 13, 2023 19:00
lukebakken added a commit that referenced this pull request Dec 13, 2023
Discovered while testing
#10108 by using the
lukebakken/docker-rabbitmq-cluster project.

That project, by default, uses longnames for node names. When testing
classic peer discovery, starting a peer node would time out every time.
@lukebakken lukebakken force-pushed the lukebakken/figure-out-rabbitmqctl-stdin branch from 21e8de4 to 9c05a43 Compare December 13, 2023 22:00
lukebakken added a commit that referenced this pull request Dec 13, 2023
Discovered while testing
#10108 by using the
lukebakken/docker-rabbitmq-cluster project.

That project, by default, uses longnames for node names. When testing
classic peer discovery, starting a peer node would time out every time.

Ensure `host` is set
lukebakken added a commit that referenced this pull request Dec 13, 2023
Discovered while testing
#10108 by using the
lukebakken/docker-rabbitmq-cluster project.

That project, by default, uses longnames for node names. When testing
classic peer discovery, starting a peer node would time out every time.

Ensure `host` is set
@lukebakken lukebakken force-pushed the lukebakken/figure-out-rabbitmqctl-stdin branch from 9c05a43 to 77295d5 Compare December 14, 2023 04:35
@lukebakken lukebakken marked this pull request as ready for review December 14, 2023 04:37
lukebakken added a commit that referenced this pull request Dec 14, 2023
Discovered while testing
#10108 by using the
lukebakken/docker-rabbitmq-cluster project.

That project, by default, uses longnames for node names. When testing
classic peer discovery, starting a peer node would time out every time.

Ensure `host` is set
This change ensures that you do not have to redirect `stdin` from
`/dev/null` to use `rabbitmqctl` and related utilities in a `while` /
`read` shell loop.

References:
* https://github.com/lukebakken/vesc-1073/blob/main/delete.bash#L24-L32
* rabbitmq/support-tools#38
@lukebakken lukebakken force-pushed the lukebakken/figure-out-rabbitmqctl-stdin branch from 77295d5 to cb28ffc Compare December 14, 2023 14:14
@michaelklishin
Copy link
Collaborator

michaelklishin commented Dec 14, 2023

This does work as expected but I had to build a generic UNIX package in order to observe the difference. Bazel-build sbin/rabbitmqctl somehow wasn't picking up the changes.

@michaelklishin michaelklishin merged commit 9a1d342 into main Dec 14, 2023
@michaelklishin michaelklishin deleted the lukebakken/figure-out-rabbitmqctl-stdin branch December 14, 2023 18:25
@lukebakken
Copy link
Collaborator Author

Bazel-build sbin/rabbitmqctl somehow wasn't picking up the changes

I had to remove the contents of the generated escript/ directory for the make build to re-gen the escript. Just FYI!

michaelklishin pushed a commit that referenced this pull request Feb 29, 2024
Discovered while testing
#10108 by using the
lukebakken/docker-rabbitmq-cluster project.

That project, by default, uses longnames for node names. When testing
classic peer discovery, starting a peer node would time out every time.

Ensure `host` is set
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.

2 participants