From 065f5e59223bc12fe37661f5a2b48e14821ceb8d Mon Sep 17 00:00:00 2001 From: Jose Diaz-Gonzalez Date: Sun, 7 Feb 2021 22:09:29 -0500 Subject: [PATCH] fix: do not inject max-size option when not using local or json-file log drivers Closes #4376 --- docs/appendices/0.23.0-migration-guide.md | 1 + plugins/logs/go.mod | 2 + plugins/logs/go.sum | 1 + plugins/logs/triggers.go | 37 +++++++++++++--- tests/unit/cron.bats | 1 - tests/unit/logs.bats | 52 +++++++++++++++++++++++ 6 files changed, 88 insertions(+), 6 deletions(-) diff --git a/docs/appendices/0.23.0-migration-guide.md b/docs/appendices/0.23.0-migration-guide.md index 05f8d353d..b46cb8bc4 100644 --- a/docs/appendices/0.23.0-migration-guide.md +++ b/docs/appendices/0.23.0-migration-guide.md @@ -5,3 +5,4 @@ - The `plugin:list` command no longer outputs the version for the `plugn` binary. - Users building docker images that run Dokku will need to use a new sudoer wrapper for the `docker-image-labeler` binary to work correctly. A reference version has been placed in the `docker` skeleton directory. This should only impact platform developers, and users of our Docker image will already have the file available. - The `dokku` user's cron is now in use by Dokku itself. Customizations will be overwritten. Users are encouraged to use a cron entry in `/etc/cron.d/dokku` to avoid this issue. +- As of 0.23.0, Dokku will now inject the `max-size` log driver option for applications. This is restricted to app-configured log driver values empty, `local` or `json-file` in 0.23.1 to increase setup compatibility. Users who configure alternative log drivers at the system level will need to either set the global `max-size` property to `unlimited` or switch to the built-in `vector` logging support. diff --git a/plugins/logs/go.mod b/plugins/logs/go.mod index 5f0bdacb7..ba17db430 100644 --- a/plugins/logs/go.mod +++ b/plugins/logs/go.mod @@ -5,9 +5,11 @@ go 1.15 require ( github.com/codeskyblue/go-sh v0.0.0-20190412065543-76bd3d59ff27 github.com/dokku/dokku/plugins/common v0.0.0-00010101000000-000000000000 + github.com/dokku/dokku/plugins/docker-options v0.0.0-20210208020425-f7beb3d95ddd github.com/joncalhoun/qson v0.0.0-20200422171543-84433dcd3da0 github.com/ryanuber/columnize v1.1.2-0.20190319233515-9e6335e58db3 // indirect github.com/spf13/pflag v1.0.5 // indirect ) replace github.com/dokku/dokku/plugins/common => ../common +replace github.com/dokku/dokku/plugins/docker-options => ../docker-options diff --git a/plugins/logs/go.sum b/plugins/logs/go.sum index 00274255e..f7d6d2bd7 100644 --- a/plugins/logs/go.sum +++ b/plugins/logs/go.sum @@ -4,6 +4,7 @@ github.com/codeskyblue/go-sh v0.0.0-20190412065543-76bd3d59ff27 h1:HHUr4P/aKh4qu github.com/codeskyblue/go-sh v0.0.0-20190412065543-76bd3d59ff27/go.mod h1:VQx0hjo2oUeQkQUET7wRwradO6f+fN5jzXgB/zROxxE= github.com/joncalhoun/qson v0.0.0-20200422171543-84433dcd3da0 h1:ct2XA1aDw8A07Dr8gtrrZgIgLKcZNAl2o9nn0WRMK4Y= github.com/joncalhoun/qson v0.0.0-20200422171543-84433dcd3da0/go.mod h1:DFXrEwSRX0p/aSvxE21319menCBFeQO0jXpRj7LEZUA= +github.com/kballard/go-shellquote v0.0.0-20180428030007-95032a82bc51/go.mod h1:CzGEWj7cYgsdH8dAjBGEr58BoE7ScuLd+fwFZ44+/x8= github.com/ryanuber/columnize v1.1.2-0.20190319233515-9e6335e58db3 h1:utdYOikI1XjNtTFGCwSM6OmFJblU4ld4gACoJsbadJg= github.com/ryanuber/columnize v1.1.2-0.20190319233515-9e6335e58db3/go.mod h1:sm1tb6uqfes/u+d4ooFouqFdy9/2g9QGwK3SQygK0Ts= github.com/spf13/pflag v1.0.5 h1:iy+VFUOCP1a+8yFto/drg2CJ5u0yRoB7fZw3DKv/JXA= diff --git a/plugins/logs/triggers.go b/plugins/logs/triggers.go index 175eb0c6e..fdadb55b7 100644 --- a/plugins/logs/triggers.go +++ b/plugins/logs/triggers.go @@ -6,8 +6,10 @@ import ( "io/ioutil" "os" "path/filepath" + "strings" "github.com/dokku/dokku/plugins/common" + dockeroptions "github.com/dokku/dokku/plugins/docker-options" ) // TriggerDockerArgsProcessDeploy outputs the logs plugin docker options for an app @@ -17,13 +19,38 @@ func TriggerDockerArgsProcessDeploy(appName string) error { return err } - maxSize := common.PropertyGet("logs", appName, "max-size") - if maxSize == "" { - maxSize = common.PropertyGetDefault("logs", "--global", "max-size", MaxSize) + allowedDrivers := map[string]bool{ + "local": true, + "json-file": true, } - if maxSize != "unlimited" { - fmt.Printf(" --log-opt max-size=%s ", maxSize) + ignoreMaxSize := false + options, err := dockeroptions.GetDockerOptionsForPhase(appName, "deploy") + if err != nil { + return err + } + + for _, option := range options { + if !strings.HasPrefix(option, "--log-driver=") { + continue + } + + logDriver := strings.TrimPrefix(option, "--log-driver=") + if !allowedDrivers[logDriver] { + ignoreMaxSize = true + } + break + } + + if !ignoreMaxSize { + maxSize := common.PropertyGet("logs", appName, "max-size") + if maxSize == "" { + maxSize = common.PropertyGetDefault("logs", "--global", "max-size", MaxSize) + } + + if maxSize != "unlimited" { + fmt.Printf(" --log-opt max-size=%s ", maxSize) + } } fmt.Print(string(stdin)) diff --git a/tests/unit/cron.bats b/tests/unit/cron.bats index 75aecb55c..4595cf3b2 100644 --- a/tests/unit/cron.bats +++ b/tests/unit/cron.bats @@ -119,7 +119,6 @@ teardown() { echo "status: $status" assert_success - run /bin/bash -c "cat /var/spool/cron/crontabs/dokku" echo "output: $output" echo "status: $status" diff --git a/tests/unit/logs.bats b/tests/unit/logs.bats index d0d3ea7e5..f32084ad8 100644 --- a/tests/unit/logs.bats +++ b/tests/unit/logs.bats @@ -369,6 +369,58 @@ teardown() { assert_output "10m" } +@test "(logs) logs:set max-size with alternate log-driver" { + run create_app + echo "output: $output" + echo "status: $status" + assert_success + + run /bin/bash -c "dokku logs:set $TEST_APP max-size 20m" 2>&1 + echo "output: $output" + echo "status: $status" + assert_success + assert_output_contains "Setting max-size" + + run /bin/bash -c "echo "" | dokku plugin:trigger docker-args-process-deploy $TEST_APP 2>&1 | xargs" + echo "output: $output" + echo "status: $status" + assert_success + assert_output "--log-opt max-size=20m" + + run /bin/bash -c "dokku docker-options:add $TEST_APP deploy --log-driver=local" 2>&1 + echo "output: $output" + echo "status: $status" + assert_success + + run /bin/bash -c "echo "" | dokku plugin:trigger docker-args-process-deploy $TEST_APP 2>&1 | xargs" + echo "output: $output" + echo "status: $status" + assert_success + assert_output "--log-opt max-size=20m" + + run /bin/bash -c "dokku docker-options:add $TEST_APP deploy --log-driver=json-file" 2>&1 + echo "output: $output" + echo "status: $status" + assert_success + + run /bin/bash -c "echo "" | dokku plugin:trigger docker-args-process-deploy $TEST_APP 2>&1 | xargs" + echo "output: $output" + echo "status: $status" + assert_success + assert_output "--log-opt max-size=20m" + + run /bin/bash -c "dokku docker-options:add $TEST_APP deploy --log-driver=journald" 2>&1 + echo "output: $output" + echo "status: $status" + assert_success + + run /bin/bash -c "echo "" | dokku plugin:trigger docker-args-process-deploy $TEST_APP 2>&1" + echo "output: $output" + echo "status: $status" + assert_success + assert_output "" +} + @test "(logs) logs:vector" { run /bin/bash -c "dokku logs:vector-logs 2>&1" echo "output: $output"