Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

WIP: add configurable detach delay to logs #5307

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
+94 −0
Draft

WIP: add configurable detach delay to logs #5307

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 46 additions & 0 deletions cli/command/container/logs.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,14 @@ package container
import (
"context"
"io"
"strconv"
"time"

"github.com/docker/cli/cli"
"github.com/docker/cli/cli/command"
"github.com/docker/cli/cli/command/completion"
"github.com/docker/docker/api/types/container"
"github.com/docker/docker/api/types/events"
"github.com/docker/docker/pkg/stdcopy"
"github.com/spf13/cobra"
)
Expand All @@ -20,6 +23,8 @@ type logsOptions struct {
details bool
tail string

detachDelay int

container string
}

Expand Down Expand Up @@ -49,6 +54,7 @@ func NewLogsCommand(dockerCli command.Cli) *cobra.Command {
flags.BoolVarP(&opts.timestamps, "timestamps", "t", false, "Show timestamps")
flags.BoolVar(&opts.details, "details", false, "Show extra details provided to logs")
flags.StringVarP(&opts.tail, "tail", "n", "all", "Number of lines to show from the end of the logs")
flags.IntVarP(&opts.detachDelay, "delay", "d", 0, "Number of seconds to wait for container restart before exiting")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could do what tail does and put this behind a separate -F option:

     -f      The -f option causes tail to not stop when end of file is reached, but rather to wait for additional data to be ap‐
             pended to the input.  The -f option is ignored if the standard input is a pipe, but not if it is a FIFO.

     -F      The -F option implies the -f option, but tail will also check to see if the file being followed has been renamed or
             rotated.  The file is closed and reopened when tail detects that the filename being read from has a new inode number.

             If the file being followed does not (yet) exist or if it is removed, tail will keep looking and will display the file
             from the beginning if and when it is created.

             The -F option is the same as the -f option if reading from standard input rather than a file.

So basically:

  • -f would only work on existing containers and exit when the container ends
  • -F would never exit, it would always just wait for the container, even if it doesn't exist at the time

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ooh, I like that UX.

return cmd
}

Expand All @@ -58,6 +64,45 @@ func runLogs(ctx context.Context, dockerCli command.Cli, opts *logsOptions) erro
return err
}

since := opts.since
for {
restarting := make(chan events.Message)
if opts.detachDelay != 0 {
go func() {
eventsCtx, eventsCtxCancel := context.WithCancel(ctx)
eventC, eventErrs := dockerCli.Client().Events(eventsCtx, events.ListOptions{})
defer eventsCtxCancel()
for {
select {
case event := <-eventC:
if event.Action == events.ActionRestart && event.Actor.ID == c.ID {
restarting <- event
return
}
case <-eventErrs:
return
}
}
}()
}

opts.since = since
err = streamLogs(ctx, dockerCli, opts, c)

if opts.detachDelay == 0 {
return err
}

select {
case restartEvent := <-restarting:
since = strconv.FormatInt(restartEvent.Time, 10)
case <-time.After(time.Duration(opts.detachDelay) * time.Second):
return err
}
}
}

func streamLogs(ctx context.Context, dockerCli command.Cli, opts *logsOptions, c container.InspectResponse) error {
responseBody, err := dockerCli.Client().ContainerLogs(ctx, c.ID, container.LogsOptions{
ShowStdout: true,
ShowStderr: true,
Expand All @@ -78,5 +123,6 @@ func runLogs(ctx context.Context, dockerCli command.Cli, opts *logsOptions) erro
} else {
_, err = stdcopy.StdCopy(dockerCli.Out(), dockerCli.Err(), responseBody)
}

return err
}
1 change: 1 addition & 0 deletions docs/reference/commandline/container_logs.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ Fetch the logs of a container

| Name | Type | Default | Description |
|:---------------------|:---------|:--------|:---------------------------------------------------------------------------------------------------|
| `-d`, `--delay` | `int` | `0` | Number of seconds to wait for container restart before exiting |
| `--details` | `bool` | | Show extra details provided to logs |
| `-f`, `--follow` | `bool` | | Follow log output |
| `--since` | `string` | | Show logs since timestamp (e.g. `2013-01-02T13:23:37Z`) or relative (e.g. `42m` for 42 minutes) |
Expand Down
1 change: 1 addition & 0 deletions docs/reference/commandline/logs.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ Fetch the logs of a container

| Name | Type | Default | Description |
|:---------------------|:---------|:--------|:---------------------------------------------------------------------------------------------------|
| `-d`, `--delay` | `int` | `0` | Number of seconds to wait for container restart before exiting |
| `--details` | `bool` | | Show extra details provided to logs |
| `-f`, `--follow` | `bool` | | Follow log output |
| `--since` | `string` | | Show logs since timestamp (e.g. `2013-01-02T13:23:37Z`) or relative (e.g. `42m` for 42 minutes) |
Expand Down
46 changes: 46 additions & 0 deletions e2e/container/logs_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
package container

import (
"strings"
"testing"
"time"

"github.com/docker/cli/e2e/internal/fixtures"
"gotest.tools/v3/icmd"
"gotest.tools/v3/poll"
)

func TestLogsReattach(t *testing.T) {
result := icmd.RunCommand("docker", "run", "-d", fixtures.AlpineImage,
"sh", "-c", "echo hi; while true; do sleep 1; done")
result.Assert(t, icmd.Success)
containerID := strings.TrimSpace(result.Stdout())

cmd := icmd.Command("docker", "logs", "-f", "-d", "5", containerID)
// cmd := icmd.Command("docker", "logs", containerID)
result = icmd.StartCmd(cmd)

poll.WaitOn(t, func(t poll.LogT) poll.Result {
if strings.Contains(result.Stdout(), "hi") {
return poll.Success()
}
return poll.Continue("waiting")
}, poll.WithDelay(1*time.Second), poll.WithTimeout(5*time.Second))

icmd.RunCommand("docker", "restart", containerID).Assert(t, icmd.Success)

poll.WaitOn(t, func(t poll.LogT) poll.Result {
// if there is another "hi" then the container was successfully restarted,
// printed "hi" again and `docker logs` stayed attached
if strings.Contains(result.Stdout(), "hi\nhi") { //nolint:dupword
return poll.Success()
}
return poll.Continue(result.Stdout())
}, poll.WithDelay(1*time.Second), poll.WithTimeout(10*time.Second))

icmd.RunCommand("docker", "stop", containerID).Assert(t, icmd.Success)

icmd.WaitOnCmd(time.Second*10, result).Assert(t, icmd.Expected{
ExitCode: 0,
})
}
Loading