From 965feb5726d4360afe5a4b5e9cbef4c1e912976b Mon Sep 17 00:00:00 2001 From: apolcyn Date: Fri, 20 May 2022 12:45:33 -0700 Subject: [PATCH] xds: Remove aggregate and logical dns clusters env var guard (#29742) * Remove aggregate and logical dns clusters env var guard --- src/core/ext/xds/xds_cluster.cc | 16 ----- .../xds/xds_cluster_type_end2end_test.cc | 72 ------------------- .../end2end/xds/xds_ring_hash_end2end_test.cc | 4 -- 3 files changed, 92 deletions(-) diff --git a/src/core/ext/xds/xds_cluster.cc b/src/core/ext/xds/xds_cluster.cc index dfc256f4e0f..769f469e39a 100644 --- a/src/core/ext/xds/xds_cluster.cc +++ b/src/core/ext/xds/xds_cluster.cc @@ -206,19 +206,6 @@ grpc_error_handle CdsLogicalDnsParse( return GRPC_ERROR_NONE; } -// TODO(donnadionne): Check to see if cluster types aggregate_cluster and -// logical_dns are enabled, this will be -// removed once the cluster types are fully integration-tested and enabled by -// default. -bool XdsAggregateAndLogicalDnsClusterEnabled() { - char* value = gpr_getenv( - "GRPC_XDS_EXPERIMENTAL_ENABLE_AGGREGATE_AND_LOGICAL_DNS_CLUSTER"); - bool parsed_value; - bool parse_succeeded = gpr_parse_bool_value(value, &parsed_value); - gpr_free(value); - return parse_succeeded && parsed_value; -} - grpc_error_handle CdsResourceParse( const XdsEncodingContext& context, const envoy_config_cluster_v3_Cluster* cluster, bool /*is_v2*/, @@ -250,9 +237,6 @@ grpc_error_handle CdsResourceParse( if (service_name.size != 0) { cds_update->eds_service_name = UpbStringToStdString(service_name); } - } else if (!XdsAggregateAndLogicalDnsClusterEnabled()) { - errors.push_back( - GRPC_ERROR_CREATE_FROM_STATIC_STRING("DiscoveryType is not valid.")); } else if (envoy_config_cluster_v3_Cluster_type(cluster) == envoy_config_cluster_v3_Cluster_LOGICAL_DNS) { cds_update->cluster_type = XdsClusterResource::ClusterType::LOGICAL_DNS; diff --git a/test/cpp/end2end/xds/xds_cluster_type_end2end_test.cc b/test/cpp/end2end/xds/xds_cluster_type_end2end_test.cc index e167a85080a..889214d19e3 100644 --- a/test/cpp/end2end/xds/xds_cluster_type_end2end_test.cc +++ b/test/cpp/end2end/xds/xds_cluster_type_end2end_test.cc @@ -80,8 +80,6 @@ INSTANTIATE_TEST_SUITE_P(XdsTest, LogicalDNSClusterTest, ::testing::Values(XdsTestType()), &XdsTestType::Name); TEST_P(LogicalDNSClusterTest, Basic) { - ScopedExperimentalEnvVar env_var( - "GRPC_XDS_EXPERIMENTAL_ENABLE_AGGREGATE_AND_LOGICAL_DNS_CLUSTER"); CreateAndStartBackends(1); // Create Logical DNS Cluster auto cluster = default_cluster_; @@ -108,8 +106,6 @@ TEST_P(LogicalDNSClusterTest, Basic) { } TEST_P(LogicalDNSClusterTest, MissingLoadAssignment) { - ScopedExperimentalEnvVar env_var( - "GRPC_XDS_EXPERIMENTAL_ENABLE_AGGREGATE_AND_LOGICAL_DNS_CLUSTER"); // Create Logical DNS Cluster auto cluster = default_cluster_; cluster.set_type(Cluster::LOGICAL_DNS); @@ -122,8 +118,6 @@ TEST_P(LogicalDNSClusterTest, MissingLoadAssignment) { } TEST_P(LogicalDNSClusterTest, MissingLocalities) { - ScopedExperimentalEnvVar env_var( - "GRPC_XDS_EXPERIMENTAL_ENABLE_AGGREGATE_AND_LOGICAL_DNS_CLUSTER"); // Create Logical DNS Cluster auto cluster = default_cluster_; cluster.set_type(Cluster::LOGICAL_DNS); @@ -138,8 +132,6 @@ TEST_P(LogicalDNSClusterTest, MissingLocalities) { } TEST_P(LogicalDNSClusterTest, MultipleLocalities) { - ScopedExperimentalEnvVar env_var( - "GRPC_XDS_EXPERIMENTAL_ENABLE_AGGREGATE_AND_LOGICAL_DNS_CLUSTER"); // Create Logical DNS Cluster auto cluster = default_cluster_; cluster.set_type(Cluster::LOGICAL_DNS); @@ -156,8 +148,6 @@ TEST_P(LogicalDNSClusterTest, MultipleLocalities) { } TEST_P(LogicalDNSClusterTest, MissingEndpoints) { - ScopedExperimentalEnvVar env_var( - "GRPC_XDS_EXPERIMENTAL_ENABLE_AGGREGATE_AND_LOGICAL_DNS_CLUSTER"); // Create Logical DNS Cluster auto cluster = default_cluster_; cluster.set_type(Cluster::LOGICAL_DNS); @@ -172,8 +162,6 @@ TEST_P(LogicalDNSClusterTest, MissingEndpoints) { } TEST_P(LogicalDNSClusterTest, MultipleEndpoints) { - ScopedExperimentalEnvVar env_var( - "GRPC_XDS_EXPERIMENTAL_ENABLE_AGGREGATE_AND_LOGICAL_DNS_CLUSTER"); // Create Logical DNS Cluster auto cluster = default_cluster_; cluster.set_type(Cluster::LOGICAL_DNS); @@ -190,8 +178,6 @@ TEST_P(LogicalDNSClusterTest, MultipleEndpoints) { } TEST_P(LogicalDNSClusterTest, EmptyEndpoint) { - ScopedExperimentalEnvVar env_var( - "GRPC_XDS_EXPERIMENTAL_ENABLE_AGGREGATE_AND_LOGICAL_DNS_CLUSTER"); // Create Logical DNS Cluster auto cluster = default_cluster_; cluster.set_type(Cluster::LOGICAL_DNS); @@ -204,8 +190,6 @@ TEST_P(LogicalDNSClusterTest, EmptyEndpoint) { } TEST_P(LogicalDNSClusterTest, EndpointMissingAddress) { - ScopedExperimentalEnvVar env_var( - "GRPC_XDS_EXPERIMENTAL_ENABLE_AGGREGATE_AND_LOGICAL_DNS_CLUSTER"); // Create Logical DNS Cluster auto cluster = default_cluster_; cluster.set_type(Cluster::LOGICAL_DNS); @@ -221,8 +205,6 @@ TEST_P(LogicalDNSClusterTest, EndpointMissingAddress) { } TEST_P(LogicalDNSClusterTest, AddressMissingSocketAddress) { - ScopedExperimentalEnvVar env_var( - "GRPC_XDS_EXPERIMENTAL_ENABLE_AGGREGATE_AND_LOGICAL_DNS_CLUSTER"); // Create Logical DNS Cluster auto cluster = default_cluster_; cluster.set_type(Cluster::LOGICAL_DNS); @@ -239,8 +221,6 @@ TEST_P(LogicalDNSClusterTest, AddressMissingSocketAddress) { } TEST_P(LogicalDNSClusterTest, SocketAddressHasResolverName) { - ScopedExperimentalEnvVar env_var( - "GRPC_XDS_EXPERIMENTAL_ENABLE_AGGREGATE_AND_LOGICAL_DNS_CLUSTER"); // Create Logical DNS Cluster auto cluster = default_cluster_; cluster.set_type(Cluster::LOGICAL_DNS); @@ -260,8 +240,6 @@ TEST_P(LogicalDNSClusterTest, SocketAddressHasResolverName) { } TEST_P(LogicalDNSClusterTest, SocketAddressMissingAddress) { - ScopedExperimentalEnvVar env_var( - "GRPC_XDS_EXPERIMENTAL_ENABLE_AGGREGATE_AND_LOGICAL_DNS_CLUSTER"); // Create Logical DNS Cluster auto cluster = default_cluster_; cluster.set_type(Cluster::LOGICAL_DNS); @@ -279,8 +257,6 @@ TEST_P(LogicalDNSClusterTest, SocketAddressMissingAddress) { } TEST_P(LogicalDNSClusterTest, SocketAddressMissingPort) { - ScopedExperimentalEnvVar env_var( - "GRPC_XDS_EXPERIMENTAL_ENABLE_AGGREGATE_AND_LOGICAL_DNS_CLUSTER"); // Create Logical DNS Cluster auto cluster = default_cluster_; cluster.set_type(Cluster::LOGICAL_DNS); @@ -298,18 +274,6 @@ TEST_P(LogicalDNSClusterTest, SocketAddressMissingPort) { ::testing::HasSubstr("SocketAddress port_value field not set")); } -// Test that CDS client should send a NACK if cluster type is Logical DNS but -// the feature is not yet supported. -TEST_P(LogicalDNSClusterTest, Disabled) { - auto cluster = default_cluster_; - cluster.set_type(Cluster::LOGICAL_DNS); - balancer_->ads_service()->SetCdsResource(cluster); - const auto response_state = WaitForCdsNack(DEBUG_LOCATION); - ASSERT_TRUE(response_state.has_value()) << "timed out waiting for NACK"; - EXPECT_THAT(response_state->error_message, - ::testing::HasSubstr("DiscoveryType is not valid.")); -} - // // aggregate cluster tests // @@ -323,8 +287,6 @@ INSTANTIATE_TEST_SUITE_P(XdsTest, AggregateClusterTest, ::testing::Values(XdsTestType()), &XdsTestType::Name); TEST_P(AggregateClusterTest, ) { - ScopedExperimentalEnvVar env_var( - "GRPC_XDS_EXPERIMENTAL_ENABLE_AGGREGATE_AND_LOGICAL_DNS_CLUSTER"); CreateAndStartBackends(2); const char* kNewCluster1Name = "new_cluster_1"; const char* kNewEdsService1Name = "new_eds_service_name_1"; @@ -376,8 +338,6 @@ TEST_P(AggregateClusterTest, ) { } TEST_P(AggregateClusterTest, DiamondDependency) { - ScopedExperimentalEnvVar env_var( - "GRPC_XDS_EXPERIMENTAL_ENABLE_AGGREGATE_AND_LOGICAL_DNS_CLUSTER"); const char* kNewClusterName1 = "new_cluster_1"; const char* kNewEdsServiceName1 = "new_eds_service_name_1"; const char* kNewClusterName2 = "new_cluster_2"; @@ -446,8 +406,6 @@ TEST_P(AggregateClusterTest, DiamondDependency) { // CONNECTING (because the failover timer was not running), so we // incorrectly failed the RPCs. TEST_P(AggregateClusterTest, FallBackWithConnectivityChurn) { - ScopedExperimentalEnvVar env_var( - "GRPC_XDS_EXPERIMENTAL_ENABLE_AGGREGATE_AND_LOGICAL_DNS_CLUSTER"); CreateAndStartBackends(2); const char* kClusterName1 = "cluster1"; const char* kClusterName2 = "cluster2"; @@ -575,8 +533,6 @@ TEST_P(AggregateClusterTest, FallBackWithConnectivityChurn) { } TEST_P(AggregateClusterTest, EdsToLogicalDns) { - ScopedExperimentalEnvVar env_var( - "GRPC_XDS_EXPERIMENTAL_ENABLE_AGGREGATE_AND_LOGICAL_DNS_CLUSTER"); CreateAndStartBackends(2); const char* kNewCluster1Name = "new_cluster_1"; const char* kNewEdsService1Name = "new_eds_service_name_1"; @@ -636,8 +592,6 @@ TEST_P(AggregateClusterTest, EdsToLogicalDns) { } TEST_P(AggregateClusterTest, LogicalDnsToEds) { - ScopedExperimentalEnvVar env_var( - "GRPC_XDS_EXPERIMENTAL_ENABLE_AGGREGATE_AND_LOGICAL_DNS_CLUSTER"); CreateAndStartBackends(2); const char* kNewCluster2Name = "new_cluster_2"; const char* kNewEdsService2Name = "new_eds_service_name_2"; @@ -705,8 +659,6 @@ TEST_P(AggregateClusterTest, LogicalDnsToEds) { // name to be reused incorrectly, which triggered an assertion failure // in the xds_cluster_impl policy caused by changing its cluster name. TEST_P(AggregateClusterTest, ReconfigEdsWhileLogicalDnsChildFails) { - ScopedExperimentalEnvVar env_var( - "GRPC_XDS_EXPERIMENTAL_ENABLE_AGGREGATE_AND_LOGICAL_DNS_CLUSTER"); CreateAndStartBackends(2); const char* kNewCluster1Name = "new_cluster_1"; const char* kNewEdsService1Name = "new_eds_service_name_1"; @@ -773,8 +725,6 @@ TEST_P(AggregateClusterTest, ReconfigEdsWhileLogicalDnsChildFails) { } TEST_P(AggregateClusterTest, MultipleClustersWithSameLocalities) { - ScopedExperimentalEnvVar env_var( - "GRPC_XDS_EXPERIMENTAL_ENABLE_AGGREGATE_AND_LOGICAL_DNS_CLUSTER"); CreateAndStartBackends(2); const char* kNewClusterName1 = "new_cluster_1"; const char* kNewEdsServiceName1 = "new_eds_service_name_1"; @@ -820,8 +770,6 @@ TEST_P(AggregateClusterTest, MultipleClustersWithSameLocalities) { } TEST_P(AggregateClusterTest, RecursionDepthJustBelowMax) { - ScopedExperimentalEnvVar env_var( - "GRPC_XDS_EXPERIMENTAL_ENABLE_AGGREGATE_AND_LOGICAL_DNS_CLUSTER"); // Populate EDS resource. CreateAndStartBackends(1); EdsResourceArgs args({{"locality0", CreateEndpointsForBackends()}}); @@ -846,8 +794,6 @@ TEST_P(AggregateClusterTest, RecursionDepthJustBelowMax) { } TEST_P(AggregateClusterTest, RecursionMaxDepth) { - ScopedExperimentalEnvVar env_var( - "GRPC_XDS_EXPERIMENTAL_ENABLE_AGGREGATE_AND_LOGICAL_DNS_CLUSTER"); // Populate EDS resource. CreateAndStartBackends(1); EdsResourceArgs args({{"locality0", CreateEndpointsForBackends()}}); @@ -875,24 +821,6 @@ TEST_P(AggregateClusterTest, RecursionMaxDepth) { ::testing::HasSubstr("aggregate cluster graph exceeds max depth")); } -// Test that CDS client should send a NACK if cluster type is AGGREGATE but -// the feature is not yet supported. -TEST_P(AggregateClusterTest, Disabled) { - auto cluster = default_cluster_; - CustomClusterType* custom_cluster = cluster.mutable_cluster_type(); - custom_cluster->set_name("envoy.clusters.aggregate"); - ClusterConfig cluster_config; - cluster_config.add_clusters("cluster1"); - cluster_config.add_clusters("cluster2"); - custom_cluster->mutable_typed_config()->PackFrom(cluster_config); - cluster.set_type(Cluster::LOGICAL_DNS); - balancer_->ads_service()->SetCdsResource(cluster); - const auto response_state = WaitForCdsNack(DEBUG_LOCATION); - ASSERT_TRUE(response_state.has_value()) << "timed out waiting for NACK"; - EXPECT_THAT(response_state->error_message, - ::testing::HasSubstr("DiscoveryType is not valid.")); -} - } // namespace } // namespace testing } // namespace grpc diff --git a/test/cpp/end2end/xds/xds_ring_hash_end2end_test.cc b/test/cpp/end2end/xds/xds_ring_hash_end2end_test.cc index 53de6f5e771..4dacd5b9092 100644 --- a/test/cpp/end2end/xds/xds_ring_hash_end2end_test.cc +++ b/test/cpp/end2end/xds/xds_ring_hash_end2end_test.cc @@ -87,8 +87,6 @@ INSTANTIATE_TEST_SUITE_P( &XdsTestType::Name); TEST_P(RingHashTest, AggregateClusterFallBackFromRingHashAtStartup) { - ScopedExperimentalEnvVar env_var( - "GRPC_XDS_EXPERIMENTAL_ENABLE_AGGREGATE_AND_LOGICAL_DNS_CLUSTER"); CreateAndStartBackends(2); const char* kNewCluster1Name = "new_cluster_1"; const char* kNewEdsService1Name = "new_eds_service_name_1"; @@ -150,8 +148,6 @@ TEST_P(RingHashTest, AggregateClusterFallBackFromRingHashAtStartup) { TEST_P(RingHashTest, AggregateClusterFallBackFromRingHashToLogicalDnsAtStartup) { - ScopedExperimentalEnvVar env_var( - "GRPC_XDS_EXPERIMENTAL_ENABLE_AGGREGATE_AND_LOGICAL_DNS_CLUSTER"); CreateAndStartBackends(1); const char* kEdsClusterName = "eds_cluster"; const char* kLogicalDNSClusterName = "logical_dns_cluster";