From 01aa3f8204c300efd11d936348642fe45d69beca Mon Sep 17 00:00:00 2001
From: alyssawilk <alyssar@google.com>
Date: Thu, 30 Aug 2018 17:45:01 -0400
Subject: [PATCH] test: hopefully deflaking echo integration test (#4304)

At least one failure mode is that when the listener was released, some other test would yoink the released port, and the "make sure we can not connect to a removed listener" check would unexpectedly result in a connection. Running the test as exclusive should fix that particular failure mode, and allow us to see if others exist.

I believe the reason the test was flaking more often when run in parallel with -l trace is because the test ran more slowly, the lag between the listener releasing the port and the raw connection driver increased, so the likelihood that another test would snag the port also increased.

Risk Level: Low (test only)
Testing: 1000 runs with "exclusive"
Docs Changes: n/a
Release Notes: n/a

Fixes #3997

Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
---
 test/integration/BUILD                    |  2 ++
 test/integration/echo_integration_test.cc | 23 +++++++++++++++++++----
 test/integration/utility.cc               |  2 +-
 test/integration/utility.h                |  3 ++-
 4 files changed, 24 insertions(+), 6 deletions(-)

diff --git a/test/integration/BUILD b/test/integration/BUILD
index 7d6409f32778..343bc1e86ac8 100644
--- a/test/integration/BUILD
+++ b/test/integration/BUILD
@@ -344,6 +344,8 @@ envoy_cc_test(
     srcs = [
         "echo_integration_test.cc",
     ],
+    # This test must be run in exclusive mode: see comments in AddRemoveListener
+    tags = ["exclusive"],
     deps = [
         ":integration_lib",
         "//source/extensions/filters/network/echo:config",
diff --git a/test/integration/echo_integration_test.cc b/test/integration/echo_integration_test.cc
index 342ba5866f0d..12445e7b51a0 100644
--- a/test/integration/echo_integration_test.cc
+++ b/test/integration/echo_integration_test.cc
@@ -118,10 +118,25 @@ TEST_P(EchoIntegrationTest, AddRemoveListener) {
   listener_removed.waitReady();
 
   // Now connect. This should fail.
-  RawConnectionDriver connection2(
-      new_listener_port, buffer,
-      [&](Network::ClientConnection&, const Buffer::Instance&) -> void { FAIL(); }, version_);
-  connection2.run();
+  // Allow for a few attempts, in order to handle a race (likely due to lack of
+  // LEV_OPT_CLOSE_ON_FREE, which would break listener reuse)
+  //
+  // In order for this test to work, it must be tagged as "exclusive" in its
+  // build file. Otherwise, it's possible that when the listener is destroyed
+  // above, another test would start listening on the released port, and this
+  // connect would unexpectedly succeed.
+  bool connect_fail = false;
+  for (int i = 0; i < 10; ++i) {
+    RawConnectionDriver connection2(
+        new_listener_port, buffer,
+        [&](Network::ClientConnection&, const Buffer::Instance&) -> void { FAIL(); }, version_);
+    connection2.run(Event::Dispatcher::RunType::NonBlock);
+    if (connection2.connection().state() == Network::Connection::State::Closed) {
+      connect_fail = true;
+      break;
+    }
+  }
+  ASSERT_TRUE(connect_fail);
 }
 
 } // namespace Envoy
diff --git a/test/integration/utility.cc b/test/integration/utility.cc
index f2440ff0893e..ff815315c653 100644
--- a/test/integration/utility.cc
+++ b/test/integration/utility.cc
@@ -122,7 +122,7 @@ RawConnectionDriver::RawConnectionDriver(uint32_t port, Buffer::Instance& initia
 
 RawConnectionDriver::~RawConnectionDriver() {}
 
-void RawConnectionDriver::run() { dispatcher_->run(Event::Dispatcher::RunType::Block); }
+void RawConnectionDriver::run(Event::Dispatcher::RunType run_type) { dispatcher_->run(run_type); }
 
 void RawConnectionDriver::close() { client_->close(Network::ConnectionCloseType::FlushWrite); }
 
diff --git a/test/integration/utility.h b/test/integration/utility.h
index a1e1d7c9e449..a1bd07f5ece9 100644
--- a/test/integration/utility.h
+++ b/test/integration/utility.h
@@ -61,7 +61,8 @@ class RawConnectionDriver {
   RawConnectionDriver(uint32_t port, Buffer::Instance& initial_data, ReadCallback data_callback,
                       Network::Address::IpVersion version);
   ~RawConnectionDriver();
-  void run();
+  const Network::Connection& connection() { return *client_; }
+  void run(Event::Dispatcher::RunType run_type = Event::Dispatcher::RunType::Block);
   void close();
 
 private: