Skip to content

Commit

Permalink
internal: merge xds/envconfig into env package
Browse files Browse the repository at this point in the history
  • Loading branch information
dfawley committed Nov 2, 2021
1 parent 51cd4df commit d89ddc1
Show file tree
Hide file tree
Showing 12 changed files with 111 additions and 106 deletions.
4 changes: 1 addition & 3 deletions internal/xds/env/env.go → internal/envconfig/xds.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,7 @@
*
*/

// Package env acts a single source of definition for all environment variables
// related to the xDS implementation in gRPC.
package env
package envconfig

import (
"os"
Expand Down
8 changes: 4 additions & 4 deletions internal/xds/bootstrap.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ import (
"os"

"google.golang.org/grpc/grpclog"
"google.golang.org/grpc/internal/xds/env"
"google.golang.org/grpc/internal/envconfig"
)

var logger = grpclog.Component("internal/xds")
Expand Down Expand Up @@ -79,11 +79,11 @@ func SetupBootstrapFile(opts BootstrapOptions) (func(), error) {
}
logger.Infof("Created bootstrap file at %q with contents: %s\n", f.Name(), bootstrapContents)

origBootstrapFileName := env.BootstrapFileName
env.BootstrapFileName = f.Name()
origBootstrapFileName := envconfig.BootstrapFileName
envconfig.BootstrapFileName = f.Name()
return func() {
os.Remove(f.Name())
env.BootstrapFileName = origBootstrapFileName
envconfig.BootstrapFileName = origBootstrapFileName
}, nil
}

Expand Down
11 changes: 6 additions & 5 deletions xds/googledirectpath/googlec2p.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,20 +29,21 @@ import (
"fmt"
"time"

v3corepb "github.com/envoyproxy/go-control-plane/envoy/config/core/v3"
"google.golang.org/grpc"
"google.golang.org/grpc/credentials/google"
"google.golang.org/grpc/grpclog"
"google.golang.org/grpc/internal/envconfig"
"google.golang.org/grpc/internal/googlecloud"
internalgrpclog "google.golang.org/grpc/internal/grpclog"
"google.golang.org/grpc/internal/grpcrand"
"google.golang.org/grpc/internal/xds/env"
"google.golang.org/grpc/resolver"
_ "google.golang.org/grpc/xds" // To register xds resolvers and balancers.
"google.golang.org/grpc/xds/internal/version"
"google.golang.org/grpc/xds/internal/xdsclient"
"google.golang.org/grpc/xds/internal/xdsclient/bootstrap"
"google.golang.org/protobuf/types/known/structpb"

v3corepb "github.com/envoyproxy/go-control-plane/envoy/config/core/v3"
)

