Skip to content
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

Partially revert #30248 #31945

Merged
merged 1 commit into from
Sep 15, 2023
Merged

Partially revert #30248 #31945

merged 1 commit into from
Sep 15, 2023

Conversation

tigrato
Copy link
Contributor

@tigrato tigrato commented Sep 15, 2023

This PR removes the ReadTimeout and WriteTimeout settings from kube/proxy.Server.

The revert is required because both settings were terminating watch streams earlier and causing several issues when parsing the long-lived data stream.

@tigrato tigrato mentioned this pull request Sep 15, 2023
@tigrato tigrato added this pull request to the merge queue Sep 15, 2023
@jentfoo
Copy link
Contributor

jentfoo commented Sep 15, 2023

Can you confirm that neither timeout is able to be set? It could be worth adding a comment for the next time we stumble on this listener.

@tigrato
Copy link
Contributor Author

tigrato commented Sep 15, 2023

Can you confirm that neither timeout is able to be set? It could be worth adding a comment for the next time we stumble on this listener.

Yes. We cannot set any of those parameters.
Added a comment and a check to prevent setting a non-zero ReadTimeout and WriteTimeout

@tigrato tigrato removed this pull request from the merge queue due to a manual request Sep 15, 2023
@tigrato tigrato force-pushed the tigrato/fix-watchers-kube branch from 17f4300 to efad2bf Compare September 15, 2023 14:14
@tigrato tigrato enabled auto-merge September 15, 2023 14:14
@tigrato tigrato added this pull request to the merge queue Sep 15, 2023
@tigrato tigrato removed this pull request from the merge queue due to a manual request Sep 15, 2023
This PR removes the `ReadTimeout` and `WriteTimeout` settings from
`kube/proxy.Server`.

The revert is required because both settings were terminating watch
streams earlier and causing several when parsing the long lived data
stream.

Signed-off-by: Tiago Silva <[email protected]>
@tigrato tigrato force-pushed the tigrato/fix-watchers-kube branch from efad2bf to baaa44e Compare September 15, 2023 14:36
@tigrato tigrato enabled auto-merge September 15, 2023 14:36
@tigrato tigrato added this pull request to the merge queue Sep 15, 2023
Merged via the queue into master with commit 85a1a3b Sep 15, 2023
@tigrato tigrato deleted the tigrato/fix-watchers-kube branch September 15, 2023 15:18
@public-teleport-github-review-bot

@tigrato See the table below for backport results.

Branch Result
branch/v14 Create PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport/branch/v14 kubernetes-access merge-for-v14 size/sm test-plan-problem Issues which have been surfaced by running the manual release test plan
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants