Simplify fallback test client and parameterize fallback deadline (#29126)

Simplify fallback test client and parameterize fallback deadline

test/cpp/interop/BUILD

@@ -45,6 +45,7 @@ grpc_cc_binary(
     ],
     external_deps = [
         "absl/flags:flag",
+        "absl/time:time",
     ],
     language = "C++",
     tags = ["no_windows"],

test/cpp/interop/grpclb_fallback_test.cc

@@ -33,6 +33,7 @@
 #include <thread>
 
 #include "absl/flags/flag.h"
+#include "absl/time/time.h"
 
 #include <grpc/support/alloc.h>
 #include <grpc/support/log.h>
@@ -54,21 +55,14 @@
 ABSL_FLAG(std::string, custom_credentials_type, "",
           "User provided credentials type.");
 ABSL_FLAG(std::string, server_uri, "localhost:1000", "Server URI target");
-ABSL_FLAG(std::string, unroute_lb_and_backend_addrs_cmd, "exit 1",
+ABSL_FLAG(std::string, induce_fallback_cmd, "exit 1",
-          "Shell command used to make LB and backend addresses unroutable");
+          "Shell command to induce fallback, e.g. by unrouting addresses");
-ABSL_FLAG(std::string, blackhole_lb_and_backend_addrs_cmd, "exit 1",
+ABSL_FLAG(int, fallback_deadline_seconds, 1,
-          "Shell command used to make LB and backend addresses blackholed");
+          "Number of seconds to wait for fallback to occur after inducing it");
-ABSL_FLAG(
+ABSL_FLAG(std::string, test_case, "",
-    std::string, test_case, "",
+          "Test case to run. Valid options are:\n\n"
-    "Test case to run. Valid options are:\n\n"
+          "fallback_before_startup : fallback before making RPCs to backends"
-    "fast_fallback_before_startup : fallback before establishing connection to "
+          "fallback_after_startup : fallback after making RPCs to backends");
-    "LB;\n"
-    "fast_fallback_after_startup : fallback after startup due to LB/backend "
-    "addresses becoming unroutable;\n"
-    "slow_fallback_before_startup : fallback before startup due to LB address "
-    "being blackholed;\n"
-    "slow_fallback_after_startup : fallback after startup due to LB/backend "
-    "addresses becoming blackholed;\n");
 
 #ifdef LINUX_VERSION_CODE
 #if LINUX_VERSION_CODE >= KERNEL_VERSION(2, 6, 37)
@@ -187,94 +181,69 @@ void RunCommand(const std::string& command) {
   }
 }
 
