Changing the base load balancer class over to use priorities (#2160)

Changing the LoadBalancerBase to use priorities, following the simplistic strategy docced up in the eds proto, where we route to the highest priority level with non-zero healthy endpoints.

This results in simple priority-based load balancing for RoundRobinLoadBalancer, LeastRequestLoadBalancer, and RandomLoadBalancer.

Risk Level: Medium
It's a big change but then again no one is using it :-)

Testing:
Existing tests parameterized to target P=0 and P=1 to verify equivalence.
Additional unit tests and one integration test explicitly testing primary/failover/... inteeractions

Docs Changes:
envoyproxy/data-plane-api#326

Release Notes:
Added support for priorities for several types of load balancer (RoundRobinLoadBalancer, LeastRequestLoadBalancer, RandomLoadBalancer)

Continued progress on #1929

RAW_RELEASE_NOTES.md

@@ -14,4 +14,5 @@ final version.
 * Added support for dynamic response header values (`%CLIENT_IP%` and `%PROTOCOL%`).
 * Added native DogStatsD support. :ref:`DogStatsdSink <envoy_api_msg_DogStatsdSink>`
 * grpc-json: Added support inline descriptor in config.
+* Added support for priorities for several types of load balancer.
 * Added idle timeout to TCP proxy.

include/envoy/upstream/cluster_manager.h

@@ -134,6 +134,13 @@ class ClusterManager {
    *                     or "static" if CDS is not in use.
    */
   virtual const std::string versionInfo() const PURE;
+
+  /**
+   * Return the local cluster name, if it was configured.
+   *
+   * @return std::string the local cluster name, or "" if no local cluster was configured.
+   */
+  virtual const std::string& localClusterName() const PURE;
 };
 
 typedef std::unique_ptr<ClusterManager> ClusterManagerPtr;

include/envoy/upstream/upstream.h

@@ -129,25 +129,6 @@ class HostSet {
 
   virtual ~HostSet() {}
 
-  // TODO(alyssawilk) remove this once LBs use PrioritySet.
-  // It is generally incorrect to subscribe to updates to individual HostSet
-  // as one misses the addition of new HostSet to a PrioritySet.
-  /**
-   * Called when cluster host membership is about to change.
-   * @param hosts_added supplies the newly added hosts, if any.
-   * @param hosts_removed supplies the removed hosts, if any.
-   */
-  typedef std::function<void(uint32_t priority, const std::vector<HostSharedPtr>& hosts_added,
-                             const std::vector<HostSharedPtr>& hosts_removed)>
-      MemberUpdateCb;
-
-  /**
-   * Install a callback that will be invoked when the cluster membership changes.
-   * @param callback supplies the callback to invoke.
-   * @return Common::CallbackHandle* the callback handle.
-   */
-  virtual Common::CallbackHandle* addMemberUpdateCb(MemberUpdateCb callback) const PURE;
-
   /**
    * @return all hosts that make up the set at the current time.
    */

source/common/upstream/cluster_manager_impl.cc

@@ -220,6 +220,7 @@ ClusterManagerImpl::ClusterManagerImpl(const envoy::api::v2::Bootstrap& bootstra
 
   Optional<std::string> local_cluster_name;
   if (!cm_config.local_cluster_name().empty()) {
+    local_cluster_name_ = cm_config.local_cluster_name();
     local_cluster_name.value(cm_config.local_cluster_name());
     if (primary_clusters_.find(local_cluster_name.value()) == primary_clusters_.end()) {
       throw EnvoyException(

source/common/upstream/cluster_manager_impl.h

@@ -175,6 +175,7 @@ class ClusterManagerImpl : public ClusterManager, Logger::Loggable<Logger::Id::u
   Config::GrpcMux& adsMux() override { return *ads_mux_; }
 
   const std::string versionInfo() const override;
+  const std::string& localClusterName() const override { return local_cluster_name_; }
 
 private:
   /**
@@ -265,6 +266,8 @@ class ClusterManagerImpl : public ClusterManager, Logger::Loggable<Logger::Id::u
   ClusterManagerInitHelper init_helper_;
   Config::GrpcMuxPtr ads_mux_;
   LoadStatsReporterPtr load_stats_reporter_;
+  // The name of the local cluster of this Envoy instance if defined, else the empty string.
+  std::string local_cluster_name_;
 };
 
 } // namespace Upstream

source/common/upstream/eds.cc

@@ -25,9 +25,10 @@ EdsClusterImpl::EdsClusterImpl(const envoy::api::v2::Cluster& cluster, Runtime::
                                bool added_via_api)
     : BaseDynamicClusterImpl(cluster, cm.sourceAddress(), runtime, stats, ssl_context_manager,
                              added_via_api),
-      local_info_(local_info), cluster_name_(cluster.eds_cluster_config().service_name().empty()
-                                                 ? cluster.name()
-                                                 : cluster.eds_cluster_config().service_name()) {
+      cm_(cm), local_info_(local_info),
+      cluster_name_(cluster.eds_cluster_config().service_name().empty()
+                        ? cluster.name()
+                        : cluster.eds_cluster_config().service_name()) {
   Config::Utility::checkLocalInfo("eds", local_info);
   const auto& eds_config = cluster.eds_cluster_config().eds_config();
   subscription_ = Config::SubscriptionFactory::subscriptionFromConfigSource<
@@ -64,6 +65,14 @@ void EdsClusterImpl::onConfigUpdate(const ResourceVector& resources) {
   }
   for (const auto& locality_lb_endpoint : cluster_load_assignment.endpoints()) {
     const uint32_t priority = locality_lb_endpoint.priority();
+    if (priority > 0 && info()->lbType() == LoadBalancerType::RingHash) {
+      throw EnvoyException(
+          fmt::format("Unexpected non-zero priority for RingHash cluster '{}'.", cluster_name_));
+    }
+    if (priority > 0 && !cluster_name_.empty() && cluster_name_ == cm_.localClusterName()) {
+      throw EnvoyException(
+          fmt::format("Unexpected non-zero priority for local cluster '{}'.", cluster_name_));
+    }
     if (new_hosts.size() <= priority) {
       new_hosts.resize(priority + 1);
     }

source/common/upstream/eds.h

@@ -38,6 +38,7 @@ class EdsClusterImpl : public BaseDynamicClusterImpl,
   // ClusterImplBase
   void startPreInit() override;
 
+  const ClusterManager& cm_;
   std::unique_ptr<Config::Subscription<envoy::api::v2::ClusterLoadAssignment>> subscription_;
   const LocalInfo::LocalInfo& local_info_;
   const std::string cluster_name_;