More unit test & tweaks

- unit test signal handler - more DistributedProcessImpl unit tests - tweak .bazelrc: use newer dockage image for docker builds/tests to resolve golang/go#37436 Signed-off-by: Otto van der Schaaf <[email protected]>
oschaaf · Dec 21, 2020 · d651918 · d651918
1 parent 1732d5a
commit d651918
Show file tree

Hide file tree

Showing 6 changed files with 130 additions and 12 deletions.
diff --git a/.bazelrc b/.bazelrc
@@ -254,7 +254,8 @@ build:remote-clang-cl --config=rbe-toolchain-clang-cl
 
 # Docker sandbox
 # NOTE: Update this from https://github.com/envoyproxy/envoy-build-tools/blob/master/toolchains/rbe_toolchains_config.bzl#L8
-build:docker-sandbox --experimental_docker_image=envoyproxy/envoy-build-ubuntu:19a268cfe3d12625380e7c61d2467c8779b58b56
+# We diverge on this SHA to unbreak local docker runs on buggy kernels. See https://github.com/golang/go/issues/37436
+build:docker-sandbox --experimental_docker_image=envoyproxy/envoy-build-ubuntu:3972d4b7ce29ba68e2aeb59186d6565cd3c4f6da
 build:docker-sandbox --spawn_strategy=docker
 build:docker-sandbox --strategy=Javac=docker
 build:docker-sandbox --strategy=Closure=docker

