upstream: implement Cluster's load_assignment field#3864
upstream: implement Cluster's load_assignment field#3864htuch merged 6 commits intoenvoyproxy:masterfrom dio:cluster-load-assignment-impl
Conversation
|
cc. @danielhochman |
Signed-off-by: Dhi Aurrahman <dio@rockybars.com>
Signed-off-by: Dhi Aurrahman <dio@rockybars.com>
| @@ -544,10 +565,10 @@ Management Server Unreachability | |||
| -------------------------------- | |||
|
|
|||
| When Envoy instance looses connectivity with the management server, Envoy will latch on to | |||
There was a problem hiding this comment.
while you're here, do you mind s/looses/loses/
|
|
||
| If active health checking is configured for an upstream cluster, a specific additional configuration | ||
| for each registered member can be specified by setting the | ||
| :ref:`health check config<envoy_api_msg_endpoint.Endpoint.HealthCheckConfig>` |
There was a problem hiding this comment.
nit: can we keep the CamelCase for HealthCheckConfig and Endpoint? or at least make it consistent for each of the linked resources in this paragraph.
| If active health checking is configured for an upstream cluster, a specific additional configuration | ||
| for each registered member can be specified by setting the | ||
| :ref:`health check config<envoy_api_msg_endpoint.Endpoint.HealthCheckConfig>` | ||
| in :ref:`endpoint message<envoy_api_msg_endpoint.Endpoint>` |
There was a problem hiding this comment.
slight rewording, add the after in
| - endpoint: | ||
| health_check_config: | ||
| port_value: 8080 | ||
|
|
Signed-off-by: Dhi Aurrahman <dio@rockybars.com>
Signed-off-by: Dhi Aurrahman <dio@rockybars.com>
|
Thanks, @danielhochman. Updated in 18e98f4. |
Signed-off-by: Dhi Aurrahman <dio@rockybars.com>
|
Since this is a re-submitted PR of #3261, probably this requires @mattklein123 @alyssawilk @htuch and @zuercher help to check too. |
zuercher
left a comment
zuercher
left a comment
There was a problem hiding this comment.
Couple minor things from me, but otherwise looks good!
| When Envoy instance looses connectivity with the management server, Envoy will latch on to | ||
| the previous configuration while actively retrying in the background to reestablish the | ||
| connection with the management server. | ||
| When Envoy instance loses connectivity with the management server, Envoy will latch on to |
There was a problem hiding this comment.
While we're here, it should read "When an Envoy instance loses..."
| : Config::Utility::translateClusterHosts(cluster.hosts())) { | ||
| const auto& locality_lb_endpoints = load_assignment_.endpoints(); | ||
| if (locality_lb_endpoints.size() != 1 || locality_lb_endpoints[0].lb_endpoints().size() != 1) { | ||
| throw EnvoyException(fmt::format("LOGICAL_DNS clusters must have {}", |
There was a problem hiding this comment.
nit: just if (cluster.has_load_assignment()) { /* throw exception message1 */ } else { /* throw exception message 2 */ }
(it'll be easier to search for the error string, but I don't feel strongly)
Signed-off-by: Dhi Aurrahman <dio@rockybars.com>
4 participants
Description:
This patch implements
load_assigmentfield in CDS'Cluster.This change specifically adds the implementation of the new
load_assigmentfieldfor clusters with discovery-type:
STATIC,STRICT_DNSandLOGICAL_DNS.While adding this
load_assigmentfield implementation toCluster,this patch also allows specifying optional (active) health check config per specified upstream host.
Risk Level: medium
Testing: unit tests
Docs Changes:
Release Notes: N/A
Fixes #439
Signed-off-by: Dhi Aurrahman dio@rockybars.com