health check: add support health checking EDS endpoints with differen…

…t port (#3007)

This patch adds a way to override endpoint health check port.

Only EDS cluster is affected for now.

Risk Level:

Low. This is an optional feature.
Testing:

Unit test
Docs Changes:
To be updated (unhiding)

Release Notes:
To be updated

Partially remedies #439

API Changes:
To be updated (unhiding)

Signed-off-by: Dhi Aurrahman <[email protected]>

include/envoy/upstream/host_description.h

@@ -93,6 +93,11 @@ class HostDescription {
    *         unknown.
    */
   virtual const envoy::api::v2::core::Locality& locality() const PURE;
+
+  /**
+   * @return the address used to health check the host.
+   */
+  virtual Network::Address::InstanceConstSharedPtr healthCheckAddress() const PURE;
 };
 
 typedef std::shared_ptr<const HostDescription> HostDescriptionConstSharedPtr;

include/envoy/upstream/upstream.h

@@ -62,6 +62,14 @@ class Host : virtual public HostDescription {
   createConnection(Event::Dispatcher& dispatcher,
                    const Network::ConnectionSocket::OptionsSharedPtr& options) const PURE;
 
+  /**
+   * Create a health check connection for this host.
+   * @param dispatcher supplies the owning dispatcher.
+   * @return the connection data.
+   */
+  virtual CreateConnectionData
+  createHealthCheckConnection(Event::Dispatcher& dispatcher) const PURE;
+
   /**
    * @return host specific gauges.
    */

source/common/upstream/BUILD

@@ -371,5 +371,6 @@ envoy_cc_library(
         "//source/common/stats:stats_lib",
         "//source/common/upstream:locality_lib",
         "@envoy_api//envoy/api/v2/core:base_cc",
+        "@envoy_api//envoy/api/v2/endpoint:endpoint_cc",
     ],
 )

source/common/upstream/eds.cc

@@ -82,7 +82,8 @@ void EdsClusterImpl::onConfigUpdate(const ResourceVector& resources) {
     for (const auto& lb_endpoint : locality_lb_endpoint.lb_endpoints()) {
       priority_state[priority].first->emplace_back(new HostImpl(
           info_, "", resolveProtoAddress(lb_endpoint.endpoint().address()), lb_endpoint.metadata(),
-          lb_endpoint.load_balancing_weight().value(), locality_lb_endpoint.locality()));
+          lb_endpoint.load_balancing_weight().value(), locality_lb_endpoint.locality(),
+          lb_endpoint.endpoint().health_check_config()));
       const auto& health_status = lb_endpoint.health_status();
       if (health_status == envoy::api::v2::core::HealthStatus::UNHEALTHY ||
           health_status == envoy::api::v2::core::HealthStatus::DRAINING ||

source/common/upstream/health_checker_impl.cc

@@ -91,7 +91,7 @@ const RequestInfo::RequestInfoImpl
 void HttpHealthCheckerImpl::HttpActiveHealthCheckSession::onInterval() {
   if (!client_) {
     Upstream::Host::CreateConnectionData conn =
-        host_->createConnection(parent_.dispatcher_, nullptr);
+        host_->createHealthCheckConnection(parent_.dispatcher_);
     client_.reset(parent_.createCodecClient(conn));
     client_->addConnectionCallbacks(connection_callback_impl_);
     expect_reset_ = false;
@@ -267,7 +267,7 @@ void TcpHealthCheckerImpl::TcpActiveHealthCheckSession::onEvent(Network::Connect
 
 void TcpHealthCheckerImpl::TcpActiveHealthCheckSession::onInterval() {
   if (!client_) {
-    client_ = host_->createConnection(parent_.dispatcher_, nullptr).connection_;
+    client_ = host_->createHealthCheckConnection(parent_.dispatcher_).connection_;
     session_callbacks_.reset(new TcpSessionCallbacks(*this));
     client_->addConnectionCallbacks(*session_callbacks_);
     client_->addReadFilter(session_callbacks_);
@@ -522,7 +522,7 @@ void GrpcHealthCheckerImpl::GrpcActiveHealthCheckSession::onEvent(Network::Conne
 void GrpcHealthCheckerImpl::GrpcActiveHealthCheckSession::onInterval() {
   if (!client_) {
     Upstream::Host::CreateConnectionData conn =
-        host_->createConnection(parent_.dispatcher_, nullptr);
+        host_->createHealthCheckConnection(parent_.dispatcher_);
     client_ = parent_.createCodecClient(conn);
     client_->addConnectionCallbacks(connection_callback_impl_);
     client_->setCodecConnectionCallbacks(http_connection_callback_impl_);

source/common/upstream/logical_dns_cluster.h

@@ -43,7 +43,8 @@ class LogicalDnsCluster : public ClusterImplBase {
     LogicalHost(ClusterInfoConstSharedPtr cluster, const std::string& hostname,
                 Network::Address::InstanceConstSharedPtr address, LogicalDnsCluster& parent)
         : HostImpl(cluster, hostname, address, envoy::api::v2::core::Metadata::default_instance(),
-                   1, envoy::api::v2::core::Locality().default_instance()),
+                   1, envoy::api::v2::core::Locality().default_instance(),
+                   envoy::api::v2::endpoint::Endpoint::HealthCheckConfig().default_instance()),
           parent_(parent) {}
 
     // Upstream::Host
@@ -77,7 +78,10 @@ class LogicalDnsCluster : public ClusterImplBase {
     const envoy::api::v2::core::Locality& locality() const override {
       return envoy::api::v2::core::Locality().default_instance();
     }
-
+    // TODO(dio): To support different address port.
+    Network::Address::InstanceConstSharedPtr healthCheckAddress() const override {
+      return address_;
+    }
     Network::Address::InstanceConstSharedPtr address_;
     HostConstSharedPtr logical_host_;
   };

source/common/upstream/original_dst_cluster.cc

@@ -60,9 +60,11 @@ HostConstSharedPtr OriginalDstCluster::LoadBalancer::chooseHost(LoadBalancerCont
         Network::Address::InstanceConstSharedPtr host_ip_port(
             Network::Utility::copyInternetAddressAndPort(*dst_ip));
         // Create a host we can use immediately.
-        host.reset(new HostImpl(info_, info_->name() + dst_addr.asString(), std::move(host_ip_port),
-                                envoy::api::v2::core::Metadata::default_instance(), 1,
-                                envoy::api::v2::core::Locality().default_instance()));
+        host.reset(new HostImpl(
+            info_, info_->name() + dst_addr.asString(), std::move(host_ip_port),
+            envoy::api::v2::core::Metadata::default_instance(), 1,
+            envoy::api::v2::core::Locality().default_instance(),
+            envoy::api::v2::endpoint::Endpoint::HealthCheckConfig().default_instance()));
 
         ENVOY_LOG(debug, "Created host {}.", host->address()->asString());
         // Add the new host to the map. We just failed to find it in

source/common/upstream/upstream_impl.cc

@@ -25,7 +25,6 @@
 #include "common/network/address_impl.h"
 #include "common/network/resolver_impl.h"
 #include "common/network/socket_option_impl.h"
-#include "common/network/utility.h"
 #include "common/protobuf/protobuf.h"
 #include "common/protobuf/utility.h"
 #include "common/upstream/eds.h"
@@ -90,6 +89,12 @@ HostImpl::createConnection(Event::Dispatcher& dispatcher,
   return {createConnection(dispatcher, *cluster_, address_, options), shared_from_this()};
 }
 
+Host::CreateConnectionData
+HostImpl::createHealthCheckConnection(Event::Dispatcher& dispatcher) const {
+  return {createConnection(dispatcher, *cluster_, healthCheckAddress(), nullptr),
+          shared_from_this()};
+}
+
 Network::ClientConnectionPtr
 HostImpl::createConnection(Event::Dispatcher& dispatcher, const ClusterInfo& cluster,
                            Network::Address::InstanceConstSharedPtr address,
@@ -602,7 +607,8 @@ StaticClusterImpl::StaticClusterImpl(const envoy::api::v2::Cluster& cluster,
   for (const auto& host : cluster.hosts()) {
     initial_hosts_->emplace_back(HostSharedPtr{new HostImpl(
         info_, "", resolveProtoAddress(host), envoy::api::v2::core::Metadata::default_instance(), 1,
-        envoy::api::v2::core::Locality().default_instance())});
+        envoy::api::v2::core::Locality().default_instance(),
+        envoy::api::v2::endpoint::Endpoint::HealthCheckConfig().default_instance())});
   }
 }
 
@@ -826,10 +832,11 @@ void StrictDnsClusterImpl::ResolveTarget::startResolve() {
           // a new address that has port in it. We need to both support IPv6 as well as potentially
           // move port handling into the DNS interface itself, which would work better for SRV.
           ASSERT(address != nullptr);
-          new_hosts.emplace_back(new HostImpl(parent_.info_, dns_address_,
-                                              Network::Utility::getAddressWithPort(*address, port_),
-                                              envoy::api::v2::core::Metadata::default_instance(), 1,
-                                              envoy::api::v2::core::Locality().default_instance()));
+          new_hosts.emplace_back(new HostImpl(
+              parent_.info_, dns_address_, Network::Utility::getAddressWithPort(*address, port_),
+              envoy::api::v2::core::Metadata::default_instance(), 1,
+              envoy::api::v2::core::Locality().default_instance(),
+              envoy::api::v2::endpoint::Endpoint::HealthCheckConfig().default_instance()));
         }
 
         HostVector hosts_added;

source/common/upstream/upstream_impl.h

@@ -12,6 +12,7 @@
 #include <vector>
 
 #include "envoy/api/v2/core/base.pb.h"
+#include "envoy/api/v2/endpoint/endpoint.pb.h"
 #include "envoy/event/timer.h"
 #include "envoy/local_info/local_info.h"
 #include "envoy/network/dns.h"
@@ -29,6 +30,7 @@
 #include "common/common/logger.h"
 #include "common/config/metadata.h"
 #include "common/config/well_known_names.h"
+#include "common/network/utility.h"
 #include "common/stats/stats_impl.h"
 #include "common/upstream/load_balancer_impl.h"
 #include "common/upstream/locality.h"
@@ -52,11 +54,17 @@ class HealthCheckHostMonitorNullImpl : public HealthCheckHostMonitor {
  */
 class HostDescriptionImpl : virtual public HostDescription {
 public:
-  HostDescriptionImpl(ClusterInfoConstSharedPtr cluster, const std::string& hostname,
-                      Network::Address::InstanceConstSharedPtr dest_address,
-                      const envoy::api::v2::core::Metadata& metadata,
-                      const envoy::api::v2::core::Locality& locality)
+  HostDescriptionImpl(
+      ClusterInfoConstSharedPtr cluster, const std::string& hostname,
+      Network::Address::InstanceConstSharedPtr dest_address,
+      const envoy::api::v2::core::Metadata& metadata,
+      const envoy::api::v2::core::Locality& locality,
+      const envoy::api::v2::endpoint::Endpoint::HealthCheckConfig& health_check_config)
       : cluster_(cluster), hostname_(hostname), address_(dest_address),
+        health_check_address_(health_check_config.port_value() == 0
+                                  ? dest_address
+                                  : Network::Utility::getAddressWithPort(
+                                        *dest_address, health_check_config.port_value())),
         canary_(Config::Metadata::metadataValue(metadata, Config::MetadataFilters::get().ENVOY_LB,
                                                 Config::MetadataEnvoyLbKeys::get().CANARY)
                     .bool_value()),
@@ -89,12 +97,16 @@ class HostDescriptionImpl : virtual public HostDescription {
   const HostStats& stats() const override { return stats_; }
   const std::string& hostname() const override { return hostname_; }
   Network::Address::InstanceConstSharedPtr address() const override { return address_; }
+  Network::Address::InstanceConstSharedPtr healthCheckAddress() const override {
+    return health_check_address_;
+  }
   const envoy::api::v2::core::Locality& locality() const override { return locality_; }
 
 protected:
   ClusterInfoConstSharedPtr cluster_;
   const std::string hostname_;
   Network::Address::InstanceConstSharedPtr address_;
+  Network::Address::InstanceConstSharedPtr health_check_address_;
   const bool canary_;
   const envoy::api::v2::core::Metadata metadata_;
   const envoy::api::v2::core::Locality locality_;
@@ -114,8 +126,10 @@ class HostImpl : public HostDescriptionImpl,
   HostImpl(ClusterInfoConstSharedPtr cluster, const std::string& hostname,
            Network::Address::InstanceConstSharedPtr address,
            const envoy::api::v2::core::Metadata& metadata, uint32_t initial_weight,
-           const envoy::api::v2::core::Locality& locality)
-      : HostDescriptionImpl(cluster, hostname, address, metadata, locality), used_(true) {
+           const envoy::api::v2::core::Locality& locality,
+           const envoy::api::v2::endpoint::Endpoint::HealthCheckConfig& health_check_config)
+      : HostDescriptionImpl(cluster, hostname, address, metadata, locality, health_check_config),
+        used_(true) {
     weight(initial_weight);
   }
 
@@ -124,6 +138,7 @@ class HostImpl : public HostDescriptionImpl,
   CreateConnectionData
   createConnection(Event::Dispatcher& dispatcher,
                    const Network::ConnectionSocket::OptionsSharedPtr& options) const override;
+  CreateConnectionData createHealthCheckConnection(Event::Dispatcher& dispatcher) const override;
   std::list<Stats::GaugeSharedPtr> gauges() const override { return stats_store_.gauges(); }
   void healthFlagClear(HealthFlag flag) override { health_flags_ &= ~enumToInt(flag); }
   bool healthFlagGet(HealthFlag flag) const override { return health_flags_ & enumToInt(flag); }

test/common/http/conn_manager_impl_test.cc

@@ -1199,11 +1199,12 @@ TEST_F(HttpConnectionManagerImplTest, WebSocketConnectTimeoutError) {
   Upstream::MockHost::MockCreateConnectionData conn_info;
 
   conn_info.connection_ = upstream_connection;
-  conn_info.host_description_.reset(
-      new Upstream::HostImpl(cluster_manager_.thread_local_cluster_.cluster_.info_, "newhost",
-                             Network::Utility::resolveUrl("tcp://127.0.0.1:80"),
-                             envoy::api::v2::core::Metadata::default_instance(), 1,
-                             envoy::api::v2::core::Locality().default_instance()));
+  conn_info.host_description_.reset(new Upstream::HostImpl(
+      cluster_manager_.thread_local_cluster_.cluster_.info_, "newhost",
+      Network::Utility::resolveUrl("tcp://127.0.0.1:80"),
+      envoy::api::v2::core::Metadata::default_instance(), 1,
+      envoy::api::v2::core::Locality().default_instance(),
+      envoy::api::v2::endpoint::Endpoint::HealthCheckConfig().default_instance()));
   EXPECT_CALL(*connect_timer, enableTimer(_));
   EXPECT_CALL(cluster_manager_, tcpConnForCluster_("fake_cluster", _)).WillOnce(Return(conn_info));
 
@@ -1247,11 +1248,12 @@ TEST_F(HttpConnectionManagerImplTest, WebSocketConnectionFailure) {
   Upstream::MockHost::MockCreateConnectionData conn_info;
 
   conn_info.connection_ = upstream_connection;
-  conn_info.host_description_.reset(
-      new Upstream::HostImpl(cluster_manager_.thread_local_cluster_.cluster_.info_, "newhost",
-                             Network::Utility::resolveUrl("tcp://127.0.0.1:80"),
-                             envoy::api::v2::core::Metadata::default_instance(), 1,
-                             envoy::api::v2::core::Locality().default_instance()));
+  conn_info.host_description_.reset(new Upstream::HostImpl(
+      cluster_manager_.thread_local_cluster_.cluster_.info_, "newhost",
+      Network::Utility::resolveUrl("tcp://127.0.0.1:80"),
+      envoy::api::v2::core::Metadata::default_instance(), 1,
+      envoy::api::v2::core::Locality().default_instance(),
+      envoy::api::v2::endpoint::Endpoint::HealthCheckConfig().default_instance()));
   EXPECT_CALL(*connect_timer, enableTimer(_));
   EXPECT_CALL(cluster_manager_, tcpConnForCluster_("fake_cluster", _)).WillOnce(Return(conn_info));
 
@@ -1302,11 +1304,12 @@ TEST_F(HttpConnectionManagerImplTest, WebSocketPrefixAndAutoHostRewrite) {
   auto raw_header_ptr = headers.get();
 
   conn_info.connection_ = upstream_connection;
-  conn_info.host_description_.reset(
-      new Upstream::HostImpl(cluster_manager_.thread_local_cluster_.cluster_.info_, "newhost",
-                             Network::Utility::resolveUrl("tcp://127.0.0.1:80"),
-                             envoy::api::v2::core::Metadata::default_instance(), 1,
-                             envoy::api::v2::core::Locality().default_instance()));
+  conn_info.host_description_.reset(new Upstream::HostImpl(
+      cluster_manager_.thread_local_cluster_.cluster_.info_, "newhost",
+      Network::Utility::resolveUrl("tcp://127.0.0.1:80"),
+      envoy::api::v2::core::Metadata::default_instance(), 1,
+      envoy::api::v2::core::Locality().default_instance(),
+      envoy::api::v2::endpoint::Endpoint::HealthCheckConfig().default_instance()));
   EXPECT_CALL(cluster_manager_, tcpConnForCluster_("fake_cluster", _)).WillOnce(Return(conn_info));
 
   ON_CALL(route_config_provider_.route_config_->route_->route_entry_, useWebSocket())
@@ -1348,11 +1351,12 @@ TEST_F(HttpConnectionManagerImplTest, WebSocketEarlyData) {
   Upstream::MockHost::MockCreateConnectionData conn_info;
 
   conn_info.connection_ = upstream_connection;
-  conn_info.host_description_.reset(
-      new Upstream::HostImpl(cluster_manager_.thread_local_cluster_.cluster_.info_, "newhost",
-                             Network::Utility::resolveUrl("tcp://127.0.0.1:80"),
-                             envoy::api::v2::core::Metadata::default_instance(), 1,
-                             envoy::api::v2::core::Locality().default_instance()));
+  conn_info.host_description_.reset(new Upstream::HostImpl(
+      cluster_manager_.thread_local_cluster_.cluster_.info_, "newhost",
+      Network::Utility::resolveUrl("tcp://127.0.0.1:80"),
+      envoy::api::v2::core::Metadata::default_instance(), 1,
+      envoy::api::v2::core::Locality().default_instance(),
+      envoy::api::v2::endpoint::Endpoint::HealthCheckConfig().default_instance()));
   EXPECT_CALL(*connect_timer, enableTimer(_));
   EXPECT_CALL(cluster_manager_, tcpConnForCluster_("fake_cluster", _)).WillOnce(Return(conn_info));
 
@@ -1400,11 +1404,12 @@ TEST_F(HttpConnectionManagerImplTest, WebSocketEarlyEndStream) {
   Upstream::MockHost::MockCreateConnectionData conn_info;
 
   conn_info.connection_ = upstream_connection;
-  conn_info.host_description_.reset(
-      new Upstream::HostImpl(cluster_manager_.thread_local_cluster_.cluster_.info_, "newhost",
-                             Network::Utility::resolveUrl("tcp://127.0.0.1:80"),
-                             envoy::api::v2::core::Metadata::default_instance(), 1,
-                             envoy::api::v2::core::Locality().default_instance()));
+  conn_info.host_description_.reset(new Upstream::HostImpl(
+      cluster_manager_.thread_local_cluster_.cluster_.info_, "newhost",
+      Network::Utility::resolveUrl("tcp://127.0.0.1:80"),
+      envoy::api::v2::core::Metadata::default_instance(), 1,
+      envoy::api::v2::core::Locality().default_instance(),
+      envoy::api::v2::endpoint::Endpoint::HealthCheckConfig().default_instance()));
   EXPECT_CALL(*connect_timer, enableTimer(_));
   EXPECT_CALL(cluster_manager_, tcpConnForCluster_("fake_cluster", _)).WillOnce(Return(conn_info));