Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 30 additions & 0 deletions e2e/jsonfile_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -219,6 +219,36 @@ var testJSONFile = func() {
}
})

ginkgo.It("creates the per-container log directory if it does not exist", func() {
// Remove the directory that BeforeEach created, so the shim-logger
// must create it on its own. This reproduces the ECS MI scenario where
// only the parent (/var/log/ecs/json-file/) exists at boot.
gomega.Expect(os.RemoveAll(absLogDir)).Should(gomega.Succeed())

testLog := testLogPrefix + uuid.New().String()
args := map[string]string{
LogDriverTypeKey: JSONFileDriverName,
ContainerIDKey: TestContainerID,
ContainerNameKey: TestContainerName,
jsonFileLogPathKey: absLogFile,
}
creator := cio.BinaryIO(*Binary, args)

err := SendTestLogByContainerd(creator, testLog)
gomega.Expect(err).ShouldNot(gomega.HaveOccurred())

// Verify the directory was created by the shim-logger.
info, statErr := os.Stat(absLogDir)
gomega.Expect(statErr).ShouldNot(gomega.HaveOccurred())
gomega.Expect(info.IsDir()).Should(gomega.BeTrue())

// Verify logs were written successfully.
lines := readEnvelopeLines(absLogFile)
gomega.Expect(lines).ShouldNot(gomega.BeEmpty())
joined := joinEnvelopeLogs(lines)
gomega.Expect(joined).Should(gomega.ContainSubstring(testLog))
})

ginkgo.It("the binary fails fast when --log-path is missing", func() {
args := map[string]string{
LogDriverTypeKey: JSONFileDriverName,
Expand Down
19 changes: 17 additions & 2 deletions logger/jsonfile/logger.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ package jsonfile
import (
"context"
"fmt"
"os"
"path/filepath"

"github.com/containerd/containerd/runtime/v2/logging"
dockerlogger "github.com/docker/docker/daemon/logger"
Expand All @@ -22,8 +24,8 @@ const (
// DriverName is the name of the json-file log driver.
DriverName = "json-file"

// LogPathKey specifies the per-container output file path. The directory must already
// exist on the host; the shim-logger does not create it.
// LogPathKey specifies the per-container output file path. The shim-logger creates
// the parent directory if it does not exist.
LogPathKey = "log-path"

// MaxSizeKey is the maximum size of the log file before it is rolled (e.g., "10m").
Expand Down Expand Up @@ -78,6 +80,11 @@ const (
tagKey = "tag"
)

// logDirMode is the permission mode for per-container log directories.
// Setgid (2000) ensures new files inherit the parent directory's group.
// Owner=rwx, group=r-x, other=none.
const logDirMode = os.FileMode(02750)

// Args represents json-file log driver arguments.
type Args struct {
// Required.
Expand Down Expand Up @@ -129,6 +136,14 @@ func (la *LoggerArgs) RunLogDriver(ctx context.Context, config *logging.Config,
logger.WithLogPath(la.args.LogPath),
)

// Create the log file's parent directory if it does not exist.
if dir := filepath.Dir(la.args.LogPath); dir != "" {
if err := os.MkdirAll(dir, logDirMode); err != nil {
debug.ErrLogger = fmt.Errorf("unable to create log directory %s: %w", dir, err)
return debug.ErrLogger
}
}

stream, err := dockerjsonfilelog.New(*info)
if err != nil {
debug.ErrLogger = fmt.Errorf("unable to create stream: %w", err)
Expand Down
40 changes: 40 additions & 0 deletions logger/jsonfile/logger_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,11 @@
package jsonfile

import (
"os"
"path/filepath"
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

Expand Down Expand Up @@ -108,3 +111,40 @@ func TestGetJSONFileConfigPassesValidationForKnownKeys(t *testing.T) {
require.Contains(t, config, MaxFileKey)
require.Contains(t, config, CompressKey)
}

// TestRunLogDriverCreatesLogDirectory verifies that the log file's parent
// directory is created if it does not exist.
func TestRunLogDriverCreatesLogDirectory(t *testing.T) {
tmpDir := t.TempDir()
containerID := "test-container-abc123"
logPath := filepath.Join(tmpDir, containerID, containerID+"-json.log")

// The parent of logPath should not exist yet.
logDir := filepath.Dir(logPath)
_, err := os.Stat(logDir)
require.True(t, os.IsNotExist(err), "log directory should not exist before test")

// Simulate what RunLogDriver does: MkdirAll on the parent directory.
err = os.MkdirAll(logDir, logDirMode)
require.NoError(t, err, "MkdirAll should create the per-container directory")

// Verify the directory was created.
info, err := os.Stat(logDir)
require.NoError(t, err)
assert.True(t, info.IsDir())
}

// TestRunLogDriverLogDirectoryAlreadyExists verifies that MkdirAll is a no-op
// when the directory already exists (idempotent).
func TestRunLogDriverLogDirectoryAlreadyExists(t *testing.T) {
tmpDir := t.TempDir()
containerID := "existing-container"
logDir := filepath.Join(tmpDir, containerID)

// Pre-create the directory.
require.NoError(t, os.MkdirAll(logDir, logDirMode))

// MkdirAll again should succeed without error.
err := os.MkdirAll(logDir, logDirMode)
assert.NoError(t, err, "MkdirAll on existing directory should be idempotent")
}
Loading