Fix stop after EOF error condition #1654
Conversation
Signed-off-by: Martin Michaelis <[email protected]>
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: mgjm The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
| @@ -270,7 +270,7 @@ func (c *ConmonClient) redirectResponseToOutputStreams( | |||
| return nil | |||
| } | |||
| } | |||
| if er == io.EOF || (cfg.ContainerStdin && !cfg.StopAfterStdinEOF) { | |||
| if er == io.EOF && cfg.ContainerStdin && !cfg.StopAfterStdinEOF { | |||
There was a problem hiding this comment.
Hm, we have to test if this works with CRI-O, I know we had some issues with that in the past.
There was a problem hiding this comment.
The more I think about it, it probably should just be
if er == io.EOF {
return nil
}There was a problem hiding this comment.
cfg.ContainerStdin && !cfg.StopAfterStdinEOF feels like a workaround, is that related to #1655?
There was a problem hiding this comment.
Likely we need to detect the EOF and then drain the streams.
|
@mgjm are you still working on this? |
What type of PR is this?
/kind bug
What this PR does / why we need it:
While implementing attach for podman the attach stream would never terminate (at least without setting
cfg.StopAfterStdinEOF, but then it would not wait for all output).This change fixes this issue in my case, but it might be the wrong fix. Maybe it should instead be just the following:
Which issue(s) this PR fixes:
None
Special notes for your reviewer:
Does this PR introduce a user-facing change?