Fix empty LogPath with non-blocking logging mode

This fixes an issue where the container LogPath was empty when the
non-blocking logging mode was enabled. This change sets the LogPath on
the container as soon as the path is generated, instead of setting the
LogPath on a logger struct and then attempting to pull it off that
logger at a later point. That attempt to pull the LogPath off the logger
was error prone since it assumed that the logger would only ever be a
single type.

Prior to this change docker inspect returned an empty string for
LogPath. This caused issues with tools that rely on docker inspect
output to discover container logs, e.g. Kubernetes.

This commit also removes some LogPath methods that are now unnecessary
and are never invoked.

Signed-off-by: junzhe and mnussbaum <code@getbraintree.com>
Upstream-commit: 20ca612a59c45c0bd58c71c199a7ebd2a6bf1a9e
Component: engine

components/engine/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
 }
 

components/engine/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)
+}

components/engine/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()

components/engine/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