diff --git a/BUILD b/BUILD index 6f314e7a9be..38b8ca1c595 100644 --- a/BUILD +++ b/BUILD @@ -1231,21 +1231,6 @@ grpc_cc_library( ], ) -grpc_cc_library( - name = "grpc_grpclb_balancer_addresses", - srcs = [ - "src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb_balancer_addresses.cc", - ], - hdrs = [ - "src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb_balancer_addresses.h", - ], - language = "c++", - deps = [ - "grpc_base", - "grpc_client_channel", - ], -) - grpc_cc_library( name = "grpc_lb_policy_grpclb", srcs = [ @@ -1266,7 +1251,6 @@ grpc_cc_library( deps = [ "grpc_base", "grpc_client_channel", - "grpc_grpclb_balancer_addresses", "grpc_lb_upb", "grpc_resolver_fake", "grpc_transport_chttp2_client_insecure", @@ -1293,7 +1277,6 @@ grpc_cc_library( deps = [ "grpc_base", "grpc_client_channel", - "grpc_grpclb_balancer_addresses", "grpc_lb_upb", "grpc_resolver_fake", "grpc_secure", @@ -1619,7 +1602,6 @@ grpc_cc_library( deps = [ "grpc_base", "grpc_client_channel", - "grpc_grpclb_balancer_addresses", "grpc_resolver_dns_selection", ], ) diff --git a/BUILD.gn b/BUILD.gn index 36e959a562c..ba976743ba3 100644 --- a/BUILD.gn +++ b/BUILD.gn @@ -236,8 +236,6 @@ config("grpc_config") { "src/core/ext/filters/client_channel/lb_policy/grpclb/client_load_reporting_filter.h", "src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb.cc", "src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb.h", - "src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb_balancer_addresses.cc", - "src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb_balancer_addresses.h", "src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb_channel.h", "src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb_channel_secure.cc", "src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb_client_stats.cc", diff --git a/CMakeLists.txt b/CMakeLists.txt index cb1e9e2a09e..9acd2778b1d 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -1808,7 +1808,6 @@ add_library(grpc src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb_channel_secure.cc src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb_client_stats.cc src/core/ext/filters/client_channel/lb_policy/grpclb/load_balancer_api.cc - src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb_balancer_addresses.cc src/core/ext/upb-generated/src/proto/grpc/lb/v1/load_balancer.upb.c src/core/ext/filters/client_channel/resolver/fake/fake_resolver.cc src/core/ext/filters/client_channel/lb_policy/xds/cds.cc @@ -3303,7 +3302,6 @@ add_library(grpc_unsecure src/core/ext/filters/client_channel/resolver/dns/c_ares/grpc_ares_wrapper_posix.cc src/core/ext/filters/client_channel/resolver/dns/c_ares/grpc_ares_wrapper_windows.cc src/core/ext/filters/client_channel/resolver/dns/dns_resolver_selection.cc - src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb_balancer_addresses.cc src/core/ext/filters/client_channel/resolver/dns/native/dns_resolver.cc src/core/ext/filters/client_channel/resolver/sockaddr/sockaddr_resolver.cc src/core/ext/filters/client_channel/resolver/fake/fake_resolver.cc diff --git a/Makefile b/Makefile index 4a90b1b5ef7..5bf48adbd66 100644 --- a/Makefile +++ b/Makefile @@ -4273,7 +4273,6 @@ LIBGRPC_SRC = \ src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb_channel_secure.cc \ src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb_client_stats.cc \ src/core/ext/filters/client_channel/lb_policy/grpclb/load_balancer_api.cc \ - src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb_balancer_addresses.cc \ src/core/ext/upb-generated/src/proto/grpc/lb/v1/load_balancer.upb.c \ src/core/ext/filters/client_channel/resolver/fake/fake_resolver.cc \ src/core/ext/filters/client_channel/lb_policy/xds/cds.cc \ @@ -5710,7 +5709,6 @@ LIBGRPC_UNSECURE_SRC = \ src/core/ext/filters/client_channel/resolver/dns/c_ares/grpc_ares_wrapper_posix.cc \ src/core/ext/filters/client_channel/resolver/dns/c_ares/grpc_ares_wrapper_windows.cc \ src/core/ext/filters/client_channel/resolver/dns/dns_resolver_selection.cc \ - src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb_balancer_addresses.cc \ src/core/ext/filters/client_channel/resolver/dns/native/dns_resolver.cc \ src/core/ext/filters/client_channel/resolver/sockaddr/sockaddr_resolver.cc \ src/core/ext/filters/client_channel/resolver/fake/fake_resolver.cc \ diff --git a/build.yaml b/build.yaml index 7b7df27df97..c84bbd36bdb 100644 --- a/build.yaml +++ b/build.yaml @@ -1052,14 +1052,6 @@ filegroups: plugin: grpc_deadline_filter uses: - grpc_base -- name: grpc_grpclb_balancer_addresses - headers: - - src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb_balancer_addresses.h - src: - - src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb_balancer_addresses.cc - uses: - - grpc_base - - grpc_client_channel - name: grpc_health_upb headers: - src/core/ext/upb-generated/src/proto/grpc/health/v1/health.upb.h @@ -1115,7 +1107,6 @@ filegroups: uses: - grpc_base - grpc_client_channel - - grpc_grpclb_balancer_addresses - grpc_lb_upb - grpc_resolver_fake - name: grpc_lb_policy_grpclb_secure @@ -1137,7 +1128,6 @@ filegroups: uses: - grpc_base - grpc_client_channel - - grpc_grpclb_balancer_addresses - grpc_lb_upb - grpc_resolver_fake - grpc_secure @@ -1228,7 +1218,6 @@ filegroups: - grpc_base - grpc_client_channel - grpc_resolver_dns_selection - - grpc_grpclb_balancer_addresses - name: grpc_resolver_dns_native src: - src/core/ext/filters/client_channel/resolver/dns/native/dns_resolver.cc diff --git a/config.m4 b/config.m4 index 5c779152a5d..6def67546d8 100644 --- a/config.m4 +++ b/config.m4 @@ -52,7 +52,6 @@ if test "$PHP_GRPC" != "no"; then src/core/ext/filters/client_channel/lb_policy.cc \ src/core/ext/filters/client_channel/lb_policy/grpclb/client_load_reporting_filter.cc \ src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb.cc \ - src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb_balancer_addresses.cc \ src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb_channel_secure.cc \ src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb_client_stats.cc \ src/core/ext/filters/client_channel/lb_policy/grpclb/load_balancer_api.cc \ diff --git a/config.w32 b/config.w32 index 3d8131464d6..c7dc7b09e02 100644 --- a/config.w32 +++ b/config.w32 @@ -21,7 +21,6 @@ if (PHP_GRPC != "no") { "src\\core\\ext\\filters\\client_channel\\lb_policy.cc " + "src\\core\\ext\\filters\\client_channel\\lb_policy\\grpclb\\client_load_reporting_filter.cc " + "src\\core\\ext\\filters\\client_channel\\lb_policy\\grpclb\\grpclb.cc " + - "src\\core\\ext\\filters\\client_channel\\lb_policy\\grpclb\\grpclb_balancer_addresses.cc " + "src\\core\\ext\\filters\\client_channel\\lb_policy\\grpclb\\grpclb_channel_secure.cc " + "src\\core\\ext\\filters\\client_channel\\lb_policy\\grpclb\\grpclb_client_stats.cc " + "src\\core\\ext\\filters\\client_channel\\lb_policy\\grpclb\\load_balancer_api.cc " + diff --git a/gRPC-C++.podspec b/gRPC-C++.podspec index 01dd0e4d827..5bc0b71bb9e 100644 --- a/gRPC-C++.podspec +++ b/gRPC-C++.podspec @@ -232,7 +232,6 @@ Pod::Spec.new do |s| 'src/core/ext/filters/client_channel/lb_policy.h', 'src/core/ext/filters/client_channel/lb_policy/grpclb/client_load_reporting_filter.h', 'src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb.h', - 'src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb_balancer_addresses.h', 'src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb_channel.h', 'src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb_client_stats.h', 'src/core/ext/filters/client_channel/lb_policy/grpclb/load_balancer_api.h', @@ -654,7 +653,6 @@ Pod::Spec.new do |s| 'src/core/ext/filters/client_channel/lb_policy.h', 'src/core/ext/filters/client_channel/lb_policy/grpclb/client_load_reporting_filter.h', 'src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb.h', - 'src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb_balancer_addresses.h', 'src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb_channel.h', 'src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb_client_stats.h', 'src/core/ext/filters/client_channel/lb_policy/grpclb/load_balancer_api.h', diff --git a/gRPC-Core.podspec b/gRPC-Core.podspec index e30bf486961..e5a7011daff 100644 --- a/gRPC-Core.podspec +++ b/gRPC-Core.podspec @@ -205,8 +205,6 @@ Pod::Spec.new do |s| 'src/core/ext/filters/client_channel/lb_policy/grpclb/client_load_reporting_filter.h', 'src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb.cc', 'src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb.h', - 'src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb_balancer_addresses.cc', - 'src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb_balancer_addresses.h', 'src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb_channel.h', 'src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb_channel_secure.cc', 'src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb_client_stats.cc', @@ -971,7 +969,6 @@ Pod::Spec.new do |s| 'src/core/ext/filters/client_channel/lb_policy.h', 'src/core/ext/filters/client_channel/lb_policy/grpclb/client_load_reporting_filter.h', 'src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb.h', - 'src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb_balancer_addresses.h', 'src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb_channel.h', 'src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb_client_stats.h', 'src/core/ext/filters/client_channel/lb_policy/grpclb/load_balancer_api.h', diff --git a/grpc.gemspec b/grpc.gemspec index cd79a908123..78b6ffe021f 100644 --- a/grpc.gemspec +++ b/grpc.gemspec @@ -127,8 +127,6 @@ Gem::Specification.new do |s| s.files += %w( src/core/ext/filters/client_channel/lb_policy/grpclb/client_load_reporting_filter.h ) s.files += %w( src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb.cc ) s.files += %w( src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb.h ) - s.files += %w( src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb_balancer_addresses.cc ) - s.files += %w( src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb_balancer_addresses.h ) s.files += %w( src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb_channel.h ) s.files += %w( src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb_channel_secure.cc ) s.files += %w( src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb_client_stats.cc ) diff --git a/grpc.gyp b/grpc.gyp index 2f285fdb172..c57db04d5d7 100644 --- a/grpc.gyp +++ b/grpc.gyp @@ -815,7 +815,6 @@ 'src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb_channel_secure.cc', 'src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb_client_stats.cc', 'src/core/ext/filters/client_channel/lb_policy/grpclb/load_balancer_api.cc', - 'src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb_balancer_addresses.cc', 'src/core/ext/upb-generated/src/proto/grpc/lb/v1/load_balancer.upb.c', 'src/core/ext/filters/client_channel/resolver/fake/fake_resolver.cc', 'src/core/ext/filters/client_channel/lb_policy/xds/cds.cc', @@ -1660,7 +1659,6 @@ 'src/core/ext/filters/client_channel/resolver/dns/c_ares/grpc_ares_wrapper_posix.cc', 'src/core/ext/filters/client_channel/resolver/dns/c_ares/grpc_ares_wrapper_windows.cc', 'src/core/ext/filters/client_channel/resolver/dns/dns_resolver_selection.cc', - 'src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb_balancer_addresses.cc', 'src/core/ext/filters/client_channel/resolver/dns/native/dns_resolver.cc', 'src/core/ext/filters/client_channel/resolver/sockaddr/sockaddr_resolver.cc', 'src/core/ext/filters/client_channel/resolver/fake/fake_resolver.cc', diff --git a/package.xml b/package.xml index 130c5dec338..f75bb6a8dbc 100644 --- a/package.xml +++ b/package.xml @@ -110,8 +110,6 @@ - - diff --git a/src/core/ext/filters/client_channel/client_channel.cc b/src/core/ext/filters/client_channel/client_channel.cc index 5c486515f0c..7e3fe1b3be9 100644 --- a/src/core/ext/filters/client_channel/client_channel.cc +++ b/src/core/ext/filters/client_channel/client_channel.cc @@ -1638,6 +1638,26 @@ void ChannelData::ProcessLbPolicy( grpc_channel_args_find(resolver_result.args, GRPC_ARG_LB_POLICY_NAME); local_policy_name = grpc_channel_arg_get_string(channel_arg); } + // Special case: If at least one balancer address is present, we use + // the grpclb policy, regardless of what the resolver has returned. + bool found_balancer_address = false; + for (size_t i = 0; i < resolver_result.addresses.size(); ++i) { + const ServerAddress& address = resolver_result.addresses[i]; + if (address.IsBalancer()) { + found_balancer_address = true; + break; + } + } + if (found_balancer_address) { + if (local_policy_name != nullptr && + strcmp(local_policy_name, "grpclb") != 0) { + gpr_log(GPR_INFO, + "resolver requested LB policy %s but provided at least one " + "balancer address -- forcing use of grpclb LB policy", + local_policy_name); + } + local_policy_name = "grpclb"; + } // Use pick_first if nothing was specified and we didn't select grpclb // above. lb_policy_name->reset(gpr_strdup( diff --git a/src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb.cc b/src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb.cc index aa7b9fe029f..cdcf997e62b 100644 --- a/src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb.cc +++ b/src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb.cc @@ -73,7 +73,6 @@ #include "src/core/ext/filters/client_channel/client_channel.h" #include "src/core/ext/filters/client_channel/lb_policy/grpclb/client_load_reporting_filter.h" #include "src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb.h" -#include "src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb_balancer_addresses.h" #include "src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb_channel.h" #include "src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb_client_stats.h" #include "src/core/ext/filters/client_channel/lb_policy/grpclb/load_balancer_api.h" @@ -1268,11 +1267,25 @@ void GrpcLb::BalancerCallState::OnBalancerStatusReceivedLocked( // helper code for creating balancer channel // -ServerAddressList ExtractBalancerAddresses(const grpc_channel_args& args) { - const ServerAddressList* addresses = - FindGrpclbBalancerAddressesInChannelArgs(args); - if (addresses != nullptr) return *addresses; - return ServerAddressList(); +ServerAddressList ExtractBalancerAddresses(const ServerAddressList& addresses) { + ServerAddressList balancer_addresses; + for (size_t i = 0; i < addresses.size(); ++i) { + if (addresses[i].IsBalancer()) { + // Strip out the is_balancer channel arg, since we don't want to + // recursively use the grpclb policy in the channel used to talk to + // the balancers. Note that we do NOT strip out the balancer_name + // channel arg, since we need that to set the authority correctly + // to talk to the balancers. + static const char* args_to_remove[] = { + GRPC_ARG_ADDRESS_IS_BALANCER, + }; + balancer_addresses.emplace_back( + addresses[i].address(), + grpc_channel_args_copy_and_remove(addresses[i].args(), args_to_remove, + GPR_ARRAY_SIZE(args_to_remove))); + } + } + return balancer_addresses; } /* Returns the channel args for the LB channel, used to create a bidirectional @@ -1478,25 +1491,27 @@ void GrpcLb::UpdateLocked(UpdateArgs args) { // helpers for UpdateLocked() // -ServerAddressList AddNullLbTokenToAddresses( - const ServerAddressList& addresses) { +// Returns the backend addresses extracted from the given addresses. +ServerAddressList ExtractBackendAddresses(const ServerAddressList& addresses) { static const char* lb_token = ""; grpc_arg arg = grpc_channel_arg_pointer_create( const_cast(GRPC_ARG_GRPCLB_ADDRESS_LB_TOKEN), const_cast(lb_token), &lb_token_arg_vtable); - ServerAddressList addresses_out; + ServerAddressList backend_addresses; for (size_t i = 0; i < addresses.size(); ++i) { - addresses_out.emplace_back( - addresses[i].address(), - grpc_channel_args_copy_and_add(addresses[i].args(), &arg, 1)); + if (!addresses[i].IsBalancer()) { + backend_addresses.emplace_back( + addresses[i].address(), + grpc_channel_args_copy_and_add(addresses[i].args(), &arg, 1)); + } } - return addresses_out; + return backend_addresses; } void GrpcLb::ProcessAddressesAndChannelArgsLocked( const ServerAddressList& addresses, const grpc_channel_args& args) { // Update fallback address list. - fallback_backend_addresses_ = AddNullLbTokenToAddresses(addresses); + fallback_backend_addresses_ = ExtractBackendAddresses(addresses); // Make sure that GRPC_ARG_LB_POLICY_NAME is set in channel args, // since we use this to trigger the client_load_reporting filter. static const char* args_to_remove[] = {GRPC_ARG_LB_POLICY_NAME}; @@ -1506,7 +1521,7 @@ void GrpcLb::ProcessAddressesAndChannelArgsLocked( args_ = grpc_channel_args_copy_and_add_and_remove( &args, args_to_remove, GPR_ARRAY_SIZE(args_to_remove), &new_arg, 1); // Construct args for balancer channel. - ServerAddressList balancer_addresses = ExtractBalancerAddresses(args); + ServerAddressList balancer_addresses = ExtractBalancerAddresses(addresses); grpc_channel_args* lb_channel_args = BuildBalancerChannelArgs( balancer_addresses, response_generator_.get(), &args); // Create balancer channel if needed. diff --git a/src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb_balancer_addresses.cc b/src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb_balancer_addresses.cc deleted file mode 100644 index 2888c3b94ae..00000000000 --- a/src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb_balancer_addresses.cc +++ /dev/null @@ -1,89 +0,0 @@ -// -// Copyright 2019 gRPC authors. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. -// - -#include - -#include "src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb_balancer_addresses.h" - -#include "src/core/lib/channel/channel_args.h" -#include "src/core/lib/gpr/useful.h" - -// Channel arg key for the list of balancer addresses. -#define GRPC_ARG_GRPCLB_BALANCER_ADDRESSES "grpc.grpclb_balancer_addresses" -// Channel arg key for a string indicating an address's balancer name. -#define GRPC_ARG_ADDRESS_BALANCER_NAME "grpc.address_balancer_name" - -namespace grpc_core { - -namespace { - -void* BalancerAddressesArgCopy(void* p) { - ServerAddressList* address_list = static_cast(p); - return new ServerAddressList(*address_list); -} - -void BalancerAddressesArgDestroy(void* p) { - ServerAddressList* address_list = static_cast(p); - delete address_list; -} - -int BalancerAddressesArgCmp(void* p, void* q) { - ServerAddressList* address_list1 = static_cast(p); - ServerAddressList* address_list2 = static_cast(q); - if (address_list1 == nullptr || address_list2 == nullptr) { - return GPR_ICMP(address_list1, address_list2); - } - if (address_list1->size() > address_list2->size()) return 1; - if (address_list1->size() < address_list2->size()) return -1; - for (size_t i = 0; i < address_list1->size(); ++i) { - int retval = (*address_list1)[i].Cmp((*address_list2)[i]); - if (retval != 0) return retval; - } - return 0; -} - -const grpc_arg_pointer_vtable kBalancerAddressesArgVtable = { - BalancerAddressesArgCopy, BalancerAddressesArgDestroy, - BalancerAddressesArgCmp}; - -} // namespace - -grpc_arg CreateGrpclbBalancerAddressesArg( - const ServerAddressList* address_list) { - return grpc_channel_arg_pointer_create( - const_cast(GRPC_ARG_GRPCLB_BALANCER_ADDRESSES), - const_cast(address_list), - &kBalancerAddressesArgVtable); -} - -const ServerAddressList* FindGrpclbBalancerAddressesInChannelArgs( - const grpc_channel_args& args) { - return grpc_channel_args_find_pointer( - &args, const_cast(GRPC_ARG_GRPCLB_BALANCER_ADDRESSES)); -} - -grpc_arg CreateGrpclbBalancerNameArg(const char* balancer_name) { - return grpc_channel_arg_string_create( - const_cast(GRPC_ARG_ADDRESS_BALANCER_NAME), - const_cast(balancer_name)); -} - -const char* FindGrpclbBalancerNameInChannelArgs(const grpc_channel_args& args) { - return grpc_channel_args_find_string( - &args, const_cast(GRPC_ARG_ADDRESS_BALANCER_NAME)); -} - -} // namespace grpc_core diff --git a/src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb_balancer_addresses.h b/src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb_balancer_addresses.h deleted file mode 100644 index 9b6b259deca..00000000000 --- a/src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb_balancer_addresses.h +++ /dev/null @@ -1,40 +0,0 @@ -// -// Copyright 2019 gRPC authors. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. -// - -#ifndef GRPC_CORE_EXT_FILTERS_CLIENT_CHANNEL_LB_POLICY_GRPCLB_GRPCLB_BALANCER_ADDRESSES_H -#define GRPC_CORE_EXT_FILTERS_CLIENT_CHANNEL_LB_POLICY_GRPCLB_GRPCLB_BALANCER_ADDRESSES_H - -#include - -#include - -#include "src/core/ext/filters/client_channel/server_address.h" - -namespace grpc_core { - -grpc_arg CreateGrpclbBalancerAddressesArg( - const ServerAddressList* address_list); -const ServerAddressList* FindGrpclbBalancerAddressesInChannelArgs( - const grpc_channel_args& args); - -grpc_arg CreateGrpclbBalancerNameArg(const char* balancer_name); -const char* FindGrpclbBalancerNameInChannelArgs(const grpc_channel_args& args); - -} // namespace grpc_core - -#endif /* \ -GRPC_CORE_EXT_FILTERS_CLIENT_CHANNEL_LB_POLICY_GRPCLB_GRPCLB_BALANCER_ADDRESSES_H \ - */ diff --git a/src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb_channel_secure.cc b/src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb_channel_secure.cc index 414f8274317..5bc4f5157ad 100644 --- a/src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb_channel_secure.cc +++ b/src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb_channel_secure.cc @@ -27,7 +27,6 @@ #include #include "src/core/ext/filters/client_channel/client_channel.h" -#include "src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb_balancer_addresses.h" #include "src/core/ext/filters/client_channel/server_address.h" #include "src/core/lib/channel/channel_args.h" #include "src/core/lib/gpr/string.h" @@ -56,8 +55,8 @@ RefCountedPtr CreateTargetAuthorityTable( grpc_sockaddr_to_string(&addr_str, &addresses[i].address(), true) > 0); target_authority_entries[i].key = grpc_slice_from_copied_string(addr_str); gpr_free(addr_str); - const char* balancer_name = - FindGrpclbBalancerNameInChannelArgs(*addresses[i].args()); + char* balancer_name = grpc_channel_arg_get_string(grpc_channel_args_find( + addresses[i].args(), GRPC_ARG_ADDRESS_BALANCER_NAME)); target_authority_entries[i].value.reset(gpr_strdup(balancer_name)); } RefCountedPtr target_authority_table = diff --git a/src/core/ext/filters/client_channel/lb_policy/subchannel_list.h b/src/core/ext/filters/client_channel/lb_policy/subchannel_list.h index 2d8988926e4..4932a6869f1 100644 --- a/src/core/ext/filters/client_channel/lb_policy/subchannel_list.h +++ b/src/core/ext/filters/client_channel/lb_policy/subchannel_list.h @@ -370,6 +370,13 @@ SubchannelList::SubchannelList( GRPC_ARG_SERVICE_CONFIG}; // Create a subchannel for each address. for (size_t i = 0; i < addresses.size(); i++) { + // TODO(roth): we should ideally hide this from the LB policy code. In + // principle, if we're dealing with this special case in the client_channel + // code for selecting grpclb, then we should also strip out these addresses + // there if we're not using grpclb. + if (addresses[i].IsBalancer()) { + continue; + } InlinedVector args_to_add; const size_t subchannel_address_arg_index = args_to_add.size(); args_to_add.emplace_back( diff --git a/src/core/ext/filters/client_channel/resolver/dns/c_ares/dns_resolver_ares.cc b/src/core/ext/filters/client_channel/resolver/dns/c_ares/dns_resolver_ares.cc index e52178a911a..3f281537e2d 100644 --- a/src/core/ext/filters/client_channel/resolver/dns/c_ares/dns_resolver_ares.cc +++ b/src/core/ext/filters/client_channel/resolver/dns/c_ares/dns_resolver_ares.cc @@ -30,7 +30,6 @@ #include #include "src/core/ext/filters/client_channel/http_connect_handshaker.h" -#include "src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb_balancer_addresses.h" #include "src/core/ext/filters/client_channel/lb_policy_registry.h" #include "src/core/ext/filters/client_channel/resolver/dns/c_ares/grpc_ares_wrapper.h" #include "src/core/ext/filters/client_channel/resolver/dns/dns_resolver_selection.h" @@ -108,10 +107,8 @@ class AresDnsResolver : public Resolver { grpc_millis last_resolution_timestamp_ = -1; /// retry backoff state BackOff backoff_; - /// currently resolving backend addresses + /// currently resolving addresses std::unique_ptr addresses_; - /// currently resolving balancer addresses - std::unique_ptr balancer_addresses_; /// currently resolving service config char* service_config_json_ = nullptr; // has shutdown been initiated @@ -343,11 +340,9 @@ void AresDnsResolver::OnResolvedLocked(void* arg, grpc_error* error) { r->Unref(DEBUG_LOCATION, "OnResolvedLocked() shutdown"); return; } - if (r->addresses_ != nullptr || r->balancer_addresses_ != nullptr) { + if (r->addresses_ != nullptr) { Result result; - if (r->addresses_ != nullptr) { - result.addresses = std::move(*r->addresses_); - } + result.addresses = std::move(*r->addresses_); if (r->service_config_json_ != nullptr) { char* service_config_string = ChooseServiceConfig( r->service_config_json_, &result.service_config_error); @@ -361,16 +356,9 @@ void AresDnsResolver::OnResolvedLocked(void* arg, grpc_error* error) { } gpr_free(service_config_string); } - InlinedVector new_args; - if (r->balancer_addresses_ != nullptr) { - new_args.push_back( - CreateGrpclbBalancerAddressesArg(r->balancer_addresses_.get())); - } - result.args = grpc_channel_args_copy_and_add( - r->channel_args_, new_args.data(), new_args.size()); + result.args = grpc_channel_args_copy(r->channel_args_); r->result_handler()->ReturnResult(std::move(result)); r->addresses_.reset(); - r->balancer_addresses_.reset(); // Reset backoff state so that we start from the beginning when the // next request gets triggered. r->backoff_.Reset(); @@ -449,8 +437,7 @@ void AresDnsResolver::StartResolvingLocked() { GRPC_CLOSURE_INIT(&on_resolved_, OnResolved, this, grpc_schedule_on_exec_ctx); pending_request_ = grpc_dns_lookup_ares_locked( dns_server_, name_to_resolve_, kDefaultPort, interested_parties_, - &on_resolved_, &addresses_, - enable_srv_queries_ ? &balancer_addresses_ : nullptr, + &on_resolved_, &addresses_, enable_srv_queries_ /* check_grpclb */, request_service_config_ ? &service_config_json_ : nullptr, query_timeout_ms_, combiner()); last_resolution_timestamp_ = grpc_core::ExecCtx::Get()->Now(); diff --git a/src/core/ext/filters/client_channel/resolver/dns/c_ares/grpc_ares_wrapper.cc b/src/core/ext/filters/client_channel/resolver/dns/c_ares/grpc_ares_wrapper.cc index a689d0a2d97..d2c00000fdc 100644 --- a/src/core/ext/filters/client_channel/resolver/dns/c_ares/grpc_ares_wrapper.cc +++ b/src/core/ext/filters/client_channel/resolver/dns/c_ares/grpc_ares_wrapper.cc @@ -33,7 +33,6 @@ #include #include -#include "src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb_balancer_addresses.h" #include "src/core/ext/filters/client_channel/parse_address.h" #include "src/core/ext/filters/client_channel/resolver/dns/c_ares/grpc_ares_ev_driver.h" #include "src/core/lib/gpr/string.h" @@ -61,8 +60,6 @@ struct grpc_ares_request { grpc_closure* on_done; /** the pointer to receive the resolved addresses */ std::unique_ptr* addresses_out; - /** the pointer to receive the resolved balancer addresses */ - std::unique_ptr* balancer_addresses_out; /** the pointer to receive the service config in JSON */ char** service_config_json_out; /** the evernt driver used by this request */ @@ -187,17 +184,17 @@ static void on_hostbyname_done_locked(void* arg, int status, int /*timeouts*/, GRPC_CARES_TRACE_LOG( "request:%p on_hostbyname_done_locked host=%s ARES_SUCCESS", r, hr->host); - std::unique_ptr* address_list_ptr = - hr->is_balancer ? r->balancer_addresses_out : r->addresses_out; - if (*address_list_ptr == nullptr) { - *address_list_ptr = grpc_core::MakeUnique(); + if (*r->addresses_out == nullptr) { + *r->addresses_out = grpc_core::MakeUnique(); } - ServerAddressList& addresses = **address_list_ptr; + ServerAddressList& addresses = **r->addresses_out; for (size_t i = 0; hostent->h_addr_list[i] != nullptr; ++i) { - grpc_core::InlinedVector args_to_add; + grpc_core::InlinedVector args_to_add; if (hr->is_balancer) { - args_to_add.emplace_back( - grpc_core::CreateGrpclbBalancerNameArg(hr->host)); + args_to_add.emplace_back(grpc_channel_arg_integer_create( + const_cast(GRPC_ARG_ADDRESS_IS_BALANCER), 1)); + args_to_add.emplace_back(grpc_channel_arg_string_create( + const_cast(GRPC_ARG_ADDRESS_BALANCER_NAME), hr->host)); } grpc_channel_args* args = grpc_channel_args_copy_and_add( nullptr, args_to_add.data(), args_to_add.size()); @@ -353,7 +350,7 @@ done: void grpc_dns_lookup_ares_continue_after_check_localhost_and_ip_literals_locked( grpc_ares_request* r, const char* dns_server, const char* name, const char* default_port, grpc_pollset_set* interested_parties, - int query_timeout_ms, grpc_core::Combiner* combiner) { + bool check_grpclb, int query_timeout_ms, grpc_core::Combiner* combiner) { grpc_error* error = GRPC_ERROR_NONE; grpc_ares_hostbyname_request* hr = nullptr; ares_channel* channel = nullptr; @@ -428,7 +425,7 @@ void grpc_dns_lookup_ares_continue_after_check_localhost_and_ip_literals_locked( /*is_balancer=*/false); ares_gethostbyname(*channel, hr->host, AF_INET, on_hostbyname_done_locked, hr); - if (r->balancer_addresses_out != nullptr) { + if (check_grpclb) { /* Query the SRV record */ grpc_ares_request_ref_locked(r); char* service_name; @@ -591,8 +588,7 @@ static bool grpc_ares_maybe_resolve_localhost_manually_locked( static grpc_ares_request* grpc_dns_lookup_ares_locked_impl( const char* dns_server, const char* name, const char* default_port, grpc_pollset_set* interested_parties, grpc_closure* on_done, - std::unique_ptr* addrs, - std::unique_ptr* balancer_addrs, + std::unique_ptr* addrs, bool check_grpclb, char** service_config_json, int query_timeout_ms, grpc_core::Combiner* combiner) { grpc_ares_request* r = @@ -600,7 +596,6 @@ static grpc_ares_request* grpc_dns_lookup_ares_locked_impl( r->ev_driver = nullptr; r->on_done = on_done; r->addresses_out = addrs; - r->balancer_addresses_out = balancer_addrs; r->service_config_json_out = service_config_json; r->error = GRPC_ERROR_NONE; r->pending_queries = 0; @@ -623,21 +618,20 @@ static grpc_ares_request* grpc_dns_lookup_ares_locked_impl( // as to cut down on lookups over the network, especially in tests: // https://github.com/grpc/proposal/pull/79 if (target_matches_localhost(name)) { - r->balancer_addresses_out = nullptr; + check_grpclb = false; r->service_config_json_out = nullptr; } // Look up name using c-ares lib. grpc_dns_lookup_ares_continue_after_check_localhost_and_ip_literals_locked( - r, dns_server, name, default_port, interested_parties, query_timeout_ms, - combiner); + r, dns_server, name, default_port, interested_parties, check_grpclb, + query_timeout_ms, combiner); return r; } grpc_ares_request* (*grpc_dns_lookup_ares_locked)( const char* dns_server, const char* name, const char* default_port, grpc_pollset_set* interested_parties, grpc_closure* on_done, - std::unique_ptr* addrs, - std::unique_ptr* balancer_addrs, + std::unique_ptr* addrs, bool check_grpclb, char** service_config_json, int query_timeout_ms, grpc_core::Combiner* combiner) = grpc_dns_lookup_ares_locked_impl; @@ -715,6 +709,7 @@ static void on_dns_lookup_done_locked(void* arg, grpc_error* error) { static_cast(gpr_zalloc( sizeof(grpc_resolved_address) * (*resolved_addresses)->naddrs)); for (size_t i = 0; i < (*resolved_addresses)->naddrs; ++i) { + GPR_ASSERT(!(*r->addresses)[i].IsBalancer()); memcpy(&(*resolved_addresses)->addrs[i], &(*r->addresses)[i].address(), sizeof(grpc_resolved_address)); } @@ -741,9 +736,9 @@ static void grpc_resolve_address_invoke_dns_lookup_ares_locked( grpc_schedule_on_exec_ctx); r->ares_request = grpc_dns_lookup_ares_locked( nullptr /* dns_server */, r->name, r->default_port, r->interested_parties, - &r->on_dns_lookup_done_locked, &r->addresses, - nullptr /* balancer_addresses */, nullptr /* service_config_json */, - GRPC_DNS_ARES_DEFAULT_QUERY_TIMEOUT_MS, r->combiner); + &r->on_dns_lookup_done_locked, &r->addresses, false /* check_grpclb */, + nullptr /* service_config_json */, GRPC_DNS_ARES_DEFAULT_QUERY_TIMEOUT_MS, + r->combiner); } static void grpc_resolve_address_ares_impl(const char* name, diff --git a/src/core/ext/filters/client_channel/resolver/dns/c_ares/grpc_ares_wrapper.h b/src/core/ext/filters/client_channel/resolver/dns/c_ares/grpc_ares_wrapper.h index bfa888ce9f5..115018155b3 100644 --- a/src/core/ext/filters/client_channel/resolver/dns/c_ares/grpc_ares_wrapper.h +++ b/src/core/ext/filters/client_channel/resolver/dns/c_ares/grpc_ares_wrapper.h @@ -63,8 +63,7 @@ extern void (*grpc_resolve_address_ares)(const char* name, extern grpc_ares_request* (*grpc_dns_lookup_ares_locked)( const char* dns_server, const char* name, const char* default_port, grpc_pollset_set* interested_parties, grpc_closure* on_done, - std::unique_ptr* addresses, - std::unique_ptr* balancer_addresses, + std::unique_ptr* addresses, bool check_grpclb, char** service_config_json, int query_timeout_ms, grpc_core::Combiner* combiner); diff --git a/src/core/ext/filters/client_channel/resolver/dns/c_ares/grpc_ares_wrapper_fallback.cc b/src/core/ext/filters/client_channel/resolver/dns/c_ares/grpc_ares_wrapper_fallback.cc index be880f6c1c2..fe631f873e9 100644 --- a/src/core/ext/filters/client_channel/resolver/dns/c_ares/grpc_ares_wrapper_fallback.cc +++ b/src/core/ext/filters/client_channel/resolver/dns/c_ares/grpc_ares_wrapper_fallback.cc @@ -29,8 +29,7 @@ struct grpc_ares_request { static grpc_ares_request* grpc_dns_lookup_ares_locked_impl( const char* dns_server, const char* name, const char* default_port, grpc_pollset_set* interested_parties, grpc_closure* on_done, - std::unique_ptr* addrs, - std::unique_ptr* balancer_addrs, + std::unique_ptr* addrs, bool check_grpclb, char** service_config_json, int query_timeout_ms, grpc_core::Combiner* combiner) { return NULL; @@ -39,8 +38,7 @@ static grpc_ares_request* grpc_dns_lookup_ares_locked_impl( grpc_ares_request* (*grpc_dns_lookup_ares_locked)( const char* dns_server, const char* name, const char* default_port, grpc_pollset_set* interested_parties, grpc_closure* on_done, - std::unique_ptr* addrs, - std::unique_ptr* balancer_addrs, + std::unique_ptr* addrs, bool check_grpclb, char** service_config_json, int query_timeout_ms, grpc_core::Combiner* combiner) = grpc_dns_lookup_ares_locked_impl; diff --git a/src/core/ext/filters/client_channel/server_address.cc b/src/core/ext/filters/client_channel/server_address.cc index 93d361a8154..d46896b754b 100644 --- a/src/core/ext/filters/client_channel/server_address.cc +++ b/src/core/ext/filters/client_channel/server_address.cc @@ -37,12 +37,15 @@ ServerAddress::ServerAddress(const void* address, size_t address_len, address_.len = static_cast(address_len); } -int ServerAddress::Cmp(const ServerAddress& other) const { - if (address_.len > other.address_.len) return 1; - if (address_.len < other.address_.len) return -1; - int retval = memcmp(address_.addr, other.address_.addr, address_.len); - if (retval != 0) return retval; - return grpc_channel_args_compare(args_, other.args_); +bool ServerAddress::operator==(const ServerAddress& other) const { + return address_.len == other.address_.len && + memcmp(address_.addr, other.address_.addr, address_.len) == 0 && + grpc_channel_args_compare(args_, other.args_) == 0; +} + +bool ServerAddress::IsBalancer() const { + return grpc_channel_arg_get_bool( + grpc_channel_args_find(args_, GRPC_ARG_ADDRESS_IS_BALANCER), false); } } // namespace grpc_core diff --git a/src/core/ext/filters/client_channel/server_address.h b/src/core/ext/filters/client_channel/server_address.h index 10f49f2f344..acd71358810 100644 --- a/src/core/ext/filters/client_channel/server_address.h +++ b/src/core/ext/filters/client_channel/server_address.h @@ -25,6 +25,13 @@ #include "src/core/lib/gprpp/inlined_vector.h" #include "src/core/lib/iomgr/resolve_address.h" +// Channel arg key for a bool indicating whether an address is a grpclb +// load balancer (as opposed to a backend). +#define GRPC_ARG_ADDRESS_IS_BALANCER "grpc.address_is_balancer" + +// Channel arg key for a string indicating an address's balancer name. +#define GRPC_ARG_ADDRESS_BALANCER_NAME "grpc.address_balancer_name" + namespace grpc_core { // @@ -66,13 +73,13 @@ class ServerAddress { return *this; } - bool operator==(const ServerAddress& other) const { return Cmp(other) == 0; } - - int Cmp(const ServerAddress& other) const; + bool operator==(const ServerAddress& other) const; const grpc_resolved_address& address() const { return address_; } const grpc_channel_args* args() const { return args_; } + bool IsBalancer() const; + private: grpc_resolved_address address_; grpc_channel_args* args_; diff --git a/src/python/grpcio/grpc_core_dependencies.py b/src/python/grpcio/grpc_core_dependencies.py index 1190869bcff..ac1d19dd129 100644 --- a/src/python/grpcio/grpc_core_dependencies.py +++ b/src/python/grpcio/grpc_core_dependencies.py @@ -30,7 +30,6 @@ CORE_SOURCE_FILES = [ 'src/core/ext/filters/client_channel/lb_policy.cc', 'src/core/ext/filters/client_channel/lb_policy/grpclb/client_load_reporting_filter.cc', 'src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb.cc', - 'src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb_balancer_addresses.cc', 'src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb_channel_secure.cc', 'src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb_client_stats.cc', 'src/core/ext/filters/client_channel/lb_policy/grpclb/load_balancer_api.cc', diff --git a/test/core/client_channel/resolvers/dns_resolver_connectivity_test.cc b/test/core/client_channel/resolvers/dns_resolver_connectivity_test.cc index 0b3126321a4..95eafeff54f 100644 --- a/test/core/client_channel/resolvers/dns_resolver_connectivity_test.cc +++ b/test/core/client_channel/resolvers/dns_resolver_connectivity_test.cc @@ -64,9 +64,8 @@ static grpc_ares_request* my_dns_lookup_ares_locked( const char* /*dns_server*/, const char* addr, const char* /*default_port*/, grpc_pollset_set* /*interested_parties*/, grpc_closure* on_done, std::unique_ptr* addresses, - std::unique_ptr* /*balancer_addresses*/, - char** /*service_config_json*/, int /*query_timeout_ms*/, - grpc_core::Combiner* /*combiner*/) { + bool /*check_grpclb*/, char** /*service_config_json*/, + int /*query_timeout_ms*/, grpc_core::Combiner* /*combiner*/) { gpr_mu_lock(&g_mu); GPR_ASSERT(0 == strcmp("test", addr)); grpc_error* error = GRPC_ERROR_NONE; diff --git a/test/core/client_channel/resolvers/dns_resolver_cooldown_test.cc b/test/core/client_channel/resolvers/dns_resolver_cooldown_test.cc index 14b0d9fe40e..94699bd3b31 100644 --- a/test/core/client_channel/resolvers/dns_resolver_cooldown_test.cc +++ b/test/core/client_channel/resolvers/dns_resolver_cooldown_test.cc @@ -42,8 +42,7 @@ static grpc_core::Combiner* g_combiner; static grpc_ares_request* (*g_default_dns_lookup_ares_locked)( const char* dns_server, const char* name, const char* default_port, grpc_pollset_set* interested_parties, grpc_closure* on_done, - std::unique_ptr* addresses, - std::unique_ptr* balancer_addresses, + std::unique_ptr* addresses, bool check_grpclb, char** service_config_json, int query_timeout_ms, grpc_core::Combiner* combiner); @@ -94,14 +93,12 @@ static grpc_address_resolver_vtable test_resolver = { static grpc_ares_request* test_dns_lookup_ares_locked( const char* dns_server, const char* name, const char* default_port, grpc_pollset_set* /*interested_parties*/, grpc_closure* on_done, - std::unique_ptr* addresses, - std::unique_ptr* balancer_addresses, + std::unique_ptr* addresses, bool check_grpclb, char** service_config_json, int query_timeout_ms, grpc_core::Combiner* combiner) { grpc_ares_request* result = g_default_dns_lookup_ares_locked( dns_server, name, default_port, g_iomgr_args.pollset_set, on_done, - addresses, balancer_addresses, service_config_json, query_timeout_ms, - combiner); + addresses, check_grpclb, service_config_json, query_timeout_ms, combiner); ++g_resolution_count; static grpc_millis last_resolution_time = 0; grpc_millis now = diff --git a/test/core/client_channel/resolvers/fake_resolver_test.cc b/test/core/client_channel/resolvers/fake_resolver_test.cc index c9b5828d92e..01d82cbdd05 100644 --- a/test/core/client_channel/resolvers/fake_resolver_test.cc +++ b/test/core/client_channel/resolvers/fake_resolver_test.cc @@ -86,18 +86,29 @@ static grpc_core::Resolver::Result create_new_resolver_result() { static size_t test_counter = 0; const size_t num_addresses = 2; char* uri_string; + char* balancer_name; // Create address list. grpc_core::Resolver::Result result; for (size_t i = 0; i < num_addresses; ++i) { gpr_asprintf(&uri_string, "ipv4:127.0.0.1:100%" PRIuPTR, test_counter * num_addresses + i); grpc_uri* uri = grpc_uri_parse(uri_string, true); + gpr_asprintf(&balancer_name, "balancer%" PRIuPTR, + test_counter * num_addresses + i); grpc_resolved_address address; GPR_ASSERT(grpc_parse_uri(uri, &address)); grpc_core::InlinedVector args_to_add; - result.addresses.emplace_back( - address.addr, address.len, - grpc_channel_args_copy_and_add(nullptr, nullptr, 0)); + const bool is_balancer = num_addresses % 2; + if (is_balancer) { + args_to_add.emplace_back(grpc_channel_arg_integer_create( + const_cast(GRPC_ARG_ADDRESS_IS_BALANCER), 1)); + args_to_add.emplace_back(grpc_channel_arg_string_create( + const_cast(GRPC_ARG_ADDRESS_BALANCER_NAME), balancer_name)); + } + grpc_channel_args* args = grpc_channel_args_copy_and_add( + nullptr, args_to_add.data(), args_to_add.size()); + result.addresses.emplace_back(address.addr, address.len, args); + gpr_free(balancer_name); grpc_uri_destroy(uri); gpr_free(uri_string); } diff --git a/test/core/end2end/fuzzers/api_fuzzer.cc b/test/core/end2end/fuzzers/api_fuzzer.cc index 3f365ecf26c..eecf9363bfb 100644 --- a/test/core/end2end/fuzzers/api_fuzzer.cc +++ b/test/core/end2end/fuzzers/api_fuzzer.cc @@ -378,9 +378,8 @@ grpc_ares_request* my_dns_lookup_ares_locked( const char* /*dns_server*/, const char* addr, const char* /*default_port*/, grpc_pollset_set* /*interested_parties*/, grpc_closure* on_done, std::unique_ptr* addresses, - std::unique_ptr* /*balancer_addresses*/, - char** /*service_config_json*/, int /*query_timeout*/, - grpc_core::Combiner* /*combiner*/) { + bool /*check_grpclb*/, char** /*service_config_json*/, + int /*query_timeout*/, grpc_core::Combiner* /*combiner*/) { addr_req* r = static_cast(gpr_malloc(sizeof(*r))); r->addr = gpr_strdup(addr); r->on_done = on_done; diff --git a/test/core/end2end/goaway_server_test.cc b/test/core/end2end/goaway_server_test.cc index 94e9d7552f4..86f27295a5f 100644 --- a/test/core/end2end/goaway_server_test.cc +++ b/test/core/end2end/goaway_server_test.cc @@ -47,8 +47,7 @@ static int g_resolve_port = -1; static grpc_ares_request* (*iomgr_dns_lookup_ares_locked)( const char* dns_server, const char* addr, const char* default_port, grpc_pollset_set* interested_parties, grpc_closure* on_done, - std::unique_ptr* addresses, - std::unique_ptr* balancer_addresses, + std::unique_ptr* addresses, bool check_grpclb, char** service_config_json, int query_timeout_ms, grpc_core::Combiner* combiner); @@ -105,14 +104,13 @@ static grpc_address_resolver_vtable test_resolver = { static grpc_ares_request* my_dns_lookup_ares_locked( const char* dns_server, const char* addr, const char* default_port, grpc_pollset_set* interested_parties, grpc_closure* on_done, - std::unique_ptr* addresses, - std::unique_ptr* balancer_addresses, + std::unique_ptr* addresses, bool check_grpclb, char** service_config_json, int query_timeout_ms, grpc_core::Combiner* combiner) { if (0 != strcmp(addr, "test")) { return iomgr_dns_lookup_ares_locked( dns_server, addr, default_port, interested_parties, on_done, addresses, - balancer_addresses, service_config_json, query_timeout_ms, combiner); + check_grpclb, service_config_json, query_timeout_ms, combiner); } grpc_error* error = GRPC_ERROR_NONE; diff --git a/test/cpp/client/client_channel_stress_test.cc b/test/cpp/client/client_channel_stress_test.cc index 01343e5d634..c19be153bba 100644 --- a/test/cpp/client/client_channel_stress_test.cc +++ b/test/cpp/client/client_channel_stress_test.cc @@ -35,7 +35,6 @@ #include #include -#include "src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb_balancer_addresses.h" #include "src/core/ext/filters/client_channel/parse_address.h" #include "src/core/ext/filters/client_channel/resolver/fake/fake_resolver.h" #include "src/core/ext/filters/client_channel/server_address.h" @@ -152,7 +151,7 @@ class ClientChannelStressTest { for (const auto& balancer_server : balancer_servers_) { // Select each address with probability of 0.8. if (std::rand() % 10 < 8) { - addresses.emplace_back(AddressData{balancer_server.port_, ""}); + addresses.emplace_back(AddressData{balancer_server.port_, true, ""}); } } std::shuffle(addresses.begin(), addresses.end(), @@ -214,12 +213,13 @@ class ClientChannelStressTest { struct AddressData { int port; + bool is_balancer; grpc::string balancer_name; }; - static grpc_core::ServerAddressList CreateAddressListFromAddressDataList( - const std::vector& address_data) { - grpc_core::ServerAddressList addresses; + void SetNextResolution(const std::vector& address_data) { + grpc_core::ExecCtx exec_ctx; + grpc_core::Resolver::Result result; for (const auto& addr : address_data) { char* lb_uri_str; gpr_asprintf(&lb_uri_str, "ipv4:127.0.0.1:%d", addr.port); @@ -227,34 +227,20 @@ class ClientChannelStressTest { GPR_ASSERT(lb_uri != nullptr); grpc_resolved_address address; GPR_ASSERT(grpc_parse_uri(lb_uri, &address)); - grpc_arg arg = - grpc_core::CreateGrpclbBalancerNameArg(addr.balancer_name.c_str()); - grpc_channel_args* args = - grpc_channel_args_copy_and_add(nullptr, &arg, 1); - addresses.emplace_back(address.addr, address.len, args); + std::vector args_to_add; + if (addr.is_balancer) { + args_to_add.emplace_back(grpc_channel_arg_integer_create( + const_cast(GRPC_ARG_ADDRESS_IS_BALANCER), 1)); + args_to_add.emplace_back(grpc_channel_arg_string_create( + const_cast(GRPC_ARG_ADDRESS_BALANCER_NAME), + const_cast(addr.balancer_name.c_str()))); + } + grpc_channel_args* args = grpc_channel_args_copy_and_add( + nullptr, args_to_add.data(), args_to_add.size()); + result.addresses.emplace_back(address.addr, address.len, args); grpc_uri_destroy(lb_uri); gpr_free(lb_uri_str); } - return addresses; - } - - static grpc_core::Resolver::Result MakeResolverResult( - const std::vector& balancer_address_data) { - grpc_core::Resolver::Result result; - grpc_error* error = GRPC_ERROR_NONE; - result.service_config = grpc_core::ServiceConfig::Create( - "{\"loadBalancingConfig\":[{\"grpclb\":{}}]}", &error); - GPR_ASSERT(error == GRPC_ERROR_NONE); - grpc_core::ServerAddressList balancer_addresses = - CreateAddressListFromAddressDataList(balancer_address_data); - grpc_arg arg = CreateGrpclbBalancerAddressesArg(&balancer_addresses); - result.args = grpc_channel_args_copy_and_add(nullptr, &arg, 1); - return result; - } - - void SetNextResolution(const std::vector& address_data) { - grpc_core::ExecCtx exec_ctx; - grpc_core::Resolver::Result result = MakeResolverResult(address_data); response_generator_->SetResponse(std::move(result)); } diff --git a/test/cpp/end2end/grpclb_end2end_test.cc b/test/cpp/end2end/grpclb_end2end_test.cc index f4674e1ba68..6ebcd6f62bd 100644 --- a/test/cpp/end2end/grpclb_end2end_test.cc +++ b/test/cpp/end2end/grpclb_end2end_test.cc @@ -35,7 +35,6 @@ #include #include "src/core/ext/filters/client_channel/backup_poller.h" -#include "src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb_balancer_addresses.h" #include "src/core/ext/filters/client_channel/parse_address.h" #include "src/core/ext/filters/client_channel/resolver/fake/fake_resolver.h" #include "src/core/ext/filters/client_channel/server_address.h" @@ -84,13 +83,6 @@ namespace grpc { namespace testing { namespace { -constexpr char kDefaultServiceConfig[] = - "{\n" - " \"loadBalancingConfig\":[\n" - " { \"grpclb\":{} }\n" - " ]\n" - "}"; - template class CountedService : public ServiceType { public: @@ -522,10 +514,11 @@ class GrpclbEnd2endTest : public ::testing::Test { struct AddressData { int port; + bool is_balancer; grpc::string balancer_name; }; - static grpc_core::ServerAddressList CreateLbAddressesFromAddressDataList( + grpc_core::ServerAddressList CreateLbAddressesFromAddressDataList( const std::vector& address_data) { grpc_core::ServerAddressList addresses; for (const auto& addr : address_data) { @@ -535,10 +528,16 @@ class GrpclbEnd2endTest : public ::testing::Test { GPR_ASSERT(lb_uri != nullptr); grpc_resolved_address address; GPR_ASSERT(grpc_parse_uri(lb_uri, &address)); - grpc_arg arg = - grpc_core::CreateGrpclbBalancerNameArg(addr.balancer_name.c_str()); - grpc_channel_args* args = - grpc_channel_args_copy_and_add(nullptr, &arg, 1); + std::vector args_to_add; + if (addr.is_balancer) { + args_to_add.emplace_back(grpc_channel_arg_integer_create( + const_cast(GRPC_ARG_ADDRESS_IS_BALANCER), 1)); + args_to_add.emplace_back(grpc_channel_arg_string_create( + const_cast(GRPC_ARG_ADDRESS_BALANCER_NAME), + const_cast(addr.balancer_name.c_str()))); + } + grpc_channel_args* args = grpc_channel_args_copy_and_add( + nullptr, args_to_add.data(), args_to_add.size()); addresses.emplace_back(address.addr, address.len, args); grpc_uri_destroy(lb_uri); gpr_free(lb_uri_str); @@ -546,50 +545,34 @@ class GrpclbEnd2endTest : public ::testing::Test { return addresses; } - static grpc_core::Resolver::Result MakeResolverResult( - const std::vector& balancer_address_data, - const std::vector& backend_address_data = {}, - const char* service_config_json = kDefaultServiceConfig) { - grpc_core::Resolver::Result result; - result.addresses = - CreateLbAddressesFromAddressDataList(backend_address_data); - grpc_error* error = GRPC_ERROR_NONE; - result.service_config = - grpc_core::ServiceConfig::Create(service_config_json, &error); - GPR_ASSERT(error == GRPC_ERROR_NONE); - grpc_core::ServerAddressList balancer_addresses = - CreateLbAddressesFromAddressDataList(balancer_address_data); - grpc_arg arg = CreateGrpclbBalancerAddressesArg(&balancer_addresses); - result.args = grpc_channel_args_copy_and_add(nullptr, &arg, 1); - return result; - } - void SetNextResolutionAllBalancers( - const char* service_config_json = kDefaultServiceConfig) { + const char* service_config_json = nullptr) { std::vector addresses; for (size_t i = 0; i < balancers_.size(); ++i) { - addresses.emplace_back(AddressData{balancers_[i]->port_, ""}); + addresses.emplace_back(AddressData{balancers_[i]->port_, true, ""}); } - SetNextResolution(addresses, {}, service_config_json); + SetNextResolution(addresses, service_config_json); } - void SetNextResolution( - const std::vector& balancer_address_data, - const std::vector& backend_address_data = {}, - const char* service_config_json = kDefaultServiceConfig) { + void SetNextResolution(const std::vector& address_data, + const char* service_config_json = nullptr) { grpc_core::ExecCtx exec_ctx; - grpc_core::Resolver::Result result = MakeResolverResult( - balancer_address_data, backend_address_data, service_config_json); + grpc_core::Resolver::Result result; + result.addresses = CreateLbAddressesFromAddressDataList(address_data); + if (service_config_json != nullptr) { + grpc_error* error = GRPC_ERROR_NONE; + result.service_config = + grpc_core::ServiceConfig::Create(service_config_json, &error); + GRPC_ERROR_UNREF(error); + } response_generator_->SetResponse(std::move(result)); } void SetNextReresolutionResponse( - const std::vector& balancer_address_data, - const std::vector& backend_address_data = {}, - const char* service_config_json = kDefaultServiceConfig) { + const std::vector& address_data) { grpc_core::ExecCtx exec_ctx; - grpc_core::Resolver::Result result = MakeResolverResult( - balancer_address_data, backend_address_data, service_config_json); + grpc_core::Resolver::Result result; + result.addresses = CreateLbAddressesFromAddressDataList(address_data); response_generator_->SetReresolutionResponse(std::move(result)); } @@ -764,11 +747,44 @@ TEST_F(SingleBalancerTest, SelectGrpclbWithMigrationServiceConfig) { EXPECT_EQ("grpclb", channel_->GetLoadBalancingPolicyName()); } +TEST_F(SingleBalancerTest, + DoNotSpecialCaseUseGrpclbWithLoadBalancingConfigTest) { + const int kFallbackTimeoutMs = 200 * grpc_test_slowdown_factor(); + ResetStub(kFallbackTimeoutMs); + SetNextResolution({AddressData{backends_[0]->port_, false, ""}, + AddressData{balancers_[0]->port_, true, ""}}, + "{\n" + " \"loadBalancingConfig\":[\n" + " {\"pick_first\":{} }\n" + " ]\n" + "}"); + CheckRpcSendOk(); + // Check LB policy name for the channel. + EXPECT_EQ("pick_first", channel_->GetLoadBalancingPolicyName()); +} + +TEST_F( + SingleBalancerTest, + DoNotSpecialCaseUseGrpclbWithLoadBalancingConfigTestAndNoBackendAddress) { + const int kFallbackTimeoutMs = 200 * grpc_test_slowdown_factor(); + ResetStub(kFallbackTimeoutMs); + SetNextResolution({AddressData{balancers_[0]->port_, true, ""}}, + "{\n" + " \"loadBalancingConfig\":[\n" + " {\"pick_first\":{} }\n" + " ]\n" + "}"); + // This should fail since we do not have a non-balancer backend + CheckRpcSendFailure(); + // Check LB policy name for the channel. + EXPECT_EQ("pick_first", channel_->GetLoadBalancingPolicyName()); +} + TEST_F(SingleBalancerTest, SelectGrpclbWithMigrationServiceConfigAndNoAddresses) { const int kFallbackTimeoutMs = 200 * grpc_test_slowdown_factor(); ResetStub(kFallbackTimeoutMs); - SetNextResolution({}, {}, + SetNextResolution({}, "{\n" " \"loadBalancingConfig\":[\n" " { \"does_not_exist\":{} },\n" @@ -788,6 +804,23 @@ TEST_F(SingleBalancerTest, EXPECT_EQ("grpclb", channel_->GetLoadBalancingPolicyName()); } +TEST_F(SingleBalancerTest, + SelectGrpclbWithMigrationServiceConfigAndNoBalancerAddresses) { + const int kFallbackTimeoutMs = 200 * grpc_test_slowdown_factor(); + ResetStub(kFallbackTimeoutMs); + // Resolution includes fallback address but no balancers. + SetNextResolution({AddressData{backends_[0]->port_, false, ""}}, + "{\n" + " \"loadBalancingConfig\":[\n" + " { \"does_not_exist\":{} },\n" + " { \"grpclb\":{} }\n" + " ]\n" + "}"); + CheckRpcSendOk(1, 1000 /* timeout_ms */, true /* wait_for_ready */); + // Check LB policy name for the channel. + EXPECT_EQ("grpclb", channel_->GetLoadBalancingPolicyName()); +} + TEST_F(SingleBalancerTest, UsePickFirstChildPolicy) { SetNextResolutionAllBalancers( "{\n" @@ -842,7 +875,7 @@ TEST_F(SingleBalancerTest, SwapChildPolicy) { EXPECT_EQ(backends_[i]->service_.request_count(), 0UL); } // Send new resolution that removes child policy from service config. - SetNextResolutionAllBalancers(); + SetNextResolutionAllBalancers("{}"); WaitForAllBackends(); CheckRpcSendOk(kNumRpcs, 1000 /* timeout_ms */, true /* wait_for_ready */); // Check that every backend saw the same number of requests. This verifies @@ -870,11 +903,9 @@ TEST_F(SingleBalancerTest, UpdatesGoToMostRecentChildPolicy) { SetNextResolution( { // Unreachable balancer. - {unreachable_balancer_port, ""}, - }, - { + {unreachable_balancer_port, true, ""}, // Fallback address: first backend. - {backends_[0]->port_, ""}, + {backends_[0]->port_, false, ""}, }, "{\n" " \"loadBalancingConfig\":[\n" @@ -892,11 +923,9 @@ TEST_F(SingleBalancerTest, UpdatesGoToMostRecentChildPolicy) { SetNextResolution( { // Unreachable balancer. - {unreachable_balancer_port, ""}, - }, - { + {unreachable_balancer_port, true, ""}, // Fallback address: unreachable backend. - {unreachable_backend_port, ""}, + {unreachable_backend_port, false, ""}, }, "{\n" " \"loadBalancingConfig\":[\n" @@ -917,12 +946,10 @@ TEST_F(SingleBalancerTest, UpdatesGoToMostRecentChildPolicy) { SetNextResolution( { // Unreachable balancer. - {unreachable_balancer_port, ""}, - }, - { + {unreachable_balancer_port, true, ""}, // Fallback address: second and third backends. - {backends_[1]->port_, ""}, - {backends_[2]->port_, ""}, + {backends_[1]->port_, false, ""}, + {backends_[2]->port_, false, ""}, }, "{\n" " \"loadBalancingConfig\":[\n" @@ -961,7 +988,7 @@ TEST_F(SingleBalancerTest, SameBackendListedMultipleTimes) { TEST_F(SingleBalancerTest, SecureNaming) { ResetStub(0, kApplicationTargetName_ + ";lb"); - SetNextResolution({AddressData{balancers_[0]->port_, "lb"}}); + SetNextResolution({AddressData{balancers_[0]->port_, true, "lb"}}); const size_t kNumRpcsPerAddress = 100; ScheduleResponseForBalancer( 0, BalancerServiceImpl::BuildResponseForBackends(GetBackendPorts(), {}), @@ -993,7 +1020,7 @@ TEST_F(SingleBalancerTest, SecureNamingDeathTest) { ASSERT_DEATH_IF_SUPPORTED( { ResetStub(0, kApplicationTargetName_ + ";lb"); - SetNextResolution({AddressData{balancers_[0]->port_, "woops"}}); + SetNextResolution({AddressData{balancers_[0]->port_, true, "woops"}}); channel_->WaitForConnected(grpc_timeout_seconds_to_deadline(1)); }, ""); @@ -1053,13 +1080,12 @@ TEST_F(SingleBalancerTest, Fallback) { const size_t kNumBackendsInResolution = backends_.size() / 2; ResetStub(kFallbackTimeoutMs); - std::vector balancer_addresses; - balancer_addresses.emplace_back(AddressData{balancers_[0]->port_, ""}); - std::vector backend_addresses; + std::vector addresses; + addresses.emplace_back(AddressData{balancers_[0]->port_, true, ""}); for (size_t i = 0; i < kNumBackendsInResolution; ++i) { - backend_addresses.emplace_back(AddressData{backends_[i]->port_, ""}); + addresses.emplace_back(AddressData{backends_[i]->port_, false, ""}); } - SetNextResolution(balancer_addresses, backend_addresses); + SetNextResolution(addresses); // Send non-empty serverlist only after kServerlistDelayMs. ScheduleResponseForBalancer( @@ -1122,13 +1148,12 @@ TEST_F(SingleBalancerTest, FallbackUpdate) { const size_t kNumBackendsInResolutionUpdate = backends_.size() / 3; ResetStub(kFallbackTimeoutMs); - std::vector balancer_addresses; - balancer_addresses.emplace_back(AddressData{balancers_[0]->port_, ""}); - std::vector backend_addresses; + std::vector addresses; + addresses.emplace_back(AddressData{balancers_[0]->port_, true, ""}); for (size_t i = 0; i < kNumBackendsInResolution; ++i) { - backend_addresses.emplace_back(AddressData{backends_[i]->port_, ""}); + addresses.emplace_back(AddressData{backends_[i]->port_, false, ""}); } - SetNextResolution(balancer_addresses, backend_addresses); + SetNextResolution(addresses); // Send non-empty serverlist only after kServerlistDelayMs. ScheduleResponseForBalancer( @@ -1158,14 +1183,13 @@ TEST_F(SingleBalancerTest, FallbackUpdate) { EXPECT_EQ(0U, backends_[i]->service_.request_count()); } - balancer_addresses.clear(); - balancer_addresses.emplace_back(AddressData{balancers_[0]->port_, ""}); - backend_addresses.clear(); + addresses.clear(); + addresses.emplace_back(AddressData{balancers_[0]->port_, true, ""}); for (size_t i = kNumBackendsInResolution; i < kNumBackendsInResolution + kNumBackendsInResolutionUpdate; ++i) { - backend_addresses.emplace_back(AddressData{backends_[i]->port_, ""}); + addresses.emplace_back(AddressData{backends_[i]->port_, false, ""}); } - SetNextResolution(balancer_addresses, backend_addresses); + SetNextResolution(addresses); // Wait until the resolution update has been processed and all the new // fallback backends are reachable. @@ -1229,15 +1253,14 @@ TEST_F(SingleBalancerTest, // First two backends are fallback, last two are pointed to by balancer. const size_t kNumFallbackBackends = 2; const size_t kNumBalancerBackends = backends_.size() - kNumFallbackBackends; - std::vector backend_addresses; + std::vector addresses; for (size_t i = 0; i < kNumFallbackBackends; ++i) { - backend_addresses.emplace_back(AddressData{backends_[i]->port_, ""}); + addresses.emplace_back(AddressData{backends_[i]->port_, false, ""}); } - std::vector balancer_addresses; for (size_t i = 0; i < balancers_.size(); ++i) { - balancer_addresses.emplace_back(AddressData{balancers_[i]->port_, ""}); + addresses.emplace_back(AddressData{balancers_[i]->port_, true, ""}); } - SetNextResolution(balancer_addresses, backend_addresses); + SetNextResolution(addresses); ScheduleResponseForBalancer(0, BalancerServiceImpl::BuildResponseForBackends( GetBackendPorts(kNumFallbackBackends), {}), @@ -1284,15 +1307,14 @@ TEST_F(SingleBalancerTest, // First two backends are fallback, last two are pointed to by balancer. const size_t kNumFallbackBackends = 2; const size_t kNumBalancerBackends = backends_.size() - kNumFallbackBackends; - std::vector backend_addresses; + std::vector addresses; for (size_t i = 0; i < kNumFallbackBackends; ++i) { - backend_addresses.emplace_back(AddressData{backends_[i]->port_, ""}); + addresses.emplace_back(AddressData{backends_[i]->port_, false, ""}); } - std::vector balancer_addresses; for (size_t i = 0; i < balancers_.size(); ++i) { - balancer_addresses.emplace_back(AddressData{balancers_[i]->port_, ""}); + addresses.emplace_back(AddressData{balancers_[i]->port_, true, ""}); } - SetNextResolution(balancer_addresses, backend_addresses); + SetNextResolution(addresses); ScheduleResponseForBalancer(0, BalancerServiceImpl::BuildResponseForBackends( GetBackendPorts(kNumFallbackBackends), {}), @@ -1336,12 +1358,10 @@ TEST_F(SingleBalancerTest, FallbackEarlyWhenBalancerChannelFails) { const int kFallbackTimeoutMs = 10000 * grpc_test_slowdown_factor(); ResetStub(kFallbackTimeoutMs); // Return an unreachable balancer and one fallback backend. - std::vector balancer_addresses; - balancer_addresses.emplace_back( - AddressData{grpc_pick_unused_port_or_die(), ""}); - std::vector backend_addresses; - backend_addresses.emplace_back(AddressData{backends_[0]->port_, ""}); - SetNextResolution(balancer_addresses, backend_addresses); + std::vector addresses; + addresses.emplace_back(AddressData{grpc_pick_unused_port_or_die(), true, ""}); + addresses.emplace_back(AddressData{backends_[0]->port_, false, ""}); + SetNextResolution(addresses); // Send RPC with deadline less than the fallback timeout and make sure it // succeeds. CheckRpcSendOk(/* times */ 1, /* timeout_ms */ 1000, @@ -1352,11 +1372,10 @@ TEST_F(SingleBalancerTest, FallbackEarlyWhenBalancerCallFails) { const int kFallbackTimeoutMs = 10000 * grpc_test_slowdown_factor(); ResetStub(kFallbackTimeoutMs); // Return one balancer and one fallback backend. - std::vector balancer_addresses; - balancer_addresses.emplace_back(AddressData{balancers_[0]->port_, ""}); - std::vector backend_addresses; - backend_addresses.emplace_back(AddressData{backends_[0]->port_, ""}); - SetNextResolution(balancer_addresses, backend_addresses); + std::vector addresses; + addresses.emplace_back(AddressData{balancers_[0]->port_, true, ""}); + addresses.emplace_back(AddressData{backends_[0]->port_, false, ""}); + SetNextResolution(addresses); // Balancer drops call without sending a serverlist. balancers_[0]->service_.NotifyDoneWithServerlists(); // Send RPC with deadline less than the fallback timeout and make sure it @@ -1369,11 +1388,10 @@ TEST_F(SingleBalancerTest, FallbackControlledByBalancer_BeforeFirstServerlist) { const int kFallbackTimeoutMs = 10000 * grpc_test_slowdown_factor(); ResetStub(kFallbackTimeoutMs); // Return one balancer and one fallback backend. - std::vector balancer_addresses; - balancer_addresses.emplace_back(AddressData{balancers_[0]->port_, ""}); - std::vector backend_addresses; - backend_addresses.emplace_back(AddressData{backends_[0]->port_, ""}); - SetNextResolution(balancer_addresses, backend_addresses); + std::vector addresses; + addresses.emplace_back(AddressData{balancers_[0]->port_, true, ""}); + addresses.emplace_back(AddressData{backends_[0]->port_, false, ""}); + SetNextResolution(addresses); // Balancer explicitly tells client to fallback. LoadBalanceResponse resp; resp.mutable_fallback_response(); @@ -1386,11 +1404,10 @@ TEST_F(SingleBalancerTest, FallbackControlledByBalancer_BeforeFirstServerlist) { TEST_F(SingleBalancerTest, FallbackControlledByBalancer_AfterFirstServerlist) { // Return one balancer and one fallback backend (backend 0). - std::vector balancer_addresses; - balancer_addresses.emplace_back(AddressData{balancers_[0]->port_, ""}); - std::vector backend_addresses; - backend_addresses.emplace_back(AddressData{backends_[0]->port_, ""}); - SetNextResolution(balancer_addresses, backend_addresses); + std::vector addresses; + addresses.emplace_back(AddressData{balancers_[0]->port_, true, ""}); + addresses.emplace_back(AddressData{backends_[0]->port_, false, ""}); + SetNextResolution(addresses); // Balancer initially sends serverlist, then tells client to fall back, // then sends the serverlist again. // The serverlist points to backend 1. @@ -1466,7 +1483,7 @@ TEST_F(UpdatesTest, UpdateBalancersButKeepUsingOriginalBalancer) { EXPECT_EQ(0U, balancers_[2]->service_.response_count()); std::vector addresses; - addresses.emplace_back(AddressData{balancers_[1]->port_, ""}); + addresses.emplace_back(AddressData{balancers_[1]->port_, true, ""}); gpr_log(GPR_INFO, "========= ABOUT TO UPDATE 1 =========="); SetNextResolution(addresses); gpr_log(GPR_INFO, "========= UPDATE 1 DONE =========="); @@ -1525,9 +1542,9 @@ TEST_F(UpdatesTest, UpdateBalancersRepeated) { EXPECT_EQ(0U, balancers_[2]->service_.response_count()); std::vector addresses; - addresses.emplace_back(AddressData{balancers_[0]->port_, ""}); - addresses.emplace_back(AddressData{balancers_[1]->port_, ""}); - addresses.emplace_back(AddressData{balancers_[2]->port_, ""}); + addresses.emplace_back(AddressData{balancers_[0]->port_, true, ""}); + addresses.emplace_back(AddressData{balancers_[1]->port_, true, ""}); + addresses.emplace_back(AddressData{balancers_[2]->port_, true, ""}); gpr_log(GPR_INFO, "========= ABOUT TO UPDATE 1 =========="); SetNextResolution(addresses); gpr_log(GPR_INFO, "========= UPDATE 1 DONE =========="); @@ -1545,8 +1562,8 @@ TEST_F(UpdatesTest, UpdateBalancersRepeated) { balancers_[0]->service_.NotifyDoneWithServerlists(); addresses.clear(); - addresses.emplace_back(AddressData{balancers_[0]->port_, ""}); - addresses.emplace_back(AddressData{balancers_[1]->port_, ""}); + addresses.emplace_back(AddressData{balancers_[0]->port_, true, ""}); + addresses.emplace_back(AddressData{balancers_[1]->port_, true, ""}); gpr_log(GPR_INFO, "========= ABOUT TO UPDATE 2 =========="); SetNextResolution(addresses); gpr_log(GPR_INFO, "========= UPDATE 2 DONE =========="); @@ -1566,7 +1583,7 @@ TEST_F(UpdatesTest, UpdateBalancersRepeated) { TEST_F(UpdatesTest, UpdateBalancersDeadUpdate) { std::vector addresses; - addresses.emplace_back(AddressData{balancers_[0]->port_, ""}); + addresses.emplace_back(AddressData{balancers_[0]->port_, true, ""}); SetNextResolution(addresses); const std::vector first_backend{GetBackendPorts()[0]}; const std::vector second_backend{GetBackendPorts()[1]}; @@ -1606,7 +1623,7 @@ TEST_F(UpdatesTest, UpdateBalancersDeadUpdate) { EXPECT_EQ(0U, balancers_[2]->service_.response_count()); addresses.clear(); - addresses.emplace_back(AddressData{balancers_[1]->port_, ""}); + addresses.emplace_back(AddressData{balancers_[1]->port_, true, ""}); gpr_log(GPR_INFO, "========= ABOUT TO UPDATE 1 =========="); SetNextResolution(addresses); gpr_log(GPR_INFO, "========= UPDATE 1 DONE =========="); @@ -1643,20 +1660,18 @@ TEST_F(UpdatesTest, ReresolveDeadBackend) { ResetStub(500); // The first resolution contains the addresses of a balancer that never // responds, and a fallback backend. - std::vector balancer_addresses; - balancer_addresses.emplace_back(AddressData{balancers_[0]->port_, ""}); - std::vector backend_addresses; - backend_addresses.emplace_back(AddressData{backends_[0]->port_, ""}); - SetNextResolution(balancer_addresses, backend_addresses); + std::vector addresses; + addresses.emplace_back(AddressData{balancers_[0]->port_, true, ""}); + addresses.emplace_back(AddressData{backends_[0]->port_, false, ""}); + SetNextResolution(addresses); // Ask channel to connect to trigger resolver creation. channel_->GetState(true); // The re-resolution result will contain the addresses of the same balancer // and a new fallback backend. - balancer_addresses.clear(); - balancer_addresses.emplace_back(AddressData{balancers_[0]->port_, ""}); - backend_addresses.clear(); - backend_addresses.emplace_back(AddressData{backends_[1]->port_, ""}); - SetNextReresolutionResponse(balancer_addresses, backend_addresses); + addresses.clear(); + addresses.emplace_back(AddressData{balancers_[0]->port_, true, ""}); + addresses.emplace_back(AddressData{backends_[1]->port_, false, ""}); + SetNextReresolutionResponse(addresses); // Start servers and send 10 RPCs per server. gpr_log(GPR_INFO, "========= BEFORE FIRST BATCH =========="); @@ -1705,10 +1720,10 @@ TEST_F(UpdatesWithClientLoadReportingTest, ReresolveDeadBalancer) { // Ask channel to connect to trigger resolver creation. channel_->GetState(true); std::vector addresses; - addresses.emplace_back(AddressData{balancers_[0]->port_, ""}); + addresses.emplace_back(AddressData{balancers_[0]->port_, true, ""}); SetNextResolution(addresses); addresses.clear(); - addresses.emplace_back(AddressData{balancers_[1]->port_, ""}); + addresses.emplace_back(AddressData{balancers_[1]->port_, true, ""}); SetNextReresolutionResponse(addresses); const std::vector first_backend{GetBackendPorts()[0]}; const std::vector second_backend{GetBackendPorts()[1]}; diff --git a/test/cpp/naming/resolver_component_test.cc b/test/cpp/naming/resolver_component_test.cc index 5c72534b5a2..e74f5666b81 100644 --- a/test/cpp/naming/resolver_component_test.cc +++ b/test/cpp/naming/resolver_component_test.cc @@ -39,7 +39,6 @@ #include "test/cpp/util/test_config.h" #include "src/core/ext/filters/client_channel/client_channel.h" -#include "src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb_balancer_addresses.h" #include "src/core/ext/filters/client_channel/parse_address.h" #include "src/core/ext/filters/client_channel/resolver.h" #include "src/core/ext/filters/client_channel/resolver/dns/c_ares/grpc_ares_ev_driver.h" @@ -464,20 +463,19 @@ class CheckingResultHandler : public ResultHandler { void CheckResult(const grpc_core::Resolver::Result& result) override { ArgsStruct* args = args_struct(); + gpr_log(GPR_INFO, "num addrs found: %" PRIdPTR ". expected %" PRIdPTR, + result.addresses.size(), args->expected_addrs.size()); + GPR_ASSERT(result.addresses.size() == args->expected_addrs.size()); std::vector found_lb_addrs; - AddActualAddresses(result.addresses, /*is_balancer=*/false, - &found_lb_addrs); - const grpc_core::ServerAddressList* balancer_addresses = - grpc_core::FindGrpclbBalancerAddressesInChannelArgs(*result.args); - if (balancer_addresses != nullptr) { - AddActualAddresses(*balancer_addresses, /*is_balancer=*/true, - &found_lb_addrs); + for (size_t i = 0; i < result.addresses.size(); i++) { + const grpc_core::ServerAddress& addr = result.addresses[i]; + char* str; + grpc_sockaddr_to_string(&str, &addr.address(), 1 /* normalize */); + gpr_log(GPR_INFO, "%s", str); + found_lb_addrs.emplace_back( + GrpcLBAddress(std::string(str), addr.IsBalancer())); + gpr_free(str); } - gpr_log(GPR_INFO, - "found %" PRIdPTR " backend addresses and %" PRIdPTR - " balancer addresses", - result.addresses.size(), - balancer_addresses == nullptr ? 0L : balancer_addresses->size()); if (args->expected_addrs.size() != found_lb_addrs.size()) { gpr_log(GPR_DEBUG, "found lb addrs size is: %" PRIdPTR @@ -497,20 +495,6 @@ class CheckingResultHandler : public ResultHandler { CheckLBPolicyResultLocked(result.args, args); } } - - private: - static void AddActualAddresses(const grpc_core::ServerAddressList& addresses, - bool is_balancer, - std::vector* out) { - for (size_t i = 0; i < addresses.size(); i++) { - const grpc_core::ServerAddress& addr = addresses[i]; - char* str; - grpc_sockaddr_to_string(&str, &addr.address(), 1 /* normalize */); - gpr_log(GPR_INFO, "%s", str); - out->emplace_back(GrpcLBAddress(std::string(str), is_balancer)); - gpr_free(str); - } - } }; int g_fake_non_responsive_dns_server_port = -1; diff --git a/tools/distrib/check_include_guards.py b/tools/distrib/check_include_guards.py index 2983441176d..bc774227fff 100755 --- a/tools/distrib/check_include_guards.py +++ b/tools/distrib/check_include_guards.py @@ -44,7 +44,7 @@ class GuardValidator(object): self.ifndef_re = re.compile(r'#ifndef ([A-Z][A-Z_1-9]*)') self.define_re = re.compile(r'#define ([A-Z][A-Z_1-9]*)') self.endif_c_re = re.compile( - r'#endif /\* (?: *\\\n *)?([A-Z][A-Z_1-9]*) (?:\\\n *)?\*/$') + r'#endif /\* ([A-Z][A-Z_1-9]*) (?:\\ *\n *)?\*/') self.endif_cpp_re = re.compile(r'#endif // ([A-Z][A-Z_1-9]*)') self.failed = False @@ -122,7 +122,7 @@ class GuardValidator(object): # Is there a properly commented #endif? endif_re = self.endif_cpp_re if cpp_header else self.endif_c_re flines = fcontents.rstrip().splitlines() - match = endif_re.search('\n'.join(flines[-3:])) + match = endif_re.search('\n'.join(flines[-2:])) if not match: # No endif. Check if we have the last line as just '#endif' and if so # replace it with a properly commented one. diff --git a/tools/doxygen/Doxyfile.core.internal b/tools/doxygen/Doxyfile.core.internal index af674ec328f..174dfe42442 100644 --- a/tools/doxygen/Doxyfile.core.internal +++ b/tools/doxygen/Doxyfile.core.internal @@ -903,8 +903,6 @@ src/core/ext/filters/client_channel/lb_policy/grpclb/client_load_reporting_filte src/core/ext/filters/client_channel/lb_policy/grpclb/client_load_reporting_filter.h \ src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb.cc \ src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb.h \ -src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb_balancer_addresses.cc \ -src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb_balancer_addresses.h \ src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb_channel.h \ src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb_channel_secure.cc \ src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb_client_stats.cc \