From f08962aec0ac9a0299d49a0de2d1a8a527a56870 Mon Sep 17 00:00:00 2001 From: Bernardo Tolosa Date: Mon, 31 Aug 2020 21:51:39 +0800 Subject: [PATCH] Remove context timeout for subsequent calls Signed-off-by: Bernardo Tolosa --- cmd/agent/app/reporter/grpc/builder.go | 12 +-- cmd/agent/app/reporter/grpc/builder_test.go | 110 ++++++++++---------- 2 files changed, 56 insertions(+), 66 deletions(-) diff --git a/cmd/agent/app/reporter/grpc/builder.go b/cmd/agent/app/reporter/grpc/builder.go index c702bee66db..d1a2a04ec04 100644 --- a/cmd/agent/app/reporter/grpc/builder.go +++ b/cmd/agent/app/reporter/grpc/builder.go @@ -19,12 +19,10 @@ import ( "errors" "fmt" "strings" - "time" grpc_retry "github.com/grpc-ecosystem/go-grpc-middleware/retry" "go.uber.org/zap" "google.golang.org/grpc" - "google.golang.org/grpc/connectivity" "google.golang.org/grpc/credentials" "google.golang.org/grpc/resolver" "google.golang.org/grpc/resolver/manual" @@ -101,18 +99,10 @@ func (b *ConnBuilder) CreateConnection(logger *zap.Logger) (*grpc.ClientConn, er go func(cc *grpc.ClientConn) { logger.Info("Checking connection to collector") - ctx, cancel := context.WithTimeout(context.Background(), time.Second) - defer cancel() for { s := cc.GetState() logger.Info("Agent collector connection state change", zap.String("dialTarget", dialTarget), zap.Stringer("status", s)) - if s == connectivity.Ready { - return - } - if !cc.WaitForStateChange(ctx, s) { - logger.Error("Could not get collector connection state", zap.String("error", ctx.Err().Error())) - return - } + cc.WaitForStateChange(context.Background(), s) } }(conn) diff --git a/cmd/agent/app/reporter/grpc/builder_test.go b/cmd/agent/app/reporter/grpc/builder_test.go index eac95734e14..31949754a2a 100644 --- a/cmd/agent/app/reporter/grpc/builder_test.go +++ b/cmd/agent/app/reporter/grpc/builder_test.go @@ -72,72 +72,72 @@ func TestBuilderWithCollectors(t *testing.T) { defer s1.Stop() tests := []struct { - target string - name string - hostPorts []string - checkSuffixOnly bool - notifier discovery.Notifier - discoverer discovery.Discoverer - expectedError string - checkConnectioState bool - expectedState string + target string + name string + hostPorts []string + checkSuffixOnly bool + notifier discovery.Notifier + discoverer discovery.Discoverer + expectedError string + checkConnectionState bool + expectedState string }{ { - target: "///round_robin", - name: "with roundrobin schema", - hostPorts: []string{"127.0.0.1:9876", "127.0.0.1:9877", "127.0.0.1:9878"}, - checkSuffixOnly: true, - notifier: nil, - discoverer: nil, - checkConnectioState: false, + target: "///round_robin", + name: "with roundrobin schema", + hostPorts: []string{"127.0.0.1:9876", "127.0.0.1:9877", "127.0.0.1:9878"}, + checkSuffixOnly: true, + notifier: nil, + discoverer: nil, + checkConnectionState: false, }, { - target: "127.0.0.1:9876", - name: "with single host", - hostPorts: []string{"127.0.0.1:9876"}, - checkSuffixOnly: false, - notifier: nil, - discoverer: nil, - checkConnectioState: false, + target: "127.0.0.1:9876", + name: "with single host", + hostPorts: []string{"127.0.0.1:9876"}, + checkSuffixOnly: false, + notifier: nil, + discoverer: nil, + checkConnectionState: false, }, { - target: "///round_robin", - name: "with custom resolver and fixed discoverer", - hostPorts: []string{"dns://random_stuff"}, - checkSuffixOnly: true, - notifier: noopNotifier{}, - discoverer: discovery.FixedDiscoverer{}, - checkConnectioState: false, + target: "///round_robin", + name: "with custom resolver and fixed discoverer", + hostPorts: []string{"dns://random_stuff"}, + checkSuffixOnly: true, + notifier: noopNotifier{}, + discoverer: discovery.FixedDiscoverer{}, + checkConnectionState: false, }, { - target: "", - name: "without collectorPorts and resolver", - hostPorts: nil, - checkSuffixOnly: false, - notifier: nil, - discoverer: nil, - expectedError: "at least one collector hostPort address is required when resolver is not available", - checkConnectioState: false, + target: "", + name: "without collectorPorts and resolver", + hostPorts: nil, + checkSuffixOnly: false, + notifier: nil, + discoverer: nil, + expectedError: "at least one collector hostPort address is required when resolver is not available", + checkConnectionState: false, }, { - target: addr1.String(), - name: "with collector connection status ready", - hostPorts: []string{addr1.String()}, - checkSuffixOnly: false, - notifier: nil, - discoverer: nil, - checkConnectioState: true, - expectedState: "READY", + target: addr1.String(), + name: "with collector connection status ready", + hostPorts: []string{addr1.String()}, + checkSuffixOnly: false, + notifier: nil, + discoverer: nil, + checkConnectionState: true, + expectedState: "READY", }, { - target: "random_stuff", - name: "with collector connection status failure", - hostPorts: []string{"random_stuff"}, - checkSuffixOnly: false, - notifier: nil, - discoverer: nil, - checkConnectioState: true, - expectedState: "TRANSIENT_FAILURE", + target: "random_stuff", + name: "with collector connection status failure", + hostPorts: []string{"random_stuff"}, + checkSuffixOnly: false, + notifier: nil, + discoverer: nil, + checkConnectionState: true, + expectedState: "TRANSIENT_FAILURE", }, } @@ -153,7 +153,7 @@ func TestBuilderWithCollectors(t *testing.T) { if test.expectedError == "" { require.NoError(t, err) require.NotNil(t, conn) - if test.checkConnectioState { + if test.checkConnectionState { assertConnectionState(t, conn, test.expectedState) } if test.checkSuffixOnly {