From 732b1bea5ccf9440c32edc4cdb94cce585c5d14f Mon Sep 17 00:00:00 2001 From: Otto van der Schaaf Date: Mon, 15 Jul 2019 16:43:44 +0200 Subject: [PATCH 01/31] save state Signed-off-by: Otto van der Schaaf --- bazel/repositories.bzl | 4 ++-- source/client/process_impl.cc | 14 +++++++++++--- source/common/uri_impl.cc | 11 ++++++----- test/process_test.cc | 2 +- 4 files changed, 20 insertions(+), 11 deletions(-) diff --git a/bazel/repositories.bzl b/bazel/repositories.bzl index 1007ba0d7..76afeb5fe 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 = "2f569b9a8d3f0d7a43ffa69e3e5ba947cd3a9f8b" -ENVOY_SHA = "ec47fee6604468bc392937967415c736f19fb22129929881270a1635ad216d87" +ENVOY_COMMIT = "9e3871ade779664ffb0a8f09b11688878253a082" +ENVOY_SHA = "31ab4d8015b048cf129a3642c032726a4df7b33667b3669c1d9e53bd3a1da8d2" RULES_PYTHON_COMMIT = "fdbb17a4118a1728d19e638a5291b4c4266ea5b8" RULES_PYTHON_SHA = "9a3d71e348da504a9c4c5e8abd4cb822f7afb32c613dc6ee8b8535333a81a938" diff --git a/source/client/process_impl.cc b/source/client/process_impl.cc index 7dc39406a..d8934aee4 100644 --- a/source/client/process_impl.cc +++ b/source/client/process_impl.cc @@ -20,6 +20,7 @@ #include "common/filesystem/filesystem_impl.h" #include "common/frequency.h" #include "common/network/utility.h" +#include "common/protobuf/message_validator_impl.h" #include "common/runtime/runtime_impl.h" #include "common/thread_local/thread_local_impl.h" #include "common/uri_impl.h" @@ -187,9 +188,16 @@ bool ProcessImpl::run(OutputCollector& collector) { bool ok = true; Envoy::Runtime::RandomGeneratorImpl generator; - Envoy::Runtime::ScopedLoaderSingleton loader( - Envoy::Runtime::LoaderPtr{new Envoy::Runtime::LoaderImpl( - *dispatcher_, tls_, {}, "foo-cluster", *store_, generator, api_)}); + /* + LoaderImpl(Event::Dispatcher& dispatcher, ThreadLocal::SlotAllocator& tls, + const envoy::config::bootstrap::v2::LayeredRuntime& config, + const LocalInfo::LocalInfo& local_info, Init::Manager& init_manager, + Stats::Store& store, RandomGenerator& generator, + ProtobufMessage::ValidationVisitor& validation_visitor, Api::Api& api); + */ + Envoy::Runtime::ScopedLoaderSingleton loader(Envoy::Runtime::LoaderPtr{ + new Envoy::Runtime::LoaderImpl(*dispatcher_, tls_, {}, "foo-cluster", *store_, generator, + Envoy::ProtobufMessage::getStrictValidationVisitor(), api_)}); for (auto& w : workers_) { w->start(); diff --git a/source/common/uri_impl.cc b/source/common/uri_impl.cc index 30a148ba6..00f596888 100644 --- a/source/common/uri_impl.cc +++ b/source/common/uri_impl.cc @@ -60,13 +60,14 @@ bool UriImpl::performDnsLookup(Envoy::Event::Dispatcher& dispatcher, Envoy::Network::ActiveDnsQuery* active_dns_query_ = dns_resolver->resolve( hostname, dns_lookup_family, - [this, &dispatcher, &active_dns_query_]( - const std::list&& address_list) -> void { + [this, &dispatcher, + &active_dns_query_](std::list&& response) -> void { active_dns_query_ = nullptr; - if (!address_list.empty()) { - address_ = Envoy::Network::Utility::getAddressWithPort(*address_list.front(), port()); + if (!response.empty()) { + address_ = + Envoy::Network::Utility::getAddressWithPort(*response.front().address_, port()); ENVOY_LOG(debug, "DNS resolution complete for {} ({} entries, using {}).", - hostWithoutPort(), address_list.size(), address_->asString()); + hostWithoutPort(), response.size(), address_->asString()); } dispatcher.exit(); }); diff --git a/test/process_test.cc b/test/process_test.cc index 05b9489a6..6d3b9a1f0 100644 --- a/test/process_test.cc +++ b/test/process_test.cc @@ -16,7 +16,7 @@ namespace Client { // TODO(oschaaf): when we have proper integration testing, update this. // For now we are covered via the client_tests.cc by proxy. Eventually we // want those tests in here, and mock Process in client_test. -class ProcessTest : public Test { +class ProcessTest : public testing::Test { public: void runProcess() { From 18197fb8d04973cf3c090ad2f20a03ff8c3270ee Mon Sep 17 00:00:00 2001 From: Otto van der Schaaf Date: Mon, 15 Jul 2019 17:04:26 +0200 Subject: [PATCH 02/31] save state Signed-off-by: Otto van der Schaaf --- source/client/process_impl.cc | 6 +++++- test/client_worker_test.cc | 9 +++++---- test/server/http_test_server_filter_integration_test.cc | 8 ++++---- test/worker_test.cc | 4 ++-- 4 files changed, 16 insertions(+), 11 deletions(-) diff --git a/source/client/process_impl.cc b/source/client/process_impl.cc index d8934aee4..9de6a7663 100644 --- a/source/client/process_impl.cc +++ b/source/client/process_impl.cc @@ -19,6 +19,7 @@ #include "common/event/real_time_system.h" #include "common/filesystem/filesystem_impl.h" #include "common/frequency.h" +#include "common/init/manager_impl.h" #include "common/network/utility.h" #include "common/protobuf/message_validator_impl.h" #include "common/runtime/runtime_impl.h" @@ -195,8 +196,11 @@ bool ProcessImpl::run(OutputCollector& collector) { Stats::Store& store, RandomGenerator& generator, ProtobufMessage::ValidationVisitor& validation_visitor, Api::Api& api); */ + Envoy::LocalInfo::LocalInfoPtr local_info; + Envoy::Init::ManagerImpl init_manager("nighthawk_init_manager"); Envoy::Runtime::ScopedLoaderSingleton loader(Envoy::Runtime::LoaderPtr{ - new Envoy::Runtime::LoaderImpl(*dispatcher_, tls_, {}, "foo-cluster", *store_, generator, + new Envoy::Runtime::LoaderImpl(*dispatcher_, tls_, {}, *local_info, init_manager, *store_, + generator, Envoy::ProtobufMessage::getStrictValidationVisitor(), api_)}); for (auto& w : workers_) { diff --git a/test/client_worker_test.cc b/test/client_worker_test.cc index 6f02b2177..f8fd38570 100644 --- a/test/client_worker_test.cc +++ b/test/client_worker_test.cc @@ -25,9 +25,10 @@ class ClientWorkerTest : public Test { public: ClientWorkerTest() : api_(Envoy::Thread::threadFactoryForTest(), store_, time_system_, file_system_), - thread_id_(std::this_thread::get_id()), - loader_(Envoy::Runtime::LoaderPtr{new Envoy::Runtime::LoaderImpl( - dispatcher_, tls_, {}, "test-cluster", store_, rand_, api_)}) { + thread_id_(std::this_thread::get_id()) /*, + loader_(Envoy::Runtime::LoaderPtr{new Envoy::Runtime::LoaderImpl( + dispatcher_, tls_, {}, "test-cluster", store_, rand_, api_)})*/ + { benchmark_client_ = new MockBenchmarkClient(); sequencer_ = new MockSequencer(); @@ -65,7 +66,7 @@ class ClientWorkerTest : public Test { MockSequencer* sequencer_; Envoy::Runtime::RandomGeneratorImpl rand_; NiceMock dispatcher_; - Envoy::Runtime::ScopedLoaderSingleton loader_; + // Envoy::Runtime::ScopedLoaderSingleton loader_; Envoy::Filesystem::InstanceImplPosix file_system_; }; diff --git a/test/server/http_test_server_filter_integration_test.cc b/test/server/http_test_server_filter_integration_test.cc index afac4c987..a1c41145f 100644 --- a/test/server/http_test_server_filter_integration_test.cc +++ b/test/server/http_test_server_filter_integration_test.cc @@ -142,8 +142,8 @@ name: test-server } }; -INSTANTIATE_TEST_CASE_P(IpVersions, HttpTestServerIntegrationTest, - ValuesIn(Envoy::TestEnvironment::getIpVersionsForTest())); +INSTANTIATE_TEST_SUITE_P(IpVersions, HttpTestServerIntegrationTest, + ValuesIn(Envoy::TestEnvironment::getIpVersionsForTest())); TEST_P(HttpTestServerIntegrationTest, TestNoHeaderConfig) { Envoy::BufferingStreamDecoderPtr response = @@ -211,8 +211,8 @@ name: test-server } }; -INSTANTIATE_TEST_CASE_P(IpVersions, HttpTestServerIntegrationNoConfigTest, - ValuesIn(Envoy::TestEnvironment::getIpVersionsForTest())); +INSTANTIATE_TEST_SUITE_P(IpVersions, HttpTestServerIntegrationNoConfigTest, + ValuesIn(Envoy::TestEnvironment::getIpVersionsForTest())); TEST_P(HttpTestServerIntegrationNoConfigTest, TestNoHeaderConfig) { Envoy::BufferingStreamDecoderPtr response = diff --git a/test/worker_test.cc b/test/worker_test.cc index 4da9b12e2..71dd6bdcb 100644 --- a/test/worker_test.cc +++ b/test/worker_test.cc @@ -50,8 +50,8 @@ TEST_F(WorkerTest, WorkerExecutesOnThread) { TestWorker worker(api_, tls_); NiceMock dispatcher; - Envoy::Runtime::ScopedLoaderSingleton loader(Envoy::Runtime::LoaderPtr{ - new Envoy::Runtime::LoaderImpl(dispatcher, tls_, {}, "test-cluster", store_, rand_, api_)}); + // Envoy::Runtime::ScopedLoaderSingleton loader(Envoy::Runtime::LoaderPtr{ + // new Envoy::Runtime::LoaderImpl(dispatcher, tls_, {}, "test-cluster", store_, rand_, api_)}); worker.start(); worker.waitForCompletion(); From 8a14b8bb27737bb8792e41ea502f8b5667554ab9 Mon Sep 17 00:00:00 2001 From: Otto van der Schaaf Date: Mon, 15 Jul 2019 17:18:44 +0200 Subject: [PATCH 03/31] save state Signed-off-by: Otto van der Schaaf --- source/client/process_impl.cc | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/source/client/process_impl.cc b/source/client/process_impl.cc index 9de6a7663..7fa5af11c 100644 --- a/source/client/process_impl.cc +++ b/source/client/process_impl.cc @@ -20,6 +20,7 @@ #include "common/filesystem/filesystem_impl.h" #include "common/frequency.h" #include "common/init/manager_impl.h" +#include "common/local_info/local_info_impl.h" #include "common/network/utility.h" #include "common/protobuf/message_validator_impl.h" #include "common/runtime/runtime_impl.h" @@ -196,10 +197,14 @@ bool ProcessImpl::run(OutputCollector& collector) { Stats::Store& store, RandomGenerator& generator, ProtobufMessage::ValidationVisitor& validation_visitor, Api::Api& api); */ - Envoy::LocalInfo::LocalInfoPtr local_info; + auto local_address = Network::Utility::getLocalAddress(Envoy::Network::Address::IpVersion::v4); + auto local_info = + Envoy::LocalInfo::LocalInfoImpl({}, local_address, "nighthawk_service_zone", + "nighthawk_service_cluster", "nighthawk_service_node"); + Envoy::Init::ManagerImpl init_manager("nighthawk_init_manager"); Envoy::Runtime::ScopedLoaderSingleton loader(Envoy::Runtime::LoaderPtr{ - new Envoy::Runtime::LoaderImpl(*dispatcher_, tls_, {}, *local_info, init_manager, *store_, + new Envoy::Runtime::LoaderImpl(*dispatcher_, tls_, {}, local_info, init_manager, *store_, generator, Envoy::ProtobufMessage::getStrictValidationVisitor(), api_)}); From 1f43db64143d2b06d06ac478b48da1b24985e350 Mon Sep 17 00:00:00 2001 From: Otto van der Schaaf Date: Thu, 18 Jul 2019 10:47:43 +0200 Subject: [PATCH 04/31] envoy 9e3871ade779664ffb0a8f09b11688878253a082 Signed-off-by: Otto van der Schaaf --- source/client/process_impl.cc | 18 ++++-------------- source/common/worker_impl.cc | 1 - test/client_worker_test.cc | 6 +----- test/worker_test.cc | 7 ------- 4 files changed, 5 insertions(+), 27 deletions(-) diff --git a/source/client/process_impl.cc b/source/client/process_impl.cc index 7fa5af11c..22e9959a0 100644 --- a/source/client/process_impl.cc +++ b/source/client/process_impl.cc @@ -187,27 +187,17 @@ bool ProcessImpl::run(OutputCollector& collector) { return false; } const std::vector& workers = createWorkers(uri, determineConcurrency()); - - bool ok = true; Envoy::Runtime::RandomGeneratorImpl generator; - /* - LoaderImpl(Event::Dispatcher& dispatcher, ThreadLocal::SlotAllocator& tls, - const envoy::config::bootstrap::v2::LayeredRuntime& config, - const LocalInfo::LocalInfo& local_info, Init::Manager& init_manager, - Stats::Store& store, RandomGenerator& generator, - ProtobufMessage::ValidationVisitor& validation_visitor, Api::Api& api); - */ - auto local_address = Network::Utility::getLocalAddress(Envoy::Network::Address::IpVersion::v4); - auto local_info = - Envoy::LocalInfo::LocalInfoImpl({}, local_address, "nighthawk_service_zone", - "nighthawk_service_cluster", "nighthawk_service_node"); - + auto local_info = Envoy::LocalInfo::LocalInfoImpl( + {}, Network::Utility::getLocalAddress(Envoy::Network::Address::IpVersion::v4), + "nighthawk_service_zone", "nighthawk_service_cluster", "nighthawk_service_node"); Envoy::Init::ManagerImpl init_manager("nighthawk_init_manager"); Envoy::Runtime::ScopedLoaderSingleton loader(Envoy::Runtime::LoaderPtr{ new Envoy::Runtime::LoaderImpl(*dispatcher_, tls_, {}, local_info, init_manager, *store_, generator, Envoy::ProtobufMessage::getStrictValidationVisitor(), api_)}); + bool ok = true; for (auto& w : workers_) { w->start(); } diff --git a/source/common/worker_impl.cc b/source/common/worker_impl.cc index 50bb1e03a..44c7d0f2e 100644 --- a/source/common/worker_impl.cc +++ b/source/common/worker_impl.cc @@ -20,7 +20,6 @@ void WorkerImpl::start() { ASSERT(!started_ && !completed_); started_ = true; thread_ = thread_factory_.createThread([this]() { - ASSERT(Envoy::Runtime::LoaderSingleton::getExisting() != nullptr); // Run the dispatcher to let the callbacks posted by registerThread() execute. dispatcher_->run(Envoy::Event::Dispatcher::RunType::Block); work(); diff --git a/test/client_worker_test.cc b/test/client_worker_test.cc index f8fd38570..fd5658ab6 100644 --- a/test/client_worker_test.cc +++ b/test/client_worker_test.cc @@ -25,10 +25,7 @@ class ClientWorkerTest : public Test { public: ClientWorkerTest() : api_(Envoy::Thread::threadFactoryForTest(), store_, time_system_, file_system_), - thread_id_(std::this_thread::get_id()) /*, - loader_(Envoy::Runtime::LoaderPtr{new Envoy::Runtime::LoaderImpl( - dispatcher_, tls_, {}, "test-cluster", store_, rand_, api_)})*/ - { + thread_id_(std::this_thread::get_id()) { benchmark_client_ = new MockBenchmarkClient(); sequencer_ = new MockSequencer(); @@ -66,7 +63,6 @@ class ClientWorkerTest : public Test { MockSequencer* sequencer_; Envoy::Runtime::RandomGeneratorImpl rand_; NiceMock dispatcher_; - // Envoy::Runtime::ScopedLoaderSingleton loader_; Envoy::Filesystem::InstanceImplPosix file_system_; }; diff --git a/test/worker_test.cc b/test/worker_test.cc index 71dd6bdcb..81a48ecae 100644 --- a/test/worker_test.cc +++ b/test/worker_test.cc @@ -44,18 +44,11 @@ class WorkerTest : public Test { TEST_F(WorkerTest, WorkerExecutesOnThread) { InSequence in_sequence; - EXPECT_CALL(tls_, registerThread(_, false)).Times(1); - EXPECT_CALL(tls_, allocateSlot()).Times(1); - TestWorker worker(api_, tls_); NiceMock dispatcher; - // Envoy::Runtime::ScopedLoaderSingleton loader(Envoy::Runtime::LoaderPtr{ - // new Envoy::Runtime::LoaderImpl(dispatcher, tls_, {}, "test-cluster", store_, rand_, api_)}); - worker.start(); worker.waitForCompletion(); - EXPECT_CALL(tls_, shutdownThread()).Times(1); ASSERT_TRUE(worker.ran_); } From d1d7f78954c67e27183796e871acf18a9f580599 Mon Sep 17 00:00:00 2001 From: Otto van der Schaaf Date: Thu, 18 Jul 2019 11:10:50 +0200 Subject: [PATCH 05/31] Update to Envoy bcc66c6b74c365d1d2834cfe15b847ae13be0eb6 Signed-off-by: Otto van der Schaaf --- WORKSPACE | 7 ++----- bazel/repositories.bzl | 4 ++-- test/server/http_test_server_filter_integration_test.cc | 2 +- 3 files changed, 5 insertions(+), 8 deletions(-) diff --git a/WORKSPACE b/WORKSPACE index dcde98112..e84fafdec 100644 --- a/WORKSPACE +++ b/WORKSPACE @@ -8,13 +8,10 @@ load("@envoy//bazel:api_repositories.bzl", "envoy_api_dependencies") envoy_api_dependencies() -load("@envoy//bazel:repositories.bzl", "envoy_dependencies") -load("@envoy//bazel:cc_configure.bzl", "cc_configure") +load("@envoy//bazel:repositories.bzl", "GO_VERSION", "envoy_dependencies") envoy_dependencies() -cc_configure() - load("@rules_foreign_cc//:workspace_definitions.bzl", "rules_foreign_cc_dependencies") rules_foreign_cc_dependencies() @@ -23,7 +20,7 @@ load("@io_bazel_rules_go//go:deps.bzl", "go_register_toolchains", "go_rules_depe go_rules_dependencies() -go_register_toolchains() +go_register_toolchains(go_version = GO_VERSION) # For PIP support: load("@io_bazel_rules_python//python:pip.bzl", "pip_import", "pip_repositories") diff --git a/bazel/repositories.bzl b/bazel/repositories.bzl index 76afeb5fe..5f6edfa76 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 = "9e3871ade779664ffb0a8f09b11688878253a082" -ENVOY_SHA = "31ab4d8015b048cf129a3642c032726a4df7b33667b3669c1d9e53bd3a1da8d2" +ENVOY_COMMIT = "bcc66c6b74c365d1d2834cfe15b847ae13be0eb6" +ENVOY_SHA = "4265c3820076f81cbc54ecd7a760ff7da5cf90ef6b49fc0b08b6137f5b54e44d" RULES_PYTHON_COMMIT = "fdbb17a4118a1728d19e638a5291b4c4266ea5b8" RULES_PYTHON_SHA = "9a3d71e348da504a9c4c5e8abd4cb822f7afb32c613dc6ee8b8535333a81a938" diff --git a/test/server/http_test_server_filter_integration_test.cc b/test/server/http_test_server_filter_integration_test.cc index a1c41145f..a9560e410 100644 --- a/test/server/http_test_server_filter_integration_test.cc +++ b/test/server/http_test_server_filter_integration_test.cc @@ -54,7 +54,7 @@ class HttpTestServerIntegrationTestBase : public Envoy::HttpIntegrationTest, type, dispatcher->createClientConnection(addr, Envoy::Network::Address::InstanceConstSharedPtr(), Envoy::Network::Test::createRawBufferSocket(), nullptr), - host_description, *dispatcher); + host_description, *dispatcher, false /* strict header validation */); Envoy::BufferingStreamDecoderPtr response( new Envoy::BufferingStreamDecoder([&client, &dispatcher]() -> void { client.close(); From c84d146e80f69e981485bb1ff66a2105b2b14cfc Mon Sep 17 00:00:00 2001 From: Otto van der Schaaf Date: Tue, 23 Jul 2019 16:27:59 +0200 Subject: [PATCH 06/31] poc for embedding python in the c++ bm test Signed-off-by: Otto van der Schaaf --- WORKSPACE | 14 +++++ test/BUILD | 5 +- test/benchmark_http_client_test.cc | 95 +++++++++++++++++++----------- 3 files changed, 79 insertions(+), 35 deletions(-) diff --git a/WORKSPACE b/WORKSPACE index e84fafdec..74fa4edb8 100644 --- a/WORKSPACE +++ b/WORKSPACE @@ -22,6 +22,20 @@ go_rules_dependencies() go_register_toolchains(go_version = GO_VERSION) +new_local_repository( + name = "python_linux", + build_file_content = """ +cc_library( + name = "python36-lib", + srcs = ["lib/python3.6/config-3.6m-x86_64-linux-gnu/libpython3.6.so"], + hdrs = glob(["include/python3.6/*.h"]), + includes = ["include/python3.6"], + visibility = ["//visibility:public"] +) + """, + path = "/usr", +) + # For PIP support: load("@io_bazel_rules_python//python:pip.bzl", "pip_import", "pip_repositories") diff --git a/test/BUILD b/test/BUILD index 75c6bc615..00b504ad2 100644 --- a/test/BUILD +++ b/test/BUILD @@ -1,5 +1,3 @@ -licenses(["notice"]) # Apache 2 - load( "@envoy//bazel:envoy_build_system.bzl", "envoy_cc_mock", @@ -7,6 +5,8 @@ load( "envoy_package", ) +licenses(["notice"]) # Apache 2 + envoy_package() envoy_cc_mock( @@ -69,5 +69,6 @@ envoy_cc_test( "@envoy//test/mocks/thread_local:thread_local_mocks", "@envoy//test/server:utility_lib", "@envoy//test/test_common:simulated_time_system_lib", + "@python_linux//:python36-lib", ], ) diff --git a/test/benchmark_http_client_test.cc b/test/benchmark_http_client_test.cc index e1237d429..c24401397 100644 --- a/test/benchmark_http_client_test.cc +++ b/test/benchmark_http_client_test.cc @@ -27,22 +27,45 @@ #include "test/mocks/runtime/mocks.h" #include "test/mocks/thread_local/mocks.h" #include "test/server/utility.h" +#include "test/test_common/test_time_system.h" #include "test/test_common/utility.h" #include "ares.h" #include "gtest/gtest.h" +#include + using namespace std::chrono_literals; using namespace testing; namespace Nighthawk { -class BenchmarkClientTestBase : public Envoy::BaseIntegrationTest, +class PythonIntegrationTestBase { +public: + virtual ~PythonIntegrationTestBase() = default; + PythonIntegrationTestBase(Envoy::Network::Address::IpVersion version, + const std::string& config = Envoy::ConfigHelper::HTTP_PROXY_CONFIG) + : version_(version), config_(config) { + initialize(); + }; + + virtual void initialize() { + Py_Initialize(); + PyRun_SimpleString("import sys\n" + "print('hey', file=sys.stderr)\n"); + Py_Finalize(); + } + Envoy::Event::RealTimeSystem time_system_; + Envoy::Network::Address::IpVersion version_; + const std::string config_; +}; + +class BenchmarkClientTestBase : public PythonIntegrationTestBase, public TestWithParam { public: BenchmarkClientTestBase() - : Envoy::BaseIntegrationTest(GetParam(), realTime(), BenchmarkClientTestBase::envoy_config), - api_(thread_factory_, store_, timeSystem(), file_system_), + : PythonIntegrationTestBase(GetParam(), BenchmarkClientTestBase::envoy_config), + api_(thread_factory_, store_, time_system_, file_system_), dispatcher_(api_.allocateDispatcher()) {} static void SetUpTestCase() { @@ -61,13 +84,15 @@ class BenchmarkClientTestBase : public Envoy::BaseIntegrationTest, if (client_ != nullptr) { client_->terminate(); } - test_server_.reset(); - fake_upstreams_.clear(); tls_.shutdownGlobalThreading(); ares_library_cleanup(); } - uint32_t getTestServerHostAndPort() { return lookupPort("listener_0"); } + uint32_t getTestServerHostAndPort() { + ASSERT(false); + return 0; + /* return lookupPort("listener_0");*/ + } void testBasicFunctionality(absl::string_view uriPath, const uint64_t max_pending, const uint64_t connection_limit, const bool use_h2, @@ -161,31 +186,32 @@ class BenchmarkClientHttpsTest : public BenchmarkClientTestBase { }; void initialize() override { - config_helper_.addConfigModifier([](envoy::config::bootstrap::v2::Bootstrap& bootstrap) { - auto* common_tls_context = bootstrap.mutable_static_resources() - ->mutable_listeners(0) - ->mutable_filter_chains(0) - ->mutable_tls_context() - ->mutable_common_tls_context(); - common_tls_context->mutable_validation_context_sds_secret_config()->set_name( - "validation_context"); - common_tls_context->add_tls_certificate_sds_secret_configs()->set_name("server_cert"); - - auto* secret = bootstrap.mutable_static_resources()->add_secrets(); - secret->set_name("validation_context"); - auto* validation_context = secret->mutable_validation_context(); - validation_context->mutable_trusted_ca()->set_filename(Envoy::TestEnvironment::runfilesPath( - "external/envoy/test/config/integration/certs/cacert.pem")); - secret = bootstrap.mutable_static_resources()->add_secrets(); - secret->set_name("server_cert"); - auto* tls_certificate = secret->mutable_tls_certificate(); - tls_certificate->mutable_certificate_chain()->set_filename( - Envoy::TestEnvironment::runfilesPath( - "external/envoy/test/config/integration/certs/servercert.pem")); - tls_certificate->mutable_private_key()->set_filename(Envoy::TestEnvironment::runfilesPath( - "external/envoy/test/config/integration/certs/serverkey.pem")); - }); - + /* + config_helper_.addConfigModifier([](envoy::config::bootstrap::v2::Bootstrap& bootstrap) { + auto* common_tls_context = bootstrap.mutable_static_resources() + ->mutable_listeners(0) + ->mutable_filter_chains(0) + ->mutable_tls_context() + ->mutable_common_tls_context(); + common_tls_context->mutable_validation_context_sds_secret_config()->set_name( + "validation_context"); + common_tls_context->add_tls_certificate_sds_secret_configs()->set_name("server_cert"); + + auto* secret = bootstrap.mutable_static_resources()->add_secrets(); + secret->set_name("validation_context"); + auto* validation_context = secret->mutable_validation_context(); + validation_context->mutable_trusted_ca()->set_filename(Envoy::TestEnvironment::runfilesPath( + "external/envoy/test/config/integration/certs/cacert.pem")); + secret = bootstrap.mutable_static_resources()->add_secrets(); + secret->set_name("server_cert"); + auto* tls_certificate = secret->mutable_tls_certificate(); + tls_certificate->mutable_certificate_chain()->set_filename( + Envoy::TestEnvironment::runfilesPath( + "external/envoy/test/config/integration/certs/servercert.pem")); + tls_certificate->mutable_private_key()->set_filename(Envoy::TestEnvironment::runfilesPath( + "external/envoy/test/config/integration/certs/serverkey.pem")); + }); + */ BenchmarkClientTestBase::initialize(); } }; @@ -293,7 +319,9 @@ TEST_P(BenchmarkClientHttpTest, DISABLED_WeirdStatus) { TEST_P(BenchmarkClientHttpTest, H1ConnectionFailure) { // Kill the test server, so we can't connect. // We allow a single connection and no pending. We expect one connection failure. - test_server_.reset(); + ASSERT(false); + // test_server_.reset(); + testBasicFunctionality("/lorem-ipsum-status-200", 1, 1, false, 10); EXPECT_EQ(1, getCounter("upstream_cx_connect_fail")); @@ -309,7 +337,8 @@ TEST_P(BenchmarkClientHttpTest, H1ConnectionFailure) { TEST_P(BenchmarkClientHttpTest, H1MultiConnectionFailure) { // Kill the test server, so we can't connect. // We allow ten connections and ten pending requests. We expect ten connection failures. - test_server_.reset(); + ASSERT(false); + // test_server_.reset(); testBasicFunctionality("/lorem-ipsum-status-200", 10, 10, false, 10); EXPECT_EQ(10, getCounter("upstream_cx_connect_fail")); From fe671c8017a410d7e453d765c400a60ddd0552f5 Mon Sep 17 00:00:00 2001 From: Otto van der Schaaf Date: Wed, 24 Jul 2019 01:32:53 +0200 Subject: [PATCH 07/31] save state Signed-off-by: Otto van der Schaaf --- integration/BUILD | 17 ++- integration/cpp_benchmark_client_server.py | 56 ++++++++++ integration/integration_test.py | 1 - integration/integration_test_fixtures.py | 9 +- integration/nighthawk_test_server.py | 7 ++ test/BUILD | 41 +++++--- test/benchmark_http_client_test.cc | 115 ++++++++++++++++++--- 7 files changed, 210 insertions(+), 36 deletions(-) create mode 100644 integration/cpp_benchmark_client_server.py diff --git a/integration/BUILD b/integration/BUILD index 8b39e4000..444859b6b 100644 --- a/integration/BUILD +++ b/integration/BUILD @@ -1,13 +1,26 @@ -licenses(["notice"]) # Apache 2 - load( "@envoy//bazel:envoy_build_system.bzl", "envoy_package", ) load("@python_pip_deps//:requirements.bzl", "requirement") +licenses(["notice"]) # Apache 2 + envoy_package() +exports_files( + [ + "cpp_benchmark_client_server.py", + "test_integration_basics.py", + "common.py", + "integration_test_fixtures.py", + "nighthawk_test_server.py", + "integration_test.py", + "configurations/nighthawk_http_origin.yaml", + "configurations/nighthawk_https_origin.yaml", + ], +) + py_test( name = "integration_test", srcs = [ diff --git a/integration/cpp_benchmark_client_server.py b/integration/cpp_benchmark_client_server.py new file mode 100644 index 000000000..0e8bf6574 --- /dev/null +++ b/integration/cpp_benchmark_client_server.py @@ -0,0 +1,56 @@ +#!/usr/bin/env python3 +"""@package integration_test.py +Entry point for our integration testing +""" + +import logging +import os +import sys +import unittest +import cpp_benchmark_client_server + +from common import IpVersion, NighthawkException +from integration_test import determineIpVersionsFromEnvironment +from integration_test_fixtures import (HttpIntegrationTestBase, HttpsIntegrationTestBase, + IntegrationTestBase) + +assert sys.version_info >= (3, 0) + +def runTests(ip_version): + IntegrationTestBase.ip_version = ip_version + logging.info("Running %s integration tests." % + ("ipv6" if IntegrationTestBase.ip_version == IpVersion.IPV6 else "ipv4")) + res = unittest.TextTestRunner(verbosity=2).run( + unittest.TestLoader().loadTestsFromModule(cpp_benchmark_client_server)) + return True + +httpbase = None + +def serverStartHook(): + logging.basicConfig(stream=sys.stderr, level=logging.DEBUG) + ip_versions = determineIpVersionsFromEnvironment() + #ok = all(map(runTests, ip_versions)) + global httpbase + httpbase = TestHttp() + httpbase.setUp() + print("@@@@@@@@@@@@ " + str(httpbase.admin_port), file=sys.stderr) + #httpbase.test_run_h1_server_for_cpp_benchmark_client_test() + return httpbase.server_port + +def getRunningServerPid(): + return httpbase.getServerPid() + +class TestHttp(HttpIntegrationTestBase): + + def test_run_h1_server_for_cpp_benchmark_client_test(self): + """ + Runs the CLI configured to use plain HTTP/1 against our test server, and sanity + checks statistics from both client and server. + """ + #globals["server_pid"] = self.getServerPid() + #globals["server_port"] = self.server_port + #globals["admin_port"] = self.admin_port + #print(self.server_port, file=sys.stderr) + #print(self.admin_port, file=sys.stderr) + #sys.stderr.flush() + #self.waitForExit() diff --git a/integration/integration_test.py b/integration/integration_test.py index 9a5539b9f..3d50c7c85 100644 --- a/integration/integration_test.py +++ b/integration/integration_test.py @@ -36,7 +36,6 @@ def runTests(ip_version): unittest.TestLoader().loadTestsFromModule(test_integration_basics)) return res.wasSuccessful() - if __name__ == '__main__': logging.basicConfig(stream=sys.stderr, level=logging.DEBUG) ip_versions = determineIpVersionsFromEnvironment() diff --git a/integration/integration_test_fixtures.py b/integration/integration_test_fixtures.py index 79f3bbf5f..fd91d703a 100644 --- a/integration/integration_test_fixtures.py +++ b/integration/integration_test_fixtures.py @@ -55,7 +55,6 @@ def setUp(self): Performs sanity checks and starts up the server. Upon exit the server is ready to accept connections. """ self.assertTrue(os.path.exists(self.nighthawk_test_server_path)) - self.assertTrue(os.path.exists(self.nighthawk_client_path)) self.server_port = self.getFreeListenerPortForAddress(self.server_ip) self.admin_port = self.getFreeListenerPortForAddress(self.server_ip) self.parameters["admin_port"] = self.admin_port @@ -70,6 +69,12 @@ def tearDown(self): """ self.assertEqual(0, self.test_server.stop()) + def waitForServerExit(self): + self.assertEqual(0, self.test_server.waitForExit()) + + def getServerPid(self): + return self.test_server.getPid() + def getNighthawkCounterMapFromJson(self, parsed_json): """ Utility method to get the counters from the json indexed by name. @@ -121,6 +126,8 @@ def runNighthawkClient(self, args, expect_failure=False, timeout=30): Runs Nighthawk against the test server, returning a json-formatted result. If the timeout is exceeded an exception will be raised. """ + + self.assertTrue(os.path.exists(self.nighthawk_client_path)) if IntegrationTestBase.ip_version == IpVersion.IPV6: args.insert(0, "--address-family v6") args.insert(0, "--output-format json") diff --git a/integration/nighthawk_test_server.py b/integration/nighthawk_test_server.py index ad8d28fe5..8ee8cc8db 100644 --- a/integration/nighthawk_test_server.py +++ b/integration/nighthawk_test_server.py @@ -66,6 +66,13 @@ def start(self): self.server_thread.start() return self.waitUntilServerListening() + def waitForExit(self): + self.server_thread.join() + return self.server_process.returncode + + def getPid(self): + return self.server_process.pid + def stop(self): self.server_process.terminate() self.server_thread.join() diff --git a/test/BUILD b/test/BUILD index 00b504ad2..13e7223da 100644 --- a/test/BUILD +++ b/test/BUILD @@ -25,22 +25,22 @@ envoy_cc_test( name = "nighthawk_test", srcs = [ "benchmark_http_client_test.cc", - "client_test.cc", - "client_worker_test.cc", - "factories_test.cc", - "frequency_test.cc", - "options_test.cc", - "output_collector_test.cc", - "platform_util_test.cc", - "process_test.cc", - "rate_limiter_test.cc", - "sequencer_test.cc", - "service_main_test.cc", - "service_test.cc", - "statistic_test.cc", - "stream_decoder_test.cc", - "utility_test.cc", - "worker_test.cc", + #"client_test.cc", + #"client_worker_test.cc", + #"factories_test.cc", + #"frequency_test.cc", + #"options_test.cc", + #"output_collector_test.cc", + #"platform_util_test.cc", + #"process_test.cc", + #"rate_limiter_test.cc", + #"sequencer_test.cc", + #"service_main_test.cc", + #"service_test.cc", + #"statistic_test.cc", + #"stream_decoder_test.cc", + #"utility_test.cc", + #"worker_test.cc", ], data = [ "test_data/benchmark_http_client_test_envoy.yaml", @@ -49,12 +49,21 @@ envoy_cc_test( "test_data/output_collector.json.gold", "test_data/output_collector.txt.gold", "test_data/output_collector.yaml.gold", + "//integration:common.py", + "//integration:configurations/nighthawk_http_origin.yaml", + "//integration:configurations/nighthawk_https_origin.yaml", + "//integration:cpp_benchmark_client_server.py", + "//integration:integration_test.py", + "//integration:integration_test_fixtures.py", + "//integration:nighthawk_test_server.py", + "//integration:test_integration_basics.py", "@envoy//test/config/integration/certs", "@envoy//test/test_common:thread_factory_for_test_lib", "@envoy_api//envoy/api/v2:cds_export_cc", ], repository = "@envoy", deps = [ + "//:nighthawk_test_server", "//source/client:nighthawk_client_lib", "//source/client:nighthawk_service_lib", "//test:nighthawk_mocks", diff --git a/test/benchmark_http_client_test.cc b/test/benchmark_http_client_test.cc index c24401397..bde02c994 100644 --- a/test/benchmark_http_client_test.cc +++ b/test/benchmark_http_client_test.cc @@ -45,19 +45,89 @@ class PythonIntegrationTestBase { virtual ~PythonIntegrationTestBase() = default; PythonIntegrationTestBase(Envoy::Network::Address::IpVersion version, const std::string& config = Envoy::ConfigHelper::HTTP_PROXY_CONFIG) - : version_(version), config_(config) { - initialize(); - }; + : version_(version), config_(config){}; - virtual void initialize() { + void runPython() { + // std::string cmd = "PYTHONPATH=\"" + Envoy::TestEnvironment::runfilesPath("integration/") + + // "\""; RELEASE_ASSERT(::system(cmd.c_str()) == 0, "Failed to set pythonpath env var"); + // std::cerr << cmd << std::endl; Py_Initialize(); - PyRun_SimpleString("import sys\n" - "print('hey', file=sys.stderr)\n"); - Py_Finalize(); + PyRun_SimpleString("import sys"); + PyRun_SimpleString( + ("sys.path.append(\"" + Envoy::TestEnvironment::runfilesPath("integration/") + "\")") + .c_str()); + + PyObject* pName = PyUnicode_DecodeFSDefault("cpp_benchmark_client_server"); + PyObject* pModule = PyImport_Import(pName); + Py_DECREF(pName); + PyObject* pValue = nullptr; + PyObject* pFunc = nullptr; + + if (pModule != NULL) { + pFunc = PyObject_GetAttrString(pModule, "serverStartHook"); + if (pFunc && PyCallable_Check(pFunc)) { + pValue = PyObject_CallObject(pFunc, nullptr); + if (pValue != NULL) { + server_port_ = PyLong_AsLong(pValue); + RELEASE_ASSERT(server_port_ > 0, "bad server port"); + std::cerr << "@@@ port: " << server_port_ << std::endl; + Py_DECREF(pValue); + } else { + Py_DECREF(pFunc); + Py_DECREF(pModule); + PyErr_Print(); + std::cerr << "call failed" << std::endl; + } + } else { + if (PyErr_Occurred()) { + PyErr_Print(); + std::cerr << "failed to find cuntion" << std::endl; + } + } + + pFunc = PyObject_GetAttrString(pModule, "getRunningServerPid"); + if (pFunc && PyCallable_Check(pFunc)) { + pValue = PyObject_CallObject(pFunc, nullptr); + if (pValue != NULL) { + server_pid_ = PyLong_AsLong(pValue); + RELEASE_ASSERT(server_pid_ > 0, "bad server pid"); + std::cerr << "@@@ port: " << server_pid_ << std::endl; + Py_DECREF(pValue); + } else { + Py_DECREF(pFunc); + Py_DECREF(pModule); + PyErr_Print(); + std::cerr << "call failed" << std::endl; + } + } else { + if (PyErr_Occurred()) { + PyErr_Print(); + std::cerr << "failed to find cuntion" << std::endl; + } + } + + Py_XDECREF(pFunc); + Py_DECREF(pModule); + } else { + PyErr_Print(); + std::cerr << "failed to load: " << pModule << "," << pFunc << std::endl; + } + + if (Py_FinalizeEx() < 0) { + std::cerr << "finalize failed" << std::endl; + } + + std::cerr << "finish" << std::endl; } + + virtual void initialize(){}; + Envoy::Event::RealTimeSystem time_system_; Envoy::Network::Address::IpVersion version_; const std::string config_; + std::thread python_thread_; + int server_port_{0}; + int server_pid_{0}; }; class BenchmarkClientTestBase : public PythonIntegrationTestBase, @@ -78,9 +148,16 @@ class BenchmarkClientTestBase : public PythonIntegrationTestBase, void SetUp() override { ares_library_init(ARES_LIB_INIT_ALL); Envoy::Event::Libevent::Global::initialize(); + python_thread_ = std::thread([this]() { runPython(); }); + sleep(1); } void TearDown() override { + std::cerr << "prejoin " << server_pid_ << std::endl; + kill(server_pid_, SIGTERM); + sleep(1); + python_thread_.join(); + std::cerr << "postjoin" << std::endl; if (client_ != nullptr) { client_->terminate(); } @@ -89,9 +166,10 @@ class BenchmarkClientTestBase : public PythonIntegrationTestBase, } uint32_t getTestServerHostAndPort() { - ASSERT(false); - return 0; - /* return lookupPort("listener_0");*/ + while (server_port_ == 0) { + usleep(1000); + } + return server_port_; } void testBasicFunctionality(absl::string_view uriPath, const uint64_t max_pending, @@ -168,7 +246,10 @@ class BenchmarkClientTestBase : public PythonIntegrationTestBase, class BenchmarkClientHttpTest : public BenchmarkClientTestBase { public: - void SetUp() override { BenchmarkClientHttpTest::initialize(); } + void SetUp() override { + BenchmarkClientTestBase::SetUp(); + BenchmarkClientHttpTest::initialize(); + } void setupBenchmarkClient(absl::string_view uriPath, bool use_h2, bool prefetch_connections) override { @@ -178,7 +259,11 @@ class BenchmarkClientHttpTest : public BenchmarkClientTestBase { class BenchmarkClientHttpsTest : public BenchmarkClientTestBase { public: - void SetUp() override { BenchmarkClientHttpsTest::initialize(); } + void SetUp() override { + BenchmarkClientTestBase::SetUp(); + + BenchmarkClientHttpsTest::initialize(); + } void setupBenchmarkClient(absl::string_view uriPath, bool use_h2, bool prefetch_connections) override { @@ -319,8 +404,7 @@ TEST_P(BenchmarkClientHttpTest, DISABLED_WeirdStatus) { TEST_P(BenchmarkClientHttpTest, H1ConnectionFailure) { // Kill the test server, so we can't connect. // We allow a single connection and no pending. We expect one connection failure. - ASSERT(false); - // test_server_.reset(); + kill(server_pid_, SIGTERM); testBasicFunctionality("/lorem-ipsum-status-200", 1, 1, false, 10); @@ -337,8 +421,7 @@ TEST_P(BenchmarkClientHttpTest, H1ConnectionFailure) { TEST_P(BenchmarkClientHttpTest, H1MultiConnectionFailure) { // Kill the test server, so we can't connect. // We allow ten connections and ten pending requests. We expect ten connection failures. - ASSERT(false); - // test_server_.reset(); + kill(server_pid_, SIGTERM); testBasicFunctionality("/lorem-ipsum-status-200", 10, 10, false, 10); EXPECT_EQ(10, getCounter("upstream_cx_connect_fail")); From 9dd32b4d593e61391e1b5a7b54114148b8f92d71 Mon Sep 17 00:00:00 2001 From: Otto van der Schaaf Date: Wed, 24 Jul 2019 17:53:04 +0200 Subject: [PATCH 08/31] save state Signed-off-by: Otto van der Schaaf --- integration/cpp_benchmark_client_server.py | 55 ++--- integration/integration_test_fixtures.py | 6 +- integration/nighthawk_test_server.py | 2 +- test/BUILD | 1 + test/benchmark_http_client_test.cc | 224 +++++++----------- .../benchmark_http_client_test_envoy.yaml | 18 +- .../benchmark_https_client_test_envoy.yaml | 50 ++++ 7 files changed, 174 insertions(+), 182 deletions(-) create mode 100644 test/test_data/benchmark_https_client_test_envoy.yaml diff --git a/integration/cpp_benchmark_client_server.py b/integration/cpp_benchmark_client_server.py index 0e8bf6574..7d718ba3f 100644 --- a/integration/cpp_benchmark_client_server.py +++ b/integration/cpp_benchmark_client_server.py @@ -10,47 +10,44 @@ import cpp_benchmark_client_server from common import IpVersion, NighthawkException -from integration_test import determineIpVersionsFromEnvironment from integration_test_fixtures import (HttpIntegrationTestBase, HttpsIntegrationTestBase, IntegrationTestBase) assert sys.version_info >= (3, 0) -def runTests(ip_version): - IntegrationTestBase.ip_version = ip_version - logging.info("Running %s integration tests." % - ("ipv6" if IntegrationTestBase.ip_version == IpVersion.IPV6 else "ipv4")) - res = unittest.TextTestRunner(verbosity=2).run( - unittest.TestLoader().loadTestsFromModule(cpp_benchmark_client_server)) - return True - httpbase = None -def serverStartHook(): +def serverStartHook(ip_version, is_https): + IntegrationTestBase.ip_version = ip_version logging.basicConfig(stream=sys.stderr, level=logging.DEBUG) - ip_versions = determineIpVersionsFromEnvironment() - #ok = all(map(runTests, ip_versions)) + global httpbase - httpbase = TestHttp() + + if is_https: + httpbase = HttpsIntegrationTestBase() + httpbase.overrideTestServerConfigPath("test/test_data/benchmark_https_client_test_envoy.yaml") + else: + httpbase = HttpIntegrationTestBase() + httpbase.overrideTestServerConfigPath("test/test_data/benchmark_http_client_test_envoy.yaml") + httpbase.setUp() - print("@@@@@@@@@@@@ " + str(httpbase.admin_port), file=sys.stderr) - #httpbase.test_run_h1_server_for_cpp_benchmark_client_test() return httpbase.server_port +def serverStartHookHttpIPV4(): + return serverStartHook(IpVersion.IPV4, False) + +def serverStartHookHttpIPV6(): + return serverStartHook(IpVersion.IPV6, False) + +def serverStartHookHttpsIPV4(): + return serverStartHook(IpVersion.IPV4, True) + +def serverStartHookHttpsIPV6(): + return serverStartHook(IpVersion.IPV6, True) + def getRunningServerPid(): return httpbase.getServerPid() -class TestHttp(HttpIntegrationTestBase): - - def test_run_h1_server_for_cpp_benchmark_client_test(self): - """ - Runs the CLI configured to use plain HTTP/1 against our test server, and sanity - checks statistics from both client and server. - """ - #globals["server_pid"] = self.getServerPid() - #globals["server_port"] = self.server_port - #globals["admin_port"] = self.admin_port - #print(self.server_port, file=sys.stderr) - #print(self.admin_port, file=sys.stderr) - #sys.stderr.flush() - #self.waitForExit() +def waitForExit(): + return httpbase.waitForServerExit() + diff --git a/integration/integration_test_fixtures.py b/integration/integration_test_fixtures.py index fd91d703a..6c9eca7b4 100644 --- a/integration/integration_test_fixtures.py +++ b/integration/integration_test_fixtures.py @@ -35,6 +35,10 @@ def __init__(self, *args, **kwargs): self.server_port = -1 self.admin_port = -1 self.parameters = {} + self.parameters["test_rundir"] = self.test_rundir + + def overrideTestServerConfigPath(self, config_path): + self.nighthawk_test_config_path = os.path.join(self.test_rundir, config_path) # TODO(oschaaf): For the NH test server, add a way to let it determine a port by itself and pull that # out. @@ -70,7 +74,7 @@ def tearDown(self): self.assertEqual(0, self.test_server.stop()) def waitForServerExit(self): - self.assertEqual(0, self.test_server.waitForExit()) + return self.test_server.waitForExit() def getServerPid(self): return self.test_server.getPid() diff --git a/integration/nighthawk_test_server.py b/integration/nighthawk_test_server.py index 8ee8cc8db..d1ef67190 100644 --- a/integration/nighthawk_test_server.py +++ b/integration/nighthawk_test_server.py @@ -43,7 +43,7 @@ def serverThreadRunner(self): parameterized_config_path = tmp.name tmp.write(config) - args = [self.server_binary_path, self.server_binary_config_path_arg, parameterized_config_path] + args = [self.server_binary_path, self.server_binary_config_path_arg, parameterized_config_path, "--base-id", str(self.server_port)] logging.info("Test server popen() args: [%s]" % args) self.server_process = subprocess.Popen(args) self.server_process.communicate() diff --git a/test/BUILD b/test/BUILD index 13e7223da..813f96c72 100644 --- a/test/BUILD +++ b/test/BUILD @@ -44,6 +44,7 @@ envoy_cc_test( ], data = [ "test_data/benchmark_http_client_test_envoy.yaml", + "test_data/benchmark_https_client_test_envoy.yaml", "test_data/hdr_proto_json.gold", "test_data/lorem_ipsum.txt", "test_data/output_collector.json.gold", diff --git a/test/benchmark_http_client_test.cc b/test/benchmark_http_client_test.cc index bde02c994..be4b0c32f 100644 --- a/test/benchmark_http_client_test.cc +++ b/test/benchmark_http_client_test.cc @@ -40,92 +40,41 @@ using namespace testing; namespace Nighthawk { +using PyObjectPtr = std::unique_ptr>; + class PythonIntegrationTestBase { + public: virtual ~PythonIntegrationTestBase() = default; - PythonIntegrationTestBase(Envoy::Network::Address::IpVersion version, - const std::string& config = Envoy::ConfigHelper::HTTP_PROXY_CONFIG) - : version_(version), config_(config){}; - - void runPython() { - // std::string cmd = "PYTHONPATH=\"" + Envoy::TestEnvironment::runfilesPath("integration/") + - // "\""; RELEASE_ASSERT(::system(cmd.c_str()) == 0, "Failed to set pythonpath env var"); - // std::cerr << cmd << std::endl; - Py_Initialize(); - PyRun_SimpleString("import sys"); - PyRun_SimpleString( - ("sys.path.append(\"" + Envoy::TestEnvironment::runfilesPath("integration/") + "\")") - .c_str()); + PythonIntegrationTestBase(Envoy::Network::Address::IpVersion version) : version_(version){}; - PyObject* pName = PyUnicode_DecodeFSDefault("cpp_benchmark_client_server"); - PyObject* pModule = PyImport_Import(pName); - Py_DECREF(pName); - PyObject* pValue = nullptr; - PyObject* pFunc = nullptr; - - if (pModule != NULL) { - pFunc = PyObject_GetAttrString(pModule, "serverStartHook"); - if (pFunc && PyCallable_Check(pFunc)) { - pValue = PyObject_CallObject(pFunc, nullptr); - if (pValue != NULL) { - server_port_ = PyLong_AsLong(pValue); - RELEASE_ASSERT(server_port_ > 0, "bad server port"); - std::cerr << "@@@ port: " << server_port_ << std::endl; - Py_DECREF(pValue); - } else { - Py_DECREF(pFunc); - Py_DECREF(pModule); - PyErr_Print(); - std::cerr << "call failed" << std::endl; - } - } else { - if (PyErr_Occurred()) { - PyErr_Print(); - std::cerr << "failed to find cuntion" << std::endl; - } + void pythonAssert(bool condition, absl::string_view message) { + if (!condition) { + if (PyErr_Occurred()) { + PyErr_Print(); + std::cerr.flush(); } - - pFunc = PyObject_GetAttrString(pModule, "getRunningServerPid"); - if (pFunc && PyCallable_Check(pFunc)) { - pValue = PyObject_CallObject(pFunc, nullptr); - if (pValue != NULL) { - server_pid_ = PyLong_AsLong(pValue); - RELEASE_ASSERT(server_pid_ > 0, "bad server pid"); - std::cerr << "@@@ port: " << server_pid_ << std::endl; - Py_DECREF(pValue); - } else { - Py_DECREF(pFunc); - Py_DECREF(pModule); - PyErr_Print(); - std::cerr << "call failed" << std::endl; - } - } else { - if (PyErr_Occurred()) { - PyErr_Print(); - std::cerr << "failed to find cuntion" << std::endl; - } - } - - Py_XDECREF(pFunc); - Py_DECREF(pModule); - } else { - PyErr_Print(); - std::cerr << "failed to load: " << pModule << "," << pFunc << std::endl; + RELEASE_ASSERT(false, std::string(message).c_str()); } - - if (Py_FinalizeEx() < 0) { - std::cerr << "finalize failed" << std::endl; - } - - std::cerr << "finish" << std::endl; } - virtual void initialize(){}; + int getIntValueFromPythonCall(const PyObjectPtr& module, absl::string_view method_name) { + PyObjectPtr value(nullptr, py_object_deleter_); + PyObjectPtr func(nullptr, py_object_deleter_); + func.reset(PyObject_GetAttrString(module.get(), std::string(method_name).c_str())); + pythonAssert(func != nullptr, "couldn't find function"); + pythonAssert(PyCallable_Check(func.get()), "function not callable"); + value.reset(PyObject_CallObject(func.get(), nullptr)); + pythonAssert(value != nullptr, "call failed"); + int return_value = PyLong_AsLong(value.get()); + pythonAssert(PyErr_Occurred() == nullptr && value > 0, "bad value"); + return return_value; + } + std::function py_object_deleter_ = [](PyObject* p) { Py_DECREF(p); }; Envoy::Event::RealTimeSystem time_system_; Envoy::Network::Address::IpVersion version_; - const std::string config_; - std::thread python_thread_; + PyObjectPtr module_; int server_port_{0}; int server_pid_{0}; }; @@ -134,30 +83,23 @@ class BenchmarkClientTestBase : public PythonIntegrationTestBase, public TestWithParam { public: BenchmarkClientTestBase() - : PythonIntegrationTestBase(GetParam(), BenchmarkClientTestBase::envoy_config), + : PythonIntegrationTestBase(GetParam()), api_(thread_factory_, store_, time_system_, file_system_), dispatcher_(api_.allocateDispatcher()) {} - static void SetUpTestCase() { - Envoy::Filesystem::InstanceImplPosix file_system; - envoy_config = file_system.fileReadToEnd(Envoy::TestEnvironment::runfilesPath( - "test/test_data/benchmark_http_client_test_envoy.yaml")); - envoy_config = Envoy::TestEnvironment::substitute(envoy_config); - } - void SetUp() override { ares_library_init(ARES_LIB_INIT_ALL); Envoy::Event::Libevent::Global::initialize(); - python_thread_ = std::thread([this]() { runPython(); }); - sleep(1); } void TearDown() override { - std::cerr << "prejoin " << server_pid_ << std::endl; - kill(server_pid_, SIGTERM); - sleep(1); - python_thread_.join(); - std::cerr << "postjoin" << std::endl; + if (server_pid_ != 0) { + kill(server_pid_, SIGTERM); + int exit_val = getIntValueFromPythonCall(module_, "waitForExit"); + RELEASE_ASSERT(exit_val == 0, "test server error on exit"); + module_.release(); + pythonAssert(Py_FinalizeEx() == 0, "finalize failed"); + } if (client_ != nullptr) { client_->terminate(); } @@ -165,12 +107,7 @@ class BenchmarkClientTestBase : public PythonIntegrationTestBase, ares_library_cleanup(); } - uint32_t getTestServerHostAndPort() { - while (server_port_ == 0) { - usleep(1000); - } - return server_port_; - } + uint32_t getTestServerHostAndPort() { return server_port_; } void testBasicFunctionality(absl::string_view uriPath, const uint64_t max_pending, const uint64_t connection_limit, const bool use_h2, @@ -241,14 +178,31 @@ class BenchmarkClientTestBase : public PythonIntegrationTestBase, NiceMock runtime_; std::unique_ptr client_; Envoy::Filesystem::InstanceImplPosix file_system_; - static std::string envoy_config; }; class BenchmarkClientHttpTest : public BenchmarkClientTestBase { public: void SetUp() override { BenchmarkClientTestBase::SetUp(); - BenchmarkClientHttpTest::initialize(); + Py_Initialize(); + PyRun_SimpleString("import sys"); + PyRun_SimpleString( + ("sys.path.append(\"" + Envoy::TestEnvironment::runfilesPath("integration/") + "\")") + .c_str()); + + PyObjectPtr pName(nullptr, py_object_deleter_); + + pName.reset(PyUnicode_DecodeFSDefault("cpp_benchmark_client_server")); + module_.reset(PyImport_Import(pName.get())); + pythonAssert(module_ != nullptr, "failed to load"); + + if (GetParam() == Envoy::Network::Address::IpVersion::v4) { + server_port_ = getIntValueFromPythonCall(module_, "serverStartHookHttpIPV4"); + } else { + server_port_ = getIntValueFromPythonCall(module_, "serverStartHookHttpIPV6"); + } + + server_pid_ = getIntValueFromPythonCall(module_, "getRunningServerPid"); } void setupBenchmarkClient(absl::string_view uriPath, bool use_h2, @@ -262,47 +216,33 @@ class BenchmarkClientHttpsTest : public BenchmarkClientTestBase { void SetUp() override { BenchmarkClientTestBase::SetUp(); - BenchmarkClientHttpsTest::initialize(); + Py_Initialize(); + PyRun_SimpleString("import sys"); + PyRun_SimpleString( + ("sys.path.append(\"" + Envoy::TestEnvironment::runfilesPath("integration/") + "\")") + .c_str()); + + PyObjectPtr pName(nullptr, py_object_deleter_); + + pName.reset(PyUnicode_DecodeFSDefault("cpp_benchmark_client_server")); + module_.reset(PyImport_Import(pName.get())); + pythonAssert(module_ != nullptr, "failed to load"); + + if (GetParam() == Envoy::Network::Address::IpVersion::v4) { + server_port_ = getIntValueFromPythonCall(module_, "serverStartHookHttpsIPV4"); + } else { + server_port_ = getIntValueFromPythonCall(module_, "serverStartHookHttpsIPV6"); + } + + server_pid_ = getIntValueFromPythonCall(module_, "getRunningServerPid"); } void setupBenchmarkClient(absl::string_view uriPath, bool use_h2, bool prefetch_connections) override { doSetupBenchmarkClient(uriPath, true, use_h2, prefetch_connections); }; - - void initialize() override { - /* - config_helper_.addConfigModifier([](envoy::config::bootstrap::v2::Bootstrap& bootstrap) { - auto* common_tls_context = bootstrap.mutable_static_resources() - ->mutable_listeners(0) - ->mutable_filter_chains(0) - ->mutable_tls_context() - ->mutable_common_tls_context(); - common_tls_context->mutable_validation_context_sds_secret_config()->set_name( - "validation_context"); - common_tls_context->add_tls_certificate_sds_secret_configs()->set_name("server_cert"); - - auto* secret = bootstrap.mutable_static_resources()->add_secrets(); - secret->set_name("validation_context"); - auto* validation_context = secret->mutable_validation_context(); - validation_context->mutable_trusted_ca()->set_filename(Envoy::TestEnvironment::runfilesPath( - "external/envoy/test/config/integration/certs/cacert.pem")); - secret = bootstrap.mutable_static_resources()->add_secrets(); - secret->set_name("server_cert"); - auto* tls_certificate = secret->mutable_tls_certificate(); - tls_certificate->mutable_certificate_chain()->set_filename( - Envoy::TestEnvironment::runfilesPath( - "external/envoy/test/config/integration/certs/servercert.pem")); - tls_certificate->mutable_private_key()->set_filename(Envoy::TestEnvironment::runfilesPath( - "external/envoy/test/config/integration/certs/serverkey.pem")); - }); - */ - BenchmarkClientTestBase::initialize(); - } }; -std::string BenchmarkClientTestBase::envoy_config; - INSTANTIATE_TEST_SUITE_P(IpVersions, BenchmarkClientHttpTest, ValuesIn(Envoy::TestEnvironment::getIpVersionsForTest()), Envoy::TestUtility::ipTestParamsToString); @@ -314,7 +254,7 @@ TEST_P(BenchmarkClientHttpTest, BasicTestH1) { testBasicFunctionality("/lorem-ipsum-status-200", 1, 1, false, 10); EXPECT_EQ(1, getCounter("upstream_cx_http1_total")); - EXPECT_LE(3621, getCounter("upstream_cx_rx_bytes_total")); + // EXPECT_LE(3621, getCounter("upstream_cx_rx_bytes_total")); EXPECT_EQ(1, getCounter("upstream_cx_total")); EXPECT_LE(78, getCounter("upstream_cx_tx_bytes_total")); EXPECT_EQ(1, getCounter("upstream_rq_pending_total")); @@ -401,11 +341,19 @@ TEST_P(BenchmarkClientHttpTest, DISABLED_WeirdStatus) { EXPECT_EQ(7, nonZeroValuedCounterCount()); } -TEST_P(BenchmarkClientHttpTest, H1ConnectionFailure) { - // Kill the test server, so we can't connect. - // We allow a single connection and no pending. We expect one connection failure. - kill(server_pid_, SIGTERM); +class BenchmarkClientNoServerTest : public BenchmarkClientTestBase { +public: + void SetUp() override {} + void TearDown() override {} + void setupBenchmarkClient(absl::string_view uriPath, bool use_h2, + bool prefetch_connections) override { + doSetupBenchmarkClient(uriPath, false, use_h2, prefetch_connections); + }; +}; + +TEST_P(BenchmarkClientNoServerTest, H1ConnectionFailure) { + // We allow a single connection and no pending. We expect one connection failure. testBasicFunctionality("/lorem-ipsum-status-200", 1, 1, false, 10); EXPECT_EQ(1, getCounter("upstream_cx_connect_fail")); @@ -418,10 +366,8 @@ TEST_P(BenchmarkClientHttpTest, H1ConnectionFailure) { EXPECT_EQ(8, nonZeroValuedCounterCount()); } -TEST_P(BenchmarkClientHttpTest, H1MultiConnectionFailure) { - // Kill the test server, so we can't connect. +TEST_P(BenchmarkClientNoServerTest, H1MultiConnectionFailure) { // We allow ten connections and ten pending requests. We expect ten connection failures. - kill(server_pid_, SIGTERM); testBasicFunctionality("/lorem-ipsum-status-200", 10, 10, false, 10); EXPECT_EQ(10, getCounter("upstream_cx_connect_fail")); diff --git a/test/test_data/benchmark_http_client_test_envoy.yaml b/test/test_data/benchmark_http_client_test_envoy.yaml index bab05ffc9..1e777a824 100644 --- a/test/test_data/benchmark_http_client_test_envoy.yaml +++ b/test/test_data/benchmark_http_client_test_envoy.yaml @@ -2,21 +2,15 @@ admin: access_log_path: /dev/null address: socket_address: - address: {{ ip_loopback_address }} - port_value: 0 + address: $server_ip + port_value: $admin_port static_resources: - clusters: - name: cluster_0 - hosts: - socket_address: - address: {{ ip_loopback_address }} - port_value: 0 listeners: - name: listener_0 address: socket_address: - address: {{ ip_loopback_address }} - port_value: 0 + address: $server_ip + port_value: $server_port filter_chains: - filters: - name: envoy.http_connection_manager @@ -36,13 +30,13 @@ static_resources: direct_response: status: 200 body: - filename: {{ test_rundir }}/test/test_data/lorem_ipsum.txt + filename: $test_rundir/test/test_data/lorem_ipsum.txt - match: prefix: /404 direct_response: status: 404 body: - filename: {{ test_rundir }}/test/test_data/lorem_ipsum.txt + filename: $test_rundir/test/test_data/lorem_ipsum.txt http_filters: - name: envoy.router config: diff --git a/test/test_data/benchmark_https_client_test_envoy.yaml b/test/test_data/benchmark_https_client_test_envoy.yaml new file mode 100644 index 000000000..7ae59065b --- /dev/null +++ b/test/test_data/benchmark_https_client_test_envoy.yaml @@ -0,0 +1,50 @@ +admin: + access_log_path: /dev/null + address: + socket_address: + address: $server_ip + port_value: $admin_port +static_resources: + listeners: + - name: listener_0 + address: + socket_address: + address: $server_ip + port_value: $server_port + filter_chains: + - filters: + - name: envoy.http_connection_manager + config: + generate_request_id: false + codec_type: auto + stat_prefix: ingress_http + route_config: + name: local_route + virtual_hosts: + - name: service + domains: + - "*" + routes: + - match: + prefix: /lorem-ipsum-status-200 + direct_response: + status: 200 + body: + filename: $test_rundir/test/test_data/lorem_ipsum.txt + - match: + prefix: /404 + direct_response: + status: 404 + body: + filename: $test_rundir/test/test_data/lorem_ipsum.txt + http_filters: + - name: envoy.router + config: + dynamic_stats: false + tls_context: + common_tls_context: + tls_certificates: + - certificate_chain: + filename: $ssl_cert_path + private_key: + filename: $ssl_key_path \ No newline at end of file From b05575c5d120321c70add1abf42d83e4bb228a5a Mon Sep 17 00:00:00 2001 From: Otto van der Schaaf Date: Wed, 24 Jul 2019 18:16:10 +0200 Subject: [PATCH 09/31] Formatting fixes Signed-off-by: Otto van der Schaaf --- integration/BUILD | 4 +-- integration/cpp_benchmark_client_server.py | 8 +++++- integration/integration_test.py | 1 + integration/integration_test_fixtures.py | 2 +- integration/nighthawk_test_server.py | 6 +++- test/BUILD | 32 +++++++++++----------- test/benchmark_http_client_test.cc | 8 +++--- 7 files changed, 36 insertions(+), 25 deletions(-) diff --git a/integration/BUILD b/integration/BUILD index 444859b6b..3bb9a48aa 100644 --- a/integration/BUILD +++ b/integration/BUILD @@ -1,11 +1,11 @@ +licenses(["notice"]) # Apache 2 + load( "@envoy//bazel:envoy_build_system.bzl", "envoy_package", ) load("@python_pip_deps//:requirements.bzl", "requirement") -licenses(["notice"]) # Apache 2 - envoy_package() exports_files( diff --git a/integration/cpp_benchmark_client_server.py b/integration/cpp_benchmark_client_server.py index 7d718ba3f..28130bd75 100644 --- a/integration/cpp_benchmark_client_server.py +++ b/integration/cpp_benchmark_client_server.py @@ -17,6 +17,7 @@ httpbase = None + def serverStartHook(ip_version, is_https): IntegrationTestBase.ip_version = ip_version logging.basicConfig(stream=sys.stderr, level=logging.DEBUG) @@ -33,21 +34,26 @@ def serverStartHook(ip_version, is_https): httpbase.setUp() return httpbase.server_port + def serverStartHookHttpIPV4(): return serverStartHook(IpVersion.IPV4, False) + def serverStartHookHttpIPV6(): return serverStartHook(IpVersion.IPV6, False) + def serverStartHookHttpsIPV4(): return serverStartHook(IpVersion.IPV4, True) + def serverStartHookHttpsIPV6(): return serverStartHook(IpVersion.IPV6, True) + def getRunningServerPid(): return httpbase.getServerPid() + def waitForExit(): return httpbase.waitForServerExit() - diff --git a/integration/integration_test.py b/integration/integration_test.py index 3d50c7c85..9a5539b9f 100644 --- a/integration/integration_test.py +++ b/integration/integration_test.py @@ -36,6 +36,7 @@ def runTests(ip_version): unittest.TestLoader().loadTestsFromModule(test_integration_basics)) return res.wasSuccessful() + if __name__ == '__main__': logging.basicConfig(stream=sys.stderr, level=logging.DEBUG) ip_versions = determineIpVersionsFromEnvironment() diff --git a/integration/integration_test_fixtures.py b/integration/integration_test_fixtures.py index 6c9eca7b4..5f85493e1 100644 --- a/integration/integration_test_fixtures.py +++ b/integration/integration_test_fixtures.py @@ -131,7 +131,7 @@ def runNighthawkClient(self, args, expect_failure=False, timeout=30): If the timeout is exceeded an exception will be raised. """ - self.assertTrue(os.path.exists(self.nighthawk_client_path)) + self.assertTrue(os.path.exists(self.nighthawk_client_path)) if IntegrationTestBase.ip_version == IpVersion.IPV6: args.insert(0, "--address-family v6") args.insert(0, "--output-format json") diff --git a/integration/nighthawk_test_server.py b/integration/nighthawk_test_server.py index d1ef67190..bf8f9c16c 100644 --- a/integration/nighthawk_test_server.py +++ b/integration/nighthawk_test_server.py @@ -43,7 +43,11 @@ def serverThreadRunner(self): parameterized_config_path = tmp.name tmp.write(config) - args = [self.server_binary_path, self.server_binary_config_path_arg, parameterized_config_path, "--base-id", str(self.server_port)] + args = [ + self.server_binary_path, self.server_binary_config_path_arg, parameterized_config_path, + "--base-id", + str(self.server_port) + ] logging.info("Test server popen() args: [%s]" % args) self.server_process = subprocess.Popen(args) self.server_process.communicate() diff --git a/test/BUILD b/test/BUILD index 813f96c72..7bdb66301 100644 --- a/test/BUILD +++ b/test/BUILD @@ -25,22 +25,22 @@ envoy_cc_test( name = "nighthawk_test", srcs = [ "benchmark_http_client_test.cc", - #"client_test.cc", - #"client_worker_test.cc", - #"factories_test.cc", - #"frequency_test.cc", - #"options_test.cc", - #"output_collector_test.cc", - #"platform_util_test.cc", - #"process_test.cc", - #"rate_limiter_test.cc", - #"sequencer_test.cc", - #"service_main_test.cc", - #"service_test.cc", - #"statistic_test.cc", - #"stream_decoder_test.cc", - #"utility_test.cc", - #"worker_test.cc", + "client_test.cc", + "client_worker_test.cc", + "factories_test.cc", + "frequency_test.cc", + "options_test.cc", + "output_collector_test.cc", + "platform_util_test.cc", + "process_test.cc", + "rate_limiter_test.cc", + "sequencer_test.cc", + "service_main_test.cc", + "service_test.cc", + "statistic_test.cc", + "stream_decoder_test.cc", + "utility_test.cc", + "worker_test.cc", ], data = [ "test_data/benchmark_http_client_test_envoy.yaml", diff --git a/test/benchmark_http_client_test.cc b/test/benchmark_http_client_test.cc index be4b0c32f..c5c6e1d97 100644 --- a/test/benchmark_http_client_test.cc +++ b/test/benchmark_http_client_test.cc @@ -1,3 +1,5 @@ +#include + #include #include "envoy/thread_local/thread_local.h" @@ -33,8 +35,6 @@ #include "ares.h" #include "gtest/gtest.h" -#include - using namespace std::chrono_literals; using namespace testing; @@ -72,7 +72,7 @@ class PythonIntegrationTestBase { } std::function py_object_deleter_ = [](PyObject* p) { Py_DECREF(p); }; - Envoy::Event::RealTimeSystem time_system_; + Envoy::Event::RealTimeSystem time_system_; // NO_CHECK_FORMAT(real_time) Envoy::Network::Address::IpVersion version_; PyObjectPtr module_; int server_port_{0}; @@ -254,7 +254,7 @@ TEST_P(BenchmarkClientHttpTest, BasicTestH1) { testBasicFunctionality("/lorem-ipsum-status-200", 1, 1, false, 10); EXPECT_EQ(1, getCounter("upstream_cx_http1_total")); - // EXPECT_LE(3621, getCounter("upstream_cx_rx_bytes_total")); + EXPECT_LE(3621, getCounter("upstream_cx_rx_bytes_total")); EXPECT_EQ(1, getCounter("upstream_cx_total")); EXPECT_LE(78, getCounter("upstream_cx_tx_bytes_total")); EXPECT_EQ(1, getCounter("upstream_rq_pending_total")); From 4cc57bf8d7c8f341fb6ace9a7bc9c6b286494617 Mon Sep 17 00:00:00 2001 From: Otto van der Schaaf Date: Wed, 24 Jul 2019 18:17:26 +0200 Subject: [PATCH 10/31] accidental whitespace Signed-off-by: Otto van der Schaaf --- integration/integration_test.py | 1 + 1 file changed, 1 insertion(+) diff --git a/integration/integration_test.py b/integration/integration_test.py index 9a5539b9f..46aa051e0 100644 --- a/integration/integration_test.py +++ b/integration/integration_test.py @@ -37,6 +37,7 @@ def runTests(ip_version): return res.wasSuccessful() + if __name__ == '__main__': logging.basicConfig(stream=sys.stderr, level=logging.DEBUG) ip_versions = determineIpVersionsFromEnvironment() From b84f137998f375d7b3271086b252bfe20d90773e Mon Sep 17 00:00:00 2001 From: Otto van der Schaaf Date: Wed, 24 Jul 2019 18:18:04 +0200 Subject: [PATCH 11/31] revert whitespace Signed-off-by: Otto van der Schaaf --- integration/integration_test.py | 1 - 1 file changed, 1 deletion(-) diff --git a/integration/integration_test.py b/integration/integration_test.py index 46aa051e0..9a5539b9f 100644 --- a/integration/integration_test.py +++ b/integration/integration_test.py @@ -37,7 +37,6 @@ def runTests(ip_version): return res.wasSuccessful() - if __name__ == '__main__': logging.basicConfig(stream=sys.stderr, level=logging.DEBUG) ip_versions = determineIpVersionsFromEnvironment() From 00fe42dcb775cde9abf3cf5a946d1bca7a905bb8 Mon Sep 17 00:00:00 2001 From: Otto van der Schaaf Date: Wed, 24 Jul 2019 20:26:52 +0200 Subject: [PATCH 12/31] Rip out the python c-api - It caused x-version trouble. - gperftools reported leaks , seemingly unfixable - gcovr ran into trouble In short, probably more trouble then it's worth, though it would have been a nice alternative to the pipe IPC we rely on with this change. Signed-off-by: Otto van der Schaaf --- WORKSPACE | 14 --- integration/cpp_benchmark_client_server.py | 35 +++--- test/BUILD | 37 +++--- test/benchmark_http_client_test.cc | 124 ++++++++------------- 4 files changed, 82 insertions(+), 128 deletions(-) diff --git a/WORKSPACE b/WORKSPACE index 74fa4edb8..e84fafdec 100644 --- a/WORKSPACE +++ b/WORKSPACE @@ -22,20 +22,6 @@ go_rules_dependencies() go_register_toolchains(go_version = GO_VERSION) -new_local_repository( - name = "python_linux", - build_file_content = """ -cc_library( - name = "python36-lib", - srcs = ["lib/python3.6/config-3.6m-x86_64-linux-gnu/libpython3.6.so"], - hdrs = glob(["include/python3.6/*.h"]), - includes = ["include/python3.6"], - visibility = ["//visibility:public"] -) - """, - path = "/usr", -) - # For PIP support: load("@io_bazel_rules_python//python:pip.bzl", "pip_import", "pip_repositories") diff --git a/integration/cpp_benchmark_client_server.py b/integration/cpp_benchmark_client_server.py index 28130bd75..5ff8a27b4 100644 --- a/integration/cpp_benchmark_client_server.py +++ b/integration/cpp_benchmark_client_server.py @@ -7,7 +7,6 @@ import os import sys import unittest -import cpp_benchmark_client_server from common import IpVersion, NighthawkException from integration_test_fixtures import (HttpIntegrationTestBase, HttpsIntegrationTestBase, @@ -21,7 +20,6 @@ def serverStartHook(ip_version, is_https): IntegrationTestBase.ip_version = ip_version logging.basicConfig(stream=sys.stderr, level=logging.DEBUG) - global httpbase if is_https: @@ -35,25 +33,26 @@ def serverStartHook(ip_version, is_https): return httpbase.server_port -def serverStartHookHttpIPV4(): - return serverStartHook(IpVersion.IPV4, False) - - -def serverStartHookHttpIPV6(): - return serverStartHook(IpVersion.IPV6, False) - - -def serverStartHookHttpsIPV4(): - return serverStartHook(IpVersion.IPV4, True) - - -def serverStartHookHttpsIPV6(): - return serverStartHook(IpVersion.IPV6, True) - - def getRunningServerPid(): return httpbase.getServerPid() def waitForExit(): return httpbase.waitForServerExit() + + +def main(): + if len(sys.argv) != 3: + print("cpp_benchmark_client_server.py [ipv4|ipv6] [http|https]") + return -1 + port = serverStartHook( + IpVersion.IPV6 if str.lower(sys.argv[1]) == "ipv6" else IpVersion.IPV4, + str.lower(sys.argv[2]) == "https") + print(str(port)) + print(str(getRunningServerPid())) + sys.stdout.flush() + return waitForExit() + + +if __name__ == '__main__': + main() diff --git a/test/BUILD b/test/BUILD index 7bdb66301..45335dac0 100644 --- a/test/BUILD +++ b/test/BUILD @@ -1,3 +1,5 @@ +licenses(["notice"]) # Apache 2 + load( "@envoy//bazel:envoy_build_system.bzl", "envoy_cc_mock", @@ -5,8 +7,6 @@ load( "envoy_package", ) -licenses(["notice"]) # Apache 2 - envoy_package() envoy_cc_mock( @@ -25,22 +25,22 @@ envoy_cc_test( name = "nighthawk_test", srcs = [ "benchmark_http_client_test.cc", - "client_test.cc", - "client_worker_test.cc", - "factories_test.cc", - "frequency_test.cc", - "options_test.cc", - "output_collector_test.cc", - "platform_util_test.cc", - "process_test.cc", - "rate_limiter_test.cc", - "sequencer_test.cc", - "service_main_test.cc", - "service_test.cc", - "statistic_test.cc", - "stream_decoder_test.cc", - "utility_test.cc", - "worker_test.cc", + #"client_test.cc", + #"client_worker_test.cc", + #"factories_test.cc", + #"frequency_test.cc", + #"options_test.cc", + #"output_collector_test.cc", + #"platform_util_test.cc", + #"process_test.cc", + #"rate_limiter_test.cc", + #"sequencer_test.cc", + #"service_main_test.cc", + #"service_test.cc", + #"statistic_test.cc", + #"stream_decoder_test.cc", + #"utility_test.cc", + # "worker_test.cc", ], data = [ "test_data/benchmark_http_client_test_envoy.yaml", @@ -79,6 +79,5 @@ envoy_cc_test( "@envoy//test/mocks/thread_local:thread_local_mocks", "@envoy//test/server:utility_lib", "@envoy//test/test_common:simulated_time_system_lib", - "@python_linux//:python36-lib", ], ) diff --git a/test/benchmark_http_client_test.cc b/test/benchmark_http_client_test.cc index c5c6e1d97..1c4905a39 100644 --- a/test/benchmark_http_client_test.cc +++ b/test/benchmark_http_client_test.cc @@ -1,5 +1,3 @@ -#include - #include #include "envoy/thread_local/thread_local.h" @@ -40,47 +38,62 @@ using namespace testing; namespace Nighthawk { -using PyObjectPtr = std::unique_ptr>; - -class PythonIntegrationTestBase { - +class PythonIntegrationTestBase : public TestWithParam { public: virtual ~PythonIntegrationTestBase() = default; - PythonIntegrationTestBase(Envoy::Network::Address::IpVersion version) : version_(version){}; + PythonIntegrationTestBase(Envoy::Network::Address::IpVersion version) + : version_(version), pipe_(nullptr){}; - void pythonAssert(bool condition, absl::string_view message) { - if (!condition) { - if (PyErr_Occurred()) { - PyErr_Print(); - std::cerr.flush(); - } - RELEASE_ASSERT(false, std::string(message).c_str()); + void startPythonIntegrationWrapper(bool use_https) { + std::string script = + Envoy::TestEnvironment::runfilesPath("integration/cpp_benchmark_client_server.py"); + + std::string args; + if (GetParam() == Envoy::Network::Address::IpVersion::v4) { + args.append(" ipv4"); + } else { + args.append(" ipv6"); } + if (use_https) { + args.append(" https"); + } else { + args.append(" http"); + } + script.append(args); + + char buffer[128]; + script = "python3 " + script; + pipe_ = popen(script.c_str(), "r"); + RELEASE_ASSERT(pipe_ != nullptr, "Failed to open pipe"); + RELEASE_ASSERT(fgets(buffer, sizeof(buffer), pipe_) != nullptr, "Expected more data"); + RELEASE_ASSERT(absl::SimpleAtoi(buffer, &server_port_), "couldn't understand server_pid"); + + RELEASE_ASSERT(fgets(buffer, sizeof(buffer), pipe_) != nullptr, "Expected more data"); + RELEASE_ASSERT(absl::SimpleAtoi(buffer, &server_pid_), "couldn't understand server_pid"); } - int getIntValueFromPythonCall(const PyObjectPtr& module, absl::string_view method_name) { - PyObjectPtr value(nullptr, py_object_deleter_); - PyObjectPtr func(nullptr, py_object_deleter_); - func.reset(PyObject_GetAttrString(module.get(), std::string(method_name).c_str())); - pythonAssert(func != nullptr, "couldn't find function"); - pythonAssert(PyCallable_Check(func.get()), "function not callable"); - value.reset(PyObject_CallObject(func.get(), nullptr)); - pythonAssert(value != nullptr, "call failed"); - int return_value = PyLong_AsLong(value.get()); - pythonAssert(PyErr_Occurred() == nullptr && value > 0, "bad value"); - return return_value; + void TearDown() { + if (server_pid_ != 0) { + kill(server_pid_, SIGTERM); + } + if (pipe_ != nullptr) { + char buffer[128]; + // We don't expect any output, lets print it when that happens + while (fgets(buffer, sizeof(buffer), pipe_) != nullptr) { + std::cerr << "python stdout: " << buffer << std::endl; + } + RELEASE_ASSERT(!pclose(pipe_), "Failure closing pipe"); + } } - std::function py_object_deleter_ = [](PyObject* p) { Py_DECREF(p); }; Envoy::Event::RealTimeSystem time_system_; // NO_CHECK_FORMAT(real_time) Envoy::Network::Address::IpVersion version_; - PyObjectPtr module_; int server_port_{0}; int server_pid_{0}; + FILE* pipe_; }; -class BenchmarkClientTestBase : public PythonIntegrationTestBase, - public TestWithParam { +class BenchmarkClientTestBase : public PythonIntegrationTestBase { public: BenchmarkClientTestBase() : PythonIntegrationTestBase(GetParam()), @@ -93,18 +106,12 @@ class BenchmarkClientTestBase : public PythonIntegrationTestBase, } void TearDown() override { - if (server_pid_ != 0) { - kill(server_pid_, SIGTERM); - int exit_val = getIntValueFromPythonCall(module_, "waitForExit"); - RELEASE_ASSERT(exit_val == 0, "test server error on exit"); - module_.release(); - pythonAssert(Py_FinalizeEx() == 0, "finalize failed"); - } + tls_.shutdownGlobalThreading(); + ares_library_cleanup(); if (client_ != nullptr) { client_->terminate(); } - tls_.shutdownGlobalThreading(); - ares_library_cleanup(); + PythonIntegrationTestBase::TearDown(); } uint32_t getTestServerHostAndPort() { return server_port_; } @@ -184,25 +191,7 @@ class BenchmarkClientHttpTest : public BenchmarkClientTestBase { public: void SetUp() override { BenchmarkClientTestBase::SetUp(); - Py_Initialize(); - PyRun_SimpleString("import sys"); - PyRun_SimpleString( - ("sys.path.append(\"" + Envoy::TestEnvironment::runfilesPath("integration/") + "\")") - .c_str()); - - PyObjectPtr pName(nullptr, py_object_deleter_); - - pName.reset(PyUnicode_DecodeFSDefault("cpp_benchmark_client_server")); - module_.reset(PyImport_Import(pName.get())); - pythonAssert(module_ != nullptr, "failed to load"); - - if (GetParam() == Envoy::Network::Address::IpVersion::v4) { - server_port_ = getIntValueFromPythonCall(module_, "serverStartHookHttpIPV4"); - } else { - server_port_ = getIntValueFromPythonCall(module_, "serverStartHookHttpIPV6"); - } - - server_pid_ = getIntValueFromPythonCall(module_, "getRunningServerPid"); + startPythonIntegrationWrapper(false); } void setupBenchmarkClient(absl::string_view uriPath, bool use_h2, @@ -215,26 +204,7 @@ class BenchmarkClientHttpsTest : public BenchmarkClientTestBase { public: void SetUp() override { BenchmarkClientTestBase::SetUp(); - - Py_Initialize(); - PyRun_SimpleString("import sys"); - PyRun_SimpleString( - ("sys.path.append(\"" + Envoy::TestEnvironment::runfilesPath("integration/") + "\")") - .c_str()); - - PyObjectPtr pName(nullptr, py_object_deleter_); - - pName.reset(PyUnicode_DecodeFSDefault("cpp_benchmark_client_server")); - module_.reset(PyImport_Import(pName.get())); - pythonAssert(module_ != nullptr, "failed to load"); - - if (GetParam() == Envoy::Network::Address::IpVersion::v4) { - server_port_ = getIntValueFromPythonCall(module_, "serverStartHookHttpsIPV4"); - } else { - server_port_ = getIntValueFromPythonCall(module_, "serverStartHookHttpsIPV6"); - } - - server_pid_ = getIntValueFromPythonCall(module_, "getRunningServerPid"); + startPythonIntegrationWrapper(true); } void setupBenchmarkClient(absl::string_view uriPath, bool use_h2, From b50fda5bf05325a0b3d210eec205e06b35704bff Mon Sep 17 00:00:00 2001 From: Otto van der Schaaf Date: Thu, 25 Jul 2019 14:35:18 +0200 Subject: [PATCH 13/31] Dep & CI update - Update CI image: 8246167b9d238797cbc6c03dccc9e3921c37617d - Update Envoy: bcc66c6b74c365d1d2834cfe15b847ae13be0eb6 Prelude to updating coverage testing with lcov Benchmark http client still needs to be fixed Signed-off-by: Otto van der Schaaf --- .circleci/config.yml | 2 +- .gitignore | 1 + bazel/repositories.bzl | 4 ++-- source/common/ssl.h | 6 ++++-- test/BUILD | 34 +++++++++++++++++----------------- 5 files changed, 25 insertions(+), 22 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index 94552fad6..4a102e52f 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -1,6 +1,6 @@ references: envoy-build-image: &envoy-build-image - envoyproxy/envoy-build:d0cefa7f071dbd4ef24399c2db8656c3a5d8c3ef + envoyproxy/envoy-build:8246167b9d238797cbc6c03dccc9e3921c37617d version: 2 jobs: diff --git a/.gitignore b/.gitignore index 8b53bf503..c3f6be666 100644 --- a/.gitignore +++ b/.gitignore @@ -9,6 +9,7 @@ generated/ .cmake .rnd .vscode/ +venv/* bazel.output.txt envoy/ *.pyc diff --git a/bazel/repositories.bzl b/bazel/repositories.bzl index 5f6edfa76..7bfed6468 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 = "bcc66c6b74c365d1d2834cfe15b847ae13be0eb6" -ENVOY_SHA = "4265c3820076f81cbc54ecd7a760ff7da5cf90ef6b49fc0b08b6137f5b54e44d" +ENVOY_COMMIT = "ae6bc0185c235a68d5c3845fd54daa270f210685" +ENVOY_SHA = "512714e27baec34fdb76e0ca5bd8e0e4614ba97bb3cf0bc94f0b79818bcfd788" RULES_PYTHON_COMMIT = "fdbb17a4118a1728d19e638a5291b4c4266ea5b8" RULES_PYTHON_SHA = "9a3d71e348da504a9c4c5e8abd4cb822f7afb32c613dc6ee8b8535333a81a938" diff --git a/source/common/ssl.h b/source/common/ssl.h index 2eff0bd21..7fd04c195 100644 --- a/source/common/ssl.h +++ b/source/common/ssl.h @@ -7,6 +7,7 @@ #include "common/secret/secret_manager_impl.h" +#include "server/http/config_tracker_impl.h" #include "server/transport_socket_config_impl.h" #include "extensions/transport_sockets/tls/context_config_impl.h" @@ -26,8 +27,8 @@ class MinimalTransportSocketFactoryContext Envoy::Extensions::TransportSockets::Tls::ContextManagerImpl& ssl_context_manager, Envoy::ProtobufMessage::ValidationVisitor& validation_visitor) : ssl_context_manager_(ssl_context_manager), stats_scope_(std::move(stats_scope)), - dispatcher_(dispatcher), random_(random), stats_(stats), api_(api), - validation_visitor_(validation_visitor) {} + secret_manager_(config_tracker_), dispatcher_(dispatcher), random_(random), stats_(stats), + api_(api), validation_visitor_(validation_visitor) {} Envoy::Server::Admin& admin() override { NOT_IMPLEMENTED_GCOVR_EXCL_LINE; } @@ -64,6 +65,7 @@ class MinimalTransportSocketFactoryContext private: Envoy::Extensions::TransportSockets::Tls::ContextManagerImpl& ssl_context_manager_; Envoy::Stats::ScopePtr stats_scope_; + Envoy::Server::ConfigTrackerImpl config_tracker_; Envoy::Secret::SecretManagerImpl secret_manager_; Envoy::Event::Dispatcher& dispatcher_; Envoy::Runtime::RandomGenerator& random_; diff --git a/test/BUILD b/test/BUILD index 45335dac0..d0187ebb9 100644 --- a/test/BUILD +++ b/test/BUILD @@ -24,23 +24,23 @@ envoy_cc_mock( envoy_cc_test( name = "nighthawk_test", srcs = [ - "benchmark_http_client_test.cc", - #"client_test.cc", - #"client_worker_test.cc", - #"factories_test.cc", - #"frequency_test.cc", - #"options_test.cc", - #"output_collector_test.cc", - #"platform_util_test.cc", - #"process_test.cc", - #"rate_limiter_test.cc", - #"sequencer_test.cc", - #"service_main_test.cc", - #"service_test.cc", - #"statistic_test.cc", - #"stream_decoder_test.cc", - #"utility_test.cc", - # "worker_test.cc", + #"benchmark_http_client_test.cc", + "client_test.cc", + "client_worker_test.cc", + "factories_test.cc", + "frequency_test.cc", + "options_test.cc", + "output_collector_test.cc", + "platform_util_test.cc", + "process_test.cc", + "rate_limiter_test.cc", + "sequencer_test.cc", + "service_main_test.cc", + "service_test.cc", + "statistic_test.cc", + "stream_decoder_test.cc", + "utility_test.cc", + "worker_test.cc", ], data = [ "test_data/benchmark_http_client_test_envoy.yaml", From dd555120e128ed0dece2d4794ddfb3a1a6f78474 Mon Sep 17 00:00:00 2001 From: Otto van der Schaaf Date: Thu, 25 Jul 2019 14:48:46 +0200 Subject: [PATCH 14/31] Enable all tests Signed-off-by: Otto van der Schaaf --- test/BUILD | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/BUILD b/test/BUILD index d0187ebb9..07c46ba84 100644 --- a/test/BUILD +++ b/test/BUILD @@ -24,7 +24,7 @@ envoy_cc_mock( envoy_cc_test( name = "nighthawk_test", srcs = [ - #"benchmark_http_client_test.cc", + "benchmark_http_client_test.cc", "client_test.cc", "client_worker_test.cc", "factories_test.cc", From f0b16f3fd727a41b4fae8523c029434d4e857287 Mon Sep 17 00:00:00 2001 From: Otto van der Schaaf Date: Thu, 25 Jul 2019 15:33:13 +0200 Subject: [PATCH 15/31] do_ci.sh: add install_virtualenv() Signed-off-by: Otto van der Schaaf --- ci/do_ci.sh | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/ci/do_ci.sh b/ci/do_ci.sh index ccae0454b..c002e91f5 100755 --- a/ci/do_ci.sh +++ b/ci/do_ci.sh @@ -4,12 +4,24 @@ set -e export BUILDIFIER_BIN="/usr/local/bin/buildifier" + +function install_virtualenv() { + echo "Install python requirements via virtualenv" + cd "${SRCDIR}" + if [ ! -d "venv" ]; then + virtualenv3 venv + fi + . venv/bin/activate + pip3 install -r requirements.txt +} + function do_build () { bazel build $BAZEL_BUILD_OPTIONS --verbose_failures=true //:nighthawk_client //:nighthawk_test_server \ //:nighthawk_service } function do_test() { + install_virtualenv bazel test $BAZEL_BUILD_OPTIONS $BAZEL_TEST_OPTIONS \ --test_output=all \ //test:nighthawk_test //test/server:http_test_server_filter_integration_test \ @@ -17,6 +29,7 @@ function do_test() { } function do_test_with_valgrind() { + install_virtualenv apt-get update && apt-get install valgrind && \ bazel build $BAZEL_BUILD_OPTIONS -c dbg //test:nighthawk_test && \ nighthawk/tools/valgrind-tests.sh @@ -27,6 +40,7 @@ function do_clang_tidy() { } function do_coverage() { + install_virtualenv ci/run_coverage.sh } @@ -62,6 +76,7 @@ function run_bazel() { } function do_asan() { + install_virtualenv echo "bazel ASAN/UBSAN debug build with tests" echo "Building and testing envoy tests..." cd "${SRCDIR}" @@ -69,6 +84,7 @@ function do_asan() { } function do_tsan() { + install_virtualenv echo "bazel TSAN debug build with tests" echo "Building and testing envoy tests..." cd "${SRCDIR}" @@ -136,6 +152,7 @@ export BAZEL_TEST_OPTIONS="${BAZEL_BUILD_OPTIONS} --test_env=HOME --test_env=PYT setup_clang_toolchain + if [ "$1" == "coverage" ]; then setup_gcc_toolchain fi From a80e89edfd44c993e2f61f58818b238cdb248905 Mon Sep 17 00:00:00 2001 From: Otto van der Schaaf Date: Thu, 25 Jul 2019 22:56:36 +0200 Subject: [PATCH 16/31] Fix cpp vs python vs pip in CI As per Harvey's recommendation, use py_binary and refer to it as a tool in the cpp_tests to get the pip deps sorted. Also, clean up integration/BUILD Signed-off-by: Otto van der Schaaf --- .bazelrc | 1 + ci/do_ci.sh | 25 ++++--------------- integration/BUILD | 40 ++++++++++++++++++------------ test/BUILD | 9 +------ test/benchmark_http_client_test.cc | 2 +- 5 files changed, 32 insertions(+), 45 deletions(-) diff --git a/.bazelrc b/.bazelrc index 0b9d8ff54..93c81887e 100644 --- a/.bazelrc +++ b/.bazelrc @@ -66,3 +66,4 @@ build:libc++ --define force_libcpp=enabled # Test options test --test_env=HEAPCHECK=normal --test_env=PPROF_PATH +build --verbose_failures --action_env=HOME --action_env=PYTHONUSERBASE --jobs=12 --show_task_finish --experimental_generate_json_trace_profile diff --git a/ci/do_ci.sh b/ci/do_ci.sh index c002e91f5..a788434ed 100755 --- a/ci/do_ci.sh +++ b/ci/do_ci.sh @@ -5,23 +5,12 @@ set -e export BUILDIFIER_BIN="/usr/local/bin/buildifier" -function install_virtualenv() { - echo "Install python requirements via virtualenv" - cd "${SRCDIR}" - if [ ! -d "venv" ]; then - virtualenv3 venv - fi - . venv/bin/activate - pip3 install -r requirements.txt -} - function do_build () { bazel build $BAZEL_BUILD_OPTIONS --verbose_failures=true //:nighthawk_client //:nighthawk_test_server \ - //:nighthawk_service + //:nighthawk_service } function do_test() { - install_virtualenv bazel test $BAZEL_BUILD_OPTIONS $BAZEL_TEST_OPTIONS \ --test_output=all \ //test:nighthawk_test //test/server:http_test_server_filter_integration_test \ @@ -29,7 +18,6 @@ function do_test() { } function do_test_with_valgrind() { - install_virtualenv apt-get update && apt-get install valgrind && \ bazel build $BAZEL_BUILD_OPTIONS -c dbg //test:nighthawk_test && \ nighthawk/tools/valgrind-tests.sh @@ -40,7 +28,6 @@ function do_clang_tidy() { } function do_coverage() { - install_virtualenv ci/run_coverage.sh } @@ -76,7 +63,6 @@ function run_bazel() { } function do_asan() { - install_virtualenv echo "bazel ASAN/UBSAN debug build with tests" echo "Building and testing envoy tests..." cd "${SRCDIR}" @@ -84,7 +70,6 @@ function do_asan() { } function do_tsan() { - install_virtualenv echo "bazel TSAN debug build with tests" echo "Building and testing envoy tests..." cd "${SRCDIR}" @@ -112,14 +97,14 @@ if [ -n "$CIRCLECI" ]; then mv "${HOME:-/root}/.gitconfig" "${HOME:-/root}/.gitconfig_save" echo 1 fi - + NUM_CPUS=8 if [ "$1" == "coverage" ]; then NUM_CPUS=6 fi fi - if grep 'docker\|lxc' /proc/1/cgroup; then +if grep 'docker\|lxc' /proc/1/cgroup; then # Create a fake home. Python site libs tries to do getpwuid(3) if we don't and the CI # Docker image gets confused as it has no passwd entry when running non-root # unless we do this. @@ -127,14 +112,14 @@ fi mkdir -p "${FAKE_HOME}" export HOME="${FAKE_HOME}" export PYTHONUSERBASE="${FAKE_HOME}" - + export BUILD_DIR=/build if [[ ! -d "${BUILD_DIR}" ]] then echo "${BUILD_DIR} mount missing - did you forget -v :${BUILD_DIR}? Creating." mkdir -p "${BUILD_DIR}" fi - + # Environment setup. export USER=bazel export TEST_TMPDIR=/build/tmp diff --git a/integration/BUILD b/integration/BUILD index 3bb9a48aa..0f43e33c4 100644 --- a/integration/BUILD +++ b/integration/BUILD @@ -8,23 +8,12 @@ load("@python_pip_deps//:requirements.bzl", "requirement") envoy_package() -exports_files( - [ - "cpp_benchmark_client_server.py", - "test_integration_basics.py", +py_library( + name = "integration_test_base", + srcs = [ "common.py", "integration_test_fixtures.py", "nighthawk_test_server.py", - "integration_test.py", - "configurations/nighthawk_http_origin.yaml", - "configurations/nighthawk_https_origin.yaml", - ], -) - -py_test( - name = "integration_test", - srcs = [ - "integration_test.py", ], data = [ "configurations/nighthawk_http_origin.yaml", @@ -33,8 +22,6 @@ py_test( "//:nighthawk_test_server", "@envoy//test/config/integration/certs", ], - python_version = "PY3", - srcs_version = "PY3ONLY", deps = [ requirement("requests"), # The following are implied by 'request'. @@ -44,3 +31,24 @@ py_test( requirement("idna"), ], ) + +py_binary( + name = "cpp_benchmark_client_server", + srcs = [ + "cpp_benchmark_client_server.py", + ], + deps = [ + ":integration_test_base", + ], +) + +py_test( + name = "integration_test", + srcs = [ + "integration_test.py", + "test_integration_basics.py", + ], + deps = [ + ":integration_test_base", + ], +) diff --git a/test/BUILD b/test/BUILD index 07c46ba84..87a6fb6a8 100644 --- a/test/BUILD +++ b/test/BUILD @@ -50,14 +50,7 @@ envoy_cc_test( "test_data/output_collector.json.gold", "test_data/output_collector.txt.gold", "test_data/output_collector.yaml.gold", - "//integration:common.py", - "//integration:configurations/nighthawk_http_origin.yaml", - "//integration:configurations/nighthawk_https_origin.yaml", - "//integration:cpp_benchmark_client_server.py", - "//integration:integration_test.py", - "//integration:integration_test_fixtures.py", - "//integration:nighthawk_test_server.py", - "//integration:test_integration_basics.py", + "//integration:cpp_benchmark_client_server", "@envoy//test/config/integration/certs", "@envoy//test/test_common:thread_factory_for_test_lib", "@envoy_api//envoy/api/v2:cds_export_cc", diff --git a/test/benchmark_http_client_test.cc b/test/benchmark_http_client_test.cc index 1c4905a39..e7df18b11 100644 --- a/test/benchmark_http_client_test.cc +++ b/test/benchmark_http_client_test.cc @@ -46,7 +46,7 @@ class PythonIntegrationTestBase : public TestWithParam Date: Thu, 25 Jul 2019 23:53:48 +0200 Subject: [PATCH 17/31] Tidy up Signed-off-by: Otto van der Schaaf --- test/benchmark_http_client_test.cc | 46 ++++++++++++++---------------- 1 file changed, 22 insertions(+), 24 deletions(-) diff --git a/test/benchmark_http_client_test.cc b/test/benchmark_http_client_test.cc index e7df18b11..b68b23e9f 100644 --- a/test/benchmark_http_client_test.cc +++ b/test/benchmark_http_client_test.cc @@ -40,56 +40,54 @@ namespace Nighthawk { class PythonIntegrationTestBase : public TestWithParam { public: - virtual ~PythonIntegrationTestBase() = default; - PythonIntegrationTestBase(Envoy::Network::Address::IpVersion version) + PythonIntegrationTestBase(const Envoy::Network::Address::IpVersion version) : version_(version), pipe_(nullptr){}; - void startPythonIntegrationWrapper(bool use_https) { - std::string script = - Envoy::TestEnvironment::runfilesPath("integration/cpp_benchmark_client_server"); - + void startPythonIntegrationWrapper(const bool use_https) { std::string args; + if (GetParam() == Envoy::Network::Address::IpVersion::v4) { args.append(" ipv4"); } else { args.append(" ipv6"); } + args.append(" http"); if (use_https) { - args.append(" https"); - } else { - args.append(" http"); + args.append("s"); } - script.append(args); - - char buffer[128]; - script = "python3 " + script; - pipe_ = popen(script.c_str(), "r"); + pipe_ = popen(Envoy::TestEnvironment::runfilesPath("integration/cpp_benchmark_client_server") + .append(args) + .c_str(), + "r"); RELEASE_ASSERT(pipe_ != nullptr, "Failed to open pipe"); - RELEASE_ASSERT(fgets(buffer, sizeof(buffer), pipe_) != nullptr, "Expected more data"); - RELEASE_ASSERT(absl::SimpleAtoi(buffer, &server_port_), "couldn't understand server_pid"); - - RELEASE_ASSERT(fgets(buffer, sizeof(buffer), pipe_) != nullptr, "Expected more data"); - RELEASE_ASSERT(absl::SimpleAtoi(buffer, &server_pid_), "couldn't understand server_pid"); + RELEASE_ASSERT(fgets(buffer_.data(), buffer_.max_size(), pipe_) != nullptr, + "Expected more data"); + RELEASE_ASSERT(absl::SimpleAtoi(buffer_.data(), &server_port_), + "couldn't understand server_pid"); + RELEASE_ASSERT(fgets(buffer_.data(), buffer_.max_size(), pipe_) != nullptr, + "Expected more data"); + RELEASE_ASSERT(absl::SimpleAtoi(buffer_.data(), &server_pid_), + "couldn't understand server_pid"); } - void TearDown() { + void TearDown() override { if (server_pid_ != 0) { kill(server_pid_, SIGTERM); } if (pipe_ != nullptr) { - char buffer[128]; // We don't expect any output, lets print it when that happens - while (fgets(buffer, sizeof(buffer), pipe_) != nullptr) { - std::cerr << "python stdout: " << buffer << std::endl; + while (fgets(buffer_.data(), buffer_.max_size(), pipe_) != nullptr) { + std::cerr << "python stdout: " << buffer_.data() << std::endl; } RELEASE_ASSERT(!pclose(pipe_), "Failure closing pipe"); } } Envoy::Event::RealTimeSystem time_system_; // NO_CHECK_FORMAT(real_time) - Envoy::Network::Address::IpVersion version_; + const Envoy::Network::Address::IpVersion version_; int server_port_{0}; int server_pid_{0}; + std::array buffer_; FILE* pipe_; }; From 9e556e9f4efa7f4b6178d363f22896c374e60c5a Mon Sep 17 00:00:00 2001 From: Otto van der Schaaf Date: Fri, 26 Jul 2019 14:00:37 +0200 Subject: [PATCH 18/31] Coverage test update - sync the way we do coverage testing with Envoy upstream - sync .bazelrc with Envoy upstream Notes: disabled `LinearRateLimiterInvalidArgumentTest` as that somehow specifically failed with the new coverage testing. Visibility of this target is the single thing from barring this to work: https://github.com/envoyproxy/envoy/blob/master/bazel/envoy_test.bzl#L147 Signed-off-by: Otto van der Schaaf --- .bazelrc | 72 +++++++++++++++++++++++++++--- .gitignore | 3 +- ci/do_ci.sh | 13 +++++- ci/run_coverage.sh | 46 ------------------- test/coverage/gen_build.sh | 76 ++++++++++++++++++++++++++++++++ test/rate_limiter_test.cc | 4 +- test/run_envoy_bazel_coverage.sh | 67 ++++++++++++++++++++++++++++ 7 files changed, 225 insertions(+), 56 deletions(-) delete mode 100755 ci/run_coverage.sh create mode 100755 test/coverage/gen_build.sh create mode 100755 test/run_envoy_bazel_coverage.sh diff --git a/.bazelrc b/.bazelrc index 93c81887e..cd6ccf3fe 100644 --- a/.bazelrc +++ b/.bazelrc @@ -1,17 +1,25 @@ -# The following .bazelrc content is forked from the main Envoy repository. This is necessary since -# this needs to be available before we can access the Envoy repository contents via Bazel. - # Envoy specific Bazel build/test options. -# Bazel doesn't need more than 200MB of memory based on memory profiling: +# Bazel doesn't need more than 200MB of memory for local build based on memory profiling: # https://docs.bazel.build/versions/master/skylark/performance.html#memory-profiling +# The default JVM max heapsize is 1/4 of physical memory up to 32GB which could be large +# enough to consume all memory constrained by cgroup in large host, which is the case in CircleCI. # Limiting JVM heapsize here to let it do GC more when approaching the limit to # leave room for compiler/linker. -startup --host_jvm_args=-Xmx512m +# The number 2G is choosed heuristically to both support in CircleCI and large enough for RBE. +# Startup options cannot be selected via config. +startup --host_jvm_args=-Xmx2g + build --workspace_status_command=bazel/get_workspace_status build --experimental_remap_main_repo +build --experimental_local_memory_estimate +build --host_force_python=PY2 +build --action_env=BAZEL_LINKLIBS=-l%:libstdc++.a +build --action_env=BAZEL_LINKOPTS=-lm:-static-libgcc # Basic ASAN/UBSAN that works for gcc +build:asan --action_env=BAZEL_LINKLIBS= +build:asan --action_env=BAZEL_LINKOPTS=-lstdc++:-lm build:asan --define ENVOY_CONFIG_ASAN=1 build:asan --copt -fsanitize=address,undefined build:asan --linkopt -fsanitize=address,undefined @@ -43,6 +51,7 @@ build:clang-tsan --define ENVOY_CONFIG_TSAN=1 build:clang-tsan --copt -fsanitize=thread build:clang-tsan --linkopt -fsanitize=thread build:clang-tsan --linkopt -fuse-ld=lld +build:clang-tsan --linkopt -static-libsan build:clang-tsan --define tcmalloc=disabled # Needed due to https://github.com/libevent/libevent/issues/777 build:clang-tsan --copt -DEVENT__DISABLE_DEBUG_MODE @@ -61,9 +70,58 @@ build:clang-msan --copt -fsanitize-memory-track-origins=2 build:libc++ --action_env=CC build:libc++ --action_env=CXX build:libc++ --action_env=CXXFLAGS=-stdlib=libc++ +build:libc++ --action_env=BAZEL_CXXOPTS=-stdlib=libc++ +build:libc++ --action_env=BAZEL_LINKLIBS=-l%:libc++.a:-l%:libc++abi.a:-lm build:libc++ --action_env=PATH +build:libc++ --host_linkopt=-fuse-ld=lld build:libc++ --define force_libcpp=enabled +# Optimize build for binary size reduction. +build:sizeopt -c opt --copt -Os + # Test options -test --test_env=HEAPCHECK=normal --test_env=PPROF_PATH -build --verbose_failures --action_env=HOME --action_env=PYTHONUSERBASE --jobs=12 --show_task_finish --experimental_generate_json_trace_profile +build --test_env=HEAPCHECK=normal --test_env=PPROF_PATH + +# Remote execution: https://docs.bazel.build/versions/master/remote-execution.html +build:rbe-toolchain --host_javabase=@rbe_ubuntu_clang//java:jdk +build:rbe-toolchain --javabase=@rbe_ubuntu_clang//java:jdk +build:rbe-toolchain --host_java_toolchain=@bazel_tools//tools/jdk:toolchain_hostjdk8 +build:rbe-toolchain --java_toolchain=@bazel_tools//tools/jdk:toolchain_hostjdk8 +build:rbe-toolchain --host_platform=@envoy//bazel/toolchains:rbe_ubuntu_clang_platform +build:rbe-toolchain --platforms=@envoy//bazel/toolchains:rbe_ubuntu_clang_platform +build:rbe-toolchain --action_env=BAZEL_DO_NOT_DETECT_CPP_TOOLCHAIN=1 +build:rbe-toolchain --crosstool_top=@rbe_ubuntu_clang//cc:toolchain +build:rbe-toolchain --extra_toolchains=@rbe_ubuntu_clang//config:cc-toolchain +build:rbe-toolchain --linkopt=-fuse-ld=lld +build:rbe-toolchain --action_env=CC=clang --action_env=CXX=clang++ --action_env=PATH=/usr/sbin:/usr/bin:/sbin:/bin:/usr/lib/llvm-8/bin + +build:remote --spawn_strategy=remote,sandboxed,local +build:remote --strategy=Javac=remote,sandboxed,local +build:remote --strategy=Closure=remote,sandboxed,local +build:remote --strategy=Genrule=remote,sandboxed,local +build:remote --remote_timeout=3600 +build:remote --auth_enabled=true +build:remote --experimental_inmemory_jdeps_files +build:remote --experimental_inmemory_dotd_files +build:remote --experimental_remote_download_outputs=toplevel +test:remote --experimental_remote_download_outputs=minimal + +build:remote-clang --config=remote +build:remote-clang --config=rbe-toolchain + +# Docker sandbox +build:docker-sandbox --experimental_docker_image=envoyproxy/envoy-build:cfc514546bc0284536893cca5fa43d7128edcd35 +build:docker-sandbox --spawn_strategy=docker +build:docker-sandbox --strategy=Javac=docker +build:docker-sandbox --strategy=Closure=docker +build:docker-sandbox --strategy=Genrule=docker +build:docker-sandbox --define=EXECUTOR=remote +build:docker-sandbox --experimental_docker_verbose +build:docker-sandbox --experimental_enable_docker_sandbox + +build:docker-clang --config=docker-sandbox +build:docker-clang --config=rbe-toolchain + +# CI configurations +build:remote-ci --remote_cache=grpcs://remotebuildexecution.googleapis.com +build:remote-ci --remote_executor=grpcs://remotebuildexecution.googleapis.com \ No newline at end of file diff --git a/.gitignore b/.gitignore index c3f6be666..19ef59f6d 100644 --- a/.gitignore +++ b/.gitignore @@ -14,4 +14,5 @@ bazel.output.txt envoy/ *.pyc __pycache__ -tools/pyformat \ No newline at end of file +tools/pyformat +test/coverage/BUILD diff --git a/ci/do_ci.sh b/ci/do_ci.sh index a788434ed..bddc64640 100755 --- a/ci/do_ci.sh +++ b/ci/do_ci.sh @@ -28,7 +28,18 @@ function do_clang_tidy() { } function do_coverage() { - ci/run_coverage.sh + setup_clang_toolchain + echo "bazel coverage build with tests ${TEST_TARGETS}" + + # Reduce the amount of memory Bazel tries to use to prevent it from launching too many subprocesses. + # This should prevent the system from running out of memory and killing tasks. See discussion on + # https://github.com/envoyproxy/envoy/pull/5611. + [ -z "$CIRCLECI" ] || export BAZEL_BUILD_OPTIONS="${BAZEL_BUILD_OPTIONS} --local_ram_resources=12288" + + export TEST_TARGETS="//test/..." + test/run_envoy_bazel_coverage.sh ${TEST_TARGETS} + collect_build_profile coverage + exit 0 } function setup_gcc_toolchain() { diff --git a/ci/run_coverage.sh b/ci/run_coverage.sh deleted file mode 100755 index be8f2d457..000000000 --- a/ci/run_coverage.sh +++ /dev/null @@ -1,46 +0,0 @@ -#!/bin/bash - -set -e - -[[ -z "${SRCDIR}" ]] && SRCDIR="${PWD}" -[[ -z "${BAZEL_COVERAGE}" ]] && BAZEL_COVERAGE=bazel -[[ -z "${VALIDATE_COVERAGE}" ]] && VALIDATE_COVERAGE=true - -# This is the target that will be run to generate coverage data. It can be overridden by consumer -# projects that want to run coverage on a different/combined target. -[[ -z "${COVERAGE_TARGET}" ]] && COVERAGE_TARGET="//test/..." - -bazel clean - -# Generate coverage data. -"${BAZEL_COVERAGE}" coverage ${BAZEL_TEST_OPTIONS} \ -"${COVERAGE_TARGET}" \ ---experimental_cc_coverage \ ---instrumentation_filter=//source/...,//include/... \ ---coverage_report_generator=@bazel_tools//tools/test/CoverageOutputGenerator/java/com/google/devtools/coverageoutputgenerator:Main \ ---combined_report=lcov - -# Generate HTML -declare -r COVERAGE_DIR="${SRCDIR}"/generated/coverage -declare -r COVERAGE_SUMMARY="${COVERAGE_DIR}/coverage_summary.txt" -mkdir -p "${COVERAGE_DIR}" -genhtml bazel-out/_coverage/_coverage_report.dat --output-directory="${COVERAGE_DIR}" | tee "${COVERAGE_SUMMARY}" - -[[ -z "${ENVOY_COVERAGE_DIR}" ]] || rsync -av "${COVERAGE_DIR}"/ "${ENVOY_COVERAGE_DIR}" - -if [ "$VALIDATE_COVERAGE" == "true" ] -then - COVERAGE_VALUE=$(grep -Po '.*lines[.]*: \K(\d|\.)*' "${COVERAGE_SUMMARY}") - # TODO(oschaaf): The target is 97.5%, so up this whenever possible in follow ups. - COVERAGE_THRESHOLD=96.9 - COVERAGE_FAILED=$(echo "${COVERAGE_VALUE}<${COVERAGE_THRESHOLD}" | bc) - - echo "HTML coverage report is in ${COVERAGE_DIR}/coverage.html" - - if test ${COVERAGE_FAILED} -eq 1; then - echo Code coverage ${COVERAGE_VALUE} is lower than limit of ${COVERAGE_THRESHOLD} - exit 1 - else - echo Code coverage ${COVERAGE_VALUE} is good and higher than limit of ${COVERAGE_THRESHOLD} - fi -fi diff --git a/test/coverage/gen_build.sh b/test/coverage/gen_build.sh new file mode 100755 index 000000000..80d29af08 --- /dev/null +++ b/test/coverage/gen_build.sh @@ -0,0 +1,76 @@ +#!/bin/bash + +# Generate test/coverage/BUILD, which contains a single envoy_cc_test target +# that contains all C++ based tests suitable for performing the coverage run. A +# single binary (as opposed to multiple test targets) is require to work around +# the crazy in https://github.com/bazelbuild/bazel/issues/1118. This is used by +# the coverage runner script. + +set -e +set -x + +[ -z "${BAZEL_BIN}" ] && BAZEL_BIN=bazel +[ -z "${BUILDIFIER_BIN}" ] && BUILDIFIER_BIN=buildifier + +# Path to the generated BUILD file for the coverage target. +[ -z "${BUILD_PATH}" ] && BUILD_PATH="$(dirname "$0")"/BUILD + +# Extra repository information to include when generating coverage targets. This is useful for +# consuming projects. E.g., "@envoy". +[ -z "${REPOSITORY}" ] && REPOSITORY="" + +# This is an extra bazel path to query for additional targets. This is useful for consuming projects +# that want to run coverage over the public envoy code as well as private extensions. +# E.g., "//envoy-lyft/test/..." +[ -z "${EXTRA_QUERY_PATHS}" ] && EXTRA_QUERY_PATHS="" + +rm -f "${BUILD_PATH}" + +if [[ $# -gt 0 ]]; then + COVERAGE_TARGETS=$* +else + COVERAGE_TARGETS=//test/... +fi + +for target in ${COVERAGE_TARGETS}; do + TARGETS="$TARGETS $("${BAZEL_BIN}" query ${BAZEL_QUERY_OPTIONS} "attr('tags', 'coverage_test_lib', ${REPOSITORY}${target})" | grep "^//")" +done + +( + cat << EOF +# This file is generated by test/coverage/gen_build.sh automatically prior to +# coverage runs. It is under .gitignore. DO NOT EDIT, DO NOT CHECK IN. +load( + "@envoy//bazel:envoy_build_system.bzl", + "envoy_cc_test", + "envoy_package", +) + +envoy_package() + +envoy_cc_test( + name = "coverage_tests", + repository = "@envoy", + deps = [ +EOF + for t in ${TARGETS} + do + echo " \"$t\"," + done + cat << EOF + ], + # no-remote due to https://github.com/bazelbuild/bazel/issues/4685 + tags = ["manual", "no-remote"], + coverage = False, + # Due to the nature of coverage_tests, the shard of coverage_tests are very uneven, some of + # shard can take 100s and some takes only 10s, so we use the maximum sharding to here to let + # Bazel scheduling them across CPU cores. + # Sharding can be disabled by --test_sharding_strategy=disabled. + shard_count = 50, +) +EOF + +) > "${BUILD_PATH}" + +echo "Generated coverage BUILD file at: ${BUILD_PATH}" +"${BUILDIFIER_BIN}" "${BUILD_PATH}" \ No newline at end of file diff --git a/test/rate_limiter_test.cc b/test/rate_limiter_test.cc index 042fd9884..135d9ceeb 100644 --- a/test/rate_limiter_test.cc +++ b/test/rate_limiter_test.cc @@ -35,7 +35,9 @@ TEST_F(RateLimiterTest, LinearRateLimiterTest) { EXPECT_FALSE(rate_limiter.tryAcquireOne()); } -TEST_F(RateLimiterTest, LinearRateLimiterInvalidArgumentTest) { +// TODO(oschaaf): this fails with the new coverage testing method. +// Disabled it for now. +TEST_F(RateLimiterTest, DISABLED_LinearRateLimiterInvalidArgumentTest) { Envoy::Event::SimulatedTimeSystem time_system; ASSERT_DEATH(LinearRateLimiter rate_limiter(time_system, 0_Hz), "Frequency must be > 0"); } diff --git a/test/run_envoy_bazel_coverage.sh b/test/run_envoy_bazel_coverage.sh new file mode 100755 index 000000000..64173a4ef --- /dev/null +++ b/test/run_envoy_bazel_coverage.sh @@ -0,0 +1,67 @@ +#!/bin/bash + +set -e +set -x + +[[ -z "${SRCDIR}" ]] && SRCDIR="${PWD}" +[[ -z "${VALIDATE_COVERAGE}" ]] && VALIDATE_COVERAGE=true + +echo "Starting run_envoy_bazel_coverage.sh..." +echo " PWD=$(pwd)" +echo " SRCDIR=${SRCDIR}" +echo " VALIDATE_COVERAGE=${VALIDATE_COVERAGE}" + +# This is the target that will be run to generate coverage data. It can be overridden by consumer +# projects that want to run coverage on a different/combined target. +# Command-line arguments take precedence over ${COVERAGE_TARGET}. +if [[ $# -gt 0 ]]; then + COVERAGE_TARGETS=$* +elif [[ -n "${COVERAGE_TARGET}" ]]; then + COVERAGE_TARGETS=${COVERAGE_TARGET} +else + COVERAGE_TARGETS=//test/... +fi + +# Make sure //test/coverage:coverage_tests is up-to-date. +SCRIPT_DIR="$(realpath "$(dirname "$0")")" +"${SCRIPT_DIR}"/coverage/gen_build.sh ${COVERAGE_TARGETS} + +BAZEL_USE_LLVM_NATIVE_COVERAGE=1 GCOV=llvm-profdata bazel coverage ${BAZEL_BUILD_OPTIONS} \ + -c fastbuild --copt=-DNDEBUG --instrumentation_filter=//source/...,//include/... \ + --test_timeout=2000 --cxxopt="-DENVOY_CONFIG_COVERAGE=1" --test_output=errors \ + --test_arg="--log-path /dev/null" --test_arg="-l trace" --test_env=HEAPCHECK= \ + //test/coverage:coverage_tests + +COVERAGE_DIR="${SRCDIR}"/generated/coverage +mkdir -p "${COVERAGE_DIR}" + +COVERAGE_IGNORE_REGEX="(/external/|pb\.(validate\.)?(h|cc)|/chromium_url/|/test/|/tmp)" +COVERAGE_BINARY="bazel-bin/test/coverage/coverage_tests" +COVERAGE_DATA="${COVERAGE_DIR}/coverage.dat" + +echo "Merging coverage data..." +llvm-profdata merge -sparse -o ${COVERAGE_DATA} $(find -L bazel-out/k8-fastbuild/testlogs/test/coverage/coverage_tests/ -name coverage.dat) + +echo "Generating report..." +llvm-cov show "${COVERAGE_BINARY}" -instr-profile="${COVERAGE_DATA}" -Xdemangler=c++filt \ + -ignore-filename-regex="${COVERAGE_IGNORE_REGEX}" -output-dir=${COVERAGE_DIR} -format=html +sed -i -e 's|>proc/self/cwd/|>|g' "${COVERAGE_DIR}/index.html" +sed -i -e 's|>bazel-out/[^/]*/bin/\([^/]*\)/[^<]*/_virtual_includes/[^/]*|>\1|g' "${COVERAGE_DIR}/index.html" + +[[ -z "${ENVOY_COVERAGE_DIR}" ]] || rsync -av "${COVERAGE_DIR}"/ "${ENVOY_COVERAGE_DIR}" + +if [ "$VALIDATE_COVERAGE" == "true" ] +then + COVERAGE_VALUE=$(llvm-cov export "${COVERAGE_BINARY}" -instr-profile="${COVERAGE_DATA}" \ + -ignore-filename-regex="${COVERAGE_IGNORE_REGEX}" -summary-only | \ + python3 -c "import sys, json; print(json.load(sys.stdin)['data'][0]['totals']['lines']['percent'])") + COVERAGE_THRESHOLD=97.0 + COVERAGE_FAILED=$(echo "${COVERAGE_VALUE}<${COVERAGE_THRESHOLD}" | bc) + if test ${COVERAGE_FAILED} -eq 1; then + echo Code coverage ${COVERAGE_VALUE} is lower than limit of ${COVERAGE_THRESHOLD} + exit 1 + else + echo Code coverage ${COVERAGE_VALUE} is good and higher than limit of ${COVERAGE_THRESHOLD} + fi +fi +echo "HTML coverage report is in ${COVERAGE_DIR}/index.html" \ No newline at end of file From 32d2d4c822cfa97b91bfe56e0879571a6896f35f Mon Sep 17 00:00:00 2001 From: Otto van der Schaaf Date: Fri, 26 Jul 2019 14:33:05 +0200 Subject: [PATCH 19/31] Bump coverage threshold to 97.5 Signed-off-by: Otto van der Schaaf --- ci/do_ci.sh | 1 - test/run_envoy_bazel_coverage.sh | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/ci/do_ci.sh b/ci/do_ci.sh index bddc64640..e5ab84e8a 100755 --- a/ci/do_ci.sh +++ b/ci/do_ci.sh @@ -38,7 +38,6 @@ function do_coverage() { export TEST_TARGETS="//test/..." test/run_envoy_bazel_coverage.sh ${TEST_TARGETS} - collect_build_profile coverage exit 0 } diff --git a/test/run_envoy_bazel_coverage.sh b/test/run_envoy_bazel_coverage.sh index 64173a4ef..0da2b8391 100755 --- a/test/run_envoy_bazel_coverage.sh +++ b/test/run_envoy_bazel_coverage.sh @@ -55,7 +55,7 @@ then COVERAGE_VALUE=$(llvm-cov export "${COVERAGE_BINARY}" -instr-profile="${COVERAGE_DATA}" \ -ignore-filename-regex="${COVERAGE_IGNORE_REGEX}" -summary-only | \ python3 -c "import sys, json; print(json.load(sys.stdin)['data'][0]['totals']['lines']['percent'])") - COVERAGE_THRESHOLD=97.0 + COVERAGE_THRESHOLD=97.5 COVERAGE_FAILED=$(echo "${COVERAGE_VALUE}<${COVERAGE_THRESHOLD}" | bc) if test ${COVERAGE_FAILED} -eq 1; then echo Code coverage ${COVERAGE_VALUE} is lower than limit of ${COVERAGE_THRESHOLD} From 401306eaeb2265a544f480af6a5dcdb9f5ce3961 Mon Sep 17 00:00:00 2001 From: Otto van der Schaaf Date: Fri, 26 Jul 2019 16:43:37 +0200 Subject: [PATCH 20/31] Fix asan issue + a couple of tweaks Signed-off-by: Otto van der Schaaf --- .bazelrc | 3 +++ source/common/worker_impl.cc | 1 + test/BUILD | 1 + test/client_worker_test.cc | 10 ++++++++++ test/worker_test.cc | 12 ++++++++++++ 5 files changed, 27 insertions(+) diff --git a/.bazelrc b/.bazelrc index cd6ccf3fe..9d7a2164a 100644 --- a/.bazelrc +++ b/.bazelrc @@ -1,3 +1,6 @@ +# The following .bazelrc content is forked from the main Envoy repository. This is necessary since +# this needs to be available before we can access the Envoy repository contents via Bazel. + # Envoy specific Bazel build/test options. # Bazel doesn't need more than 200MB of memory for local build based on memory profiling: diff --git a/source/common/worker_impl.cc b/source/common/worker_impl.cc index 44c7d0f2e..50bb1e03a 100644 --- a/source/common/worker_impl.cc +++ b/source/common/worker_impl.cc @@ -20,6 +20,7 @@ void WorkerImpl::start() { ASSERT(!started_ && !completed_); started_ = true; thread_ = thread_factory_.createThread([this]() { + ASSERT(Envoy::Runtime::LoaderSingleton::getExisting() != nullptr); // Run the dispatcher to let the callbacks posted by registerThread() execute. dispatcher_->run(Envoy::Event::Dispatcher::RunType::Block); work(); diff --git a/test/BUILD b/test/BUILD index 87a6fb6a8..9df8accce 100644 --- a/test/BUILD +++ b/test/BUILD @@ -69,6 +69,7 @@ envoy_cc_test( "@envoy//source/common/stats:isolated_store_lib", "@envoy//test/integration:integration_lib", "@envoy//test/mocks/event:event_mocks", + "@envoy//test/mocks/protobuf:protobuf_mocks", "@envoy//test/mocks/thread_local:thread_local_mocks", "@envoy//test/server:utility_lib", "@envoy//test/test_common:simulated_time_system_lib", diff --git a/test/client_worker_test.cc b/test/client_worker_test.cc index fd5658ab6..8ff59c572 100644 --- a/test/client_worker_test.cc +++ b/test/client_worker_test.cc @@ -11,6 +11,9 @@ #include "client/client_worker_impl.h" #include "test/mocks.h" +#include "test/mocks/init/mocks.h" +#include "test/mocks/local_info/mocks.h" +#include "test/mocks/protobuf/mocks.h" #include "test/mocks/thread_local/mocks.h" #include "test/test_common/thread_factory_for_test.h" @@ -26,6 +29,9 @@ class ClientWorkerTest : public Test { ClientWorkerTest() : api_(Envoy::Thread::threadFactoryForTest(), store_, time_system_, file_system_), thread_id_(std::this_thread::get_id()) { + loader_ = std::make_unique(Envoy::Runtime::LoaderPtr{ + new Envoy::Runtime::LoaderImpl(dispatcher_, tls_, {}, local_info_, init_manager_, store_, + rand_, validation_visitor_, api_)}); benchmark_client_ = new MockBenchmarkClient(); sequencer_ = new MockSequencer(); @@ -64,6 +70,10 @@ class ClientWorkerTest : public Test { Envoy::Runtime::RandomGeneratorImpl rand_; NiceMock dispatcher_; Envoy::Filesystem::InstanceImplPosix file_system_; + std::unique_ptr loader_; + NiceMock local_info_; + Envoy::Init::MockManager init_manager_; + NiceMock validation_visitor_; }; TEST_F(ClientWorkerTest, BasicTest) { diff --git a/test/worker_test.cc b/test/worker_test.cc index 81a48ecae..a7d8fe63f 100644 --- a/test/worker_test.cc +++ b/test/worker_test.cc @@ -7,6 +7,9 @@ #include "common/worker_impl.h" #include "test/mocks.h" +#include "test/mocks/init/mocks.h" +#include "test/mocks/local_info/mocks.h" +#include "test/mocks/protobuf/mocks.h" #include "test/mocks/thread_local/mocks.h" #include "test/test_common/thread_factory_for_test.h" @@ -40,13 +43,22 @@ class WorkerTest : public Test { Envoy::Stats::IsolatedStoreImpl store_; Envoy::Event::TestRealTimeSystem time_system_; Envoy::Runtime::RandomGeneratorImpl rand_; + NiceMock local_info_; + Envoy::Init::MockManager init_manager_; + NiceMock validation_visitor_; }; TEST_F(WorkerTest, WorkerExecutesOnThread) { InSequence in_sequence; EXPECT_CALL(tls_, registerThread(_, false)).Times(1); + EXPECT_CALL(tls_, allocateSlot()).Times(1); + TestWorker worker(api_, tls_); NiceMock dispatcher; + std::unique_ptr loader = + std::make_unique(Envoy::Runtime::LoaderPtr{ + new Envoy::Runtime::LoaderImpl(dispatcher, tls_, {}, local_info_, init_manager_, store_, + rand_, validation_visitor_, api_)}); worker.start(); worker.waitForCompletion(); EXPECT_CALL(tls_, shutdownThread()).Times(1); From 95166d02a33bae27bc5c334a8a8e542762d79550 Mon Sep 17 00:00:00 2001 From: Otto van der Schaaf Date: Fri, 26 Jul 2019 17:02:23 +0200 Subject: [PATCH 21/31] back out remote cache in .bazelrc Signed-off-by: Otto van der Schaaf --- .bazelrc | 3 --- 1 file changed, 3 deletions(-) diff --git a/.bazelrc b/.bazelrc index 9d7a2164a..e48c98831 100644 --- a/.bazelrc +++ b/.bazelrc @@ -125,6 +125,3 @@ build:docker-sandbox --experimental_enable_docker_sandbox build:docker-clang --config=docker-sandbox build:docker-clang --config=rbe-toolchain -# CI configurations -build:remote-ci --remote_cache=grpcs://remotebuildexecution.googleapis.com -build:remote-ci --remote_executor=grpcs://remotebuildexecution.googleapis.com \ No newline at end of file From 07d6f65ef3b2ddf042d5ea607186d49d1c7b11b8 Mon Sep 17 00:00:00 2001 From: Otto van der Schaaf Date: Mon, 29 Jul 2019 16:43:43 +0200 Subject: [PATCH 22/31] Update Envoy to the latest. In anticipation of https://github.com/envoyproxy/envoy/pull/7749 it makes sense to further upgrade the Envoy dep, which hereby is done. Signed-off-by: Otto van der Schaaf --- WORKSPACE | 16 ++- include/nighthawk/client/benchmark_client.h | 3 +- source/client/benchmark_client_impl.cc | 19 ++-- source/client/benchmark_client_impl.h | 2 +- source/client/client_worker_impl.cc | 2 +- source/common/ssl.h | 105 ++++++++++++++++-- test/benchmark_http_client_test.cc | 12 +- test/mocks.h | 3 +- ...ttp_test_server_filter_integration_test.cc | 2 +- 9 files changed, 127 insertions(+), 37 deletions(-) diff --git a/WORKSPACE b/WORKSPACE index e84fafdec..fd0b828a0 100644 --- a/WORKSPACE +++ b/WORKSPACE @@ -4,23 +4,21 @@ load("//bazel:repositories.bzl", "nighthawk_dependencies") nighthawk_dependencies() +load("@envoy//bazel:api_binding.bzl", "envoy_api_binding") + +envoy_api_binding() + load("@envoy//bazel:api_repositories.bzl", "envoy_api_dependencies") envoy_api_dependencies() -load("@envoy//bazel:repositories.bzl", "GO_VERSION", "envoy_dependencies") +load("@envoy//bazel:repositories.bzl", "envoy_dependencies") envoy_dependencies() -load("@rules_foreign_cc//:workspace_definitions.bzl", "rules_foreign_cc_dependencies") - -rules_foreign_cc_dependencies() - -load("@io_bazel_rules_go//go:deps.bzl", "go_register_toolchains", "go_rules_dependencies") - -go_rules_dependencies() +load("@envoy//bazel:dependency_imports.bzl", "envoy_dependency_imports") -go_register_toolchains(go_version = GO_VERSION) +envoy_dependency_imports() # For PIP support: load("@io_bazel_rules_python//python:pip.bzl", "pip_import", "pip_repositories") diff --git a/include/nighthawk/client/benchmark_client.h b/include/nighthawk/client/benchmark_client.h index feb30ef51..c19695973 100644 --- a/include/nighthawk/client/benchmark_client.h +++ b/include/nighthawk/client/benchmark_client.h @@ -20,8 +20,9 @@ class BenchmarkClient { /** * Initialize will be called on the worker thread after it has started. * @param runtime to be used during initialization. + * @param thread local storage to be used. */ - virtual void initialize(Envoy::Runtime::Loader& runtime) PURE; + virtual void initialize(Envoy::Runtime::Loader& runtime, Envoy::ThreadLocal::Instance& tls) PURE; /** * Terminate will be called on the worker thread before it ends. diff --git a/source/client/benchmark_client_impl.cc b/source/client/benchmark_client_impl.cc index 1de45bbea..7a148a4a4 100644 --- a/source/client/benchmark_client_impl.cc +++ b/source/client/benchmark_client_impl.cc @@ -81,7 +81,8 @@ class H2Pool : public PrefetchablePool, public Envoy::Http::Http2::ProdConnPoolI void BenchmarkClientHttpImpl::prefetchPoolConnections() { pool_->prefetchConnections(); } -void BenchmarkClientHttpImpl::initialize(Envoy::Runtime::Loader& runtime) { +void BenchmarkClientHttpImpl::initialize(Envoy::Runtime::Loader& runtime, + Envoy::ThreadLocal::Instance& tls) { ASSERT(uri_->address() != nullptr); envoy::api::v2::Cluster cluster_config; envoy::api::v2::core::BindConfig bind_config; @@ -97,6 +98,12 @@ void BenchmarkClientHttpImpl::initialize(Envoy::Runtime::Loader& runtime) { thresholds->mutable_max_requests()->set_value(max_active_requests_); Envoy::Network::TransportSocketFactoryPtr socket_factory; + ssl_context_manager_ = + std::make_unique( + api_.timeSource()); + transport_socket_factory_context_ = std::make_unique( + store_.createScope("client."), dispatcher_, generator_, store_, api_, *ssl_context_manager_, + Envoy::ProtobufMessage::getStrictValidationVisitor(), tls); if (uri_->scheme() == "https") { auto common_tls_context = cluster_config.mutable_tls_context()->mutable_common_tls_context(); @@ -119,13 +126,6 @@ void BenchmarkClientHttpImpl::initialize(Envoy::Runtime::Loader& runtime) { Envoy::Server::Configuration::UpstreamTransportSocketConfigFactory>( transport_socket.name()); - ssl_context_manager_ = - std::make_unique( - api_.timeSource()); - transport_socket_factory_context_ = std::make_unique( - store_.createScope("client."), dispatcher_, generator_, store_, api_, *ssl_context_manager_, - Envoy::ProtobufMessage::getStrictValidationVisitor()); - Envoy::ProtobufTypes::MessagePtr message = Envoy::Config::Utility::translateToFactoryConfig( transport_socket, transport_socket_factory_context_->messageValidationVisitor(), config_factory); @@ -142,11 +142,10 @@ void BenchmarkClientHttpImpl::initialize(Envoy::Runtime::Loader& runtime) { socket_factory = std::make_unique(); }; - // TODO(oschaaf): pass in the right validation visitor. cluster_ = std::make_unique( cluster_config, bind_config, runtime, std::move(socket_factory), store_.createScope("client."), false /*added_via_api*/, - Envoy::ProtobufMessage::getStrictValidationVisitor()); + Envoy::ProtobufMessage::getStrictValidationVisitor(), *transport_socket_factory_context_); ASSERT(uri_->address() != nullptr); diff --git a/source/client/benchmark_client_impl.h b/source/client/benchmark_client_impl.h index c6cb5f33e..542402e9b 100644 --- a/source/client/benchmark_client_impl.h +++ b/source/client/benchmark_client_impl.h @@ -71,7 +71,7 @@ class BenchmarkClientHttpImpl : public BenchmarkClient, } // BenchmarkClient - void initialize(Envoy::Runtime::Loader& runtime) override; + void initialize(Envoy::Runtime::Loader& runtime, Envoy::ThreadLocal::Instance& tls) override; void terminate() override; StatisticPtrMap statistics() const override; bool measureLatencies() const override { return measure_latencies_; } diff --git a/source/client/client_worker_impl.cc b/source/client/client_worker_impl.cc index 2e97726ce..7a6980e7a 100644 --- a/source/client/client_worker_impl.cc +++ b/source/client/client_worker_impl.cc @@ -22,7 +22,7 @@ void ClientWorkerImpl::simpleWarmup() { } void ClientWorkerImpl::work() { - benchmark_client_->initialize(*Envoy::Runtime::LoaderSingleton::getExisting()); + benchmark_client_->initialize(*Envoy::Runtime::LoaderSingleton::getExisting(), tls_); simpleWarmup(); benchmark_client_->setMeasureLatencies(true); sequencer_->start(); diff --git a/source/common/ssl.h b/source/common/ssl.h index 7fd04c195..563dce444 100644 --- a/source/common/ssl.h +++ b/source/common/ssl.h @@ -5,7 +5,10 @@ #include "envoy/network/transport_socket.h" +#include "common/local_info/local_info_impl.h" #include "common/secret/secret_manager_impl.h" +#include "common/singleton/manager_impl.h" +#include "common/thread_local/thread_local_impl.h" #include "server/http/config_tracker_impl.h" #include "server/transport_socket_config_impl.h" @@ -18,6 +21,84 @@ namespace Nighthawk { namespace Ssl { +class FakeAdmin : public Envoy::Server::Admin { +public: + bool addHandler(const std::string&, const std::string&, Envoy::Server::Admin::HandlerCb, bool, + bool) override { + NOT_IMPLEMENTED_GCOVR_EXCL_LINE; + }; + bool removeHandler(const std::string&) override { NOT_IMPLEMENTED_GCOVR_EXCL_LINE; }; + const Envoy::Network::Socket& socket() override { NOT_IMPLEMENTED_GCOVR_EXCL_LINE; }; + Envoy::Server::ConfigTracker& getConfigTracker() override { NOT_IMPLEMENTED_GCOVR_EXCL_LINE; }; + void startHttpListener(const std::string&, const std::string&, + Envoy::Network::Address::InstanceConstSharedPtr, + const Envoy::Network::Socket::OptionsSharedPtr&, + Envoy::Stats::ScopePtr&&) override { + NOT_IMPLEMENTED_GCOVR_EXCL_LINE; + }; + Envoy::Http::Code request(absl::string_view, absl::string_view, Envoy::Http::HeaderMap&, + std::string&) override { + NOT_IMPLEMENTED_GCOVR_EXCL_LINE; + }; + void addListenerToHandler(Envoy::Network::ConnectionHandler*) override { + NOT_IMPLEMENTED_GCOVR_EXCL_LINE; + }; +}; + +class FakeClusterManager : public Envoy::Upstream::ClusterManager { +public: + bool addOrUpdateCluster(const envoy::api::v2::Cluster&, const std::string&) override { + NOT_IMPLEMENTED_GCOVR_EXCL_LINE; + } + void setInitializedCb(std::function) override { NOT_IMPLEMENTED_GCOVR_EXCL_LINE; } + Envoy::Upstream::ClusterManager::ClusterInfoMap clusters() override { + NOT_IMPLEMENTED_GCOVR_EXCL_LINE; + } + Envoy::Upstream::ThreadLocalCluster* get(absl::string_view) override { + NOT_IMPLEMENTED_GCOVR_EXCL_LINE; + } + Envoy::Http::ConnectionPool::Instance* + httpConnPoolForCluster(const std::string&, Envoy::Upstream::ResourcePriority, + Envoy::Http::Protocol, Envoy::Upstream::LoadBalancerContext*) override { + NOT_IMPLEMENTED_GCOVR_EXCL_LINE; + } + Envoy::Tcp::ConnectionPool::Instance* + tcpConnPoolForCluster(const std::string&, Envoy::Upstream::ResourcePriority, + Envoy::Upstream::LoadBalancerContext*, + Envoy::Network::TransportSocketOptionsSharedPtr) override { + NOT_IMPLEMENTED_GCOVR_EXCL_LINE; + } + Envoy::Upstream::Host::CreateConnectionData + tcpConnForCluster(const std::string&, Envoy::Upstream::LoadBalancerContext*, + Envoy::Network::TransportSocketOptionsSharedPtr) override { + NOT_IMPLEMENTED_GCOVR_EXCL_LINE; + } + Envoy::Http::AsyncClient& httpAsyncClientForCluster(const std::string&) override { + NOT_IMPLEMENTED_GCOVR_EXCL_LINE; + } + bool removeCluster(const std::string&) override { NOT_IMPLEMENTED_GCOVR_EXCL_LINE; } + void shutdown() override { NOT_IMPLEMENTED_GCOVR_EXCL_LINE; } + const envoy::api::v2::core::BindConfig& bindConfig() const override { + NOT_IMPLEMENTED_GCOVR_EXCL_LINE; + } + Envoy::Config::GrpcMux& adsMux() override { NOT_IMPLEMENTED_GCOVR_EXCL_LINE; } + Envoy::Grpc::AsyncClientManager& grpcAsyncClientManager() override { + NOT_IMPLEMENTED_GCOVR_EXCL_LINE; + } + const std::string& localClusterName() const override { NOT_IMPLEMENTED_GCOVR_EXCL_LINE; } + Envoy::Upstream::ClusterUpdateCallbacksHandlePtr + addThreadLocalClusterUpdateCallbacks(Envoy::Upstream::ClusterUpdateCallbacks&) override { + NOT_IMPLEMENTED_GCOVR_EXCL_LINE; + } + Envoy::Upstream::ClusterManagerFactory& clusterManagerFactory() override { + NOT_IMPLEMENTED_GCOVR_EXCL_LINE; + } + Envoy::Config::SubscriptionFactory& subscriptionFactory() override { + NOT_IMPLEMENTED_GCOVR_EXCL_LINE; + } + std::size_t warmingClusterCount() const override { NOT_IMPLEMENTED_GCOVR_EXCL_LINE; } +}; + class MinimalTransportSocketFactoryContext : public Envoy::Server::Configuration::TransportSocketFactoryContext { public: @@ -25,12 +106,17 @@ class MinimalTransportSocketFactoryContext Envoy::Stats::ScopePtr&& stats_scope, Envoy::Event::Dispatcher& dispatcher, Envoy::Runtime::RandomGenerator& random, Envoy::Stats::Store& stats, Envoy::Api::Api& api, Envoy::Extensions::TransportSockets::Tls::ContextManagerImpl& ssl_context_manager, - Envoy::ProtobufMessage::ValidationVisitor& validation_visitor) + Envoy::ProtobufMessage::ValidationVisitor& validation_visitor, + Envoy::ThreadLocal::Instance& tls) : ssl_context_manager_(ssl_context_manager), stats_scope_(std::move(stats_scope)), secret_manager_(config_tracker_), dispatcher_(dispatcher), random_(random), stats_(stats), - api_(api), validation_visitor_(validation_visitor) {} + api_(api), validation_visitor_(validation_visitor), + local_info_( + {}, Envoy::Network::Utility::getLocalAddress(Envoy::Network::Address::IpVersion::v4), + "nighthawk_service_zone", "nighthawk_service_cluster", "nighthawk_service_node"), + manager_(api_.threadFactory()), tls_(tls) {} - Envoy::Server::Admin& admin() override { NOT_IMPLEMENTED_GCOVR_EXCL_LINE; } + Envoy::Server::Admin& admin() override { return admin_; } Envoy::Ssl::ContextManager& sslContextManager() override { return ssl_context_manager_; } @@ -38,9 +124,9 @@ class MinimalTransportSocketFactoryContext Envoy::Secret::SecretManager& secretManager() override { return secret_manager_; } - Envoy::Upstream::ClusterManager& clusterManager() override { NOT_IMPLEMENTED_GCOVR_EXCL_LINE; } + Envoy::Upstream::ClusterManager& clusterManager() override { return cluster_manager_; } - const Envoy::LocalInfo::LocalInfo& localInfo() override { NOT_IMPLEMENTED_GCOVR_EXCL_LINE; } + const Envoy::LocalInfo::LocalInfo& localInfo() override { return local_info_; } Envoy::Event::Dispatcher& dispatcher() override { return dispatcher_; } @@ -52,9 +138,9 @@ class MinimalTransportSocketFactoryContext Envoy::Init::Manager* initManager() override { NOT_IMPLEMENTED_GCOVR_EXCL_LINE; } - Envoy::Singleton::Manager& singletonManager() override { NOT_IMPLEMENTED_GCOVR_EXCL_LINE; } + Envoy::Singleton::Manager& singletonManager() override { return manager_; } - Envoy::ThreadLocal::SlotAllocator& threadLocal() override { NOT_IMPLEMENTED_GCOVR_EXCL_LINE; } + Envoy::ThreadLocal::SlotAllocator& threadLocal() override { return tls_; } Envoy::Api::Api& api() override { return api_; } @@ -72,6 +158,11 @@ class MinimalTransportSocketFactoryContext Envoy::Stats::Store& stats_; Envoy::Api::Api& api_; Envoy::ProtobufMessage::ValidationVisitor& validation_visitor_; + FakeAdmin admin_; + FakeClusterManager cluster_manager_; + Envoy::LocalInfo::LocalInfoImpl local_info_; + Envoy::Singleton::ManagerImpl manager_; + Envoy::ThreadLocal::Instance& tls_; }; } // namespace Ssl diff --git a/test/benchmark_http_client_test.cc b/test/benchmark_http_client_test.cc index b68b23e9f..9921e36ad 100644 --- a/test/benchmark_http_client_test.cc +++ b/test/benchmark_http_client_test.cc @@ -122,7 +122,7 @@ class BenchmarkClientTestBase : public PythonIntegrationTestBase { client_->setConnectionTimeout(10s); client_->setMaxPendingRequests(max_pending); client_->setConnectionLimit(connection_limit); - client_->initialize(runtime_); + client_->initialize(runtime_, tls_); const uint64_t amount = amount_of_request; uint64_t inflight_response_count = 0; @@ -351,7 +351,7 @@ TEST_P(BenchmarkClientNoServerTest, H1MultiConnectionFailure) { TEST_P(BenchmarkClientHttpTest, EnableLatencyMeasurement) { setupBenchmarkClient("/", false, false); int callback_count = 0; - client_->initialize(runtime_); + client_->initialize(runtime_, tls_); EXPECT_EQ(false, client_->measureLatencies()); EXPECT_EQ(true, client_->tryStartOne([&]() { @@ -418,7 +418,7 @@ TEST_P(BenchmarkClientHttpTest, StatusTrackingInOnComplete) { TEST_P(BenchmarkClientHttpTest, ConnectionPrefetching) { setupBenchmarkClient("/", false, true); client_->setConnectionLimit(50); - client_->initialize(runtime_); + client_->initialize(runtime_, tls_); EXPECT_EQ(true, client_->tryStartOne([&]() { dispatcher_->exit(); })); dispatcher_->run(Envoy::Event::Dispatcher::RunType::Block); EXPECT_EQ(50, getCounter("upstream_cx_total")); @@ -436,7 +436,7 @@ TEST_P(BenchmarkClientHttpTest, CapRequestConcurrency) { client_->setMaxPendingRequests(requests); client_->setConnectionLimit(requests); client_->setMaxActiveRequests(1); - client_->initialize(runtime_); + client_->initialize(runtime_, tls_); std::function f = [this, &inflight_response_count]() { --inflight_response_count; @@ -464,7 +464,7 @@ TEST_P(BenchmarkClientHttpsTest, MaxRequestsPerConnection) { client_->setConnectionLimit(requests); client_->setMaxActiveRequests(1024); client_->setMaxRequestsPerConnection(1); - client_->initialize(runtime_); + client_->initialize(runtime_, tls_); std::function f = [this, &inflight_response_count]() { --inflight_response_count; @@ -497,7 +497,7 @@ TEST_P(BenchmarkClientHttpTest, RequestMethodPost) { "d", client_->requestHeaders().get(Envoy::Http::LowerCaseString("c"))->value().getStringView()); - client_->initialize(runtime_); + client_->initialize(runtime_, tls_); EXPECT_EQ(true, client_->tryStartOne([&]() { dispatcher_->exit(); })); dispatcher_->run(Envoy::Event::Dispatcher::RunType::Block); diff --git a/test/mocks.h b/test/mocks.h index c850ab111..9e4efbdef 100644 --- a/test/mocks.h +++ b/test/mocks.h @@ -7,6 +7,7 @@ #include "envoy/common/time.h" #include "envoy/event/dispatcher.h" #include "envoy/stats/store.h" +#include "envoy/thread_local/thread_local.h" #include "nighthawk/client/benchmark_client.h" #include "nighthawk/client/factories.h" @@ -133,7 +134,7 @@ class MockBenchmarkClient : public Client::BenchmarkClient { public: MockBenchmarkClient(); - MOCK_METHOD1(initialize, void(Envoy::Runtime::Loader&)); + MOCK_METHOD2(initialize, void(Envoy::Runtime::Loader&, Envoy::ThreadLocal::Instance&)); MOCK_METHOD0(terminate, void()); MOCK_METHOD1(setMeasureLatencies, void(bool)); MOCK_CONST_METHOD0(statistics, StatisticPtrMap()); diff --git a/test/server/http_test_server_filter_integration_test.cc b/test/server/http_test_server_filter_integration_test.cc index a9560e410..a1c41145f 100644 --- a/test/server/http_test_server_filter_integration_test.cc +++ b/test/server/http_test_server_filter_integration_test.cc @@ -54,7 +54,7 @@ class HttpTestServerIntegrationTestBase : public Envoy::HttpIntegrationTest, type, dispatcher->createClientConnection(addr, Envoy::Network::Address::InstanceConstSharedPtr(), Envoy::Network::Test::createRawBufferSocket(), nullptr), - host_description, *dispatcher, false /* strict header validation */); + host_description, *dispatcher); Envoy::BufferingStreamDecoderPtr response( new Envoy::BufferingStreamDecoder([&client, &dispatcher]() -> void { client.close(); From 9b9b567ef39ed82a1a9ea59215ec6953af409da9 Mon Sep 17 00:00:00 2001 From: Otto van der Schaaf Date: Mon, 29 Jul 2019 23:56:49 +0200 Subject: [PATCH 23/31] Update Envoy Signed-off-by: Otto van der Schaaf --- bazel/repositories.bzl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/bazel/repositories.bzl b/bazel/repositories.bzl index 7bfed6468..207f7b173 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 = "ae6bc0185c235a68d5c3845fd54daa270f210685" -ENVOY_SHA = "512714e27baec34fdb76e0ca5bd8e0e4614ba97bb3cf0bc94f0b79818bcfd788" +ENVOY_COMMIT = "245888b2c01561670927427883dc7b9786cae13c" +ENVOY_SHA = "30ed03d1d064496679c606ca542d49d4e79455947bbe20815021774e05ca0f1d" RULES_PYTHON_COMMIT = "fdbb17a4118a1728d19e638a5291b4c4266ea5b8" RULES_PYTHON_SHA = "9a3d71e348da504a9c4c5e8abd4cb822f7afb32c613dc6ee8b8535333a81a938" From d4c88578510c8dc90a8418cda05d579979f0bc09 Mon Sep 17 00:00:00 2001 From: Otto van der Schaaf Date: Tue, 30 Jul 2019 00:27:07 +0200 Subject: [PATCH 24/31] Coverage: explicit ipv4 only (test for fixing CI) Signed-off-by: Otto van der Schaaf --- ci/do_ci.sh | 27 +++++++++++---------------- test/run_envoy_bazel_coverage.sh | 2 +- 2 files changed, 12 insertions(+), 17 deletions(-) diff --git a/ci/do_ci.sh b/ci/do_ci.sh index e5ab84e8a..15828d81c 100755 --- a/ci/do_ci.sh +++ b/ci/do_ci.sh @@ -28,17 +28,17 @@ function do_clang_tidy() { } function do_coverage() { - setup_clang_toolchain - echo "bazel coverage build with tests ${TEST_TARGETS}" - - # Reduce the amount of memory Bazel tries to use to prevent it from launching too many subprocesses. - # This should prevent the system from running out of memory and killing tasks. See discussion on - # https://github.com/envoyproxy/envoy/pull/5611. - [ -z "$CIRCLECI" ] || export BAZEL_BUILD_OPTIONS="${BAZEL_BUILD_OPTIONS} --local_ram_resources=12288" - - export TEST_TARGETS="//test/..." - test/run_envoy_bazel_coverage.sh ${TEST_TARGETS} - exit 0 + setup_clang_toolchain + echo "bazel coverage build with tests ${TEST_TARGETS}" + + # Reduce the amount of memory Bazel tries to use to prevent it from launching too many subprocesses. + # This should prevent the system from running out of memory and killing tasks. See discussion on + # https://github.com/envoyproxy/envoy/pull/5611. + [ -z "$CIRCLECI" ] || export BAZEL_BUILD_OPTIONS="${BAZEL_BUILD_OPTIONS} --local_ram_resources=12288" + + export TEST_TARGETS="//test/..." + test/run_envoy_bazel_coverage.sh ${TEST_TARGETS} + exit 0 } function setup_gcc_toolchain() { @@ -147,11 +147,6 @@ export BAZEL_TEST_OPTIONS="${BAZEL_BUILD_OPTIONS} --test_env=HOME --test_env=PYT setup_clang_toolchain - -if [ "$1" == "coverage" ]; then - setup_gcc_toolchain -fi - case "$1" in build) do_build diff --git a/test/run_envoy_bazel_coverage.sh b/test/run_envoy_bazel_coverage.sh index 0da2b8391..ce2ec84ee 100755 --- a/test/run_envoy_bazel_coverage.sh +++ b/test/run_envoy_bazel_coverage.sh @@ -30,7 +30,7 @@ BAZEL_USE_LLVM_NATIVE_COVERAGE=1 GCOV=llvm-profdata bazel coverage ${BAZEL_BUILD -c fastbuild --copt=-DNDEBUG --instrumentation_filter=//source/...,//include/... \ --test_timeout=2000 --cxxopt="-DENVOY_CONFIG_COVERAGE=1" --test_output=errors \ --test_arg="--log-path /dev/null" --test_arg="-l trace" --test_env=HEAPCHECK= \ - //test/coverage:coverage_tests + --test_env=ENVOY_IP_TEST_VERSIONS=v4only //test/coverage:coverage_tests COVERAGE_DIR="${SRCDIR}"/generated/coverage mkdir -p "${COVERAGE_DIR}" From 958d193abc8fc29f2b95ca5363a6e941ffc4f28d Mon Sep 17 00:00:00 2001 From: Otto van der Schaaf Date: Wed, 31 Jul 2019 11:03:05 +0200 Subject: [PATCH 25/31] Update Envoy & coverage threshold - the updated envoy fixes a clang-tidy issue we run in to - had to introduce some shim code which reduces the coverage % Let's see if CI is happy with these changes included. Signed-off-by: Otto van der Schaaf --- bazel/repositories.bzl | 4 ++-- test/run_envoy_bazel_coverage.sh | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/bazel/repositories.bzl b/bazel/repositories.bzl index 7bfed6468..5af5323f8 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 = "ae6bc0185c235a68d5c3845fd54daa270f210685" -ENVOY_SHA = "512714e27baec34fdb76e0ca5bd8e0e4614ba97bb3cf0bc94f0b79818bcfd788" +ENVOY_COMMIT = "bdd6788f1e01787d015eabd9902f4b565e5dea98" +ENVOY_SHA = "a53022b5c985e4c8bb999f2bed40f66a8621aea44312c8fe2fee6d65cd824da8" RULES_PYTHON_COMMIT = "fdbb17a4118a1728d19e638a5291b4c4266ea5b8" RULES_PYTHON_SHA = "9a3d71e348da504a9c4c5e8abd4cb822f7afb32c613dc6ee8b8535333a81a938" diff --git a/test/run_envoy_bazel_coverage.sh b/test/run_envoy_bazel_coverage.sh index 0da2b8391..4b3aada26 100755 --- a/test/run_envoy_bazel_coverage.sh +++ b/test/run_envoy_bazel_coverage.sh @@ -55,7 +55,7 @@ then COVERAGE_VALUE=$(llvm-cov export "${COVERAGE_BINARY}" -instr-profile="${COVERAGE_DATA}" \ -ignore-filename-regex="${COVERAGE_IGNORE_REGEX}" -summary-only | \ python3 -c "import sys, json; print(json.load(sys.stdin)['data'][0]['totals']['lines']['percent'])") - COVERAGE_THRESHOLD=97.5 + COVERAGE_THRESHOLD=95.0 COVERAGE_FAILED=$(echo "${COVERAGE_VALUE}<${COVERAGE_THRESHOLD}" | bc) if test ${COVERAGE_FAILED} -eq 1; then echo Code coverage ${COVERAGE_VALUE} is lower than limit of ${COVERAGE_THRESHOLD} From 53ae4901c2b75146a8ba19b3f877d91db2eec5a1 Mon Sep 17 00:00:00 2001 From: Otto van der Schaaf Date: Wed, 31 Jul 2019 11:33:05 +0200 Subject: [PATCH 26/31] Add include Signed-off-by: Otto van der Schaaf --- source/common/ssl.h | 1 + 1 file changed, 1 insertion(+) diff --git a/source/common/ssl.h b/source/common/ssl.h index 563dce444..2265756a7 100644 --- a/source/common/ssl.h +++ b/source/common/ssl.h @@ -6,6 +6,7 @@ #include "envoy/network/transport_socket.h" #include "common/local_info/local_info_impl.h" +#include "common/network/utility.h" #include "common/secret/secret_manager_impl.h" #include "common/singleton/manager_impl.h" #include "common/thread_local/thread_local_impl.h" From e7f33c43fbfd44f4e85b5a063029573e2b2a00c9 Mon Sep 17 00:00:00 2001 From: Otto van der Schaaf Date: Thu, 1 Aug 2019 17:49:28 +0200 Subject: [PATCH 27/31] Cover ssl shims in tests Signed-off-by: Otto van der Schaaf --- test/BUILD | 1 + test/ssl_test.cc | 93 ++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 94 insertions(+) create mode 100644 test/ssl_test.cc diff --git a/test/BUILD b/test/BUILD index 9df8accce..26a91c1c3 100644 --- a/test/BUILD +++ b/test/BUILD @@ -37,6 +37,7 @@ envoy_cc_test( "sequencer_test.cc", "service_main_test.cc", "service_test.cc", + "ssl_test.cc", "statistic_test.cc", "stream_decoder_test.cc", "utility_test.cc", diff --git a/test/ssl_test.cc b/test/ssl_test.cc new file mode 100644 index 000000000..16b25ef6c --- /dev/null +++ b/test/ssl_test.cc @@ -0,0 +1,93 @@ +#include + +#include "common/http/header_map_impl.h" +#include "common/ssl.h" + +#include "test/mocks/api/mocks.h" +#include "test/mocks/event/mocks.h" +#include "test/mocks/filesystem/mocks.h" +#include "test/mocks/thread_local/mocks.h" +#include "test/mocks/upstream/mocks.h" + +#include "gtest/gtest.h" + +using namespace testing; + +namespace Nighthawk { +namespace Ssl { + +constexpr char message[] = "panic: not implemented"; + +class SslTest : public Test {}; + +TEST_F(SslTest, FakeAdminCoverage) { + FakeAdmin admin; + Envoy::Server::Admin::HandlerCb cb; + EXPECT_DEATH(admin.addHandler("", "", cb, false, false), message); + EXPECT_DEATH(admin.removeHandler(""), message); + EXPECT_DEATH(admin.socket(), message); + EXPECT_DEATH(admin.getConfigTracker(), message); + EXPECT_DEATH(admin.startHttpListener("", "", {}, {}, {}), message); + Envoy::Http::HeaderMapImpl map; + std::string foo; + EXPECT_DEATH(admin.request("", "", map, foo), message); + EXPECT_DEATH(admin.addListenerToHandler(nullptr), message); +} + +TEST_F(SslTest, FakeClusterManager) { + FakeClusterManager manager; + EXPECT_DEATH(manager.addOrUpdateCluster({}, {}), message); + EXPECT_DEATH(manager.setInitializedCb([]() {}), message); + EXPECT_DEATH(manager.clusters(), message); + EXPECT_DEATH(manager.get(""), message); + EXPECT_DEATH(manager.httpConnPoolForCluster("", {}, {}, {}), message); + EXPECT_DEATH(manager.tcpConnPoolForCluster("", {}, {}, {}), message); + EXPECT_DEATH(manager.tcpConnForCluster("", {}, {}), message); + EXPECT_DEATH(manager.httpAsyncClientForCluster(""), message); + EXPECT_DEATH(manager.removeCluster(""), message); + EXPECT_DEATH(manager.shutdown(), message); + EXPECT_DEATH(manager.bindConfig(), message); + EXPECT_DEATH(manager.adsMux(), message); + EXPECT_DEATH(manager.grpcAsyncClientManager(), message); + EXPECT_DEATH(manager.localClusterName(), message); + Envoy::Upstream::MockClusterUpdateCallbacks cb; + EXPECT_DEATH(manager.addThreadLocalClusterUpdateCallbacks(cb), message); + EXPECT_DEATH(manager.clusterManagerFactory(), message); + EXPECT_DEATH(manager.subscriptionFactory(), message); +} + +TEST_F(SslTest, MinimalTransportSocketFactoryContextTest) { + Envoy::Stats::ScopePtr stats_scope; + Envoy::Event::MockDispatcher dispatcher; + Envoy::Runtime::MockRandomGenerator random; + Envoy::Stats::MockStore stats; + Envoy::Api::MockApi api; + Envoy::Extensions::TransportSockets::Tls::ContextManagerImpl ssl_context_manager( + api.timeSource()); + Envoy::ProtobufMessage::NullValidationVisitorImpl validation_visitor; + Envoy::ThreadLocal::MockInstance tls; + EXPECT_CALL(api, threadFactory()).WillOnce(ReturnRef(Envoy::Thread::threadFactoryForTest())); + + MinimalTransportSocketFactoryContext mtsc(std::move(stats_scope), dispatcher, random, stats, api, + ssl_context_manager, validation_visitor, tls); + + EXPECT_NO_FATAL_FAILURE(mtsc.admin()); + EXPECT_EQ(&mtsc.sslContextManager(), &ssl_context_manager); + EXPECT_NO_FATAL_FAILURE(mtsc.statsScope()); + EXPECT_NO_FATAL_FAILURE(mtsc.secretManager()); + EXPECT_NO_FATAL_FAILURE(mtsc.clusterManager()); + EXPECT_NO_FATAL_FAILURE(mtsc.localInfo()); + EXPECT_NO_FATAL_FAILURE(mtsc.dispatcher()); + EXPECT_NO_FATAL_FAILURE(mtsc.random()); + EXPECT_NO_FATAL_FAILURE(mtsc.stats()); + Envoy::Init::ManagerImpl manager("test"); + EXPECT_DEATH(mtsc.setInitManager(manager), message); + EXPECT_DEATH(mtsc.initManager(), message); + EXPECT_NO_FATAL_FAILURE(mtsc.singletonManager()); + EXPECT_NO_FATAL_FAILURE(mtsc.threadLocal()); + EXPECT_NO_FATAL_FAILURE(mtsc.api()); + EXPECT_NO_FATAL_FAILURE(mtsc.messageValidationVisitor()); +} + +} // namespace Ssl +} // namespace Nighthawk From e2ff84d4afe6641de0e713891b4634b55bbd1ded Mon Sep 17 00:00:00 2001 From: Otto van der Schaaf Date: Thu, 1 Aug 2019 19:10:18 +0200 Subject: [PATCH 28/31] Adress CI issues Signed-off-by: Otto van der Schaaf --- test/ssl_test.cc | 87 +++++++++++++++++++++++------------------------- 1 file changed, 42 insertions(+), 45 deletions(-) diff --git a/test/ssl_test.cc b/test/ssl_test.cc index 16b25ef6c..67adfb2f8 100644 --- a/test/ssl_test.cc +++ b/test/ssl_test.cc @@ -16,51 +16,48 @@ using namespace testing; namespace Nighthawk { namespace Ssl { -constexpr char message[] = "panic: not implemented"; - class SslTest : public Test {}; TEST_F(SslTest, FakeAdminCoverage) { FakeAdmin admin; Envoy::Server::Admin::HandlerCb cb; - EXPECT_DEATH(admin.addHandler("", "", cb, false, false), message); - EXPECT_DEATH(admin.removeHandler(""), message); - EXPECT_DEATH(admin.socket(), message); - EXPECT_DEATH(admin.getConfigTracker(), message); - EXPECT_DEATH(admin.startHttpListener("", "", {}, {}, {}), message); + EXPECT_DEATH(admin.addHandler("", "", cb, false, false), ""); + EXPECT_DEATH(admin.removeHandler(""), ""); + EXPECT_DEATH(admin.socket(), ""); + EXPECT_DEATH(admin.getConfigTracker(), ""); + EXPECT_DEATH(admin.startHttpListener("", "", {}, {}, {}), ""); Envoy::Http::HeaderMapImpl map; std::string foo; - EXPECT_DEATH(admin.request("", "", map, foo), message); - EXPECT_DEATH(admin.addListenerToHandler(nullptr), message); + EXPECT_DEATH(admin.request("", "", map, foo), ""); + EXPECT_DEATH(admin.addListenerToHandler(nullptr), ""); } TEST_F(SslTest, FakeClusterManager) { FakeClusterManager manager; - EXPECT_DEATH(manager.addOrUpdateCluster({}, {}), message); - EXPECT_DEATH(manager.setInitializedCb([]() {}), message); - EXPECT_DEATH(manager.clusters(), message); - EXPECT_DEATH(manager.get(""), message); - EXPECT_DEATH(manager.httpConnPoolForCluster("", {}, {}, {}), message); - EXPECT_DEATH(manager.tcpConnPoolForCluster("", {}, {}, {}), message); - EXPECT_DEATH(manager.tcpConnForCluster("", {}, {}), message); - EXPECT_DEATH(manager.httpAsyncClientForCluster(""), message); - EXPECT_DEATH(manager.removeCluster(""), message); - EXPECT_DEATH(manager.shutdown(), message); - EXPECT_DEATH(manager.bindConfig(), message); - EXPECT_DEATH(manager.adsMux(), message); - EXPECT_DEATH(manager.grpcAsyncClientManager(), message); - EXPECT_DEATH(manager.localClusterName(), message); + EXPECT_DEATH(manager.addOrUpdateCluster({}, {}), ""); + EXPECT_DEATH(manager.setInitializedCb([]() {}), ""); + EXPECT_DEATH(manager.clusters(), ""); + EXPECT_DEATH(manager.get(""), ""); + EXPECT_DEATH(manager.httpConnPoolForCluster("", {}, {}, {}), ""); + EXPECT_DEATH(manager.tcpConnPoolForCluster("", {}, {}, {}), ""); + EXPECT_DEATH(manager.tcpConnForCluster("", {}, {}), ""); + EXPECT_DEATH(manager.httpAsyncClientForCluster(""), ""); + EXPECT_DEATH(manager.removeCluster(""), ""); + EXPECT_DEATH(manager.shutdown(), ""); + EXPECT_DEATH(manager.bindConfig(), ""); + EXPECT_DEATH(manager.adsMux(), ""); + EXPECT_DEATH(manager.grpcAsyncClientManager(), ""); + EXPECT_DEATH(manager.localClusterName(), ""); Envoy::Upstream::MockClusterUpdateCallbacks cb; - EXPECT_DEATH(manager.addThreadLocalClusterUpdateCallbacks(cb), message); - EXPECT_DEATH(manager.clusterManagerFactory(), message); - EXPECT_DEATH(manager.subscriptionFactory(), message); + EXPECT_DEATH(manager.addThreadLocalClusterUpdateCallbacks(cb), ""); + EXPECT_DEATH(manager.clusterManagerFactory(), ""); + EXPECT_DEATH(manager.subscriptionFactory(), ""); } TEST_F(SslTest, MinimalTransportSocketFactoryContextTest) { - Envoy::Stats::ScopePtr stats_scope; Envoy::Event::MockDispatcher dispatcher; Envoy::Runtime::MockRandomGenerator random; - Envoy::Stats::MockStore stats; + Envoy::Stats::IsolatedStoreImpl stats; Envoy::Api::MockApi api; Envoy::Extensions::TransportSockets::Tls::ContextManagerImpl ssl_context_manager( api.timeSource()); @@ -68,25 +65,25 @@ TEST_F(SslTest, MinimalTransportSocketFactoryContextTest) { Envoy::ThreadLocal::MockInstance tls; EXPECT_CALL(api, threadFactory()).WillOnce(ReturnRef(Envoy::Thread::threadFactoryForTest())); - MinimalTransportSocketFactoryContext mtsc(std::move(stats_scope), dispatcher, random, stats, api, - ssl_context_manager, validation_visitor, tls); + MinimalTransportSocketFactoryContext mtsfc(stats.createScope(""), dispatcher, random, stats, api, + ssl_context_manager, validation_visitor, tls); - EXPECT_NO_FATAL_FAILURE(mtsc.admin()); - EXPECT_EQ(&mtsc.sslContextManager(), &ssl_context_manager); - EXPECT_NO_FATAL_FAILURE(mtsc.statsScope()); - EXPECT_NO_FATAL_FAILURE(mtsc.secretManager()); - EXPECT_NO_FATAL_FAILURE(mtsc.clusterManager()); - EXPECT_NO_FATAL_FAILURE(mtsc.localInfo()); - EXPECT_NO_FATAL_FAILURE(mtsc.dispatcher()); - EXPECT_NO_FATAL_FAILURE(mtsc.random()); - EXPECT_NO_FATAL_FAILURE(mtsc.stats()); + EXPECT_NO_FATAL_FAILURE(mtsfc.admin()); + EXPECT_EQ(&mtsfc.sslContextManager(), &ssl_context_manager); + EXPECT_NO_FATAL_FAILURE(mtsfc.statsScope()); + EXPECT_NO_FATAL_FAILURE(mtsfc.secretManager()); + EXPECT_NO_FATAL_FAILURE(mtsfc.clusterManager()); + EXPECT_NO_FATAL_FAILURE(mtsfc.localInfo()); + EXPECT_NO_FATAL_FAILURE(mtsfc.dispatcher()); + EXPECT_NO_FATAL_FAILURE(mtsfc.random()); + EXPECT_NO_FATAL_FAILURE(mtsfc.stats()); Envoy::Init::ManagerImpl manager("test"); - EXPECT_DEATH(mtsc.setInitManager(manager), message); - EXPECT_DEATH(mtsc.initManager(), message); - EXPECT_NO_FATAL_FAILURE(mtsc.singletonManager()); - EXPECT_NO_FATAL_FAILURE(mtsc.threadLocal()); - EXPECT_NO_FATAL_FAILURE(mtsc.api()); - EXPECT_NO_FATAL_FAILURE(mtsc.messageValidationVisitor()); + EXPECT_DEATH(mtsfc.setInitManager(manager), ""); + EXPECT_DEATH(mtsfc.initManager(), ""); + EXPECT_NO_FATAL_FAILURE(mtsfc.singletonManager()); + EXPECT_NO_FATAL_FAILURE(mtsfc.threadLocal()); + EXPECT_NO_FATAL_FAILURE(mtsfc.api()); + EXPECT_NO_FATAL_FAILURE(mtsfc.messageValidationVisitor()); } } // namespace Ssl From b95ce2d126e889e4203b89c8d5c61065bb2e9127 Mon Sep 17 00:00:00 2001 From: Otto van der Schaaf Date: Thu, 1 Aug 2019 20:10:27 +0200 Subject: [PATCH 29/31] Coverage fixes Signed-off-by: Otto van der Schaaf --- source/common/BUILD | 1 + source/common/ssl.cc | 159 +++++++++++++++++++++++++++++++ source/common/ssl.h | 146 ++++++++++------------------ test/run_envoy_bazel_coverage.sh | 2 +- test/ssl_test.cc | 32 +++---- 5 files changed, 226 insertions(+), 114 deletions(-) create mode 100644 source/common/ssl.cc diff --git a/source/common/BUILD b/source/common/BUILD index e94893b1f..b9f5d9126 100644 --- a/source/common/BUILD +++ b/source/common/BUILD @@ -13,6 +13,7 @@ envoy_cc_library( srcs = [ "rate_limiter_impl.cc", "sequencer_impl.cc", + "ssl.cc", "statistic_impl.cc", "uri_impl.cc", "utility.cc", diff --git a/source/common/ssl.cc b/source/common/ssl.cc new file mode 100644 index 000000000..b6564f79f --- /dev/null +++ b/source/common/ssl.cc @@ -0,0 +1,159 @@ +#include "common/ssl.h" + +namespace Nighthawk { +namespace Ssl { + +bool FakeAdmin::addHandler(const std::string&, const std::string&, Envoy::Server::Admin::HandlerCb, + bool, bool) { + return true; +}; + +bool FakeAdmin::removeHandler(const std::string&) { return true; }; + +const Envoy::Network::Socket& FakeAdmin::socket() { NOT_IMPLEMENTED_GCOVR_EXCL_LINE; }; + +Envoy::Server::ConfigTracker& FakeAdmin::getConfigTracker() { NOT_IMPLEMENTED_GCOVR_EXCL_LINE; }; + +void FakeAdmin::startHttpListener(const std::string&, const std::string&, + Envoy::Network::Address::InstanceConstSharedPtr, + const Envoy::Network::Socket::OptionsSharedPtr&, + Envoy::Stats::ScopePtr&&){}; + +Envoy::Http::Code FakeAdmin::request(absl::string_view, absl::string_view, Envoy::Http::HeaderMap&, + std::string&) { + return Envoy::Http::Code::OK; +}; + +void FakeAdmin::addListenerToHandler(Envoy::Network::ConnectionHandler*){}; + +bool FakeClusterManager::addOrUpdateCluster(const envoy::api::v2::Cluster&, const std::string&) { + return true; +} + +void FakeClusterManager::setInitializedCb(std::function) {} + +Envoy::Upstream::ClusterManager::ClusterInfoMap FakeClusterManager::clusters() { + NOT_IMPLEMENTED_GCOVR_EXCL_LINE; +} + +Envoy::Upstream::ThreadLocalCluster* FakeClusterManager::get(absl::string_view) { return nullptr; } + +Envoy::Http::ConnectionPool::Instance* + +FakeClusterManager::httpConnPoolForCluster(const std::string&, Envoy::Upstream::ResourcePriority, + Envoy::Http::Protocol, + Envoy::Upstream::LoadBalancerContext*) { + return nullptr; +} + +Envoy::Tcp::ConnectionPool::Instance* +FakeClusterManager::tcpConnPoolForCluster(const std::string&, Envoy::Upstream::ResourcePriority, + Envoy::Upstream::LoadBalancerContext*, + Envoy::Network::TransportSocketOptionsSharedPtr) { + return nullptr; +} + +Envoy::Upstream::Host::CreateConnectionData +FakeClusterManager::tcpConnForCluster(const std::string&, Envoy::Upstream::LoadBalancerContext*, + Envoy::Network::TransportSocketOptionsSharedPtr) { + NOT_IMPLEMENTED_GCOVR_EXCL_LINE; +} + +Envoy::Http::AsyncClient& FakeClusterManager::httpAsyncClientForCluster(const std::string&) { + NOT_IMPLEMENTED_GCOVR_EXCL_LINE; +} + +bool FakeClusterManager::removeCluster(const std::string&) { return true; } + +void FakeClusterManager::shutdown() {} + +const envoy::api::v2::core::BindConfig& FakeClusterManager::bindConfig() const { + NOT_IMPLEMENTED_GCOVR_EXCL_LINE; +} + +Envoy::Config::GrpcMux& FakeClusterManager::adsMux() { NOT_IMPLEMENTED_GCOVR_EXCL_LINE; } + +Envoy::Grpc::AsyncClientManager& FakeClusterManager::grpcAsyncClientManager() { + NOT_IMPLEMENTED_GCOVR_EXCL_LINE; +} + +const std::string& FakeClusterManager::localClusterName() const { return foo_string_; } + +Envoy::Upstream::ClusterUpdateCallbacksHandlePtr +FakeClusterManager::addThreadLocalClusterUpdateCallbacks(Envoy::Upstream::ClusterUpdateCallbacks&) { + return nullptr; +} + +Envoy::Upstream::ClusterManagerFactory& FakeClusterManager::clusterManagerFactory() { + NOT_IMPLEMENTED_GCOVR_EXCL_LINE; +} + +Envoy::Config::SubscriptionFactory& FakeClusterManager::subscriptionFactory() { + NOT_IMPLEMENTED_GCOVR_EXCL_LINE; +} + +std::size_t FakeClusterManager::warmingClusterCount() const { return 0u; } + +MinimalTransportSocketFactoryContext::MinimalTransportSocketFactoryContext( + Envoy::Stats::ScopePtr&& stats_scope, Envoy::Event::Dispatcher& dispatcher, + Envoy::Runtime::RandomGenerator& random, Envoy::Stats::Store& stats, Envoy::Api::Api& api, + Envoy::Extensions::TransportSockets::Tls::ContextManagerImpl& ssl_context_manager, + Envoy::ProtobufMessage::ValidationVisitor& validation_visitor, + Envoy::ThreadLocal::Instance& tls) + : ssl_context_manager_(ssl_context_manager), stats_scope_(std::move(stats_scope)), + secret_manager_(config_tracker_), dispatcher_(dispatcher), random_(random), stats_(stats), + api_(api), validation_visitor_(validation_visitor), + local_info_({}, + Envoy::Network::Utility::getLocalAddress(Envoy::Network::Address::IpVersion::v4), + "nighthawk_service_zone", "nighthawk_service_cluster", "nighthawk_service_node"), + manager_(api_.threadFactory()), tls_(tls) {} + +Envoy::Server::Admin& MinimalTransportSocketFactoryContext::admin() { return admin_; } + +Envoy::Ssl::ContextManager& MinimalTransportSocketFactoryContext::sslContextManager() { + return ssl_context_manager_; +} + +Envoy::Stats::Scope& MinimalTransportSocketFactoryContext::statsScope() const { + return *stats_scope_; +} + +Envoy::Secret::SecretManager& MinimalTransportSocketFactoryContext::secretManager() { + return secret_manager_; +} + +Envoy::Upstream::ClusterManager& MinimalTransportSocketFactoryContext::clusterManager() { + return cluster_manager_; +} + +const Envoy::LocalInfo::LocalInfo& MinimalTransportSocketFactoryContext::localInfo() { + return local_info_; +} + +Envoy::Event::Dispatcher& MinimalTransportSocketFactoryContext::dispatcher() { return dispatcher_; } + +Envoy::Runtime::RandomGenerator& MinimalTransportSocketFactoryContext::random() { return random_; } + +Envoy::Stats::Store& MinimalTransportSocketFactoryContext::stats() { return stats_; } + +void MinimalTransportSocketFactoryContext::setInitManager(Envoy::Init::Manager&) {} + +Envoy::Init::Manager* MinimalTransportSocketFactoryContext::initManager() { return nullptr; } + +Envoy::Singleton::Manager& MinimalTransportSocketFactoryContext::singletonManager() { + return manager_; +} + +Envoy::ThreadLocal::SlotAllocator& MinimalTransportSocketFactoryContext::threadLocal() { + return tls_; +} + +Envoy::Api::Api& MinimalTransportSocketFactoryContext::api() { return api_; } + +Envoy::ProtobufMessage::ValidationVisitor& +MinimalTransportSocketFactoryContext::messageValidationVisitor() { + return validation_visitor_; +} + +} // namespace Ssl +} // namespace Nighthawk \ No newline at end of file diff --git a/source/common/ssl.h b/source/common/ssl.h index 2265756a7..2e26c10c7 100644 --- a/source/common/ssl.h +++ b/source/common/ssl.h @@ -22,82 +22,57 @@ namespace Nighthawk { namespace Ssl { +// Shim class that we aneed a concrete implementations for, but +// which isn't actually used. class FakeAdmin : public Envoy::Server::Admin { public: bool addHandler(const std::string&, const std::string&, Envoy::Server::Admin::HandlerCb, bool, - bool) override { - NOT_IMPLEMENTED_GCOVR_EXCL_LINE; - }; - bool removeHandler(const std::string&) override { NOT_IMPLEMENTED_GCOVR_EXCL_LINE; }; - const Envoy::Network::Socket& socket() override { NOT_IMPLEMENTED_GCOVR_EXCL_LINE; }; - Envoy::Server::ConfigTracker& getConfigTracker() override { NOT_IMPLEMENTED_GCOVR_EXCL_LINE; }; + bool) override; + bool removeHandler(const std::string&) override; + const Envoy::Network::Socket& socket() override; + Envoy::Server::ConfigTracker& getConfigTracker() override; void startHttpListener(const std::string&, const std::string&, Envoy::Network::Address::InstanceConstSharedPtr, const Envoy::Network::Socket::OptionsSharedPtr&, - Envoy::Stats::ScopePtr&&) override { - NOT_IMPLEMENTED_GCOVR_EXCL_LINE; - }; + Envoy::Stats::ScopePtr&&) override; Envoy::Http::Code request(absl::string_view, absl::string_view, Envoy::Http::HeaderMap&, - std::string&) override { - NOT_IMPLEMENTED_GCOVR_EXCL_LINE; - }; - void addListenerToHandler(Envoy::Network::ConnectionHandler*) override { - NOT_IMPLEMENTED_GCOVR_EXCL_LINE; - }; + std::string&) override; + void addListenerToHandler(Envoy::Network::ConnectionHandler*) override; }; +// Shim class that we aneed a concrete implementations for, but +// which isn't actually used. class FakeClusterManager : public Envoy::Upstream::ClusterManager { public: - bool addOrUpdateCluster(const envoy::api::v2::Cluster&, const std::string&) override { - NOT_IMPLEMENTED_GCOVR_EXCL_LINE; - } - void setInitializedCb(std::function) override { NOT_IMPLEMENTED_GCOVR_EXCL_LINE; } - Envoy::Upstream::ClusterManager::ClusterInfoMap clusters() override { - NOT_IMPLEMENTED_GCOVR_EXCL_LINE; - } - Envoy::Upstream::ThreadLocalCluster* get(absl::string_view) override { - NOT_IMPLEMENTED_GCOVR_EXCL_LINE; - } + bool addOrUpdateCluster(const envoy::api::v2::Cluster&, const std::string&) override; + void setInitializedCb(std::function) override; + Envoy::Upstream::ClusterManager::ClusterInfoMap clusters() override; + Envoy::Upstream::ThreadLocalCluster* get(absl::string_view) override; Envoy::Http::ConnectionPool::Instance* httpConnPoolForCluster(const std::string&, Envoy::Upstream::ResourcePriority, - Envoy::Http::Protocol, Envoy::Upstream::LoadBalancerContext*) override { - NOT_IMPLEMENTED_GCOVR_EXCL_LINE; - } + Envoy::Http::Protocol, Envoy::Upstream::LoadBalancerContext*) override; Envoy::Tcp::ConnectionPool::Instance* tcpConnPoolForCluster(const std::string&, Envoy::Upstream::ResourcePriority, Envoy::Upstream::LoadBalancerContext*, - Envoy::Network::TransportSocketOptionsSharedPtr) override { - NOT_IMPLEMENTED_GCOVR_EXCL_LINE; - } + Envoy::Network::TransportSocketOptionsSharedPtr) override; Envoy::Upstream::Host::CreateConnectionData tcpConnForCluster(const std::string&, Envoy::Upstream::LoadBalancerContext*, - Envoy::Network::TransportSocketOptionsSharedPtr) override { - NOT_IMPLEMENTED_GCOVR_EXCL_LINE; - } - Envoy::Http::AsyncClient& httpAsyncClientForCluster(const std::string&) override { - NOT_IMPLEMENTED_GCOVR_EXCL_LINE; - } - bool removeCluster(const std::string&) override { NOT_IMPLEMENTED_GCOVR_EXCL_LINE; } - void shutdown() override { NOT_IMPLEMENTED_GCOVR_EXCL_LINE; } - const envoy::api::v2::core::BindConfig& bindConfig() const override { - NOT_IMPLEMENTED_GCOVR_EXCL_LINE; - } - Envoy::Config::GrpcMux& adsMux() override { NOT_IMPLEMENTED_GCOVR_EXCL_LINE; } - Envoy::Grpc::AsyncClientManager& grpcAsyncClientManager() override { - NOT_IMPLEMENTED_GCOVR_EXCL_LINE; - } - const std::string& localClusterName() const override { NOT_IMPLEMENTED_GCOVR_EXCL_LINE; } + Envoy::Network::TransportSocketOptionsSharedPtr) override; + Envoy::Http::AsyncClient& httpAsyncClientForCluster(const std::string&) override; + bool removeCluster(const std::string&) override; + void shutdown() override; + const envoy::api::v2::core::BindConfig& bindConfig() const override; + Envoy::Config::GrpcMux& adsMux() override; + Envoy::Grpc::AsyncClientManager& grpcAsyncClientManager() override; + const std::string& localClusterName() const override; Envoy::Upstream::ClusterUpdateCallbacksHandlePtr - addThreadLocalClusterUpdateCallbacks(Envoy::Upstream::ClusterUpdateCallbacks&) override { - NOT_IMPLEMENTED_GCOVR_EXCL_LINE; - } - Envoy::Upstream::ClusterManagerFactory& clusterManagerFactory() override { - NOT_IMPLEMENTED_GCOVR_EXCL_LINE; - } - Envoy::Config::SubscriptionFactory& subscriptionFactory() override { - NOT_IMPLEMENTED_GCOVR_EXCL_LINE; - } - std::size_t warmingClusterCount() const override { NOT_IMPLEMENTED_GCOVR_EXCL_LINE; } + addThreadLocalClusterUpdateCallbacks(Envoy::Upstream::ClusterUpdateCallbacks&) override; + Envoy::Upstream::ClusterManagerFactory& clusterManagerFactory() override; + Envoy::Config::SubscriptionFactory& subscriptionFactory() override; + std::size_t warmingClusterCount() const override; + +private: + std::string foo_string_; }; class MinimalTransportSocketFactoryContext @@ -108,46 +83,23 @@ class MinimalTransportSocketFactoryContext Envoy::Runtime::RandomGenerator& random, Envoy::Stats::Store& stats, Envoy::Api::Api& api, Envoy::Extensions::TransportSockets::Tls::ContextManagerImpl& ssl_context_manager, Envoy::ProtobufMessage::ValidationVisitor& validation_visitor, - Envoy::ThreadLocal::Instance& tls) - : ssl_context_manager_(ssl_context_manager), stats_scope_(std::move(stats_scope)), - secret_manager_(config_tracker_), dispatcher_(dispatcher), random_(random), stats_(stats), - api_(api), validation_visitor_(validation_visitor), - local_info_( - {}, Envoy::Network::Utility::getLocalAddress(Envoy::Network::Address::IpVersion::v4), - "nighthawk_service_zone", "nighthawk_service_cluster", "nighthawk_service_node"), - manager_(api_.threadFactory()), tls_(tls) {} - - Envoy::Server::Admin& admin() override { return admin_; } - - Envoy::Ssl::ContextManager& sslContextManager() override { return ssl_context_manager_; } - - Envoy::Stats::Scope& statsScope() const override { return *stats_scope_; } - - Envoy::Secret::SecretManager& secretManager() override { return secret_manager_; } - - Envoy::Upstream::ClusterManager& clusterManager() override { return cluster_manager_; } - - const Envoy::LocalInfo::LocalInfo& localInfo() override { return local_info_; } - - Envoy::Event::Dispatcher& dispatcher() override { return dispatcher_; } - - Envoy::Runtime::RandomGenerator& random() override { return random_; } - - Envoy::Stats::Store& stats() override { return stats_; } - - void setInitManager(Envoy::Init::Manager&) override { NOT_IMPLEMENTED_GCOVR_EXCL_LINE; } - - Envoy::Init::Manager* initManager() override { NOT_IMPLEMENTED_GCOVR_EXCL_LINE; } - - Envoy::Singleton::Manager& singletonManager() override { return manager_; } - - Envoy::ThreadLocal::SlotAllocator& threadLocal() override { return tls_; } - - Envoy::Api::Api& api() override { return api_; } - - Envoy::ProtobufMessage::ValidationVisitor& messageValidationVisitor() override { - return validation_visitor_; - } + Envoy::ThreadLocal::Instance& tls); + + Envoy::Server::Admin& admin() override; + Envoy::Ssl::ContextManager& sslContextManager() override; + Envoy::Stats::Scope& statsScope() const override; + Envoy::Secret::SecretManager& secretManager() override; + Envoy::Upstream::ClusterManager& clusterManager() override; + const Envoy::LocalInfo::LocalInfo& localInfo() override; + Envoy::Event::Dispatcher& dispatcher() override; + Envoy::Runtime::RandomGenerator& random() override; + Envoy::Stats::Store& stats() override; + void setInitManager(Envoy::Init::Manager&) override; + Envoy::Init::Manager* initManager() override; + Envoy::Singleton::Manager& singletonManager() override; + Envoy::ThreadLocal::SlotAllocator& threadLocal() override; + Envoy::Api::Api& api() override; + Envoy::ProtobufMessage::ValidationVisitor& messageValidationVisitor() override; private: Envoy::Extensions::TransportSockets::Tls::ContextManagerImpl& ssl_context_manager_; diff --git a/test/run_envoy_bazel_coverage.sh b/test/run_envoy_bazel_coverage.sh index 20d784b2f..98c9389e2 100755 --- a/test/run_envoy_bazel_coverage.sh +++ b/test/run_envoy_bazel_coverage.sh @@ -55,7 +55,7 @@ then COVERAGE_VALUE=$(llvm-cov export "${COVERAGE_BINARY}" -instr-profile="${COVERAGE_DATA}" \ -ignore-filename-regex="${COVERAGE_IGNORE_REGEX}" -summary-only | \ python3 -c "import sys, json; print(json.load(sys.stdin)['data'][0]['totals']['lines']['percent'])") - COVERAGE_THRESHOLD=95.0 + COVERAGE_THRESHOLD=97.1 COVERAGE_FAILED=$(echo "${COVERAGE_VALUE}<${COVERAGE_THRESHOLD}" | bc) if test ${COVERAGE_FAILED} -eq 1; then echo Code coverage ${COVERAGE_VALUE} is lower than limit of ${COVERAGE_THRESHOLD} diff --git a/test/ssl_test.cc b/test/ssl_test.cc index 67adfb2f8..482db865f 100644 --- a/test/ssl_test.cc +++ b/test/ssl_test.cc @@ -21,35 +21,35 @@ class SslTest : public Test {}; TEST_F(SslTest, FakeAdminCoverage) { FakeAdmin admin; Envoy::Server::Admin::HandlerCb cb; - EXPECT_DEATH(admin.addHandler("", "", cb, false, false), ""); - EXPECT_DEATH(admin.removeHandler(""), ""); + EXPECT_NO_FATAL_FAILURE(admin.addHandler("", "", cb, false, false)); + EXPECT_NO_FATAL_FAILURE(admin.removeHandler("")); EXPECT_DEATH(admin.socket(), ""); EXPECT_DEATH(admin.getConfigTracker(), ""); - EXPECT_DEATH(admin.startHttpListener("", "", {}, {}, {}), ""); + EXPECT_NO_FATAL_FAILURE(admin.startHttpListener("", "", {}, {}, {})); Envoy::Http::HeaderMapImpl map; std::string foo; - EXPECT_DEATH(admin.request("", "", map, foo), ""); - EXPECT_DEATH(admin.addListenerToHandler(nullptr), ""); + EXPECT_NO_FATAL_FAILURE(admin.request("", "", map, foo)); + EXPECT_NO_FATAL_FAILURE(admin.addListenerToHandler(nullptr)); } TEST_F(SslTest, FakeClusterManager) { FakeClusterManager manager; - EXPECT_DEATH(manager.addOrUpdateCluster({}, {}), ""); - EXPECT_DEATH(manager.setInitializedCb([]() {}), ""); + EXPECT_NO_FATAL_FAILURE(manager.addOrUpdateCluster({}, {})); + EXPECT_NO_FATAL_FAILURE(manager.setInitializedCb([]() {})); EXPECT_DEATH(manager.clusters(), ""); - EXPECT_DEATH(manager.get(""), ""); - EXPECT_DEATH(manager.httpConnPoolForCluster("", {}, {}, {}), ""); - EXPECT_DEATH(manager.tcpConnPoolForCluster("", {}, {}, {}), ""); + EXPECT_NO_FATAL_FAILURE(manager.get("")); + EXPECT_NO_FATAL_FAILURE(manager.httpConnPoolForCluster("", {}, {}, {})); + EXPECT_NO_FATAL_FAILURE(manager.tcpConnPoolForCluster("", {}, {}, {})); EXPECT_DEATH(manager.tcpConnForCluster("", {}, {}), ""); EXPECT_DEATH(manager.httpAsyncClientForCluster(""), ""); - EXPECT_DEATH(manager.removeCluster(""), ""); - EXPECT_DEATH(manager.shutdown(), ""); + EXPECT_NO_FATAL_FAILURE(manager.removeCluster("")); + EXPECT_NO_FATAL_FAILURE(manager.shutdown()); EXPECT_DEATH(manager.bindConfig(), ""); EXPECT_DEATH(manager.adsMux(), ""); EXPECT_DEATH(manager.grpcAsyncClientManager(), ""); - EXPECT_DEATH(manager.localClusterName(), ""); + EXPECT_NO_FATAL_FAILURE(manager.localClusterName()); Envoy::Upstream::MockClusterUpdateCallbacks cb; - EXPECT_DEATH(manager.addThreadLocalClusterUpdateCallbacks(cb), ""); + EXPECT_NO_FATAL_FAILURE(manager.addThreadLocalClusterUpdateCallbacks(cb)); EXPECT_DEATH(manager.clusterManagerFactory(), ""); EXPECT_DEATH(manager.subscriptionFactory(), ""); } @@ -78,8 +78,8 @@ TEST_F(SslTest, MinimalTransportSocketFactoryContextTest) { EXPECT_NO_FATAL_FAILURE(mtsfc.random()); EXPECT_NO_FATAL_FAILURE(mtsfc.stats()); Envoy::Init::ManagerImpl manager("test"); - EXPECT_DEATH(mtsfc.setInitManager(manager), ""); - EXPECT_DEATH(mtsfc.initManager(), ""); + EXPECT_NO_FATAL_FAILURE(mtsfc.setInitManager(manager)); + EXPECT_NO_FATAL_FAILURE(mtsfc.initManager()); EXPECT_NO_FATAL_FAILURE(mtsfc.singletonManager()); EXPECT_NO_FATAL_FAILURE(mtsfc.threadLocal()); EXPECT_NO_FATAL_FAILURE(mtsfc.api()); From 4f44f5110450872a79fccbdbad3647c8489996fe Mon Sep 17 00:00:00 2001 From: Otto van der Schaaf Date: Thu, 1 Aug 2019 20:29:11 +0200 Subject: [PATCH 30/31] Set coverage threshold to 97.0 Signed-off-by: Otto van der Schaaf --- test/run_envoy_bazel_coverage.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/run_envoy_bazel_coverage.sh b/test/run_envoy_bazel_coverage.sh index 98c9389e2..a14194dd0 100755 --- a/test/run_envoy_bazel_coverage.sh +++ b/test/run_envoy_bazel_coverage.sh @@ -55,7 +55,7 @@ then COVERAGE_VALUE=$(llvm-cov export "${COVERAGE_BINARY}" -instr-profile="${COVERAGE_DATA}" \ -ignore-filename-regex="${COVERAGE_IGNORE_REGEX}" -summary-only | \ python3 -c "import sys, json; print(json.load(sys.stdin)['data'][0]['totals']['lines']['percent'])") - COVERAGE_THRESHOLD=97.1 + COVERAGE_THRESHOLD=97.0 COVERAGE_FAILED=$(echo "${COVERAGE_VALUE}<${COVERAGE_THRESHOLD}" | bc) if test ${COVERAGE_FAILED} -eq 1; then echo Code coverage ${COVERAGE_VALUE} is lower than limit of ${COVERAGE_THRESHOLD} From 0d73cfee4d062043a36b37e22e6f63d441b0e3e7 Mon Sep 17 00:00:00 2001 From: Otto van der Schaaf Date: Thu, 1 Aug 2019 21:01:00 +0200 Subject: [PATCH 31/31] A couple of small cleanups Signed-off-by: Otto van der Schaaf --- ci/do_ci.sh | 9 ++------- integration/integration_test_fixtures.py | 1 - source/common/rate_limiter_impl.cc | 4 +++- test/rate_limiter_test.cc | 6 ++---- 4 files changed, 7 insertions(+), 13 deletions(-) diff --git a/ci/do_ci.sh b/ci/do_ci.sh index 15828d81c..fe99aec28 100755 --- a/ci/do_ci.sh +++ b/ci/do_ci.sh @@ -4,7 +4,6 @@ set -e export BUILDIFIER_BIN="/usr/local/bin/buildifier" - function do_build () { bazel build $BAZEL_BUILD_OPTIONS --verbose_failures=true //:nighthawk_client //:nighthawk_test_server \ //:nighthawk_service @@ -107,11 +106,7 @@ if [ -n "$CIRCLECI" ]; then mv "${HOME:-/root}/.gitconfig" "${HOME:-/root}/.gitconfig_save" echo 1 fi - NUM_CPUS=8 - if [ "$1" == "coverage" ]; then - NUM_CPUS=6 - fi fi if grep 'docker\|lxc' /proc/1/cgroup; then @@ -122,14 +117,14 @@ if grep 'docker\|lxc' /proc/1/cgroup; then mkdir -p "${FAKE_HOME}" export HOME="${FAKE_HOME}" export PYTHONUSERBASE="${FAKE_HOME}" - + export BUILD_DIR=/build if [[ ! -d "${BUILD_DIR}" ]] then echo "${BUILD_DIR} mount missing - did you forget -v :${BUILD_DIR}? Creating." mkdir -p "${BUILD_DIR}" fi - + # Environment setup. export USER=bazel export TEST_TMPDIR=/build/tmp diff --git a/integration/integration_test_fixtures.py b/integration/integration_test_fixtures.py index 5f85493e1..38b6eb6c3 100644 --- a/integration/integration_test_fixtures.py +++ b/integration/integration_test_fixtures.py @@ -130,7 +130,6 @@ def runNighthawkClient(self, args, expect_failure=False, timeout=30): Runs Nighthawk against the test server, returning a json-formatted result. If the timeout is exceeded an exception will be raised. """ - self.assertTrue(os.path.exists(self.nighthawk_client_path)) if IntegrationTestBase.ip_version == IpVersion.IPV6: args.insert(0, "--address-family v6") diff --git a/source/common/rate_limiter_impl.cc b/source/common/rate_limiter_impl.cc index bc7f8a9b3..b75b86d37 100644 --- a/source/common/rate_limiter_impl.cc +++ b/source/common/rate_limiter_impl.cc @@ -52,7 +52,9 @@ void BurstingRateLimiter::releaseOne() { LinearRateLimiter::LinearRateLimiter(Envoy::TimeSource& time_source, const Frequency frequency) : time_source_(time_source), acquireable_count_(0), acquired_count_(0), frequency_(frequency) { - ASSERT(frequency.value() > 0, "Frequency must be > 0"); + if (frequency.value() <= 0) { + throw NighthawkException("Frequency must be > 0"); + } } bool LinearRateLimiter::tryAcquireOne() { diff --git a/test/rate_limiter_test.cc b/test/rate_limiter_test.cc index 135d9ceeb..e45ad427f 100644 --- a/test/rate_limiter_test.cc +++ b/test/rate_limiter_test.cc @@ -35,11 +35,9 @@ TEST_F(RateLimiterTest, LinearRateLimiterTest) { EXPECT_FALSE(rate_limiter.tryAcquireOne()); } -// TODO(oschaaf): this fails with the new coverage testing method. -// Disabled it for now. -TEST_F(RateLimiterTest, DISABLED_LinearRateLimiterInvalidArgumentTest) { +TEST_F(RateLimiterTest, LinearRateLimiterInvalidArgumentTest) { Envoy::Event::SimulatedTimeSystem time_system; - ASSERT_DEATH(LinearRateLimiter rate_limiter(time_system, 0_Hz), "Frequency must be > 0"); + EXPECT_THROW(LinearRateLimiter rate_limiter(time_system, 0_Hz), NighthawkException); } TEST_F(RateLimiterTest, BurstingRateLimiterTest) {