diff --git a/source/client/distributed_process_impl.cc b/source/client/distributed_process_impl.cc
@@ -36,6 +36,14 @@ DistributedProcessImpl::sendDistributedRequest(
 
 bool DistributedProcessImpl::run(OutputCollector& collector) {
   Nighthawk::Client::CommandLineOptionsPtr options = options_.toCommandLineOptions();
+
+  if (!options_.sink().has_value()) {
+    // TODO(XXX): without a sink, the request above could yield a full execution response,
+    // Alternatively, we error out completely and reject early instead today.
+    ENVOY_LOG(error, "Distributed request MUST have a sink configured today.");
+    return false;
+  }
+
   ::nighthawk::client::DistributedRequest request;
   *(request.mutable_execution_request()->mutable_start_request()->mutable_options()) = *options;
   const std::string kTestId = "test-execution-id";
@@ -59,14 +67,7 @@ bool DistributedProcessImpl::run(OutputCollector& collector) {
   ::nighthawk::client::DistributedRequest distributed_sink_request;
   ::nighthawk::client::SinkRequest sink_request;
 
-  if (options_.sink().has_value()) {
-    distributed_sink_request.add_services()->MergeFrom(options_.sink().value().address());
-  } else {
-    // TODO(XXX): without a sink, the request above should yield a full execution response,
-    // or we error out completely and reject earlier.
-    ENVOY_LOG(error, "Distributed request MUST have a sink configured today.");
-    return false;
-  }
+  distributed_sink_request.add_services()->MergeFrom(options_.sink().value().address());
   sink_request.set_execution_id(kTestId);
   *(distributed_sink_request.mutable_sink_request()) = sink_request;
 

diff --git a/source/common/nighthawk_distributor_client_impl.cc b/source/common/nighthawk_distributor_client_impl.cc
@@ -14,9 +14,10 @@ NighthawkDistributorClientImpl::DistributedRequest(
       stream(nighthawk_distributor_stub.DistributedRequestStream(&context));
   ENVOY_LOG_MISC(trace, "Write {}", distributed_request.DebugString());
   if (!stream->Write(distributed_request)) {
-    return absl::UnavailableError("Failed to write request to the Nighthawk Service gRPC channel.");
+    return absl::UnavailableError(
+        "Failed to write request to the Nighthawk Distributor gRPC channel.");
   } else if (!stream->WritesDone()) {
-    return absl::InternalError("WritesDone() failed on the Nighthawk Service gRPC channel.");
+    return absl::InternalError("WritesDone() failed on the Nighthawk Distributor gRPC channel.");
   }
 
   bool got_response = false;

diff --git a/test/common/BUILD b/test/common/BUILD
@@ -69,3 +69,12 @@ envoy_cc_test(
         "@envoy//source/common/common:random_generator_lib_with_external_headers",
     ],
 )
+
+envoy_cc_test(
+    name = "signal_handler_test",
+    srcs = ["signal_handler_test.cc"],
+    repository = "@envoy",
+    deps = [
+        "//source/common:nighthawk_common_lib",
+    ],
+)
diff --git a/test/common/signal_handler_test.cc b/test/common/signal_handler_test.cc
@@ -0,0 +1,35 @@
+#include <csignal>
+#include <future>
+
+#include "common/signal_handler.h"
+
+#include "gtest/gtest.h"
+
+namespace Nighthawk {
+namespace {
+
+TEST(SignalHandlerTest, SignalGetsHandled) {
+  for (const auto signal : {SIGTERM, SIGINT}) {
+    bool signal_handled = false;
+    std::promise<void> signal_all_threads_running;
+
+    SignalHandler signal_handler([&signal_handled, &signal_all_threads_running]() {
+      signal_handled = true;
+      signal_all_threads_running.set_value();
+    });
+    std::raise(signal);
+    signal_all_threads_running.get_future().wait();
+    EXPECT_TRUE(signal_handled);
+  }
+}
+
+TEST(SignalHandlerTest, DeststructDoesNotFireHandler) {
+  bool signal_handled = false;
+  {
+    SignalHandler signal_handler([&signal_handled]() { signal_handled = true; });
+  }
+  EXPECT_FALSE(signal_handled);
+}
+
+} // namespace
+} // namespace Nighthawk
diff --git a/test/distributed_process_impl_test.cc b/test/distributed_process_impl_test.cc
@@ -40,7 +40,8 @@ TEST_F(DistributedProcessImplTest, InitDistributedExecutionAndQuerySink) {
   // - One to query the sink afterwards.
   // When that finishes, we test expectations for execution-id's and sink-result handling.
 
-  OptionsPtr options = TestUtility::createOptionsImpl("foo --sink bar:443 https://foo/");
+  OptionsPtr options =
+      TestUtility::createOptionsImpl("foo --sink bar:443 https://foo/ --services service1:444");
   nighthawk::client::MockNighthawkDistributorStub stub;
   DistributedProcessImpl process(*options, stub);
   OutputCollectorImpl collector(simTime(), *options);
@@ -103,6 +104,76 @@ TEST_F(DistributedProcessImplTest, InitDistributedExecutionAndQuerySink) {
   process.shutdown();
 }
 
+TEST_F(DistributedProcessImplTest, WriteFailureOnDistributorLoadTestInitiations) {
+  OptionsPtr options = TestUtility::createOptionsImpl("foo --sink bar:443 https://foo/");
+  nighthawk::client::MockNighthawkDistributorStub stub;
+  DistributedProcessImpl process(*options, stub);
+  OutputCollectorImpl collector(simTime(), *options);
+  DistributedRequest observed_init_request;
+
+  EXPECT_CALL(stub, DistributedRequestStreamRaw)
+      .WillOnce([&observed_init_request](grpc_impl::ClientContext*) {
+        // Simulate a write failure on the load test initiation request.
+        auto* mock_reader_writer =
+            new MockClientReaderWriter<DistributedRequest, DistributedResponse>();
+        EXPECT_CALL(*mock_reader_writer, Write(_, _))
+            .WillOnce(
+                ::testing::DoAll(::testing::SaveArg<0>(&observed_init_request), Return(false)));
+        return mock_reader_writer;
+      });
+
+  EXPECT_FALSE(process.run(collector));
+  process.shutdown();
+}
+
+TEST_F(DistributedProcessImplTest, WriteFailureOnSinkRequest) {
+  OptionsPtr options = TestUtility::createOptionsImpl("foo --sink bar:443 https://foo/");
+  nighthawk::client::MockNighthawkDistributorStub stub;
+  DistributedProcessImpl process(*options, stub);
+  OutputCollectorImpl collector(simTime(), *options);
+  DistributedRequest observed_init_request;
+  DistributedRequest observed_sink_request;
+
+  EXPECT_CALL(stub, DistributedRequestStreamRaw)
+      .Times(2)
+      .WillOnce([&observed_init_request](grpc_impl::ClientContext*) {
+        auto* mock_reader_writer =
+            new MockClientReaderWriter<DistributedRequest, DistributedResponse>();
+        DistributedResponse dictated_response;
+        EXPECT_CALL(*mock_reader_writer, Read(_))
+            .WillOnce(
+                ::testing::DoAll(::testing::SetArgPointee<0>(dictated_response), Return(true)))
+            .WillOnce(Return(false));
+        EXPECT_CALL(*mock_reader_writer, Write(_, _))
+            .WillOnce(
+                ::testing::DoAll(::testing::SaveArg<0>(&observed_init_request), Return(true)));
+        EXPECT_CALL(*mock_reader_writer, WritesDone()).WillOnce(Return(true));
+        EXPECT_CALL(*mock_reader_writer, Finish()).WillOnce(Return(::grpc::Status::OK));
+        return mock_reader_writer;
+      })
+      .WillOnce([&observed_sink_request](grpc_impl::ClientContext*) {
+        // Simulate a write failure on the sink request.
+        auto* mock_reader_writer =
+            new MockClientReaderWriter<DistributedRequest, DistributedResponse>();
+        EXPECT_CALL(*mock_reader_writer, Write(_, _))
+            .WillOnce(
+                ::testing::DoAll(::testing::SaveArg<0>(&observed_sink_request), Return(false)));
+        return mock_reader_writer;
+      });
+
+  EXPECT_FALSE(process.run(collector));
+  process.shutdown();
+}
+
+TEST_F(DistributedProcessImplTest, NoSinkConfigurationResultsInFailure) {
+  // Not specifying a sink configuration should fail, at least today.
+  OptionsPtr options = TestUtility::createOptionsImpl("foo https://foo/");
+  OutputCollectorImpl collector(simTime(), *options);
+  nighthawk::client::MockNighthawkDistributorStub stub;
+  DistributedProcessImpl process(*options, stub);
+  EXPECT_FALSE(process.run(collector));
+}
+
 TEST_F(DistributedProcessImplTest, RequestExecutionCancellationDoesNotCrash) {
   // This call isn't supported yet, and issues a log warning up usage. We don't expect great things
   // from it, just that it doesn't crash, even when called at an inappropriate time like here where