mobile: moving hds prod factory out of E-M build (envoyproxy#37291)

Risk Level: low
Testing: updated tests
Docs Changes: n/a
Release Notes: n/a
envoyproxy/envoy-mobile#176

Signed-off-by: Alyssa Wilk <[email protected]>

mobile/library/common/engine_common.cc

@@ -70,8 +70,8 @@ class ServerLite : public Server::InstanceBase {
   }
   std::unique_ptr<Server::HdsDelegateApi>
   maybeCreateHdsDelegate(Server::Configuration::ServerFactoryContext&, Stats::Scope&,
-                         Grpc::RawAsyncClientPtr&&, Envoy::Stats::Store&, Ssl::ContextManager&,
-                         Upstream::ClusterInfoFactory&) override {
+                         Grpc::RawAsyncClientPtr&&, Envoy::Stats::Store&,
+                         Ssl::ContextManager&) override {
     return nullptr;
   }
 };

mobile/test/performance/files_em_does_not_use

@@ -1,4 +1,5 @@
 # .github/workflows/mobile-perf.yml regression tests these files are not used in the E-M build.
+source/common/upstream/prod_cluster_info_factory.h
 source/extensions/filters/http/on_demand/config.h
 source/extensions/filters/http/on_demand/on_demand_update.h
 source/common/http/route_config_update_requster.h

source/common/upstream/BUILD