-void RunFallbackBeforeStartupTest(
+void WaitForFallbackAndDoRPCs(TestService::Stub* stub) {
-    const std::string& break_lb_and_backend_conns_cmd,
+  int fallback_retry_count = 0;
-    int per_rpc_deadline_seconds) {
+  bool fallback = false;
-  std::unique_ptr<TestService::Stub> stub = CreateFallbackTestStub();
+  absl::Time fallback_deadline =
-  RunCommand(break_lb_and_backend_conns_cmd);
+      absl::Now() +
-  for (size_t i = 0; i < 30; i++) {
+      absl::Seconds(absl::GetFlag(FLAGS_fallback_deadline_seconds));
-    GrpclbRouteType grpclb_route_type =
+  while (absl::Now() < fallback_deadline) {
-        DoRPCAndGetPath(stub.get(), per_rpc_deadline_seconds);
+    GrpclbRouteType grpclb_route_type = DoRPCAndGetPath(stub, 1);
-    if (grpclb_route_type != GrpclbRouteType::GRPCLB_ROUTE_TYPE_FALLBACK) {
+    if (grpclb_route_type == GrpclbRouteType::GRPCLB_ROUTE_TYPE_BACKEND) {
-      gpr_log(GPR_ERROR, "Expected grpclb route type: FALLBACK. Got: %d",
+      gpr_log(GPR_ERROR,
-              grpclb_route_type);
+              "Got grpclb route type backend. Backends are "
-      abort();
+              "supposed to be unreachable, so this test is broken");
+      GPR_ASSERT(0);
     }
-    std::this_thread::sleep_for(std::chrono::seconds(1));
-  }
-}
-
-void DoFastFallbackBeforeStartup() {
-  RunFallbackBeforeStartupTest(
-      absl::GetFlag(FLAGS_unroute_lb_and_backend_addrs_cmd), 9);
-}
-
-void DoSlowFallbackBeforeStartup() {
-  RunFallbackBeforeStartupTest(
-      absl::GetFlag(FLAGS_blackhole_lb_and_backend_addrs_cmd), 20);
-}
-
-void RunFallbackAfterStartupTest(
-    const std::string& break_lb_and_backend_conns_cmd) {
-  std::unique_ptr<TestService::Stub> stub = CreateFallbackTestStub();
-  GrpclbRouteType grpclb_route_type = DoRPCAndGetPath(stub.get(), 20);
-  if (grpclb_route_type != GrpclbRouteType::GRPCLB_ROUTE_TYPE_BACKEND) {
-    gpr_log(GPR_ERROR, "Expected grpclb route type: BACKEND. Got: %d",
-            grpclb_route_type);
-    abort();
-  }
-  RunCommand(break_lb_and_backend_conns_cmd);
-  for (size_t i = 0; i < 40; i++) {
-    GrpclbRouteType grpclb_route_type =
-        DoWaitForReadyRPCAndGetPath(stub.get(), 1);
-    // Backends should be unreachable by now, otherwise the test is broken.
-    GPR_ASSERT(grpclb_route_type != GrpclbRouteType::GRPCLB_ROUTE_TYPE_BACKEND);
     if (grpclb_route_type == GrpclbRouteType::GRPCLB_ROUTE_TYPE_FALLBACK) {
       gpr_log(GPR_INFO,
-              "Made one successul RPC to a fallback. Now expect the same for "
+              "Made one successful RPC to a fallback. Now expect the same for "
               "the rest.");
+      fallback = true;
       break;
     } else {
-      gpr_log(GPR_ERROR, "Retryable RPC failure on iteration: %" PRIdPTR, i);
+      gpr_log(GPR_ERROR, "Retryable RPC failure on iteration: %d",
+              fallback_retry_count);
     }
+    fallback_retry_count++;
   }
-  for (size_t i = 0; i < 30; i++) {
+  if (!fallback) {
-    GrpclbRouteType grpclb_route_type = DoRPCAndGetPath(stub.get(), 20);
+    gpr_log(GPR_ERROR, "Didn't fall back within deadline");
-    if (grpclb_route_type != GrpclbRouteType::GRPCLB_ROUTE_TYPE_FALLBACK) {
+    GPR_ASSERT(0);
-      gpr_log(GPR_ERROR, "Expected grpclb route type: FALLBACK. Got: %d",
+  }
-              grpclb_route_type);
+  for (int i = 0; i < 30; i++) {
-      abort();
+    GrpclbRouteType grpclb_route_type = DoRPCAndGetPath(stub, 20);
-    }
+    GPR_ASSERT(grpclb_route_type ==
+               GrpclbRouteType::GRPCLB_ROUTE_TYPE_FALLBACK);
     std::this_thread::sleep_for(std::chrono::seconds(1));
   }
 }
 
-void DoFastFallbackAfterStartup() {
+void DoFallbackBeforeStartupTest() {
-  RunFallbackAfterStartupTest(
+  std::unique_ptr<TestService::Stub> stub = CreateFallbackTestStub();
-      absl::GetFlag(FLAGS_unroute_lb_and_backend_addrs_cmd));
+  RunCommand(absl::GetFlag(FLAGS_induce_fallback_cmd));
+  WaitForFallbackAndDoRPCs(stub.get());
 }
 
-void DoSlowFallbackAfterStartup() {
+void DoFallbackAfterStartupTest() {
-  RunFallbackAfterStartupTest(
+  std::unique_ptr<TestService::Stub> stub = CreateFallbackTestStub();
-      absl::GetFlag(FLAGS_blackhole_lb_and_backend_addrs_cmd));
+  GrpclbRouteType grpclb_route_type = DoRPCAndGetPath(stub.get(), 20);
+  GPR_ASSERT(grpclb_route_type == GrpclbRouteType::GRPCLB_ROUTE_TYPE_BACKEND);
+  RunCommand(absl::GetFlag(FLAGS_induce_fallback_cmd));
+  WaitForFallbackAndDoRPCs(stub.get());
 }
+
 }  // namespace
 
 int main(int argc, char** argv) {
   grpc::testing::InitTest(&argc, &argv, true);
   gpr_log(GPR_INFO, "Testing: %s", absl::GetFlag(FLAGS_test_case).c_str());
-  if (absl::GetFlag(FLAGS_test_case) == "fast_fallback_before_startup") {
+  if (absl::GetFlag(FLAGS_test_case) == "fallback_before_startup") {
-    DoFastFallbackBeforeStartup();
+    DoFallbackBeforeStartupTest();
-    gpr_log(GPR_INFO, "DoFastFallbackBeforeStartup done!");
+    gpr_log(GPR_INFO, "DoFallbackBeforeStartup done!");
-  } else if (absl::GetFlag(FLAGS_test_case) == "slow_fallback_before_startup") {
+  } else if (absl::GetFlag(FLAGS_test_case) == "fallback_after_startup") {
-    DoSlowFallbackBeforeStartup();
+    DoFallbackAfterStartupTest();
-    gpr_log(GPR_INFO, "DoSlowFallbackBeforeStartup done!");
+    gpr_log(GPR_INFO, "DoFallbackBeforeStartup done!");
-  } else if (absl::GetFlag(FLAGS_test_case) == "fast_fallback_after_startup") {
-    DoFastFallbackAfterStartup();
-    gpr_log(GPR_INFO, "DoFastFallbackAfterStartup done!");
-  } else if (absl::GetFlag(FLAGS_test_case) == "slow_fallback_after_startup") {
-    DoSlowFallbackAfterStartup();
-    gpr_log(GPR_INFO, "DoSlowFallbackAfterStartup done!");
   } else {
     gpr_log(GPR_ERROR, "Invalid test case: %s",
             absl::GetFlag(FLAGS_test_case).c_str());