Merge pull request #36272 from mnussbaum/36255-fix_log_path

Fix empty LogPath with non-blocking logging mode

container/container.go

@@ -38,7 +38,7 @@ import (
 	"github.com/docker/docker/runconfig"
 	"github.com/docker/docker/volume"
 	"github.com/docker/go-connections/nat"
-	"github.com/docker/go-units"
+	units "github.com/docker/go-units"
 	"github.com/docker/libnetwork"
 	"github.com/docker/libnetwork/netlabel"
 	"github.com/docker/libnetwork/options"
@@ -391,6 +391,8 @@ func (container *Container) StartLogger() (logger.Logger, error) {
 		if err != nil {
 			return nil, err
 		}
+
+		container.LogPath = info.LogPath
 	}
 
 	l, err := initDriver(info)
@@ -974,11 +976,6 @@ func (container *Container) startLogging() error {
 	copier.Run()
 	container.LogDriver = l
 
-	// set LogPath field only for json-file logdriver
-	if jl, ok := l.(*jsonfilelog.JSONFileLogger); ok {
-		container.LogPath = jl.LogPath()
-	}
-
 	return nil
 }
 

container/container_unit_test.go

@@ -1,12 +1,16 @@
 package container // import "github.com/docker/docker/container"
 
 import (
+	"fmt"
+	"io/ioutil"
 	"path/filepath"
 	"testing"
 
 	"github.com/docker/docker/api/types/container"
 	swarmtypes "github.com/docker/docker/api/types/swarm"
+	"github.com/docker/docker/daemon/logger/jsonfilelog"
 	"github.com/docker/docker/pkg/signal"
+	"github.com/stretchr/testify/require"
 )
 
 func TestContainerStopSignal(t *testing.T) {
@@ -66,3 +70,52 @@ func TestContainerSecretReferenceDestTarget(t *testing.T) {
 		t.Fatalf("expected secret dest %q; received %q", expected, d)
 	}
 }
+
+func TestContainerLogPathSetForJSONFileLogger(t *testing.T) {
+	containerRoot, err := ioutil.TempDir("", "TestContainerLogPathSetForJSONFileLogger")
+	require.NoError(t, err)
+
+	c := &Container{
+		Config: &container.Config{},
+		HostConfig: &container.HostConfig{
+			LogConfig: container.LogConfig{
+				Type: jsonfilelog.Name,
+			},
+		},
+		ID:   "TestContainerLogPathSetForJSONFileLogger",
+		Root: containerRoot,
+	}
+
+	_, err = c.StartLogger()
+	require.NoError(t, err)
+
+	expectedLogPath, err := filepath.Abs(filepath.Join(containerRoot, fmt.Sprintf("%s-json.log", c.ID)))
+	require.NoError(t, err)
+	require.Equal(t, c.LogPath, expectedLogPath)
+}
+
+func TestContainerLogPathSetForRingLogger(t *testing.T) {
+	containerRoot, err := ioutil.TempDir("", "TestContainerLogPathSetForRingLogger")
+	require.NoError(t, err)
+
+	c := &Container{
+		Config: &container.Config{},
+		HostConfig: &container.HostConfig{
+			LogConfig: container.LogConfig{
+				Type: jsonfilelog.Name,
+				Config: map[string]string{
+					"mode": string(container.LogModeNonBlock),
+				},
+			},
+		},
+		ID:   "TestContainerLogPathSetForRingLogger",
+		Root: containerRoot,
+	}
+
+	_, err = c.StartLogger()
+	require.NoError(t, err)
+
+	expectedLogPath, err := filepath.Abs(filepath.Join(containerRoot, fmt.Sprintf("%s-json.log", c.ID)))
+	require.NoError(t, err)
+	require.Equal(t, c.LogPath, expectedLogPath)
+}

daemon/logger/jsonfilelog/jsonfilelog.go

@@ -150,11 +150,6 @@ func ValidateLogOpt(cfg map[string]string) error {
 	return nil
 }
 
-// LogPath returns the location the given json logger logs to.
-func (l *JSONFileLogger) LogPath() string {
-	return l.writer.LogPath()
-}
-
 // Close closes underlying file and signals all readers to stop.
 func (l *JSONFileLogger) Close() error {
 	l.mu.Lock()

daemon/logger/loggerutils/logfile.go

@@ -130,13 +130,6 @@ func rotate(name string, maxFiles int) error {
 	return nil
 }
 
-// LogPath returns the location the given writer logs to.
-func (w *LogFile) LogPath() string {
-	w.mu.Lock()
-	defer w.mu.Unlock()
-	return w.f.Name()
-}
-
 // MaxFiles return maximum number of files
 func (w *LogFile) MaxFiles() int {
 	return w.maxFiles