@@ -312,6 +312,7 @@ envoy_cc_library(
     deps = [
         ":health_checker_lib",
         ":locality_endpoint_lib",
+        ":prod_cluster_info_factory_lib",
         ":upstream_includes",
         "//envoy/api:api_interface",
         "//envoy/event:dispatcher_interface",

source/common/upstream/health_discovery_service.cc

@@ -29,14 +29,14 @@ static constexpr uint32_t RetryMaxDelayMilliseconds = 30000;
 
 HdsDelegate::HdsDelegate(Server::Configuration::ServerFactoryContext& server_context,
                          Stats::Scope& scope, Grpc::RawAsyncClientPtr async_client,
-                         Envoy::Stats::Store& stats, Ssl::ContextManager& ssl_context_manager,
-                         ClusterInfoFactory& info_factory)
+                         Envoy::Stats::Store& stats, Ssl::ContextManager& ssl_context_manager)
     : stats_{ALL_HDS_STATS(POOL_COUNTER_PREFIX(scope, "hds_delegate."))},
       service_method_(*Protobuf::DescriptorPool::generated_pool()->FindMethodByName(
           "envoy.service.health.v3.HealthDiscoveryService.StreamHealthCheck")),
       async_client_(std::move(async_client)), dispatcher_(server_context.mainThreadDispatcher()),
       server_context_(server_context), store_stats_(stats),
-      ssl_context_manager_(ssl_context_manager), info_factory_(info_factory),
+      ssl_context_manager_(ssl_context_manager),
+      info_factory_(std::make_unique<ProdClusterInfoFactory>()),
       tls_(server_context_.threadLocal()) {
   health_check_request_.mutable_health_check_request()->mutable_node()->MergeFrom(
       server_context.localInfo().node());
@@ -199,7 +199,7 @@ absl::Status
 HdsDelegate::updateHdsCluster(HdsClusterPtr cluster,
                               const envoy::config::cluster::v3::Cluster& cluster_config,
                               const envoy::config::core::v3::BindConfig& bind_config) {
-  return cluster->update(cluster_config, bind_config, info_factory_, tls_);
+  return cluster->update(cluster_config, bind_config, *info_factory_, tls_);
 }
 
 HdsClusterPtr
@@ -208,7 +208,7 @@ HdsDelegate::createHdsCluster(const envoy::config::cluster::v3::Cluster& cluster
   // Create HdsCluster.
   auto new_cluster =
       std::make_shared<HdsCluster>(server_context_, std::move(cluster_config), bind_config,
-                                   store_stats_, ssl_context_manager_, false, info_factory_, tls_);
+                                   store_stats_, ssl_context_manager_, false, *info_factory_, tls_);
 
   // Begin HCs in the background.
   new_cluster->initialize([] {});

source/common/upstream/health_discovery_service.h

@@ -20,6 +20,7 @@
 #include "source/common/network/resolver_impl.h"
 #include "source/common/upstream/health_checker_impl.h"
 #include "source/common/upstream/locality_endpoint.h"
+#include "source/common/upstream/prod_cluster_info_factory.h"
 #include "source/common/upstream/upstream_impl.h"
 #include "source/server/transport_socket_config_impl.h"
 
@@ -138,7 +139,7 @@ class HdsDelegate : Grpc::AsyncStreamCallbacks<envoy::service::health::v3::Healt
 public:
   HdsDelegate(Server::Configuration::ServerFactoryContext& server_context, Stats::Scope& scope,
               Grpc::RawAsyncClientPtr async_client, Envoy::Stats::Store& stats,
-              Ssl::ContextManager& ssl_context_manager, ClusterInfoFactory& info_factory);
+              Ssl::ContextManager& ssl_context_manager);
 
   // Grpc::AsyncStreamCallbacks
   void onCreateInitialMetadata(Http::RequestHeaderMap& metadata) override;
@@ -180,7 +181,7 @@ class HdsDelegate : Grpc::AsyncStreamCallbacks<envoy::service::health::v3::Healt
   Server::Configuration::ServerFactoryContext& server_context_;
   Envoy::Stats::Store& store_stats_;
   Ssl::ContextManager& ssl_context_manager_;
-  ClusterInfoFactory& info_factory_;
+  std::unique_ptr<ClusterInfoFactory> info_factory_;
   ThreadLocal::SlotAllocator& tls_;
 
   envoy::service::health::v3::HealthCheckRequestOrEndpointHealthResponse health_check_request_;

source/server/BUILD

@@ -461,7 +461,6 @@ envoy_cc_library(
         "//source/common/stats:thread_local_store_lib",
         "//source/common/tls:context_lib",
         "//source/common/upstream:cluster_manager_lib",
-        "//source/common/upstream:prod_cluster_info_factory_lib",
         "//source/common/version:version_lib",
         "//source/server/admin:admin_lib",
         "@com_google_absl//absl/container:node_hash_map",
@@ -480,6 +479,7 @@ envoy_cc_library(
         ":server_base_lib",
         "//source/common/memory:heap_shrinker_lib",
         "//source/common/upstream:health_discovery_service_lib",
+        "//source/common/upstream:prod_cluster_info_factory_lib",
         "//source/server:null_overload_manager_lib",
         "//source/server:overload_manager_lib",
     ],

source/server/instance_impl.cc

@@ -27,12 +27,13 @@ std::unique_ptr<Server::GuardDog> InstanceImpl::maybeCreateGuardDog(absl::string
                                                 config().mainThreadWatchdogConfig(), api(), name);
 }
 
-std::unique_ptr<HdsDelegateApi> InstanceImpl::maybeCreateHdsDelegate(
-    Configuration::ServerFactoryContext& server_context, Stats::Scope& scope,
-    Grpc::RawAsyncClientPtr&& async_client, Envoy::Stats::Store& stats,
-    Ssl::ContextManager& ssl_context_manager, Upstream::ClusterInfoFactory& info_factory) {
+std::unique_ptr<HdsDelegateApi>
+InstanceImpl::maybeCreateHdsDelegate(Configuration::ServerFactoryContext& server_context,
+                                     Stats::Scope& scope, Grpc::RawAsyncClientPtr&& async_client,
+                                     Envoy::Stats::Store& stats,
+                                     Ssl::ContextManager& ssl_context_manager) {
   return std::make_unique<Upstream::HdsDelegate>(server_context, scope, std::move(async_client),
-                                                 stats, ssl_context_manager, info_factory);
+                                                 stats, ssl_context_manager);
 }
 
 } // namespace Server

source/server/instance_impl.h

