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

Error running Consul health check due to Consul API update #4149

Closed
maestroes opened this issue Feb 16, 2022 · 1 comment · Fixed by #4153
Closed

Error running Consul health check due to Consul API update #4149

maestroes opened this issue Feb 16, 2022 · 1 comment · Fixed by #4153
Assignees
Milestone

Comments

@maestroes
Copy link

Problem description

Due to API update in Consul 1.11 the HTTP response code changed from 500 to 404 when an attempt is made to de-register or update non-existent check ID: hashicorp/consul#11950.

This causes an issue on node restart: if the node starts longer than cluster_formation.consul.deregister_after, subsequent attempts to re-register fail with Error running Consul health check: "404" error.

Proposal

It seems like 404 HTTP response code should be handled the same way as 500 in rabbit_peer_discovery_consul send_health_check_pass() with logging a warning and calling maybe_re_register(): https://github.com/rabbitmq/rabbitmq-server/blob/master/deps/rabbitmq_peer_discovery_consul/src/rabbit_peer_discovery_consul.erl#L501.

Environment

RabbitMQ 3.8.22
Erlang 23.3.4.6

rabbitmq_peer_discovery_consul 3.8.22
rabbitmq_shovel 3.8.22
rabbitmq_shovel_management 3.8.22
rabbitmq_stomp 3.8.22

cluster_formation.consul.svc_ttl = 30
cluster_formation.consul.deregister_after = 60

node start time ~ 2 min

@michaelklishin
Copy link
Member

Agreed. Thanks for reporting this change in Consul. Our team never really understood why it responded with a 500 in that case.

@michaelklishin michaelklishin added this to the 3.8.28 milestone Feb 16, 2022
@michaelklishin michaelklishin self-assigned this Feb 16, 2022
michaelklishin added a commit that referenced this issue Feb 16, 2022
mergify bot pushed a commit that referenced this issue Feb 16, 2022
as suggested by @maestroes.

Closes #4149.

(cherry picked from commit dc131a7)
mergify bot pushed a commit that referenced this issue Feb 16, 2022
as suggested by @maestroes.

Closes #4149.

(cherry picked from commit dc131a7)
(cherry picked from commit 25414d3)
mergify bot pushed a commit that referenced this issue Feb 16, 2022
as suggested by @maestroes.

Closes #4149.

(cherry picked from commit dc131a7)
(cherry picked from commit 25414d3)
(cherry picked from commit 6779877)
michaelklishin added a commit that referenced this issue Mar 7, 2022
michaelklishin added a commit that referenced this issue Mar 18, 2022
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 a pull request may close this issue.

2 participants