Skip to content

rabbitmq_peer_discovery_consul: Set bind_addr to 127.0.0.1 in test config #12858

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 3 commits into from
Dec 2, 2024

Conversation

dumbbell
Copy link
Collaborator

@dumbbell dumbbell commented Nov 29, 2024

Why

The test configuration was querying a network interface IP address based on its name. However, the name, "eth0", is very specific to Linux. This broke the test on other systems.

How

We still have to set an explicit bind_addr because Consul refuses to start if the host has multiple private IPv4 addresses, as it is the case in CI.

Therefore, we hard-code 127.0.0.1 as the IPv4 address to use because it has a great chance to exist about anywhere.

V2: We abandon the idea of not setting bind_addr because GitHub Actions default hosts have several private IPv4 addresses configured.

@dumbbell dumbbell self-assigned this Nov 29, 2024
@michaelklishin
Copy link
Collaborator

Allowing Consul to bind to all interfaces is definitely a better idea.

@dumbbell dumbbell force-pushed the consul-do-not-set-bind_addr-in-test-config branch from 13f8389 to 5ba26fc Compare December 2, 2024 10:13
…config

[Why]
The test configuration was querying a network interface IP address based
on its name. However, the name, "eth0", is very specific to Linux. This
broke the test on other systems.

[How]
We still have to set an explicit `bind_addr` because Consul refuses to
start if the host has multiple private IPv4 addresses, as it is the case
in CI.

Therefore, we hard-code 127.0.0.1 as the IPv4 address to use because it has a
great chance to exist about anywhere.
[Why]
It helps diagnose any startup issues. Typically a problem with the
configuration where the log file is not yet created.
@dumbbell dumbbell force-pushed the consul-do-not-set-bind_addr-in-test-config branch from 5ba26fc to b03f0bd Compare December 2, 2024 10:14
@dumbbell dumbbell changed the title rabbitmq_peer_discovery_consul: Don't set bind_addr in test config rabbitmq_peer_discovery_consul: Set bind_addr to 127.0.0.1 in test config Dec 2, 2024
@dumbbell
Copy link
Collaborator Author

dumbbell commented Dec 2, 2024

Allowing Consul to bind to all interfaces is definitely a better idea.

Unfortunately that didn’t work. GitHub Actions hosts have several IPv4 addresses configured and the Consul daemon doesn’t like it.

@dumbbell dumbbell marked this pull request as ready for review December 2, 2024 10:38
@dumbbell dumbbell merged commit ae4f37b into main Dec 2, 2024
271 checks passed
@dumbbell dumbbell deleted the consul-do-not-set-bind_addr-in-test-config branch December 2, 2024 10:38
@michaelklishin
Copy link
Collaborator

@dumbbell should we backport this to v4.0.x?

@dumbbell
Copy link
Collaborator Author

dumbbell commented Dec 2, 2024

I will backport the pull requests in order tomorrow. Thanks!

dumbbell added a commit that referenced this pull request Dec 3, 2024
rabbitmq_peer_discovery_consul: Set `bind_addr` to 127.0.0.1 in test config (backport #12858)
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