From 068085005ef378f6320fdce90a67b104399b796d Mon Sep 17 00:00:00 2001 From: Shijiang Wei Date: Wed, 2 Mar 2016 20:22:18 +0800 Subject: [PATCH] validate log-opt when creating containers AGAIN Signed-off-by: Shijiang Wei --- container/container.go | 13 ------------- daemon/create.go | 6 ------ daemon/daemon.go | 5 +++++ daemon/logs.go | 18 ++++++++++++++++-- integration-cli/docker_cli_create_test.go | 6 ++++-- 5 files changed, 25 insertions(+), 23 deletions(-) diff --git a/container/container.go b/container/container.go index ad4e728663..85a260eb96 100644 --- a/container/container.go +++ b/container/container.go @@ -282,19 +282,6 @@ func (container *Container) exposes(p nat.Port) bool { return exists } -// GetLogConfig returns the log configuration for the container. -func (container *Container) GetLogConfig(defaultConfig containertypes.LogConfig) containertypes.LogConfig { - cfg := container.HostConfig.LogConfig - if cfg.Type != "" || len(cfg.Config) > 0 { // container has log driver configured - if cfg.Type == "" { - cfg.Type = jsonfilelog.Name - } - return cfg - } - // Use daemon's default log config for containers - return defaultConfig -} - // StartLogger starts a new logger driver for the container. func (container *Container) StartLogger(cfg containertypes.LogConfig) (logger.Logger, error) { c, err := logger.GetLogDriver(cfg.Type) diff --git a/daemon/create.go b/daemon/create.go index b7b9001d63..425c4344bb 100644 --- a/daemon/create.go +++ b/daemon/create.go @@ -5,7 +5,6 @@ import ( "github.com/Sirupsen/logrus" "github.com/docker/docker/container" - "github.com/docker/docker/daemon/logger" "github.com/docker/docker/image" "github.com/docker/docker/layer" "github.com/docker/docker/pkg/idtools" @@ -81,11 +80,6 @@ func (daemon *Daemon) create(params types.ContainerCreateConfig) (retC *containe } }() - logCfg := container.GetLogConfig(daemon.defaultLogConfig) - if err := logger.ValidateLogOpts(logCfg.Type, logCfg.Config); err != nil { - return nil, err - } - if err := daemon.setSecurityOptions(container, params.HostConfig); err != nil { return nil, err } diff --git a/daemon/daemon.go b/daemon/daemon.go index 1f26e7f553..892e83dac0 100644 --- a/daemon/daemon.go +++ b/daemon/daemon.go @@ -1472,6 +1472,11 @@ func (daemon *Daemon) verifyContainerSettings(hostConfig *containertypes.HostCon return nil, nil } + logCfg := daemon.getLogConfig(hostConfig.LogConfig) + if err := logger.ValidateLogOpts(logCfg.Type, logCfg.Config); err != nil { + return nil, err + } + for port := range hostConfig.PortBindings { _, portStr := nat.SplitProtoPort(string(port)) if _, err := nat.ParsePort(portStr); err != nil { diff --git a/daemon/logs.go b/daemon/logs.go index 1e94802399..8172df175c 100644 --- a/daemon/logs.go +++ b/daemon/logs.go @@ -13,6 +13,7 @@ import ( "github.com/docker/docker/daemon/logger/jsonfilelog" "github.com/docker/docker/pkg/ioutils" "github.com/docker/docker/pkg/stdcopy" + containertypes "github.com/docker/engine-api/types/container" timetypes "github.com/docker/engine-api/types/time" ) @@ -103,7 +104,7 @@ func (daemon *Daemon) getLogger(container *container.Container) (logger.Logger, if container.LogDriver != nil && container.IsRunning() { return container.LogDriver, nil } - cfg := container.GetLogConfig(daemon.defaultLogConfig) + cfg := daemon.getLogConfig(container.HostConfig.LogConfig) if err := logger.ValidateLogOpts(cfg.Type, cfg.Config); err != nil { return nil, err } @@ -112,7 +113,7 @@ func (daemon *Daemon) getLogger(container *container.Container) (logger.Logger, // StartLogging initializes and starts the container logging stream. func (daemon *Daemon) StartLogging(container *container.Container) error { - cfg := container.GetLogConfig(daemon.defaultLogConfig) + cfg := daemon.getLogConfig(container.HostConfig.LogConfig) if cfg.Type == "none" { return nil // do not start logging routines } @@ -137,3 +138,16 @@ func (daemon *Daemon) StartLogging(container *container.Container) error { return nil } + +// getLogConfig returns the log configuration for the container. +func (daemon *Daemon) getLogConfig(cfg containertypes.LogConfig) containertypes.LogConfig { + if cfg.Type != "" || len(cfg.Config) > 0 { // container has log driver configured + if cfg.Type == "" { + cfg.Type = jsonfilelog.Name + } + return cfg + } + + // Use daemon's default log config for containers + return daemon.defaultLogConfig +} diff --git a/integration-cli/docker_cli_create_test.go b/integration-cli/docker_cli_create_test.go index ec7b8f4a58..c0f1854178 100644 --- a/integration-cli/docker_cli_create_test.go +++ b/integration-cli/docker_cli_create_test.go @@ -443,8 +443,10 @@ func (s *DockerSuite) TestCreateWithWorkdir(c *check.C) { func (s *DockerSuite) TestCreateWithInvalidLogOpts(c *check.C) { name := "test-invalidate-log-opts" - _, _, err := dockerCmdWithError("create", "--name", name, "--log-opt", "invalid=true") + out, _, err := dockerCmdWithError("create", "--name", name, "--log-opt", "invalid=true", "busybox") c.Assert(err, checker.NotNil) - out, _ := dockerCmd(c, "ps", "-a") + c.Assert(out, checker.Contains, "unknown log opt") + + out, _ = dockerCmd(c, "ps", "-a") c.Assert(out, checker.Not(checker.Contains), name) }