@@ -19,8 +19,7 @@ class InstanceImpl : public InstanceBase {
   std::unique_ptr<HdsDelegateApi>
   maybeCreateHdsDelegate(Configuration::ServerFactoryContext& server_context, Stats::Scope& scope,
                          Grpc::RawAsyncClientPtr&& async_client, Envoy::Stats::Store& stats,
-                         Ssl::ContextManager& ssl_context_manager,
-                         Upstream::ClusterInfoFactory& info_factory) override;
+                         Ssl::ContextManager& ssl_context_manager) override;
 
 private:
   std::unique_ptr<Memory::HeapShrinker> heap_shrinker_;

source/server/server.cc

@@ -842,7 +842,7 @@ void InstanceBase::onRuntimeReady() {
       hds_delegate_ =
           maybeCreateHdsDelegate(serverFactoryContext(), *stats_store_.rootScope(),
                                  factory_or_error.value()->createUncachedRawAsyncClient(),
-                                 stats_store_, *ssl_context_manager_, info_factory_);
+                                 stats_store_, *ssl_context_manager_);
     }
     END_TRY
     CATCH(const EnvoyException& e,

source/server/server.h

@@ -40,7 +40,6 @@
 #include "source/common/runtime/runtime_impl.h"
 #include "source/common/secret/secret_manager_impl.h"
 #include "source/common/singleton/manager_impl.h"
-#include "source/common/upstream/prod_cluster_info_factory.h"
 
 #ifdef ENVOY_ADMIN_FUNCTIONALITY
 #include "source/server/admin/admin.h"
@@ -263,8 +262,7 @@ class InstanceBase : Logger::Loggable<Logger::Id::main>,
   virtual std::unique_ptr<HdsDelegateApi>
   maybeCreateHdsDelegate(Configuration::ServerFactoryContext& server_context, Stats::Scope& scope,
                          Grpc::RawAsyncClientPtr&& async_client, Envoy::Stats::Store& stats,
-                         Ssl::ContextManager& ssl_context_manager,
-                         Upstream::ClusterInfoFactory& info_factory) PURE;
+                         Ssl::ContextManager& ssl_context_manager) PURE;
 
   void run() override;
 
@@ -417,7 +415,6 @@ class InstanceBase : Logger::Loggable<Logger::Id::main>,
   SystemTime bootstrap_config_update_time_;
   Grpc::AsyncClientManagerPtr async_client_manager_;
   Config::XdsManagerPtr xds_manager_;
-  Upstream::ProdClusterInfoFactory info_factory_;
   std::unique_ptr<HdsDelegateApi> hds_delegate_;
   std::unique_ptr<OverloadManager> overload_manager_;
   std::unique_ptr<OverloadManager> null_overload_manager_;

test/common/upstream/hds_test.cc

@@ -54,6 +54,9 @@ class HdsDelegateFriend {
     ASSERT_TRUE(hd.processMessage(std::move(message)).ok());
   };
   HdsDelegateStats getStats(HdsDelegate& hd) { return hd.stats_; };
+  static void swapFactory(HdsDelegate& hd, std::unique_ptr<ClusterInfoFactory>&& factory) {
+    hd.info_factory_ = std::move(factory);
+  }
 };
 
 class HdsTest : public testing::Test {
@@ -93,9 +96,12 @@ class HdsTest : public testing::Test {
         }))
         .WillRepeatedly(testing::ReturnNew<NiceMock<Event::MockTimer>>());
 
