[release/1.7] Fix fatal concurrency error in port forwarding#11306
[release/1.7] Fix fatal concurrency error in port forwarding#11306fuweid merged 1 commit intocontainerd:release/1.7from
Conversation
|
Hi @ningmingxiao. Thanks for your PR. I'm waiting for a containerd member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions 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-sigs/prow repository. |
|
/ok-to-test |
There was a problem hiding this comment.
This looks good to me as a stop gap solution; maybe we could also consider updating klog module to pull in JSON encoder changes as a more robust solution but even that is not bullet proof.
Unfortunately I am unable to brainstorm any (unit/integration) test to validate; it is a difficult thing to test. Maybe others have ideas?
If others agree, it would be good to have in release/1.6 as well.
| // | ||
| // In both cases, we should RESET the httpStream. | ||
| klog.ErrorS(err, "forwarding port", "conn", h.conn, "request", p.requestID, "port", portString) | ||
| klog.Errorf("forwarding port conn=%p request=%s port=%s failed:%v", h.conn, p.requestID, portString, err) |
There was a problem hiding this comment.
minor nit on formatting; non-blocking though.
| klog.Errorf("forwarding port conn=%p request=%s port=%s failed:%v", h.conn, p.requestID, portString, err) | |
| klog.Errorf("(conn=%p, request=%s) forwarding port %s failed: %v", h.conn, p.requestID, portString, err) |
There was a problem hiding this comment.
done. do we need update klog to v2.130.1
There was a problem hiding this comment.
Thanks @ningmingxiao; I think the proposed fix here is good enough solution. I did a proof of concept and was able to reproduce the issue using the JSON encoder if the connection has a exported map field that is not annotated to be hidden. See #11302 (comment) for more information.
Signed-off-by: ningmingxiao <ning.mingxiao@zte.com.cn>
1568c12 to
0fe9f0b
Compare
fix:#11302