Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 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
4 changes: 4 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,10 @@ test-e2e-for-awslogs:
test-e2e-for-fluentd:
go test -tags e2e -timeout 30m ./e2e -test.v -ginkgo.v --binary "$(AWS_CONTAINERD_LOGGERS_BINARY)" --log-driver "fluentd"

.PHONY: test-e2e-for-json-file
test-e2e-for-json-file:
go test -tags e2e -timeout 30m ./e2e -test.v -ginkgo.v --binary "$(AWS_CONTAINERD_LOGGERS_BINARY)" --log-driver "json-file"

.PHONY: test-e2e-for-splunk
test-e2e-for-splunk:
go test -tags e2e -timeout 30m ./e2e -test.v -ginkgo.v --binary "$(AWS_CONTAINERD_LOGGERS_BINARY)" --log-driver "splunk" --splunk-token ${SPLUNK_TOKEN}
Expand Down
52 changes: 52 additions & 0 deletions args.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
"github.com/aws/shim-loggers-for-containerd/logger"
"github.com/aws/shim-loggers-for-containerd/logger/awslogs"
"github.com/aws/shim-loggers-for-containerd/logger/fluentd"
"github.com/aws/shim-loggers-for-containerd/logger/jsonfile"
"github.com/aws/shim-loggers-for-containerd/logger/splunk"

units "github.com/docker/go-units"
Expand Down Expand Up @@ -178,6 +179,57 @@ func getFluentdArgs() *fluentd.Args {
}
}

// getJSONFileArgs gets json-file specified arguments for the json-file log driver.
// log-path is required; everything else is optional and forwarded to moby's jsonfilelog
// as-is. Note that moby validates option *keys* in ValidateLogOpts but defers value
// validation (e.g., max-size format, max-file >= 1, compress requires max-file >= 2)
// to writer construction in jsonfilelog.New.
func getJSONFileArgs() (*jsonfile.Args, error) {
logPath, err := getRequiredValue(jsonfile.LogPathKey)
if err != nil {
return nil, err
}

// Why every viper read below is GetString — including for max-file (int) and
// compress (bool):
Comment thread
JoseVillalta marked this conversation as resolved.
Outdated
//
// 1. The wire to moby is a map[string]string. jsonfilelog.New(info logger.Info)
// reads from info.Config which is typed map[string]string and parses each
// value itself (units.FromHumanSize for max-size, strconv.Atoi for max-file,
// strconv.ParseBool for compress). Holding the values as strings here means
// zero conversion and zero round-trip risk on the way out.
// 2. We need to distinguish "unset" from "zero" so getJSONFileConfig can omit
// unset keys from the config map. The runtime guardrails inside
// jsonfilelog.New are written assuming missing-key-means-default. With
// GetString, "" == unset and we get to that distinction for free. With
// GetInt/GetBool, viper returns 0 / false for unset, which collide with a
// legal explicit value: max-file=0 is illegal but compress=false is the
// default, so we cannot tell unset from explicit-false. Tri-state would
// require *string or a sentinel — more code for no payoff.
// 3. Validation is moby's job. moby owns the validator and the runtime
// guardrails. If we typed the args ourselves, we'd be tempted to validate
// ourselves and either duplicate moby's rules or quietly diverge from them.
// Better to keep this layer dumb and let moby be the single source of truth.
//
// Trade-off: "--max-file 05" is forwarded verbatim to moby and "--compress yes"
// fails at writer-construction time with moby's error rather than at args-parse
// time with a friendlier one. The other drivers (splunk, fluentd) accept the
// same trade-off; doing better is a repo-wide refactor, not a json-file-specific
// one.
return &jsonfile.Args{
LogPath: logPath,
MaxSize: viper.GetString(jsonfile.MaxSizeKey),
MaxFile: viper.GetString(jsonfile.MaxFileKey),
Compress: viper.GetString(jsonfile.CompressKey),
Labels: viper.GetString(jsonfile.JSONFileLabelsKey),
LabelsRegex: viper.GetString(jsonfile.JSONFileLabelsRegexKey),
Env: viper.GetString(jsonfile.JSONFileEnvKey),
EnvRegex: viper.GetString(jsonfile.JSONFileEnvRegexKey),
Tag: viper.GetString(jsonfile.JSONFileTagKey),
TagSpecified: isFlagPassed(jsonfile.JSONFileTagKey),
}, nil
}

