Merge pull request #20858 from mountkin/validate-log-opts-again

validate log-opt when creating containers AGAIN (fixing drunkard's code)

container/container.go

@@ -289,19 +289,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)

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
 	}

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 {

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
+}

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)
 }