From d83070ec0d9043f713b6a63e1963c593b447208c Mon Sep 17 00:00:00 2001 From: Zach Reyes <39203661+zasweq@users.noreply.github.com> Date: Fri, 30 Sep 2022 16:46:17 -0400 Subject: [PATCH] Changed Outlier Detection Env Var to default true (#5673) --- internal/envconfig/xds.go | 6 +- internal/internal.go | 16 -- test/xds/xds_client_outlier_detection_test.go | 18 --- xds/internal/balancer/balancer.go | 13 +- .../cdsbalancer/cdsbalancer_security_test.go | 15 +- .../balancer/cdsbalancer/cdsbalancer_test.go | 16 +- .../clusterresolver/clusterresolver_test.go | 9 -- .../clusterresolver/configbuilder_test.go | 153 +----------------- .../balancer/outlierdetection/balancer.go | 8 - .../outlierdetection/balancer_test.go | 8 +- .../e2e_test/outlierdetection_test.go | 4 - .../xdsresource/unmarshal_cds_test.go | 3 - 12 files changed, 27 insertions(+), 242 deletions(-) diff --git a/internal/envconfig/xds.go b/internal/envconfig/xds.go index 55aaeea8b455..af09711a3e88 100644 --- a/internal/envconfig/xds.go +++ b/internal/envconfig/xds.go @@ -84,9 +84,9 @@ var ( // "GRPC_XDS_EXPERIMENTAL_RBAC" to "false". XDSRBAC = !strings.EqualFold(os.Getenv(rbacSupportEnv), "false") // XDSOutlierDetection indicates whether outlier detection support is - // enabled, which can be enabled by setting the environment variable - // "GRPC_EXPERIMENTAL_ENABLE_OUTLIER_DETECTION" to "true". - XDSOutlierDetection = strings.EqualFold(os.Getenv(outlierDetectionSupportEnv), "true") + // enabled, which can be disabled by setting the environment variable + // "GRPC_EXPERIMENTAL_ENABLE_OUTLIER_DETECTION" to "false". + XDSOutlierDetection = !strings.EqualFold(os.Getenv(outlierDetectionSupportEnv), "false") // XDSFederation indicates whether federation support is enabled. XDSFederation = strings.EqualFold(os.Getenv(federationEnv), "true") diff --git a/internal/internal.go b/internal/internal.go index 9ce1f18ae9d6..87cacb5b808c 100644 --- a/internal/internal.go +++ b/internal/internal.go @@ -120,22 +120,6 @@ var ( // // TODO: Remove this function once the RBAC env var is removed. UnregisterRBACHTTPFilterForTesting func() - - // RegisterOutlierDetectionBalancerForTesting registers the Outlier - // Detection Balancer for testing purposes, regardless of the Outlier - // Detection environment variable. - // - // TODO: Remove this function once the Outlier Detection env var is removed. - RegisterOutlierDetectionBalancerForTesting func() - - // UnregisterOutlierDetectionBalancerForTesting unregisters the Outlier - // Detection Balancer for testing purposes. This is needed because there is - // no way to unregister the Outlier Detection Balancer after registering it - // solely for testing purposes using - // RegisterOutlierDetectionBalancerForTesting(). - // - // TODO: Remove this function once the Outlier Detection env var is removed. - UnregisterOutlierDetectionBalancerForTesting func() ) // HealthChecker defines the signature of the client-side LB channel health checking function. diff --git a/test/xds/xds_client_outlier_detection_test.go b/test/xds/xds_client_outlier_detection_test.go index b53439cf66ce..fe47bc9b828a 100644 --- a/test/xds/xds_client_outlier_detection_test.go +++ b/test/xds/xds_client_outlier_detection_test.go @@ -32,8 +32,6 @@ import ( v3routepb "github.com/envoyproxy/go-control-plane/envoy/config/route/v3" "google.golang.org/grpc" "google.golang.org/grpc/credentials/insecure" - "google.golang.org/grpc/internal" - "google.golang.org/grpc/internal/envconfig" "google.golang.org/grpc/internal/stubserver" "google.golang.org/grpc/internal/testutils/xds/e2e" testgrpc "google.golang.org/grpc/test/grpc_testing" @@ -49,14 +47,6 @@ import ( // Detection balancer. This test verifies that an RPC is able to proceed // normally with this configuration. func (s) TestOutlierDetection_NoopConfig(t *testing.T) { - oldOD := envconfig.XDSOutlierDetection - envconfig.XDSOutlierDetection = true - internal.RegisterOutlierDetectionBalancerForTesting() - defer func() { - envconfig.XDSOutlierDetection = oldOD - internal.UnregisterOutlierDetectionBalancerForTesting() - }() - managementServer, nodeID, _, resolver, cleanup1 := e2e.SetupManagementServer(t, nil) defer cleanup1() @@ -129,14 +119,6 @@ func clusterWithOutlierDetection(clusterName, edsServiceName string, secLevel e2 // Detection Balancer should eject the connection to the backend which // constantly errors, and thus RPC's should mainly go to backend 1 and 2. func (s) TestOutlierDetectionWithOutlier(t *testing.T) { - oldOD := envconfig.XDSOutlierDetection - envconfig.XDSOutlierDetection = true - internal.RegisterOutlierDetectionBalancerForTesting() - defer func() { - envconfig.XDSOutlierDetection = oldOD - internal.UnregisterOutlierDetectionBalancerForTesting() - }() - managementServer, nodeID, _, resolver, cleanup := e2e.SetupManagementServer(t, nil) defer cleanup() diff --git a/xds/internal/balancer/balancer.go b/xds/internal/balancer/balancer.go index 8d81aced2dd5..68ed789f2a4d 100644 --- a/xds/internal/balancer/balancer.go +++ b/xds/internal/balancer/balancer.go @@ -20,10 +20,11 @@ package balancer import ( - _ "google.golang.org/grpc/balancer/weightedtarget" // Register the weighted_target balancer - _ "google.golang.org/grpc/xds/internal/balancer/cdsbalancer" // Register the CDS balancer - _ "google.golang.org/grpc/xds/internal/balancer/clusterimpl" // Register the xds_cluster_impl balancer - _ "google.golang.org/grpc/xds/internal/balancer/clustermanager" // Register the xds_cluster_manager balancer - _ "google.golang.org/grpc/xds/internal/balancer/clusterresolver" // Register the xds_cluster_resolver balancer - _ "google.golang.org/grpc/xds/internal/balancer/priority" // Register the priority balancer + _ "google.golang.org/grpc/balancer/weightedtarget" // Register the weighted_target balancer + _ "google.golang.org/grpc/xds/internal/balancer/cdsbalancer" // Register the CDS balancer + _ "google.golang.org/grpc/xds/internal/balancer/clusterimpl" // Register the xds_cluster_impl balancer + _ "google.golang.org/grpc/xds/internal/balancer/clustermanager" // Register the xds_cluster_manager balancer + _ "google.golang.org/grpc/xds/internal/balancer/clusterresolver" // Register the xds_cluster_resolver balancer + _ "google.golang.org/grpc/xds/internal/balancer/outlierdetection" // Register the outlier_detection balancer + _ "google.golang.org/grpc/xds/internal/balancer/priority" // Register the priority balancer ) diff --git a/xds/internal/balancer/cdsbalancer/cdsbalancer_security_test.go b/xds/internal/balancer/cdsbalancer/cdsbalancer_security_test.go index 685e77adf463..7d5898ada83d 100644 --- a/xds/internal/balancer/cdsbalancer/cdsbalancer_security_test.go +++ b/xds/internal/balancer/cdsbalancer/cdsbalancer_security_test.go @@ -34,7 +34,6 @@ import ( "google.golang.org/grpc/internal/testutils" "google.golang.org/grpc/internal/xds/matcher" "google.golang.org/grpc/resolver" - "google.golang.org/grpc/xds/internal/balancer/outlierdetection" "google.golang.org/grpc/xds/internal/testutils/fakeclient" "google.golang.org/grpc/xds/internal/xdsclient/bootstrap" "google.golang.org/grpc/xds/internal/xdsclient/xdsresource" @@ -251,7 +250,7 @@ func (s) TestSecurityConfigWithoutXDSCreds(t *testing.T) { // returned to the CDS balancer, because we have overridden the // newChildBalancer function as part of test setup. cdsUpdate := xdsresource.ClusterUpdate{ClusterName: serviceName} - wantCCS := edsCCS(serviceName, nil, false, nil, outlierdetection.LBConfig{}) + wantCCS := edsCCS(serviceName, nil, false, nil, noopODLBCfg) ctx, ctxCancel := context.WithTimeout(context.Background(), defaultTestTimeout) defer ctxCancel() if err := invokeWatchCbAndWait(ctx, xdsC, cdsWatchInfo{cdsUpdate, nil}, wantCCS, edsB); err != nil { @@ -307,7 +306,7 @@ func (s) TestNoSecurityConfigWithXDSCreds(t *testing.T) { // newChildBalancer function as part of test setup. No security config is // passed to the CDS balancer as part of this update. cdsUpdate := xdsresource.ClusterUpdate{ClusterName: serviceName} - wantCCS := edsCCS(serviceName, nil, false, nil, outlierdetection.LBConfig{}) + wantCCS := edsCCS(serviceName, nil, false, nil, noopODLBCfg) ctx, ctxCancel := context.WithTimeout(context.Background(), defaultTestTimeout) defer ctxCancel() if err := invokeWatchCbAndWait(ctx, xdsC, cdsWatchInfo{cdsUpdate, nil}, wantCCS, edsB); err != nil { @@ -463,7 +462,7 @@ func (s) TestSecurityConfigUpdate_BadToGood(t *testing.T) { // create a new EDS balancer. The fake EDS balancer created above will be // returned to the CDS balancer, because we have overridden the // newChildBalancer function as part of test setup. - wantCCS := edsCCS(serviceName, nil, false, nil, outlierdetection.LBConfig{}) + wantCCS := edsCCS(serviceName, nil, false, nil, noopODLBCfg) if err := invokeWatchCbAndWait(ctx, xdsC, cdsWatchInfo{cdsUpdateWithGoodSecurityCfg, nil}, wantCCS, edsB); err != nil { t.Fatal(err) } @@ -497,7 +496,7 @@ func (s) TestGoodSecurityConfig(t *testing.T) { // create a new EDS balancer. The fake EDS balancer created above will be // returned to the CDS balancer, because we have overridden the // newChildBalancer function as part of test setup. - wantCCS := edsCCS(serviceName, nil, false, nil, outlierdetection.LBConfig{}) + wantCCS := edsCCS(serviceName, nil, false, nil, noopODLBCfg) ctx, ctxCancel := context.WithTimeout(context.Background(), defaultTestTimeout) defer ctxCancel() if err := invokeWatchCbAndWait(ctx, xdsC, cdsWatchInfo{cdsUpdateWithGoodSecurityCfg, nil}, wantCCS, edsB); err != nil { @@ -550,7 +549,7 @@ func (s) TestSecurityConfigUpdate_GoodToFallback(t *testing.T) { // create a new EDS balancer. The fake EDS balancer created above will be // returned to the CDS balancer, because we have overridden the // newChildBalancer function as part of test setup. - wantCCS := edsCCS(serviceName, nil, false, nil, outlierdetection.LBConfig{}) + wantCCS := edsCCS(serviceName, nil, false, nil, noopODLBCfg) ctx, ctxCancel := context.WithTimeout(context.Background(), defaultTestTimeout) defer ctxCancel() if err := invokeWatchCbAndWait(ctx, xdsC, cdsWatchInfo{cdsUpdateWithGoodSecurityCfg, nil}, wantCCS, edsB); err != nil { @@ -600,7 +599,7 @@ func (s) TestSecurityConfigUpdate_GoodToBad(t *testing.T) { // create a new EDS balancer. The fake EDS balancer created above will be // returned to the CDS balancer, because we have overridden the // newChildBalancer function as part of test setup. - wantCCS := edsCCS(serviceName, nil, false, nil, outlierdetection.LBConfig{}) + wantCCS := edsCCS(serviceName, nil, false, nil, noopODLBCfg) ctx, ctxCancel := context.WithTimeout(context.Background(), defaultTestTimeout) defer ctxCancel() if err := invokeWatchCbAndWait(ctx, xdsC, cdsWatchInfo{cdsUpdateWithGoodSecurityCfg, nil}, wantCCS, edsB); err != nil { @@ -678,7 +677,7 @@ func (s) TestSecurityConfigUpdate_GoodToGood(t *testing.T) { SubjectAltNameMatchers: testSANMatchers, }, } - wantCCS := edsCCS(serviceName, nil, false, nil, outlierdetection.LBConfig{}) + wantCCS := edsCCS(serviceName, nil, false, nil, noopODLBCfg) ctx, ctxCancel := context.WithTimeout(context.Background(), defaultTestTimeout) defer ctxCancel() if err := invokeWatchCbAndWait(ctx, xdsC, cdsWatchInfo{cdsUpdate, nil}, wantCCS, edsB); err != nil { diff --git a/xds/internal/balancer/cdsbalancer/cdsbalancer_test.go b/xds/internal/balancer/cdsbalancer/cdsbalancer_test.go index dfa47913ae29..fa94e13f442a 100644 --- a/xds/internal/balancer/cdsbalancer/cdsbalancer_test.go +++ b/xds/internal/balancer/cdsbalancer/cdsbalancer_test.go @@ -29,7 +29,6 @@ import ( "google.golang.org/grpc/balancer" "google.golang.org/grpc/connectivity" "google.golang.org/grpc/internal" - "google.golang.org/grpc/internal/envconfig" "google.golang.org/grpc/internal/grpctest" internalserviceconfig "google.golang.org/grpc/internal/serviceconfig" "google.golang.org/grpc/internal/testutils" @@ -366,14 +365,11 @@ func (s) TestUpdateClientConnStateWithSameState(t *testing.T) { // different updates and verifies that the expect ClientConnState is propagated // to the edsBalancer. func (s) TestHandleClusterUpdate(t *testing.T) { - oldOutlierDetection := envconfig.XDSOutlierDetection - envconfig.XDSOutlierDetection = true xdsC, cdsB, edsB, _, cancel := setupWithWatch(t) xdsC.SetBootstrapConfig(&bootstrap.Config{ XDSServer: defaultTestAuthorityServerConfig, }) defer func() { - envconfig.XDSOutlierDetection = oldOutlierDetection cancel() cdsB.Close() }() @@ -506,7 +502,7 @@ func (s) TestHandleClusterUpdateError(t *testing.T) { // returned to the CDS balancer, because we have overridden the // newChildBalancer function as part of test setup. cdsUpdate := xdsresource.ClusterUpdate{ClusterName: serviceName} - wantCCS := edsCCS(serviceName, nil, false, nil, outlierdetection.LBConfig{}) + wantCCS := edsCCS(serviceName, nil, false, nil, noopODLBCfg) if err := invokeWatchCbAndWait(ctx, xdsC, cdsWatchInfo{cdsUpdate, nil}, wantCCS, edsB); err != nil { t.Fatal(err) } @@ -591,7 +587,7 @@ func (s) TestResolverError(t *testing.T) { // returned to the CDS balancer, because we have overridden the // newChildBalancer function as part of test setup. cdsUpdate := xdsresource.ClusterUpdate{ClusterName: serviceName} - wantCCS := edsCCS(serviceName, nil, false, nil, outlierdetection.LBConfig{}) + wantCCS := edsCCS(serviceName, nil, false, nil, noopODLBCfg) if err := invokeWatchCbAndWait(ctx, xdsC, cdsWatchInfo{cdsUpdate, nil}, wantCCS, edsB); err != nil { t.Fatal(err) } @@ -640,7 +636,7 @@ func (s) TestUpdateSubConnState(t *testing.T) { // returned to the CDS balancer, because we have overridden the // newChildBalancer function as part of test setup. cdsUpdate := xdsresource.ClusterUpdate{ClusterName: serviceName} - wantCCS := edsCCS(serviceName, nil, false, nil, outlierdetection.LBConfig{}) + wantCCS := edsCCS(serviceName, nil, false, nil, noopODLBCfg) ctx, ctxCancel := context.WithTimeout(context.Background(), defaultTestTimeout) defer ctxCancel() if err := invokeWatchCbAndWait(ctx, xdsC, cdsWatchInfo{cdsUpdate, nil}, wantCCS, edsB); err != nil { @@ -675,7 +671,7 @@ func (s) TestCircuitBreaking(t *testing.T) { // the service's counter with the new max requests. var maxRequests uint32 = 1 cdsUpdate := xdsresource.ClusterUpdate{ClusterName: clusterName, MaxRequests: &maxRequests} - wantCCS := edsCCS(clusterName, &maxRequests, false, nil, outlierdetection.LBConfig{}) + wantCCS := edsCCS(clusterName, &maxRequests, false, nil, noopODLBCfg) ctx, ctxCancel := context.WithTimeout(context.Background(), defaultTestTimeout) defer ctxCancel() if err := invokeWatchCbAndWait(ctx, xdsC, cdsWatchInfo{cdsUpdate, nil}, wantCCS, edsB); err != nil { @@ -708,7 +704,7 @@ func (s) TestClose(t *testing.T) { // returned to the CDS balancer, because we have overridden the // newChildBalancer function as part of test setup. cdsUpdate := xdsresource.ClusterUpdate{ClusterName: serviceName} - wantCCS := edsCCS(serviceName, nil, false, nil, outlierdetection.LBConfig{}) + wantCCS := edsCCS(serviceName, nil, false, nil, noopODLBCfg) ctx, ctxCancel := context.WithTimeout(context.Background(), defaultTestTimeout) defer ctxCancel() if err := invokeWatchCbAndWait(ctx, xdsC, cdsWatchInfo{cdsUpdate, nil}, wantCCS, edsB); err != nil { @@ -779,7 +775,7 @@ func (s) TestExitIdle(t *testing.T) { // returned to the CDS balancer, because we have overridden the // newChildBalancer function as part of test setup. cdsUpdate := xdsresource.ClusterUpdate{ClusterName: serviceName} - wantCCS := edsCCS(serviceName, nil, false, nil, outlierdetection.LBConfig{}) + wantCCS := edsCCS(serviceName, nil, false, nil, noopODLBCfg) ctx, ctxCancel := context.WithTimeout(context.Background(), defaultTestTimeout) defer ctxCancel() if err := invokeWatchCbAndWait(ctx, xdsC, cdsWatchInfo{cdsUpdate, nil}, wantCCS, edsB); err != nil { diff --git a/xds/internal/balancer/clusterresolver/clusterresolver_test.go b/xds/internal/balancer/clusterresolver/clusterresolver_test.go index f6f6249d9e89..1973e1549188 100644 --- a/xds/internal/balancer/clusterresolver/clusterresolver_test.go +++ b/xds/internal/balancer/clusterresolver/clusterresolver_test.go @@ -30,8 +30,6 @@ import ( "google.golang.org/grpc/balancer/roundrobin" "google.golang.org/grpc/balancer/weightedtarget" "google.golang.org/grpc/connectivity" - "google.golang.org/grpc/internal" - "google.golang.org/grpc/internal/envconfig" "google.golang.org/grpc/internal/grpctest" internalserviceconfig "google.golang.org/grpc/internal/serviceconfig" "google.golang.org/grpc/internal/testutils" @@ -533,13 +531,6 @@ func newLBConfigWithOneEDSAndOutlierDetection(edsServiceName string, odCfg outli // Configuration sent downward should have a top level Outlier Detection Policy // for each priority. func (s) TestOutlierDetection(t *testing.T) { - oldOutlierDetection := envconfig.XDSOutlierDetection - envconfig.XDSOutlierDetection = true - internal.RegisterOutlierDetectionBalancerForTesting() - defer func() { - envconfig.XDSOutlierDetection = oldOutlierDetection - }() - edsLBCh := testutils.NewChannel() xdsC, cleanup := setup(edsLBCh) defer cleanup() diff --git a/xds/internal/balancer/clusterresolver/configbuilder_test.go b/xds/internal/balancer/clusterresolver/configbuilder_test.go index d050df11b02a..f3a830291605 100644 --- a/xds/internal/balancer/clusterresolver/configbuilder_test.go +++ b/xds/internal/balancer/clusterresolver/configbuilder_test.go @@ -31,7 +31,6 @@ import ( "google.golang.org/grpc/balancer/roundrobin" "google.golang.org/grpc/balancer/weightedroundrobin" "google.golang.org/grpc/balancer/weightedtarget" - "google.golang.org/grpc/internal/envconfig" "google.golang.org/grpc/internal/hierarchy" internalserviceconfig "google.golang.org/grpc/internal/serviceconfig" "google.golang.org/grpc/resolver" @@ -179,156 +178,10 @@ func TestBuildPriorityConfigJSON(t *testing.T) { } } +// TestBuildPriorityConfig tests the priority config generation. Each top level +// balancer per priority should be an Outlier Detection balancer, with a Cluster +// Impl Balancer as a child. func TestBuildPriorityConfig(t *testing.T) { - gotConfig, gotAddrs, _ := buildPriorityConfig([]priorityConfig{ - { - mechanism: DiscoveryMechanism{ - Cluster: testClusterName, - LoadReportingServer: testLRSServerConfig, - MaxConcurrentRequests: newUint32(testMaxRequests), - Type: DiscoveryMechanismTypeEDS, - EDSServiceName: testEDSServiceName, - }, - edsResp: xdsresource.EndpointsUpdate{ - Drops: []xdsresource.OverloadDropConfig{ - { - Category: testDropCategory, - Numerator: testDropOverMillion, - Denominator: million, - }, - }, - Localities: []xdsresource.Locality{ - testLocalitiesP0[0], - testLocalitiesP0[1], - testLocalitiesP1[0], - testLocalitiesP1[1], - }, - }, - childNameGen: newNameGenerator(0), - }, - { - mechanism: DiscoveryMechanism{ - Cluster: testClusterName2, - Type: DiscoveryMechanismTypeLogicalDNS, - }, - addresses: testAddressStrs[4], - childNameGen: newNameGenerator(1), - }, - }, nil) - - wantConfig := &priority.LBConfig{ - Children: map[string]*priority.Child{ - "priority-0-0": { - Config: &internalserviceconfig.BalancerConfig{ - Name: clusterimpl.Name, - Config: &clusterimpl.LBConfig{ - Cluster: testClusterName, - EDSServiceName: testEDSServiceName, - LoadReportingServer: testLRSServerConfig, - MaxConcurrentRequests: newUint32(testMaxRequests), - DropCategories: []clusterimpl.DropConfig{ - { - Category: testDropCategory, - RequestsPerMillion: testDropOverMillion, - }, - }, - ChildPolicy: &internalserviceconfig.BalancerConfig{ - Name: weightedtarget.Name, - Config: &weightedtarget.LBConfig{ - Targets: map[string]weightedtarget.Target{ - assertString(testLocalityIDs[0].ToString): { - Weight: 20, - ChildPolicy: &internalserviceconfig.BalancerConfig{Name: roundrobin.Name}, - }, - assertString(testLocalityIDs[1].ToString): { - Weight: 80, - ChildPolicy: &internalserviceconfig.BalancerConfig{Name: roundrobin.Name}, - }, - }, - }, - }, - }, - }, - IgnoreReresolutionRequests: true, - }, - "priority-0-1": { - Config: &internalserviceconfig.BalancerConfig{ - Name: clusterimpl.Name, - Config: &clusterimpl.LBConfig{ - Cluster: testClusterName, - EDSServiceName: testEDSServiceName, - LoadReportingServer: testLRSServerConfig, - MaxConcurrentRequests: newUint32(testMaxRequests), - DropCategories: []clusterimpl.DropConfig{ - { - Category: testDropCategory, - RequestsPerMillion: testDropOverMillion, - }, - }, - ChildPolicy: &internalserviceconfig.BalancerConfig{ - Name: weightedtarget.Name, - Config: &weightedtarget.LBConfig{ - Targets: map[string]weightedtarget.Target{ - assertString(testLocalityIDs[2].ToString): { - Weight: 20, - ChildPolicy: &internalserviceconfig.BalancerConfig{Name: roundrobin.Name}, - }, - assertString(testLocalityIDs[3].ToString): { - Weight: 80, - ChildPolicy: &internalserviceconfig.BalancerConfig{Name: roundrobin.Name}, - }, - }, - }, - }, - }, - }, - IgnoreReresolutionRequests: true, - }, - "priority-1": { - Config: &internalserviceconfig.BalancerConfig{ - Name: clusterimpl.Name, - Config: &clusterimpl.LBConfig{ - Cluster: testClusterName2, - ChildPolicy: &internalserviceconfig.BalancerConfig{Name: "pick_first"}, - }, - }, - IgnoreReresolutionRequests: false, - }, - }, - Priorities: []string{"priority-0-0", "priority-0-1", "priority-1"}, - } - wantAddrs := []resolver.Address{ - testAddrWithAttrs(testAddressStrs[0][0], nil, "priority-0-0", &testLocalityIDs[0]), - testAddrWithAttrs(testAddressStrs[0][1], nil, "priority-0-0", &testLocalityIDs[0]), - testAddrWithAttrs(testAddressStrs[1][0], nil, "priority-0-0", &testLocalityIDs[1]), - testAddrWithAttrs(testAddressStrs[1][1], nil, "priority-0-0", &testLocalityIDs[1]), - testAddrWithAttrs(testAddressStrs[2][0], nil, "priority-0-1", &testLocalityIDs[2]), - testAddrWithAttrs(testAddressStrs[2][1], nil, "priority-0-1", &testLocalityIDs[2]), - testAddrWithAttrs(testAddressStrs[3][0], nil, "priority-0-1", &testLocalityIDs[3]), - testAddrWithAttrs(testAddressStrs[3][1], nil, "priority-0-1", &testLocalityIDs[3]), - testAddrWithAttrs(testAddressStrs[4][0], nil, "priority-1", nil), - testAddrWithAttrs(testAddressStrs[4][1], nil, "priority-1", nil), - } - - if diff := cmp.Diff(gotConfig, wantConfig); diff != "" { - t.Errorf("buildPriorityConfig() diff (-got +want) %v", diff) - } - if diff := cmp.Diff(gotAddrs, wantAddrs, addrCmpOpts); diff != "" { - t.Errorf("buildPriorityConfig() diff (-got +want) %v", diff) - } -} - -// TestBuildPriorityConfigWithOutlierDetection tests the priority config -// generation with Outlier Detection toggled on. Each top level balancer per -// priority should be an Outlier Detection balancer, with a Cluster Impl -// Balancer as a child. -func TestBuildPriorityConfigWithOutlierDetection(t *testing.T) { - oldOutlierDetection := envconfig.XDSOutlierDetection - envconfig.XDSOutlierDetection = true - defer func() { - envconfig.XDSOutlierDetection = oldOutlierDetection - }() - gotConfig, _, _ := buildPriorityConfig([]priorityConfig{ { // EDS - OD config should be the top level for both of the EDS diff --git a/xds/internal/balancer/outlierdetection/balancer.go b/xds/internal/balancer/outlierdetection/balancer.go index 8e54a4a10d5e..062a8e5e48d2 100644 --- a/xds/internal/balancer/outlierdetection/balancer.go +++ b/xds/internal/balancer/outlierdetection/balancer.go @@ -33,7 +33,6 @@ import ( "google.golang.org/grpc/balancer" "google.golang.org/grpc/connectivity" - "google.golang.org/grpc/internal" "google.golang.org/grpc/internal/balancer/gracefulswitch" "google.golang.org/grpc/internal/buffer" "google.golang.org/grpc/internal/envconfig" @@ -57,13 +56,6 @@ func init() { if envconfig.XDSOutlierDetection { balancer.Register(bb{}) } - // TODO: Remove these once the Outlier Detection env var is removed. - internal.RegisterOutlierDetectionBalancerForTesting = func() { - balancer.Register(bb{}) - } - internal.UnregisterOutlierDetectionBalancerForTesting = func() { - internal.BalancerUnregister(Name) - } } type bb struct{} diff --git a/xds/internal/balancer/outlierdetection/balancer_test.go b/xds/internal/balancer/outlierdetection/balancer_test.go index 151684ac7fd4..15e85fdb4661 100644 --- a/xds/internal/balancer/outlierdetection/balancer_test.go +++ b/xds/internal/balancer/outlierdetection/balancer_test.go @@ -33,7 +33,6 @@ import ( "github.com/google/go-cmp/cmp/cmpopts" "google.golang.org/grpc/balancer" "google.golang.org/grpc/connectivity" - "google.golang.org/grpc/internal" "google.golang.org/grpc/internal/balancer/stub" "google.golang.org/grpc/internal/grpcsync" "google.golang.org/grpc/internal/grpctest" @@ -300,17 +299,13 @@ type subConnWithState struct { func setup(t *testing.T) (*outlierDetectionBalancer, *testutils.TestClientConn, func()) { t.Helper() - internal.RegisterOutlierDetectionBalancerForTesting() builder := balancer.Get(Name) if builder == nil { t.Fatalf("balancer.Get(%q) returned nil", Name) } tcc := testutils.NewTestClientConn(t) odB := builder.Build(tcc, balancer.BuildOptions{}) - return odB.(*outlierDetectionBalancer), tcc, func() { - odB.Close() - internal.UnregisterOutlierDetectionBalancerForTesting() - } + return odB.(*outlierDetectionBalancer), tcc, odB.Close } type emptyChildConfig struct { @@ -361,7 +356,6 @@ func (s) TestChildBasicOperations(t *testing.T) { }) od, tcc, _ := setup(t) - defer internal.UnregisterOutlierDetectionBalancerForTesting() // This first config update should cause a child to be built and forwarded // it's first update. diff --git a/xds/internal/balancer/outlierdetection/e2e_test/outlierdetection_test.go b/xds/internal/balancer/outlierdetection/e2e_test/outlierdetection_test.go index a1987bf98a99..75e084723fce 100644 --- a/xds/internal/balancer/outlierdetection/e2e_test/outlierdetection_test.go +++ b/xds/internal/balancer/outlierdetection/e2e_test/outlierdetection_test.go @@ -199,8 +199,6 @@ func (s) TestOutlierDetectionAlgorithmsE2E(t *testing.T) { } for _, test := range tests { t.Run(test.name, func(t *testing.T) { - internal.RegisterOutlierDetectionBalancerForTesting() - defer internal.UnregisterOutlierDetectionBalancerForTesting() addresses, cancel := setupBackends(t) defer cancel() @@ -265,8 +263,6 @@ func (s) TestOutlierDetectionAlgorithmsE2E(t *testing.T) { // requiring counting RPC's, the Outlier Detection Balancer should start // ejecting any upstreams as specified in the configuration. func (s) TestNoopConfiguration(t *testing.T) { - internal.RegisterOutlierDetectionBalancerForTesting() - defer internal.UnregisterOutlierDetectionBalancerForTesting() addresses, cancel := setupBackends(t) defer cancel() diff --git a/xds/internal/xdsclient/xdsresource/unmarshal_cds_test.go b/xds/internal/xdsclient/xdsresource/unmarshal_cds_test.go index 67e057a6c400..fa8f4d4bbcc8 100644 --- a/xds/internal/xdsclient/xdsresource/unmarshal_cds_test.go +++ b/xds/internal/xdsclient/xdsresource/unmarshal_cds_test.go @@ -1690,9 +1690,6 @@ func (s) TestUnmarshalCluster(t *testing.T) { } func (s) TestValidateClusterWithOutlierDetection(t *testing.T) { - oldOutlierDetectionSupportEnv := envconfig.XDSOutlierDetection - envconfig.XDSOutlierDetection = true - defer func() { envconfig.XDSOutlierDetection = oldOutlierDetectionSupportEnv }() odToClusterProto := func(od *v3clusterpb.OutlierDetection) *v3clusterpb.Cluster { // Cluster parsing doesn't fail with respect to fields orthogonal to // outlier detection.