Enhance parseDialTarget Function to Support Named Pipes

[CharityKathure]

This update modifies the parseDialTarget function to support named pipes in addition to existing protocols. The function now checks for the npipe scheme, returning the appropriate network and address when a named pipe is specified. The corresponding test case for TestParseDialTarget has been updated to include scenarios for named pipe targets, ensuring correct parsing and validation of input.

Changes made:

• Updated the parseDialTarget function to handle the "npipe" scheme alongside the existing "unix" scheme.

• Added new test cases in TestParseDialTarget to validate the functionality for named pipes, enhancing test coverage for different address formats.

RELEASE NOTES: None

[linux-foundation-easycla[bot]]

• :white_check_mark:login: CharityKathure / (1f158c66608455290ea60ffbf95b84c0c2863995)
• :white_check_mark:login: CharityKathure / (1f158c66608455290ea60ffbf95b84c0c2863995, 589e4b7d8d9fc896f26e2aba0551dc89430279d0)

The committers listed above are authorized under a signed CLA.

[codecov[bot]]

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 82.01%. Comparing base (b850ea5) to head (589e4b7). Report is 15 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #7736      +/-   ##
==========================================
+ Coverage   81.96%   82.01%   +0.05%     
==========================================
  Files         362      362              
  Lines       28119    28119              
==========================================
+ Hits        23048    23063      +15     
+ Misses       3863     3854       -9     
+ Partials     1208     1202       -6     

Files with missing lines	Coverage Δ
internal/transport/http_util.go	93.30% <100.00%> (ø)

... and 13 files with indirect coverage changes

[easwars]

Could you please describe your use-case a bit more?

This PR only changes the code that is executed when a connection is attempted to a resolved backend address. If your intention was to add support for a grpc client channel to be created to a server whose target URI is a windows named pipe, then these changes will not be sufficient.

So, it would help if we can have a discussion about what your use-case is and take it from there. Thanks.

[CharityKathure]

@easwars Thanks for the response. The primary goal of this PR is to ensure proper handling of Windows named pipes (npipe) in the parseDialTarget function.

The use case: When Docker starts containerd, the Docker daemon uses a named pipe (npipe:////./pipe/docker-containerd) to communicate with containerd on Windows. The issue arises when Docker attempts to ping this named pipe after some idle time. The parseDialTarget function, which is used during this ping operation, was defaulting to a TCP network for the npipe address. As a result, the dialer expected a TCP port, leading to errors such as:

Err: connection error: desc = "transport: Error while dialing: dial tcp: lookup tcp/////./pipe/docker-containerd: unknown port" library=grpc

To address this, I modified the function to treat npipe similarly to unix, ensuring that when a named pipe address is passed, the correct network is identified (i.e., npipe instead of tcp). This change fixes the issue by preventing TCP from being mistakenly selected for npipe addresses.

[easwars]

Can you please clarify where and how gRPC is used in this communication between Docker and containerd?

The issue arises when Docker attempts to ping this named pipe after some idle time

What sort of ping are you referring to here?

[CharityKathure]

@easwars In this context, Docker starts containerd, and a new client connection to containerd is established using gRPC.

The client is configured to connect to an address (r.GRPC.Address), which is a named pipe, and periodic checks are performed to ensure containerd is serving correctly by calling client.IsServing in containerd. This invokes the Check method in gRPC, to perform a health check.

func (c *healthClient) Check(ctx context.Context, in *HealthCheckRequest, opts ...grpc.CallOption) (*HealthCheckResponse, error) {
  // ...
}

However, you're correct—modifying parseDialTarget alone won’t fully resolve the problem. While the function now correctly identifies the npipe network instead of defaulting to TCP, there’s still an issue with the gRPC dialer not recognizing npipe, which results in the following error:

latest balancer error: connection error: desc = "transport: Error while dialing: dial npipe: unknown network npipe"

[easwars]

Yes, if gRPC has to fully and correctly support npipes, support for a new name resolution scheme has to added. We have two options:

• You could implement your own custom name resolver using the name resolver API found here: https://github.com/grpc/grpc-go/blob/8212cf0376831ce8b88b824128f113ae6e90b4c8/resolver/resolver.go#L314
  • You can find an example for the same here: https://github.com/grpc/grpc-go/blob/8212cf0376831ce8b88b824128f113ae6e90b4c8/examples/features/name_resolving/client/main.go#L87
  • We wouldn't be able to merge this into grpc-go though, since we would want to be consistent with other grpc language implementations. But you could maintain your custom name resolver implementation and you should be able to use it with grpc-go.
• If you require cross language support, I'd recommend filing an issue in our proposal repository here: https://github.com/grpc/proposal

Please let us know if you have more questions. Thanks.

[easwars]

I'll close this PR at this point. But feel free to file an issue if you'd like to discuss anything further.