From 068b3c9adb36aa9b352f89dfe1b3d32dfc2fa0d0 Mon Sep 17 00:00:00 2001 From: Jose Diaz-Gonzalez Date: Fri, 25 Feb 2022 22:20:59 -0500 Subject: [PATCH 1/2] feat: set the default memory unit type to megabytes This makes it easier to support all schedulers out of the box - all other schedulers assume megabytes when no unit is specified. --- docs/deployment/schedulers/docker-local.md | 4 ++-- plugins/resource/functions.go | 17 +++++++++++++++++ plugins/resource/triggers.go | 2 ++ tests/unit/resource_1.bats | 12 ++++++++++++ 4 files changed, 33 insertions(+), 2 deletions(-) diff --git a/docs/deployment/schedulers/docker-local.md b/docs/deployment/schedulers/docker-local.md index 5a8188c8e..df2048f78 100644 --- a/docs/deployment/schedulers/docker-local.md +++ b/docs/deployment/schedulers/docker-local.md @@ -113,7 +113,7 @@ The `docker-local` scheduler supports a minimal list of resource _limits_ and _r - cpu: (docker option: `--cpus`), is specified in number of CPUs a process can access. - See the ["CPU" section](https://docs.docker.com/config/containers/resource_constraints/#cpu) of the Docker Runtime Options documentation for more information. -- memory: (docker option: `--memory`) should be specified with a suffix of `b` (bytes), `k` (kilobytes), `m` (megabytes), `g` (gigabytes). +- memory: (docker option: `--memory`) should be specified with a suffix of `b` (bytes), `k` (kilobytes), `m` (megabytes), `g` (gigabytes). Default unit is `m` (megabytes). - See the ["Memory" section](https://docs.docker.com/config/containers/resource_constraints/#memory) of the Docker Runtime Options documentation for more information. - memory-swap: (docker option: `--memory-swap`) should be specified with a suffix of `b` (bytes), `k` (kilobytes), `m` (megabytes), `g` (gigabytes) - See the ["Memory" section](https://docs.docker.com/config/containers/resource_constraints/#memory) of the Docker Runtime Options documentation for more information. @@ -122,5 +122,5 @@ The `docker-local` scheduler supports a minimal list of resource _limits_ and _r ### Resource Reservations -- memory: (docker option: `--memory-reservation`) should be specified with a suffix of `b` (bytes), `k` (kilobytes), `m` (megabytes), `g` (gigabytes) +- memory: (docker option: `--memory-reservation`) should be specified with a suffix of `b` (bytes), `k` (kilobytes), `m` (megabytes), `g` (gigabytes). Default unit is `m` (megabytes). - See the ["Memory" section](https://docs.docker.com/config/containers/resource_constraints/#memory) of the Docker Runtime Options documentation for more information. diff --git a/plugins/resource/functions.go b/plugins/resource/functions.go index 5d03442db..31c8d6775 100644 --- a/plugins/resource/functions.go +++ b/plugins/resource/functions.go @@ -2,10 +2,27 @@ package resource import ( "fmt" + "strings" "github.com/dokku/dokku/plugins/common" ) +func addMemorySuffixForDocker(key string, value string) string { + if key == "memory" { + hasSuffix := false + suffixes := []string{"b", "k", "m", "g"} + for _, suffix := range suffixes { + if strings.HasSuffix(value, suffix) { + hasSuffix = true + } + } + if !hasSuffix { + value = value + "m" + } + } + return value +} + func clearByResourceType(appName string, processType string, resourceType string) { noun := "limits" if resourceType == "reserve" { diff --git a/plugins/resource/triggers.go b/plugins/resource/triggers.go index 01db7f0d1..c80d264d4 100644 --- a/plugins/resource/triggers.go +++ b/plugins/resource/triggers.go @@ -75,6 +75,7 @@ func TriggerDockerArgsProcessDeploy(appName string, processType string) error { if value == "" { continue } + value = addMemorySuffixForDocker(key, value) fmt.Printf(" --%s=%s ", key, value) } @@ -82,6 +83,7 @@ func TriggerDockerArgsProcessDeploy(appName string, processType string) error { if value == "" { continue } + value = addMemorySuffixForDocker(key, value) fmt.Printf(" --%s-reservation=%s ", key, value) } diff --git a/tests/unit/resource_1.bats b/tests/unit/resource_1.bats index 54919bb02..85d2a19fb 100644 --- a/tests/unit/resource_1.bats +++ b/tests/unit/resource_1.bats @@ -51,6 +51,18 @@ teardown() { echo "status: $status" assert_output "536870912" + run /bin/bash -c "dokku resource:limit --memory 512 $TEST_APP" + echo "output: $output" + echo "status: $status" + assert_success + + dokku ps:rebuild "$TEST_APP" + CID=$(< $DOKKU_ROOT/$TEST_APP/CONTAINER.web.1) + run /bin/bash -c "docker inspect --format '{{.HostConfig.Memory}}' $CID" + echo "output: $output" + echo "status: $status" + assert_output "536870912" + run /bin/bash -c "dokku resource:limit --memory 1024MB --process-type worker $TEST_APP" echo "output: $output" echo "status: $status" From 16ff0328cdd72bdc8e603aa9e20f4db54812add4 Mon Sep 17 00:00:00 2001 From: Jose Diaz-Gonzalez Date: Fri, 25 Feb 2022 23:23:59 -0500 Subject: [PATCH 2/2] fix: also support KB, MB, GB, and B as valid suffixes --- plugins/resource/functions.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/resource/functions.go b/plugins/resource/functions.go index 31c8d6775..20369fae1 100644 --- a/plugins/resource/functions.go +++ b/plugins/resource/functions.go @@ -10,7 +10,7 @@ import ( func addMemorySuffixForDocker(key string, value string) string { if key == "memory" { hasSuffix := false - suffixes := []string{"b", "k", "m", "g"} + suffixes := []string{"b", "k", "m", "g", "KB", "MB", "GB"} for _, suffix := range suffixes { if strings.HasSuffix(value, suffix) { hasSuffix = true