From 7a3e602c11d4f9a5312401743cbfda30554a4d14 Mon Sep 17 00:00:00 2001 From: eric846 <56563761+eric846@users.noreply.github.com> Date: Thu, 28 Mar 2024 09:37:48 -0500 Subject: [PATCH] Update Envoy to 38c5c86 (Mar 25, 2024) (#1110) - `.bazelrc` updated from upstream - `tools/code_format/config.yaml` updated from upstream - In an incredibly confusing refactor: - Envoy's `FaultFilterConfig` started taking a `CommonFactoryContext` instead of a `Runtime` and `TimeSource`. - Updated `HttpDynamicDelayDecoderFilterConfig` to store a `CommonFactoryContext&` instead of a `TimeSource&`, so that it has a `CommonFactoryContext&` to use when constructing a `FaultFilterConfig`. - How to get a `CommonFactoryContext` when creating the `HttpDynamicDelayDecoderFilterConfig`? `HttpDynamicDelayDecoderFilterConfigFactory` has access to a `Envoy::Server::Configuration::FactoryContext`, which has a `serverFactoryContext()` method. `ServerFactoryContext` is a subclass of `CommonFactoryContext`. - Envoy has started calling `serverFactoryContext()` on the `NighthawkServerInstance`, which had a `PANIC` placeholder there. This was discovered in the integration tests. - Gave `NighthawkServerInstance` a reference to a `NighthawkServerFactoryContext` to return in `serverFactoryContext()`, replacing the `PANIC` placeholder. - In order to remove a circular dependency, took away `NighthawkServerFactoryContext`'s reference to a `NighthawkServerInstance`, to which it delegated many calls. Now `NighthawkServerFactoryContext` stores its own references to the underlying objects formerly wrapped in the `NighthawkServerInstance`. - I originally assumed the above two things were related, but it appears that both arose here by coincidence. Signed-off-by: eric846 <56563761+eric846@users.noreply.github.com> --- .bazelrc | 4 +- bazel/repositories.bzl | 4 +- source/client/process_impl.cc | 99 ++++++++++++------- source/server/http_dynamic_delay_filter.cc | 7 +- source/server/http_dynamic_delay_filter.h | 13 ++- .../http_dynamic_delay_filter_config.cc | 2 +- tools/code_format/config.yaml | 7 +- 7 files changed, 81 insertions(+), 55 deletions(-) diff --git a/.bazelrc b/.bazelrc index 0f6216cbc..d946f6dda 100644 --- a/.bazelrc +++ b/.bazelrc @@ -370,7 +370,7 @@ build:compile-time-options --@envoy//source/extensions/filters/http/kill_request # Docker sandbox # NOTE: Update this from https://github.com/envoyproxy/envoy-build-tools/blob/main/toolchains/rbe_toolchains_config.bzl#L8 -build:docker-sandbox --experimental_docker_image=envoyproxy/envoy-build-ubuntu:0ca52447572ee105a4730da5e76fe47c9c5a7c64@sha256:d736c58f06f36848e7966752cc7e01519cc1b5101a178d5c6634807e8ac3deab +build:docker-sandbox --experimental_docker_image=envoyproxy/envoy-build-ubuntu:f94a38f62220a2b017878b790b6ea98a0f6c5f9c@sha256:2dd96b6f43c08ccabd5f4747fce5854f5f96af509b32e5cf6493f136e9833649 build:docker-sandbox --spawn_strategy=docker build:docker-sandbox --strategy=Javac=docker build:docker-sandbox --strategy=Closure=docker @@ -534,7 +534,7 @@ build:bes-envoy-engflow --bes_upload_mode=fully_async build:rbe-envoy-engflow --config=cache-envoy-engflow build:rbe-envoy-engflow --config=bes-envoy-engflow build:rbe-envoy-engflow --remote_executor=grpcs://morganite.cluster.engflow.com -build:rbe-envoy-engflow --remote_default_exec_properties=container-image=docker://docker.io/envoyproxy/envoy-build-ubuntu:0ca52447572ee105a4730da5e76fe47c9c5a7c64@sha256:d736c58f06f36848e7966752cc7e01519cc1b5101a178d5c6634807e8ac3deab +build:rbe-envoy-engflow --remote_default_exec_properties=container-image=docker://docker.io/envoyproxy/envoy-build-ubuntu:f94a38f62220a2b017878b790b6ea98a0f6c5f9c@sha256:2dd96b6f43c08ccabd5f4747fce5854f5f96af509b32e5cf6493f136e9833649 ############################################################################# # debug: Various Bazel debugging flags diff --git a/bazel/repositories.bzl b/bazel/repositories.bzl index fed27a6d8..22d6bd6a5 100644 --- a/bazel/repositories.bzl +++ b/bazel/repositories.bzl @@ -1,7 +1,7 @@ load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive") -ENVOY_COMMIT = "efd9bef80764663be31b115301812a5bb647fad5" -ENVOY_SHA = "424017e3dd804d3e0ec46aa84e009c413fb6a51ade43ac8339b5a73e1a5d3107" +ENVOY_COMMIT = "38c5c867ab92c60bb0fc32bde0f6565516ba0ff4" +ENVOY_SHA = "f6fb93e9c11c13bbee2234ffc43c6f525f3dd1294516885559bc930211f9b336" HDR_HISTOGRAM_C_VERSION = "0.11.2" # October 12th, 2020 HDR_HISTOGRAM_C_SHA = "637f28b5f64de2e268131e4e34e6eef0b91cf5ff99167db447d9b2825eae6bad" diff --git a/source/client/process_impl.cc b/source/client/process_impl.cc index 16fbc4666..6081e537b 100644 --- a/source/client/process_impl.cc +++ b/source/client/process_impl.cc @@ -130,20 +130,20 @@ class StatsConfigImpl : public Envoy::Server::Configuration::StatsConfig { // when Nighthawk is running are implemented. class NighthawkServerInstance : public Envoy::Server::Instance { public: - NighthawkServerInstance(Envoy::OptRef admin, Envoy::Api::Api& api, - Envoy::Event::Dispatcher& dispatcher, - Envoy::AccessLog::AccessLogManager& log_manager, - Envoy::Server::Options& options, Envoy::Runtime::Loader& runtime, - Envoy::Singleton::Manager& singleton_manager, - Envoy::ThreadLocal::Instance& tls, - Envoy::LocalInfo::LocalInfo& local_info, - Envoy::ProtobufMessage::ProdValidationContextImpl& validation_context, - Envoy::Grpc::Context& grpc_context, - Envoy::Router::Context& router_context) + NighthawkServerInstance( + Envoy::OptRef admin, Envoy::Api::Api& api, + Envoy::Event::Dispatcher& dispatcher, Envoy::AccessLog::AccessLogManager& log_manager, + Envoy::Server::Options& options, Envoy::Runtime::Loader& runtime, + Envoy::Singleton::Manager& singleton_manager, Envoy::ThreadLocal::Instance& tls, + Envoy::LocalInfo::LocalInfo& local_info, + Envoy::ProtobufMessage::ProdValidationContextImpl& validation_context, + Envoy::Grpc::Context& grpc_context, Envoy::Router::Context& router_context, + Envoy::Server::Configuration::ServerFactoryContext& server_factory_context) : admin_(admin), api_(api), dispatcher_(dispatcher), log_manager_(log_manager), options_(options), runtime_(runtime), singleton_manager_(singleton_manager), tls_(tls), local_info_(local_info), validation_context_(validation_context), - grpc_context_(grpc_context), router_context_(router_context) {} + grpc_context_(grpc_context), router_context_(router_context), + server_factory_context_(server_factory_context) {} void run() override { PANIC("NighthawkServerInstance::run not implemented"); } Envoy::OptRef admin() override { return admin_; } @@ -223,12 +223,14 @@ class NighthawkServerInstance : public Envoy::Server::Instance { Envoy::ProtobufMessage::ValidationContext& messageValidationContext() override { return validation_context_; } - Envoy::Server::Configuration::StatsConfig& statsConfig() override { return stats_config_; } + Envoy::Server::Configuration::StatsConfig& statsConfig() override { + PANIC("NighthawkServerInstance::statsConfig not implemented"); + } envoy::config::bootstrap::v3::Bootstrap& bootstrap() override { PANIC("NighthawkServerInstance::bootstrap not implemented"); } Envoy::Server::Configuration::ServerFactoryContext& serverFactoryContext() override { - PANIC("NighthawkServerInstance::serverFactoryContext not implemented"); + return server_factory_context_; } Envoy::Server::Configuration::TransportSocketFactoryContext& transportSocketFactoryContext() override { @@ -260,28 +262,39 @@ class NighthawkServerInstance : public Envoy::Server::Instance { Envoy::ProtobufMessage::ProdValidationContextImpl& validation_context_; Envoy::Grpc::Context& grpc_context_; Envoy::Router::Context& router_context_; - StatsConfigImpl stats_config_; + Envoy::Server::Configuration::ServerFactoryContext& server_factory_context_; }; // Implementation of Envoy::Server::Configuration::ServerFactoryContext. class NighthawkServerFactoryContext : public Envoy::Server::Configuration::ServerFactoryContext { public: - NighthawkServerFactoryContext(Envoy::Server::Instance& server, Envoy::Stats::Scope& server_scope) - : server_(server), server_scope_(server_scope) {} + NighthawkServerFactoryContext( + Envoy::OptRef admin, Envoy::Api::Api& api, + Envoy::Event::Dispatcher& dispatcher, Envoy::AccessLog::AccessLogManager& log_manager, + Envoy::Server::Options& options, Envoy::Runtime::Loader& runtime, + Envoy::Singleton::Manager& singleton_manager, Envoy::ThreadLocal::Instance& tls, + Envoy::LocalInfo::LocalInfo& local_info, + Envoy::ProtobufMessage::ProdValidationContextImpl& validation_context, + Envoy::Grpc::Context& grpc_context, Envoy::Router::Context& router_context, + Envoy::Stats::Scope& server_scope) + : admin_(admin), api_(api), dispatcher_(dispatcher), log_manager_(log_manager), + options_(options), runtime_(runtime), singleton_manager_(singleton_manager), tls_(tls), + local_info_(local_info), validation_context_(validation_context), + grpc_context_(grpc_context), router_context_(router_context), server_scope_(server_scope) {} - const Envoy::Server::Options& options() override { return server_.options(); }; + const Envoy::Server::Options& options() override { return options_; }; - Envoy::Event::Dispatcher& mainThreadDispatcher() override { return server_.dispatcher(); } + Envoy::Event::Dispatcher& mainThreadDispatcher() override { return dispatcher_; } - Envoy::Api::Api& api() override { return server_.api(); } + Envoy::Api::Api& api() override { return api_; } - Envoy::LocalInfo::LocalInfo& localInfo() const override { return server_.localInfo(); } + Envoy::LocalInfo::LocalInfo& localInfo() const override { return local_info_; } - Envoy::OptRef admin() override { return server_.admin(); } + Envoy::OptRef admin() override { return admin_; } - Envoy::Runtime::Loader& runtime() override { return server_.runtime(); } + Envoy::Runtime::Loader& runtime() override { return runtime_; } - Envoy::Singleton::Manager& singletonManager() override { return server_.singletonManager(); } + Envoy::Singleton::Manager& singletonManager() override { return singleton_manager_; } Envoy::ProtobufMessage::ValidationVisitor& messageValidationVisitor() override { return Envoy::ProtobufMessage::getStrictValidationVisitor(); @@ -293,21 +306,19 @@ class NighthawkServerFactoryContext : public Envoy::Server::Configuration::Serve Envoy::Stats::Scope& serverScope() override { return server_scope_; }; - Envoy::ThreadLocal::SlotAllocator& threadLocal() override { return server_.threadLocal(); } + Envoy::ThreadLocal::SlotAllocator& threadLocal() override { return tls_; } Envoy::Upstream::ClusterManager& clusterManager() override { PANIC("NighthawkServerFactoryContext::clusterManager not implemented"); }; Envoy::ProtobufMessage::ValidationContext& messageValidationContext() override { - return server_.messageValidationContext(); + return validation_context_; }; - Envoy::TimeSource& timeSource() override { return server_.timeSource(); }; + Envoy::TimeSource& timeSource() override { return api_.timeSource(); }; - Envoy::AccessLog::AccessLogManager& accessLogManager() override { - return server_.accessLogManager(); - } + Envoy::AccessLog::AccessLogManager& accessLogManager() override { return log_manager_; } Envoy::Server::ServerLifecycleNotifier& lifecycleNotifier() override { PANIC("NighthawkServerFactoryContext::lifecycleNotifier not implemented"); @@ -321,9 +332,9 @@ class NighthawkServerFactoryContext : public Envoy::Server::Configuration::Serve PANIC("NighthawkServerFactoryContext::initManager not implemented"); }; - Envoy::Grpc::Context& grpcContext() override { return server_.grpcContext(); }; + Envoy::Grpc::Context& grpcContext() override { return grpc_context_; }; - Envoy::Router::Context& routerContext() override { return server_.routerContext(); }; + Envoy::Router::Context& routerContext() override { return router_context_; }; Envoy::ProcessContextOptRef processContext() override { PANIC("NighthawkServerFactoryContext::processContext not implemented"); @@ -333,9 +344,7 @@ class NighthawkServerFactoryContext : public Envoy::Server::Configuration::Serve PANIC("NighthawkServerFactoryContext::drainManager not implemented"); }; - Envoy::Server::Configuration::StatsConfig& statsConfig() override { - return server_.statsConfig(); - } + Envoy::Server::Configuration::StatsConfig& statsConfig() override { return stats_config_; } envoy::config::bootstrap::v3::Bootstrap& bootstrap() override { PANIC("NighthawkServerFactoryContext::bootstrap not implemented"); @@ -354,7 +363,19 @@ class NighthawkServerFactoryContext : public Envoy::Server::Configuration::Serve } private: - Envoy::Server::Instance& server_; + Envoy::OptRef admin_; + Envoy::Api::Api& api_; + Envoy::Event::Dispatcher& dispatcher_; + Envoy::AccessLog::AccessLogManager& log_manager_; + Envoy::Server::Options& options_; + Envoy::Runtime::Loader& runtime_; + Envoy::Singleton::Manager& singleton_manager_; + Envoy::ThreadLocal::Instance& tls_; + Envoy::LocalInfo::LocalInfo& local_info_; + Envoy::ProtobufMessage::ProdValidationContextImpl& validation_context_; + Envoy::Grpc::Context& grpc_context_; + Envoy::Router::Context& router_context_; + StatsConfigImpl stats_config_; // Using the default value. Envoy::Stats::Scope& server_scope_; }; @@ -832,12 +853,14 @@ bool ProcessImpl::runInternal(OutputCollector& collector, const UriPtr& tracing_ return false; } + server_factory_context_ = std::make_unique( + admin_, *api_, *dispatcher_, access_log_manager_, envoy_options_, *runtime_loader_.get(), + *singleton_manager_, tls_, *local_info_, validation_context_, grpc_context_, + router_context_, scope_root_); server_ = std::make_unique( admin_, *api_, *dispatcher_, access_log_manager_, envoy_options_, *runtime_loader_.get(), *singleton_manager_, tls_, *local_info_, validation_context_, grpc_context_, - router_context_); - server_factory_context_ = - std::make_unique(*server_, scope_root_); + router_context_, *server_factory_context_); ssl_context_manager_ = std::make_unique( *server_factory_context_); diff --git a/source/server/http_dynamic_delay_filter.cc b/source/server/http_dynamic_delay_filter.cc index 07f877564..0b22da4cd 100644 --- a/source/server/http_dynamic_delay_filter.cc +++ b/source/server/http_dynamic_delay_filter.cc @@ -73,10 +73,11 @@ computeEffectiveConfiguration(std::shared_ptr b HttpDynamicDelayDecoderFilterConfig::HttpDynamicDelayDecoderFilterConfig( const DynamicDelayConfiguration& proto_config, Envoy::Runtime::Loader& runtime, - const std::string& stats_prefix, Envoy::Stats::Scope& scope, Envoy::TimeSource& time_source) + const std::string& stats_prefix, Envoy::Stats::Scope& scope, + Envoy::Server::Configuration::CommonFactoryContext& common_factory_context) : FilterConfigurationBase("dynamic-delay"), runtime_(runtime), stats_prefix_(absl::StrCat(stats_prefix, fmt::format("{}.", filter_name()))), scope_(scope), - time_source_(time_source), + common_factory_context_(common_factory_context), server_config_(std::make_shared(proto_config)) {} std::shared_ptr @@ -165,7 +166,7 @@ HttpDynamicDelayDecoderFilter::translateOurConfigIntoFaultFilterConfig( fault_config.mutable_delay()->mutable_percentage()->set_numerator(100); fault_config.mutable_delay()->mutable_header_delay(); return std::make_shared( - fault_config, config.runtime(), config.stats_prefix(), config.scope(), config.time_source()); + fault_config, config.stats_prefix(), config.scope(), config.common_factory_context()); } void HttpDynamicDelayDecoderFilter::setDecoderFilterCallbacks( diff --git a/source/server/http_dynamic_delay_filter.h b/source/server/http_dynamic_delay_filter.h index 7d21bc251..ef39be93c 100644 --- a/source/server/http_dynamic_delay_filter.h +++ b/source/server/http_dynamic_delay_filter.h @@ -34,12 +34,12 @@ class HttpDynamicDelayDecoderFilterConfig : public FilterConfigurationBase { * @param stats_prefix Prefix to use by the filter when it names statistics. E.g. * dynamic-delay.fault.delays_injected: 1 * @param scope Statistics scope to be used by the filter. - * @param time_source Time source to be used by the filter. + * @param common_factory_context CommonFactoryContext to be used by the filter. */ HttpDynamicDelayDecoderFilterConfig( const nighthawk::server::DynamicDelayConfiguration& proto_config, Envoy::Runtime::Loader& runtime, const std::string& stats_prefix, Envoy::Stats::Scope& scope, - Envoy::TimeSource& time_source); + Envoy::Server::Configuration::CommonFactoryContext& common_factory_context); /** * Increments the number of globally active filter instances. */ @@ -67,9 +67,12 @@ class HttpDynamicDelayDecoderFilterConfig : public FilterConfigurationBase { Envoy::Stats::Scope& scope() { return scope_; } /** - * @return Envoy::TimeSource& to be used by filter instantiations associated to this. + * @return Envoy::Server::Configuration::CommonFactoryContext& to be used by filter + * instantiations associated to this. */ - Envoy::TimeSource& time_source() { return time_source_; } + Envoy::Server::Configuration::CommonFactoryContext& common_factory_context() { + return common_factory_context_; + } /** * @return std::string to be used by filter instantiations associated to this. @@ -92,7 +95,7 @@ class HttpDynamicDelayDecoderFilterConfig : public FilterConfigurationBase { Envoy::Runtime::Loader& runtime_; const std::string stats_prefix_; Envoy::Stats::Scope& scope_; - Envoy::TimeSource& time_source_; + Envoy::Server::Configuration::CommonFactoryContext& common_factory_context_; std::shared_ptr server_config_; }; diff --git a/source/server/http_dynamic_delay_filter_config.cc b/source/server/http_dynamic_delay_filter_config.cc index 0405a8ab1..59e4fc3a5 100644 --- a/source/server/http_dynamic_delay_filter_config.cc +++ b/source/server/http_dynamic_delay_filter_config.cc @@ -43,7 +43,7 @@ class HttpDynamicDelayDecoderFilterConfigFactory std::make_shared( Nighthawk::Server::HttpDynamicDelayDecoderFilterConfig( proto_config, context.serverFactoryContext().runtime(), "" /*stats_prefix*/, - context.scope(), context.serverFactoryContext().timeSource())); + context.scope(), context.serverFactoryContext())); return [config](Envoy::Http::FilterChainFactoryCallbacks& callbacks) -> void { auto* filter = new Nighthawk::Server::HttpDynamicDelayDecoderFilter(config); diff --git a/tools/code_format/config.yaml b/tools/code_format/config.yaml index 9ac325220..ffb5535e3 100644 --- a/tools/code_format/config.yaml +++ b/tools/code_format/config.yaml @@ -133,19 +133,16 @@ paths: - source/common/tcp_proxy/tcp_proxy.cc - source/common/config/subscription_factory_impl.cc - source/common/config/xds_resource.cc - - source/common/runtime/runtime_impl.cc - source/common/filter/config_discovery_impl.cc - source/common/json/json_internal.cc - source/common/router/scoped_rds.cc - source/common/router/config_impl.cc - source/common/router/scoped_config_impl.cc - - source/common/router/router_ratelimit.cc - source/common/router/header_parser.cc - - source/common/filesystem/inotify/watcher_impl.cc - source/common/filesystem/posix/directory_iterator_impl.cc + - source/common/filesystem/inotify/watcher_impl.cc - source/common/filesystem/kqueue/watcher_impl.cc - source/common/filesystem/win32/directory_iterator_impl.cc - - source/common/filesystem/win32/watcher_impl.cc - source/common/common/utility.cc - source/common/common/regex.cc - source/common/common/matchers.cc @@ -154,6 +151,7 @@ paths: - source/server/overload_manager_impl.cc - source/server/config_validation/server.cc - source/server/admin/html/active_stats.js + - source/server/admin/runtime_handler.cc - source/server/server.cc - source/server/hot_restarting_base.cc - source/server/hot_restart_impl.cc @@ -173,6 +171,7 @@ paths: - source/common/local_reply/local_reply.cc - source/common/tls/context_impl.cc - source/common/tls/context_config_impl.cc + - source/common/config/watched_directory.cc # Only one C++ file should instantiate grpc_init grpc_init: