xds: enable ringhash and retry by default (#4776)

Author: menghanl

.github/workflows/testing.yml

@@ -47,10 +47,6 @@ jobs:
             goversion: 1.17
             testflags: -race
 
-          - type: tests
-            goversion: 1.17
-            grpcenv: GRPC_XDS_EXPERIMENTAL_ENABLE_RETRY=true
-
           - type: extras
             goversion: 1.17
 

internal/xds/env/env.go

@@ -62,16 +62,16 @@ var (
 	// When both bootstrap FileName and FileContent are set, FileName is used.
 	BootstrapFileContent = os.Getenv(BootstrapFileContentEnv)
 	// RingHashSupport indicates whether ring hash support is enabled, which can
-	// be enabled by setting the environment variable
-	// "GRPC_XDS_EXPERIMENTAL_ENABLE_RING_HASH" to "true".
-	RingHashSupport = strings.EqualFold(os.Getenv(ringHashSupportEnv), "true")
+	// be disabled by setting the environment variable
+	// "GRPC_XDS_EXPERIMENTAL_ENABLE_RING_HASH" to "false".
+	RingHashSupport = !strings.EqualFold(os.Getenv(ringHashSupportEnv), "false")
 	// ClientSideSecuritySupport is used to control processing of security
 	// configuration on the client-side.
 	//
 	// Note that there is no env var protection for the server-side because we
 	// have a brand new API on the server-side and users explicitly need to use
 	// the new API to get security integration on the server.
-	ClientSideSecuritySupport = strings.EqualFold(os.Getenv(clientSideSecuritySupportEnv), "true")
+	ClientSideSecuritySupport = !strings.EqualFold(os.Getenv(clientSideSecuritySupportEnv), "false")
 	// AggregateAndDNSSupportEnv indicates whether processing of aggregated
 	// cluster and DNS cluster is enabled, which can be enabled by setting the
 	// environment variable
@@ -80,7 +80,7 @@ var (
 	AggregateAndDNSSupportEnv = strings.EqualFold(os.Getenv(aggregateAndDNSSupportEnv), "true")
 
 	// RetrySupport indicates whether xDS retry is enabled.
-	RetrySupport = strings.EqualFold(os.Getenv(retrySupportEnv), "true")
+	RetrySupport = !strings.EqualFold(os.Getenv(retrySupportEnv), "false")
 
 	// C2PResolverSupport indicates whether support for C2P resolver is enabled.
 	// This can be enabled by setting the environment variable
@@ -89,15 +89,3 @@ var (
 	// C2PResolverTestOnlyTrafficDirectorURI is the TD URI for testing.
 	C2PResolverTestOnlyTrafficDirectorURI = os.Getenv(c2pResolverTestOnlyTrafficDirectorURIEnv)
 )
-
-func init() {
-	// Set the env var used to control processing of security configuration on
-	// the client-side to true by default.
-	// TODO(easwars): Remove this env var completely in 1.42.x release.
-	//
-	// If the env var is set explicitly, honor it.
-	ClientSideSecuritySupport = true
-	if val, ok := os.LookupEnv(clientSideSecuritySupportEnv); ok {
-		ClientSideSecuritySupport = strings.EqualFold(val, "true")
-	}
-}

test/retry_test.go

@@ -23,7 +23,6 @@ import (
 	"fmt"
 	"io"
 	"net"
-	"os"
 	"reflect"
 	"strconv"
 	"strings"
@@ -116,69 +115,6 @@ func (s) TestRetryUnary(t *testing.T) {
 	}
 }
 
-func (s) TestRetryDisabledByDefault(t *testing.T) {
-	if strings.EqualFold(os.Getenv("GRPC_GO_RETRY"), "on") ||
-		strings.EqualFold(os.Getenv("GRPC_XDS_EXPERIMENTAL_ENABLE_RETRY"), "true") {
-		return
-	}
-	i := -1
-	ss := &stubserver.StubServer{
-		EmptyCallF: func(context.Context, *testpb.Empty) (*testpb.Empty, error) {
-			i++
-			switch i {
-			case 0:
-				return nil, status.New(codes.AlreadyExists, "retryable error").Err()
-			}
-			return &testpb.Empty{}, nil
-		},
-	}
-	if err := ss.Start([]grpc.ServerOption{}); err != nil {
-		t.Fatalf("Error starting endpoint server: %v", err)
-	}
-	defer ss.Stop()
-	ss.NewServiceConfig(`{
-    "methodConfig": [{
-      "name": [{"service": "grpc.testing.TestService"}],
-      "waitForReady": true,
-      "retryPolicy": {
-        "MaxAttempts": 4,
-        "InitialBackoff": ".01s",
-        "MaxBackoff": ".01s",
-        "BackoffMultiplier": 1.0,
-        "RetryableStatusCodes": [ "ALREADY_EXISTS" ]
-      }
-    }]}`)
-	ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second)
-	for {
-		if ctx.Err() != nil {
-			t.Fatalf("Timed out waiting for service config update")
-		}
-		if ss.CC.GetMethodConfig("/grpc.testing.TestService/EmptyCall").WaitForReady != nil {
-			break
-		}
-		time.Sleep(time.Millisecond)
-	}
-	cancel()
-
-	testCases := []struct {
-		code  codes.Code
-		count int
-	}{
-		{codes.AlreadyExists, 0},
-	}
-	for _, tc := range testCases {
-		ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
-		_, err := ss.Client.EmptyCall(ctx, &testpb.Empty{})
-		cancel()
-		if status.Code(err) != tc.code {
-			t.Fatalf("EmptyCall(_, _) = _, %v; want _, <Code() = %v>", err, tc.code)
-		}
-		if i != tc.count {
-			t.Fatalf("i = %v; want %v", i, tc.count)
-		}
-	}
-}
-
 func (s) TestRetryThrottling(t *testing.T) {
 	defer enableRetry()()
 	i := -1