Skip to content

Commit a08f913

Browse files
committed
pkg/bindings/containers: do not ignore ErrUnexpectedEOF
Do not ignore ErrUnexpectedEOF from DemuxHeader(), if we fail to parse the header there must have been a clear protocal error between client and server which should be reported and not silently ignored. I wonder ig this might explain why we have missing remote exec/attach output without any error, it is possible we are eating some internal errors due this. Commit ba8eba8 added the ErrUnexpectedEOF check but without any explanation why that would be needed. The tests from that commit pass without it locally but not in CI. With some debugging best I found the issue is actually a test bug. The channel is not consumed until it is closed which means the main test exists before the log reading goroutine is done. And if the main test exists the first step it does is to kill the podman service which then can trigger the ErrUnexpectedEOF server on the still open http connection and thus the test case failed there. Signed-off-by: Paul Holzinger <[email protected]>
1 parent 2a9729d commit a08f913

File tree

3 files changed

+8
-3
lines changed

3 files changed

+8
-3
lines changed

pkg/bindings/containers/attach.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -219,7 +219,7 @@ func Attach(ctx context.Context, nameOrID string, stdin io.Reader, stdout io.Wri
219219
// Read multiplexed channels and write to appropriate stream
220220
fd, l, err := DemuxHeader(socket, buffer)
221221
if err != nil {
222-
if errors.Is(err, io.EOF) || errors.Is(err, io.ErrUnexpectedEOF) {
222+
if errors.Is(err, io.EOF) {
223223
return nil
224224
}
225225
return err
@@ -540,7 +540,7 @@ func ExecStartAndAttach(ctx context.Context, sessionID string, options *ExecStar
540540
// Read multiplexed channels and write to appropriate stream
541541
fd, l, err := DemuxHeader(socket, buffer)
542542
if err != nil {
543-
if errors.Is(err, io.EOF) || errors.Is(err, io.ErrUnexpectedEOF) {
543+
if errors.Is(err, io.EOF) {
544544
return nil
545545
}
546546
return err

pkg/bindings/containers/logs.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ func Logs(ctx context.Context, nameOrID string, options *LogOptions, stdoutChan,
4444
for {
4545
fd, l, err := DemuxHeader(response.Body, buffer)
4646
if err != nil {
47-
if errors.Is(err, io.EOF) || errors.Is(err, io.ErrUnexpectedEOF) {
47+
if errors.Is(err, io.EOF) {
4848
return nil
4949
}
5050
return err

pkg/bindings/test/containers_test.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -383,6 +383,11 @@ var _ = Describe("Podman containers ", func() {
383383
o = strings.TrimSpace(o)
384384
_, err = time.Parse(time.RFC1123Z, o)
385385
Expect(err).ShouldNot(HaveOccurred())
386+
387+
// drain the line channel and make sure there are no more log lines
388+
for l := range stdoutChan {
389+
Fail("container logs returned more than one line: " + l)
390+
}
386391
})
387392

388393
It("podman top", func() {

0 commit comments

Comments
 (0)