diff --git a/docs/configuration/listeners/listeners.rst b/docs/configuration/listeners/listeners.rst index dbaed6aa63ea..1673d9604ea6 100644 --- a/docs/configuration/listeners/listeners.rst +++ b/docs/configuration/listeners/listeners.rst @@ -90,6 +90,8 @@ statistics: listener_added, Counter, Total listeners added (either via static config or LDS) listener_modified, Counter, Total listeners modified (via LDS) listener_removed, Counter, Total listeners removed (via LDS) + listener_create_success, Counter, Total listener objects successfully added to workers. + listener_create_failure, Counter, Total failed listener object additions to workers. total_listeners_warming, Gauge, Number of currently warming listeners total_listeners_active, Gauge, Number of currently active listeners total_listeners_draining, Gauge, Number of currently draining listeners diff --git a/include/envoy/server/BUILD b/include/envoy/server/BUILD index abce7d27e525..bc25419a7558 100644 --- a/include/envoy/server/BUILD +++ b/include/envoy/server/BUILD @@ -111,6 +111,8 @@ envoy_cc_library( "//include/envoy/thread_local:thread_local_interface", "//include/envoy/tracing:http_tracer_interface", "//include/envoy/upstream:cluster_manager_interface", + "//source/common/common:macros", + "//source/common/protobuf", ], ) diff --git a/include/envoy/server/filter_config.h b/include/envoy/server/filter_config.h index 5a56629570fa..086076325a01 100644 --- a/include/envoy/server/filter_config.h +++ b/include/envoy/server/filter_config.h @@ -15,6 +15,9 @@ #include "envoy/tracing/http_tracer.h" #include "envoy/upstream/cluster_manager.h" +#include "common/common/macros.h" +#include "common/protobuf/protobuf.h" + namespace Envoy { namespace Server { @@ -150,6 +153,25 @@ class NamedNetworkFilterConfigFactory { virtual NetworkFilterFactoryCb createFilterFactory(const Json::Object& config, FactoryContext& context) PURE; + /** + * v2 variant of createFilterFactory(..), where filter configs are specified as proto. This may be + * optionally implemented today, but will in the future become compulsory once v1 is deprecated. + */ + virtual NetworkFilterFactoryCb createFilterFactoryFromProto(const Protobuf::Message& config, + FactoryContext& context) { + UNREFERENCED_PARAMETER(config); + UNREFERENCED_PARAMETER(context); + return NetworkFilterFactoryCb(); + } + + /** + * @return ProtobufTypes::MessagePtr create empty config proto message for v2. The filter + * config, which arrives in an opaque google.protobuf.Struct message, will be converted to + * JSON and then parsed into this empty proto. Optional today, will be compulsory when v1 + * is deprecated. + */ + virtual ProtobufTypes::MessagePtr createEmptyConfigProto() { return nullptr; } + /** * @return std::string the identifying name for a particular implementation of a network filter * produced by the factory. @@ -197,6 +219,28 @@ class NamedHttpFilterConfigFactory { const std::string& stat_prefix, FactoryContext& context) PURE; + /** + * v2 API variant of createFilterFactory(..), where filter configs are specified as proto. This + * may be optionally implemented today, but will in the future become compulsory once v1 is + * deprecated. + */ + virtual HttpFilterFactoryCb createFilterFactoryFromProto(const ProtobufWkt::Message& config, + const std::string& stat_prefix, + FactoryContext& context) { + UNREFERENCED_PARAMETER(config); + UNREFERENCED_PARAMETER(stat_prefix); + UNREFERENCED_PARAMETER(context); + return HttpFilterFactoryCb(); + } + + /** + * @return ProtobufTypes::MessagePtr create empty config proto message for v2. The filter + * config, which arrives in an opaque google.protobuf.Struct message, will be converted to + * JSON and then parsed into this empty proto. Optional today, will be compulsory when v1 + * is deprecated. + */ + virtual ProtobufTypes::MessagePtr createEmptyConfigProto() { return nullptr; } + /** * @return std::string the identifying name for a particular implementation of an http filter * produced by the factory. diff --git a/source/common/config/filter_json.cc b/source/common/config/filter_json.cc index 8cda2d473373..99ed03740e36 100644 --- a/source/common/config/filter_json.cc +++ b/source/common/config/filter_json.cc @@ -125,8 +125,10 @@ void FilterJson::translateHttpConnectionManager( JSON_UTIL_SET_STRING(*json_filter, *filter, name); JSON_UTIL_SET_STRING(*json_filter, *filter->mutable_deprecated_v1(), type); - const auto status = Protobuf::util::JsonStringToMessage( - json_filter->getObject("config")->asJsonString(), filter->mutable_config()); + const std::string json_config = "{\"deprecatedV1\": true, \"value\": " + + json_filter->getObject("config")->asJsonString() + "}"; + + const auto status = Protobuf::util::JsonStringToMessage(json_config, filter->mutable_config()); // JSON schema has already validated that this is a valid JSON object. ASSERT(status.ok()); UNREFERENCED_PARAMETER(status); diff --git a/source/common/config/lds_json.cc b/source/common/config/lds_json.cc index 7f0a6fae7855..4ddd06569ca9 100644 --- a/source/common/config/lds_json.cc +++ b/source/common/config/lds_json.cc @@ -28,8 +28,10 @@ void LdsJson::translateListener(const Json::Object& json_listener, JSON_UTIL_SET_STRING(*json_filter, *filter, name); JSON_UTIL_SET_STRING(*json_filter, *filter->mutable_deprecated_v1(), type); - const auto status = Protobuf::util::JsonStringToMessage( - json_filter->getObject("config")->asJsonString(), filter->mutable_config()); + const std::string json_config = "{\"deprecatedV1\": true, \"value\": " + + json_filter->getObject("config")->asJsonString() + "}"; + + const auto status = Protobuf::util::JsonStringToMessage(json_config, filter->mutable_config()); // JSON schema has already validated that this is a valid JSON object. ASSERT(status.ok()); UNREFERENCED_PARAMETER(status); diff --git a/source/common/protobuf/BUILD b/source/common/protobuf/BUILD index 1c4af65574e0..c0eb42ecbd64 100644 --- a/source/common/protobuf/BUILD +++ b/source/common/protobuf/BUILD @@ -21,6 +21,7 @@ envoy_cc_library( external_deps = ["protobuf"], deps = [ ":protobuf", + "//source/common/common:assert_lib", "//source/common/common:utility_lib", "//source/common/filesystem:filesystem_lib", "//source/common/json:json_loader_lib", diff --git a/source/common/protobuf/protobuf.h b/source/common/protobuf/protobuf.h index 3529abcc4123..0622a5250878 100644 --- a/source/common/protobuf/protobuf.h +++ b/source/common/protobuf/protobuf.h @@ -6,6 +6,7 @@ #include "google/protobuf/descriptor.h" #include "google/protobuf/descriptor.pb.h" +#include "google/protobuf/empty.pb.h" #include "google/protobuf/io/coded_stream.h" #include "google/protobuf/io/zero_copy_stream.h" #include "google/protobuf/io/zero_copy_stream_impl_lite.h" @@ -42,6 +43,8 @@ namespace ProtobufWkt = google::protobuf; // import. namespace ProtobufTypes { +typedef std::unique_ptr MessagePtr; + typedef std::string String; typedef int64_t Int64; diff --git a/source/common/protobuf/utility.cc b/source/common/protobuf/utility.cc index 38ab917c4528..b83ffd1b049b 100644 --- a/source/common/protobuf/utility.cc +++ b/source/common/protobuf/utility.cc @@ -1,5 +1,6 @@ #include "common/protobuf/utility.h" +#include "common/common/assert.h" #include "common/filesystem/filesystem_impl.h" #include "common/json/json_loader.h" @@ -12,18 +13,30 @@ MissingFieldException::MissingFieldException(const std::string& field_name, : EnvoyException( fmt::format("Field '{}' is missing in: {}", field_name, message.DebugString())) {} +void MessageUtil::loadFromJson(const std::string& json, Protobuf::Message& message) { + const auto status = Protobuf::util::JsonStringToMessage(json, &message); + if (!status.ok()) { + throw EnvoyException("Unable to parse JSON as proto (" + status.ToString() + "): " + json); + } +} + void MessageUtil::loadFromFile(const std::string& path, Protobuf::Message& message) { - ProtobufUtil::Status status; const std::string contents = Filesystem::fileReadToEnd(path); if (StringUtil::endsWith(path, ".yaml")) { const std::string json = Json::Factory::loadFromYamlString(contents)->asJsonString(); - status = Protobuf::util::JsonStringToMessage(json, &message); + loadFromJson(json, message); } else { - status = Protobuf::util::JsonStringToMessage(contents, &message); - } - if (!status.ok()) { - throw EnvoyException("Unable to parse JSON as proto: " + contents); + loadFromJson(contents, message); } } +Json::ObjectSharedPtr WktUtil::getJsonObjectFromStruct(const Protobuf::Struct& message) { + Protobuf::util::JsonOptions json_options; + ProtobufTypes::String json; + const auto status = Protobuf::util::MessageToJsonString(message, &json, json_options); + // This should always succeed unless something crash-worthy such as out-of-memory. + RELEASE_ASSERT(status.ok()); + return Json::Factory::loadFromString(json); +} + } // namespace Envoy diff --git a/source/common/protobuf/utility.h b/source/common/protobuf/utility.h index 05c8c13be791..c7af1cc14b7c 100644 --- a/source/common/protobuf/utility.h +++ b/source/common/protobuf/utility.h @@ -1,6 +1,7 @@ #pragma once #include "envoy/common/exception.h" +#include "envoy/json/json_object.h" #include "common/common/utility.h" #include "common/protobuf/protobuf.h" @@ -48,10 +49,28 @@ class RepeatedPtrUtil { class MessageUtil { public: static std::size_t hash(const Protobuf::Message& message) { - return std::hash{}(message.SerializeAsString()); + // Use Protobuf::io::CodedOutputStream to force deterministic serialization, so that the same + // message doesn't hash to different values. + std::string text; + Protobuf::io::StringOutputStream string_stream(&text); + Protobuf::io::CodedOutputStream coded_stream(&string_stream); + coded_stream.SetSerializationDeterministic(true); + message.SerializeToCodedStream(&coded_stream); + return std::hash{}(text); } + static void loadFromJson(const std::string& json, Protobuf::Message& message); static void loadFromFile(const std::string& path, Protobuf::Message& message); }; +class WktUtil { +public: + /** + * Extract JSON object from a google.protobuf.Struct. + * @param message message of type type.googleapis.com/google.protobuf.Struct. + * @return Json::ObjectSharedPtr of JSON object or nullptr if unable to extract. + */ + static Json::ObjectSharedPtr getJsonObjectFromStruct(const Protobuf::Struct& message); +}; + } // namespace Envoy diff --git a/source/server/config/network/http_connection_manager.cc b/source/server/config/network/http_connection_manager.cc index a08d4640d23e..3ae7a96f61e3 100644 --- a/source/server/config/network/http_connection_manager.cc +++ b/source/server/config/network/http_connection_manager.cc @@ -33,8 +33,11 @@ const std::string HttpConnectionManagerConfig::DEFAULT_SERVER_STRING = "envoy"; SINGLETON_MANAGER_REGISTRATION(date_provider); SINGLETON_MANAGER_REGISTRATION(route_config_provider_manager); -NetworkFilterFactoryCb HttpConnectionManagerFilterConfigFactory::createFilterFactory( - const Json::Object& json_http_connection_manager, FactoryContext& context) { +namespace { + +NetworkFilterFactoryCb createHttpConnectionManagerFilterFactory( + const envoy::api::v2::filter::HttpConnectionManager& http_connection_manager, + FactoryContext& context) { std::shared_ptr date_provider = context.singletonManager().getTyped( SINGLETON_MANAGER_REGISTERED_NAME(date_provider), [&context] { @@ -50,9 +53,6 @@ NetworkFilterFactoryCb HttpConnectionManagerFilterConfigFactory::createFilterFac context.threadLocal()); }); - envoy::api::v2::filter::HttpConnectionManager http_connection_manager; - Config::FilterJson::translateHttpConnectionManager(json_http_connection_manager, - http_connection_manager); std::shared_ptr http_config(new HttpConnectionManagerConfig( http_connection_manager, context, *date_provider, *route_config_provider_manager)); @@ -67,6 +67,22 @@ NetworkFilterFactoryCb HttpConnectionManagerFilterConfigFactory::createFilterFac }; } +} // namespace + +NetworkFilterFactoryCb HttpConnectionManagerFilterConfigFactory::createFilterFactory( + const Json::Object& json_http_connection_manager, FactoryContext& context) { + envoy::api::v2::filter::HttpConnectionManager http_connection_manager; + Config::FilterJson::translateHttpConnectionManager(json_http_connection_manager, + http_connection_manager); + return createHttpConnectionManagerFilterFactory(http_connection_manager, context); +} + +NetworkFilterFactoryCb HttpConnectionManagerFilterConfigFactory::createFilterFactoryFromProto( + const Protobuf::Message& config, FactoryContext& context) { + return createHttpConnectionManagerFilterFactory( + dynamic_cast(config), context); +} + /** * Static registration for the HTTP connection manager filter. */ @@ -210,23 +226,26 @@ HttpConnectionManagerConfig::HttpConnectionManagerConfig( ENVOY_LOG(info, " filter #{}", i); ENVOY_LOG(info, " name: {}", string_name); - Protobuf::util::JsonOptions json_options; - ProtobufTypes::String json_config; - const auto status = - Protobuf::util::MessageToJsonString(proto_config, &json_config, json_options); - // This should always succeed unless something crash-worthy such as out-of-memory. - RELEASE_ASSERT(status.ok()); - UNREFERENCED_PARAMETER(status); - const Json::ObjectSharedPtr filter_config = Json::Factory::loadFromString(json_config); - + const Json::ObjectSharedPtr filter_config = WktUtil::getJsonObjectFromStruct(proto_config); const HttpFilterType type = stringToType(string_type); // Now see if there is a factory that will accept the config. NamedHttpFilterConfigFactory* factory = Registry::FactoryRegistry::getFactory(string_name); if (factory != nullptr) { - HttpFilterFactoryCb callback = - factory->createFilterFactory(*filter_config, stats_prefix_, context_); + HttpFilterFactoryCb callback; + if (filter_config->getBoolean("deprecatedV1", false)) { + callback = factory->createFilterFactory(*filter_config->getObject("value", true), + stats_prefix_, context); + } else { + auto message = factory->createEmptyConfigProto(); + if (!message) { + throw EnvoyException( + fmt::format("Filter factory for '{}' has unexpected proto config", string_name)); + } + MessageUtil::loadFromJson(filter_config->asJsonString(), *message); + callback = factory->createFilterFactoryFromProto(*message, stats_prefix_, context); + } filter_factories_.push_back(callback); } else { // DEPRECATED @@ -234,7 +253,8 @@ HttpConnectionManagerConfig::HttpConnectionManagerConfig( bool found_filter = false; for (HttpFilterConfigFactory* config_factory : filterConfigFactories()) { HttpFilterFactoryCb callback = config_factory->tryCreateFilterFactory( - type, string_name, *filter_config, stats_prefix_, context_.server()); + type, string_name, *filter_config->getObject("value", true), stats_prefix_, + context_.server()); if (callback) { filter_factories_.push_back(callback); found_filter = true; @@ -287,7 +307,7 @@ HttpFilterType HttpConnectionManagerConfig::stringToType(const std::string& type } else if (type == "encoder") { return HttpFilterType::Encoder; } else { - ASSERT(type == "both"); + ASSERT(type == "both" || type.empty()); return HttpFilterType::Both; } } diff --git a/source/server/config/network/http_connection_manager.h b/source/server/config/network/http_connection_manager.h index 9463f566e060..738b92d0ca20 100644 --- a/source/server/config/network/http_connection_manager.h +++ b/source/server/config/network/http_connection_manager.h @@ -27,7 +27,13 @@ class HttpConnectionManagerFilterConfigFactory : Logger::Loggable( + new envoy::api::v2::filter::HttpConnectionManager()); + } std::string name() override { return "http_connection_manager"; } NetworkFilterType type() override { return NetworkFilterType::Read; } }; diff --git a/source/server/configuration_impl.cc b/source/server/configuration_impl.cc index 9ea7f17571b4..0d0941a2693b 100644 --- a/source/server/configuration_impl.cc +++ b/source/server/configuration_impl.cc @@ -52,7 +52,11 @@ void MainImpl::initialize(const Json::Object& json, const envoy::api::v2::Bootst server.listenerManager().addOrUpdateListener(listener); } - if (json.hasObject("lds")) { + if (bootstrap.has_lds_config()) { + lds_api_.reset(new LdsApi(bootstrap.lds_config(), *cluster_manager_, server.dispatcher(), + server.random(), server.initManager(), server.localInfo(), + server.stats(), server.listenerManager())); + } else if (json.hasObject("lds")) { envoy::api::v2::ConfigSource lds_config; Config::Utility::translateLdsConfig(*json.getObject("lds"), lds_config); lds_api_.reset(new LdsApi(lds_config, *cluster_manager_, server.dispatcher(), server.random(), diff --git a/source/server/listener_manager_impl.cc b/source/server/listener_manager_impl.cc index a5ab77680c93..8241cf0078d6 100644 --- a/source/server/listener_manager_impl.cc +++ b/source/server/listener_manager_impl.cc @@ -25,15 +25,7 @@ ProdListenerComponentFactory::createFilterFactoryList_( const auto& proto_config = filters[i].config(); ENVOY_LOG(info, " filter #{}:", i); ENVOY_LOG(info, " name: {}", string_name); - - Protobuf::util::JsonOptions json_options; - ProtobufTypes::String json_config; - const auto status = - Protobuf::util::MessageToJsonString(proto_config, &json_config, json_options); - // This should always succeed unless something crash-worthy such as out-of-memory. - RELEASE_ASSERT(status.ok()); - UNREFERENCED_PARAMETER(status); - const Json::ObjectSharedPtr filter_config = Json::Factory::loadFromString(json_config); + const Json::ObjectSharedPtr filter_config = WktUtil::getJsonObjectFromStruct(proto_config); // Map filter type string to enum. Configuration::NetworkFilterType type; @@ -42,7 +34,7 @@ ProdListenerComponentFactory::createFilterFactoryList_( } else if (string_type == "write") { type = Configuration::NetworkFilterType::Write; } else { - ASSERT(string_type == "both"); + ASSERT(string_type == "both" || string_type.empty()); type = Configuration::NetworkFilterType::Both; } @@ -51,8 +43,18 @@ ProdListenerComponentFactory::createFilterFactoryList_( Registry::FactoryRegistry::getFactory( string_name); if (factory != nullptr) { - Configuration::NetworkFilterFactoryCb callback = - factory->createFilterFactory(*filter_config, context); + Configuration::NetworkFilterFactoryCb callback; + if (filter_config->getBoolean("deprecatedV1", false)) { + callback = factory->createFilterFactory(*filter_config->getObject("value", true), context); + } else { + auto message = factory->createEmptyConfigProto(); + if (!message) { + throw EnvoyException( + fmt::format("Filter factory for '{}' has unexpected proto config", string_name)); + } + MessageUtil::loadFromJson(filter_config->asJsonString(), *message); + callback = factory->createFilterFactoryFromProto(*message, context); + } ret.push_back(callback); } else { // DEPRECATED @@ -60,8 +62,8 @@ ProdListenerComponentFactory::createFilterFactoryList_( bool found_filter = false; for (Configuration::NetworkFilterConfigFactory* config_factory : Configuration::MainImpl::filterConfigFactories()) { - Configuration::NetworkFilterFactoryCb callback = - config_factory->tryCreateFilterFactory(type, string_name, *filter_config, server); + Configuration::NetworkFilterFactoryCb callback = config_factory->tryCreateFilterFactory( + type, string_name, *filter_config->getObject("value", true), server); if (callback) { ret.push_back(callback); found_filter = true; @@ -412,6 +414,9 @@ void ListenerManagerImpl::addListenerToWorker(Worker& worker, ListenerImpl& list stats_.listener_create_failure_.inc(); removeListener(listener.name()); } + if (success) { + stats_.listener_create_success_.inc(); + } }); }); } diff --git a/source/server/listener_manager_impl.h b/source/server/listener_manager_impl.h index c70fc252892f..bb675380d947 100644 --- a/source/server/listener_manager_impl.h +++ b/source/server/listener_manager_impl.h @@ -58,6 +58,7 @@ typedef std::unique_ptr ListenerImplPtr; COUNTER(listener_added) \ COUNTER(listener_modified) \ COUNTER(listener_removed) \ + COUNTER(listener_create_success) \ COUNTER(listener_create_failure) \ GAUGE (total_listeners_warming) \ GAUGE (total_listeners_active) \ diff --git a/test/config/integration/BUILD b/test/config/integration/BUILD index 3d771fccbb17..51bdc3b92d71 100644 --- a/test/config/integration/BUILD +++ b/test/config/integration/BUILD @@ -28,6 +28,8 @@ filegroup( "server_xds.cds.yaml", "server_xds.eds.yaml", "server_xds.json", + "server_xds.lds.yaml", + "server_xds.rds.yaml", ], ) diff --git a/test/config/integration/server_xds.bootstrap.yaml b/test/config/integration/server_xds.bootstrap.yaml index a9ce362693b6..3f48297241aa 100644 --- a/test/config/integration/server_xds.bootstrap.yaml +++ b/test/config/integration/server_xds.bootstrap.yaml @@ -1,2 +1,4 @@ +lds_config: + path: {{ lds_json_path }} cds_config: path: {{ cds_json_path }} diff --git a/test/config/integration/server_xds.json b/test/config/integration/server_xds.json index f2f9c045e375..111bc3b9d12d 100644 --- a/test/config/integration/server_xds.json +++ b/test/config/integration/server_xds.json @@ -1,39 +1,6 @@ { - "listeners": [ - { - "address": "tcp://{{ ip_loopback_address }}:0", - "filters": [ - { - "type": "read", - "name": "http_connection_manager", - "config": { - "codec_type": "http2", - "drain_timeout_ms": 5000, - "stat_prefix": "router", - "route_config": - { - "virtual_hosts": [ - { - "name": "integration", - "domains": [ "*" ], - "routes": [ - { - "prefix": "/test/long/url", - "cluster": "cluster_1" - } - ] - } - ] - }, - "filters": [ - { "type": "decoder", "name": "router", "config": {}} - ] - } - }] - }], - + "listeners": [], "admin": { "access_log_path": "/dev/null", "address": "tcp://{{ ip_loopback_address }}:0" }, - "cluster_manager": { "clusters": [] } diff --git a/test/config/integration/server_xds.lds.yaml b/test/config/integration/server_xds.lds.yaml new file mode 100644 index 000000000000..48c836b5a805 --- /dev/null +++ b/test/config/integration/server_xds.lds.yaml @@ -0,0 +1,19 @@ +versionInfo: "0" +resources: +- "@type": type.googleapis.com/envoy.api.v2.Listener + name: listener_0 + address: + socketAddress: + address: {{ ntop_ip_loopback_address }} + portValue: 0 + filterChains: + - filters: + - name: http_connection_manager + config: + codecType: HTTP2 + drainTimeout: 5s + statPrefix: router + rds: + routeConfigName: route_config_0 + configSource: { path: {{ rds_json_path }} } + httpFilters: [{ name: router, config: { deprecatedV1: true }}] diff --git a/test/config/integration/server_xds.rds.yaml b/test/config/integration/server_xds.rds.yaml new file mode 100644 index 000000000000..252b2b46395b --- /dev/null +++ b/test/config/integration/server_xds.rds.yaml @@ -0,0 +1,10 @@ +versionInfo: "0" +resources: +- "@type": type.googleapis.com/envoy.api.v2.RouteConfiguration + name: route_config_0 + virtual_hosts: + - name: integration + domains: [ "*" ] + routes: + - match: { prefix: "/test/long/url" } + route: { cluster: cluster_1 } diff --git a/test/integration/integration.cc b/test/integration/integration.cc index beb955acccb5..1c4a7ad6bc46 100644 --- a/test/integration/integration.cc +++ b/test/integration/integration.cc @@ -332,12 +332,19 @@ void BaseIntegrationTest::createApiTestServer(const std::string& json_path, api_filesystem_config.eds_path_, port_map_, version_); const std::string cds_path = TestEnvironment::temporaryFileSubstitute( api_filesystem_config.cds_path_, {{"eds_json_path", eds_path}}, port_map_, version_); + const std::string rds_path = TestEnvironment::temporaryFileSubstitute( + api_filesystem_config.rds_path_, port_map_, version_); + const std::string lds_path = TestEnvironment::temporaryFileSubstitute( + api_filesystem_config.lds_path_, {{"rds_json_path", rds_path}}, port_map_, version_); test_server_ = IntegrationTestServer::create( TestEnvironment::temporaryFileSubstitute(json_path, port_map_, version_), - TestEnvironment::temporaryFileSubstitute(api_filesystem_config.bootstrap_path_, - {{"cds_json_path", cds_path}}, port_map_, - version_), + TestEnvironment::temporaryFileSubstitute( + api_filesystem_config.bootstrap_path_, + {{"cds_json_path", cds_path}, {"lds_json_path", lds_path}}, port_map_, version_), version_); + // Need to ensure we have an LDS update before invoking registerTestServerPorts() below that + // needs to know about the bound listener ports. + test_server_->waitForCounterGe("listener_manager.listener_create_success", 1); } registerTestServerPorts(port_names); } diff --git a/test/integration/integration.h b/test/integration/integration.h index 2ac76ab7d983..e7c74a360fb6 100644 --- a/test/integration/integration.h +++ b/test/integration/integration.h @@ -155,6 +155,8 @@ struct ApiFilesystemConfig { std::string bootstrap_path_; std::string cds_path_; std::string eds_path_; + std::string lds_path_; + std::string rds_path_; }; /** diff --git a/test/integration/ratelimit_integration_test.cc b/test/integration/ratelimit_integration_test.cc index f870fae6fe6a..03950d13f934 100644 --- a/test/integration/ratelimit_integration_test.cc +++ b/test/integration/ratelimit_integration_test.cc @@ -165,14 +165,7 @@ TEST_P(RatelimitIntegrationTest, Error) { TEST_P(RatelimitIntegrationTest, Timeout) { initiateClientConnection(); waitForRatelimitRequest(); - // Keep polling stats until the HTTP ratelimit wait times out. - const uint32_t sleep_ms = 100; - for (int32_t timeout_wait_ms = 50000; timeout_wait_ms > 0; timeout_wait_ms -= sleep_ms) { - if (test_server_->counter("cluster.ratelimit.upstream_rq_timeout")->value() > 0) { - break; - } - std::this_thread::sleep_for(std::chrono::milliseconds(sleep_ms)); - } + test_server_->waitForCounterGe("cluster.ratelimit.upstream_rq_timeout", 1); // Rate limiter fails open waitForSuccessfulUpstreamResponse(); cleanup(); diff --git a/test/integration/server.h b/test/integration/server.h index a3b3bbef2105..5f063443f436 100644 --- a/test/integration/server.h +++ b/test/integration/server.h @@ -207,12 +207,31 @@ class IntegrationTestServer : Logger::Loggable, } void start(const Network::Address::IpVersion version); void start(); + + void waitForCounterGe(const std::string& name, uint64_t value) { + while (counter(name)->value() < value) { + std::this_thread::sleep_for(std::chrono::milliseconds(10)); + } + } + + void waitForGaugeGe(const std::string& name, uint64_t value) { + while (gauge(name)->value() < value) { + std::this_thread::sleep_for(std::chrono::milliseconds(10)); + } + } + Stats::CounterSharedPtr counter(const std::string& name) { // When using the thread local store, only counters() is thread safe. This also allows us // to test if a counter exists at all versus just defaulting to zero. return TestUtility::findCounter(*stat_store_, name); } + Stats::GaugeSharedPtr gauge(const std::string& name) { + // When using the thread local store, only gauges() is thread safe. This also allows us + // to test if a counter exists at all versus just defaulting to zero. + return TestUtility::findGauge(*stat_store_, name); + } + // TestHooks void onWorkerListenerAdded() override; void onWorkerListenerRemoved() override; diff --git a/test/integration/xds_integration_test.cc b/test/integration/xds_integration_test.cc index fa67f882640f..0fbb05bd9d9e 100644 --- a/test/integration/xds_integration_test.cc +++ b/test/integration/xds_integration_test.cc @@ -20,6 +20,8 @@ class XdsIntegrationTest : public BaseIntegrationTest, .bootstrap_path_ = "test/config/integration/server_xds.bootstrap.yaml", .cds_path_ = "test/config/integration/server_xds.cds.yaml", .eds_path_ = "test/config/integration/server_xds.eds.yaml", + .lds_path_ = "test/config/integration/server_xds.lds.yaml", + .rds_path_ = "test/config/integration/server_xds.rds.yaml", }, {"http"}); }