Fix kubelet logs --follow bug #13864
Conversation
|
Can one of the admins verify that this patch is reasonable to test? (reply "ok to test", or if you trust the user, reply "add to whitelist") If this message is too spammy, please complain to ixdy. |
k8s-github-robot
added
the
size/S
|
Labelling this PR as size/S |
|
Redirected to @lavalamp to find a reviewer. |
| @@ -258,7 +258,13 @@ func write(statusCode int, apiVersion string, codec runtime.Codec, object runtim | |||
| w.WriteHeader(statusCode) | |||
| writer := w.(io.Writer) | |||
| if flush { | |||
| writer = flushwriter.Wrap(w) | |||
| if _, ok := w.(*hijackTimeoutWriter); ok { | |||
There was a problem hiding this comment.
This is a perfect use case for a type switch:
switch innerWriter := w.(type) {
case hijackTimeoutWriter:
writer = flushwriter.Wrap(innerWriter.w)
...
}There was a problem hiding this comment.
But more importantly, what goes wrong with these writers that you need to dig inside? Presumably they're wrapped for a reason.
There was a problem hiding this comment.
@avalamp Thanks for advise, I have submitted new code. It could avoid type conversions.
feiskyer
force-pushed
the
logs-follow
branch
from
f96e80a
to
e4d9f24
Compare
|
@lavalamp Please review new commits. |
|
@feiskyer thanks, this looks much better! LGTM |
lavalamp
added
lgtm
|
@k8s-bot ok to test |
|
GCE e2e build/test failed for commit e4d9f247500255b2112e44c3ace1b06f1b958bda. |
feiskyer
force-pushed
the
logs-follow
branch
from
e4d9f24
to
608a16a
Compare
|
LGTM was before last commit, removing LGTM |
k8s-github-robot
removed
the
lgtm
|
Rebased to trigger e2e test since there is |
|
GCE e2e build/test failed for commit 608a16a10adfadaa7cadf09c30f41ea18ff0b58f. |
feiskyer
force-pushed
the
logs-follow
branch
from
608a16a
to
6936406
Compare
|
GCE e2e build/test failed for commit 6936406d3a9f5f681c085671a77058a530d391af. |
feiskyer
force-pushed
the
logs-follow
branch
from
6936406
to
4a149e2
Compare
|
GCE e2e build/test failed for commit 4a149e276ab15f6cb266bff94ef792bb7200dc1e. |
feiskyer
force-pushed
the
logs-follow
branch
from
4a149e2
to
800e8fb
Compare
|
GCE e2e build/test passed for commit 800e8fb. |
lavalamp
added
the
lgtm
|
Sorry about the testing. You hit what appear to be three different flakes. |
|
@lavalamp I see e2e testing is down at kubernetes-dev. Fortunately this PR has passed e2e testing. |
|
@k8s-bot test this [submit-queue is verifying that this PR is safe to merge] |
|
GCE e2e build/test passed for commit 800e8fb. |
|
Automatic merge from SubmitQueue |
Auto commit by PR queue bot
Fix kubelet logs --follow bug #13811