Enable load balancing policy extensions

Enables LOAD_BALANCING_POLICY_CONFIG enum value in LbPolicy and supports
typed load balancers specified in load_balancing_policy. Continues work
done by Charlie Getzen <[email protected]> in PR envoyproxy#15827.

Custom load balancers specified by load_balancing_policy are created as
implementations of ThreadAwareLoadBalancer. Thread-local load balancers
can be implemented as thread-aware load balancers that contain no logic
at the thread-aware level, i.e. the purpose of the thread-aware LB is
solely to contain the factory used to instantiate the thread-local LBs.
(In the future it might be appropriate to provide a construct that
abstracts away thread-aware aspects of ThreadAwareLoadBalancer for LBs
that don't need to be thread-aware.)

A cluster that uses LOAD_BALANCING_POLICY_CONFIG may not also set a
subset LB configuration. If the load balancer type makes use of
subsetting, it should include a subset configuration in its own
configuration message. Future work on load balancing extensions should
include moving the subset LB to use load balancing extensions.

Similarly, a cluster that uses LOAD_BALANCING_POLICY_CONFIG may not set
the CommonLbConfig, and it is not passed into load balancer creation
(mostly owing to its dubious applicability as a top level configuration
message to hierarchical load balancing policy). If the load balancer type
makes use of the CommonLbConfig, it should include a CommonLbConfig in
the configuration message for the load balancing policy.

Considerations for migration of existing load balancers:

- pieces of the ThreadAwareLoadBalancerBase implementation are specific
to the built-in hashing load balancers and should be moved into a base
class specifically for hashing load balancers. As it stands, custom load
balancing policies are required to implement a createLoadBalancer()
method even if the architecture of the LB policy does not require a
hashing load balancer. I think we would also benefit from disentangling
ThreadAwareLoadBalancerBase from LoadBalancerBase, as the former never
actually does any host picking.

- as we convert existing thread-local load balancers to thread-aware
load balancers, new local LBs will be re-created upon membership
changes. We should provide a mechanism allowing load balancers to
control whether this rebuild should occur, e.g. a callback that calls
create() for thread-aware load balancers by default, which can be
overridden to do nothing for thread-local LBs.

Signed-off-by: Eugene Chan <[email protected]>

api/envoy/config/cluster/v3/cluster.proto

@@ -110,7 +110,7 @@ message Cluster {
     // this option or not.
     CLUSTER_PROVIDED = 6;
 
-    // [#not-implemented-hide:] Use the new :ref:`load_balancing_policy
+    // Use the new :ref:`load_balancing_policy
     // <envoy_v3_api_field_config.cluster.v3.Cluster.load_balancing_policy>` field to determine the LB policy.
     // [#next-major-version: In the v3 API, we should consider deprecating the lb_policy field
     // and instead using the new load_balancing_policy field as the one and only mechanism for
@@ -720,8 +720,7 @@ message Cluster {
 
   // The :ref:`load balancer type <arch_overview_load_balancing_types>` to use
   // when picking a host in the cluster.
-  // [#comment:TODO: Remove enum constraint :ref:`LOAD_BALANCING_POLICY_CONFIG<envoy_v3_api_enum_value_config.cluster.v3.Cluster.LbPolicy.LOAD_BALANCING_POLICY_CONFIG>` when implemented.]
-  LbPolicy lb_policy = 6 [(validate.rules).enum = {defined_only: true not_in: 7}];
+  LbPolicy lb_policy = 6 [(validate.rules).enum = {defined_only: true}];
 
   // Setting this is required for specifying members of
   // :ref:`STATIC<envoy_v3_api_enum_value_config.cluster.v3.Cluster.DiscoveryType.STATIC>`,
@@ -1004,7 +1003,7 @@ message Cluster {
   // servers of this cluster.
   repeated Filter filters = 40;
 
-  // [#not-implemented-hide:] New mechanism for LB policy configuration. Used only if the
+  // New mechanism for LB policy configuration. Used only if the
   // :ref:`lb_policy<envoy_v3_api_field_config.cluster.v3.Cluster.lb_policy>` field has the value
   // :ref:`LOAD_BALANCING_POLICY_CONFIG<envoy_v3_api_enum_value_config.cluster.v3.Cluster.LbPolicy.LOAD_BALANCING_POLICY_CONFIG>`.
   LoadBalancingPolicy load_balancing_policy = 41;
@@ -1069,7 +1068,7 @@ message Cluster {
   bool connection_pool_per_downstream_connection = 51;
 }
 
-// [#not-implemented-hide:] Extensible load balancing policy configuration.
+// Extensible load balancing policy configuration.
 //
 // Every LB policy defined via this mechanism will be identified via a unique name using reverse
 // DNS notation. If the policy needs configuration parameters, it must define a message for its

envoy/upstream/load_balancer.h

@@ -173,5 +173,26 @@ class ThreadAwareLoadBalancer {
 
 using ThreadAwareLoadBalancerPtr = std::unique_ptr<ThreadAwareLoadBalancer>;
 
+/**
+ * Factory for (thread-aware) load balancers. To support a load balancing policy of
+ * LOAD_BALANCING_POLICY_CONFIG, at least one load balancer factory corresponding to a policy in
+ * load_balancing_policy must be registered with Envoy. Envoy will use the first policy for which
+ * it has a registered factory.
+ */
+class TypedLoadBalancerFactory : public Config::UntypedFactory {
+public:
+  ~TypedLoadBalancerFactory() override = default;
+
+  /**
+   * @return ThreadAwareLoadBalancerPtr a new thread-aware load balancer.
+   */
+  virtual ThreadAwareLoadBalancerPtr
+  create(const PrioritySet& priority_set, ClusterStats& stats, Stats::Scope& stats_scope,
+         Runtime::Loader& runtime, Random::RandomGenerator& random,
+         const ::envoy::config::cluster::v3::LoadBalancingPolicy::Policy& lb_policy) PURE;
+
+  std::string category() const override { return "envoy.load_balancers"; }
+};
+
 } // namespace Upstream
 } // namespace Envoy

envoy/upstream/load_balancer_type.h

@@ -22,7 +22,8 @@ enum class LoadBalancerType {
   RingHash,
   OriginalDst,
   Maglev,
-  ClusterProvided
+  ClusterProvided,
+  LoadBalancingPolicyConfig
 };
 
 /**

envoy/upstream/upstream.h

@@ -786,6 +786,13 @@ class ClusterInfo {
     return std::dynamic_pointer_cast<const Derived>(extensionProtocolOptions(name));
   }
 
+  /**
+   * @return const envoy::config::cluster::v3::LoadBalancingPolicy_Policy& the load balancing policy
+   * to use for this cluster.
+   */
+  virtual const envoy::config::cluster::v3::LoadBalancingPolicy_Policy&
+  loadBalancingPolicy() const PURE;
+
   /**
    * @return const envoy::config::cluster::v3::Cluster::CommonLbConfig& the common configuration for
    * all load balancers for this cluster.

source/common/upstream/BUILD

@@ -210,6 +210,14 @@ envoy_cc_library(
     ],
 )
 
+envoy_cc_library(
+    name = "load_balancer_factory_base_lib",
+    hdrs = ["load_balancer_factory_base.h"],
+    deps = [
+        ":load_balancer_lib",
+    ],
+)
+
 envoy_cc_library(
     name = "load_stats_reporter_lib",
     srcs = ["load_stats_reporter.cc"],

source/common/upstream/cluster_manager_impl.cc

@@ -831,6 +831,15 @@ ClusterManagerImpl::loadCluster(const envoy::config::cluster::v3::Cluster& clust
     }
   } else if (cluster_reference.info()->lbType() == LoadBalancerType::ClusterProvided) {
     cluster_entry_it->second->thread_aware_lb_ = std::move(new_cluster_pair.second);
+  } else if (cluster_reference.info()->lbType() == LoadBalancerType::LoadBalancingPolicyConfig) {
+    const auto& policy = cluster_reference.info()->loadBalancingPolicy();
+    TypedLoadBalancerFactory* typed_lb_factory =
+        Registry::FactoryRegistry<TypedLoadBalancerFactory>::getFactory(policy.name());
+    RELEASE_ASSERT(typed_lb_factory != nullptr, "should have factory for selected LB policy");
+
+    cluster_entry_it->second->thread_aware_lb_ =
+        typed_lb_factory->create(cluster_reference.prioritySet(), cluster_reference.info()->stats(),
+                                 cluster_reference.info()->statsScope(), runtime_, random_, policy);
   }
 
   updateClusterCounts();
@@ -1377,6 +1386,7 @@ ClusterManagerImpl::ThreadLocalClusterManagerImpl::ClusterEntry::ClusterEntry(
       break;
     }
     case LoadBalancerType::ClusterProvided:
+    case LoadBalancerType::LoadBalancingPolicyConfig:
     case LoadBalancerType::RingHash:
     case LoadBalancerType::Maglev:
     case LoadBalancerType::OriginalDst: {

source/common/upstream/load_balancer_factory_base.h

@@ -0,0 +1,29 @@
+#pragma once
+
+#include "envoy/upstream/load_balancer.h"
+
+namespace Envoy {
+namespace Upstream {
+
+/**
+ * Base class for cluster provided load balancers and load balancers specified by load balancing
+ * policy config. This class should be extended directly if the load balancing policy specifies a
+ * thread-aware load balancer.
+ *
+ * TODO: provide a ThreadLocalLoadBalancer construct to abstract away thread-awareness from load
+ * balancing extensions that don't require it.
+ */
+class TypedLoadBalancerFactoryBase : public TypedLoadBalancerFactory {
+public:
+  // Upstream::TypedLoadBalancerFactory
+  std::string name() const override { return name_; }
+
+protected:
+  TypedLoadBalancerFactoryBase(const std::string& name) : name_(name) {}
+
+private:
+  const std::string name_;
+};
+
+} // namespace Upstream
+} // namespace Envoy

source/common/upstream/subset_lb.cc

@@ -781,8 +781,8 @@ SubsetLoadBalancer::PrioritySubsetImpl::PrioritySubsetImpl(const SubsetLoadBalan
 
   case LoadBalancerType::OriginalDst:
   case LoadBalancerType::ClusterProvided:
-    // LoadBalancerType::OriginalDst is blocked in the factory. LoadBalancerType::ClusterProvided
-    // is impossible because the subset LB returns a null load balancer from its factory.
+  case LoadBalancerType::LoadBalancingPolicyConfig:
+    // These load balancer types can only be created when there is no subset configuration.
     NOT_REACHED_GCOVR_EXCL_LINE;
   }
 

source/common/upstream/upstream_impl.cc

@@ -808,6 +808,43 @@ ClusterInfoImpl::ClusterInfoImpl(
 
     lb_type_ = LoadBalancerType::ClusterProvided;
     break;
+  case envoy::config::cluster::v3::Cluster::LOAD_BALANCING_POLICY_CONFIG: {
+    if (config.has_lb_subset_config()) {
+      throw EnvoyException(
+          fmt::format("cluster: LB policy {} cannot be combined with lb_subset_config",
+                      envoy::config::cluster::v3::Cluster::LbPolicy_Name(config.lb_policy())));
+    }
+
+    if (config.has_common_lb_config()) {
+      throw EnvoyException(
+          fmt::format("cluster: LB policy {} cannot be combined with common_lb_config",
+                      envoy::config::cluster::v3::Cluster::LbPolicy_Name(config.lb_policy())));
+    }
+
+    if (!config.has_load_balancing_policy()) {
+      throw EnvoyException(
+          fmt::format("cluster: LB policy {} requires load_balancing_policy to be set",
+                      envoy::config::cluster::v3::Cluster::LbPolicy_Name(config.lb_policy())));
+    }
+
+    const auto& lb_policy =
+        std::find_if(config.load_balancing_policy().policies().begin(),
+                     config.load_balancing_policy().policies().end(),
+                     [](envoy::config::cluster::v3::LoadBalancingPolicy_Policy policy) {
+                       return Registry::FactoryRegistry<TypedLoadBalancerFactory>::getFactory(
+                                  policy.name()) != nullptr;
+                     });
+
+    if (lb_policy == config.load_balancing_policy().policies().end()) {
+      throw EnvoyException(fmt::format(
+          "Didn't find a registered load balancer factory implementation for cluster: '{}'",
+          name_));
+    }
+
+    lb_type_ = LoadBalancerType::LoadBalancingPolicyConfig;
+    load_balancing_policy_ = *lb_policy;
+    break;
+  }
   default:
     NOT_REACHED_GCOVR_EXCL_LINE;
   }

source/common/upstream/upstream_impl.h

@@ -568,6 +568,10 @@ class ClusterInfoImpl : public ClusterInfo,
 
   // Upstream::ClusterInfo
   bool addedViaApi() const override { return added_via_api_; }
+  const envoy::config::cluster::v3::LoadBalancingPolicy_Policy&
+  loadBalancingPolicy() const override {
+    return load_balancing_policy_;
+  }
   const envoy::config::cluster::v3::Cluster::CommonLbConfig& lbConfig() const override {
     return common_lb_config_;
   }
@@ -751,6 +755,7 @@ class ClusterInfoImpl : public ClusterInfo,
   LoadBalancerSubsetInfoImpl lb_subset_;
   const envoy::config::core::v3::Metadata metadata_;
   Envoy::Config::TypedMetadataImpl<ClusterTypedMetadataFactory> typed_metadata_;
+  envoy::config::cluster::v3::LoadBalancingPolicy_Policy load_balancing_policy_;
   const envoy::config::cluster::v3::Cluster::CommonLbConfig common_lb_config_;
   const Network::ConnectionSocket::OptionsSharedPtr cluster_socket_options_;
   const bool drain_connections_on_host_removal_;

test/common/upstream/BUILD

@@ -41,6 +41,7 @@ envoy_cc_test(
     deps = [
         ":test_cluster_manager",
         "//source/common/router:context_lib",
+        "//source/common/upstream:load_balancer_factory_base_lib",
         "//source/extensions/transport_sockets/tls:config",
         "//test/mocks/matcher:matcher_mocks",
         "//test/mocks/upstream:cds_api_mocks",