From 9ecfac7f6a1324bd7e74a0e772d395e74d4b500d Mon Sep 17 00:00:00 2001 From: Danny Canter Date: Mon, 6 May 2024 02:17:35 -0700 Subject: [PATCH] Integration: Change to grpc.NewClient Fun times: In grpc 1.63 grpc.Dial and a few of the options we use (WithBlock) are deprecated in favor of the no-IO variant NewClient. The uses in the integration tests should be easy to swap however as they don't use WithBlock anyways, so that's what this change aims to do. This also removes some context.WithTimeout's as I don't see anywhere the context is actually used in Dial if you don't also specify WithBlock (and it's especially not used now with NewClient as it doesn't even take in a context). Signed-off-by: Danny Canter --- integration/main_test.go | 4 +--- integration/remote/remote_image.go | 6 +----- integration/remote/remote_runtime.go | 4 +--- integration/remote/util/util_unix.go | 11 +++++++++-- integration/remote/util/util_windows.go | 10 +++++++++- 5 files changed, 21 insertions(+), 14 deletions(-) diff --git a/integration/main_test.go b/integration/main_test.go index 31c01f72b..1a3acb0f4 100644 --- a/integration/main_test.go +++ b/integration/main_test.go @@ -656,9 +656,7 @@ func RawRuntimeClient() (runtime.RuntimeServiceClient, error) { if err != nil { return nil, fmt.Errorf("failed to get dialer: %w", err) } - ctx, cancel := context.WithTimeout(context.Background(), timeout) - defer cancel() - conn, err := grpc.DialContext(ctx, addr, + conn, err := grpc.NewClient(addr, grpc.WithTransportCredentials(insecure.NewCredentials()), grpc.WithContextDialer(dialer), ) diff --git a/integration/remote/remote_image.go b/integration/remote/remote_image.go index 6b016eb63..7b2ff6c8b 100644 --- a/integration/remote/remote_image.go +++ b/integration/remote/remote_image.go @@ -33,7 +33,6 @@ limitations under the License. package remote import ( - "context" "errors" "fmt" "time" @@ -62,10 +61,7 @@ func NewImageService(endpoint string, connectionTimeout time.Duration) (internal return nil, err } - ctx, cancel := context.WithTimeout(context.Background(), connectionTimeout) - defer cancel() - - conn, err := grpc.DialContext(ctx, addr, + conn, err := grpc.NewClient(addr, grpc.WithTransportCredentials(insecure.NewCredentials()), grpc.WithContextDialer(dialer), grpc.WithDefaultCallOptions(grpc.MaxCallRecvMsgSize(maxMsgSize)), diff --git a/integration/remote/remote_runtime.go b/integration/remote/remote_runtime.go index 0b3b89597..956ca5488 100644 --- a/integration/remote/remote_runtime.go +++ b/integration/remote/remote_runtime.go @@ -70,10 +70,8 @@ func NewRuntimeService(endpoint string, connectionTimeout time.Duration) (intern if err != nil { return nil, err } - ctx, cancel := context.WithTimeout(context.Background(), connectionTimeout) - defer cancel() - conn, err := grpc.DialContext(ctx, addr, + conn, err := grpc.NewClient(addr, grpc.WithTransportCredentials(insecure.NewCredentials()), grpc.WithContextDialer(dialer), grpc.WithDefaultCallOptions(grpc.MaxCallRecvMsgSize(maxMsgSize)), diff --git a/integration/remote/util/util_unix.go b/integration/remote/util/util_unix.go index 7136b0cb9..e333824c4 100644 --- a/integration/remote/util/util_unix.go +++ b/integration/remote/util/util_unix.go @@ -36,6 +36,7 @@ package util import ( "context" + "errors" "fmt" "net" "net/url" @@ -99,10 +100,16 @@ func GetAddressAndDialer(endpoint string) (string, func(ctx context.Context, add return "", nil, err } if protocol != unixProtocol { - return "", nil, fmt.Errorf("only support unix socket endpoint") + return "", nil, errors.New("only support unix socket endpoint") } - return addr, dial, nil + // Use passthrough as the scheme so it allows us to use our custom dialer: + // + // "grpc.Dial uses "passthrough" as the default name resolver for backward compatibility while grpc.NewClient + // uses "dns" as its default name resolver. This subtle difference is important to legacy systems that also + // specified a custom dialer and expected it to receive the target string directly." + // https://github.com/grpc/grpc-go/blob/master/Documentation/anti-patterns.md#the-wrong-way-grpcdial + return fmt.Sprintf("passthrough:///%s", addr), dial, nil } func dial(ctx context.Context, addr string) (net.Conn, error) { diff --git a/integration/remote/util/util_windows.go b/integration/remote/util/util_windows.go index 59b94dd74..7ffee9476 100644 --- a/integration/remote/util/util_windows.go +++ b/integration/remote/util/util_windows.go @@ -34,6 +34,7 @@ package util import ( "context" + "errors" "fmt" "net" "net/url" @@ -75,6 +76,13 @@ func GetAddressAndDialer(endpoint string) (string, func(ctx context.Context, add return "", nil, err } + // Use passthrough as the scheme so it allows us to use our custom dialer: + // + // "grpc.Dial uses "passthrough" as the default name resolver for backward compatibility while grpc.NewClient + // uses "dns" as its default name resolver. This subtle difference is important to legacy systems that also + // specified a custom dialer and expected it to receive the target string directly." + // https://github.com/grpc/grpc-go/blob/master/Documentation/anti-patterns.md#the-wrong-way-grpcdial + addr = fmt.Sprintf("passthrough:///%s", addr) if protocol == tcpProtocol { return addr, tcpDial, nil } @@ -83,7 +91,7 @@ func GetAddressAndDialer(endpoint string) (string, func(ctx context.Context, add return addr, npipeDial, nil } - return "", nil, fmt.Errorf("only support tcp and npipe endpoint") + return "", nil, errors.New("only support tcp and npipe endpoint") } func tcpDial(ctx context.Context, addr string) (net.Conn, error) {