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

rabbit_peer_discovery: Pass inetrc config file to temporary hidden node #10759

Merged
merged 1 commit into from Mar 25, 2024

Conversation

dumbbell
Copy link
Member

Why

As shown in #10728, in an IPv6-only environment, kernel name resolution must be configured through an inetrc file.

The temporary hidden node must be configured exactly like the main node (and all nodes in the cluster in fact) to allow communication. Thus we must pass the same inetrc file to that temporary hidden node. This wasn’t the case before this patch.

How

We query the main node’s kernel to see if there is any inetrc file set and we use the same on the temporary hidden node’s command line.

While here, extract the handling of the proto_dist module from the TLS code. This parameter may be used outside of TLS like this IPv6-only environment.

Fixes #10728.

@lukebakken
Copy link
Collaborator

You know, I haven't tested any of this "hidden node" stuff on Windows. I'll make time to do that with this patch and will test TLS-enabled distribution as well.

@dumbbell
Copy link
Member Author

You know, I haven't tested any of this "hidden node" stuff on Windows. I'll make time to do that with this patch and will test TLS-enabled distribution as well.

That would be great! Thank you very much :-)

@lukebakken
Copy link
Collaborator

I'm just making a note here that this PR is a follow-up to #10194

@dumbbell dumbbell force-pushed the peer-disc-temporary-nodes-and-inetrc branch from 9b41fa4 to 2433239 Compare March 19, 2024 09:09
@dumbbell dumbbell modified the milestones: 3.13.1, 4.0.0 Mar 19, 2024
@dumbbell dumbbell force-pushed the peer-disc-temporary-nodes-and-inetrc branch from 2433239 to 7926a38 Compare March 19, 2024 15:18
@lukebakken lukebakken force-pushed the peer-disc-temporary-nodes-and-inetrc branch from 7926a38 to c3e2031 Compare March 20, 2024 21:22
@dumbbell dumbbell force-pushed the peer-disc-temporary-nodes-and-inetrc branch from c3e2031 to 450f727 Compare March 21, 2024 11:34
@lukebakken lukebakken force-pushed the peer-disc-temporary-nodes-and-inetrc branch 2 times, most recently from 1498e04 to 41d3c68 Compare March 21, 2024 15:45
lukebakken added a commit that referenced this pull request Mar 21, 2024
@lukebakken lukebakken force-pushed the peer-disc-temporary-nodes-and-inetrc branch from 41d3c68 to 3fd96a0 Compare March 22, 2024 16:12
lukebakken added a commit that referenced this pull request Mar 22, 2024
@lukebakken
Copy link
Collaborator

OK, I built a Windows zip with this PR, and the inetrc argument is passed to the peer node at startup:

Peer discovery: peer node arguments: #{args =>
                                           ["-ssl_dist_optfile",
                                            "C:/Users/lbakken/development/lukebakken/windows-rabbitmq-cluster/rmq1 Евгений/inter_node_tls.config",
                                            "-kernel","inetrc",
                                            "\"C:/Users/lbakken/development/lukebakken/windows-rabbitmq-cluster/erl_inetrc\"",
                                            "-proto_dist","inet_tls","-pa",
                                            "c:/Users/lbakken/scoop/apps/erlang/26.2.3/lib/ssl-11.1.2/ebin",
                                            "-boot","start_sasl","-hidden"],
                                       name => "rmq1-4-22800"}

Cluster formation worked correctly, as well. Here is the erl_inetrc file I used:

https://github.com/lukebakken/windows-rabbitmq-cluster/blob/tls-disterl-utf8/erl_inetrc

@michaelklishin michaelklishin marked this pull request as ready for review March 25, 2024 18:25
@michaelklishin
Copy link
Member

The reporter suggests that a package built off of this branch does work as expected (the way 3.12.x did in the same environment) #10728 (comment).

[Why]
As shown in #10728, in an IPv6-only environment, `kernel` name
resolution must be configured through an inetrc file.

The temporary hidden node must be configured exactly like the main node
(and all nodes in the cluster in fact) to allow communication. Thus we
must pass the same inetrc file to that temporary hidden node. This
wasn’t the case before this patch.

[How]
We query the main node’s kernel to see if there is any inetrc file set
and we use the same on the temporary hidden node’s command line.

While here, extract the handling of the `proto_dist` module from the TLS
code. This parameter may be used outside of TLS like this IPv6-only
environment.

Fixes #10728.
@michaelklishin michaelklishin force-pushed the peer-disc-temporary-nodes-and-inetrc branch from 3fd96a0 to 1bcfa47 Compare March 25, 2024 19:19
@michaelklishin
Copy link
Member

The forced push was a rebase.

@michaelklishin michaelklishin merged commit 7885182 into main Mar 25, 2024
19 checks passed
@michaelklishin michaelklishin deleted the peer-disc-temporary-nodes-and-inetrc branch March 25, 2024 22:44
michaelklishin added a commit that referenced this pull request Mar 26, 2024
rabbit_peer_discovery: Pass inetrc config file to temporary hidden node (backport #10759)
dumbbell added a commit that referenced this pull request Mar 26, 2024
…odes-and-inetrc"

The patch was a draft and should not have been merged into `main`.

This reverts commit 7885182, reversing
changes made to 524d688.
@dumbbell
Copy link
Member Author

The patch was not ready (hence the draft status) and should not have been merged. I reverted it.

michaelklishin pushed a commit that referenced this pull request Mar 27, 2024
…odes-and-inetrc"

The patch was a draft and should not have been merged into `main`.

This reverts commit 7885182, reversing
changes made to 524d688.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

RabbitMQ 3.13.0 nodes with peer discovery enabled fails to start in an IPv6-only environment
3 participants