const (
Expand Down Expand Up @@ -74,7 +75,7 @@ var (
)

func init() {
if env.C2PResolverSupport {
if envconfig.C2PResolverSupport {
resolver.Register(c2pResolverBuilder{})
}
}
Expand All @@ -98,7 +99,7 @@ func (c2pResolverBuilder) Build(t resolver.Target, cc resolver.ClientConn, opts
go func() { zoneCh <- getZone(httpReqTimeout) }()
go func() { ipv6CapableCh <- getIPv6Capable(httpReqTimeout) }()

balancerName := env.C2PResolverTestOnlyTrafficDirectorURI
balancerName := envconfig.C2PResolverTestOnlyTrafficDirectorURI
if balancerName == "" {
balancerName = tdURL
}
Expand Down Expand Up @@ -174,5 +175,5 @@ func newNode(zone string, ipv6Capable bool) *v3corepb.Node {
// direct path is enabled if this client is running on GCE, and the normal xDS
// is not used (bootstrap env vars are not set).
func runDirectPath() bool {
return env.BootstrapFileName == "" && env.BootstrapFileContent == "" && onGCE()
return envconfig.BootstrapFileName == "" && envconfig.BootstrapFileContent == "" && onGCE()
}
13 changes: 7 additions & 6 deletions xds/googledirectpath/googlec2p_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,17 +23,18 @@ import (
"testing"
"time"

v3corepb "github.com/envoyproxy/go-control-plane/envoy/config/core/v3"
"github.com/google/go-cmp/cmp"
"github.com/google/go-cmp/cmp/cmpopts"
"google.golang.org/grpc"
"google.golang.org/grpc/internal/xds/env"
"google.golang.org/grpc/internal/envconfig"
"google.golang.org/grpc/resolver"
"google.golang.org/grpc/xds/internal/version"
"google.golang.org/grpc/xds/internal/xdsclient"
"google.golang.org/grpc/xds/internal/xdsclient/bootstrap"
"google.golang.org/protobuf/testing/protocmp"
"google.golang.org/protobuf/types/known/structpb"

v3corepb "github.com/envoyproxy/go-control-plane/envoy/config/core/v3"
)

type emptyResolver struct {
Expand Down Expand Up @@ -90,7 +91,7 @@ func TestBuildWithBootstrapEnvSet(t *testing.T) {
defer replaceResolvers()()
builder := resolver.Get(c2pScheme)

for i, envP := range []*string{&env.BootstrapFileName, &env.BootstrapFileContent} {
for i, envP := range []*string{&envconfig.BootstrapFileName, &envconfig.BootstrapFileContent} {
t.Run(strconv.Itoa(i), func(t *testing.T) {
// Set bootstrap config env var.
oldEnv := *envP
Expand Down Expand Up @@ -166,10 +167,10 @@ func TestBuildXDS(t *testing.T) {
defer func() { getIPv6Capable = oldGetIPv6Capability }()

if tt.tdURI != "" {
oldURI := env.C2PResolverTestOnlyTrafficDirectorURI
env.C2PResolverTestOnlyTrafficDirectorURI = tt.tdURI
oldURI := envconfig.C2PResolverTestOnlyTrafficDirectorURI
envconfig.C2PResolverTestOnlyTrafficDirectorURI = tt.tdURI
defer func() {
env.C2PResolverTestOnlyTrafficDirectorURI = oldURI
envconfig.C2PResolverTestOnlyTrafficDirectorURI = oldURI
}()
}

Expand Down
4 changes: 2 additions & 2 deletions xds/internal/resolver/serviceconfig.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,11 @@ import (

xxhash "github.com/cespare/xxhash/v2"
"google.golang.org/grpc/codes"
"google.golang.org/grpc/internal/envconfig"
"google.golang.org/grpc/internal/grpcrand"
iresolver "google.golang.org/grpc/internal/resolver"
"google.golang.org/grpc/internal/serviceconfig"
"google.golang.org/grpc/internal/wrr"
"google.golang.org/grpc/internal/xds/env"
"google.golang.org/grpc/metadata"
"google.golang.org/grpc/status"
"google.golang.org/grpc/xds/internal/balancer/clustermanager"
Expand Down Expand Up @@ -174,7 +174,7 @@ func (cs *configSelector) SelectConfig(rpcInfo iresolver.RPCInfo) (*iresolver.RP

lbCtx := clustermanager.SetPickedCluster(rpcInfo.Context, cluster.name)
// Request Hashes are only applicable for a Ring Hash LB.
if env.RingHashSupport {
if envconfig.RingHashSupport {
lbCtx = ringhash.SetRequestHash(lbCtx, cs.generateHash(rpcInfo, rt.hashPolicies))
}

Expand Down
8 changes: 4 additions & 4 deletions xds/internal/resolver/xds_resolver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,12 +32,12 @@ import (
"google.golang.org/grpc/credentials/insecure"
xdscreds "google.golang.org/grpc/credentials/xds"
"google.golang.org/grpc/internal"
"google.golang.org/grpc/internal/envconfig"
"google.golang.org/grpc/internal/grpcrand"
"google.golang.org/grpc/internal/grpctest"
iresolver "google.golang.org/grpc/internal/resolver"
"google.golang.org/grpc/internal/testutils"
"google.golang.org/grpc/internal/wrr"
"google.golang.org/grpc/internal/xds/env"
"google.golang.org/grpc/metadata"
"google.golang.org/grpc/resolver"
"google.golang.org/grpc/serviceconfig"
Expand Down Expand Up @@ -460,9 +460,9 @@ func (s) TestXDSResolverGoodServiceUpdate(t *testing.T) {
// with a HashPolicy specifying to generate a hash. The configSelector generated should
// successfully generate a Hash.
func (s) TestXDSResolverRequestHash(t *testing.T) {
oldRH := env.RingHashSupport
env.RingHashSupport = true
defer func() { env.RingHashSupport = oldRH }()
oldRH := envconfig.RingHashSupport
envconfig.RingHashSupport = true
defer func() { envconfig.RingHashSupport = oldRH }()

xdsC := fakeclient.NewClient()
xdsR, tcc, cancel := testSetup(t, setupOpts{
Expand Down
15 changes: 8 additions & 7 deletions xds/internal/test/xds_client_affinity_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,14 +26,15 @@ import (
"fmt"
"testing"

"google.golang.org/grpc"
"google.golang.org/grpc/credentials/insecure"
"google.golang.org/grpc/internal/envconfig"
"google.golang.org/grpc/xds/internal/testutils/e2e"

v3clusterpb "github.com/envoyproxy/go-control-plane/envoy/config/cluster/v3"
v3corepb "github.com/envoyproxy/go-control-plane/envoy/config/core/v3"
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/xds/env"
testpb "google.golang.org/grpc/test/grpc_testing"
"google.golang.org/grpc/xds/internal/testutils/e2e"
)

const hashHeaderName = "session_id"
Expand Down Expand Up @@ -86,9 +87,9 @@ func ringhashCluster(clusterName, edsServiceName string) *v3clusterpb.Cluster {
// behavior in ring_hash policy.
func (s) TestClientSideAffinitySanityCheck(t *testing.T) {
defer func() func() {
old := env.RingHashSupport
env.RingHashSupport = true
return func() { env.RingHashSupport = old }
old := envconfig.RingHashSupport
envconfig.RingHashSupport = true
return func() { envconfig.RingHashSupport = old }
}()()

managementServer, nodeID, _, resolver, cleanup1 := setupManagementServer(t)
Expand Down
14 changes: 8 additions & 6 deletions xds/internal/xdsclient/bootstrap/bootstrap.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,18 +26,19 @@ import (
"fmt"
"io/ioutil"

v2corepb "github.com/envoyproxy/go-control-plane/envoy/api/v2/core"
v3corepb "github.com/envoyproxy/go-control-plane/envoy/config/core/v3"
"github.com/golang/protobuf/jsonpb"
"github.com/golang/protobuf/proto"
"google.golang.org/grpc"
"google.golang.org/grpc/credentials/google"
"google.golang.org/grpc/credentials/insecure"
"google.golang.org/grpc/credentials/tls/certprovider"
"google.golang.org/grpc/internal"
"google.golang.org/grpc/internal/envconfig"
"google.golang.org/grpc/internal/pretty"
"google.golang.org/grpc/internal/xds/env"
"google.golang.org/grpc/xds/internal/version"

v2corepb "github.com/envoyproxy/go-control-plane/envoy/api/v2/core"
v3corepb "github.com/envoyproxy/go-control-plane/envoy/config/core/v3"
)

const (
Expand Down Expand Up @@ -100,8 +101,8 @@ type xdsServer struct {
}

func bootstrapConfigFromEnvVariable() ([]byte, error) {
fName := env.BootstrapFileName
fContent := env.BootstrapFileContent
fName := envconfig.BootstrapFileName
fContent := envconfig.BootstrapFileContent

// Bootstrap file name has higher priority than bootstrap content.
if fName != "" {
Expand All @@ -119,7 +120,8 @@ func bootstrapConfigFromEnvVariable() ([]byte, error) {
return []byte(fContent), nil
}

return nil, fmt.Errorf("none of the bootstrap environment variables (%q or %q) defined", env.BootstrapFileNameEnv, env.BootstrapFileContentEnv)
return nil, fmt.Errorf("none of the bootstrap environment variables (%q or %q) defined",
envconfig.BootstrapFileNameEnv, envconfig.BootstrapFileContentEnv)
}

// NewConfig returns a new instance of Config initialized by reading the
Expand Down
45 changes: 23 additions & 22 deletions xds/internal/xdsclient/bootstrap/bootstrap_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,19 +25,20 @@ import (
"os"
"testing"

v2corepb "github.com/envoyproxy/go-control-plane/envoy/api/v2/core"
v3corepb "github.com/envoyproxy/go-control-plane/envoy/config/core/v3"
"github.com/golang/protobuf/proto"
structpb "github.com/golang/protobuf/ptypes/struct"
"github.com/google/go-cmp/cmp"

"google.golang.org/grpc"
"google.golang.org/grpc/credentials/google"
"google.golang.org/grpc/credentials/insecure"
"google.golang.org/grpc/credentials/tls/certprovider"
"google.golang.org/grpc/internal"
"google.golang.org/grpc/internal/xds/env"
"google.golang.org/grpc/internal/envconfig"
"google.golang.org/grpc/xds/internal/version"

v2corepb "github.com/envoyproxy/go-control-plane/envoy/api/v2/core"
v3corepb "github.com/envoyproxy/go-control-plane/envoy/config/core/v3"
structpb "github.com/golang/protobuf/ptypes/struct"
)

var (
Expand Down Expand Up @@ -285,9 +286,9 @@ func setupBootstrapOverride(bootstrapFileMap map[string]string) func() {
// This function overrides the bootstrap file NAME env variable, to test the
// code that reads file with the given fileName.
func testNewConfigWithFileNameEnv(t *testing.T, fileName string, wantError bool, wantConfig *Config) {
origBootstrapFileName := env.BootstrapFileName
env.BootstrapFileName = fileName
defer func() { env.BootstrapFileName = origBootstrapFileName }()
origBootstrapFileName := envconfig.BootstrapFileName
envconfig.BootstrapFileName = fileName
defer func() { envconfig.BootstrapFileName = origBootstrapFileName }()

c, err := NewConfig()
if (err != nil) != wantError {
Expand All @@ -309,9 +310,9 @@ func testNewConfigWithFileContentEnv(t *testing.T, fileName string, wantError bo
// If file reading failed, skip this test.
return
}
origBootstrapContent := env.BootstrapFileContent
env.BootstrapFileContent = string(b)
defer func() { env.BootstrapFileContent = origBootstrapContent }()
origBootstrapContent := envconfig.BootstrapFileContent
envconfig.BootstrapFileContent = string(b)
defer func() { envconfig.BootstrapFileContent = origBootstrapContent }()

c, err := NewConfig()
if (err != nil) != wantError {
Expand Down Expand Up @@ -472,35 +473,35 @@ func TestNewConfigBootstrapEnvPriority(t *testing.T) {
goodFileContent2 := v3BootstrapFileMap[goodFileName2]
goodConfig2 := nonNilCredsConfigV3

origBootstrapFileName := env.BootstrapFileName
env.BootstrapFileName = ""
defer func() { env.BootstrapFileName = origBootstrapFileName }()
origBootstrapFileName := envconfig.BootstrapFileName
envconfig.BootstrapFileName = ""
defer func() { envconfig.BootstrapFileName = origBootstrapFileName }()

origBootstrapContent := env.BootstrapFileContent
env.BootstrapFileContent = ""
defer func() { env.BootstrapFileContent = origBootstrapContent }()
origBootstrapContent := envconfig.BootstrapFileContent
envconfig.BootstrapFileContent = ""
defer func() { envconfig.BootstrapFileContent = origBootstrapContent }()

// When both env variables are empty, NewConfig should fail.
if _, err := NewConfig(); err == nil {
t.Errorf("NewConfig() returned nil error, expected to fail")
}

// When one of them is set, it should be used.
env.BootstrapFileName = goodFileName1
env.BootstrapFileContent = ""
envconfig.BootstrapFileName = goodFileName1
envconfig.BootstrapFileContent = ""
if c, err := NewConfig(); err != nil || c.compare(goodConfig1) != nil {
t.Errorf("NewConfig() = %v, %v, want: %v, %v", c, err, goodConfig1, nil)
}

env.BootstrapFileName = ""
env.BootstrapFileContent = goodFileContent2
envconfig.BootstrapFileName = ""
envconfig.BootstrapFileContent = goodFileContent2
if c, err := NewConfig(); err != nil || c.compare(goodConfig2) != nil {
t.Errorf("NewConfig() = %v, %v, want: %v, %v", c, err, goodConfig1, nil)
}

// Set both, file name should be read.
env.BootstrapFileName = goodFileName1
env.BootstrapFileContent = goodFileContent2
envconfig.BootstrapFileName = goodFileName1
envconfig.BootstrapFileContent = goodFileContent2
if c, err := NewConfig(); err != nil || c.compare(goodConfig1) != nil {
t.Errorf("NewConfig() = %v, %v, want: %v, %v", c, err, goodConfig1, nil)
}
Expand Down
Loading

0 comments on commit d89ddc1

Please sign in to comment.