// getSplunkArgs gets Splunk specified arguments for Splunk log driver.
func getSplunkArgs() (*splunk.Args, error) {
token, err := getSplunkToken()
Expand Down
95 changes: 95 additions & 0 deletions args_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (

"github.com/aws/shim-loggers-for-containerd/logger/awslogs"
"github.com/aws/shim-loggers-for-containerd/logger/fluentd"
"github.com/aws/shim-loggers-for-containerd/logger/jsonfile"
"github.com/aws/shim-loggers-for-containerd/logger/splunk"

"github.com/spf13/pflag"
Expand Down Expand Up @@ -777,3 +778,97 @@ func TestGetDockerConfigsWithEndpoint(t *testing.T) {
}
assert.DeepEqual(t, expectedEnv, gotEnv)
}

// TestGetJSONFileArgs covers the json-file driver's argument parsing:
// log-path is required; everything else is optional and forwarded as-is to moby.
// Also exercises the JSONFile* prefixed input-flag names and verifies they map to
// the moby-side option keys correctly downstream.
func TestGetJSONFileArgs(t *testing.T) {
const (
testLogPath = "/var/log/ecs/json-file/abc/abc-json.log"
testMaxSize = "10m"
testMaxFile = "5"
testCompress = "false"
testLabels = "label0,label1"
testLabelsRegex = "^app\\..*"
testEnv = "KEY1,KEY2"
testEnvRegex = "^APP_.*"
testTag = "{{.ImageName}}/{{.ID}}"
)

for _, tc := range []struct {
name string
logPath string
setOptional func()
expectErr bool
assertArgs func(t *testing.T, args *jsonfile.Args)
}{
{
name: "log-path required",
logPath: "",
expectErr: true,
},
{
name: "log-path only — optional fields default to empty",
logPath: testLogPath,
assertArgs: func(t *testing.T, args *jsonfile.Args) {
assert.Equal(t, testLogPath, args.LogPath)
assert.Equal(t, "", args.MaxSize)
assert.Equal(t, "", args.MaxFile)
assert.Equal(t, "", args.Compress)
assert.Equal(t, "", args.Labels)
assert.Equal(t, "", args.LabelsRegex)
assert.Equal(t, "", args.Env)
assert.Equal(t, "", args.EnvRegex)
assert.Equal(t, "", args.Tag)
assert.Equal(t, false, args.TagSpecified)
},
},
{
name: "all fields populated",
logPath: testLogPath,
setOptional: func() {
viper.Set(jsonfile.MaxSizeKey, testMaxSize)
viper.Set(jsonfile.MaxFileKey, testMaxFile)
viper.Set(jsonfile.CompressKey, testCompress)
viper.Set(jsonfile.JSONFileLabelsKey, testLabels)
viper.Set(jsonfile.JSONFileLabelsRegexKey, testLabelsRegex)
viper.Set(jsonfile.JSONFileEnvKey, testEnv)
viper.Set(jsonfile.JSONFileEnvRegexKey, testEnvRegex)
viper.Set(jsonfile.JSONFileTagKey, testTag)
},
assertArgs: func(t *testing.T, args *jsonfile.Args) {
assert.Equal(t, testLogPath, args.LogPath)
assert.Equal(t, testMaxSize, args.MaxSize)
assert.Equal(t, testMaxFile, args.MaxFile)
assert.Equal(t, testCompress, args.Compress)
assert.Equal(t, testLabels, args.Labels)
assert.Equal(t, testLabelsRegex, args.LabelsRegex)
assert.Equal(t, testEnv, args.Env)
assert.Equal(t, testEnvRegex, args.EnvRegex)
assert.Equal(t, testTag, args.Tag)
// Note: TagSpecified relies on isFlagPassed() inspecting pflag.CommandLine,
// not on viper.Set; we don't assert it here. See TestIsFlagPassed for that.
},
},
} {
t.Run(tc.name, func(t *testing.T) {
defer viper.Reset()

if tc.logPath != "" {
viper.Set(jsonfile.LogPathKey, tc.logPath)
}
if tc.setOptional != nil {
tc.setOptional()
}

args, err := getJSONFileArgs()
if tc.expectErr {
require.Error(t, err)
return
}
require.NoError(t, err)
tc.assertArgs(t, args)
})
}
}
11 changes: 10 additions & 1 deletion e2e/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ const (
AwslogsDriverName = "awslogs"
// FluentdDriverName is the name of fluentd driver.
FluentdDriverName = "fluentd"
// JSONFileDriverName is the name of the json-file driver.
JSONFileDriverName = "json-file"
// SplunkDriverName is the name of splunk driver.
SplunkDriverName = "splunk"
// ContainerIDKey is the key of the container id.
Expand All @@ -44,6 +46,13 @@ const (

// SendTestLogByContainerd sends a testLog to a specific shim logger by containerd.
func SendTestLogByContainerd(creator cio.Creator, testLog string) error {
return SendCommandByContainerd(creator, fmt.Sprintf("printf \"%s\"", testLog))
}

// SendCommandByContainerd runs an arbitrary shell command in a container with the given
// shim-logger creator. Use this when the test needs to control payload size or shape
// beyond a single printf (e.g., to force log rotation).
func SendCommandByContainerd(creator cio.Creator, shellCommand string) error {
// Create a new client connected to the containerd daemon
client, err := containerd.New(containerdAddress)
if err != nil {
Expand All @@ -59,7 +68,7 @@ func SendTestLogByContainerd(creator cio.Creator, testLog string) error {
} // Create a new container with the pulled image
container, err := client.NewContainer(ctx, TestContainerID, containerd.WithImage(image),
containerd.WithNewSnapshot("test-snapshot", image), containerd.WithNewSpec(oci.WithImageConfig(image),
oci.WithProcessArgs("/bin/sh", "-c", fmt.Sprintf("printf \"%s\"", testLog))))
oci.WithProcessArgs("/bin/sh", "-c", shellCommand)))
if err != nil {
return err
}
Expand Down
Loading
Loading