Skip to content

Commit d83070e

Browse files
zasweqzasweq
authored
Changed Outlier Detection Env Var to default true (#5673)
1 parent 54521b2 commit d83070e

File tree

12 files changed

+27
-242
lines changed

12 files changed

+27
-242
lines changed

‎internal/envconfig/xds.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -84,9 +84,9 @@ var (
8484
// "GRPC_XDS_EXPERIMENTAL_RBAC" to "false".
8585
XDSRBAC = !strings.EqualFold(os.Getenv(rbacSupportEnv), "false")
8686
// XDSOutlierDetection indicates whether outlier detection support is
87-
// enabled, which can be enabled by setting the environment variable
88-
// "GRPC_EXPERIMENTAL_ENABLE_OUTLIER_DETECTION" to "true".
89-
XDSOutlierDetection = strings.EqualFold(os.Getenv(outlierDetectionSupportEnv), "true")
87+
// enabled, which can be disabled by setting the environment variable
88+
// "GRPC_EXPERIMENTAL_ENABLE_OUTLIER_DETECTION" to "false".
89+
XDSOutlierDetection = !strings.EqualFold(os.Getenv(outlierDetectionSupportEnv), "false")
9090
// XDSFederation indicates whether federation support is enabled.
9191
XDSFederation = strings.EqualFold(os.Getenv(federationEnv), "true")
9292

‎internal/internal.go

Lines changed: 0 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -120,22 +120,6 @@ var (
120120
//
121121
// TODO: Remove this function once the RBAC env var is removed.
122122
UnregisterRBACHTTPFilterForTesting func()
123-
124-
// RegisterOutlierDetectionBalancerForTesting registers the Outlier
125-
// Detection Balancer for testing purposes, regardless of the Outlier
126-
// Detection environment variable.
127-
//
128-
// TODO: Remove this function once the Outlier Detection env var is removed.
129-
RegisterOutlierDetectionBalancerForTesting func()
130-
131-
// UnregisterOutlierDetectionBalancerForTesting unregisters the Outlier
132-
// Detection Balancer for testing purposes. This is needed because there is
133-
// no way to unregister the Outlier Detection Balancer after registering it
134-
// solely for testing purposes using
135-
// RegisterOutlierDetectionBalancerForTesting().
136-
//
137-
// TODO: Remove this function once the Outlier Detection env var is removed.
138-
UnregisterOutlierDetectionBalancerForTesting func()
139123
)
140124

141125
// HealthChecker defines the signature of the client-side LB channel health checking function.

‎test/xds/xds_client_outlier_detection_test.go

Lines changed: 0 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -32,8 +32,6 @@ import (
3232
v3routepb "github.com/envoyproxy/go-control-plane/envoy/config/route/v3"
3333
"google.golang.org/grpc"
3434
"google.golang.org/grpc/credentials/insecure"
35-
"google.golang.org/grpc/internal"
36-
"google.golang.org/grpc/internal/envconfig"
3735
"google.golang.org/grpc/internal/stubserver"
3836
"google.golang.org/grpc/internal/testutils/xds/e2e"
3937
testgrpc "google.golang.org/grpc/test/grpc_testing"
@@ -49,14 +47,6 @@ import (
4947
// Detection balancer. This test verifies that an RPC is able to proceed
5048
// normally with this configuration.
5149
func (s) TestOutlierDetection_NoopConfig(t *testing.T) {
52-
oldOD := envconfig.XDSOutlierDetection
53-
envconfig.XDSOutlierDetection = true
54-
internal.RegisterOutlierDetectionBalancerForTesting()
55-
defer func() {
56-
envconfig.XDSOutlierDetection = oldOD
57-
internal.UnregisterOutlierDetectionBalancerForTesting()
58-
}()
59-
6050
managementServer, nodeID, _, resolver, cleanup1 := e2e.SetupManagementServer(t, nil)
6151
defer cleanup1()
6252

@@ -129,14 +119,6 @@ func clusterWithOutlierDetection(clusterName, edsServiceName string, secLevel e2
129119
// Detection Balancer should eject the connection to the backend which
130120
// constantly errors, and thus RPC's should mainly go to backend 1 and 2.
131121
func (s) TestOutlierDetectionWithOutlier(t *testing.T) {
132-
oldOD := envconfig.XDSOutlierDetection
133-
envconfig.XDSOutlierDetection = true
134-
internal.RegisterOutlierDetectionBalancerForTesting()
135-
defer func() {
136-
envconfig.XDSOutlierDetection = oldOD
137-
internal.UnregisterOutlierDetectionBalancerForTesting()
138-
}()
139-
140122
managementServer, nodeID, _, resolver, cleanup := e2e.SetupManagementServer(t, nil)
141123
defer cleanup()
142124

‎xds/internal/balancer/balancer.go

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -20,10 +20,11 @@
2020
package balancer
2121

2222
import (
23-
_ "google.golang.org/grpc/balancer/weightedtarget" // Register the weighted_target balancer
24-
_ "google.golang.org/grpc/xds/internal/balancer/cdsbalancer" // Register the CDS balancer
25-
_ "google.golang.org/grpc/xds/internal/balancer/clusterimpl" // Register the xds_cluster_impl balancer
26-
_ "google.golang.org/grpc/xds/internal/balancer/clustermanager" // Register the xds_cluster_manager balancer
27-
_ "google.golang.org/grpc/xds/internal/balancer/clusterresolver" // Register the xds_cluster_resolver balancer
28-
_ "google.golang.org/grpc/xds/internal/balancer/priority" // Register the priority balancer
23+
_ "google.golang.org/grpc/balancer/weightedtarget" // Register the weighted_target balancer
24+
_ "google.golang.org/grpc/xds/internal/balancer/cdsbalancer" // Register the CDS balancer
25+
_ "google.golang.org/grpc/xds/internal/balancer/clusterimpl" // Register the xds_cluster_impl balancer
26+
_ "google.golang.org/grpc/xds/internal/balancer/clustermanager" // Register the xds_cluster_manager balancer
27+
_ "google.golang.org/grpc/xds/internal/balancer/clusterresolver" // Register the xds_cluster_resolver balancer
28+
_ "google.golang.org/grpc/xds/internal/balancer/outlierdetection" // Register the outlier_detection balancer
29+
_ "google.golang.org/grpc/xds/internal/balancer/priority" // Register the priority balancer
2930
)

‎xds/internal/balancer/cdsbalancer/cdsbalancer_security_test.go

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,6 @@ import (
3434
"google.golang.org/grpc/internal/testutils"
3535
"google.golang.org/grpc/internal/xds/matcher"
3636
"google.golang.org/grpc/resolver"
37-
"google.golang.org/grpc/xds/internal/balancer/outlierdetection"
3837
"google.golang.org/grpc/xds/internal/testutils/fakeclient"
3938
"google.golang.org/grpc/xds/internal/xdsclient/bootstrap"
4039
"google.golang.org/grpc/xds/internal/xdsclient/xdsresource"
@@ -251,7 +250,7 @@ func (s) TestSecurityConfigWithoutXDSCreds(t *testing.T) {
251250
// returned to the CDS balancer, because we have overridden the
252251
// newChildBalancer function as part of test setup.
253252
cdsUpdate := xdsresource.ClusterUpdate{ClusterName: serviceName}
254-
wantCCS := edsCCS(serviceName, nil, false, nil, outlierdetection.LBConfig{})
253+
wantCCS := edsCCS(serviceName, nil, false, nil, noopODLBCfg)
255254
ctx, ctxCancel := context.WithTimeout(context.Background(), defaultTestTimeout)
256255
defer ctxCancel()
257256
if err := invokeWatchCbAndWait(ctx, xdsC, cdsWatchInfo{cdsUpdate, nil}, wantCCS, edsB); err != nil {
@@ -307,7 +306,7 @@ func (s) TestNoSecurityConfigWithXDSCreds(t *testing.T) {
307306
// newChildBalancer function as part of test setup. No security config is
308307
// passed to the CDS balancer as part of this update.
309308
cdsUpdate := xdsresource.ClusterUpdate{ClusterName: serviceName}
310-
wantCCS := edsCCS(serviceName, nil, false, nil, outlierdetection.LBConfig{})
309+
wantCCS := edsCCS(serviceName, nil, false, nil, noopODLBCfg)
311310
ctx, ctxCancel := context.WithTimeout(context.Background(), defaultTestTimeout)
312311
defer ctxCancel()
313312
if err := invokeWatchCbAndWait(ctx, xdsC, cdsWatchInfo{cdsUpdate, nil}, wantCCS, edsB); err != nil {
@@ -463,7 +462,7 @@ func (s) TestSecurityConfigUpdate_BadToGood(t *testing.T) {
463462
// create a new EDS balancer. The fake EDS balancer created above will be
464463
// returned to the CDS balancer, because we have overridden the
465464
// newChildBalancer function as part of test setup.
466-
wantCCS := edsCCS(serviceName, nil, false, nil, outlierdetection.LBConfig{})
465+
wantCCS := edsCCS(serviceName, nil, false, nil, noopODLBCfg)
467466
if err := invokeWatchCbAndWait(ctx, xdsC, cdsWatchInfo{cdsUpdateWithGoodSecurityCfg, nil}, wantCCS, edsB); err != nil {
468467
t.Fatal(err)
469468
}
@@ -497,7 +496,7 @@ func (s) TestGoodSecurityConfig(t *testing.T) {
497496
// create a new EDS balancer. The fake EDS balancer created above will be
498497
// returned to the CDS balancer, because we have overridden the
499498
// newChildBalancer function as part of test setup.
500-
wantCCS := edsCCS(serviceName, nil, false, nil, outlierdetection.LBConfig{})
499+
wantCCS := edsCCS(serviceName, nil, false, nil, noopODLBCfg)
501500
ctx, ctxCancel := context.WithTimeout(context.Background(), defaultTestTimeout)
502501
defer ctxCancel()
503502
if err := invokeWatchCbAndWait(ctx, xdsC, cdsWatchInfo{cdsUpdateWithGoodSecurityCfg, nil}, wantCCS, edsB); err != nil {
@@ -550,7 +549,7 @@ func (s) TestSecurityConfigUpdate_GoodToFallback(t *testing.T) {
550549
// create a new EDS balancer. The fake EDS balancer created above will be
551550
// returned to the CDS balancer, because we have overridden the
552551
// newChildBalancer function as part of test setup.
553-
wantCCS := edsCCS(serviceName, nil, false, nil, outlierdetection.LBConfig{})
552+
wantCCS := edsCCS(serviceName, nil, false, nil, noopODLBCfg)
554553
ctx, ctxCancel := context.WithTimeout(context.Background(), defaultTestTimeout)
555554
defer ctxCancel()
556555
if err := invokeWatchCbAndWait(ctx, xdsC, cdsWatchInfo{cdsUpdateWithGoodSecurityCfg, nil}, wantCCS, edsB); err != nil {
@@ -600,7 +599,7 @@ func (s) TestSecurityConfigUpdate_GoodToBad(t *testing.T) {
600599
// create a new EDS balancer. The fake EDS balancer created above will be
601600
// returned to the CDS balancer, because we have overridden the
602601
// newChildBalancer function as part of test setup.
603-
wantCCS := edsCCS(serviceName, nil, false, nil, outlierdetection.LBConfig{})
602+
wantCCS := edsCCS(serviceName, nil, false, nil, noopODLBCfg)
604603
ctx, ctxCancel := context.WithTimeout(context.Background(), defaultTestTimeout)
605604
defer ctxCancel()
606605
if err := invokeWatchCbAndWait(ctx, xdsC, cdsWatchInfo{cdsUpdateWithGoodSecurityCfg, nil}, wantCCS, edsB); err != nil {
@@ -678,7 +677,7 @@ func (s) TestSecurityConfigUpdate_GoodToGood(t *testing.T) {
678677
SubjectAltNameMatchers: testSANMatchers,
679678
},
680679
}
681-
wantCCS := edsCCS(serviceName, nil, false, nil, outlierdetection.LBConfig{})
680+
wantCCS := edsCCS(serviceName, nil, false, nil, noopODLBCfg)
682681
ctx, ctxCancel := context.WithTimeout(context.Background(), defaultTestTimeout)
683682
defer ctxCancel()
684683
if err := invokeWatchCbAndWait(ctx, xdsC, cdsWatchInfo{cdsUpdate, nil}, wantCCS, edsB); err != nil {

‎xds/internal/balancer/cdsbalancer/cdsbalancer_test.go

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,6 @@ import (
2929
"google.golang.org/grpc/balancer"
3030
"google.golang.org/grpc/connectivity"
3131
"google.golang.org/grpc/internal"
32-
"google.golang.org/grpc/internal/envconfig"
3332
"google.golang.org/grpc/internal/grpctest"
3433
internalserviceconfig "google.golang.org/grpc/internal/serviceconfig"
3534
"google.golang.org/grpc/internal/testutils"
@@ -366,14 +365,11 @@ func (s) TestUpdateClientConnStateWithSameState(t *testing.T) {
366365
// different updates and verifies that the expect ClientConnState is propagated
367366
// to the edsBalancer.
368367
func (s) TestHandleClusterUpdate(t *testing.T) {
369-
oldOutlierDetection := envconfig.XDSOutlierDetection
370-
envconfig.XDSOutlierDetection = true
371368
xdsC, cdsB, edsB, _, cancel := setupWithWatch(t)
372369
xdsC.SetBootstrapConfig(&bootstrap.Config{
373370
XDSServer: defaultTestAuthorityServerConfig,
374371
})
375372
defer func() {
376-
envconfig.XDSOutlierDetection = oldOutlierDetection
377373
cancel()
378374
cdsB.Close()
379375
}()
@@ -506,7 +502,7 @@ func (s) TestHandleClusterUpdateError(t *testing.T) {
506502
// returned to the CDS balancer, because we have overridden the
507503
// newChildBalancer function as part of test setup.
508504
cdsUpdate := xdsresource.ClusterUpdate{ClusterName: serviceName}
509-
wantCCS := edsCCS(serviceName, nil, false, nil, outlierdetection.LBConfig{})
505+
wantCCS := edsCCS(serviceName, nil, false, nil, noopODLBCfg)
510506
if err := invokeWatchCbAndWait(ctx, xdsC, cdsWatchInfo{cdsUpdate, nil}, wantCCS, edsB); err != nil {
511507
t.Fatal(err)
512508
}
@@ -591,7 +587,7 @@ func (s) TestResolverError(t *testing.T) {
591587
// returned to the CDS balancer, because we have overridden the
592588
// newChildBalancer function as part of test setup.
593589
cdsUpdate := xdsresource.ClusterUpdate{ClusterName: serviceName}
594-
wantCCS := edsCCS(serviceName, nil, false, nil, outlierdetection.LBConfig{})
590+
wantCCS := edsCCS(serviceName, nil, false, nil, noopODLBCfg)
595591
if err := invokeWatchCbAndWait(ctx, xdsC, cdsWatchInfo{cdsUpdate, nil}, wantCCS, edsB); err != nil {
596592
t.Fatal(err)
597593
}
@@ -640,7 +636,7 @@ func (s) TestUpdateSubConnState(t *testing.T) {
640636
// returned to the CDS balancer, because we have overridden the
641637
// newChildBalancer function as part of test setup.
642638
cdsUpdate := xdsresource.ClusterUpdate{ClusterName: serviceName}
643-
wantCCS := edsCCS(serviceName, nil, false, nil, outlierdetection.LBConfig{})
639+
wantCCS := edsCCS(serviceName, nil, false, nil, noopODLBCfg)
644640
ctx, ctxCancel := context.WithTimeout(context.Background(), defaultTestTimeout)
645641
defer ctxCancel()
646642
if err := invokeWatchCbAndWait(ctx, xdsC, cdsWatchInfo{cdsUpdate, nil}, wantCCS, edsB); err != nil {
@@ -675,7 +671,7 @@ func (s) TestCircuitBreaking(t *testing.T) {
675671
// the service's counter with the new max requests.
676672
var maxRequests uint32 = 1
677673
cdsUpdate := xdsresource.ClusterUpdate{ClusterName: clusterName, MaxRequests: &maxRequests}
678-
wantCCS := edsCCS(clusterName, &maxRequests, false, nil, outlierdetection.LBConfig{})
674+
wantCCS := edsCCS(clusterName, &maxRequests, false, nil, noopODLBCfg)
679675
ctx, ctxCancel := context.WithTimeout(context.Background(), defaultTestTimeout)
680676
defer ctxCancel()
681677
if err := invokeWatchCbAndWait(ctx, xdsC, cdsWatchInfo{cdsUpdate, nil}, wantCCS, edsB); err != nil {
@@ -708,7 +704,7 @@ func (s) TestClose(t *testing.T) {
708704
// returned to the CDS balancer, because we have overridden the
709705
// newChildBalancer function as part of test setup.
710706
cdsUpdate := xdsresource.ClusterUpdate{ClusterName: serviceName}
711-
wantCCS := edsCCS(serviceName, nil, false, nil, outlierdetection.LBConfig{})
707+
wantCCS := edsCCS(serviceName, nil, false, nil, noopODLBCfg)
712708
ctx, ctxCancel := context.WithTimeout(context.Background(), defaultTestTimeout)
713709
defer ctxCancel()
714710
if err := invokeWatchCbAndWait(ctx, xdsC, cdsWatchInfo{cdsUpdate, nil}, wantCCS, edsB); err != nil {
@@ -779,7 +775,7 @@ func (s) TestExitIdle(t *testing.T) {
779775
// returned to the CDS balancer, because we have overridden the
780776
// newChildBalancer function as part of test setup.
781777
cdsUpdate := xdsresource.ClusterUpdate{ClusterName: serviceName}
782-
wantCCS := edsCCS(serviceName, nil, false, nil, outlierdetection.LBConfig{})
778+
wantCCS := edsCCS(serviceName, nil, false, nil, noopODLBCfg)
783779
ctx, ctxCancel := context.WithTimeout(context.Background(), defaultTestTimeout)
784780
defer ctxCancel()
785781
if err := invokeWatchCbAndWait(ctx, xdsC, cdsWatchInfo{cdsUpdate, nil}, wantCCS, edsB); err != nil {

‎xds/internal/balancer/clusterresolver/clusterresolver_test.go

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -30,8 +30,6 @@ import (
3030
"google.golang.org/grpc/balancer/roundrobin"
3131
"google.golang.org/grpc/balancer/weightedtarget"
3232
"google.golang.org/grpc/connectivity"
33-
"google.golang.org/grpc/internal"
34-
"google.golang.org/grpc/internal/envconfig"
3533
"google.golang.org/grpc/internal/grpctest"
3634
internalserviceconfig "google.golang.org/grpc/internal/serviceconfig"
3735
"google.golang.org/grpc/internal/testutils"
@@ -533,13 +531,6 @@ func newLBConfigWithOneEDSAndOutlierDetection(edsServiceName string, odCfg outli
533531
// Configuration sent downward should have a top level Outlier Detection Policy
534532
// for each priority.
535533
func (s) TestOutlierDetection(t *testing.T) {
536-
oldOutlierDetection := envconfig.XDSOutlierDetection
537-
envconfig.XDSOutlierDetection = true
538-
internal.RegisterOutlierDetectionBalancerForTesting()
539-
defer func() {
540-
envconfig.XDSOutlierDetection = oldOutlierDetection
541-
}()
542-
543534
edsLBCh := testutils.NewChannel()
544535
xdsC, cleanup := setup(edsLBCh)
545536
defer cleanup()

0 commit comments

Comments
 (0)