Modify Status Leader API to return Status 500 when there is no leader

[nicholasjackson]

The API endpoint /v1/status/leader can return a HTTP status 200 and no leader in the response in certain instances. This mainly occurs in a race condition when you need to check if an agent has successfully joined a cluster before making a request to register a service or submit L7 config.

This PR implements a better RESTFull behaviour in the instance that the API is called and no leader is available. When there is no leader the API will return a HTTP status InternalServerError. I am not 100% convinced this is the correct status code as in fact the API is not returning an error but this mimics other No Leader error messages which can be returned from this API.

[mikemorris]

Refs #8395, some of the tests in consul/api should likely be made more explicit as part of this change, even though they aren't currently failing because they only check response content and not HTTP status code.

[dnephin]

LGTM, this change is correct and should fix the race. Being consistent with the status code seems good for now. We can fix the status code across the board for this error at some later time.

We should rebase this PR to run the latest tests, but otherwise I think this is good to merge.

[vercel]

@dnephin is attempting to deploy a commit to the HashiCorp Team on Vercel.

A member of the Team first needs to authorize it.

[vercel]

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployments, click below or on the icon next to each commit.

consul-ui-staging – ./ui

🔍 Inspect: https://vercel.com/hashicorp/consul-ui-staging/En54zwsMkKLwKVwz7nXKf4EBqUZW
✅ Preview: Failed

[markan]

Looks reasonable to me

[dnephin]

Thank you for the PR! We were just about to merge this, thinking it would solve #10733, but after looking at the integration mentioned there (prometheus/consul_exporter source) I think unfortunate this is a breaking change. It would be more consistent with other API endpoints but would break existing users of this API endpoint.

The API endpoint /v1/status/leader can return a HTTP status 200 and no leader in the response in certain instances

Surprisingly this is actually always the case when there is no leader. There is no way for this endpoint to return a non-200 code if the query is being made to the local DC. It always returned an empty response if the leader is unknown. Maybe we should update the API docs and/or add a new API with the behaviour we expect (a non-200 code when there is no leader).

[dnephin]

Thanks again for the PR! Since this is a breaking change we won't be able to merge it, so I'm going to close this PR. I think it would be worthwhile to talk about expected behaviour for a v2 version of this endpoint in a github issue.