envoyproxy / envoy

Cloud-native high-performance edge/middle/service proxy
https://www.envoyproxy.io
Apache License 2.0
24.95k stars 4.8k forks source link

feature: Cluster level HTTP health check port setting #14045

Open mattrobenolt opened 3 years ago

mattrobenolt commented 3 years ago

Description: There's currently a configuration option within config.endpoint.v3.Endpoint.HealthCheckConfig for overriding the port_value.

The issue with this, or rather, the inconvenience, is that this is configurable per-endpoint. Ideally, we'd have the ability to configure this also at the cluster config.core.v3.HealthCheck level as well, with the current setting operating as an override if there is no cluster level setting set. This would work (assumingly) identical to the hostname value inside of the HealthCheckConfig today based on it's documentation:

By default, the host header for L7 health checks is controlled by cluster level configuration (see: host and authority). Setting this to a non-empty value allows overriding the cluster level configuration for a specific endpoint.

mattklein123 commented 3 years ago

This is a reasonable request.

rulex123 commented 3 years ago

I can help out with implementing this if you'd like.

If I understand correctly, we want to add a new port_value config to config.core.v3.HealthCheck, and if set this would determine the health check port for any endpoints whose port_value in config.endpoint.v3.Endpoint.HealthCheckConfig is not configured; any value set at the endpoint level would take precedence over this new option. If the new option is not set, then we simply fallback to the current behaviour, where each endpoint either sets a port value or defaults to the host's serving address port.

Does that make sense?

wdauchy commented 1 year ago

I can help out with implementing this if you'd like.

If I understand correctly, we want to add a new port_value config to config.core.v3.HealthCheck, and if set this would determine the health check port for any endpoints whose port_value in config.endpoint.v3.Endpoint.HealthCheckConfig is not configured; any value set at the endpoint level would take precedence over this new option. If the new option is not set, then we simply fallback to the current behaviour, where each endpoint either sets a port value or defaults to the host's serving address port.

Does that make sense?

it makes sense to me. I wonder if we could consider this to simplify cluster config, as specifying a different port number is not a rare case.

pramodrj07 commented 10 months ago

@rulex123 - Is this feature added? I saw in go-control plane that this feature alt port is not yet implemented so just clarifying.

rulex123 commented 10 months ago

@rulex123 - Is this feature added? I saw in go-control plane that this feature alt port is not yet implemented so just clarifying.

I haven't worked on this since posting back in 2020, and I don't think it has been implemented.

matheustmattioli commented 6 months ago

Hello @rulex123, are there any plans to implement this feature soon? I'm having the same issue, here we want to implement health checking at the cluster level, setting a hostname, path and port under the config.core.v3.HealthCheck. If there aren't any plans, could I try to contribute with this feature?

rulex123 commented 6 months ago

Hello @rulex123, are there any plans to implement this feature soon? I'm having the same issue, here we want to implement health checking at the cluster level, setting a hostname, path and port under the config.core.v3.HealthCheck. If there aren't any plans, could I try to contribute with this feature?

Feel free to pick this up, I don't have any plans to work on this currently :)

pramodrj07 commented 6 months ago

worked on this since posting back in 2020, and I don't think it has been implemen

@matheustmattioli - We would be very interested in getting this implemented as well. Please do let me know if you need any help related to this.

matheustmattioli commented 6 months ago

@pramodrj07 - Sorry, I was willing to work on this feature, but due to change of internal plans in my work I'll not have much time available to develop this task :(, at least for now.