portfordwardtester: avoid data loss during send+close+exit #37103

sttts · 2016-11-18T17:15:40Z

Following https://blog.netherlabs.nl/articles/2009/01/18/the-ultimate-so_linger-page-or-why-is-my-tcp-not-reliable when closing the sending connection in the port-forward-tester container.

Potentially fixing #27680

This change is

sttts · 2016-11-18T17:20:17Z

we have more places in the port-forward code path which just close a connection without CloseWrite or the rst wait: kubelet, client, apiserver. All of them have a simple io.Copy loop forwarding the data.

sttts · 2016-11-18T17:20:29Z

/cc @fraenkel

fraenkel · 2016-11-18T18:29:11Z

@sttts the e2e test is hard coded to use the 1.0 image.
I am not even sure who/where the image is built and uploaded.

sttts · 2016-11-21T08:03:05Z

Note: the e2e tests are red until somebody builds and pushes the new image.

k8s-ci-robot · 2016-11-21T09:18:42Z

Jenkins GCE etcd3 e2e failed for commit 73878f7. Full PR test history.

The magic incantation to run this job again is @k8s-bot gce etcd3 e2e test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

fraenkel · 2016-11-21T18:06:15Z

test/images/port-forward-tester/portforwardtester.go

 	if err != nil {
 		fmt.Printf("Error accepting connection: %v\n", err)
 		os.Exit(1)
 	}
