Merge pull request #23629 from ericgribkoff/delay_fail_on_failed_rpc

Update semantics of --fail_on_failed_rpc

doc/xds-test-descriptions.md

@@ -27,7 +27,9 @@ Clients should accept these arguments:
 
 *   --fail_on_failed_rpcs=BOOL
     *   If true, the client should exit with a non-zero return code if any RPCs
-        fail. Default is false.
+        fail after at least one RPC has succeeded, indicating a valid xDS config
+        was received. This accounts for any startup-related delays in receiving
+        an initial config from the load balancer. Default is false.
 *   --num_channels=CHANNELS
     *   The number of channels to create to the server.
 *   --qps=QPS

test/cpp/interop/xds_interop_client.cc

@@ -16,6 +16,7 @@
  *
  */
 
+#include <atomic>
 #include <chrono>
 #include <condition_variable>
 #include <map>
@@ -41,7 +42,8 @@
 #include "test/core/util/test_config.h"
 #include "test/cpp/util/test_config.h"
 
-DEFINE_bool(fail_on_failed_rpc, false, "Fail client if any RPCs fail.");
+DEFINE_bool(fail_on_failed_rpc, false,
+            "Fail client if any RPCs fail after first successful RPC.");
 DEFINE_int32(num_channels, 1, "Number of channels.");
 DEFINE_bool(print_response, false, "Write RPC response to stdout.");
 DEFINE_int32(qps, 1, "Qps per channel.");
@@ -80,6 +82,8 @@ int global_request_id;
 std::set<XdsStatsWatcher*> watchers;
 // Mutex for global_request_id and watchers
 std::mutex mu;
+// Whether at least one RPC has succeeded, indicating xDS resolution completed.
+std::atomic<bool> one_rpc_succeeded(false);
 
 /** Records the remote peer distribution for a given range of RPCs. */
 class XdsStatsWatcher {
@@ -223,7 +227,7 @@ class TestClient {
           std::cout << "RPC failed: " << call->status.error_code() << ": "
                     << call->status.error_message() << std::endl;
         }
-        if (FLAGS_fail_on_failed_rpc) {
+        if (FLAGS_fail_on_failed_rpc && one_rpc_succeeded.load()) {
           abort();
         }
       } else {
@@ -239,6 +243,7 @@ class TestClient {
           std::cout << "Greeting: Hello world, this is " << hostname
                     << ", from " << call->context.peer() << std::endl;
         }
+        one_rpc_succeeded = true;
       }
 
       delete call;

tools/run_tests/run_xds_tests.py

@@ -1524,15 +1524,6 @@ def wait_for_healthy_backends(gcp,
                     (timeout_sec, result))
 
 
-def wait_for_config_propagation(gcp, instance_group, client_cmd, client_env):
-    """Use client to verify config propagation from GCP->TD->client"""
-    instance_names = get_instance_names(gcp, instance_group)
-    client_process = subprocess.Popen(shlex.split(client_cmd), env=client_env)
-    wait_until_all_rpcs_go_to_given_backends(instance_names,
-                                             _WAIT_FOR_VALID_CONFIG_SEC)
-    client_process.terminate()
-
-
 def get_instance_names(gcp, instance_group):
     instance_names = []
     result = gcp.compute.instanceGroups().listInstances(
@@ -1727,15 +1718,6 @@ try:
                 metadata_to_send = '--metadata=""'
 
             if test_case in _TESTS_TO_FAIL_ON_RPC_FAILURE:
-                wait_for_config_propagation(
-                    gcp, instance_group,
-                    args.client_cmd.format(server_uri=server_uri,
-                                           stats_port=args.stats_port,
-                                           qps=args.qps,
-                                           fail_on_failed_rpc=False,
-                                           rpcs_to_send=rpcs_to_send,
-                                           metadata_to_send=metadata_to_send),
-                    client_env)
                 fail_on_failed_rpc = '--fail_on_failed_rpc=true'
             else:
                 fail_on_failed_rpc = '--fail_on_failed_rpc=false'