Skip to content

Commit

Permalink
fix: don't panic when logs waits for more than 5 seconds
Browse files Browse the repository at this point in the history
This removes panic when logs endpoint takes more than 5 seconds to respond.
The panic happened at least with podman when no new logs appear when using follow and since parameters.

We keep retrying until the context is canceled
(the retry request would fail anyway with canceled context)
or the producer is stopped,
whichever comes first.
This makes the retry behavior consistent with closed connections
handling.

This should fix #946
  • Loading branch information
martin-sucha committed Apr 24, 2023
1 parent 82e1d12 commit 92a7343
Showing 1 changed file with 17 additions and 8 deletions.
25 changes: 17 additions & 8 deletions docker.go
Original file line number Diff line number Diff line change
Expand Up @@ -634,7 +634,14 @@ func (c *DockerContainer) StartLogProducer(ctx context.Context) error {

c.stopProducer = make(chan bool)

ctx, cancel := context.WithCancel(ctx)

go func(stop <-chan bool) {
<-stop
cancel()
}(c.stopProducer)

go func() {
since := ""
// if the socket is closed we will make additional logs request with updated Since timestamp
BEGIN:
Expand All @@ -645,20 +652,22 @@ func (c *DockerContainer) StartLogProducer(ctx context.Context) error {
Since: since,
}

ctx, cancel := context.WithTimeout(ctx, time.Second*5)
defer cancel()

r, err := c.provider.client.ContainerLogs(ctx, c.GetContainerID(), options)
if err != nil {
// if we can't get the logs, panic, we can't return an error to anything
// from within this goroutine
panic(err)
// if we can't get the logs, retry in one second.
c.logger.Printf("cannot get logs for container %q: %v", c.ID, err)
if ctx.Err() != nil {
// context done.
return
}
time.Sleep(1 * time.Second)
goto BEGIN
}
defer c.provider.Close()

for {
select {
case <-stop:
case <-ctx.Done():
err := r.Close()
if err != nil {
// we can't close the read closer, this should never happen
Expand Down Expand Up @@ -709,7 +718,7 @@ func (c *DockerContainer) StartLogProducer(ctx context.Context) error {
}
}
}
}(c.stopProducer)
}()

return nil
}
Expand Down

0 comments on commit 92a7343

Please sign in to comment.