diff --git a/components/engine/api/server/container.go b/components/engine/api/server/container.go index f74ae89356..15817e0089 100644 --- a/components/engine/api/server/container.go +++ b/components/engine/api/server/container.go @@ -94,7 +94,11 @@ func (s *Server) getContainersLogs(ctx context.Context, w http.ResponseWriter, r return fmt.Errorf("Missing parameter") } - // Validate args here, because we can't return not StatusOK after job.Run() call + // Args are validated before the stream starts because when it starts we're + // sending HTTP 200 by writing an empty chunk of data to tell the client that + // daemon is going to stream. By sending this initial HTTP 200 we can't report + // any error after the stream starts (i.e. container not found, wrong parameters) + // with the appropriate status code. stdout, stderr := boolValue(r, "stdout"), boolValue(r, "stderr") if !(stdout || stderr) { return fmt.Errorf("Bad parameters: you must choose at least one stream") diff --git a/components/engine/daemon/logger/fluentd/fluentd.go b/components/engine/daemon/logger/fluentd/fluentd.go index 3ce0747473..974c2e21d9 100644 --- a/components/engine/daemon/logger/fluentd/fluentd.go +++ b/components/engine/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 { - return f.writer.Close() -} +func parseAddress(address string) (string, int, error) { + if address == "" { + return defaultHostName, defaultPort, nil + } -func (f *fluentd) Name() string { - return name + host, port, err := net.SplitHostPort(address) + 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 } diff --git a/components/engine/daemon/logger/gelf/gelf.go b/components/engine/daemon/logger/gelf/gelf.go index e980a6ce94..6a62672d72 100644 --- a/components/engine/daemon/logger/gelf/gelf.go +++ b/components/engine/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) { - url, err := url.Parse(address) - if err != nil { - return "", err - } - - // 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 + if address == "" { + return "", nil + } + if !urlutil.IsTransportURL(address) { + return "", fmt.Errorf("gelf-address should be in form proto://address, got %v", address) + } + url, err := url.Parse(address) + if err != nil { + return "", err } - 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 } diff --git a/components/engine/integration-cli/docker_cli_daemon_test.go b/components/engine/integration-cli/docker_cli_daemon_test.go index 1c4d97152e..39df191d99 100644 --- a/components/engine/integration-cli/docker_cli_daemon_test.go +++ b/components/engine/integration-cli/docker_cli_daemon_test.go @@ -1615,14 +1615,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) { if err := s.d.StartWithBusybox("--log-driver=json-file", "--log-opt=max-size=1k"); err != nil { c.Fatal(err) @@ -1697,3 +1689,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) + } +}