lb: moving maglev to extensions

changelogs/current.yaml

@@ -4,8 +4,8 @@ behavior_changes:
 # *Changes that are expected to cause an incompatibility if applicable; deployment changes are likely required*
 - area: build
   change: |
-    Moved the subset LB code into extensions. If you use the subset LB and override extensions_build_config.bzl you will
-    need to include it explicitly.
+    Moved the subset and maglev LB code into extensions. If you use these load balancers and override
+    extensions_build_config.bzl you will need to include them explicitly.
 
 minor_behavior_changes:
 # *Changes that may cause incompatibilities for some users, but should not for most*

source/common/upstream/BUILD

@@ -85,7 +85,6 @@ envoy_cc_library(
         ":load_balancer_lib",
         ":load_stats_reporter_lib",
         ":od_cds_api_lib",
-        ":maglev_lb_lib",
         ":ring_hash_lb_lib",
         ":host_utility_lib",
         "//envoy/api:api_interface",
@@ -384,19 +383,6 @@ envoy_cc_library(
     ],
 )
 
-envoy_cc_library(
-    name = "maglev_lb_lib",
-    srcs = ["maglev_lb.cc"],
-    hdrs = ["maglev_lb.h"],
-    deps = [
-        ":thread_aware_lb_lib",
-        ":upstream_lib",
-        "//source/common/common:bit_array_lib",
-        "@envoy_api//envoy/config/cluster/v3:pkg_cc_proto",
-        "@envoy_api//envoy/extensions/load_balancing_policies/maglev/v3:pkg_cc_proto",
-    ],
-)
-
 envoy_cc_library(
     name = "ring_hash_lb_lib",
     srcs = ["ring_hash_lb.cc"],

source/common/upstream/cluster_manager_impl.cc

@@ -44,7 +44,6 @@
 #include "source/common/upstream/cds_api_impl.h"
 #include "source/common/upstream/cluster_factory_impl.h"
 #include "source/common/upstream/load_balancer_impl.h"
-#include "source/common/upstream/maglev_lb.h"
 #include "source/common/upstream/priority_conn_pool_map_impl.h"
 #include "source/common/upstream/ring_hash_lb.h"
 
@@ -915,10 +914,11 @@ ClusterManagerImpl::loadCluster(const envoy::config::cluster::v3::Cluster& clust
     }
   } else if (cluster_reference.info()->lbType() == LoadBalancerType::Maglev) {
     if (!cluster_reference.info()->lbSubsetInfo().isEnabled()) {
-      cluster_entry_it->second->thread_aware_lb_ = std::make_unique<MaglevLoadBalancer>(
-          cluster_reference.prioritySet(), cluster_reference.info()->lbStats(),
-          cluster_reference.info()->statsScope(), runtime_, random_,
-          cluster_reference.info()->lbMaglevConfig(), cluster_reference.info()->lbConfig());
+      auto& factory = Config::Utility::getAndCheckFactoryByName<TypedLoadBalancerFactory>(
+          "envoy.load_balancing_policies.maglev");
+      cluster_entry_it->second->thread_aware_lb_ =
+          factory.create(*cluster_reference.info(), cluster_reference.prioritySet(), runtime_,
+                         random_, time_source_);
     }
   } else if (cluster_reference.info()->lbType() == LoadBalancerType::ClusterProvided) {
     cluster_entry_it->second->thread_aware_lb_ = std::move(new_cluster_pair.second);

source/extensions/load_balancing_policies/maglev/BUILD

@@ -1,22 +1,40 @@
 load(
     "//bazel:envoy_build_system.bzl",
     "envoy_cc_extension",
+    "envoy_cc_library",
     "envoy_extension_package",
 )
 
 licenses(["notice"])  # Apache 2
 
 envoy_extension_package()
 
+envoy_cc_library(
+    name = "maglev_lb_lib",
+    srcs = ["maglev_lb.cc"],
+    hdrs = ["maglev_lb.h"],
+    deps = [
+        "//source/common/common:bit_array_lib",
+        "//source/common/upstream:thread_aware_lb_lib",
+        "//source/common/upstream:upstream_lib",
+        "@envoy_api//envoy/config/cluster/v3:pkg_cc_proto",
+        "@envoy_api//envoy/extensions/load_balancing_policies/maglev/v3:pkg_cc_proto",
+    ],
+)
+
 envoy_cc_extension(
     name = "config",
     srcs = ["config.cc"],
     hdrs = ["config.h"],
+    extra_visibility = [
+        # previously considered core code.
+        "//test:__subpackages__",
+    ],
     deps = [
+        ":maglev_lb_lib",
         "//source/common/common:minimal_logger_lib",
         "//source/common/upstream:load_balancer_factory_base_lib",
         "//source/common/upstream:load_balancer_lib",
-        "//source/common/upstream:maglev_lb_lib",
         "@envoy_api//envoy/extensions/load_balancing_policies/maglev/v3:pkg_cc_proto",
     ],
 )

source/extensions/load_balancing_policies/maglev/config.cc

@@ -2,7 +2,7 @@
 
 #include "envoy/extensions/load_balancing_policies/maglev/v3/maglev.pb.h"
 
-#include "source/common/upstream/maglev_lb.h"
+#include "source/extensions/load_balancing_policies/maglev/maglev_lb.h"
 
 namespace Envoy {
 namespace Extensions {
@@ -18,10 +18,12 @@ Upstream::ThreadAwareLoadBalancerPtr Factory::create(const Upstream::ClusterInfo
       dynamic_cast<const envoy::extensions::load_balancing_policies::maglev::v3::Maglev*>(
           cluster_info.loadBalancingPolicy().get());
 
-  // The load balancing policy configuration will be loaded and validated in the main thread when we
-  // load the cluster configuration. So we can assume the configuration is valid here.
-  ASSERT(typed_config != nullptr,
-         "Invalid load balancing policy configuration for maglev load balancer");
+  // Assume legacy config.
+  if (!typed_config) {
+    return std::make_unique<Upstream::MaglevLoadBalancer>(
+        priority_set, cluster_info.lbStats(), cluster_info.statsScope(), runtime, random,
+        cluster_info.lbMaglevConfig(), cluster_info.lbConfig());
+  }
 
   return std::make_unique<Upstream::MaglevLoadBalancer>(
       priority_set, cluster_info.lbStats(), cluster_info.statsScope(), runtime, random,

source/common/upstream/maglev_lb.cc → source/extensions/load_balancing_policies/maglev/maglev_lb.cc

@@ -1,4 +1,4 @@
-#include "source/common/upstream/maglev_lb.h"
+#include "source/extensions/load_balancing_policies/maglev/maglev_lb.h"
 
 #include "envoy/config/cluster/v3/cluster.pb.h"
 

source/extensions/load_balancing_policies/subset/BUILD

@@ -22,9 +22,9 @@ envoy_cc_library(
         "//source/common/protobuf",
         "//source/common/protobuf:utility_lib",
         "//source/common/upstream:load_balancer_lib",
-        "//source/common/upstream:maglev_lb_lib",
         "//source/common/upstream:ring_hash_lb_lib",
         "//source/common/upstream:upstream_lib",
+        "//source/extensions/load_balancing_policies/maglev:maglev_lb_lib",
         "@envoy_api//envoy/config/cluster/v3:pkg_cc_proto",
         "@envoy_api//envoy/config/core/v3:pkg_cc_proto",
     ],

source/extensions/load_balancing_policies/subset/subset_lb.cc

@@ -12,8 +12,8 @@
 #include "source/common/config/well_known_names.h"
 #include "source/common/protobuf/utility.h"
 #include "source/common/upstream/load_balancer_impl.h"
-#include "source/common/upstream/maglev_lb.h"
 #include "source/common/upstream/ring_hash_lb.h"
+#include "source/extensions/load_balancing_policies/maglev/maglev_lb.h"
 
 #include "absl/container/node_hash_set.h"
 

test/common/upstream/BUILD

@@ -79,6 +79,7 @@ envoy_cc_test(
         "//source/extensions/clusters/strict_dns:strict_dns_cluster_lib",
         "//source/extensions/health_checkers/http:health_checker_lib",
         "//source/extensions/health_checkers/tcp:health_checker_lib",
+        "//source/extensions/load_balancing_policies/maglev:config",
         "//source/extensions/network/dns_resolver/cares:config",
         "//source/extensions/transport_sockets/tls:config",
         "//test/config:v2_link_hacks",
@@ -445,24 +446,6 @@ envoy_cc_test(
     ],
 )
 
-envoy_cc_test(
-    name = "maglev_lb_test",
-    srcs = ["maglev_lb_test.cc"],
-    deps = [
-        ":utility_lib",
-        "//source/common/upstream:maglev_lb_lib",
-        "//test/mocks:common_lib",
-        "//test/mocks/upstream:cluster_info_mocks",
-        "//test/mocks/upstream:host_mocks",
-        "//test/mocks/upstream:host_set_mocks",
-        "//test/mocks/upstream:load_balancer_context_mock",
-        "//test/mocks/upstream:priority_set_mocks",
-        "//test/test_common:simulated_time_system_lib",
-        "//test/test_common:test_runtime_lib",
-        "@envoy_api//envoy/config/cluster/v3:pkg_cc_proto",
-    ],
-)
-
 envoy_cc_test(
     name = "bounded_load_hlb_test",
     srcs = ["bounded_load_hlb_test.cc"],
@@ -484,9 +467,9 @@ envoy_cc_benchmark_binary(
     deps = [
         "//source/common/common:random_generator_lib",
         "//source/common/memory:stats_lib",
-        "//source/common/upstream:maglev_lb_lib",
         "//source/common/upstream:ring_hash_lb_lib",
         "//source/common/upstream:upstream_lib",
+        "//source/extensions/load_balancing_policies/maglev:config",
         "//source/extensions/load_balancing_policies/subset:config",
         "//test/common/upstream:utility_lib",
         "//test/mocks/upstream:cluster_info_mocks",

test/common/upstream/load_balancer_benchmark.cc

@@ -6,9 +6,9 @@
 
 #include "source/common/common/random_generator.h"
 #include "source/common/memory/stats.h"
-#include "source/common/upstream/maglev_lb.h"
 #include "source/common/upstream/ring_hash_lb.h"
 #include "source/common/upstream/upstream_impl.h"
+#include "source/extensions/load_balancing_policies/maglev/maglev_lb.h"
 #include "source/extensions/load_balancing_policies/subset/subset_lb.h"
 
 #include "test/benchmark/main.h"

test/extensions/load_balancing_policies/maglev/BUILD

@@ -11,6 +11,25 @@ licenses(["notice"])  # Apache 2
 
 envoy_package()
 
+envoy_extension_cc_test(
+    name = "maglev_lb_test",
+    srcs = ["maglev_lb_test.cc"],
+    extension_names = ["envoy.load_balancing_policies.maglev"],
+    deps = [
+        "//source/extensions/load_balancing_policies/maglev:maglev_lb_lib",
+        "//test/common/upstream:utility_lib",
+        "//test/mocks:common_lib",
+        "//test/mocks/upstream:cluster_info_mocks",
+        "//test/mocks/upstream:host_mocks",
+        "//test/mocks/upstream:host_set_mocks",
+        "//test/mocks/upstream:load_balancer_context_mock",
+        "//test/mocks/upstream:priority_set_mocks",
+        "//test/test_common:simulated_time_system_lib",
+        "//test/test_common:test_runtime_lib",
+        "@envoy_api//envoy/config/cluster/v3:pkg_cc_proto",
+    ],
+)
+
 envoy_extension_cc_test(
     name = "config_test",
     srcs = ["config_test.cc"],

test/common/upstream/maglev_lb_test.cc → test/extensions/load_balancing_policies/maglev/maglev_lb_test.cc

@@ -2,7 +2,7 @@
 
 #include "envoy/config/cluster/v3/cluster.pb.h"
 
-#include "source/common/upstream/maglev_lb.h"
+#include "source/extensions/load_balancing_policies/maglev/maglev_lb.h"
 
 #include "test/common/upstream/utility.h"
 #include "test/mocks/common.h"

test/integration/BUILD

@@ -952,6 +952,7 @@ envoy_cc_test_library(
         "base_integration_test.h",
     ],
     deps = [
+        "//source/extensions/load_balancing_policies/maglev:config",
         "//source/extensions/config_subscription/filesystem:filesystem_subscription_lib",
         "//source/server:proto_descriptors_lib",
         "//source/extensions/request_id/uuid:config",

test/mocks/upstream/cluster_info.cc

@@ -198,6 +198,7 @@ MockClusterInfo::MockClusterInfo()
             manager.applyFilterFactoryCb({}, factory_cb);
             return true;
           }));
+  ON_CALL(*this, loadBalancingPolicy).WillByDefault(ReturnRef(load_balancing_policy_));
   ON_CALL(*this, makeHeaderValidator(_)).WillByDefault(Invoke([&](Http::Protocol protocol) {
     return header_validator_factory_ ? header_validator_factory_->createClientHeaderValidator(
                                            protocol, codecStats(protocol))

test/mocks/upstream/cluster_info.h

@@ -253,6 +253,7 @@ class MockClusterInfo : public ClusterInfo {
   mutable Http::Http1::CodecStats::AtomicPtr http1_codec_stats_;
   mutable Http::Http2::CodecStats::AtomicPtr http2_codec_stats_;
   mutable Http::Http3::CodecStats::AtomicPtr http3_codec_stats_;
+  ProtobufTypes::MessagePtr load_balancing_policy_;
   Http::HeaderValidatorFactoryPtr header_validator_factory_;
 };