From 6cba63eb4794bc0a603ad0bd4fbe1a1180fe3541 Mon Sep 17 00:00:00 2001 From: Yash Tibrewal Date: Wed, 8 May 2019 03:36:37 -0700 Subject: [PATCH] reviewer comments and tests --- .../filters/client_channel/client_channel.cc | 26 +++--- .../client_channel/resolving_lb_policy.cc | 27 +++--- test/core/util/test_lb_policies.cc | 1 + test/cpp/end2end/BUILD | 1 - .../end2end/service_config_end2end_test.cc | 1 - test/cpp/naming/gen_build_yaml.py | 1 + test/cpp/naming/resolver_component_test.cc | 15 +++- .../naming/resolver_component_tests_runner.py | 82 +++++++++++++++++++ .../naming/resolver_test_record_groups.yaml | 78 ++++++++++++++++++ 9 files changed, 206 insertions(+), 26 deletions(-) diff --git a/src/core/ext/filters/client_channel/client_channel.cc b/src/core/ext/filters/client_channel/client_channel.cc index 93070bdc64e..5d5436b8f67 100644 --- a/src/core/ext/filters/client_channel/client_channel.cc +++ b/src/core/ext/filters/client_channel/client_channel.cc @@ -1242,20 +1242,20 @@ bool ChannelData::ProcessResolverResultLocked( bool service_config_changed = service_config != chand->saved_service_config_; if (service_config_changed) { chand->saved_service_config_ = std::move(service_config); - Optional - retry_throttle_data; - if (parsed_object != nullptr) { - chand->health_check_service_name_.reset( - gpr_strdup(parsed_object->health_check_service_name())); - retry_throttle_data = parsed_object->retry_throttling(); - } else { - chand->health_check_service_name_.reset(); - } - // Create service config setter to update channel state in the data - // plane combiner. Destroys itself when done. - New(chand, retry_throttle_data, - chand->saved_service_config_); } + Optional + retry_throttle_data; + if (parsed_object != nullptr) { + chand->health_check_service_name_.reset( + gpr_strdup(parsed_object->health_check_service_name())); + retry_throttle_data = parsed_object->retry_throttling(); + } else { + chand->health_check_service_name_.reset(); + } + // Create service config setter to update channel state in the data + // plane combiner. Destroys itself when done. + New(chand, retry_throttle_data, + chand->saved_service_config_); UniquePtr processed_lb_policy_name; chand->ProcessLbPolicy(result, parsed_object, &processed_lb_policy_name, lb_policy_config); diff --git a/src/core/ext/filters/client_channel/resolving_lb_policy.cc b/src/core/ext/filters/client_channel/resolving_lb_policy.cc index 698433c3854..4824f4e22c3 100644 --- a/src/core/ext/filters/client_channel/resolving_lb_policy.cc +++ b/src/core/ext/filters/client_channel/resolving_lb_policy.cc @@ -532,31 +532,33 @@ void ResolvingLoadBalancingPolicy::OnResolverResultChangedLocked( const char* lb_policy_name = nullptr; RefCountedPtr lb_policy_config; bool service_config_changed = false; + char* service_config_error_string = nullptr; if (process_resolver_result_ != nullptr) { grpc_error* service_config_error = GRPC_ERROR_NONE; service_config_changed = process_resolver_result_( process_resolver_result_user_data_, result, &lb_policy_name, &lb_policy_config, &service_config_error); if (service_config_error != GRPC_ERROR_NONE) { - if (channelz_node() != nullptr) { - trace_strings.push_back( - gpr_strdup(grpc_error_string(service_config_error))); - } + service_config_error_string = + gpr_strdup(grpc_error_string(service_config_error)); if (lb_policy_name == nullptr) { // Use an empty lb_policy_name as an indicator that we received an // invalid service config and we don't have a fallback service config. - return OnResolverError(service_config_error); + OnResolverError(service_config_error); + } else { + GRPC_ERROR_UNREF(service_config_error); } - GRPC_ERROR_UNREF(service_config_error); } } else { lb_policy_name = child_policy_name_.get(); lb_policy_config = child_lb_config_; } - GPR_ASSERT(lb_policy_name != nullptr); - // Create or update LB policy, as needed. - CreateOrUpdateLbPolicyLocked(lb_policy_name, lb_policy_config, - std::move(result), &trace_strings); + if (lb_policy_name != nullptr) { + gpr_log(GPR_ERROR, "%s", lb_policy_name); + // Create or update LB policy, as needed. + CreateOrUpdateLbPolicyLocked(lb_policy_name, lb_policy_config, + std::move(result), &trace_strings); + } // Add channel trace event. if (channelz_node() != nullptr) { if (service_config_changed) { @@ -564,10 +566,15 @@ void ResolvingLoadBalancingPolicy::OnResolverResultChangedLocked( // config in the trace, at the risk of bloating the trace logs. trace_strings.push_back(gpr_strdup("Service config changed")); } + if (service_config_error_string != nullptr) { + trace_strings.push_back(service_config_error_string); + service_config_error_string = nullptr; + } MaybeAddTraceMessagesForAddressChangesLocked(resolution_contains_addresses, &trace_strings); ConcatenateAndAddChannelTraceLocked(&trace_strings); } + gpr_free(service_config_error_string); } } // namespace grpc_core diff --git a/test/core/util/test_lb_policies.cc b/test/core/util/test_lb_policies.cc index b871f04bc9e..8f5458e782a 100644 --- a/test/core/util/test_lb_policies.cc +++ b/test/core/util/test_lb_policies.cc @@ -209,6 +209,7 @@ class InterceptTrailingFactory : public LoadBalancingPolicyFactory { OrphanablePtr CreateLoadBalancingPolicy( LoadBalancingPolicy::Args args) const override { + gpr_log(GPR_ERROR, "creating"); return OrphanablePtr( New(std::move(args), cb_, user_data_)); diff --git a/test/cpp/end2end/BUILD b/test/cpp/end2end/BUILD index d211a9e8441..519a33fd8a4 100644 --- a/test/cpp/end2end/BUILD +++ b/test/cpp/end2end/BUILD @@ -439,7 +439,6 @@ grpc_cc_test( "//src/proto/grpc/testing:echo_proto", "//src/proto/grpc/testing/duplicate:echo_duplicate_proto", "//test/core/util:grpc_test_util", - "//test/core/util:test_lb_policies", "//test/cpp/util:test_util", ], ) diff --git a/test/cpp/end2end/service_config_end2end_test.cc b/test/cpp/end2end/service_config_end2end_test.cc index b695a9f8c97..9b259bd6342 100644 --- a/test/cpp/end2end/service_config_end2end_test.cc +++ b/test/cpp/end2end/service_config_end2end_test.cc @@ -54,7 +54,6 @@ #include "src/proto/grpc/testing/echo.grpc.pb.h" #include "test/core/util/port.h" #include "test/core/util/test_config.h" -#include "test/core/util/test_lb_policies.h" #include "test/cpp/end2end/test_service_impl.h" #include diff --git a/test/cpp/naming/gen_build_yaml.py b/test/cpp/naming/gen_build_yaml.py index 9bf5ae9b2f8..ef757aeeb52 100755 --- a/test/cpp/naming/gen_build_yaml.py +++ b/test/cpp/naming/gen_build_yaml.py @@ -47,6 +47,7 @@ def _resolver_test_cases(resolver_component_data): _build_expected_addrs_cmd_arg(test_case['expected_addrs'])), ('expected_chosen_service_config', (test_case['expected_chosen_service_config'] or '')), + ('expected_service_config_error', (test_case['expected_service_config_error'] or '')), ('expected_lb_policy', (test_case['expected_lb_policy'] or '')), ('enable_srv_queries', test_case['enable_srv_queries']), ('enable_txt_queries', test_case['enable_txt_queries']), diff --git a/test/cpp/naming/resolver_component_test.cc b/test/cpp/naming/resolver_component_test.cc index 4d1beb7ec37..7903f2a932f 100644 --- a/test/cpp/naming/resolver_component_test.cc +++ b/test/cpp/naming/resolver_component_test.cc @@ -89,6 +89,8 @@ DEFINE_string(expected_addrs, "", DEFINE_string(expected_chosen_service_config, "", "Expected service config json string that gets chosen (no " "whitespace). Empty for none."); +DEFINE_string(expected_service_config_error, "", + "Expected service config error. Empty for none."); DEFINE_string( local_dns_server_address, "", "Optional. This address is placed as the uri authority if present."); @@ -179,6 +181,7 @@ struct ArgsStruct { grpc_channel_args* channel_args; vector expected_addrs; std::string expected_service_config_string; + std::string expected_service_config_error; std::string expected_lb_policy; }; @@ -241,6 +244,7 @@ void PollPollsetUntilRequestDone(ArgsStruct* args) { } void CheckServiceConfigResultLocked(const char* service_config_json, + grpc_error* service_config_error, ArgsStruct* args) { if (args->expected_service_config_string != "") { GPR_ASSERT(service_config_json != nullptr); @@ -248,6 +252,13 @@ void CheckServiceConfigResultLocked(const char* service_config_json, } else { GPR_ASSERT(service_config_json == nullptr); } + if (args->expected_service_config_error == "") { + EXPECT_EQ(service_config_error, GRPC_ERROR_NONE); + } else { + EXPECT_THAT(grpc_error_string(service_config_error), + testing::HasSubstr(args->expected_service_config_error)); + } + GRPC_ERROR_UNREF(service_config_error); } void CheckLBPolicyResultLocked(const grpc_channel_args* channel_args, @@ -462,7 +473,8 @@ class CheckingResultHandler : public ResultHandler { result.service_config == nullptr ? nullptr : result.service_config->service_config_json(); - CheckServiceConfigResultLocked(service_config_json, args); + CheckServiceConfigResultLocked( + service_config_json, GRPC_ERROR_REF(result.service_config_error), args); if (args->expected_service_config_string == "") { CheckLBPolicyResultLocked(result.args, args); } @@ -477,6 +489,7 @@ void RunResolvesRelevantRecordsTest( ArgsInit(&args); args.expected_addrs = ParseExpectedAddrs(FLAGS_expected_addrs); args.expected_service_config_string = FLAGS_expected_chosen_service_config; + args.expected_service_config_error = FLAGS_expected_service_config_error; args.expected_lb_policy = FLAGS_expected_lb_policy; // maybe build the address with an authority char* whole_uri = nullptr; diff --git a/test/cpp/naming/resolver_component_tests_runner.py b/test/cpp/naming/resolver_component_tests_runner.py index 2d875ce00ae..356574b6086 100755 --- a/test/cpp/naming/resolver_component_tests_runner.py +++ b/test/cpp/naming/resolver_component_tests_runner.py @@ -124,6 +124,7 @@ current_test_subprocess = subprocess.Popen([ '--target_name', 'no-srv-ipv4-single-target.resolver-tests-version-4.grpctestingexp.', '--expected_addrs', '5.5.5.5:443,False', '--expected_chosen_service_config', '', + '--expected_service_config_error', '', '--expected_lb_policy', '', '--enable_srv_queries', 'True', '--enable_txt_queries', 'True', @@ -138,6 +139,7 @@ current_test_subprocess = subprocess.Popen([ '--target_name', 'srv-ipv4-single-target.resolver-tests-version-4.grpctestingexp.', '--expected_addrs', '1.2.3.4:1234,True', '--expected_chosen_service_config', '', + '--expected_service_config_error', '', '--expected_lb_policy', '', '--enable_srv_queries', 'True', '--enable_txt_queries', 'True', @@ -152,6 +154,7 @@ current_test_subprocess = subprocess.Popen([ '--target_name', 'srv-ipv4-multi-target.resolver-tests-version-4.grpctestingexp.', '--expected_addrs', '1.2.3.5:1234,True;1.2.3.6:1234,True;1.2.3.7:1234,True', '--expected_chosen_service_config', '', + '--expected_service_config_error', '', '--expected_lb_policy', '', '--enable_srv_queries', 'True', '--enable_txt_queries', 'True', @@ -166,6 +169,7 @@ current_test_subprocess = subprocess.Popen([ '--target_name', 'srv-ipv6-single-target.resolver-tests-version-4.grpctestingexp.', '--expected_addrs', '[2607:f8b0:400a:801::1001]:1234,True', '--expected_chosen_service_config', '', + '--expected_service_config_error', '', '--expected_lb_policy', '', '--enable_srv_queries', 'True', '--enable_txt_queries', 'True', @@ -180,6 +184,7 @@ current_test_subprocess = subprocess.Popen([ '--target_name', 'srv-ipv6-multi-target.resolver-tests-version-4.grpctestingexp.', '--expected_addrs', '[2607:f8b0:400a:801::1002]:1234,True;[2607:f8b0:400a:801::1003]:1234,True;[2607:f8b0:400a:801::1004]:1234,True', '--expected_chosen_service_config', '', + '--expected_service_config_error', '', '--expected_lb_policy', '', '--enable_srv_queries', 'True', '--enable_txt_queries', 'True', @@ -194,6 +199,7 @@ current_test_subprocess = subprocess.Popen([ '--target_name', 'srv-ipv4-simple-service-config.resolver-tests-version-4.grpctestingexp.', '--expected_addrs', '1.2.3.4:1234,True', '--expected_chosen_service_config', '{"loadBalancingPolicy":"round_robin","methodConfig":[{"name":[{"method":"Foo","service":"SimpleService"}],"waitForReady":true}]}', + '--expected_service_config_error', '', '--expected_lb_policy', 'round_robin', '--enable_srv_queries', 'True', '--enable_txt_queries', 'True', @@ -208,6 +214,7 @@ current_test_subprocess = subprocess.Popen([ '--target_name', 'ipv4-no-srv-simple-service-config.resolver-tests-version-4.grpctestingexp.', '--expected_addrs', '1.2.3.4:443,False', '--expected_chosen_service_config', '{"loadBalancingPolicy":"round_robin","methodConfig":[{"name":[{"method":"Foo","service":"NoSrvSimpleService"}],"waitForReady":true}]}', + '--expected_service_config_error', '', '--expected_lb_policy', 'round_robin', '--enable_srv_queries', 'True', '--enable_txt_queries', 'True', @@ -222,6 +229,7 @@ current_test_subprocess = subprocess.Popen([ '--target_name', 'ipv4-no-config-for-cpp.resolver-tests-version-4.grpctestingexp.', '--expected_addrs', '1.2.3.4:443,False', '--expected_chosen_service_config', '', + '--expected_service_config_error', '', '--expected_lb_policy', '', '--enable_srv_queries', 'True', '--enable_txt_queries', 'True', @@ -236,6 +244,7 @@ current_test_subprocess = subprocess.Popen([ '--target_name', 'ipv4-cpp-config-has-zero-percentage.resolver-tests-version-4.grpctestingexp.', '--expected_addrs', '1.2.3.4:443,False', '--expected_chosen_service_config', '', + '--expected_service_config_error', '', '--expected_lb_policy', '', '--enable_srv_queries', 'True', '--enable_txt_queries', 'True', @@ -250,6 +259,7 @@ current_test_subprocess = subprocess.Popen([ '--target_name', 'ipv4-second-language-is-cpp.resolver-tests-version-4.grpctestingexp.', '--expected_addrs', '1.2.3.4:443,False', '--expected_chosen_service_config', '{"loadBalancingPolicy":"round_robin","methodConfig":[{"name":[{"method":"Foo","service":"CppService"}],"waitForReady":true}]}', + '--expected_service_config_error', '', '--expected_lb_policy', 'round_robin', '--enable_srv_queries', 'True', '--enable_txt_queries', 'True', @@ -264,6 +274,7 @@ current_test_subprocess = subprocess.Popen([ '--target_name', 'ipv4-config-with-percentages.resolver-tests-version-4.grpctestingexp.', '--expected_addrs', '1.2.3.4:443,False', '--expected_chosen_service_config', '{"loadBalancingPolicy":"round_robin","methodConfig":[{"name":[{"method":"Foo","service":"AlwaysPickedService"}],"waitForReady":true}]}', + '--expected_service_config_error', '', '--expected_lb_policy', 'round_robin', '--enable_srv_queries', 'True', '--enable_txt_queries', 'True', @@ -278,6 +289,7 @@ current_test_subprocess = subprocess.Popen([ '--target_name', 'srv-ipv4-target-has-backend-and-balancer.resolver-tests-version-4.grpctestingexp.', '--expected_addrs', '1.2.3.4:1234,True;1.2.3.4:443,False', '--expected_chosen_service_config', '', + '--expected_service_config_error', '', '--expected_lb_policy', '', '--enable_srv_queries', 'True', '--enable_txt_queries', 'True', @@ -292,6 +304,7 @@ current_test_subprocess = subprocess.Popen([ '--target_name', 'srv-ipv6-target-has-backend-and-balancer.resolver-tests-version-4.grpctestingexp.', '--expected_addrs', '[2607:f8b0:400a:801::1002]:1234,True;[2607:f8b0:400a:801::1002]:443,False', '--expected_chosen_service_config', '', + '--expected_service_config_error', '', '--expected_lb_policy', '', '--enable_srv_queries', 'True', '--enable_txt_queries', 'True', @@ -306,6 +319,7 @@ current_test_subprocess = subprocess.Popen([ '--target_name', 'ipv4-config-causing-fallback-to-tcp.resolver-tests-version-4.grpctestingexp.', '--expected_addrs', '1.2.3.4:443,False', '--expected_chosen_service_config', '{"loadBalancingPolicy":"round_robin","methodConfig":[{"name":[{"method":"Foo","service":"SimpleService"}],"waitForReady":true},{"name":[{"method":"FooTwo","service":"SimpleService"}],"waitForReady":true},{"name":[{"method":"FooThree","service":"SimpleService"}],"waitForReady":true},{"name":[{"method":"FooFour","service":"SimpleService"}],"waitForReady":true},{"name":[{"method":"FooFive","service":"SimpleService"}],"waitForReady":true},{"name":[{"method":"FooSix","service":"SimpleService"}],"waitForReady":true},{"name":[{"method":"FooSeven","service":"SimpleService"}],"waitForReady":true},{"name":[{"method":"FooEight","service":"SimpleService"}],"waitForReady":true},{"name":[{"method":"FooNine","service":"SimpleService"}],"waitForReady":true},{"name":[{"method":"FooTen","service":"SimpleService"}],"waitForReady":true},{"name":[{"method":"FooEleven","service":"SimpleService"}],"waitForReady":true},{"name":[{"method":"FooTwelve","service":"SimpleService"}],"waitForReady":true},{"name":[{"method":"FooTwelve","service":"SimpleService"}],"waitForReady":true},{"name":[{"method":"FooTwelve","service":"SimpleService"}],"waitForReady":true},{"name":[{"method":"FooTwelve","service":"SimpleService"}],"waitForReady":true}]}', + '--expected_service_config_error', '', '--expected_lb_policy', '', '--enable_srv_queries', 'True', '--enable_txt_queries', 'True', @@ -320,6 +334,7 @@ current_test_subprocess = subprocess.Popen([ '--target_name', 'srv-ipv4-single-target-srv-disabled.resolver-tests-version-4.grpctestingexp.', '--expected_addrs', '2.3.4.5:443,False', '--expected_chosen_service_config', '', + '--expected_service_config_error', '', '--expected_lb_policy', '', '--enable_srv_queries', 'False', '--enable_txt_queries', 'True', @@ -334,6 +349,7 @@ current_test_subprocess = subprocess.Popen([ '--target_name', 'srv-ipv4-multi-target-srv-disabled.resolver-tests-version-4.grpctestingexp.', '--expected_addrs', '9.2.3.5:443,False;9.2.3.6:443,False;9.2.3.7:443,False', '--expected_chosen_service_config', '', + '--expected_service_config_error', '', '--expected_lb_policy', '', '--enable_srv_queries', 'False', '--enable_txt_queries', 'True', @@ -348,6 +364,7 @@ current_test_subprocess = subprocess.Popen([ '--target_name', 'srv-ipv6-single-target-srv-disabled.resolver-tests-version-4.grpctestingexp.', '--expected_addrs', '[2600::1001]:443,False', '--expected_chosen_service_config', '', + '--expected_service_config_error', '', '--expected_lb_policy', '', '--enable_srv_queries', 'False', '--enable_txt_queries', 'True', @@ -362,6 +379,7 @@ current_test_subprocess = subprocess.Popen([ '--target_name', 'srv-ipv6-multi-target-srv-disabled.resolver-tests-version-4.grpctestingexp.', '--expected_addrs', '[2600::1002]:443,False;[2600::1003]:443,False;[2600::1004]:443,False', '--expected_chosen_service_config', '', + '--expected_service_config_error', '', '--expected_lb_policy', '', '--enable_srv_queries', 'False', '--enable_txt_queries', 'True', @@ -376,6 +394,7 @@ current_test_subprocess = subprocess.Popen([ '--target_name', 'srv-ipv4-simple-service-config-srv-disabled.resolver-tests-version-4.grpctestingexp.', '--expected_addrs', '5.5.3.4:443,False', '--expected_chosen_service_config', '{"loadBalancingPolicy":"round_robin","methodConfig":[{"name":[{"method":"Foo","service":"SimpleService"}],"waitForReady":true}]}', + '--expected_service_config_error', '', '--expected_lb_policy', 'round_robin', '--enable_srv_queries', 'False', '--enable_txt_queries', 'True', @@ -390,6 +409,7 @@ current_test_subprocess = subprocess.Popen([ '--target_name', 'srv-ipv4-simple-service-config-txt-disabled.resolver-tests-version-4.grpctestingexp.', '--expected_addrs', '1.2.3.4:1234,True', '--expected_chosen_service_config', '', + '--expected_service_config_error', '', '--expected_lb_policy', '', '--enable_srv_queries', 'True', '--enable_txt_queries', 'False', @@ -404,6 +424,7 @@ current_test_subprocess = subprocess.Popen([ '--target_name', 'ipv4-cpp-config-has-zero-percentage-txt-disabled.resolver-tests-version-4.grpctestingexp.', '--expected_addrs', '1.2.3.4:443,False', '--expected_chosen_service_config', '', + '--expected_service_config_error', '', '--expected_lb_policy', '', '--enable_srv_queries', 'True', '--enable_txt_queries', 'False', @@ -418,6 +439,7 @@ current_test_subprocess = subprocess.Popen([ '--target_name', 'ipv4-second-language-is-cpp-txt-disabled.resolver-tests-version-4.grpctestingexp.', '--expected_addrs', '1.2.3.4:443,False', '--expected_chosen_service_config', '', + '--expected_service_config_error', '', '--expected_lb_policy', '', '--enable_srv_queries', 'True', '--enable_txt_queries', 'False', @@ -426,6 +448,66 @@ current_test_subprocess.communicate() if current_test_subprocess.returncode != 0: num_test_failures += 1 +test_runner_log('Run test with target: %s' % 'ipv4-svc_cfg_bad_json.resolver-tests-version-4.grpctestingexp.') +current_test_subprocess = subprocess.Popen([ + args.test_bin_path, + '--target_name', 'ipv4-svc_cfg_bad_json.resolver-tests-version-4.grpctestingexp.', + '--expected_addrs', '1.2.3.4:443,False', + '--expected_chosen_service_config', '', + '--expected_service_config_error', 'could not parse', + '--expected_lb_policy', '', + '--enable_srv_queries', 'True', + '--enable_txt_queries', 'True', + '--local_dns_server_address', '127.0.0.1:%d' % args.dns_server_port]) +current_test_subprocess.communicate() +if current_test_subprocess.returncode != 0: + num_test_failures += 1 + +test_runner_log('Run test with target: %s' % 'ipv4-svc_cfg_bad_client_language.resolver-tests-version-4.grpctestingexp.') +current_test_subprocess = subprocess.Popen([ + args.test_bin_path, + '--target_name', 'ipv4-svc_cfg_bad_client_language.resolver-tests-version-4.grpctestingexp.', + '--expected_addrs', '1.2.3.4:443,False', + '--expected_chosen_service_config', '', + '--expected_service_config_error', 'field:clientLanguage error:should be of type array', + '--expected_lb_policy', '', + '--enable_srv_queries', 'True', + '--enable_txt_queries', 'True', + '--local_dns_server_address', '127.0.0.1:%d' % args.dns_server_port]) +current_test_subprocess.communicate() +if current_test_subprocess.returncode != 0: + num_test_failures += 1 + +test_runner_log('Run test with target: %s' % 'ipv4-svc_cfg_bad_percentage.resolver-tests-version-4.grpctestingexp.') +current_test_subprocess = subprocess.Popen([ + args.test_bin_path, + '--target_name', 'ipv4-svc_cfg_bad_percentage.resolver-tests-version-4.grpctestingexp.', + '--expected_addrs', '1.2.3.4:443,False', + '--expected_chosen_service_config', '', + '--expected_service_config_error', 'field:percentage error:should be of type number', + '--expected_lb_policy', '', + '--enable_srv_queries', 'True', + '--enable_txt_queries', 'True', + '--local_dns_server_address', '127.0.0.1:%d' % args.dns_server_port]) +current_test_subprocess.communicate() +if current_test_subprocess.returncode != 0: + num_test_failures += 1 + +test_runner_log('Run test with target: %s' % 'ipv4-svc_cfg_bad_wait_for_ready.resolver-tests-version-4.grpctestingexp.') +current_test_subprocess = subprocess.Popen([ + args.test_bin_path, + '--target_name', 'ipv4-svc_cfg_bad_wait_for_ready.resolver-tests-version-4.grpctestingexp.', + '--expected_addrs', '1.2.3.4:443,False', + '--expected_chosen_service_config', '', + '--expected_service_config_error', 'field:waitForReady error:Type should be true/false', + '--expected_lb_policy', '', + '--enable_srv_queries', 'True', + '--enable_txt_queries', 'True', + '--local_dns_server_address', '127.0.0.1:%d' % args.dns_server_port]) +current_test_subprocess.communicate() +if current_test_subprocess.returncode != 0: + num_test_failures += 1 + test_runner_log('now kill DNS server') dns_server_subprocess.kill() dns_server_subprocess.wait() diff --git a/test/cpp/naming/resolver_test_record_groups.yaml b/test/cpp/naming/resolver_test_record_groups.yaml index bfb8b4ede2d..23d09ea896a 100644 --- a/test/cpp/naming/resolver_test_record_groups.yaml +++ b/test/cpp/naming/resolver_test_record_groups.yaml @@ -4,6 +4,7 @@ resolver_component_tests: - expected_addrs: - {address: '5.5.5.5:443', is_balancer: false} expected_chosen_service_config: null + expected_service_config_error: null expected_lb_policy: null enable_srv_queries: true enable_txt_queries: true @@ -14,6 +15,7 @@ resolver_component_tests: - expected_addrs: - {address: '1.2.3.4:1234', is_balancer: true} expected_chosen_service_config: null + expected_service_config_error: null expected_lb_policy: null enable_srv_queries: true enable_txt_queries: true @@ -28,6 +30,7 @@ resolver_component_tests: - {address: '1.2.3.6:1234', is_balancer: true} - {address: '1.2.3.7:1234', is_balancer: true} expected_chosen_service_config: null + expected_service_config_error: null expected_lb_policy: null enable_srv_queries: true enable_txt_queries: true @@ -42,6 +45,7 @@ resolver_component_tests: - expected_addrs: - {address: '[2607:f8b0:400a:801::1001]:1234', is_balancer: true} expected_chosen_service_config: null + expected_service_config_error: null expected_lb_policy: null enable_srv_queries: true enable_txt_queries: true @@ -56,6 +60,7 @@ resolver_component_tests: - {address: '[2607:f8b0:400a:801::1003]:1234', is_balancer: true} - {address: '[2607:f8b0:400a:801::1004]:1234', is_balancer: true} expected_chosen_service_config: null + expected_service_config_error: null expected_lb_policy: null enable_srv_queries: true enable_txt_queries: true @@ -70,6 +75,7 @@ resolver_component_tests: - expected_addrs: - {address: '1.2.3.4:1234', is_balancer: true} expected_chosen_service_config: '{"loadBalancingPolicy":"round_robin","methodConfig":[{"name":[{"method":"Foo","service":"SimpleService"}],"waitForReady":true}]}' + expected_service_config_error: null expected_lb_policy: round_robin enable_srv_queries: true enable_txt_queries: true @@ -85,6 +91,7 @@ resolver_component_tests: - expected_addrs: - {address: '1.2.3.4:443', is_balancer: false} expected_chosen_service_config: '{"loadBalancingPolicy":"round_robin","methodConfig":[{"name":[{"method":"Foo","service":"NoSrvSimpleService"}],"waitForReady":true}]}' + expected_service_config_error: null expected_lb_policy: round_robin enable_srv_queries: true enable_txt_queries: true @@ -98,6 +105,7 @@ resolver_component_tests: - expected_addrs: - {address: '1.2.3.4:443', is_balancer: false} expected_chosen_service_config: null + expected_service_config_error: null expected_lb_policy: null enable_srv_queries: true enable_txt_queries: true @@ -111,6 +119,7 @@ resolver_component_tests: - expected_addrs: - {address: '1.2.3.4:443', is_balancer: false} expected_chosen_service_config: null + expected_service_config_error: null expected_lb_policy: null enable_srv_queries: true enable_txt_queries: true @@ -124,6 +133,7 @@ resolver_component_tests: - expected_addrs: - {address: '1.2.3.4:443', is_balancer: false} expected_chosen_service_config: '{"loadBalancingPolicy":"round_robin","methodConfig":[{"name":[{"method":"Foo","service":"CppService"}],"waitForReady":true}]}' + expected_service_config_error: null expected_lb_policy: round_robin enable_srv_queries: true enable_txt_queries: true @@ -137,6 +147,7 @@ resolver_component_tests: - expected_addrs: - {address: '1.2.3.4:443', is_balancer: false} expected_chosen_service_config: '{"loadBalancingPolicy":"round_robin","methodConfig":[{"name":[{"method":"Foo","service":"AlwaysPickedService"}],"waitForReady":true}]}' + expected_service_config_error: null expected_lb_policy: round_robin enable_srv_queries: true enable_txt_queries: true @@ -151,6 +162,7 @@ resolver_component_tests: - {address: '1.2.3.4:1234', is_balancer: true} - {address: '1.2.3.4:443', is_balancer: false} expected_chosen_service_config: null + expected_service_config_error: null expected_lb_policy: null enable_srv_queries: true enable_txt_queries: true @@ -166,6 +178,7 @@ resolver_component_tests: - {address: '[2607:f8b0:400a:801::1002]:1234', is_balancer: true} - {address: '[2607:f8b0:400a:801::1002]:443', is_balancer: false} expected_chosen_service_config: null + expected_service_config_error: null expected_lb_policy: null enable_srv_queries: true enable_txt_queries: true @@ -180,6 +193,7 @@ resolver_component_tests: - expected_addrs: - {address: '1.2.3.4:443', is_balancer: false} expected_chosen_service_config: '{"loadBalancingPolicy":"round_robin","methodConfig":[{"name":[{"method":"Foo","service":"SimpleService"}],"waitForReady":true},{"name":[{"method":"FooTwo","service":"SimpleService"}],"waitForReady":true},{"name":[{"method":"FooThree","service":"SimpleService"}],"waitForReady":true},{"name":[{"method":"FooFour","service":"SimpleService"}],"waitForReady":true},{"name":[{"method":"FooFive","service":"SimpleService"}],"waitForReady":true},{"name":[{"method":"FooSix","service":"SimpleService"}],"waitForReady":true},{"name":[{"method":"FooSeven","service":"SimpleService"}],"waitForReady":true},{"name":[{"method":"FooEight","service":"SimpleService"}],"waitForReady":true},{"name":[{"method":"FooNine","service":"SimpleService"}],"waitForReady":true},{"name":[{"method":"FooTen","service":"SimpleService"}],"waitForReady":true},{"name":[{"method":"FooEleven","service":"SimpleService"}],"waitForReady":true},{"name":[{"method":"FooTwelve","service":"SimpleService"}],"waitForReady":true},{"name":[{"method":"FooTwelve","service":"SimpleService"}],"waitForReady":true},{"name":[{"method":"FooTwelve","service":"SimpleService"}],"waitForReady":true},{"name":[{"method":"FooTwelve","service":"SimpleService"}],"waitForReady":true}]}' + expected_service_config_error: null expected_lb_policy: null enable_srv_queries: true enable_txt_queries: true @@ -194,6 +208,7 @@ resolver_component_tests: - expected_addrs: - {address: '2.3.4.5:443', is_balancer: false} expected_chosen_service_config: null + expected_service_config_error: null expected_lb_policy: null enable_srv_queries: false enable_txt_queries: true @@ -210,6 +225,7 @@ resolver_component_tests: - {address: '9.2.3.6:443', is_balancer: false} - {address: '9.2.3.7:443', is_balancer: false} expected_chosen_service_config: null + expected_service_config_error: null expected_lb_policy: null enable_srv_queries: false enable_txt_queries: true @@ -228,6 +244,7 @@ resolver_component_tests: - expected_addrs: - {address: '[2600::1001]:443', is_balancer: false} expected_chosen_service_config: null + expected_service_config_error: null expected_lb_policy: null enable_srv_queries: false enable_txt_queries: true @@ -244,6 +261,7 @@ resolver_component_tests: - {address: '[2600::1003]:443', is_balancer: false} - {address: '[2600::1004]:443', is_balancer: false} expected_chosen_service_config: null + expected_service_config_error: null expected_lb_policy: null enable_srv_queries: false enable_txt_queries: true @@ -262,6 +280,7 @@ resolver_component_tests: - expected_addrs: - {address: '5.5.3.4:443', is_balancer: false} expected_chosen_service_config: '{"loadBalancingPolicy":"round_robin","methodConfig":[{"name":[{"method":"Foo","service":"SimpleService"}],"waitForReady":true}]}' + expected_service_config_error: null expected_lb_policy: round_robin enable_srv_queries: false enable_txt_queries: true @@ -279,6 +298,7 @@ resolver_component_tests: - expected_addrs: - {address: '1.2.3.4:1234', is_balancer: true} expected_chosen_service_config: null + expected_service_config_error: null expected_lb_policy: null enable_srv_queries: true enable_txt_queries: false @@ -294,6 +314,7 @@ resolver_component_tests: - expected_addrs: - {address: '1.2.3.4:443', is_balancer: false} expected_chosen_service_config: null + expected_service_config_error: null expected_lb_policy: null enable_srv_queries: true enable_txt_queries: false @@ -307,6 +328,7 @@ resolver_component_tests: - expected_addrs: - {address: '1.2.3.4:443', is_balancer: false} expected_chosen_service_config: null + expected_service_config_error: null expected_lb_policy: null enable_srv_queries: true enable_txt_queries: false @@ -317,3 +339,59 @@ resolver_component_tests: _grpc_config.ipv4-second-language-is-cpp-txt-disabled: - {TTL: '2100', data: 'grpc_config=[{"clientLanguage":["go"],"serviceConfig":{"loadBalancingPolicy":"round_robin","methodConfig":[{"name":[{"method":"Foo","service":"GoService"}],"waitForReady":true}]}},{"clientLanguage":["c++"],"serviceConfig":{"loadBalancingPolicy":"round_robin","methodConfig":[{"name":[{"method":"Foo","service":"CppService"}],"waitForReady":true}]}}]', type: TXT} +- expected_addrs: + - {address: '1.2.3.4:443', is_balancer: false} + expected_chosen_service_config: null + expected_service_config_error: 'could not parse' + expected_lb_policy: null + enable_srv_queries: true + enable_txt_queries: true + record_to_resolve: ipv4-svc_cfg_bad_json + records: + ipv4-svc_cfg_bad_json: + - {TTL: '2100', data: 1.2.3.4, type: A} + _grpc_config.ipv4-svc_cfg_bad_json: + - {TTL: '2100', data: 'grpc_config=[{]', + type: TXT} +- expected_addrs: + - {address: '1.2.3.4:443', is_balancer: false} + expected_chosen_service_config: null + expected_service_config_error: 'field:clientLanguage error:should be of type array' + expected_lb_policy: null + enable_srv_queries: true + enable_txt_queries: true + record_to_resolve: ipv4-svc_cfg_bad_client_language + records: + ipv4-svc_cfg_bad_client_language: + - {TTL: '2100', data: 1.2.3.4, type: A} + _grpc_config.ipv4-svc_cfg_bad_client_language: + - {TTL: '2100', data: 'grpc_config=[{"clientLanguage":"go","serviceConfig":{"loadBalancingPolicy":"round_robin","methodConfig":[{"name":[{"method":"Foo","service":"GoService"}],"waitForReady":true}]}},{"clientLanguage":["c++"],"serviceConfig":{"loadBalancingPolicy":"round_robin","methodConfig":[{"name":[{"method":"Foo","service":"CppService"}],"waitForReady":true}]}}]', + type: TXT} +- expected_addrs: + - {address: '1.2.3.4:443', is_balancer: false} + expected_chosen_service_config: null + expected_service_config_error: 'field:percentage error:should be of type number' + expected_lb_policy: null + enable_srv_queries: true + enable_txt_queries: true + record_to_resolve: ipv4-svc_cfg_bad_percentage + records: + ipv4-svc_cfg_bad_percentage: + - {TTL: '2100', data: 1.2.3.4, type: A} + _grpc_config.ipv4-svc_cfg_bad_percentage: + - {TTL: '2100', data: 'grpc_config=[{"percentage":"0","serviceConfig":{"loadBalancingPolicy":"round_robin","methodConfig":[{"name":[{"method":"Foo","service":"GoService"}],"waitForReady":true}]}},{"clientLanguage":["c++"],"serviceConfig":{"loadBalancingPolicy":"round_robin","methodConfig":[{"name":[{"method":"Foo","service":"CppService"}],"waitForReady":true}]}}]', + type: TXT} +- expected_addrs: + - {address: '1.2.3.4:443', is_balancer: false} + expected_chosen_service_config: null + expected_service_config_error: 'field:waitForReady error:Type should be true/false' + expected_lb_policy: null + enable_srv_queries: true + enable_txt_queries: true + record_to_resolve: ipv4-svc_cfg_bad_wait_for_ready + records: + ipv4-svc_cfg_bad_wait_for_ready: + - {TTL: '2100', data: 1.2.3.4, type: A} + _grpc_config.ipv4-svc_cfg_bad_wait_for_ready: + - {TTL: '2100', data: 'grpc_config=[{"serviceConfig":{"methodConfig":[{"name":[{"method":"Foo","service":"CppService"}],"waitForReady":"true"}]}}]', + type: TXT}