From 25085d3d9e6525b3a136fe053e4c40ff3c93ee09 Mon Sep 17 00:00:00 2001 From: "Mark D. Roth" Date: Wed, 5 Oct 2022 18:57:38 +0000 Subject: [PATCH 1/2] outlier detection: remove env var protection --- .../outlier_detection/outlier_detection.cc | 15 +-- .../outlier_detection/outlier_detection.h | 2 - .../lb_policy/xds/xds_cluster_resolver.cc | 43 ++---- src/core/ext/xds/xds_cluster.cc | 9 +- ...outlier_detection_lb_config_parser_test.cc | 6 - .../xds/xds_cluster_resource_type_test.cc | 23 +--- .../xds/xds_outlier_detection_end2end_test.cc | 125 ------------------ 7 files changed, 18 insertions(+), 205 deletions(-) diff --git a/src/core/ext/filters/client_channel/lb_policy/outlier_detection/outlier_detection.cc b/src/core/ext/filters/client_channel/lb_policy/outlier_detection/outlier_detection.cc index 31ba46f98877b..21b2354f29f63 100644 --- a/src/core/ext/filters/client_channel/lb_policy/outlier_detection/outlier_detection.cc +++ b/src/core/ext/filters/client_channel/lb_policy/outlier_detection/outlier_detection.cc @@ -71,15 +71,6 @@ namespace grpc_core { TraceFlag grpc_outlier_detection_lb_trace(false, "outlier_detection_lb"); -// TODO(donnadionne): Remove once outlier detection is no longer experimental -bool XdsOutlierDetectionEnabled() { - auto value = GetEnv("GRPC_EXPERIMENTAL_ENABLE_OUTLIER_DETECTION"); - if (!value.has_value()) return false; - bool parsed_value; - bool parse_succeeded = gpr_parse_bool_value(value->c_str(), &parsed_value); - return parse_succeeded && parsed_value; -} - namespace { constexpr absl::string_view kOutlierDetection = @@ -1150,10 +1141,8 @@ void OutlierDetectionConfig::JsonPostLoad(const Json& json, const JsonArgs&, // void RegisterOutlierDetectionLbPolicy(CoreConfiguration::Builder* builder) { - if (XdsOutlierDetectionEnabled()) { - builder->lb_policy_registry()->RegisterLoadBalancingPolicyFactory( - std::make_unique()); - } + builder->lb_policy_registry()->RegisterLoadBalancingPolicyFactory( + std::make_unique()); } } // namespace grpc_core diff --git a/src/core/ext/filters/client_channel/lb_policy/outlier_detection/outlier_detection.h b/src/core/ext/filters/client_channel/lb_policy/outlier_detection/outlier_detection.h index c078b7db6aa1a..40492697d7bf5 100644 --- a/src/core/ext/filters/client_channel/lb_policy/outlier_detection/outlier_detection.h +++ b/src/core/ext/filters/client_channel/lb_policy/outlier_detection/outlier_detection.h @@ -31,8 +31,6 @@ namespace grpc_core { -bool XdsOutlierDetectionEnabled(); - struct OutlierDetectionConfig { Duration interval = Duration::Seconds(10); Duration base_ejection_time = Duration::Milliseconds(30000); diff --git a/src/core/ext/filters/client_channel/lb_policy/xds/xds_cluster_resolver.cc b/src/core/ext/filters/client_channel/lb_policy/xds/xds_cluster_resolver.cc index 83009bd479280..b5547907ef0d5 100644 --- a/src/core/ext/filters/client_channel/lb_policy/xds/xds_cluster_resolver.cc +++ b/src/core/ext/filters/client_channel/lb_policy/xds/xds_cluster_resolver.cc @@ -951,27 +951,18 @@ XdsClusterResolverLb::CreateChildPolicyConfigLocked() { xds_cluster_impl_config["lrsLoadReportingServer"] = discovery_config.lrs_load_reporting_server->ToJson(); } - Json locality_picking_policy; - if (XdsOutlierDetectionEnabled()) { - Json::Object outlier_detection_config; - if (discovery_entry.config().outlier_detection_lb_config.has_value()) { - outlier_detection_config = - discovery_entry.config().outlier_detection_lb_config.value(); - } - outlier_detection_config["childPolicy"] = Json::Array{Json::Object{ - {"xds_cluster_impl_experimental", - std::move(xds_cluster_impl_config)}, - }}; - locality_picking_policy = Json::Array{Json::Object{ - {"outlier_detection_experimental", - std::move(outlier_detection_config)}, - }}; - } else { - locality_picking_policy = Json::Array{Json::Object{ - {"xds_cluster_impl_experimental", - std::move(xds_cluster_impl_config)}, - }}; + Json::Object outlier_detection_config; + if (discovery_entry.config().outlier_detection_lb_config.has_value()) { + outlier_detection_config = + discovery_entry.config().outlier_detection_lb_config.value(); } + outlier_detection_config["childPolicy"] = Json::Array{Json::Object{ + {"xds_cluster_impl_experimental", std::move(xds_cluster_impl_config)}, + }}; + Json locality_picking_policy = Json::Array{Json::Object{ + {"outlier_detection_experimental", + std::move(outlier_detection_config)}, + }}; // Add priority entry, with the appropriate child name. std::string child_name = discovery_entry.GetChildPolicyName(priority); priority_priorities.emplace_back(child_name); @@ -1089,8 +1080,7 @@ XdsClusterResolverLbConfig::DiscoveryMechanism::JsonLoader(const JsonArgs&) { .OptionalField("max_concurrent_requests", &DiscoveryMechanism::max_concurrent_requests) .OptionalField("outlierDetection", - &DiscoveryMechanism::outlier_detection_lb_config, - "outlier_detection") + &DiscoveryMechanism::outlier_detection_lb_config) .Finish(); return loader; } @@ -1216,15 +1206,8 @@ class XdsClusterResolverLbFactory : public LoadBalancingPolicyFactory { "requires configuration. " "Please use loadBalancingConfig field of service config instead."); } - class XdsJsonArgs : public JsonArgs { - public: - bool IsEnabled(absl::string_view key) const override { - if (key == "outlier_detection") return XdsOutlierDetectionEnabled(); - return true; - } - }; return LoadRefCountedFromJson( - json, XdsJsonArgs(), + json, JsonArgs(), "errors validating xds_cluster_resolver LB policy config"); } diff --git a/src/core/ext/xds/xds_cluster.cc b/src/core/ext/xds/xds_cluster.cc index 949f27c0b489c..1c08dcd934c3c 100644 --- a/src/core/ext/xds/xds_cluster.cc +++ b/src/core/ext/xds/xds_cluster.cc @@ -422,13 +422,8 @@ absl::StatusOr CdsResourceParse( } } } - // As long as outlier detection field is present in the cluster update, - // we will end up with a outlier detection in the cluster resource which will - // lead to the creation of outlier detection in discovery mechanism. Values - // for outlier detection will be based on fields received and - // default values. - if (XdsOutlierDetectionEnabled() && - envoy_config_cluster_v3_Cluster_has_outlier_detection(cluster)) { + // Outlier detection config. + if (envoy_config_cluster_v3_Cluster_has_outlier_detection(cluster)) { ValidationErrors::ScopedField field(&errors, ".outlier_detection"); OutlierDetectionConfig outlier_detection_update; const envoy_config_cluster_v3_OutlierDetection* outlier_detection = diff --git a/test/core/client_channel/outlier_detection_lb_config_parser_test.cc b/test/core/client_channel/outlier_detection_lb_config_parser_test.cc index 11234d36f9d43..eceb629a250ff 100644 --- a/test/core/client_channel/outlier_detection_lb_config_parser_test.cc +++ b/test/core/client_channel/outlier_detection_lb_config_parser_test.cc @@ -23,7 +23,6 @@ #include "src/core/lib/channel/channel_args.h" #include "src/core/lib/service_config/service_config_impl.h" -#include "test/core/util/scoped_env_var.h" #include "test/core/util/test_config.h" namespace grpc_core { @@ -32,14 +31,9 @@ namespace { class OutlierDetectionConfigParsingTest : public ::testing::Test { public: - OutlierDetectionConfigParsingTest() - : env_var_("GRPC_EXPERIMENTAL_ENABLE_OUTLIER_DETECTION") {} - static void SetUpTestSuite() { grpc_init(); } static void TearDownTestSuite() { grpc_shutdown_blocking(); } - - ScopedExperimentalEnvVar env_var_; }; TEST_F(OutlierDetectionConfigParsingTest, ValidConfig) { diff --git a/test/core/xds/xds_cluster_resource_type_test.cc b/test/core/xds/xds_cluster_resource_type_test.cc index 9bb0507d21343..ebbc19d94e63f 100644 --- a/test/core/xds/xds_cluster_resource_type_test.cc +++ b/test/core/xds/xds_cluster_resource_type_test.cc @@ -1014,8 +1014,7 @@ TEST_F(CircuitBreakingTest, DefaultThresholdWithMaxRequestsUnset) { using OutlierDetectionTest = XdsClusterTest; -TEST_F(OutlierDetectionTest, EnabledWithDefaults) { - ScopedExperimentalEnvVar env("GRPC_EXPERIMENTAL_ENABLE_OUTLIER_DETECTION"); +TEST_F(OutlierDetectionTest, DefaultValues) { Cluster cluster; cluster.set_name("foo"); cluster.set_type(cluster.EDS); @@ -1034,26 +1033,7 @@ TEST_F(OutlierDetectionTest, EnabledWithDefaults) { EXPECT_EQ(*resource.outlier_detection, OutlierDetectionConfig()); } -TEST_F(OutlierDetectionTest, NotEnabledWithEnvVarUnset) { - Cluster cluster; - cluster.set_name("foo"); - cluster.set_type(cluster.EDS); - cluster.mutable_eds_cluster_config()->mutable_eds_config()->mutable_self(); - cluster.mutable_outlier_detection(); - std::string serialized_resource; - ASSERT_TRUE(cluster.SerializeToString(&serialized_resource)); - auto* resource_type = XdsClusterResourceType::Get(); - auto decode_result = resource_type->Decode( - decode_context_, serialized_resource, /*is_v2=*/false); - ASSERT_TRUE(decode_result.resource.ok()) << decode_result.resource.status(); - ASSERT_TRUE(decode_result.name.has_value()); - EXPECT_EQ(*decode_result.name, "foo"); - auto& resource = static_cast(**decode_result.resource); - ASSERT_FALSE(resource.outlier_detection.has_value()); -} - TEST_F(OutlierDetectionTest, AllFieldsSet) { - ScopedExperimentalEnvVar env("GRPC_EXPERIMENTAL_ENABLE_OUTLIER_DETECTION"); Cluster cluster; cluster.set_name("foo"); cluster.set_type(cluster.EDS); @@ -1105,7 +1085,6 @@ TEST_F(OutlierDetectionTest, AllFieldsSet) { } TEST_F(OutlierDetectionTest, InvalidValues) { - ScopedExperimentalEnvVar env("GRPC_EXPERIMENTAL_ENABLE_OUTLIER_DETECTION"); Cluster cluster; cluster.set_name("foo"); cluster.set_type(cluster.EDS); diff --git a/test/cpp/end2end/xds/xds_outlier_detection_end2end_test.cc b/test/cpp/end2end/xds/xds_outlier_detection_end2end_test.cc index eeb3954844c23..646d95a10dbfb 100644 --- a/test/cpp/end2end/xds/xds_outlier_detection_end2end_test.cc +++ b/test/cpp/end2end/xds/xds_outlier_detection_end2end_test.cc @@ -58,8 +58,6 @@ INSTANTIATE_TEST_SUITE_P(XdsTest, OutlierDetectionTest, // 4. Let the ejection period pass and verify we can go back to both backends // after the uneject. TEST_P(OutlierDetectionTest, SuccessRateEjectionAndUnejection) { - ScopedExperimentalEnvVar env_var( - "GRPC_EXPERIMENTAL_ENABLE_OUTLIER_DETECTION"); CreateAndStartBackends(2); auto cluster = default_cluster_; cluster.set_lb_policy(Cluster::RING_HASH); @@ -122,8 +120,6 @@ TEST_P(OutlierDetectionTest, SuccessRateEjectionAndUnejection) { // We don't eject more than max_ejection_percent (default 10%) of the backends // beyond the first one. TEST_P(OutlierDetectionTest, SuccessRateMaxPercent) { - ScopedExperimentalEnvVar env_var( - "GRPC_EXPERIMENTAL_ENABLE_OUTLIER_DETECTION"); CreateAndStartBackends(4); auto cluster = default_cluster_; cluster.set_lb_policy(Cluster::RING_HASH); @@ -219,8 +215,6 @@ TEST_P(OutlierDetectionTest, SuccessRateMaxPercent) { // Success rate stdev_factor is honored, a higher value would ensure ejection // does not occur. TEST_P(OutlierDetectionTest, SuccessRateStdevFactor) { - ScopedExperimentalEnvVar env_var( - "GRPC_EXPERIMENTAL_ENABLE_OUTLIER_DETECTION"); CreateAndStartBackends(2); auto cluster = default_cluster_; cluster.set_lb_policy(Cluster::RING_HASH); @@ -285,8 +279,6 @@ TEST_P(OutlierDetectionTest, SuccessRateStdevFactor) { // the randomized number between 1 to 100 will always be great, so nothing will // be ejected. TEST_P(OutlierDetectionTest, SuccessRateEnforcementPercentage) { - ScopedExperimentalEnvVar env_var( - "GRPC_EXPERIMENTAL_ENABLE_OUTLIER_DETECTION"); CreateAndStartBackends(2); auto cluster = default_cluster_; cluster.set_lb_policy(Cluster::RING_HASH); @@ -346,8 +338,6 @@ TEST_P(OutlierDetectionTest, SuccessRateEnforcementPercentage) { // Success rate does not eject if there are less than minimum_hosts backends // Set success_rate_minimum_hosts to 3 when we only have 2 backends TEST_P(OutlierDetectionTest, SuccessRateMinimumHosts) { - ScopedExperimentalEnvVar env_var( - "GRPC_EXPERIMENTAL_ENABLE_OUTLIER_DETECTION"); CreateAndStartBackends(2); auto cluster = default_cluster_; cluster.set_lb_policy(Cluster::RING_HASH); @@ -408,8 +398,6 @@ TEST_P(OutlierDetectionTest, SuccessRateMinimumHosts) { // Set success_rate_request_volume to 4 when we only send 3 RPC in the // interval. TEST_P(OutlierDetectionTest, SuccessRateRequestVolume) { - ScopedExperimentalEnvVar env_var( - "GRPC_EXPERIMENTAL_ENABLE_OUTLIER_DETECTION"); CreateAndStartBackends(2); auto cluster = default_cluster_; cluster.set_lb_policy(Cluster::RING_HASH); @@ -477,8 +465,6 @@ TEST_P(OutlierDetectionTest, SuccessRateRequestVolume) { // 4. Let the ejection period pass and verify that traffic will again go both // backends as we have unejected the backend. TEST_P(OutlierDetectionTest, FailurePercentageEjectionAndUnejection) { - ScopedExperimentalEnvVar env_var( - "GRPC_EXPERIMENTAL_ENABLE_OUTLIER_DETECTION"); CreateAndStartBackends(2); auto cluster = default_cluster_; cluster.set_lb_policy(Cluster::RING_HASH); @@ -548,8 +534,6 @@ TEST_P(OutlierDetectionTest, FailurePercentageEjectionAndUnejection) { // We don't eject more than max_ejection_percent (default 10%) of the backends // beyond the first one. TEST_P(OutlierDetectionTest, FailurePercentageMaxPercentage) { - ScopedExperimentalEnvVar env_var( - "GRPC_EXPERIMENTAL_ENABLE_OUTLIER_DETECTION"); CreateAndStartBackends(4); auto cluster = default_cluster_; cluster.set_lb_policy(Cluster::RING_HASH); @@ -645,8 +629,6 @@ TEST_P(OutlierDetectionTest, FailurePercentageMaxPercentage) { // Failure percentage threshold is honored, a higher value would ensure ejection // does not occur TEST_P(OutlierDetectionTest, FailurePercentageThreshold) { - ScopedExperimentalEnvVar env_var( - "GRPC_EXPERIMENTAL_ENABLE_OUTLIER_DETECTION"); CreateAndStartBackends(2); auto cluster = default_cluster_; cluster.set_lb_policy(Cluster::RING_HASH); @@ -708,8 +690,6 @@ TEST_P(OutlierDetectionTest, FailurePercentageThreshold) { // guarantee the randomized number between 1 to 100 will always be great, so // nothing will be ejected. TEST_P(OutlierDetectionTest, FailurePercentageEnforcementPercentage) { - ScopedExperimentalEnvVar env_var( - "GRPC_EXPERIMENTAL_ENABLE_OUTLIER_DETECTION"); CreateAndStartBackends(2); auto cluster = default_cluster_; cluster.set_lb_policy(Cluster::RING_HASH); @@ -770,8 +750,6 @@ TEST_P(OutlierDetectionTest, FailurePercentageEnforcementPercentage) { // Failure percentage does not eject if there are less than minimum_hosts // backends Set success_rate_minimum_hosts to 3 when we only have 2 backends TEST_P(OutlierDetectionTest, FailurePercentageMinimumHosts) { - ScopedExperimentalEnvVar env_var( - "GRPC_EXPERIMENTAL_ENABLE_OUTLIER_DETECTION"); CreateAndStartBackends(2); auto cluster = default_cluster_; cluster.set_lb_policy(Cluster::RING_HASH); @@ -838,8 +816,6 @@ TEST_P(OutlierDetectionTest, FailurePercentageMinimumHosts) { // Set success_rate_request_volume to 4 when we only send 3 RPC in the // interval. TEST_P(OutlierDetectionTest, FailurePercentageRequestVolume) { - ScopedExperimentalEnvVar env_var( - "GRPC_EXPERIMENTAL_ENABLE_OUTLIER_DETECTION"); CreateAndStartBackends(2); auto cluster = default_cluster_; cluster.set_lb_policy(Cluster::RING_HASH); @@ -904,8 +880,6 @@ TEST_P(OutlierDetectionTest, FailurePercentageRequestVolume) { // Configure success rate to eject 1 and failure percentage to eject 2. // Verify that maximum 2 backends are ejected, not 3! TEST_P(OutlierDetectionTest, SuccessRateAndFailurePercentage) { - ScopedExperimentalEnvVar env_var( - "GRPC_EXPERIMENTAL_ENABLE_OUTLIER_DETECTION"); CreateAndStartBackends(4); auto cluster = default_cluster_; cluster.set_lb_policy(Cluster::RING_HASH); @@ -1025,8 +999,6 @@ TEST_P(OutlierDetectionTest, SuccessRateAndFailurePercentage) { // that there will be no ejection taking place since we can't do any // calculations. TEST_P(OutlierDetectionTest, SuccessRateAndFailurePercentageBothDisabled) { - ScopedExperimentalEnvVar env_var( - "GRPC_EXPERIMENTAL_ENABLE_OUTLIER_DETECTION"); CreateAndStartBackends(2); auto cluster = default_cluster_; cluster.set_lb_policy(Cluster::RING_HASH); @@ -1073,100 +1045,9 @@ TEST_P(OutlierDetectionTest, SuccessRateAndFailurePercentageBothDisabled) { EXPECT_EQ(100, backends_[1]->backend_service()->request_count()); } -// GRPC_EXPERIMENTAL_ENABLE_OUTLIER_DETECTION not configured so feature -// disabled. -TEST_P(OutlierDetectionTest, - SuccessRateAndFailurePercentageEjectionPolicyDisabled) { - CreateAndStartBackends(4); - auto cluster = default_cluster_; - cluster.set_lb_policy(Cluster::RING_HASH); - // Setup outlier failure percentage parameters. - // Any failure will cause an potential ejection with the probability of 100% - // (to eliminate flakiness of the test). - auto* outlier_detection = cluster.mutable_outlier_detection(); - SetProtoDuration(grpc_core::Duration::Seconds(1), - outlier_detection->mutable_interval()); - outlier_detection->mutable_max_ejection_percent()->set_value(50); - // This stdev of 500 will ensure the number of ok RPC and error RPC we send - // will make 1 outlier out of the 4 backends. - outlier_detection->mutable_success_rate_stdev_factor()->set_value(500); - outlier_detection->mutable_enforcing_success_rate()->set_value(100); - outlier_detection->mutable_success_rate_minimum_hosts()->set_value(1); - outlier_detection->mutable_success_rate_request_volume()->set_value(1); - outlier_detection->mutable_failure_percentage_threshold()->set_value(0); - outlier_detection->mutable_enforcing_failure_percentage()->set_value(100); - outlier_detection->mutable_failure_percentage_minimum_hosts()->set_value(1); - outlier_detection->mutable_failure_percentage_request_volume()->set_value(1); - balancer_->ads_service()->SetCdsResource(cluster); - auto new_route_config = default_route_config_; - auto* route = new_route_config.mutable_virtual_hosts(0)->mutable_routes(0); - auto* hash_policy = route->mutable_route()->add_hash_policy(); - hash_policy->mutable_header()->set_header_name("address_hash"); - SetListenerAndRouteConfiguration(balancer_.get(), default_listener_, - new_route_config); - EdsResourceArgs args({{"locality0", CreateEndpointsForBackends()}}); - balancer_->ads_service()->SetEdsResource(BuildEdsResource(args)); - // Note each type of RPC will contains a header value that will always be - // hashed to a specific backend as the header value matches the value used - // to create the entry in the ring. - std::vector> metadata = { - {"address_hash", CreateMetadataValueThatHashesToBackend(0)}}; - std::vector> metadata1 = { - {"address_hash", CreateMetadataValueThatHashesToBackend(1)}}; - std::vector> metadata2 = { - {"address_hash", CreateMetadataValueThatHashesToBackend(2)}}; - std::vector> metadata3 = { - {"address_hash", CreateMetadataValueThatHashesToBackend(3)}}; - const auto rpc_options = RpcOptions().set_metadata(metadata); - const auto rpc_options1 = RpcOptions().set_metadata(metadata1); - const auto rpc_options2 = RpcOptions().set_metadata(metadata2); - const auto rpc_options3 = RpcOptions().set_metadata(metadata3); - WaitForBackend(DEBUG_LOCATION, 0, /*check_status=*/nullptr, - WaitForBackendOptions(), rpc_options); - WaitForBackend(DEBUG_LOCATION, 1, /*check_status=*/nullptr, - WaitForBackendOptions(), rpc_options1); - WaitForBackend(DEBUG_LOCATION, 2, /*check_status=*/nullptr, - WaitForBackendOptions(), rpc_options2); - WaitForBackend(DEBUG_LOCATION, 3, /*check_status=*/nullptr, - WaitForBackendOptions(), rpc_options3); - // Cause 2 errors on 1 backend and 1 error on 2 backends and wait for 1 - // outlier detection interval to pass. The errors should have caused 2 - // ejctionss but since the policy is disabled we are not ejecting any and - // traffic flow as usual and RPCs reach destinated backends. - CheckRpcSendFailure( - DEBUG_LOCATION, StatusCode::CANCELLED, "", - RpcOptions().set_metadata(metadata).set_server_expected_error( - StatusCode::CANCELLED)); - CheckRpcSendFailure( - DEBUG_LOCATION, StatusCode::CANCELLED, "", - RpcOptions().set_metadata(metadata).set_server_expected_error( - StatusCode::CANCELLED)); - CheckRpcSendFailure( - DEBUG_LOCATION, StatusCode::CANCELLED, "", - RpcOptions().set_metadata(metadata1).set_server_expected_error( - StatusCode::CANCELLED)); - CheckRpcSendFailure( - DEBUG_LOCATION, StatusCode::CANCELLED, "", - RpcOptions().set_metadata(metadata2).set_server_expected_error( - StatusCode::CANCELLED)); - gpr_sleep_until(grpc_timeout_milliseconds_to_deadline( - 3000 * grpc_test_slowdown_factor())); - ResetBackendCounters(); - CheckRpcSendOk(DEBUG_LOCATION, 100, rpc_options); - CheckRpcSendOk(DEBUG_LOCATION, 100, rpc_options1); - CheckRpcSendOk(DEBUG_LOCATION, 100, rpc_options2); - CheckRpcSendOk(DEBUG_LOCATION, 100, rpc_options3); - EXPECT_EQ(100, backends_[0]->backend_service()->request_count()); - EXPECT_EQ(100, backends_[1]->backend_service()->request_count()); - EXPECT_EQ(100, backends_[2]->backend_service()->request_count()); - EXPECT_EQ(100, backends_[3]->backend_service()->request_count()); -} - // Tests that we uneject any ejected addresses when the OD policy is // disabled. TEST_P(OutlierDetectionTest, DisableOutlierDetectionWhileAddressesAreEjected) { - ScopedExperimentalEnvVar env_var( - "GRPC_EXPERIMENTAL_ENABLE_OUTLIER_DETECTION"); CreateAndStartBackends(2); auto cluster = default_cluster_; cluster.set_lb_policy(Cluster::RING_HASH); @@ -1250,14 +1131,8 @@ int main(int argc, char** argv) { // Workaround Apple CFStream bug grpc_core::SetEnv("grpc_cfstream", "0"); #endif - // TODO(roth): This is a hack to ensure that the outlier_detection LB policy - // is always registered at gRPC init time. When the LB policy registry is - // moved to the new CoreConfiguration system, change this to use - // CoreConfiguration::BuildSpecialConfiguration() instead. - grpc_core::SetEnv("GRPC_EXPERIMENTAL_ENABLE_OUTLIER_DETECTION", "true"); grpc_init(); const auto result = RUN_ALL_TESTS(); grpc_shutdown(); - grpc_core::UnsetEnv("GRPC_EXPERIMENTAL_ENABLE_OUTLIER_DETECTION"); return result; } From a38e860895e0a32263781555cfe085a91b84363b Mon Sep 17 00:00:00 2001 From: "Mark D. Roth" Date: Wed, 5 Oct 2022 19:37:12 +0000 Subject: [PATCH 2/2] fix sanity --- BUILD | 2 -- build_autogenerated.yaml | 4 +--- .../lb_policy/outlier_detection/outlier_detection.cc | 2 -- .../client_channel/lb_policy/xds/xds_cluster_resolver.cc | 1 - test/core/xds/BUILD | 1 - test/core/xds/xds_cluster_resource_type_test.cc | 1 - test/cpp/end2end/xds/BUILD | 1 - test/cpp/end2end/xds/xds_outlier_detection_end2end_test.cc | 3 --- 8 files changed, 1 insertion(+), 14 deletions(-) diff --git a/BUILD b/BUILD index 22f49c44ff4e4..3975f53c4e1ea 100644 --- a/BUILD +++ b/BUILD @@ -4998,7 +4998,6 @@ grpc_cc_library( "grpc_lb_policy_ring_hash", "grpc_lb_xds_channel_args", "grpc_lb_xds_common", - "grpc_outlier_detection_header", "grpc_public_hdrs", "grpc_resolver", "grpc_resolver_fake", @@ -5294,7 +5293,6 @@ grpc_cc_library( "closure", "config", "debug_location", - "env", "gpr", "grpc_base", "grpc_client_channel", diff --git a/build_autogenerated.yaml b/build_autogenerated.yaml index e238ea7bf524f..bc2c1a92a9f6f 100644 --- a/build_autogenerated.yaml +++ b/build_autogenerated.yaml @@ -10999,8 +10999,7 @@ targets: gtest: true build: test language: c++ - headers: - - test/core/util/scoped_env_var.h + headers: [] src: - src/proto/grpc/testing/xds/v3/address.proto - src/proto/grpc/testing/xds/v3/aggregate_cluster.proto @@ -11487,7 +11486,6 @@ targets: run: false language: c++ headers: - - test/core/util/scoped_env_var.h - test/cpp/end2end/counted_service.h - test/cpp/end2end/test_service_impl.h - test/cpp/end2end/xds/no_op_http_filter.h diff --git a/src/core/ext/filters/client_channel/lb_policy/outlier_detection/outlier_detection.cc b/src/core/ext/filters/client_channel/lb_policy/outlier_detection/outlier_detection.cc index 21b2354f29f63..b5047a79788c0 100644 --- a/src/core/ext/filters/client_channel/lb_policy/outlier_detection/outlier_detection.cc +++ b/src/core/ext/filters/client_channel/lb_policy/outlier_detection/outlier_detection.cc @@ -46,9 +46,7 @@ #include "src/core/lib/channel/channel_args.h" #include "src/core/lib/config/core_configuration.h" #include "src/core/lib/debug/trace.h" -#include "src/core/lib/gpr/string.h" #include "src/core/lib/gprpp/debug_location.h" -#include "src/core/lib/gprpp/env.h" #include "src/core/lib/gprpp/orphanable.h" #include "src/core/lib/gprpp/ref_counted.h" #include "src/core/lib/gprpp/ref_counted_ptr.h" diff --git a/src/core/ext/filters/client_channel/lb_policy/xds/xds_cluster_resolver.cc b/src/core/ext/filters/client_channel/lb_policy/xds/xds_cluster_resolver.cc index b5547907ef0d5..ed7e740479e20 100644 --- a/src/core/ext/filters/client_channel/lb_policy/xds/xds_cluster_resolver.cc +++ b/src/core/ext/filters/client_channel/lb_policy/xds/xds_cluster_resolver.cc @@ -40,7 +40,6 @@ #include "src/core/ext/filters/client_channel/lb_policy/address_filtering.h" #include "src/core/ext/filters/client_channel/lb_policy/child_policy_handler.h" -#include "src/core/ext/filters/client_channel/lb_policy/outlier_detection/outlier_detection.h" #include "src/core/ext/filters/client_channel/lb_policy/ring_hash/ring_hash.h" #include "src/core/ext/filters/client_channel/lb_policy/xds/xds.h" #include "src/core/ext/filters/client_channel/lb_policy/xds/xds_channel_args.h" diff --git a/test/core/xds/BUILD b/test/core/xds/BUILD index 12d0c790e77f6..b73cdcac2ff34 100644 --- a/test/core/xds/BUILD +++ b/test/core/xds/BUILD @@ -186,7 +186,6 @@ grpc_cc_test( "//src/proto/grpc/testing/xds/v3:cluster_proto", "//src/proto/grpc/testing/xds/v3:tls_proto", "//test/core/util:grpc_test_util", - "//test/core/util:scoped_env_var", ], ) diff --git a/test/core/xds/xds_cluster_resource_type_test.cc b/test/core/xds/xds_cluster_resource_type_test.cc index ebbc19d94e63f..f8e3a83bbc228 100644 --- a/test/core/xds/xds_cluster_resource_type_test.cc +++ b/test/core/xds/xds_cluster_resource_type_test.cc @@ -52,7 +52,6 @@ #include "src/proto/grpc/testing/xds/v3/endpoint.pb.h" #include "src/proto/grpc/testing/xds/v3/outlier_detection.pb.h" #include "src/proto/grpc/testing/xds/v3/tls.pb.h" -#include "test/core/util/scoped_env_var.h" #include "test/core/util/test_config.h" using envoy::config::cluster::v3::Cluster; diff --git a/test/cpp/end2end/xds/BUILD b/test/cpp/end2end/xds/BUILD index 68650ad560ae6..895b160d016ac 100644 --- a/test/cpp/end2end/xds/BUILD +++ b/test/cpp/end2end/xds/BUILD @@ -295,7 +295,6 @@ grpc_cc_test( "//src/proto/grpc/testing/xds/v3:fault_proto", "//src/proto/grpc/testing/xds/v3:router_proto", "//test/core/util:grpc_test_util", - "//test/core/util:scoped_env_var", ], ) diff --git a/test/cpp/end2end/xds/xds_outlier_detection_end2end_test.cc b/test/cpp/end2end/xds/xds_outlier_detection_end2end_test.cc index 646d95a10dbfb..477fd61817c60 100644 --- a/test/cpp/end2end/xds/xds_outlier_detection_end2end_test.cc +++ b/test/cpp/end2end/xds/xds_outlier_detection_end2end_test.cc @@ -25,7 +25,6 @@ #include "src/proto/grpc/testing/xds/v3/fault.grpc.pb.h" #include "src/proto/grpc/testing/xds/v3/outlier_detection.grpc.pb.h" #include "src/proto/grpc/testing/xds/v3/router.grpc.pb.h" -#include "test/core/util/scoped_env_var.h" #include "test/cpp/end2end/xds/no_op_http_filter.h" #include "test/cpp/end2end/xds/xds_end2end_test_lib.h" @@ -33,8 +32,6 @@ namespace grpc { namespace testing { namespace { -using ::grpc_core::testing::ScopedExperimentalEnvVar; - class OutlierDetectionTest : public XdsEnd2endTest { protected: std::string CreateMetadataValueThatHashesToBackend(int index) {