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
remove TODOs from http package and prober #108803
remove TODOs from http package and prober #108803
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
/approve
/priority backlog need to fix the linting error |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM other than the linter failure
exec execprobe.Prober | ||
// probe types needs different httpprobe instances so they don't | ||
// share a connection pool which can cause collisions to the | ||
// same host:port and transient failures. See #49740. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
n.b. #49740 was a workaround, fixed upstream in golang 1.10+ with golang/go#22091
Should be safe to collapse these.
6a1b3a2
to
1df526b
Compare
I guess we can continue with this one... |
Ping @tallclair can you approve? |
no, looks like you need a sig-node-approver (I already approved, but it only counts for pkg/kubelet/prober) |
/assign @mrunalp |
/lgtm nice, this divide by 3 the number of clients |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: mrunalp, SergeyKanzhelev, tallclair 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 |
What type of PR is this?
/kind cleanup
/area kubelet
/sig node
What this PR does / why we need it:
This is a clean up of old workaround with three http connection pools that was fixed in golang/go#22091. Also removed a few old TODOs.
Special notes for your reviewer:
I was looking at something else in this code and found that these old TODOs will unlikely be fixed, and the workaround with three http pools is unnatural and not needed any longer. Just a simple clean up.