From 05dc9846e1266e6a3629c26851acb633a380dd17 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Tue, 22 Nov 2016 14:51:22 +0100 Subject: [PATCH] remove client-side for supported logging drivers The `docker logs` command performed a client-side check if the container's logging driver was supported. Now that we allow the client to connect to both "older" and "newer" daemon versions, this check is best done daemon-side. This patch remove the check on the client side, and leaves validation to the daemon, which should be the source of truth. Signed-off-by: Sebastiaan van Stijn --- cli/command/container/logs.go | 20 +++++--------------- daemon/logger/logger.go | 2 +- daemon/logs.go | 7 +++++-- integration-cli/docker_cli_daemon_test.go | 2 +- integration-cli/docker_cli_logs_test.go | 4 ++-- 5 files changed, 14 insertions(+), 21 deletions(-) diff --git a/cli/command/container/logs.go b/cli/command/container/logs.go index 3a37cedf43..9f1d9f90dd 100644 --- a/cli/command/container/logs.go +++ b/cli/command/container/logs.go @@ -1,7 +1,6 @@ package container import ( - "fmt" "io" "golang.org/x/net/context" @@ -13,11 +12,6 @@ import ( "github.com/spf13/cobra" ) -var validDrivers = map[string]bool{ - "json-file": true, - "journald": true, -} - type logsOptions struct { follow bool since string @@ -54,15 +48,6 @@ func NewLogsCommand(dockerCli *command.DockerCli) *cobra.Command { func runLogs(dockerCli *command.DockerCli, opts *logsOptions) error { ctx := context.Background() - c, err := dockerCli.Client().ContainerInspect(ctx, opts.container) - if err != nil { - return err - } - - if !validDrivers[c.HostConfig.LogConfig.Type] { - return fmt.Errorf("\"logs\" command is supported only for \"json-file\" and \"journald\" logging drivers (got: %s)", c.HostConfig.LogConfig.Type) - } - options := types.ContainerLogsOptions{ ShowStdout: true, ShowStderr: true, @@ -78,6 +63,11 @@ func runLogs(dockerCli *command.DockerCli, opts *logsOptions) error { } defer responseBody.Close() + c, err := dockerCli.Client().ContainerInspect(ctx, opts.container) + if err != nil { + return err + } + if c.Config.Tty { _, err = io.Copy(dockerCli.Out(), responseBody) } else { diff --git a/daemon/logger/logger.go b/daemon/logger/logger.go index d091997358..672f57f819 100644 --- a/daemon/logger/logger.go +++ b/daemon/logger/logger.go @@ -18,7 +18,7 @@ import ( ) // ErrReadLogsNotSupported is returned when the logger does not support reading logs. -var ErrReadLogsNotSupported = errors.New("configured logging reader does not support reading") +var ErrReadLogsNotSupported = errors.New("configured logging driver does not support reading") const ( // TimeFormat is the time format used for timestamps sent to log readers. diff --git a/daemon/logs.go b/daemon/logs.go index 32a6938869..7a76b7c7de 100644 --- a/daemon/logs.go +++ b/daemon/logs.go @@ -21,13 +21,16 @@ import ( // ContainerLogs hooks up a container's stdout and stderr streams // configured with the given struct. func (daemon *Daemon) ContainerLogs(ctx context.Context, containerName string, config *backend.ContainerLogsConfig, started chan struct{}) error { + if !(config.ShowStdout || config.ShowStderr) { + return fmt.Errorf("You must choose at least one stream") + } container, err := daemon.GetContainer(containerName) if err != nil { return err } - if !(config.ShowStdout || config.ShowStderr) { - return fmt.Errorf("You must choose at least one stream") + if container.HostConfig.LogConfig.Type == "none" { + return logger.ErrReadLogsNotSupported } cLog, err := daemon.getLogger(container) diff --git a/integration-cli/docker_cli_daemon_test.go b/integration-cli/docker_cli_daemon_test.go index 34b8d994ee..bb9cec09ef 100644 --- a/integration-cli/docker_cli_daemon_test.go +++ b/integration-cli/docker_cli_daemon_test.go @@ -1189,7 +1189,7 @@ func (s *DockerDaemonSuite) TestDaemonLoggingDriverNoneLogsError(c *check.C) { out, err = s.d.Cmd("logs", "test") c.Assert(err, check.NotNil, check.Commentf("Logs should fail with 'none' driver")) - expected := `"logs" command is supported only for "json-file" and "journald" logging drivers (got: none)` + expected := `configured logging driver does not support reading` c.Assert(out, checker.Contains, expected) } diff --git a/integration-cli/docker_cli_logs_test.go b/integration-cli/docker_cli_logs_test.go index d2dcad1052..f815ce62f5 100644 --- a/integration-cli/docker_cli_logs_test.go +++ b/integration-cli/docker_cli_logs_test.go @@ -310,8 +310,8 @@ func (s *DockerSuite) TestLogsFollowGoroutinesNoOutput(c *check.C) { func (s *DockerSuite) TestLogsCLIContainerNotFound(c *check.C) { name := "testlogsnocontainer" out, _, _ := dockerCmdWithError("logs", name) - message := fmt.Sprintf("Error: No such container: %s\n", name) - c.Assert(out, checker.Equals, message) + message := fmt.Sprintf("No such container: %s\n", name) + c.Assert(out, checker.Contains, message) } func (s *DockerSuite) TestLogsWithDetails(c *check.C) {