diff --git a/balancer/rls/internal/adaptive/adaptive_test.go b/balancer/rls/internal/adaptive/adaptive_test.go index 2205b533eec7..8b74a7cfa567 100644 --- a/balancer/rls/internal/adaptive/adaptive_test.go +++ b/balancer/rls/internal/adaptive/adaptive_test.go @@ -25,13 +25,13 @@ import ( ) // stats returns a tuple with accepts, throttles for the current time. -func (th *Throttler) stats() (int64, int64) { +func (t *Throttler) stats() (int64, int64) { now := timeNowFunc() - th.mu.Lock() - a, t := th.accepts.sum(now), th.throttles.sum(now) - th.mu.Unlock() - return a, t + t.mu.Lock() + a, th := t.accepts.sum(now), t.throttles.sum(now) + t.mu.Unlock() + return a, th } // Enums for responses. diff --git a/benchmark/benchmain/main.go b/benchmark/benchmain/main.go index 9486c65daf31..cfa4e5c2d400 100644 --- a/benchmark/benchmain/main.go +++ b/benchmark/benchmain/main.go @@ -61,7 +61,6 @@ import ( "google.golang.org/grpc" "google.golang.org/grpc/benchmark" - bm "google.golang.org/grpc/benchmark" "google.golang.org/grpc/benchmark/flags" "google.golang.org/grpc/benchmark/latency" "google.golang.org/grpc/benchmark/stats" @@ -378,11 +377,11 @@ func makeClients(bf stats.Features) ([]testgrpc.BenchmarkServiceClient, func()) })) } lis = nw.Listener(lis) - stopper := bm.StartServer(bm.ServerInfo{Type: "protobuf", Listener: lis}, sopts...) + stopper := benchmark.StartServer(benchmark.ServerInfo{Type: "protobuf", Listener: lis}, sopts...) conns := make([]*grpc.ClientConn, bf.Connections) clients := make([]testgrpc.BenchmarkServiceClient, bf.Connections) for cn := 0; cn < bf.Connections; cn++ { - conns[cn] = bm.NewClientConn("" /* target not used */, opts...) + conns[cn] = benchmark.NewClientConn("" /* target not used */, opts...) clients[cn] = testgrpc.NewBenchmarkServiceClient(conns[cn]) } @@ -430,7 +429,7 @@ func makeFuncStream(bf stats.Features) (rpcCallFunc, rpcCleanupFunc) { if bf.EnablePreloader { req = preparedMsg[cn][pos] } else { - pl := bm.NewPayload(testpb.PayloadType_COMPRESSABLE, reqSizeBytes) + pl := benchmark.NewPayload(testpb.PayloadType_COMPRESSABLE, reqSizeBytes) req = &testpb.SimpleRequest{ ResponseType: pl.Type, ResponseSize: int32(respSizeBytes), @@ -488,7 +487,7 @@ func setupStream(bf stats.Features, unconstrained bool) ([][]testgrpc.BenchmarkS } } - pl := bm.NewPayload(testpb.PayloadType_COMPRESSABLE, bf.ReqSizeBytes) + pl := benchmark.NewPayload(testpb.PayloadType_COMPRESSABLE, bf.ReqSizeBytes) req := &testpb.SimpleRequest{ ResponseType: pl.Type, ResponseSize: int32(bf.RespSizeBytes), @@ -515,13 +514,13 @@ func prepareMessages(streams [][]testgrpc.BenchmarkService_StreamingCallClient, // Makes a UnaryCall gRPC request using the given BenchmarkServiceClient and // request and response sizes. func unaryCaller(client testgrpc.BenchmarkServiceClient, reqSize, respSize int) { - if err := bm.DoUnaryCall(client, reqSize, respSize); err != nil { + if err := benchmark.DoUnaryCall(client, reqSize, respSize); err != nil { logger.Fatalf("DoUnaryCall failed: %v", err) } } func streamCaller(stream testgrpc.BenchmarkService_StreamingCallClient, req any) { - if err := bm.DoStreamingRoundTripPreloaded(stream, req); err != nil { + if err := benchmark.DoStreamingRoundTripPreloaded(stream, req); err != nil { logger.Fatalf("DoStreamingRoundTrip failed: %v", err) } } diff --git a/test/end2end_test.go b/test/end2end_test.go index 61fbc2c91869..184940ec209c 100644 --- a/test/end2end_test.go +++ b/test/end2end_test.go @@ -735,8 +735,8 @@ type nopCompressor struct { grpc.Compressor } -// NewNopCompressor creates a compressor to test the case that type is not supported. -func NewNopCompressor() grpc.Compressor { +// newNopCompressor creates a compressor to test the case that type is not supported. +func newNopCompressor() grpc.Compressor { return &nopCompressor{grpc.NewGZIPCompressor()} } @@ -748,8 +748,8 @@ type nopDecompressor struct { grpc.Decompressor } -// NewNopDecompressor creates a decompressor to test the case that type is not supported. -func NewNopDecompressor() grpc.Decompressor { +// newNopDecompressor creates a decompressor to test the case that type is not supported. +func newNopDecompressor() grpc.Decompressor { return &nopDecompressor{grpc.NewGZIPDecompressor()} } @@ -775,8 +775,8 @@ func (te *test) configDial(opts ...grpc.DialOption) ([]grpc.DialOption, string) } if te.clientNopCompression { opts = append(opts, - grpc.WithCompressor(NewNopCompressor()), - grpc.WithDecompressor(NewNopDecompressor()), + grpc.WithCompressor(newNopCompressor()), + grpc.WithDecompressor(newNopDecompressor()), ) } if te.unaryClientInt != nil { diff --git a/test/tools/go.mod b/test/tools/go.mod index 5b14c32e3574..01b2765de048 100644 --- a/test/tools/go.mod +++ b/test/tools/go.mod @@ -5,7 +5,6 @@ go 1.19 require ( github.com/client9/misspell v0.3.4 github.com/golang/protobuf v1.5.3 - golang.org/x/lint v0.0.0-20210508222113-6edffad5e616 golang.org/x/tools v0.14.0 honnef.co/go/tools v0.4.6 ) diff --git a/test/tools/go.sum b/test/tools/go.sum index 6e190414e922..81050dea623a 100644 --- a/test/tools/go.sum +++ b/test/tools/go.sum @@ -7,28 +7,15 @@ github.com/golang/protobuf v1.5.3 h1:KhyjKVUg7Usr/dYsdSqoFveMYd5ko72D+zANwlG1mmg github.com/golang/protobuf v1.5.3/go.mod h1:XVQd3VNwM+JqD3oG2Ue2ip4fOMUkwXdXDdiuN0vRsmY= github.com/google/go-cmp v0.5.5 h1:Khx7svrCpmxxtHBq5j2mp/xVjsi8hQMfNLvJFAlrGgU= github.com/google/go-cmp v0.5.5/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= -golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w= -golang.org/x/crypto v0.0.0-20191011191535-87dc89f01550/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI= golang.org/x/exp/typeparams v0.0.0-20231006140011-7918f672742d h1:NRn/Afz91uVUyEsxMp4lGGxpr5y1qz+Iko60dbkfvLQ= golang.org/x/exp/typeparams v0.0.0-20231006140011-7918f672742d/go.mod h1:AbB0pIl9nAr9wVwH+Z2ZpaocVmF5I4GyWCDIsVjR0bk= -golang.org/x/lint v0.0.0-20210508222113-6edffad5e616 h1:VLliZ0d+/avPrXXH+OakdXhpJuEoBZuwh1m2j7U6Iug= -golang.org/x/lint v0.0.0-20210508222113-6edffad5e616/go.mod h1:3xt1FjdF8hUf6vQPIChWIBhFzV8gjjsPE/fR3IyQdNY= -golang.org/x/mod v0.1.1-0.20191105210325-c90efee705ee/go.mod h1:QqPTAvyqsEbceGzBzNggFXnrqF1CaUcvgkdR5Ot7KZg= golang.org/x/mod v0.13.0 h1:I/DsJXRlw/8l/0c24sM9yb0T4z9liZTduXvdAWYiysY= golang.org/x/mod v0.13.0/go.mod h1:hTbmBsO62+eylJbnUtE2MGJUyE7QWk4xUqPFrRgJ+7c= -golang.org/x/net v0.0.0-20190404232315-eb5bcb51f2a3/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg= -golang.org/x/net v0.0.0-20190620200207-3b0461eec859/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s= -golang.org/x/sync v0.0.0-20190423024810-112230192c58/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.4.0 h1:zxkM55ReGkDlKSM+Fu41A+zmbZuaPVbGMzvvdUPznYQ= -golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= -golang.org/x/sys v0.0.0-20190412213103-97732733099d/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.13.0 h1:Af8nKPmuFypiUBjVoU9V20FiaFXOcuZI21p0ycVYYGE= golang.org/x/sys v0.13.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= -golang.org/x/tools v0.0.0-20200130002326-2f3ba24bd6e7/go.mod h1:TB2adYChydJhpapKDTa4BR/hXlZSLoq2Wpct/0txZ28= golang.org/x/tools v0.14.0 h1:jvNa2pY0M4r62jkRQ6RwEZZyPcymeL9XZMLBbV7U2nc= golang.org/x/tools v0.14.0/go.mod h1:uYBEerGOWcJyEORxN+Ek8+TT266gXkNlHdJBwexUsBg= -golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543 h1:E7g+9GITq07hpfrRu66IVDexMakfv52eLZ2CXBWiKr4= golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= google.golang.org/protobuf v1.26.0-rc.1/go.mod h1:jlhhOSvTdKEhbULTjvd4ARK9grFBp09yW+WbY/TyQbw= diff --git a/test/tools/tools.go b/test/tools/tools.go index 646a144ccca1..d585cb6eb9aa 100644 --- a/test/tools/tools.go +++ b/test/tools/tools.go @@ -28,7 +28,6 @@ package tools import ( _ "github.com/client9/misspell/cmd/misspell" _ "github.com/golang/protobuf/protoc-gen-go" - _ "golang.org/x/lint/golint" _ "golang.org/x/tools/cmd/goimports" _ "honnef.co/go/tools/cmd/staticcheck" ) diff --git a/vet.sh b/vet.sh index ee29f4381275..896dc38f5063 100755 --- a/vet.sh +++ b/vet.sh @@ -35,7 +35,6 @@ if [[ "$1" = "-install" ]]; then # Install the pinned versions as defined in module tools. pushd ./test/tools go install \ - golang.org/x/lint/golint \ golang.org/x/tools/cmd/goimports \ honnef.co/go/tools/cmd/staticcheck \ github.com/client9/misspell/cmd/misspell @@ -77,6 +76,10 @@ fi not grep 'func Test[^(]' *_test.go not grep 'func Test[^(]' test/*.go +# - Check for typos in test function names +git grep 'func (s) ' -- "*_test.go" | not grep -v 'func (s) Test' +git grep 'func [A-Z]' -- "*_test.go" | not grep -v 'func Test\|Benchmark\|Example' + # - Do not import x/net/context. not git grep -l 'x/net/context' -- "*.go" @@ -94,15 +97,14 @@ git grep -l -e 'grpclog.I' --or -e 'grpclog.W' --or -e 'grpclog.E' --or -e 'grpc not git grep "\(import \|^\s*\)\"github.com/golang/protobuf/ptypes/" -- "*.go" # - Ensure all usages of grpc_testing package are renamed when importing. -not git grep "\(import \|^\s*\)\"google.golang.org/grpc/interop/grpc_testing" -- "*.go" +not git grep "\(import \|^\s*\)\"google.golang.org/grpc/interop/grpc_testing" -- "*.go" # - Ensure all xds proto imports are renamed to *pb or *grpc. git grep '"github.com/envoyproxy/go-control-plane/envoy' -- '*.go' ':(exclude)*.pb.go' | not grep -v 'pb "\|grpc "' misspell -error . -# - gofmt, goimports, golint (with exceptions for generated code), go vet, -# go mod tidy. +# - gofmt, goimports, go vet, go mod tidy. # Perform these checks on each module inside gRPC. for MOD_FILE in $(find . -name 'go.mod'); do MOD_DIR=$(dirname ${MOD_FILE}) @@ -110,7 +112,6 @@ for MOD_FILE in $(find . -name 'go.mod'); do go vet -all ./... | fail_on_output gofmt -s -d -l . 2>&1 | fail_on_output goimports -l . 2>&1 | not grep -vE "\.pb\.go" - golint ./... 2>&1 | not grep -vE "/grpc_testing_not_regenerate/.*\.pb\.go:" go mod tidy -compat=1.19 git status --porcelain 2>&1 | fail_on_output || \ @@ -119,94 +120,73 @@ for MOD_FILE in $(find . -name 'go.mod'); do done # - Collection of static analysis checks -# -# TODO(dfawley): don't use deprecated functions in examples or first-party -# plugins. -# TODO(dfawley): enable ST1019 (duplicate imports) but allow for protobufs. SC_OUT="$(mktemp)" -staticcheck -go 1.19 -checks 'inherit,-ST1015,-ST1019,-SA1019' ./... > "${SC_OUT}" || true -# Error if anything other than deprecation warnings are printed. -not grep -v "is deprecated:.*SA1019" "${SC_OUT}" -# Only ignore the following deprecated types/fields/functions. -not grep -Fv '.CredsBundle -.HeaderMap -.Metadata is deprecated: use Attributes -.NewAddress -.NewServiceConfig -.Type is deprecated: use Attributes -BuildVersion is deprecated -balancer.ErrTransientFailure -balancer.Picker -extDesc.Filename is deprecated -github.com/golang/protobuf/jsonpb is deprecated -grpc.CallCustomCodec -grpc.Code -grpc.Compressor -grpc.CustomCodec -grpc.Decompressor -grpc.MaxMsgSize -grpc.MethodConfig -grpc.NewGZIPCompressor -grpc.NewGZIPDecompressor -grpc.RPCCompressor -grpc.RPCDecompressor -grpc.ServiceConfig -grpc.WithCompressor -grpc.WithDecompressor -grpc.WithDialer -grpc.WithMaxMsgSize -grpc.WithServiceConfig -grpc.WithTimeout -http.CloseNotifier -info.SecurityVersion -proto is deprecated -proto.InternalMessageInfo is deprecated -proto.EnumName is deprecated -proto.ErrInternalBadWireType is deprecated -proto.FileDescriptor is deprecated -proto.Marshaler is deprecated -proto.MessageType is deprecated -proto.RegisterEnum is deprecated -proto.RegisterFile is deprecated -proto.RegisterType is deprecated -proto.RegisterExtension is deprecated -proto.RegisteredExtension is deprecated -proto.RegisteredExtensions is deprecated -proto.RegisterMapType is deprecated -proto.Unmarshaler is deprecated +staticcheck -go 1.19 -checks 'all' ./... > "${SC_OUT}" || true + +# Error for anything other than checks that need exclusions. +grep -v "(ST1000)" "${SC_OUT}" | grep -v "(SA1019)" | grep -v "(ST1003)" | not grep -v "(ST1019)\|\(other import of\)" + +# Exclude underscore checks for generated code. +grep "(ST1003)" "${SC_OUT}" | not grep -v '\(.pb.go:\)\|\(code_string_test.go:\)' + +# Error for duplicate imports not including grpc protos. +grep "(ST1019)\|\(other import of\)" "${SC_OUT}" | not grep -Fv 'XXXXX PleaseIgnoreUnused +channelz/grpc_channelz_v1" +go-control-plane/envoy +grpclb/grpc_lb_v1" +health/grpc_health_v1" +interop/grpc_testing" +orca/v3" +proto/grpc_gcp" +proto/grpc_lookup_v1" +reflection/grpc_reflection_v1" +reflection/grpc_reflection_v1alpha" +XXXXX PleaseIgnoreUnused' + +# Error for any package comments not in generated code. +grep "(ST1000)" "${SC_OUT}" | not grep -v "\.pb\.go:" + +# Only ignore the following deprecated types/fields/functions and exclude +# generated code. +grep "(SA1019)" "${SC_OUT}" | not grep -Fv 'XXXXX PleaseIgnoreUnused +XXXXX Protobuf related deprecation errors: +"github.com/golang/protobuf +.pb.go: +: ptypes. +proto.RegisterType +XXXXX gRPC internal usage deprecation errors: +"google.golang.org/grpc +: grpc. +: v1alpha. +: v1alphareflectionpb. +BalancerAttributes is deprecated: +CredsBundle is deprecated: +Metadata is deprecated: use Attributes instead. +NewSubConn is deprecated: +OverrideServerName is deprecated: +RemoveSubConn is deprecated: +SecurityVersion is deprecated: Target is deprecated: Use the Target field in the BuildOptions instead. -xxx_messageInfo_ -' "${SC_OUT}" - -# - special golint on package comments. -lint_package_comment_per_package() { - # Number of files in this go package. - fileCount=$(go list -f '{{len .GoFiles}}' $1) - if [ ${fileCount} -eq 0 ]; then - return 0 - fi - # Number of package errors generated by golint. - lintPackageCommentErrorsCount=$(golint --min_confidence 0 $1 | grep -c "should have a package comment") - # golint complains about every file that's missing the package comment. If the - # number of files for this package is greater than the number of errors, there's - # at least one file with package comment, good. Otherwise, fail. - if [ ${fileCount} -le ${lintPackageCommentErrorsCount} ]; then - echo "Package $1 (with ${fileCount} files) is missing package comment" - return 1 - fi -} -lint_package_comment() { - set +ex - - count=0 - for i in $(go list ./...); do - lint_package_comment_per_package "$i" - ((count += $?)) - done - - set -ex - return $count -} -lint_package_comment +UpdateAddresses is deprecated: +UpdateSubConnState is deprecated: +balancer.ErrTransientFailure is deprecated: +grpc/reflection/v1alpha/reflection.proto +XXXXX xDS deprecated fields we support +.ExactMatch +.PrefixMatch +.SafeRegexMatch +.SuffixMatch +GetContainsMatch +GetExactMatch +GetMatchSubjectAltNames +GetPrefixMatch +GetSafeRegexMatch +GetSuffixMatch +GetTlsCertificateCertificateProviderInstance +GetValidationContextCertificateProviderInstance +XXXXX TODO: Remove the below deprecation usages: +CloseNotifier +Roots.Subjects +XXXXX PleaseIgnoreUnused' echo SUCCESS diff --git a/xds/internal/balancer/outlierdetection/callcounter_test.go b/xds/internal/balancer/outlierdetection/callcounter_test.go index 8e4f5f29b5f8..9807838e65e5 100644 --- a/xds/internal/balancer/outlierdetection/callcounter_test.go +++ b/xds/internal/balancer/outlierdetection/callcounter_test.go @@ -38,19 +38,19 @@ func (b1 *bucket) Equal(b2 *bucket) bool { return b1.numFailures == b2.numFailures } -func (cc1 *callCounter) Equal(cc2 *callCounter) bool { - if cc1 == nil && cc2 == nil { +func (cc *callCounter) Equal(cc2 *callCounter) bool { + if cc == nil && cc2 == nil { return true } - if (cc1 != nil) != (cc2 != nil) { + if (cc != nil) != (cc2 != nil) { return false } - ab1 := (*bucket)(atomic.LoadPointer(&cc1.activeBucket)) + ab1 := (*bucket)(atomic.LoadPointer(&cc.activeBucket)) ab2 := (*bucket)(atomic.LoadPointer(&cc2.activeBucket)) if !ab1.Equal(ab2) { return false } - return cc1.inactiveBucket.Equal(cc2.inactiveBucket) + return cc.inactiveBucket.Equal(cc2.inactiveBucket) } // TestClear tests that clear on the call counter clears (everything set to 0) diff --git a/xds/internal/resolver/xds_resolver_test.go b/xds/internal/resolver/xds_resolver_test.go index 71e395730180..0c898e45ff9e 100644 --- a/xds/internal/resolver/xds_resolver_test.go +++ b/xds/internal/resolver/xds_resolver_test.go @@ -67,7 +67,6 @@ import ( v3discoverypb "github.com/envoyproxy/go-control-plane/envoy/service/discovery/v3" _ "google.golang.org/grpc/xds/internal/balancer/cdsbalancer" // Register the cds LB policy - _ "google.golang.org/grpc/xds/internal/balancer/cdsbalancer" // To parse LB config _ "google.golang.org/grpc/xds/internal/httpfilter/router" // Register the router filter ) diff --git a/xds/internal/xdsclient/authority_test.go b/xds/internal/xdsclient/authority_test.go index 09a81759a1f3..8168cd203f05 100644 --- a/xds/internal/xdsclient/authority_test.go +++ b/xds/internal/xdsclient/authority_test.go @@ -31,7 +31,6 @@ import ( "google.golang.org/grpc/xds/internal" "google.golang.org/grpc/xds/internal/testutils" - xdstestutils "google.golang.org/grpc/xds/internal/testutils" "google.golang.org/grpc/xds/internal/xdsclient/bootstrap" "google.golang.org/grpc/xds/internal/xdsclient/xdsresource" "google.golang.org/grpc/xds/internal/xdsclient/xdsresource/version" @@ -66,7 +65,7 @@ func setupTest(ctx context.Context, t *testing.T, opts e2e.ManagementServerOptio } a, err := newAuthority(authorityArgs{ - serverCfg: xdstestutils.ServerConfigForAddress(t, ms.Address), + serverCfg: testutils.ServerConfigForAddress(t, ms.Address), bootstrapCfg: &bootstrap.Config{ NodeProto: &v3corepb.Node{Id: nodeID}, }, diff --git a/xds/internal/xdsclient/tests/cds_watchers_test.go b/xds/internal/xdsclient/tests/cds_watchers_test.go index 615c68da7e60..e4f8ef5cf33c 100644 --- a/xds/internal/xdsclient/tests/cds_watchers_test.go +++ b/xds/internal/xdsclient/tests/cds_watchers_test.go @@ -633,7 +633,8 @@ func (s) TestCDSWatch_ValidResponseCancelsExpiryTimerBehavior(t *testing.T) { // 2. An update to other resource should result in the invocation of the watch // callback associated with that resource. It should not result in the // invocation of the watch callback associated with the deleted resource. -func (s) TesCDSWatch_ResourceRemoved(t *testing.T) { +func (s) TestCDSWatch_ResourceRemoved(t *testing.T) { + t.Skip("Disabled; see https://github.com/grpc/grpc-go/issues/6781") mgmtServer, nodeID, bootstrapContents, _, cleanup := e2e.SetupManagementServer(t, e2e.ManagementServerOptions{}) defer cleanup()