Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

xds: Remove v3Support environment variable #4174

Merged
merged 2 commits into from
Jan 28, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 4 additions & 8 deletions xds/internal/client/bootstrap/bootstrap.go
Original file line number Diff line number Diff line change
Expand Up @@ -258,14 +258,10 @@ func NewConfig() (*Config, error) {
return nil, fmt.Errorf("xds: Required field %q doesn't contain valid value in bootstrap %s", "xds_servers.channel_creds", jsonData["xds_servers"])
}

// We end up using v3 transport protocol version only if the following
// conditions are met:
// 1. Server supports v3, indicated by the presence of "xds_v3" in
// server_features.
// 2. Environment variable "GRPC_XDS_EXPERIMENTAL_V3_SUPPORT" is set to
// true.
// The default value of the enum type "version.TransportAPI" is v2.
if env.V3Support && serverSupportsV3 {
// We end up using v3 transport protocol version only if the server supports
// v3, indicated by the presence of "xds_v3" in server_features. The default
// value of the enum type "version.TransportAPI" is v2.
if serverSupportsV3 {
config.TransportAPI = version.TransportV3
}

Expand Down
52 changes: 6 additions & 46 deletions xds/internal/client/bootstrap/bootstrap_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -430,43 +430,10 @@ func TestNewConfigV2ProtoSuccess(t *testing.T) {
}
}

// TestNewConfigV3SupportNotEnabledOnClient verifies bootstrap functionality
// when the GRPC_XDS_EXPERIMENTAL_V3_SUPPORT environment variable is not enabled
// on the client. In this case, whether the server supports v3 or not, the
// client will end up using v2.
func TestNewConfigV3SupportNotEnabledOnClient(t *testing.T) {
origV3Support := env.V3Support
env.V3Support = false
defer func() { env.V3Support = origV3Support }()

cancel := setupBootstrapOverride(v3BootstrapFileMap)
defer cancel()

tests := []struct {
name string
wantConfig *Config
}{
{"serverDoesNotSupportsV3", nonNilCredsConfigV2},
{"serverSupportsV3", nonNilCredsConfigV2},
}

for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
testNewConfigWithFileNameEnv(t, test.name, false, test.wantConfig)
testNewConfigWithFileContentEnv(t, test.name, false, test.wantConfig)
})
}
}

// TestNewConfigV3SupportEnabledOnClient verifies bootstrap functionality when
// the GRPC_XDS_EXPERIMENTAL_V3_SUPPORT environment variable is enabled on the
// client. Here the client ends up using v2 or v3 based on what the server
// supports.
func TestNewConfigV3SupportEnabledOnClient(t *testing.T) {
origV3Support := env.V3Support
env.V3Support = true
defer func() { env.V3Support = origV3Support }()

// TestNewConfigV3Support verifies bootstrap functionality involving support for
// the xDS v3 transport protocol. Here the client ends up using v2 or v3 based
// on what the server supports.
func TestNewConfigV3Support(t *testing.T) {
cancel := setupBootstrapOverride(v3BootstrapFileMap)
defer cancel()

Expand All @@ -486,15 +453,12 @@ func TestNewConfigV3SupportEnabledOnClient(t *testing.T) {
}
}

// TestNewConfigBootstrapEnvPriority tests that the two env variables are read in correct priority
// TestNewConfigBootstrapEnvPriority tests that the two env variables are read
// in correct priority.
//
// the case where the bootstrap file
// environment variable is not set.
func TestNewConfigBootstrapEnvPriority(t *testing.T) {
origV3Support := env.V3Support
env.V3Support = true
defer func() { env.V3Support = origV3Support }()

oldFileReadFunc := bootstrapFileReadFunc
bootstrapFileReadFunc = func(filename string) ([]byte, error) {
return fileReadFromFileMap(v2BootstrapFileMap, filename)
Expand Down Expand Up @@ -702,10 +666,6 @@ func TestNewConfigWithCertificateProviders(t *testing.T) {
t.Fatalf("config parsing for plugin %q failed: %v", fakeCertProviderName, err)
}

origV3Support := env.V3Support
env.V3Support = true
defer func() { env.V3Support = origV3Support }()

cancel := setupBootstrapOverride(bootstrapFileMap)
defer cancel()

Expand Down
5 changes: 0 additions & 5 deletions xds/internal/env/env.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@ const (
//
// When both bootstrap FileName and FileContent are set, FileName is used.
BootstrapFileContentEnv = "GRPC_XDS_BOOTSTRAP_CONFIG"
xdsV3SupportEnv = "GRPC_XDS_EXPERIMENTAL_V3_SUPPORT"
circuitBreakingSupportEnv = "GRPC_XDS_EXPERIMENTAL_CIRCUIT_BREAKING"
timeoutSupportEnv = "GRPC_XDS_EXPERIMENTAL_ENABLE_TIMEOUT"
)
Expand All @@ -56,10 +55,6 @@ var (
//
// When both bootstrap FileName and FileContent are set, FileName is used.
BootstrapFileContent = os.Getenv(BootstrapFileContentEnv)
// V3Support indicates whether xDS v3 API support is enabled, which can be
// done by setting the environment variable
// "GRPC_XDS_EXPERIMENTAL_V3_SUPPORT" to "true".
V3Support = strings.EqualFold(os.Getenv(xdsV3SupportEnv), "true")
// CircuitBreakingSupport indicates whether circuit breaking support is
// enabled, which can be done by setting the environment variable
// "GRPC_XDS_EXPERIMENTAL_CIRCUIT_BREAKING" to "true".
Expand Down
6 changes: 0 additions & 6 deletions xds/internal/test/xds_server_integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,6 @@ import (
testpb "google.golang.org/grpc/test/grpc_testing"
"google.golang.org/grpc/testdata"
"google.golang.org/grpc/xds"
"google.golang.org/grpc/xds/internal/env"
"google.golang.org/grpc/xds/internal/testutils"
"google.golang.org/grpc/xds/internal/testutils/e2e"
"google.golang.org/grpc/xds/internal/version"
Expand Down Expand Up @@ -136,10 +135,6 @@ func createClientTLSCredentials(t *testing.T) credentials.TransportCredentials {
func commonSetup(t *testing.T) (*e2e.ManagementServer, string, net.Listener, func()) {
t.Helper()

// Turn on xDS V3 support.
origV3Support := env.V3Support
env.V3Support = true

// Spin up a xDS management server on a local port.
nodeID := uuid.New().String()
fs, err := e2e.StartManagementServer()
Expand Down Expand Up @@ -189,7 +184,6 @@ func commonSetup(t *testing.T) (*e2e.ManagementServer, string, net.Listener, fun
}()

return fs, nodeID, lis, func() {
env.V3Support = origV3Support
fs.Stop()
bootstrapCleanup()
server.Stop()
Expand Down