From a3760bc53ec7a9a422c4e771f5b5b049d5fb5fa3 Mon Sep 17 00:00:00 2001 From: Eugene Ostroukhov Date: Tue, 3 Sep 2024 16:03:15 -0700 Subject: [PATCH 1/2] [xds] Remove the GRPC_EXPERIMENTAL_XDS_FALLBACK env var (#37620) Fallback interop test is fully deployed. This variable is no longer needed. Closes #37620 COPYBARA_INTEGRATE_REVIEW=https://github.com/grpc/grpc/pull/37620 from eugeneo:no-fallback-var c21509d0a5af14ee31dfa38f650e0c38facac4c9 PiperOrigin-RevId: 670738146 --- build_autogenerated.yaml | 3 +- src/core/xds/grpc/xds_bootstrap_grpc.cc | 32 --------------- src/core/xds/grpc/xds_bootstrap_grpc.h | 5 --- test/core/xds/BUILD | 1 - test/core/xds/xds_bootstrap_test.cc | 24 +---------- .../end2end/xds/xds_fallback_end2end_test.cc | 40 ------------------- 6 files changed, 2 insertions(+), 103 deletions(-) diff --git a/build_autogenerated.yaml b/build_autogenerated.yaml index 7fbd125fe1db5..8f5100705fbfb 100644 --- a/build_autogenerated.yaml +++ b/build_autogenerated.yaml @@ -21269,8 +21269,7 @@ targets: gtest: true build: test language: c++ - headers: - - test/core/test_util/scoped_env_var.h + headers: [] src: - test/core/xds/xds_bootstrap_test.cc deps: diff --git a/src/core/xds/grpc/xds_bootstrap_grpc.cc b/src/core/xds/grpc/xds_bootstrap_grpc.cc index 58b958cb9949c..87b3541ced61b 100644 --- a/src/core/xds/grpc/xds_bootstrap_grpc.cc +++ b/src/core/xds/grpc/xds_bootstrap_grpc.cc @@ -18,8 +18,6 @@ #include -#include -#include #include #include #include @@ -36,29 +34,14 @@ #include #include -#include "src/core/lib/config/core_configuration.h" -#include "src/core/lib/gprpp/env.h" #include "src/core/lib/gprpp/ref_counted_ptr.h" -#include "src/core/lib/security/credentials/channel_creds_registry.h" #include "src/core/util/json/json.h" #include "src/core/util/json/json_object_loader.h" #include "src/core/util/json/json_reader.h" #include "src/core/util/json/json_writer.h" -#include "src/core/util/string.h" namespace grpc_core { -namespace { -bool IsFallbackExperimentEnabled() { - auto fallback_enabled = GetEnv("GRPC_EXPERIMENTAL_XDS_FALLBACK"); - bool enabled = false; - return gpr_parse_bool_value(fallback_enabled.value_or("0").c_str(), - &enabled) && - enabled; -} - -} // namespace - // // GrpcXdsBootstrap::GrpcNode::Locality // @@ -106,16 +89,6 @@ const JsonLoaderInterface* GrpcXdsBootstrap::GrpcAuthority::JsonLoader( return loader; } -void GrpcXdsBootstrap::GrpcAuthority::JsonPostLoad( - const Json& /*json*/, const JsonArgs& /*args*/, - ValidationErrors* /*errors*/) { - if (!IsFallbackExperimentEnabled()) { - if (servers_.size() > 1) { - servers_.resize(1); - } - } -} - // // GrpcXdsBootstrap // @@ -190,11 +163,6 @@ void GrpcXdsBootstrap::JsonPostLoad(const Json& /*json*/, } } } - if (!IsFallbackExperimentEnabled()) { - if (servers_.size() > 1) { - servers_.resize(1); - } - } } std::string GrpcXdsBootstrap::ToString() const { diff --git a/src/core/xds/grpc/xds_bootstrap_grpc.h b/src/core/xds/grpc/xds_bootstrap_grpc.h index a21f0017aa2e4..24c47e2eccbea 100644 --- a/src/core/xds/grpc/xds_bootstrap_grpc.h +++ b/src/core/xds/grpc/xds_bootstrap_grpc.h @@ -19,7 +19,6 @@ #include #include -#include #include #include @@ -29,9 +28,7 @@ #include -#include "src/core/lib/gprpp/ref_counted_ptr.h" #include "src/core/lib/gprpp/validation_errors.h" -#include "src/core/lib/security/credentials/channel_creds_registry.h" #include "src/core/util/json/json.h" #include "src/core/util/json/json_args.h" #include "src/core/util/json/json_object_loader.h" @@ -93,8 +90,6 @@ class GrpcXdsBootstrap final : public XdsBootstrap { } static const JsonLoaderInterface* JsonLoader(const JsonArgs&); - void JsonPostLoad(const Json& json, const JsonArgs& args, - ValidationErrors* errors); private: std::vector servers_; diff --git a/test/core/xds/BUILD b/test/core/xds/BUILD index 85652f14e99c5..f152a44953f59 100644 --- a/test/core/xds/BUILD +++ b/test/core/xds/BUILD @@ -38,7 +38,6 @@ grpc_cc_test( "//src/core:grpc_xds_client", "//src/core:xds_server_grpc", "//test/core/test_util:grpc_test_util", - "//test/core/test_util:scoped_env_var", ], ) diff --git a/test/core/xds/xds_bootstrap_test.cc b/test/core/xds/xds_bootstrap_test.cc index ee218ad523e5f..1a30de17492d5 100644 --- a/test/core/xds/xds_bootstrap_test.cc +++ b/test/core/xds/xds_bootstrap_test.cc @@ -41,7 +41,6 @@ #include "src/core/lib/gprpp/validation_errors.h" #include "src/core/lib/security/certificate_provider/certificate_provider_factory.h" #include "src/core/lib/security/credentials/channel_creds_registry.h" -#include "src/core/lib/security/credentials/tls/grpc_tls_certificate_provider.h" #include "src/core/util/json/json.h" #include "src/core/util/json/json_args.h" #include "src/core/util/json/json_object_loader.h" @@ -50,7 +49,6 @@ #include "src/core/xds/grpc/certificate_provider_store.h" #include "src/core/xds/grpc/xds_bootstrap_grpc.h" #include "src/core/xds/grpc/xds_server_grpc.h" -#include "test/core/test_util/scoped_env_var.h" #include "test/core/test_util/test_config.h" namespace grpc_core { @@ -94,16 +92,6 @@ TEST(XdsBootstrapTest, Basic) { " }" " ]," " \"ignore\": 0" - " }," - " {" - " \"server_uri\": \"fake:///lb2\"," - " \"channel_creds\": [" - " {" - " \"type\": \"fake\"," - " \"ignore\": 0" - " }" - " ]," - " \"ignore\": 0" " }" " ]," " \"authorities\": {" @@ -123,15 +111,6 @@ TEST(XdsBootstrapTest, Basic) { " \"xds_v3\"," " \"ignore_resource_deletion\"" " ]" - " }," - " {" - " \"server_uri\": \"fake:///xds_server2\"," - " \"channel_creds\": [" - " {" - " \"type\": \"fake\"" - " }" - " ]," - " \"server_features\": [\"xds_v3\"]" " }" " ]" " }," @@ -744,8 +723,7 @@ TEST(XdsBootstrapTest, XdsServerToJsonAndParse) { EXPECT_EQ(*xds_server, *output_xds_server); } -TEST(XdsBootstrapTest, NoXdsServersEnvVar) { - ScopedEnvVar fallback_enabled("GRPC_EXPERIMENTAL_XDS_FALLBACK", "1"); +TEST(XdsBootstrapTest, MultipleXdsServers) { const char* json_str = "{" " \"xds_servers\": [" diff --git a/test/cpp/end2end/xds/xds_fallback_end2end_test.cc b/test/cpp/end2end/xds/xds_fallback_end2end_test.cc index abe68ba641d27..49e22de0e0b5b 100644 --- a/test/cpp/end2end/xds/xds_fallback_end2end_test.cc +++ b/test/cpp/end2end/xds/xds_fallback_end2end_test.cc @@ -12,7 +12,6 @@ // See the License for the specific language governing permissions and // limitations under the License. // -#include #include #include #include @@ -29,18 +28,9 @@ #include #include -#include "src/core/client_channel/backup_poller.h" #include "src/core/lib/config/config_vars.h" -#include "src/core/lib/gprpp/env.h" -#include "src/cpp/client/secure_credentials.h" #include "src/proto/grpc/testing/echo.grpc.pb.h" #include "src/proto/grpc/testing/echo_messages.pb.h" -#include "src/proto/grpc/testing/xds/v3/cluster.grpc.pb.h" -#include "src/proto/grpc/testing/xds/v3/endpoint.grpc.pb.h" -#include "src/proto/grpc/testing/xds/v3/http_connection_manager.grpc.pb.h" -#include "src/proto/grpc/testing/xds/v3/listener.grpc.pb.h" -#include "src/proto/grpc/testing/xds/v3/route.grpc.pb.h" -#include "test/core/test_util/resolve_localhost_ip46.h" #include "test/core/test_util/scoped_env_var.h" #include "test/core/test_util/test_config.h" #include "test/cpp/end2end/xds/xds_end2end_test_lib.h" @@ -123,8 +113,6 @@ class XdsFallbackTest : public XdsEnd2endTest { }; TEST_P(XdsFallbackTest, FallbackAndRecover) { - grpc_core::testing::ScopedEnvVar fallback_enabled( - "GRPC_EXPERIMENTAL_XDS_FALLBACK", "1"); auto broken_balancer = CreateAndStartBalancer("Broken balancer"); broken_balancer->ads_service()->ForceADSFailure( Status(StatusCode::RESOURCE_EXHAUSTED, kErrorMessage)); @@ -151,31 +139,7 @@ TEST_P(XdsFallbackTest, FallbackAndRecover) { broken_balancer->Shutdown(); } -TEST_P(XdsFallbackTest, EnvVarNotSet) { - InitClient(XdsBootstrapBuilder().SetServers({ - balancer_->target(), - fallback_balancer_->target(), - })); - // Primary xDS server has backends_[0] configured and fallback server has - // backends_[1] - CreateAndStartBackends(2); - SetXdsResourcesForServer(balancer_.get(), 0); - SetXdsResourcesForServer(fallback_balancer_.get(), 1); - balancer_->ads_service()->ForceADSFailure( - Status(StatusCode::RESOURCE_EXHAUSTED, kErrorMessage)); - // Primary server down, failure should be reported - CheckRpcSendFailure( - DEBUG_LOCATION, StatusCode::UNAVAILABLE, - absl::StrFormat("server.example.com: UNAVAILABLE: xDS channel for server " - "localhost:%d: xDS call failed with no responses " - "received; status: RESOURCE_EXHAUSTED: test forced ADS " - "stream failure \\(node ID:xds_end2end_test\\)", - balancer_->port())); -} - TEST_P(XdsFallbackTest, PrimarySecondaryNotAvailable) { - grpc_core::testing::ScopedEnvVar fallback_enabled( - "GRPC_EXPERIMENTAL_XDS_FALLBACK", "1"); InitClient(XdsBootstrapBuilder().SetServers( {balancer_->target(), fallback_balancer_->target()})); balancer_->ads_service()->ForceADSFailure( @@ -194,8 +158,6 @@ TEST_P(XdsFallbackTest, PrimarySecondaryNotAvailable) { TEST_P(XdsFallbackTest, UsesCachedResourcesAfterFailure) { constexpr absl::string_view kServerName2 = "server2.example.com"; - grpc_core::testing::ScopedEnvVar fallback_enabled( - "GRPC_EXPERIMENTAL_XDS_FALLBACK", "1"); InitClient(XdsBootstrapBuilder().SetServers( {balancer_->target(), fallback_balancer_->target()})); // 4 backends - cross product of two data plane targets and two balancers @@ -223,8 +185,6 @@ TEST_P(XdsFallbackTest, PerAuthorityFallback) { // Use cleanup in case test assertion fails auto balancer2_cleanup = absl::MakeCleanup([&]() { fallback_balancer2->Shutdown(); }); - grpc_core::testing::ScopedEnvVar fallback_enabled( - "GRPC_EXPERIMENTAL_XDS_FALLBACK", "1"); grpc_core::testing::ScopedExperimentalEnvVar env_var( "GRPC_EXPERIMENTAL_XDS_FEDERATION"); const char* kAuthority1 = "xds1.example.com"; From 4a52db601e71d13e0d6f2beb295e3ff20e8d7d8a Mon Sep 17 00:00:00 2001 From: Tanvi Jagtap Date: Tue, 3 Sep 2024 22:59:45 -0700 Subject: [PATCH 2/2] [Gpr_To_Absl_Logging] Remove comment PiperOrigin-RevId: 670838500 --- src/core/lib/experiments/config.cc | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/core/lib/experiments/config.cc b/src/core/lib/experiments/config.cc index ac1cf8838603d..52483218793ac 100644 --- a/src/core/lib/experiments/config.cc +++ b/src/core/lib/experiments/config.cc @@ -220,9 +220,6 @@ bool IsTestExperimentEnabled(size_t experiment_id) { return (*g_test_experiments)[experiment_id]; } -// This is VLOG(2) for Open Source gRPC because of a lot of log noise -// complaints. However, internally we want LOG(INFO) so that it is easier for us -// to debug prod issues. #define GRPC_EXPERIMENT_LOG VLOG(2) void PrintExperimentsList() {