hashicorp / consul

Consul is a distributed, highly available, and data center aware solution to connect and configure applications across dynamic, distributed infrastructure.
https://www.consul.io
Other
28.3k stars 4.42k forks source link

API: Autofill Service.Address with Node.Address if Service.Address is empty #4599

Open johncowen opened 6 years ago

johncowen commented 6 years ago

Feature Description

Whenever the Consul API reports a ServiceAddress or Service.Address, if this value is blank/empty, default to the address of the Node instead of leaving the value blank.

We currently document that a blank value is expected behaviour in the catalog endpoint documentation, although changing this could improve DX and is therefore a feature request.

https://www.consul.io/api/catalog.html#serviceaddress

ServiceAddress is the IP address of the service host — if empty, node address should be used

Use Case(s)

Make it easier for API consumers/clients to retrieve the IP address of a service, even when there is no need for the registration of the service to provide an IP.

Also see:

https://github.com/hashicorp/consul-template/issues/587#issuecomment-232456696 https://github.com/hashicorp/consul/issues/4579

@mkeeler this is re: offline chat

sandstrom commented 6 years ago

This would be good!

  1. Current response is somewhat confusing, and leaves less flexibility. If we instruct a service consumer to use the Address response value, and later change things up with a dedicated service address, then the consumer would fail.

  2. It would align better with ServicePort

pierresouchay commented 4 years ago

I agree with you on the initial issue, but I think that many tools are now doing assumptions based on this value being present or not, see my comment: https://github.com/hashicorp/consul/pull/7782#issuecomment-624893181

I wonder if correct fix would not be to provide such wrappers in consul-template or various SDKs (and describe what should be the correct implementation).

Note that is also applies for instance to Check(s) status(es) for Health endpoint, see the helpers we added in our templating system (service_address, status, current_weight...): https://github.com/criteo/consul-templaterb/blob/master/TemplateAPI.md#helpers

jf commented 4 years ago

I agree with you on the initial issue, but I think that many tools are now doing assumptions based on this value being present or not, see my comment: #7782 (comment)

I'm late to this issue but... what tools would these be, and why would they care about it being empty (""?) vs being filled with an actual IP address? I fail to see why having the actual IP address reflected in Service.Address is a problem vs it's current "" value

jsosulska commented 3 years ago

Hello All!

Apologies for the long tail. We closed #7782 based on internal discussion. There are docs updates that can be performed that can better clear this ambiguity.

There is, however, a potential need for a new API to properly service both the service discovery and service management use cases and it will require a more holistic look at our API to determine what needs to be done. There'll be a follow up to this message with a bit more on that once a solution has been considered.

In the meantime, to clear the ambiguity: