WIP: portforward: symmetric half-open connection logic for client and kubelet #37072
Conversation
k8s-github-robot
added
size/M
|
@k8s-bot gci gke e2e test this |
sttts
added
release-note-none
| func (pf *PortForwarder) getListener(protocol string, hostname string, port *ForwardedPort) (net.Listener, error) { | ||
| listener, err := net.Listen(protocol, fmt.Sprintf("%s:%d", hostname, port.Local)) | ||
| func (pf *PortForwarder) getListener(protocol string, hostname string, port *ForwardedPort) (*net.TCPListener, error) { | ||
| addr, err := net.ResolveTCPAddr(protocol, fmt.Sprintf("%s:%d", hostname, port.Local)) |
|
Jenkins GCI GCE e2e failed for commit caa7851165ceaa7ff5fda85a7ec6bd9e4e7d32e1. Full PR test history. The magic incantation to run this job again is |
|
Jenkins GCI GKE smoke e2e failed for commit caa7851165ceaa7ff5fda85a7ec6bd9e4e7d32e1. Full PR test history. The magic incantation to run this job again is |
k8s-ci-robot
added
the
cncf-cla: yes
|
Jenkins Kubemark GCE e2e failed for commit caa7851165ceaa7ff5fda85a7ec6bd9e4e7d32e1. Full PR test history. The magic incantation to run this job again is |
|
Jenkins kops AWS e2e failed for commit caa7851165ceaa7ff5fda85a7ec6bd9e4e7d32e1. Full PR test history. The magic incantation to run this job again is |
|
Jenkins GCE e2e failed for commit caa7851165ceaa7ff5fda85a7ec6bd9e4e7d32e1. Full PR test history. The magic incantation to run this job again is |
|
Jenkins GCE etcd3 e2e failed for commit caa7851165ceaa7ff5fda85a7ec6bd9e4e7d32e1. Full PR test history. The magic incantation to run this job again is |
|
Jenkins GKE smoke e2e failed for commit caa7851165ceaa7ff5fda85a7ec6bd9e4e7d32e1. Full PR test history. The magic incantation to run this job again is |
sttts
force-pushed
the
sttts-symmetric-portforward-client
branch
from
caa7851
to
7abc8b0
Compare
sttts
changed the title
|
/cc @fraenkel |
| select { | ||
| case <-copiesDone: | ||
| case err := <-localError: // catch the first error, potentially ignore the second | ||
| return err |
There was a problem hiding this comment.
If both go routines error out, copiesDone could be closed. So we could miss the error, I guess.
There was a problem hiding this comment.
We can fix that by not using defer.
There was a problem hiding this comment.
The other way to rewrite this is to expect at most 2 errors. If we see any non-nil error, we leave otherwise we wait until we see 2 nil errors.
This removes the extra go func and waitgroup.
k8s-github-robot
added
size/L
sttts
force-pushed
the
sttts-symmetric-portforward-client
branch
from
7abc8b0
to
65a5e2a
Compare
sttts
force-pushed
the
sttts-symmetric-portforward-client
branch
from
65a5e2a
to
5b0cccc
Compare
|
Jenkins unit/integration failed for commit 5b0cccc53ea63cd933d916f9692da5e8f4eb43b7. Full PR test history. The magic incantation to run this job again is |
sttts
force-pushed
the
sttts-symmetric-portforward-client
branch
from
5b0cccc
to
3ac09ba
Compare
|
Jenkins Bazel Build failed for commit 3ac09ba0ccb48374ace7bfc798490d54cdc7cc8d. Full PR test history. The magic incantation to run this job again is |
|
Jenkins GCE Node e2e failed for commit 3ac09ba0ccb48374ace7bfc798490d54cdc7cc8d. Full PR test history. The magic incantation to run this job again is |
|
Jenkins verification failed for commit 3ac09ba0ccb48374ace7bfc798490d54cdc7cc8d. Full PR test history. The magic incantation to run this job again is |
sttts
force-pushed
the
sttts-symmetric-portforward-client
branch
from
3ac09ba
to
db87e30
Compare
k8s-github-robot
added
size/XL
|
@sttts PR needs rebase |
k8s-github-robot
added
the
needs-rebase
sttts
changed the title
Automatic merge from submit-queue e2e: mark portforward tests as flaky As long as we don't get #27673 and #27680 under control, this PR marks them as `[Flaky]`. There is #37072 in flight which might improve the situation, but is blocked on socat issues right now with half-open connections. cc @kubernetes/sig-testing
|
[APPROVALNOTIFIER] Needs approval from an approver in each of these OWNERS Files: We suggest the following people: |
|
This PR hasn't been active in 90 days. Closing this PR. Please reopen if you would like to work towards merging this change, if/when the PR is ready for the next round of review. cc @ncdc @smarterclayton @sttts You can add 'keep-open' label to prevent this from happening again, or add a comment to keep it open another 90 days |
This PR adds support for half-open connections in both directions to the port-forwarding code in the client (used by
kubectl port-foward) and the container runtimes in the kubelet.The layers between those two ends of a port-forward use htpstreams (using SPDY right now, but soon also websockets: #33684) instead of raw TCP. This PR is purely about tunneling TCP in SPDY (or soon websockets).
TODO:
Fixes sttts@ed021fe#commitcomment-19876346.
This is in the context of #27680 which might be a race with closed connections somewhere. Though it seems that this very change will not fix #27680 as the former is about early client close, the later about early server close.
This change is