daemon: logger: error out on daemon start if invalid logger address

If an invalid logger address is provided on daemon start it will
silently fail. As syslog driver is doing, this check should be done on
daemon start and prevent it from starting even in other drivers.
This patch also adds integration tests for this behavior.

Signed-off-by: Antonio Murdaca <runcom@linux.com>

daemon/logger/fluentd/fluentd.go

@@ -38,44 +38,20 @@ func init() {
 	}
 }
 
-func parseConfig(ctx logger.Context) (string, int, string, error) {
-	host := defaultHostName
-	port := defaultPort
-
-	config := ctx.Config
-
-	tag, err := loggerutils.ParseLogTag(ctx, "docker.{{.ID}}")
-	if err != nil {
-		return "", 0, "", err
-	}
-
-	if address := config["fluentd-address"]; address != "" {
-		if h, p, err := net.SplitHostPort(address); err != nil {
-			if !strings.Contains(err.Error(), "missing port in address") {
-				return "", 0, "", err
-			}
-			host = h
-		} else {
-			portnum, err := strconv.Atoi(p)
-			if err != nil {
-				return "", 0, "", err
-			}
-			host = h
-			port = portnum
-		}
-	}
-
-	return host, port, tag, nil
-}
-
 // New creates a fluentd logger using the configuration passed in on
 // the context. Supported context configuration variables are
 // fluentd-address & fluentd-tag.
 func New(ctx logger.Context) (logger.Logger, error) {
-	host, port, tag, err := parseConfig(ctx)
+	host, port, err := parseAddress(ctx.Config["fluentd-address"])
 	if err != nil {
 		return nil, err
 	}
+
+	tag, err := loggerutils.ParseLogTag(ctx, "docker.{{.ID}}")
+	if err != nil {
+		return nil, err
+	}
+
 	logrus.Debugf("logging driver fluentd configured for container:%s, host:%s, port:%d, tag:%s.", ctx.ContainerID, host, port, tag)
 
 	// logger tries to recoonect 2**32 - 1 times
@@ -104,6 +80,14 @@ func (f *fluentd) Log(msg *logger.Message) error {
 	return f.writer.PostWithTime(f.tag, msg.Timestamp, data)
 }
 
+func (f *fluentd) Close() error {
+	return f.writer.Close()
+}
+
+func (f *fluentd) Name() string {
+	return name
+}
+
 // ValidateLogOpt looks for fluentd specific log options fluentd-address & fluentd-tag.
 func ValidateLogOpt(cfg map[string]string) error {
 	for key := range cfg {
@@ -115,13 +99,30 @@ func ValidateLogOpt(cfg map[string]string) error {
 			return fmt.Errorf("unknown log opt '%s' for fluentd log driver", key)
 		}
 	}
+
+	if _, _, err := parseAddress(cfg["fluentd-address"]); err != nil {
+		return err
+	}
+
 	return nil
 }
 
-func (f *fluentd) Close() error {
+func parseAddress(address string) (string, int, error) {
-	return f.writer.Close()
+	if address == "" {
-}
+		return defaultHostName, defaultPort, nil
+	}
 
-func (f *fluentd) Name() string {
+	host, port, err := net.SplitHostPort(address)
-	return name
+	if err != nil {
+		if !strings.Contains(err.Error(), "missing port in address") {
+			return "", 0, fmt.Errorf("invalid fluentd-address %s: %s", address, err)
+		}
+		return host, defaultPort, nil
+	}
+
+	portnum, err := strconv.Atoi(port)
+	if err != nil {
+		return "", 0, fmt.Errorf("invalid fluentd-address %s: %s", address, err)
+	}
+	return host, portnum, nil
 }

daemon/logger/gelf/gelf.go

@@ -147,28 +147,35 @@ func ValidateLogOpt(cfg map[string]string) error {
 			return fmt.Errorf("unknown log opt '%s' for gelf log driver", key)
 		}
 	}
+
+	if _, err := parseAddress(cfg["gelf-address"]); err != nil {
+		return err
+	}
+
 	return nil
 }
 
 func parseAddress(address string) (string, error) {
-	if urlutil.IsTransportURL(address) {
+	if address == "" {
-		url, err := url.Parse(address)
+		return "", nil
-		if err != nil {
+	}
-			return "", err
+	if !urlutil.IsTransportURL(address) {
-		}
+		return "", fmt.Errorf("gelf-address should be in form proto://address, got %v", address)
-
+	}
-		// we support only udp
+	url, err := url.Parse(address)
-		if url.Scheme != "udp" {
+	if err != nil {
-			return "", fmt.Errorf("gelf: endpoint needs to be UDP")
+		return "", err
-		}
-
-		// get host and port
-		if _, _, err = net.SplitHostPort(url.Host); err != nil {
-			return "", fmt.Errorf("gelf: please provide gelf-address as udp://host:port")
-		}
-
-		return url.Host, nil
 	}
 
-	return "", nil
+	// we support only udp
+	if url.Scheme != "udp" {
+		return "", fmt.Errorf("gelf: endpoint needs to be UDP")
+	}
+
+	// get host and port
+	if _, _, err = net.SplitHostPort(url.Host); err != nil {
+		return "", fmt.Errorf("gelf: please provide gelf-address as udp://host:port")
+	}
+
+	return url.Host, nil
 }

integration-cli/docker_cli_daemon_test.go

@@ -1609,14 +1609,6 @@ func (s *DockerDaemonSuite) TestDaemonRestartWithContainerWithRestartPolicyAlway
 	c.Assert(strings.TrimSpace(out), check.Equals, id[:12])
 }
 