-    hds_delegate_ = std::make_unique<HdsDelegate>(
-        server_context_, *stats_store_.rootScope(), Grpc::RawAsyncClientPtr(async_client_),
-        stats_store_, ssl_context_manager_, test_factory_);
+    hds_delegate_ = std::make_unique<HdsDelegate>(server_context_, *stats_store_.rootScope(),
+                                                  Grpc::RawAsyncClientPtr(async_client_),
+                                                  stats_store_, ssl_context_manager_);
+    test_factory_ = new MockClusterInfoFactory();
+    HdsDelegateFriend::swapFactory(*hds_delegate_,
+                                   std::unique_ptr<ClusterInfoFactory>(test_factory_));
   }
 
   void expectCreateClientConnection() {
@@ -237,7 +243,7 @@ class HdsTest : public testing::Test {
   Event::SimulatedTimeSystem time_system_;
   envoy::config::core::v3::Node node_;
   Stats::IsolatedStoreImpl stats_store_;
-  MockClusterInfoFactory test_factory_;
+  MockClusterInfoFactory* test_factory_;
 
   std::unique_ptr<Upstream::HdsDelegate> hds_delegate_;
   HdsDelegateFriend hds_delegate_friend_;
@@ -298,7 +304,7 @@ TEST_F(HdsTest, TestProcessMessageEndpoints) {
   }
 
   // Process message
-  EXPECT_CALL(test_factory_, createClusterInfo(_)).Times(2).WillRepeatedly(Return(cluster_info_));
+  EXPECT_CALL(*test_factory_, createClusterInfo(_)).Times(2).WillRepeatedly(Return(cluster_info_));
   hds_delegate_friend_.processPrivateMessage(*hds_delegate_, std::move(message));
 
   // Check Correctness
@@ -328,7 +334,7 @@ TEST_F(HdsTest, TestHdsCluster) {
   address->mutable_socket_address()->set_port_value(1234);
 
   // Process message
-  EXPECT_CALL(test_factory_, createClusterInfo(_)).WillOnce(Return(cluster_info_));
+  EXPECT_CALL(*test_factory_, createClusterInfo(_)).WillOnce(Return(cluster_info_));
   hds_delegate_friend_.processPrivateMessage(*hds_delegate_, std::move(message));
 
   EXPECT_EQ(hds_delegate_->hdsClusters()[0]->initializePhase(),
@@ -372,7 +378,7 @@ TEST_F(HdsTest, TestProcessMessageHealthChecks) {
   }
 
   // Process message
-  EXPECT_CALL(test_factory_, createClusterInfo(_)).WillRepeatedly(Return(cluster_info_));
+  EXPECT_CALL(*test_factory_, createClusterInfo(_)).WillRepeatedly(Return(cluster_info_));
 
   hds_delegate_friend_.processPrivateMessage(*hds_delegate_, std::move(message));
 
@@ -423,7 +429,7 @@ TEST_F(HdsTest, TestProcessMessageMissingFieldsWithFallback) {
       .WillRepeatedly(Return(connection));
   EXPECT_CALL(*server_response_timer_, enableTimer(_, _)).Times(2);
   EXPECT_CALL(async_stream_, sendMessageRaw_(_, false));
-  EXPECT_CALL(test_factory_, createClusterInfo(_)).WillOnce(Return(cluster_info_));
+  EXPECT_CALL(*test_factory_, createClusterInfo(_)).WillOnce(Return(cluster_info_));
   EXPECT_CALL(*connection, setBufferLimits(_));
   EXPECT_CALL(server_context_.dispatcher_, deferredDelete_(_));
   // Process message
@@ -480,7 +486,7 @@ TEST_F(HdsTest, TestProcessMessageInvalidFieldsWithFallback) {
   EXPECT_CALL(server_context_.dispatcher_, createClientConnection_(_, _, _, _))
       .WillRepeatedly(Return(connection));
   EXPECT_CALL(*server_response_timer_, enableTimer(_, _));
-  EXPECT_CALL(test_factory_, createClusterInfo(_)).WillOnce(Return(cluster_info_));
+  EXPECT_CALL(*test_factory_, createClusterInfo(_)).WillOnce(Return(cluster_info_));
   EXPECT_CALL(*connection, setBufferLimits(_));
   EXPECT_CALL(server_context_.dispatcher_, deferredDelete_(_));
   // Process message
@@ -525,7 +531,7 @@ TEST_F(HdsTest, TestSendResponseMultipleEndpoints) {
 
   // Carry over cluster name on a call to createClusterInfo,
   // in the same way that the prod factory does.
-  EXPECT_CALL(test_factory_, createClusterInfo(_))
+  EXPECT_CALL(*test_factory_, createClusterInfo(_))
       .WillRepeatedly(Invoke([](const ClusterInfoFactory::CreateClusterInfoParams& params) {
         std::shared_ptr<Upstream::MockClusterInfo> cluster_info{
             new NiceMock<Upstream::MockClusterInfo>()};
@@ -610,7 +616,7 @@ TEST_F(HdsTest, TestSocketContext) {
   // Pull out socket_matcher object normally internal to createClusterInfo, to test that a matcher
   // would match the expected socket.
   std::unique_ptr<TransportSocketMatcherImpl> socket_matcher;
-  EXPECT_CALL(test_factory_, createClusterInfo(_))
+  EXPECT_CALL(*test_factory_, createClusterInfo(_))
       .WillRepeatedly(Invoke([&](const ClusterInfoFactory::CreateClusterInfoParams& params) {
         // Build scope, factory_context as does ProdClusterInfoFactory.
         Envoy::Stats::ScopeSharedPtr scope =
@@ -737,7 +743,7 @@ TEST_F(HdsTest, TestSendResponseOneEndpointTimeout) {
       .WillRepeatedly(Return(connection_));
   EXPECT_CALL(*server_response_timer_, enableTimer(_, _)).Times(2);
   EXPECT_CALL(async_stream_, sendMessageRaw_(_, false));
-  EXPECT_CALL(test_factory_, createClusterInfo(_)).WillOnce(Return(cluster_info_));
+  EXPECT_CALL(*test_factory_, createClusterInfo(_)).WillOnce(Return(cluster_info_));
   EXPECT_CALL(*connection_, setBufferLimits(_));
   EXPECT_CALL(server_context_.dispatcher_, deferredDelete_(_));
   // Process message
@@ -781,7 +787,7 @@ TEST_F(HdsTest, TestSameSpecifier) {
 
   EXPECT_CALL(*server_response_timer_, enableTimer(_, _)).Times(AtLeast(1));
   EXPECT_CALL(async_stream_, sendMessageRaw_(_, false));
-  EXPECT_CALL(test_factory_, createClusterInfo(_)).WillRepeatedly(Return(cluster_info_));
+  EXPECT_CALL(*test_factory_, createClusterInfo(_)).WillRepeatedly(Return(cluster_info_));
   EXPECT_CALL(server_context_.dispatcher_, deferredDelete_(_)).Times(AtLeast(1));
   hds_delegate_->onReceiveMessage(std::move(message));
   hds_delegate_->sendResponse();
@@ -816,7 +822,7 @@ TEST_F(HdsTest, TestClusterChange) {
 
   EXPECT_CALL(*server_response_timer_, enableTimer(_, _)).Times(AtLeast(1));
   EXPECT_CALL(async_stream_, sendMessageRaw_(_, false));
-  EXPECT_CALL(test_factory_, createClusterInfo(_)).WillRepeatedly(Return(cluster_info_));
+  EXPECT_CALL(*test_factory_, createClusterInfo(_)).WillRepeatedly(Return(cluster_info_));
   EXPECT_CALL(server_context_.dispatcher_, deferredDelete_(_)).Times(AtLeast(1));
   // Process message
   hds_delegate_->onReceiveMessage(std::move(message));
@@ -881,7 +887,7 @@ TEST_F(HdsTest, TestUpdateEndpoints) {
 
   EXPECT_CALL(*server_response_timer_, enableTimer(_, _)).Times(AtLeast(1));
   EXPECT_CALL(async_stream_, sendMessageRaw_(_, false));
-  EXPECT_CALL(test_factory_, createClusterInfo(_)).WillRepeatedly(Return(cluster_info_));
+  EXPECT_CALL(*test_factory_, createClusterInfo(_)).WillRepeatedly(Return(cluster_info_));
   EXPECT_CALL(server_context_.dispatcher_, deferredDelete_(_)).Times(AtLeast(1));
   // Process message
   hds_delegate_->onReceiveMessage(std::move(message));
@@ -949,7 +955,7 @@ TEST_F(HdsTest, TestUpdateEndpointsWithActiveHCflag) {
 
   EXPECT_CALL(*server_response_timer_, enableTimer(_, _)).Times(AtLeast(1));
   EXPECT_CALL(async_stream_, sendMessageRaw_(_, false));
-  EXPECT_CALL(test_factory_, createClusterInfo(_)).WillRepeatedly(Return(cluster_info_));
+  EXPECT_CALL(*test_factory_, createClusterInfo(_)).WillRepeatedly(Return(cluster_info_));
   EXPECT_CALL(server_context_.dispatcher_, deferredDelete_(_)).Times(AtLeast(1));
   // Process message
   hds_delegate_->onReceiveMessage(std::move(message));
@@ -987,7 +993,7 @@ TEST_F(HdsTest, TestUpdateHealthCheckers) {
 
   EXPECT_CALL(*server_response_timer_, enableTimer(_, _)).Times(AtLeast(1));
   EXPECT_CALL(async_stream_, sendMessageRaw_(_, false));
-  EXPECT_CALL(test_factory_, createClusterInfo(_)).WillRepeatedly(Return(cluster_info_));
+  EXPECT_CALL(*test_factory_, createClusterInfo(_)).WillRepeatedly(Return(cluster_info_));
   EXPECT_CALL(server_context_.dispatcher_, deferredDelete_(_)).Times(AtLeast(1));
   // Process message
   hds_delegate_->onReceiveMessage(std::move(message));
@@ -1041,7 +1047,7 @@ TEST_F(HdsTest, TestClusterSameName) {
 
   EXPECT_CALL(*server_response_timer_, enableTimer(_, _)).Times(AtLeast(1));
   EXPECT_CALL(async_stream_, sendMessageRaw_(_, false));
-  EXPECT_CALL(test_factory_, createClusterInfo(_)).WillRepeatedly(Return(cluster_info_));
+  EXPECT_CALL(*test_factory_, createClusterInfo(_)).WillRepeatedly(Return(cluster_info_));
   EXPECT_CALL(server_context_.dispatcher_, deferredDelete_(_)).Times(AtLeast(1));
   // Process message
   hds_delegate_->onReceiveMessage(std::move(message));
@@ -1101,7 +1107,7 @@ TEST_F(HdsTest, TestUpdateSocketContext) {
   // Pull out socket_matcher object normally internal to createClusterInfo, to test that a matcher
   // would match the expected socket.
   std::vector<std::unique_ptr<TransportSocketMatcherImpl>> socket_matchers;
-  EXPECT_CALL(test_factory_, createClusterInfo(_))
+  EXPECT_CALL(*test_factory_, createClusterInfo(_))
       .WillRepeatedly(Invoke([&](const ClusterInfoFactory::CreateClusterInfoParams& params) {
         // Build scope, factory_context as does ProdClusterInfoFactory.
         Envoy::Stats::ScopeSharedPtr scope =
@@ -1223,7 +1229,7 @@ TEST_F(HdsTest, TestCustomHealthCheckPortWhenCreate) {
   }
 
   // Process message
-  EXPECT_CALL(test_factory_, createClusterInfo(_)).WillOnce(Return(cluster_info_));
+  EXPECT_CALL(*test_factory_, createClusterInfo(_)).WillOnce(Return(cluster_info_));
   hds_delegate_friend_.processPrivateMessage(*hds_delegate_, std::move(message));
 
   // Check Correctness
@@ -1256,7 +1262,7 @@ TEST_F(HdsTest, TestCustomHealthCheckPortWhenUpdate) {
   }
 
   // Process message
-  EXPECT_CALL(test_factory_, createClusterInfo(_)).WillOnce(Return(cluster_info_));
+  EXPECT_CALL(*test_factory_, createClusterInfo(_)).WillOnce(Return(cluster_info_));
   hds_delegate_friend_.processPrivateMessage(*hds_delegate_, std::move(message));
 
   for (int i = 0; i < 3; i++) {