Skip to content

UPSTREAM: <drop>: enable TCP_USER_TIMEOUT on client connections#457

Closed
p0lyn0mial wants to merge 2 commits intoopenshift:masterfrom
p0lyn0mial:tcp-usr-timeout-dialer
Closed

UPSTREAM: <drop>: enable TCP_USER_TIMEOUT on client connections#457
p0lyn0mial wants to merge 2 commits intoopenshift:masterfrom
p0lyn0mial:tcp-usr-timeout-dialer

Conversation

@p0lyn0mial
Copy link

@p0lyn0mial p0lyn0mial commented Nov 16, 2020 •
edited
Loading

we will carry a patch that allows the clients (Kubelet, KCM, etc.) to detect broken connections to the api server more quickly than the default TCP timeout of 15 minutes.

note: this patch will be dropped in future releases once http2/ping is introduced.

the first commit uses openshift/library-go#926

the second commit was created as follows:
./hack/pin-dependency.sh github.com/openshift/library-go 093ad3cf66000cb994f8c8010da43a71ba147671 followed by ./hack/update-vendor.sh

@openshift-ci-robot openshift-ci-robot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. vendor-update Touching vendor dir or related files labels Nov 16, 2020
@p0lyn0mial p0lyn0mial force-pushed the tcp-usr-timeout-dialer branch from 1b07f04 to c5fe59b Compare November 16, 2020 11:19
@openshift-ci-robot openshift-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 16, 2020
@p0lyn0mial
Copy link
Author

p0lyn0mial commented Nov 16, 2020

/assign @sttts

@sttts
Copy link

sttts commented Nov 16, 2020

/lgtm
/approve

@openshift-ci-robot openshift-ci-robot added lgtm Indicates that a PR is ready to be merged. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Nov 16, 2020
@p0lyn0mial p0lyn0mial force-pushed the tcp-usr-timeout-dialer branch from c5fe59b to 7a3d918 Compare November 16, 2020 11:41
@openshift-ci-robot openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Nov 16, 2020
@p0lyn0mial
Copy link
Author

p0lyn0mial commented Nov 16, 2020

/retest

@p0lyn0mial p0lyn0mial force-pushed the tcp-usr-timeout-dialer branch from 7a3d918 to fc5cdaf Compare November 16, 2020 16:17
@p0lyn0mial
UPSTREAM: <drop>: enable TCP_USER_TIMEOUT on client connections
d516078 
it allows the client to detect broken connections to the api server more quickly than the default TCP timeout of 15 minutes.

note that it will be replaced by an HTTP ping mechanism in the future releases.
@p0lyn0mial
UPSTREAM: <drop>: bump(*)
7009c5a
@p0lyn0mial p0lyn0mial force-pushed the tcp-usr-timeout-dialer branch from fc5cdaf to 7009c5a Compare November 18, 2020 10:40
@p0lyn0mial
Copy link
Author

p0lyn0mial commented Nov 18, 2020

/retest

@sttts
Copy link

sttts commented Nov 18, 2020

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Nov 18, 2020
@openshift-ci-robot
Copy link

openshift-ci-robot commented Nov 18, 2020

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: p0lyn0mial, sttts

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@p0lyn0mial
Copy link
Author

p0lyn0mial commented Nov 18, 2020

/hold

it seems that the roration tests (integration) fail only against that PR - https://search.ci.openshift.org/?search=FAIL%3A+TestCertRotationContinuousRequests&maxAge=48h&context=1&type=bug%2Bjunit&name=&maxMatches=5&maxBytes=20971520&groupBy=job

@openshift-ci-robot openshift-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 18, 2020
@p0lyn0mial
Copy link
Author

p0lyn0mial commented Nov 18, 2020

/retest

@openshift-merge-robot
Copy link

openshift-merge-robot commented Nov 18, 2020 •
edited
Loading

@p0lyn0mial: The following test failed, say /retest to rerun all failed tests:

Test name Commit Details Rerun command
ci/prow/integration 7009c5a link /test integration

Full PR test history. Your PR dashboard.

Details

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/test-infra repository. I understand the commands that are listed here.

@openshift-ci-robot
Copy link

openshift-ci-robot commented Nov 26, 2020

@p0lyn0mial: PR needs rebase.

Details

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/test-infra repository.

@openshift-ci-robot openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 26, 2020
@p0lyn0mial p0lyn0mial closed this Nov 26, 2020
@p0lyn0mial p0lyn0mial deleted the tcp-usr-timeout-dialer branch November 26, 2020 07:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@aojea aojea Awaiting requested review from aojea

@damemi damemi Awaiting requested review from damemi

Assignees

@sttts sttts

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. lgtm Indicates that a PR is ready to be merged. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. vendor-update Touching vendor dir or related files

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@p0lyn0mial @sttts @openshift-ci-robot @openshift-merge-robot