From 9a6d7181ad886d9ad00a769c7d667d07ca2dc5dc Mon Sep 17 00:00:00 2001 From: Jakub Sobon Date: Wed, 18 Aug 2021 16:06:39 -0400 Subject: [PATCH] Update Envoy to 95038fe (Aug 12th 2021). (#734) - refactoring `ProcessImpl`, creating a named constructor that allows us to create bootstrap initially and pass it to `Envoy::Api::Impl()` as required since https://github.com/envoyproxy/envoy/pull/17562. Specifically: - Moving method `ProcessImpl::determineConcurrency()` out of the ProcessImpl class so that it can be used during its construction. - Moving code that extracts URIs from `process_impl.cc` into `process_bootstrap.cc`. - Adding a previously missing test case `CreatesBootstrapForH1RespectingPortInUri` into `process_bootstrap_test.cc`. - Removing a TODO that incorrectly indicated URI DNS resolution is optional. Envoy [requires](https://github.com/envoyproxy/envoy/blob/716ee8abc526d51f07ed6d3c2a5aa8a3b2481d9d/api/envoy/config/core/v3/address.proto#L64-L67) resolved IPs in the Bootstrap for cluster of type STATIC. - Creating a named constructor for `ProcessImpl` that creates the `Envoy::Api::Api` with an empty Bootstrap that is then replaced with the one generated. See an inline comment for explanation. - Moving callers onto the named constructor and making the original constructor of `ProcessImpl` private. - no changes to `.bazelrc`, `.bazelversion`, `run_envoy_docker.sh`. Signed-off-by: Jakub Sobon --- bazel/repositories.bzl | 4 +- source/client/client.cc | 10 +- source/client/process_bootstrap.cc | 51 +++++++- source/client/process_bootstrap.h | 14 +-- source/client/process_impl.cc | 158 +++++++++++++------------ source/client/process_impl.h | 26 +++-- source/client/service_impl.cc | 16 ++- test/process_bootstrap_test.cc | 179 ++++++++++++++++++----------- test/process_test.cc | 77 ++++++++----- test/service_test.cc | 2 +- 10 files changed, 333 insertions(+), 204 deletions(-) diff --git a/bazel/repositories.bzl b/bazel/repositories.bzl index 8dc990913..bcfb136ac 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 = "e85a7f408c7baee8e1ed4af39a647c98ee5f2215" # Aug 10, 2021 -ENVOY_SHA = "1745bf00a464327a0993b5229cae5e30423854a77b9b1d0c4ee96306e9aedc64" +ENVOY_COMMIT = "95038feabf260c3937465951d5da603d31ea3bd4" # Aug 12, 2021 +ENVOY_SHA = "4a584b02c24ac24362eff2550977616f86a194e61106e15f723e1cf961ca145d" HDR_HISTOGRAM_C_VERSION = "0.11.2" # October 12th, 2020 HDR_HISTOGRAM_C_SHA = "637f28b5f64de2e268131e4e34e6eef0b91cf5ff99167db447d9b2825eae6bad" diff --git a/source/client/client.cc b/source/client/client.cc index 6b95bb6ec..b6c4847ca 100644 --- a/source/client/client.cc +++ b/source/client/client.cc @@ -72,7 +72,13 @@ bool Main::run() { stub = std::make_unique(channel); process = std::make_unique(*options_, *stub); } else { - process = std::make_unique(*options_, time_system); + absl::StatusOr process_or_status = + ProcessImpl::CreateProcessImpl(*options_, time_system); + if (!process_or_status.ok()) { + ENVOY_LOG(error, "Unable to create ProcessImpl: {}", process_or_status.status().ToString()); + return false; + } + process = std::move(*process_or_status); } OutputFormatterFactoryImpl output_formatter_factory; OutputCollectorImpl output_collector(time_system, *options_); @@ -100,4 +106,4 @@ bool Main::run() { } } // namespace Client -} // namespace Nighthawk \ No newline at end of file +} // namespace Nighthawk diff --git a/source/client/process_bootstrap.cc b/source/client/process_bootstrap.cc index 9e3c9ded8..7ef789eff 100644 --- a/source/client/process_bootstrap.cc +++ b/source/client/process_bootstrap.cc @@ -12,6 +12,8 @@ #include "external/envoy_api/envoy/extensions/upstreams/http/v3/http_protocol_options.pb.h" #include "source/client/sni_utility.h" +#include "source/common/uri_impl.h" +#include "source/common/utility.h" namespace Nighthawk { namespace { @@ -175,15 +177,52 @@ Cluster createNighthawkClusterForWorker(const Client::Options& options, return cluster; } +// Extracts URIs of the targets and the request source (if specified) from the +// Nighthawk options. +// Resolves all the extracted URIs. +absl::Status extractAndResolveUrisFromOptions(Envoy::Event::Dispatcher& dispatcher, + const Client::Options& options, + std::vector* uris, + UriPtr* request_source_uri) { + try { + if (options.uri().has_value()) { + uris->push_back(std::make_unique(options.uri().value())); + } else { + for (const nighthawk::client::MultiTarget::Endpoint& endpoint : + options.multiTargetEndpoints()) { + uris->push_back(std::make_unique(fmt::format( + "{}://{}:{}{}", options.multiTargetUseHttps() ? "https" : "http", + endpoint.address().value(), endpoint.port().value(), options.multiTargetPath()))); + } + } + for (const UriPtr& uri : *uris) { + uri->resolve(dispatcher, Utility::translateFamilyOptionString(options.addressFamily())); + } + if (options.requestSource() != "") { + *request_source_uri = std::make_unique(options.requestSource()); + (*request_source_uri) + ->resolve(dispatcher, Utility::translateFamilyOptionString(options.addressFamily())); + } + } catch (const UriException& ex) { + return absl::InvalidArgumentError( + fmt::format("URI exception (for example, malformed URI syntax, bad MultiTarget path, " + "unresolvable host DNS): {}", + ex.what())); + } + return absl::OkStatus(); +} + } // namespace -absl::StatusOr createBootstrapConfiguration(const Client::Options& options, - const std::vector& uris, - const UriPtr& request_source_uri, +absl::StatusOr createBootstrapConfiguration(Envoy::Event::Dispatcher& dispatcher, + const Client::Options& options, int number_of_workers) { - if (uris.empty()) { - return absl::InvalidArgumentError( - "illegal configuration with zero endpoints, at least one uri must be specified"); + std::vector uris; + UriPtr request_source_uri; + absl::Status uri_status = + extractAndResolveUrisFromOptions(dispatcher, options, &uris, &request_source_uri); + if (!uri_status.ok()) { + return uri_status; } Bootstrap bootstrap; diff --git a/source/client/process_bootstrap.h b/source/client/process_bootstrap.h index aef2402d6..6a7554371 100644 --- a/source/client/process_bootstrap.h +++ b/source/client/process_bootstrap.h @@ -4,6 +4,7 @@ #include "nighthawk/common/uri.h" #include "external/envoy/source/common/common/statusor.h" +#include "external/envoy/source/common/event/dispatcher_impl.h" #include "external/envoy_api/envoy/config/bootstrap/v3/bootstrap.pb.h" namespace Nighthawk { @@ -14,21 +15,16 @@ namespace Nighthawk { * The created bootstrap configuration can be used to upstream requests to the * specified uris. * + * @param dispatcher is used when resolving hostnames to IP addresses in the + bootstrap. * @param options are the options this Nighthawk execution was triggered with. - * @param uris are the endpoints to which the requests will be upstreamed. At - * least one uri must be specified. It is assumed that all the uris have - * the same scheme (e.g. https). All the uri objects must already be - * resolved. - * @param request_source_uri is the address of the request source service to - * use, can be NULL if request source isn't used. If not NULL, the uri - * object must already be resolved. * @param number_of_workers indicates how many Nighthawk workers will be * upstreaming requests. A separate cluster is generated for each worker. * * @return the created bootstrap configuration. */ absl::StatusOr -createBootstrapConfiguration(const Client::Options& options, const std::vector& uris, - const UriPtr& request_source_uri, int number_of_workers); +createBootstrapConfiguration(Envoy::Event::Dispatcher& dispatcher, const Client::Options& options, + int number_of_workers); } // namespace Nighthawk diff --git a/source/client/process_impl.cc b/source/client/process_impl.cc index a8d08d3a5..b75f0df6e 100644 --- a/source/client/process_impl.cc +++ b/source/client/process_impl.cc @@ -62,6 +62,44 @@ using namespace std::chrono_literals; namespace Nighthawk { namespace Client { +namespace { + +using ::envoy::config::bootstrap::v3::Bootstrap; + +// Helps in generating a bootstrap for the process. +// This is a class only to allow the use of the ENVOY_LOG macros. +class BootstrapFactory : public Envoy::Logger::Loggable { +public: + // Determines the concurrency Nighthawk should use based on configuration + // (options) and the available machine resources. + static uint32_t determineConcurrency(const Options& options) { + uint32_t cpu_cores_with_affinity = Envoy::OptionsImplPlatform::getCpuCount(); + bool autoscale = options.concurrency() == "auto"; + // TODO(oschaaf): Maybe, in the case where the concurrency flag is left out, but + // affinity is set / we don't have affinity with all cores, we should default to autoscale. + // (e.g. we are called via taskset). + uint32_t concurrency = autoscale ? cpu_cores_with_affinity : std::stoi(options.concurrency()); + + if (autoscale) { + ENVOY_LOG(info, "Detected {} (v)CPUs with affinity..", cpu_cores_with_affinity); + } + std::string duration_as_string = + options.noDuration() ? "No time limit" + : fmt::format("Time limit: {} seconds", options.duration().count()); + ENVOY_LOG(info, "Starting {} threads / event loops. {}.", concurrency, duration_as_string); + ENVOY_LOG(info, "Global targets: {} connections and {} calls per second.", + options.connections() * concurrency, options.requestsPerSecond() * concurrency); + + if (concurrency > 1) { + ENVOY_LOG(info, " (Per-worker targets: {} connections and {} calls per second)", + options.connections(), options.requestsPerSecond()); + } + + return concurrency; + } +}; + +} // namespace // We customize ProdClusterManagerFactory for the sole purpose of returning our specialized // http1 pool to the benchmark client, which allows us to offer connection prefetching. @@ -123,17 +161,17 @@ class ClusterManagerFactory : public Envoy::Upstream::ProdClusterManagerFactory ProcessImpl::ProcessImpl(const Options& options, Envoy::Event::TimeSystem& time_system, const std::shared_ptr& process_wide) - : process_wide_(process_wide == nullptr ? std::make_shared() + : options_(options), number_of_workers_(BootstrapFactory::determineConcurrency(options_)), + process_wide_(process_wide == nullptr ? std::make_shared() : process_wide), time_system_(time_system), stats_allocator_(symbol_table_), store_root_(stats_allocator_), quic_stat_names_(store_root_.symbolTable()), api_(std::make_unique(platform_impl_.threadFactory(), store_root_, - time_system_, platform_impl_.fileSystem(), - generator_)), + time_system_, platform_impl_.fileSystem(), generator_, + bootstrap_)), dispatcher_(api_->allocateDispatcher("main_thread")), benchmark_client_factory_(options), termination_predicate_factory_(options), sequencer_factory_(options), - request_generator_factory_(options, *api_), options_(options), - init_manager_("nh_init_manager"), + request_generator_factory_(options, *api_), init_manager_("nh_init_manager"), local_info_(new Envoy::LocalInfo::LocalInfoImpl( store_root_.symbolTable(), node_, node_context_params_, Envoy::Network::Utility::getLocalAddress(Envoy::Network::Address::IpVersion::v4), @@ -153,6 +191,35 @@ ProcessImpl::ProcessImpl(const Options& options, Envoy::Event::TimeSystem& time_ configureComponentLogLevels(spdlog::level::from_str(lower)); } +absl::StatusOr +ProcessImpl::CreateProcessImpl(const Options& options, Envoy::Event::TimeSystem& time_system, + const std::shared_ptr& process_wide) { + std::unique_ptr process(new ProcessImpl(options, time_system, process_wide)); + + absl::StatusOr bootstrap = createBootstrapConfiguration( + *process->dispatcher_, process->options_, process->number_of_workers_); + if (!bootstrap.ok()) { + ENVOY_LOG(error, "Failed to create bootstrap configuration: {}", bootstrap.status().message()); + process->shutdown(); + return bootstrap.status(); + } + + // Ideally we would create the bootstrap first and then pass it to the + // constructor of Envoy::Api::Api. That cannot be done because of a circular + // dependency: + // 1) The constructor of Envoy::Api::Api requires an instance of Bootstrap. + // 2) The bootstrap generator requires an Envoy::Event::Dispatcher to resolve + // URIs to IPs required in the Bootstrap. + // 3) The constructor of Envoy::Event::Dispatcher requires Envoy::Api::Api. + // + // Replacing the bootstrap_ after the Envoy::Api::Api has been created is + // assumed to be safe, because we still do it while constructing the + // ProcessImpl, i.e. before we start running the process. + process->bootstrap_ = *bootstrap; + + return process; +} + ProcessImpl::~ProcessImpl() { RELEASE_ASSERT(shutdown_, "shutdown not called before destruction."); } @@ -241,32 +308,6 @@ void ProcessImpl::configureComponentLogLevels(spdlog::level::level_enum level) { logger_to_change->setLevel(level); } -uint32_t ProcessImpl::determineConcurrency() const { - uint32_t cpu_cores_with_affinity = Envoy::OptionsImplPlatform::getCpuCount(); - bool autoscale = options_.concurrency() == "auto"; - // TODO(oschaaf): Maybe, in the case where the concurrency flag is left out, but - // affinity is set / we don't have affinity with all cores, we should default to autoscale. - // (e.g. we are called via taskset). - uint32_t concurrency = autoscale ? cpu_cores_with_affinity : std::stoi(options_.concurrency()); - - if (autoscale) { - ENVOY_LOG(info, "Detected {} (v)CPUs with affinity..", cpu_cores_with_affinity); - } - std::string duration_as_string = - options_.noDuration() ? "No time limit" - : fmt::format("Time limit: {} seconds", options_.duration().count()); - ENVOY_LOG(info, "Starting {} threads / event loops. {}.", concurrency, duration_as_string); - ENVOY_LOG(info, "Global targets: {} connections and {} calls per second.", - options_.connections() * concurrency, options_.requestsPerSecond() * concurrency); - - if (concurrency > 1) { - ENVOY_LOG(info, " (Per-worker targets: {} connections and {} calls per second)", - options_.connections(), options_.requestsPerSecond()); - } - - return concurrency; -} - std::vector ProcessImpl::vectorizeStatisticPtrMap(const StatisticPtrMap& statistics) const { std::vector v; @@ -386,8 +427,7 @@ void ProcessImpl::setupStatsSinks(const envoy::config::bootstrap::v3::Bootstrap& } } -bool ProcessImpl::runInternal(OutputCollector& collector, const std::vector& uris, - const UriPtr& request_source_uri, const UriPtr& tracing_uri, +bool ProcessImpl::runInternal(OutputCollector& collector, const UriPtr& tracing_uri, const absl::optional& scheduled_start) { const Envoy::SystemTime now = time_system_.systemTime(); if (scheduled_start.value_or(now) < now) { @@ -399,24 +439,15 @@ bool ProcessImpl::runInternal(OutputCollector& collector, const std::vector bootstrap = - createBootstrapConfiguration(options_, uris, request_source_uri, number_of_workers); - if (!bootstrap.ok()) { - ENVOY_LOG(error, "Failed to create bootstrap configuration: {}", - bootstrap.status().message()); - return false; - } - // Needs to happen as early as possible (before createWorkers()) in the instantiation to preempt // the objects that require stats. if (!options_.statsSinks().empty()) { - store_root_.setTagProducer(Envoy::Config::Utility::createTagProducer(*bootstrap)); + store_root_.setTagProducer(Envoy::Config::Utility::createTagProducer(bootstrap_)); } - createWorkers(number_of_workers, scheduled_start); + createWorkers(number_of_workers_, scheduled_start); tls_.registerThread(*dispatcher_, true); store_root_.initializeThreading(*dispatcher_, tls_); runtime_singleton_ = std::make_unique( @@ -446,21 +477,21 @@ bool ProcessImpl::runInternal(OutputCollector& collector, const std::vectorsetPrefetchConnections(options_.prefetchConnections()); if (tracing_uri != nullptr) { - setupTracingImplementation(*bootstrap, *tracing_uri); - addTracingCluster(*bootstrap, *tracing_uri); + setupTracingImplementation(bootstrap_, *tracing_uri); + addTracingCluster(bootstrap_, *tracing_uri); } - ENVOY_LOG(debug, "Computed configuration: {}", bootstrap->DebugString()); - cluster_manager_ = cluster_manager_factory_->clusterManagerFromProto(*bootstrap); - maybeCreateTracingDriver(bootstrap->tracing()); + ENVOY_LOG(debug, "Computed configuration: {}", bootstrap_.DebugString()); + cluster_manager_ = cluster_manager_factory_->clusterManagerFromProto(bootstrap_); + maybeCreateTracingDriver(bootstrap_.tracing()); cluster_manager_->setInitializedCb( [this]() -> void { init_manager_.initialize(init_watcher_); }); Envoy::Runtime::LoaderSingleton::get().initialize(*cluster_manager_); std::list> stats_sinks; - setupStatsSinks(*bootstrap, stats_sinks); + setupStatsSinks(bootstrap_, stats_sinks); std::chrono::milliseconds stats_flush_interval = std::chrono::milliseconds( - Envoy::DurationUtil::durationToMilliseconds(bootstrap->stats_flush_interval())); + Envoy::DurationUtil::durationToMilliseconds(bootstrap_.stats_flush_interval())); if (!options_.statsSinks().empty()) { // There should be only a single live flush worker instance at any time. @@ -541,31 +572,9 @@ bool ProcessImpl::runInternal(OutputCollector& collector, const std::vector uris; - UriPtr request_source_uri; UriPtr tracing_uri; try { - // TODO(oschaaf): See if we can rid of resolving here. - // We now only do it to validate. - if (options_.uri().has_value()) { - uris.push_back(std::make_unique(options_.uri().value())); - } else { - for (const nighthawk::client::MultiTarget::Endpoint& endpoint : - options_.multiTargetEndpoints()) { - uris.push_back(std::make_unique(fmt::format( - "{}://{}:{}{}", options_.multiTargetUseHttps() ? "https" : "http", - endpoint.address().value(), endpoint.port().value(), options_.multiTargetPath()))); - } - } - for (const UriPtr& uri : uris) { - uri->resolve(*dispatcher_, Utility::translateFamilyOptionString(options_.addressFamily())); - } - if (options_.requestSource() != "") { - request_source_uri = std::make_unique(options_.requestSource()); - request_source_uri->resolve(*dispatcher_, - Utility::translateFamilyOptionString(options_.addressFamily())); - } if (options_.trace() != "") { tracing_uri = std::make_unique(options_.trace()); tracing_uri->resolve(*dispatcher_, @@ -580,8 +589,7 @@ bool ProcessImpl::run(OutputCollector& collector) { } try { - return runInternal(collector, uris, request_source_uri, tracing_uri, - options_.scheduled_start()); + return runInternal(collector, tracing_uri, options_.scheduled_start()); } catch (Envoy::EnvoyException& ex) { ENVOY_LOG(error, "Fatal exception: {}", ex.what()); throw; diff --git a/source/client/process_impl.h b/source/client/process_impl.h index b35719175..7ae191d89 100644 --- a/source/client/process_impl.h +++ b/source/client/process_impl.h @@ -17,6 +17,7 @@ #include "external/envoy/source/common/access_log/access_log_manager_impl.h" #include "external/envoy/source/common/common/logger.h" #include "external/envoy/source/common/common/random_generator.h" +#include "external/envoy/source/common/common/statusor.h" #include "external/envoy/source/common/event/real_time_system.h" #include "external/envoy/source/common/grpc/context_impl.h" #include "external/envoy/source/common/http/context_impl.h" @@ -52,7 +53,7 @@ class ClusterManagerFactory; class ProcessImpl : public Process, public Envoy::Logger::Loggable { public: /** - * Instantiates a ProcessImpl + * Creates a ProcessImpl. * @param options provides the options configuration to be used. * @param time_system provides the Envoy::Event::TimeSystem implementation that will be used. * @param process_wide optional parameter which can be used to pass a pre-setup reference to @@ -61,15 +62,11 @@ class ProcessImpl : public Process, public Envoy::Logger::Loggable& process_wide = nullptr); - ~ProcessImpl() override; + static absl::StatusOr + CreateProcessImpl(const Options& options, Envoy::Event::TimeSystem& time_system, + const std::shared_ptr& process_wide = nullptr); - /** - * @return uint32_t the concurrency we determined should run at based on configuration and - * available machine resources. - */ - uint32_t determineConcurrency() const; + ~ProcessImpl() override; /** * Runs the process. @@ -88,6 +85,10 @@ class ProcessImpl : public Process, public Envoy::Logger::Loggable& process_wide = nullptr); + void addTracingCluster(envoy::config::bootstrap::v3::Bootstrap& bootstrap, const Uri& uri) const; void setupTracingImplementation(envoy::config::bootstrap::v3::Bootstrap& bootstrap, const Uri& uri) const; @@ -113,8 +114,7 @@ class ProcessImpl : public Process, public Envoy::Logger::Loggable>& stats_sinks); - bool runInternal(OutputCollector& collector, const std::vector& uris, - const UriPtr& request_source_uri, const UriPtr& tracing_uri, + bool runInternal(OutputCollector& collector, const UriPtr& tracing_uri, const absl::optional& schedule); /** @@ -151,6 +151,8 @@ class ProcessImpl : public Process, public Envoy::Logger::Loggable node_context_params_; + const Options& options_; + const int number_of_workers_; std::shared_ptr process_wide_; Envoy::PlatformImpl platform_impl_; Envoy::Event::TimeSystem& time_system_; @@ -159,6 +161,7 @@ class ProcessImpl : public Process, public Envoy::Logger::Loggable workers_; @@ -166,7 +169,6 @@ class ProcessImpl : public Process, public Envoy::Logger::Loggable process_or_status = + ProcessImpl::CreateProcessImpl(*options, time_system_, process_wide_); + if (!process_or_status.ok()) { + response.mutable_error_detail()->set_code(grpc::StatusCode::INTERNAL); + response.mutable_error_detail()->set_message( + fmt::format("Unable to create ProcessImpl: {}", process_or_status.status().ToString())); + writeResponse(response); + return; + } + ProcessPtr process = std::move(*process_or_status); + OutputCollectorImpl output_collector(time_system_, *options); - const bool ok = process.run(output_collector); + const bool ok = process->run(output_collector); if (!ok) { response.mutable_error_detail()->set_code(grpc::StatusCode::INTERNAL); // TODO(https://github.com/envoyproxy/nighthawk/issues/181): wire through error descriptions, so @@ -55,7 +65,7 @@ void ServiceImpl::handleExecutionRequest(const nighthawk::client::ExecutionReque "benchmark request."); } *(response.mutable_output()) = output_collector.toProto(); - process.shutdown(); + process->shutdown(); // We release before writing the response to avoid a race with the client's follow up request // coming in before we release the lock, which would lead up to us declining service when // we should not. diff --git a/test/process_bootstrap_test.cc b/test/process_bootstrap_test.cc index 7564210bc..9dd071655 100644 --- a/test/process_bootstrap_test.cc +++ b/test/process_bootstrap_test.cc @@ -48,8 +48,8 @@ class CreateBootstrapConfigurationTest : public testing::Test { protected: CreateBootstrapConfigurationTest() = default; - // Resolves all the uris_, so they can be passed to createBootstrapConfiguration(). - void resolveAllUris() { + // Sets mock expectations when the code under test resolves the URIs provided in the options. + void setupUriResolutionExpectations() { ON_CALL(mock_dispatcher_, createDnsResolver(_, _)).WillByDefault(Return(mock_resolver_)); EXPECT_CALL(*mock_resolver_, resolve(_, _, _)) @@ -62,39 +62,96 @@ class CreateBootstrapConfigurationTest : public testing::Test { Envoy::TestUtility::makeDnsResponse({"127.0.0.1"})); return nullptr; })); - - for (const UriPtr& uri : uris_) { - uri->resolve(mock_dispatcher_, Envoy::Network::DnsLookupFamily::Auto); - } - - if (request_source_uri_ != nullptr) { - request_source_uri_->resolve(mock_dispatcher_, Envoy::Network::DnsLookupFamily::Auto); - } } std::shared_ptr mock_resolver_{ std::make_shared()}; NiceMock mock_dispatcher_; - std::vector uris_; - UriPtr request_source_uri_; int number_of_workers_{1}; }; -TEST_F(CreateBootstrapConfigurationTest, FailsWithoutUris) { +TEST_F(CreateBootstrapConfigurationTest, CreatesBootstrapForH1) { + setupUriResolutionExpectations(); + std::unique_ptr options = - Client::TestUtility::createOptionsImpl("nighthawk_client https://www.example.org"); + Client::TestUtility::createOptionsImpl("nighthawk_client http://www.example.org"); + + absl::StatusOr expected_bootstrap = parseBootstrapFromText(R"pb( + static_resources { + clusters { + name: "0" + type: STATIC + connect_timeout { + seconds: 30 + } + circuit_breakers { + thresholds { + max_connections { + value: 100 + } + max_pending_requests { + value: 1 + } + max_requests { + value: 100 + } + max_retries { + } + } + } + load_assignment { + cluster_name: "0" + endpoints { + lb_endpoints { + endpoint { + address { + socket_address { + address: "127.0.0.1" + port_value: 80 + } + } + } + } + } + } + typed_extension_protocol_options { + key: "envoy.extensions.upstreams.http.v3.HttpProtocolOptions" + value { + [type.googleapis.com/envoy.extensions.upstreams.http.v3.HttpProtocolOptions] { + common_http_protocol_options { + max_requests_per_connection { + value: 4294937295 + } + } + explicit_http_config { + http_protocol_options { + } + } + } + } + } + } + } + stats_flush_interval { + seconds: 5 + } + )pb"); + ASSERT_THAT(expected_bootstrap, StatusIs(absl::StatusCode::kOk)); absl::StatusOr bootstrap = - createBootstrapConfiguration(*options, uris_, request_source_uri_, number_of_workers_); - ASSERT_THAT(bootstrap, StatusIs(absl::StatusCode::kInvalidArgument)); + createBootstrapConfiguration(mock_dispatcher_, *options, number_of_workers_); + ASSERT_THAT(bootstrap, StatusIs(absl::StatusCode::kOk)); + EXPECT_THAT(*bootstrap, EqualsProto(*expected_bootstrap)); + + // Ensure the generated bootstrap is valid. + Envoy::MessageUtil::validate(*bootstrap, Envoy::ProtobufMessage::getStrictValidationVisitor()); } -TEST_F(CreateBootstrapConfigurationTest, CreatesBootstrapForH1) { - uris_.push_back(std::make_unique("http://www.example.org")); - resolveAllUris(); +TEST_F(CreateBootstrapConfigurationTest, CreatesBootstrapForH1RespectingPortInUri) { + setupUriResolutionExpectations(); std::unique_ptr options = - Client::TestUtility::createOptionsImpl("nighthawk_client http://www.example.org"); + Client::TestUtility::createOptionsImpl("nighthawk_client http://www.example.org:81"); absl::StatusOr expected_bootstrap = parseBootstrapFromText(R"pb( static_resources { @@ -127,7 +184,7 @@ TEST_F(CreateBootstrapConfigurationTest, CreatesBootstrapForH1) { address { socket_address { address: "127.0.0.1" - port_value: 80 + port_value: 81 } } } @@ -159,7 +216,7 @@ TEST_F(CreateBootstrapConfigurationTest, CreatesBootstrapForH1) { ASSERT_THAT(expected_bootstrap, StatusIs(absl::StatusCode::kOk)); absl::StatusOr bootstrap = - createBootstrapConfiguration(*options, uris_, request_source_uri_, number_of_workers_); + createBootstrapConfiguration(mock_dispatcher_, *options, number_of_workers_); ASSERT_THAT(bootstrap, StatusIs(absl::StatusCode::kOk)); EXPECT_THAT(*bootstrap, EqualsProto(*expected_bootstrap)); @@ -167,13 +224,12 @@ TEST_F(CreateBootstrapConfigurationTest, CreatesBootstrapForH1) { Envoy::MessageUtil::validate(*bootstrap, Envoy::ProtobufMessage::getStrictValidationVisitor()); } -TEST_F(CreateBootstrapConfigurationTest, CreatesBootstrapForH1WithMultipleUris) { - uris_.push_back(std::make_unique("http://www.example.org")); - uris_.push_back(std::make_unique("http://www.example2.org")); - resolveAllUris(); +TEST_F(CreateBootstrapConfigurationTest, CreatesBootstrapForH1WithMultipleTargets) { + setupUriResolutionExpectations(); - std::unique_ptr options = - Client::TestUtility::createOptionsImpl("nighthawk_client http://www.example.org"); + std::unique_ptr options = Client::TestUtility::createOptionsImpl( + "nighthawk_client --multi-target-endpoint www.example.org:80 --multi-target-endpoint " + "www.example2.org:80 --multi-target-path /"); absl::StatusOr expected_bootstrap = parseBootstrapFromText(R"pb( static_resources { @@ -248,7 +304,7 @@ TEST_F(CreateBootstrapConfigurationTest, CreatesBootstrapForH1WithMultipleUris) ASSERT_THAT(expected_bootstrap, StatusIs(absl::StatusCode::kOk)); absl::StatusOr bootstrap = - createBootstrapConfiguration(*options, uris_, request_source_uri_, number_of_workers_); + createBootstrapConfiguration(mock_dispatcher_, *options, number_of_workers_); ASSERT_THAT(bootstrap, StatusIs(absl::StatusCode::kOk)); EXPECT_THAT(*bootstrap, EqualsProto(*expected_bootstrap)); @@ -257,8 +313,7 @@ TEST_F(CreateBootstrapConfigurationTest, CreatesBootstrapForH1WithMultipleUris) } TEST_F(CreateBootstrapConfigurationTest, CreatesBootstrapForH1WithTls) { - uris_.push_back(std::make_unique("https://www.example.org")); - resolveAllUris(); + setupUriResolutionExpectations(); std::unique_ptr options = Client::TestUtility::createOptionsImpl("nighthawk_client https://www.example.org"); @@ -337,7 +392,7 @@ TEST_F(CreateBootstrapConfigurationTest, CreatesBootstrapForH1WithTls) { ASSERT_THAT(expected_bootstrap, StatusIs(absl::StatusCode::kOk)); absl::StatusOr bootstrap = - createBootstrapConfiguration(*options, uris_, request_source_uri_, number_of_workers_); + createBootstrapConfiguration(mock_dispatcher_, *options, number_of_workers_); ASSERT_THAT(bootstrap, StatusIs(absl::StatusCode::kOk)); EXPECT_THAT(*bootstrap, EqualsProto(*expected_bootstrap)); @@ -346,8 +401,7 @@ TEST_F(CreateBootstrapConfigurationTest, CreatesBootstrapForH1WithTls) { } TEST_F(CreateBootstrapConfigurationTest, CreatesBootstrapForH1AndMultipleWorkers) { - uris_.push_back(std::make_unique("http://www.example.org")); - resolveAllUris(); + setupUriResolutionExpectations(); std::unique_ptr options = Client::TestUtility::createOptionsImpl("nighthawk_client http://www.example.org"); @@ -467,8 +521,8 @@ TEST_F(CreateBootstrapConfigurationTest, CreatesBootstrapForH1AndMultipleWorkers )pb"); ASSERT_THAT(expected_bootstrap, StatusIs(absl::StatusCode::kOk)); - absl::StatusOr bootstrap = createBootstrapConfiguration( - *options, uris_, request_source_uri_, /* number_of_workers = */ 2); + absl::StatusOr bootstrap = + createBootstrapConfiguration(mock_dispatcher_, *options, /* number_of_workers = */ 2); ASSERT_THAT(bootstrap, StatusIs(absl::StatusCode::kOk)); EXPECT_THAT(*bootstrap, EqualsProto(*expected_bootstrap)); @@ -477,8 +531,7 @@ TEST_F(CreateBootstrapConfigurationTest, CreatesBootstrapForH1AndMultipleWorkers } TEST_F(CreateBootstrapConfigurationTest, CreatesBootstrapForH2) { - uris_.push_back(std::make_unique("http://www.example.org")); - resolveAllUris(); + setupUriResolutionExpectations(); std::unique_ptr options = Client::TestUtility::createOptionsImpl("nighthawk_client --h2 http://www.example.org"); @@ -549,7 +602,7 @@ TEST_F(CreateBootstrapConfigurationTest, CreatesBootstrapForH2) { ASSERT_THAT(expected_bootstrap, StatusIs(absl::StatusCode::kOk)); absl::StatusOr bootstrap = - createBootstrapConfiguration(*options, uris_, request_source_uri_, number_of_workers_); + createBootstrapConfiguration(mock_dispatcher_, *options, number_of_workers_); ASSERT_THAT(bootstrap, StatusIs(absl::StatusCode::kOk)); EXPECT_THAT(*bootstrap, EqualsProto(*expected_bootstrap)); @@ -558,8 +611,7 @@ TEST_F(CreateBootstrapConfigurationTest, CreatesBootstrapForH2) { } TEST_F(CreateBootstrapConfigurationTest, CreatesBootstrapForH2WithTls) { - uris_.push_back(std::make_unique("https://www.example.org")); - resolveAllUris(); + setupUriResolutionExpectations(); std::unique_ptr options = Client::TestUtility::createOptionsImpl("nighthawk_client --h2 https://www.example.org"); @@ -641,7 +693,7 @@ TEST_F(CreateBootstrapConfigurationTest, CreatesBootstrapForH2WithTls) { ASSERT_THAT(expected_bootstrap, StatusIs(absl::StatusCode::kOk)); absl::StatusOr bootstrap = - createBootstrapConfiguration(*options, uris_, request_source_uri_, number_of_workers_); + createBootstrapConfiguration(mock_dispatcher_, *options, number_of_workers_); ASSERT_THAT(bootstrap, StatusIs(absl::StatusCode::kOk)); EXPECT_THAT(*bootstrap, EqualsProto(*expected_bootstrap)); @@ -650,8 +702,7 @@ TEST_F(CreateBootstrapConfigurationTest, CreatesBootstrapForH2WithTls) { } TEST_F(CreateBootstrapConfigurationTest, CreatesBootstrapForH3) { - uris_.push_back(std::make_unique("https://www.example.org")); - resolveAllUris(); + setupUriResolutionExpectations(); std::unique_ptr options = Client::TestUtility::createOptionsImpl( "nighthawk_client --protocol http3 https://www.example.org"); @@ -737,7 +788,7 @@ TEST_F(CreateBootstrapConfigurationTest, CreatesBootstrapForH3) { ASSERT_THAT(expected_bootstrap, StatusIs(absl::StatusCode::kOk)); absl::StatusOr bootstrap = - createBootstrapConfiguration(*options, uris_, request_source_uri_, number_of_workers_); + createBootstrapConfiguration(mock_dispatcher_, *options, number_of_workers_); ASSERT_THAT(bootstrap, StatusIs(absl::StatusCode::kOk)); EXPECT_THAT(*bootstrap, EqualsProto(*expected_bootstrap)); @@ -746,12 +797,10 @@ TEST_F(CreateBootstrapConfigurationTest, CreatesBootstrapForH3) { } TEST_F(CreateBootstrapConfigurationTest, CreatesBootstrapWithRequestSourceAndCustomTimeout) { - uris_.push_back(std::make_unique("http://www.example.org")); - request_source_uri_ = std::make_unique("127.0.0.1:6000"); - resolveAllUris(); + setupUriResolutionExpectations(); std::unique_ptr options = Client::TestUtility::createOptionsImpl( - "nighthawk_client --timeout 10 http://www.example.org"); + "nighthawk_client --timeout 10 http://www.example.org --request-source 127.0.0.1:6000"); absl::StatusOr expected_bootstrap = parseBootstrapFromText(R"pb( static_resources { @@ -849,7 +898,7 @@ TEST_F(CreateBootstrapConfigurationTest, CreatesBootstrapWithRequestSourceAndCus ASSERT_THAT(expected_bootstrap, StatusIs(absl::StatusCode::kOk)); absl::StatusOr bootstrap = - createBootstrapConfiguration(*options, uris_, request_source_uri_, number_of_workers_); + createBootstrapConfiguration(mock_dispatcher_, *options, number_of_workers_); ASSERT_THAT(bootstrap, StatusIs(absl::StatusCode::kOk)); EXPECT_THAT(*bootstrap, EqualsProto(*expected_bootstrap)); @@ -858,12 +907,10 @@ TEST_F(CreateBootstrapConfigurationTest, CreatesBootstrapWithRequestSourceAndCus } TEST_F(CreateBootstrapConfigurationTest, CreatesBootstrapWithRequestSourceAndMultipleWorkers) { - uris_.push_back(std::make_unique("http://www.example.org")); - request_source_uri_ = std::make_unique("127.0.0.1:6000"); - resolveAllUris(); + setupUriResolutionExpectations(); std::unique_ptr options = Client::TestUtility::createOptionsImpl( - "nighthawk_client --timeout 10 http://www.example.org"); + "nighthawk_client --timeout 10 http://www.example.org --request-source 127.0.0.1:6000"); absl::StatusOr expected_bootstrap = parseBootstrapFromText(R"pb( static_resources { @@ -1046,8 +1093,8 @@ TEST_F(CreateBootstrapConfigurationTest, CreatesBootstrapWithRequestSourceAndMul )pb"); ASSERT_THAT(expected_bootstrap, StatusIs(absl::StatusCode::kOk)); - absl::StatusOr bootstrap = createBootstrapConfiguration( - *options, uris_, request_source_uri_, /* number_of_workers = */ 2); + absl::StatusOr bootstrap = + createBootstrapConfiguration(mock_dispatcher_, *options, /* number_of_workers = */ 2); ASSERT_THAT(bootstrap, StatusIs(absl::StatusCode::kOk)); EXPECT_THAT(*bootstrap, EqualsProto(*expected_bootstrap)); @@ -1056,8 +1103,7 @@ TEST_F(CreateBootstrapConfigurationTest, CreatesBootstrapWithRequestSourceAndMul } TEST_F(CreateBootstrapConfigurationTest, CreatesBootstrapWithCustomOptions) { - uris_.push_back(std::make_unique("https://www.example.org")); - resolveAllUris(); + setupUriResolutionExpectations(); const std::string stats_sink_json = "{name:\"envoy.stat_sinks.statsd\",typed_config:{\"@type\":\"type." @@ -1161,7 +1207,7 @@ TEST_F(CreateBootstrapConfigurationTest, CreatesBootstrapWithCustomOptions) { ASSERT_THAT(expected_bootstrap, StatusIs(absl::StatusCode::kOk)); absl::StatusOr bootstrap = - createBootstrapConfiguration(*options, uris_, request_source_uri_, number_of_workers_); + createBootstrapConfiguration(mock_dispatcher_, *options, number_of_workers_); ASSERT_THAT(bootstrap, StatusIs(absl::StatusCode::kOk)); EXPECT_THAT(*bootstrap, EqualsProto(*expected_bootstrap)); @@ -1170,8 +1216,7 @@ TEST_F(CreateBootstrapConfigurationTest, CreatesBootstrapWithCustomOptions) { } TEST_F(CreateBootstrapConfigurationTest, CreatesBootstrapSetsMaxRequestToAtLeastOne) { - uris_.push_back(std::make_unique("http://www.example.org")); - resolveAllUris(); + setupUriResolutionExpectations(); // The tested behavior is that even though we set --max-pending-requests 0, // the code will configure a value of 1. @@ -1241,7 +1286,7 @@ TEST_F(CreateBootstrapConfigurationTest, CreatesBootstrapSetsMaxRequestToAtLeast ASSERT_THAT(expected_bootstrap, StatusIs(absl::StatusCode::kOk)); absl::StatusOr bootstrap = - createBootstrapConfiguration(*options, uris_, request_source_uri_, number_of_workers_); + createBootstrapConfiguration(mock_dispatcher_, *options, number_of_workers_); ASSERT_THAT(bootstrap, StatusIs(absl::StatusCode::kOk)); EXPECT_THAT(*bootstrap, EqualsProto(*expected_bootstrap)); @@ -1250,8 +1295,7 @@ TEST_F(CreateBootstrapConfigurationTest, CreatesBootstrapSetsMaxRequestToAtLeast } TEST_F(CreateBootstrapConfigurationTest, CreatesBootstrapWithCustomTransportSocket) { - uris_.push_back(std::make_unique("https://www.example.org")); - resolveAllUris(); + setupUriResolutionExpectations(); const std::string transport_socket_json = "{name:\"envoy.transport_sockets.tls\"," @@ -1341,7 +1385,7 @@ TEST_F(CreateBootstrapConfigurationTest, CreatesBootstrapWithCustomTransportSock ASSERT_THAT(expected_bootstrap, StatusIs(absl::StatusCode::kOk)); absl::StatusOr bootstrap = - createBootstrapConfiguration(*options, uris_, request_source_uri_, number_of_workers_); + createBootstrapConfiguration(mock_dispatcher_, *options, number_of_workers_); ASSERT_THAT(bootstrap, StatusIs(absl::StatusCode::kOk)); EXPECT_THAT(*bootstrap, EqualsProto(*expected_bootstrap)); @@ -1350,8 +1394,7 @@ TEST_F(CreateBootstrapConfigurationTest, CreatesBootstrapWithCustomTransportSock } TEST_F(CreateBootstrapConfigurationTest, DeterminesSniFromRequestHeader) { - uris_.push_back(std::make_unique("https://www.example.org")); - resolveAllUris(); + setupUriResolutionExpectations(); std::unique_ptr options = Client::TestUtility::createOptionsImpl("nighthawk_client " @@ -1432,7 +1475,7 @@ TEST_F(CreateBootstrapConfigurationTest, DeterminesSniFromRequestHeader) { ASSERT_THAT(expected_bootstrap, StatusIs(absl::StatusCode::kOk)); absl::StatusOr bootstrap = - createBootstrapConfiguration(*options, uris_, request_source_uri_, number_of_workers_); + createBootstrapConfiguration(mock_dispatcher_, *options, number_of_workers_); ASSERT_THAT(bootstrap, StatusIs(absl::StatusCode::kOk)); EXPECT_THAT(*bootstrap, EqualsProto(*expected_bootstrap)); diff --git a/test/process_test.cc b/test/process_test.cc index 76696a9a7..8397920ff 100644 --- a/test/process_test.cc +++ b/test/process_test.cc @@ -4,6 +4,7 @@ #include "nighthawk/common/exception.h" +#include "external/envoy/source/common/common/statusor.h" #include "external/envoy/test/test_common/environment.h" #include "external/envoy/test/test_common/network_utility.h" #include "external/envoy/test/test_common/registry.h" @@ -71,9 +72,15 @@ class ProcessTest : public TestWithParam { options_(TestUtility::createOptionsImpl( fmt::format("foo --duration 1 -v error --rps 10 https://{}/", loopback_address_))){}; - void runProcess(RunExpectation expectation, bool do_cancel = false, - bool terminate_right_away = false) { - ProcessPtr process = std::make_unique(*options_, time_system_); + absl::Status runProcess(RunExpectation expectation, bool do_cancel = false, + bool terminate_right_away = false) { + absl::StatusOr process_or_status = + ProcessImpl::CreateProcessImpl(*options_, time_system_); + if (!process_or_status.ok()) { + return process_or_status.status(); + } + ProcessPtr process = std::move(process_or_status.value()); + OutputCollectorImpl collector(time_system_, *options_); std::thread cancel_thread; if (do_cancel) { @@ -113,6 +120,7 @@ class ProcessTest : public TestWithParam { } } process->shutdown(); + return absl::OkStatus(); } const std::string loopback_address_; @@ -124,18 +132,24 @@ INSTANTIATE_TEST_SUITE_P(IpVersions, ProcessTest, ValuesIn(Envoy::TestEnvironment::getIpVersionsForTest()), Envoy::TestUtility::ipTestParamsToString); +TEST_P(ProcessTest, FailsToCreateProcessOnUnresolvableHost) { + options_ = + TestUtility::createOptionsImpl("foo --h2 --duration 1 --rps 10 https://unresolveable.host/"); + EXPECT_FALSE(runProcess(RunExpectation::EXPECT_FAILURE).ok()); +} + TEST_P(ProcessTest, TwoProcessInSequence) { - runProcess(RunExpectation::EXPECT_FAILURE); + ASSERT_TRUE(runProcess(RunExpectation::EXPECT_FAILURE).ok()); options_ = TestUtility::createOptionsImpl( fmt::format("foo --h2 --duration 1 --rps 10 https://{}/", loopback_address_)); - runProcess(RunExpectation::EXPECT_FAILURE); + EXPECT_TRUE(runProcess(RunExpectation::EXPECT_FAILURE).ok()); } // TODO(oschaaf): move to python int. tests once it adds to coverage. TEST_P(ProcessTest, BadTracerSpec) { options_ = TestUtility::createOptionsImpl( fmt::format("foo --trace foo://localhost:79/api/v1/spans https://{}/", loopback_address_)); - runProcess(RunExpectation::EXPECT_FAILURE); + EXPECT_TRUE(runProcess(RunExpectation::EXPECT_FAILURE).ok()); } TEST_P(ProcessTest, CancelDuringLoadTest) { @@ -145,14 +159,14 @@ TEST_P(ProcessTest, CancelDuringLoadTest) { options_ = TestUtility::createOptionsImpl( fmt::format("foo --duration 300 --failure-predicate foo:0 --concurrency 2 https://{}/", loopback_address_)); - runProcess(RunExpectation::EXPECT_SUCCESS, true); + EXPECT_TRUE(runProcess(RunExpectation::EXPECT_SUCCESS, true).ok()); } TEST_P(ProcessTest, CancelExecutionBeforeBeginLoadTest) { options_ = TestUtility::createOptionsImpl( fmt::format("foo --duration 300 --failure-predicate foo:0 --concurrency 2 https://{}/", loopback_address_)); - runProcess(RunExpectation::EXPECT_SUCCESS, true, true); + EXPECT_TRUE(runProcess(RunExpectation::EXPECT_SUCCESS, true, true).ok()); } TEST_P(ProcessTest, RunProcessWithStatsSinkConfigured) { @@ -163,7 +177,7 @@ TEST_P(ProcessTest, RunProcessWithStatsSinkConfigured) { "--stats-sinks {} https://{}/", kSinkName, loopback_address_)); numFlushes = 0; - runProcess(RunExpectation::EXPECT_FAILURE); + ASSERT_TRUE(runProcess(RunExpectation::EXPECT_FAILURE).ok()); EXPECT_GT(numFlushes, 0); } @@ -175,7 +189,7 @@ TEST_P(ProcessTest, NoFlushWhenCancelExecutionBeforeLoadTestBegin) { "2 --stats-flush-interval 1 --stats-sinks {} https://{}/", kSinkName, loopback_address_)); numFlushes = 0; - runProcess(RunExpectation::EXPECT_SUCCESS, true, true); + ASSERT_TRUE(runProcess(RunExpectation::EXPECT_SUCCESS, true, true).ok()); EXPECT_EQ(numFlushes, 0); } @@ -191,9 +205,18 @@ class ProcessTestWithSimTime : public Envoy::Event::TestUsingSimulatedTime, Envoy::Network::Test::getLoopbackAddressUrlString(GetParam())))){}; protected: - void run(std::function verify_callback) { - auto run_thread = std::thread([this, &verify_callback] { - ProcessPtr process = std::make_unique(*options_, simTime()); + absl::Status run(std::function verify_callback) { + absl::Status process_status; + + auto run_thread = std::thread([this, &verify_callback, &process_status] { + absl::StatusOr process_or_status = + ProcessImpl::CreateProcessImpl(*options_, simTime()); + if (!process_or_status.ok()) { + process_status = process_or_status.status(); + return; + } + + ProcessPtr process = std::move(process_or_status.value()); OutputCollectorImpl collector(simTime(), *options_); const bool result = process->run(collector); process->shutdown(); @@ -216,6 +239,8 @@ class ProcessTestWithSimTime : public Envoy::Event::TestUsingSimulatedTime, simTime().setSystemTime(options_->scheduled_start().value() + 2s); // Wait for execution to wrap up. run_thread.join(); + + return process_status; } void setScheduleOnOptions(std::chrono::nanoseconds ns_since_epoch) { @@ -239,25 +264,25 @@ TEST_P(ProcessTestWithSimTime, ScheduleAheadWorks) { for (const auto& relative_schedule : std::vector{30s, 1h}) { setScheduleOnOptions( std::chrono::nanoseconds(simTime().systemTime().time_since_epoch() + relative_schedule)); - run([this](bool success, const nighthawk::client::Output& output) { - EXPECT_TRUE(success); - ASSERT_EQ(output.results_size(), 1); - EXPECT_EQ(Envoy::ProtobufUtil::TimeUtil::TimestampToNanoseconds( - output.results()[0].execution_start()), - std::chrono::duration_cast( - options_->scheduled_start().value().time_since_epoch()) - .count()); - }); + EXPECT_TRUE(run([this](bool success, const nighthawk::client::Output& output) { + EXPECT_TRUE(success); + ASSERT_EQ(output.results_size(), 1); + EXPECT_EQ(Envoy::ProtobufUtil::TimeUtil::TimestampToNanoseconds( + output.results()[0].execution_start()), + std::chrono::duration_cast( + options_->scheduled_start().value().time_since_epoch()) + .count()); + }).ok()); } } // Verify that scheduling an execution in the past yields an error. TEST_P(ProcessTestWithSimTime, ScheduleInThePastFails) { setScheduleOnOptions(std::chrono::nanoseconds(simTime().systemTime().time_since_epoch() - 1s)); - run([](bool success, const nighthawk::client::Output& output) { - EXPECT_FALSE(success); - EXPECT_EQ(output.results_size(), 0); - }); + EXPECT_TRUE(run([](bool success, const nighthawk::client::Output& output) { + EXPECT_FALSE(success); + EXPECT_EQ(output.results_size(), 0); + }).ok()); } } // namespace diff --git a/test/service_test.cc b/test/service_test.cc index 06ddadc3e..915af5fb2 100644 --- a/test/service_test.cc +++ b/test/service_test.cc @@ -239,7 +239,7 @@ TEST_P(ServiceTest, Unresolvable) { auto options = request_.mutable_start_request()->mutable_options(); options->mutable_uri()->set_value("http://unresolvable-host/"); // We expect output, because the options proto is valid. - runWithFailingValidationExpectations(true, "Unknown failure"); + runWithFailingValidationExpectations(false, "Unable to create ProcessImpl"); } } // namespace