-func (s *DockerDaemonSuite) TestDaemonCorruptedSyslogAddress(c *check.C) {
-	c.Assert(s.d.Start("--log-driver=syslog", "--log-opt", "syslog-address=corrupted:1234"), check.NotNil)
-	runCmd := exec.Command("grep", "Failed to set log opts: syslog-address should be in form proto://address", s.d.LogfileName())
-	if out, _, err := runCommandWithOutput(runCmd); err != nil {
-		c.Fatalf("Expected 'Error starting daemon' message; but doesn't exist in log: %q, err: %v", out, err)
-	}
-}
-
 func (s *DockerDaemonSuite) TestDaemonWideLogConfig(c *check.C) {
 	c.Assert(s.d.Start("--log-driver=json-file", "--log-opt=max-size=1k"), check.IsNil)
 	out, err := s.d.Cmd("run", "-d", "--name=logtest", "busybox", "top")
@@ -1689,3 +1681,27 @@ func (s *DockerDaemonSuite) TestDaemonRestartLocalVolumes(c *check.C) {
 	_, err = s.d.Cmd("volume", "inspect", "test")
 	c.Assert(err, check.IsNil)
 }
+
+func (s *DockerDaemonSuite) TestDaemonCorruptedLogDriverAddress(c *check.C) {
+	for _, driver := range []string{
+		"syslog",
+		"gelf",
+	} {
+		args := []string{"--log-driver=" + driver, "--log-opt", driver + "-address=corrupted:42"}
+		c.Assert(s.d.Start(args...), check.NotNil, check.Commentf(fmt.Sprintf("Expected daemon not to start with invalid %s-address provided", driver)))
+		expected := fmt.Sprintf("Failed to set log opts: %s-address should be in form proto://address", driver)
+		runCmd := exec.Command("grep", expected, s.d.LogfileName())
+		if out, _, err := runCommandWithOutput(runCmd); err != nil {
+			c.Fatalf("Expected %q message; but doesn't exist in log: %q, err: %v", expected, out, err)
+		}
+	}
+}
+
+func (s *DockerDaemonSuite) TestDaemonCorruptedFluentdAddress(c *check.C) {
+	c.Assert(s.d.Start("--log-driver=fluentd", "--log-opt", "fluentd-address=corrupted:c"), check.NotNil)
+	expected := "Failed to set log opts: invalid fluentd-address corrupted:c: "
+	runCmd := exec.Command("grep", expected, s.d.LogfileName())
+	if out, _, err := runCommandWithOutput(runCmd); err != nil {
+		c.Fatalf("Expected %q message; but doesn't exist in log: %q, err: %v", expected, out, err)
+	}
+}