From a360b296260f62a441d78b8d28111b0e5bd8c019 Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Wed, 9 Jul 2025 18:10:40 +0200 Subject: [PATCH] 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 ba8eba83ef 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 --- pkg/bindings/containers/attach.go | 4 ++-- pkg/bindings/containers/logs.go | 2 +- pkg/bindings/test/containers_test.go | 5 +++++ 3 files changed, 8 insertions(+), 3 deletions(-) diff --git a/pkg/bindings/containers/attach.go b/pkg/bindings/containers/attach.go index d54f386c57..ca3974a3ce 100644 --- a/pkg/bindings/containers/attach.go +++ b/pkg/bindings/containers/attach.go @@ -219,7 +219,7 @@ func Attach(ctx context.Context, nameOrID string, stdin io.Reader, stdout io.Wri // Read multiplexed channels and write to appropriate stream fd, l, err := DemuxHeader(socket, buffer) if err != nil { - if errors.Is(err, io.EOF) || errors.Is(err, io.ErrUnexpectedEOF) { + if errors.Is(err, io.EOF) { return nil } return err @@ -540,7 +540,7 @@ func ExecStartAndAttach(ctx context.Context, sessionID string, options *ExecStar // Read multiplexed channels and write to appropriate stream fd, l, err := DemuxHeader(socket, buffer) if err != nil { - if errors.Is(err, io.EOF) || errors.Is(err, io.ErrUnexpectedEOF) { + if errors.Is(err, io.EOF) { return nil } return err diff --git a/pkg/bindings/containers/logs.go b/pkg/bindings/containers/logs.go index cf953dd72e..8796c3a768 100644 --- a/pkg/bindings/containers/logs.go +++ b/pkg/bindings/containers/logs.go @@ -44,7 +44,7 @@ func Logs(ctx context.Context, nameOrID string, options *LogOptions, stdoutChan, for { fd, l, err := DemuxHeader(response.Body, buffer) if err != nil { - if errors.Is(err, io.EOF) || errors.Is(err, io.ErrUnexpectedEOF) { + if errors.Is(err, io.EOF) { return nil } return err diff --git a/pkg/bindings/test/containers_test.go b/pkg/bindings/test/containers_test.go index 64b421eec2..9de2f518d4 100644 --- a/pkg/bindings/test/containers_test.go +++ b/pkg/bindings/test/containers_test.go @@ -383,6 +383,11 @@ var _ = Describe("Podman containers ", func() { o = strings.TrimSpace(o) _, err = time.Parse(time.RFC1123Z, o) Expect(err).ShouldNot(HaveOccurred()) + + // drain the line channel and make sure there are no more log lines + for l := range stdoutChan { + Fail("container logs returned more than one line: " + l) + } }) It("podman top", func() {