-	defer conn.Close()
+	defer func() {


Why bother with a defer? If we don't make it to the bottom, what happens here won't really matter in the other cases.

No real reason, only modified the existing defer.

Removed the defer. Anything else left?

Not until we see an issue.

k8s-ci-robot · 2016-11-22T07:37:29Z

Jenkins Kubemark GCE e2e failed for commit 8870154. Full PR test history.

The magic incantation to run this job again is @k8s-bot kubemark e2e test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

sttts · 2016-11-22T07:38:44Z

@k8s-bot kubemark e2e test this

k8s-ci-robot · 2016-11-29T08:49:33Z

Jenkins GCE e2e failed for commit 6b06e6234c3858e8967ec4d8cea02665fb995d1e. Full PR test history.

The magic incantation to run this job again is @k8s-bot cvm gce e2e test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

sttts · 2016-11-29T09:34:24Z

@k8s-bot kops aws e2e test this

sttts · 2016-11-29T09:54:29Z

@k8s-bot kops aws e2e test this

sttts · 2016-11-29T10:35:22Z

@k8s-bot kops aws e2e test this

sttts · 2016-11-30T07:23:14Z

1.2 image is pushed now as gcr.io/google_containers/portforwardtester:1.2.

sttts · 2016-12-02T08:10:52Z

@k8s-bot test this issue

k8s-ci-robot · 2016-12-02T08:47:23Z

Jenkins kops AWS e2e failed for commit 2bb7903. Full PR test history.

The magic incantation to run this job again is @k8s-bot kops aws e2e test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

k8s-github-robot · 2016-12-03T07:32:44Z

Automatic merge from submit-queue (batch tested with PRs 37608, 37103, 37320, 37607, 37678)

sttts · 2016-12-05T07:03:25Z

Looks like this has made it worse.

fraenkel · 2016-12-05T12:40:27Z

Don't we need an updated socat for this to work properly?

sttts · 2016-12-05T13:29:41Z

I thought that this direction should work with the old socat. I am holding back the big PR because of the socat update.

fraenkel · 2016-12-05T13:36:53Z

Yes, I forgot that this was the minor change.

k8s-cherrypick-bot · 2016-12-12T10:21:13Z

Removing label cherrypick-candidate because no release milestone was set. This is an invalid state and thus this PR is not being considered for cherry-pick to any release branch. Please add an appropriate release milestone and then re-add the label.

#38260-origin-release-1.4 Automated cherry pick of #37103 #38260

k8s-cherrypick-bot · 2016-12-15T20:39:13Z

Commit found in the "release-1.4" branch appears to be this PR. Removing the "cherrypick-candidate" label. If this is an error find help to get your PR picked.

sttts added sig/network Categorizes an issue or PR as relevant to SIG Network. area/test kind/flake Categorizes issue or PR as related to a flaky test. labels Nov 18, 2016

googlebot added the cla: yes label Nov 18, 2016

sttts force-pushed the sttts-portforwardtester-linger-on-exit branch from f38dc09 to eb3c2ef Compare November 18, 2016 17:16

sttts assigned ncdc Nov 18, 2016

sttts force-pushed the sttts-portforwardtester-linger-on-exit branch from eb3c2ef to ba818f5 Compare November 18, 2016 17:23

k8s-github-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. release-note-label-needed labels Nov 18, 2016

sttts force-pushed the sttts-portforwardtester-linger-on-exit branch from ba818f5 to 1b1c624 Compare November 18, 2016 19:04

sttts added release-note-none Denotes a PR that doesn't merit a release note. and removed release-note-label-needed labels Nov 21, 2016

sttts force-pushed the sttts-portforwardtester-linger-on-exit branch from 1b1c624 to 73878f7 Compare November 21, 2016 08:09

k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Nov 21, 2016

sttts mentioned this pull request Nov 21, 2016

[k8s.io] Port forwarding [k8s.io] With a server that expects no client request should support a client that connects, sends no data, and disconnects [Conformance] {Kubernetes e2e suite} #27673

Closed

fraenkel reviewed Nov 21, 2016

View reviewed changes

sttts force-pushed the sttts-portforwardtester-linger-on-exit branch from 350ee23 to 8870154 Compare November 22, 2016 07:35

k8s-github-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 24, 2016

sttts force-pushed the sttts-portforwardtester-linger-on-exit branch from 8870154 to 6b06e62 Compare November 29, 2016 08:41

portfordwardtester: avoid data loss during send+close+exit

2bb7903

sttts force-pushed the sttts-portforwardtester-linger-on-exit branch from 6b06e62 to bec3150 Compare November 29, 2016 08:42

sttts removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 29, 2016

sttts force-pushed the sttts-portforwardtester-linger-on-exit branch from bec3150 to 2bb7903 Compare November 30, 2016 06:59

sttts added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 30, 2016

k8s-github-robot merged commit df89a3c into kubernetes:master Dec 3, 2016

fejta mentioned this pull request Dec 6, 2016

[k8s.io] Port forwarding [k8s.io] With a server that expects a client request should support a client that connects, sends data, and disconnects [Conformance] {Kubernetes e2e suite} #27680

Closed

sttts added the cherrypick-candidate label Dec 12, 2016

k8s-cherrypick-bot removed the cherrypick-candidate label Dec 12, 2016

sttts added this to the v1.6 milestone Dec 12, 2016

sttts added the cherrypick-candidate label Dec 12, 2016

sttts mentioned this pull request Dec 14, 2016

Wait for the port to be ready before starting #38260

Merged

jessfraz modified the milestones: v1.4, v1.6 Dec 14, 2016

jessfraz added the cherry-pick-approved Indicates a cherry-pick PR into a release branch has been approved by the release branch manager. label Dec 14, 2016

jessfraz mentioned this pull request Dec 14, 2016

Automated cherry pick of #37103 #38260 #38782

Merged

jessfraz added release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed release-note-none Denotes a PR that doesn't merit a release note. labels Dec 14, 2016

jessfraz added a commit that referenced this pull request Dec 15, 2016

Merge pull request #38782 from jessfraz/automated-cherry-pick-of-#37103-

94deaac

#38260-origin-release-1.4 Automated cherry pick of #37103 #38260

k8s-cherrypick-bot removed the cherrypick-candidate label Dec 15, 2016

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

portfordwardtester: avoid data loss during send+close+exit #37103

portfordwardtester: avoid data loss during send+close+exit #37103

sttts commented Nov 18, 2016 •

edited

Loading

sttts commented Nov 18, 2016

sttts commented Nov 18, 2016

fraenkel commented Nov 18, 2016

sttts commented Nov 21, 2016

k8s-ci-robot commented Nov 21, 2016

fraenkel Nov 21, 2016

sttts Nov 21, 2016

sttts Nov 22, 2016

fraenkel Nov 22, 2016

k8s-ci-robot commented Nov 22, 2016

sttts commented Nov 22, 2016

k8s-ci-robot commented Nov 29, 2016

sttts commented Nov 29, 2016

sttts commented Nov 29, 2016

sttts commented Nov 29, 2016

sttts commented Nov 30, 2016

sttts commented Dec 2, 2016

k8s-ci-robot commented Dec 2, 2016

k8s-github-robot commented Dec 3, 2016

sttts commented Dec 5, 2016

fraenkel commented Dec 5, 2016

sttts commented Dec 5, 2016 •

edited

Loading

fraenkel commented Dec 5, 2016

k8s-cherrypick-bot commented Dec 12, 2016

k8s-cherrypick-bot commented Dec 15, 2016

portfordwardtester: avoid data loss during send+close+exit #37103

portfordwardtester: avoid data loss during send+close+exit #37103

Conversation

sttts commented Nov 18, 2016 • edited Loading

sttts commented Nov 18, 2016

sttts commented Nov 18, 2016

fraenkel commented Nov 18, 2016

sttts commented Nov 21, 2016

k8s-ci-robot commented Nov 21, 2016

fraenkel Nov 21, 2016

Choose a reason for hiding this comment

sttts Nov 21, 2016

Choose a reason for hiding this comment

sttts Nov 22, 2016

Choose a reason for hiding this comment

fraenkel Nov 22, 2016

Choose a reason for hiding this comment

k8s-ci-robot commented Nov 22, 2016

sttts commented Nov 22, 2016

k8s-ci-robot commented Nov 29, 2016

sttts commented Nov 29, 2016

sttts commented Nov 29, 2016

sttts commented Nov 29, 2016

sttts commented Nov 30, 2016

sttts commented Dec 2, 2016

k8s-ci-robot commented Dec 2, 2016

k8s-github-robot commented Dec 3, 2016

sttts commented Dec 5, 2016

fraenkel commented Dec 5, 2016

sttts commented Dec 5, 2016 • edited Loading

fraenkel commented Dec 5, 2016

k8s-cherrypick-bot commented Dec 12, 2016

k8s-cherrypick-bot commented Dec 15, 2016

sttts commented Nov 18, 2016 •

edited

Loading

sttts commented Dec 5, 2016 •

edited

Loading