-
Notifications
You must be signed in to change notification settings - Fork 73
fix(tunnel): propagate half-closed TCP connections #259
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(tunnel): propagate half-closed TCP connections #259
Conversation
This change ensures that tunneled TCP connections propagate shut downs when they occur, allowing the opposite party to be signaled appropriately. A TCP connection can be "half-closed" when `A` shuts down `A->B` by sending a FIN to `B`, and `B` responds with an ACK. The connection becomes "fully-closed" when the same procedure is followed for the `A<-B` direction. When a TCP connection is half-closed in the `A->B` direction, data may continue to flow in the `A<-B` direction. This signaling exists as a basic function of the TCP protocol, allowing either party to indicate that they are done sending data, and that their peer can finish up as they please. Data can continue to flow in the opposite direction until they are ready to shut down their side of the connection. When cloud-provider-kind (`t`) acts as a tunnel for TCP connections between `A` and `B`, we have two connections `A-t` and `t-B`, and four flows: - `A->t` and `t->B` (representing `A->B`). - `A<-t` and `t<-B` (representing `A<-B`). Prior to this change, when `A` shut down `A->t`, the tunnel would not shut down `t->B`, leaving `A-t` half-closed and `B-t` not closed in any way. The same is true for the opposite direction, as well. obviously prevents `B` from knowing After this change, when `A` shuts down `A->t`, cloud-provider-kind immediately shuts down `t->B`. The same is true when `B` shuts down `t<-B`, `A<-t` is immediately shut down. Flow directions are maintained independently from one-another. Note: this change effectively restricts tunneled connections to only supporting `udp`, `tcp`, `tcp4`, and `tcp6` protocols. As far as I can tell, docker containers will not expose other protocol types (like unix domain sockets).
Welcome @justfalter! |
Hi @justfalter. Thanks for your PR. I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
/ok-to-test Thanks, just curious, how did you find out about this problem? is envoy doing the right thing or do we also need to tune the envoy config? |
I have a grpc service hosted in a pod, and access it via cloud-provider-kind. I found that the client-side of a grpc server-stream was not getting an io.EOF when the service's pod was restarted. The client side of the server stream would remain connected to the tunnel without tearing down or erroring ... it'd just sit there, waiting for more server-provided messages to show up.
I'm guessing that envoy is probably doing things properly as it has several orders of magnitude more users that cloud-provider-kind. |
I had an incidence some months ago with envoy because of problems with half closed connections, related to this istio/istio#43297 , that is why I was asking if with current fix it works for you. We solved it setting a timeout for idle connections, but if this is working for you I prefer to keep the defaults |
/kind bug Thanks |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: aojea, justfalter The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
This change ensures that tunneled TCP connections propagate shut downs when they occur, allowing the opposite party to be signaled appropriately.
A TCP connection can be "half-closed" when
A
shuts downA->B
by sending a FIN toB
, andB
responds with an ACK. The connection becomes "fully-closed" when the same procedure is followed for theA<-B
direction. When a TCP connection is half-closed in theA->B
direction, data may continue to flow in theA<-B
direction.This signaling exists as a basic function of the TCP protocol, allowing either party to indicate that they are done sending data, and that their peer can finish up as they please. Data can continue to flow in the opposite direction until they are ready to shut down their side of the connection.
When cloud-provider-kind (
t
) acts as a tunnel for TCP connections betweenA
andB
, we have two connectionsA-t
andt-B
, and four flows:A->t
andt->B
(representingA->B
).A<-t
andt<-B
(representingA<-B
).Prior to this change, when
A
shut downA->t
, the tunnel would not shut downt->B
, leavingA-t
half-closed andB-t
not closed in any way. The same is true for the opposite direction, as well.After this change, when
A
shuts downA->t
, cloud-provider-kind immediately shuts downt->B
. The same is true whenB
shuts downt<-B
,A<-t
is immediately shut down. Flow directions are maintained independently from one-another.Note: this change effectively restricts tunneled connections to only supporting
udp
,tcp
,tcp4
, andtcp6
protocols. As far as I can tell, docker containers will not expose other protocol types (like unix domain sockets).