fix: allow attaching to any combination of stdin/stdout/stderr#8316
fix: allow attaching to any combination of stdin/stdout/stderr#8316estesp merged 1 commit intocontainerd:mainfrom
Conversation
|
Hi @davidhsingyuchen. 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/test-infra repository. |
|
/ok-to-test |
|
@dims: Cannot trigger testing until a trusted user reviews the PR and leaves an DetailsIn response to this:
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. |
|
/ok-to-test |
|
/test all |
|
@dims: Cannot trigger testing until a trusted user reviews the PR and leaves an DetailsIn response to this:
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. |
|
/retest |
|
@dims: Cannot trigger testing until a trusted user reviews the PR and leaves an DetailsIn response to this:
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. |
|
/test pull-containerd-node-e2e |
|
@dims: Cannot trigger testing until a trusted user reviews the PR and leaves an DetailsIn response to this:
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. |
|
/ok-to-test |
|
Thanks Kazuyoshi for the review. @samuelkarp Could you help take a look, thanks! (I guess 2 approvals are needed?) |
|
@davidhsingyuchen Would you mind adding a unit test to cover the behavior you've changed? There's a unit test covering |
davidhsingyuchen
force-pushed
the
partial-attach
branch
from
dd2b6a1 to
0d61c84
Compare
|
@samuelkarp Thank you for the reminder! Done. |
|
@kzys @samuelkarp Would you mind taking a look at the latest revision, thanks! |
|
Can you re-base on current |
Before this PR, if a stdin/stdout/stderr stream is nil, and the corresponding FIFO is not an empty string, a panic will occur when Read/Write of the nil stream is invoked in io.CopyBuffer. Signed-off-by: Hsing-Yu (David) Chen <[email protected]>
davidhsingyuchen
force-pushed
the
partial-attach
branch
from
0d61c84 to
687a5f5
Compare
done! |
7 participants
Before this PR, if a stdin/stdout/stderr stream is nil, and the corresponding FIFO is not an empty string, a panic will occur when
Read/Writeof the nil stream is invoked inio.CopyBuffer.