konnectivity-client: add reusable gRPC tunnel#823
Open
ipochi wants to merge 5 commits into
Open
Conversation
This is a pure refactor with no behavioral change. Factor Proxy() stream setup and serve() goroutine launch out of CreateSingleUseGrpcTunnelWithContext into an internal newSingleStreamTunnel helper that takes a ProxyServiceClient and a closeFn. The helper invokes closeFn from the serve() goroutine on exit via the clientConn interface instead of unconditionally calling grpcConn.Close(). For the existing single-use path, closeFn is c.Close, preserving today's behavior: tunnelCtx still bounds both Proxy() establishment and the serve() goroutine, and the *grpc.ClientConn is still closed when serve() returns. This separation prepares for a future reusable tunnel API that will share one *grpc.ClientConn across many per-dial streams. In that path, closeFn will cancel only the per-dial stream context rather than closing the shared conn. No such caller is added in this commit. Add a closerFunc adapter so a func() error satisfies the existing clientConn interface without introducing a new abstraction. Signed-off-by: Imran Pochi <imranpochi@microsoft.com>
Add an exported DialFailureReason for failures opening a new Proxy stream on a reusable tunnel's shared gRPC connection. This is distinct from DialFailureEndpoint, which describes backend dial failures, and DialFailureContext, which describes caller-side cancellation. Signed-off-by: Imran Pochi <imranpochi@microsoft.com>
Add a purely additive ReusableTunnel type. ReusableTunnel composes Tunnel with Close, describing a tunnel whose DialContext may be called many times and whose underlying *grpc.ClientConn lifetime matches the tunnel's. Reusable tunnel contract is: - each returned net.Conn is independent - Done closes only after Close has been called and all in-flight per-dial child streams have drained - Done does not fire on remote unreachability alone; callers detect transport failure via DialContext errors and decide whether to Close and rebuild - Close blocks until drain; concurrent Close calls all observe the same post-condition, with only the first caller returning any ClientConn.Close error The existing Tunnel interface and CreateSingleUseGrpcTunnel* functions are unchanged. No implementation is added in this commit; the concrete reusableGrpcTunnel and CreateGRPCTunnel constructor follow separately. Signed-off-by: Imran Pochi <imranpochi@microsoft.com>
Add the concrete reusable tunnel implementation backing ReusableTunnel. CreateGRPCTunnel dials the proxy and returns a reusableGrpcTunnel that owns the underlying gRPC ClientConn for the tunnel's lifetime. Each DialContext call opens one Proxy stream over the shared connection. The implementation pre-validates tcp, synchronizes Close with child stream admission under a mutex, races stream establishment against the request context, returns typed failures for stream setup and closed-tunnel cases, and cancels per-dial stream contexts on inner dial errors so child serve goroutines can drain. Close is blocking-idempotent: the first caller cancels all child stream contexts, waits for them to drain, closes the underlying ClientConn, and fires Done. Concurrent callers wait for the same teardown to complete and return nil. Add an internal newReusableGrpcTunnel constructor so tests can inject a fake ProxyServiceClient and clientConn without dialing real gRPC. Signed-off-by: Imran Pochi <imranpochi@microsoft.com>
Add reusable_tunnel_test.go covering reusableGrpcTunnel and CreateGRPCTunnel via the existing fake-stream harness. A new fakeProxyClient implements client.ProxyServiceClient with hooks for preempting stream creation, customizing the next-allocated proxyServer, or substituting a fully custom ProxyService_ProxyClient. A closeTrackingConn implements clientConn for asserting close counts and propagating close errors. Purpose-built fake streams exercise lifecycle paths the existing pipe-backed harness cannot deterministically pin. Coverage targets the documented reusable tunnel contracts: shared transport reuse, per-dial isolation, requestCtx-bounded establishment, late-arriving inner tunnel tracking, unsupported-protocol short-circuit, inner.DialContext failure cleanup, dial-after-close typing, concurrent Close drain semantics, Close error attribution, Close-vs-DialContext stress, Done not firing on remote disappearance alone, typed DialFailureStreamSetup versus DialFailureEndpoint, and goleak smoke coverage. Signed-off-by: Imran Pochi <imranpochi@microsoft.com>
Contributor
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ipochi The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
the
cncf-cla: yes
k8s-ci-robot
added
approved
Contributor
|
@ipochi I fixed a bunch of problems on CI. I think if you rebase on the latest this should go better. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Add
ReusableTunnelandCreateGRPCTunnelso callers can reuse one gRPCClientConnand open oneProxystream perDialContext, instead of creating a new gRPC connectionper proxied dial.
Changes
ReusableTunnelinterface with explicitClose/Donedrain semantics.reusableGrpcTunnelimplementation backed by one shared gRPCClientConn.DialFailureStreamSetupso callers can distinguish shared-transport stream setup failures from backend dial failures.Notes
This does not change the konnectivity wire protocol, server, agent, existing
Tunnelinterface, or single-use tunnel behavior. Reconnect and rebuild policy remain caller-owned.