From 05c48430c5cedbab55c6a2f5c9e8c8a08899257f Mon Sep 17 00:00:00 2001 From: Jose Diaz-Gonzalez Date: Wed, 21 Feb 2024 18:13:47 -0500 Subject: [PATCH 1/6] fix: correct issue where resource values were ignored --- plugins/scheduler-k3s/functions.go | 67 ++++++++++++++++++++++-------- 1 file changed, 50 insertions(+), 17 deletions(-) diff --git a/plugins/scheduler-k3s/functions.go b/plugins/scheduler-k3s/functions.go index 287dcd75b..11c681e0d 100644 --- a/plugins/scheduler-k3s/functions.go +++ b/plugins/scheduler-k3s/functions.go @@ -833,46 +833,66 @@ func getProcessResources(appName string, processType string) (ProcessResourcesMa Memory: "512Mi", }, } - cpuLimit, err := common.PlugnTriggerOutputAsString("resource-get-property", []string{appName, processType, "limit", "cpu"}...) - if err != nil && cpuLimit != "" && cpuLimit != "0" { - _, err := resource.ParseQuantity(cpuLimit) + + emptyValues := map[string]bool{ + "": true, + "0": true, + "0m": true, + "0Mi": true, + } + + result, err := common.CallPlugnTrigger(common.PlugnTriggerInput{ + Trigger: "resource-get-property", + Args: []string{appName, processType, "limit", "cpu"}, + }) + if err == nil && !emptyValues[result.StdoutContents()] { + quantity, err := resource.ParseQuantity(result.StdoutContents()) if err != nil { return ProcessResourcesMap{}, fmt.Errorf("Error parsing cpu limit: %w", err) } - processResources.Limits.CPU = cpuLimit + processResources.Limits.CPU = quantity.String() } nvidiaGpuLimit, err := common.PlugnTriggerOutputAsString("resource-get-property", []string{appName, processType, "limit", "nvidia-gpu"}...) - if err != nil && nvidiaGpuLimit != "" && nvidiaGpuLimit != "0" { + if err == nil && nvidiaGpuLimit != "" && nvidiaGpuLimit != "0" { _, err := resource.ParseQuantity(nvidiaGpuLimit) if err != nil { return ProcessResourcesMap{}, fmt.Errorf("Error parsing nvidia-gpu limit: %w", err) } processResources.Limits.NvidiaGPU = nvidiaGpuLimit } - memoryLimit, err := common.PlugnTriggerOutputAsString("resource-get-property", []string{appName, processType, "limit", "memory"}...) - if err != nil && memoryLimit != "" && memoryLimit != "0" { - _, err := resource.ParseQuantity(memoryLimit) + result, err = common.CallPlugnTrigger(common.PlugnTriggerInput{ + Trigger: "resource-get-property", + Args: []string{appName, processType, "limit", "memory"}, + }) + if err == nil && !emptyValues[result.StdoutContents()] { + quantity, err := parseMemoryQuantity(result.StdoutContents()) if err != nil { return ProcessResourcesMap{}, fmt.Errorf("Error parsing memory limit: %w", err) } - processResources.Limits.Memory = memoryLimit + processResources.Limits.Memory = quantity } - cpuRequest, err := common.PlugnTriggerOutputAsString("resource-get-property", []string{appName, processType, "reserve", "cpu"}...) - if err != nil && cpuRequest != "" && cpuRequest != "0" { - _, err := resource.ParseQuantity(cpuRequest) + result, err = common.CallPlugnTrigger(common.PlugnTriggerInput{ + Trigger: "resource-get-property", + Args: []string{appName, processType, "reserve", "cpu"}, + }) + if err == nil && !emptyValues[result.StdoutContents()] { + quantity, err := resource.ParseQuantity(result.StdoutContents()) if err != nil { return ProcessResourcesMap{}, fmt.Errorf("Error parsing cpu request: %w", err) } - processResources.Requests.CPU = cpuRequest + processResources.Requests.CPU = quantity.String() } - memoryRequest, err := common.PlugnTriggerOutputAsString("resource-get-property", []string{appName, processType, "reserve", "memory"}...) - if err != nil && memoryRequest != "" && memoryRequest != "0" { - _, err := resource.ParseQuantity(memoryRequest) + result, err = common.CallPlugnTrigger(common.PlugnTriggerInput{ + Trigger: "resource-get-property", + Args: []string{appName, processType, "reserve", "memory"}, + }) + if err == nil && !emptyValues[result.StdoutContents()] { + quantity, err := parseMemoryQuantity(result.StdoutContents()) if err != nil { return ProcessResourcesMap{}, fmt.Errorf("Error parsing memory request: %w", err) } - processResources.Requests.Memory = memoryRequest + processResources.Requests.Memory = quantity } return processResources, nil @@ -1182,6 +1202,19 @@ func kubernetesNodeToNode(node v1.Node) Node { } } +// parseMemoryQuantity parses a string into a valid memory quantity +func parseMemoryQuantity(input string) (string, error) { + if _, err := strconv.ParseInt(input, 10, 64); err == nil { + input = fmt.Sprintf("%sMi", input) + } + quantity, err := resource.ParseQuantity(input) + if err != nil { + return "", err + } + + return quantity.String(), nil +} + func uninstallHelperCommands(ctx context.Context) error { errs, _ := errgroup.WithContext(ctx) errs.Go(func() error { From 3843ac2de2bdb5f89d20da0a0e29d67c2f0211f7 Mon Sep 17 00:00:00 2001 From: Jose Diaz-Gonzalez Date: Wed, 21 Feb 2024 18:15:11 -0500 Subject: [PATCH 2/6] fix: do not print command directly and instead use LogWarn to send to stdout If we use the standard logging method from go-execute, any subprocess calls through plugn may capture the wrong output due to handling stdout. --- plugins/common/exec.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/plugins/common/exec.go b/plugins/common/exec.go index 2db72046d..84eaa8954 100644 --- a/plugins/common/exec.go +++ b/plugins/common/exec.go @@ -125,7 +125,11 @@ func CallExecCommandWithContext(ctx context.Context, input ExecCommandInput) (Ex } if os.Getenv("DOKKU_TRACE") == "1" { - cmd.PrintCommand = true + argsSt := "" + if len(cmd.Args) > 0 { + argsSt = strings.Join(cmd.Args, " ") + } + LogWarn(fmt.Sprintf("exec: %s %s", cmd.Command, argsSt)) } if input.Stdin != nil { From 0bd785e04e83f7580cce6f26eb24d5b5092d78cd Mon Sep 17 00:00:00 2001 From: Jose Diaz-Gonzalez Date: Wed, 21 Feb 2024 18:15:37 -0500 Subject: [PATCH 3/6] feat: add logging to command plugn trigger output when executed with trace mode on --- plugins/common/plugn.go | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/plugins/common/plugn.go b/plugins/common/plugn.go index 422dbfba3..8ce905ac5 100644 --- a/plugins/common/plugn.go +++ b/plugins/common/plugn.go @@ -2,7 +2,10 @@ package common import ( "context" + "fmt" "io" + "os" + "strings" ) // PlugnTriggerInput is the input for CallPlugnTrigger @@ -41,7 +44,7 @@ func CallPlugnTrigger(input PlugnTriggerInput) (ExecCommandResponse, error) { func CallPlugnTriggerWithContext(ctx context.Context, input PlugnTriggerInput) (ExecCommandResponse, error) { args := []string{"trigger", input.Trigger} args = append(args, input.Args...) - return CallExecCommandWithContext(ctx, ExecCommandInput{ + result, err := CallExecCommandWithContext(ctx, ExecCommandInput{ Command: "plugn", Args: args, DisableStdioBuffer: input.DisableStdioBuffer, @@ -51,4 +54,15 @@ func CallPlugnTriggerWithContext(ctx context.Context, input PlugnTriggerInput) ( StreamStdout: input.StreamStdout, StreamStderr: input.StreamStderr, }) + + if os.Getenv("DOKKU_TRACE") == "1" { + for _, line := range strings.Split(result.Stderr, "\n") { + LogDebug(fmt.Sprintf("plugn trigger %s stderr: %s", input.Trigger, line)) + } + for _, line := range strings.Split(result.Stdout, "\n") { + LogDebug(fmt.Sprintf("plugn trigger %s stdout: %s", input.Trigger, line)) + } + } + + return result, err } From 14eb60160cbdf0dbda6fbe75a4772d4733d1f940 Mon Sep 17 00:00:00 2001 From: Jose Diaz-Gonzalez Date: Wed, 21 Feb 2024 18:30:48 -0500 Subject: [PATCH 4/6] fix: avoid setting empty cpu/memory requests --- plugins/scheduler-k3s/functions.go | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/plugins/scheduler-k3s/functions.go b/plugins/scheduler-k3s/functions.go index 11c681e0d..e2756cf23 100644 --- a/plugins/scheduler-k3s/functions.go +++ b/plugins/scheduler-k3s/functions.go @@ -850,7 +850,9 @@ func getProcessResources(appName string, processType string) (ProcessResourcesMa if err != nil { return ProcessResourcesMap{}, fmt.Errorf("Error parsing cpu limit: %w", err) } - processResources.Limits.CPU = quantity.String() + if quantity.MilliValue() != 0 { + processResources.Limits.CPU = quantity.String() + } } nvidiaGpuLimit, err := common.PlugnTriggerOutputAsString("resource-get-property", []string{appName, processType, "limit", "nvidia-gpu"}...) if err == nil && nvidiaGpuLimit != "" && nvidiaGpuLimit != "0" { @@ -869,7 +871,9 @@ func getProcessResources(appName string, processType string) (ProcessResourcesMa if err != nil { return ProcessResourcesMap{}, fmt.Errorf("Error parsing memory limit: %w", err) } - processResources.Limits.Memory = quantity + if quantity != "0Mi" { + processResources.Limits.Memory = quantity + } } result, err = common.CallPlugnTrigger(common.PlugnTriggerInput{ @@ -881,7 +885,9 @@ func getProcessResources(appName string, processType string) (ProcessResourcesMa if err != nil { return ProcessResourcesMap{}, fmt.Errorf("Error parsing cpu request: %w", err) } - processResources.Requests.CPU = quantity.String() + if quantity.MilliValue() != 0 { + processResources.Requests.CPU = quantity.String() + } } result, err = common.CallPlugnTrigger(common.PlugnTriggerInput{ Trigger: "resource-get-property", @@ -892,7 +898,9 @@ func getProcessResources(appName string, processType string) (ProcessResourcesMa if err != nil { return ProcessResourcesMap{}, fmt.Errorf("Error parsing memory request: %w", err) } - processResources.Requests.Memory = quantity + if quantity != "0Mi" { + processResources.Requests.Memory = quantity + } } return processResources, nil From 193b1d810306636de5f2f309f8c4578f3c295fcd Mon Sep 17 00:00:00 2001 From: Jose Diaz-Gonzalez Date: Wed, 21 Feb 2024 18:31:04 -0500 Subject: [PATCH 5/6] fix: set lower cpu/memory requests --- docs/deployment/schedulers/k3s.md | 16 ++++------------ plugins/scheduler-k3s/functions.go | 9 +++------ 2 files changed, 7 insertions(+), 18 deletions(-) diff --git a/docs/deployment/schedulers/k3s.md b/docs/deployment/schedulers/k3s.md index f21315f53..573a2eb2d 100644 --- a/docs/deployment/schedulers/k3s.md +++ b/docs/deployment/schedulers/k3s.md @@ -361,20 +361,12 @@ App logs for the `logs` command are fetched by Dokku from running containers via ### Supported Resource Management Properties -The `k3s` scheduler supports a minimal list of resource _limits_ and _reservations_. The following properties are supported: - -#### Resource Limits - -> [!NOTE] -> Cron tasks retrieve resource limits based on the computed cron task ID. If unspecified, the default will be 1 CPU and 512m RAM. +The `k3s` scheduler supports a minimal list of resource _limits_ and _reservations_: - cpu: is specified in number of CPUs a process can access. -- memory: should be specified with a suffix of `b` (bytes), `k` (kilobytes), `m` (megabytes), `g` (gigabytes). Default unit is `m` (megabytes). +- memory: should be specified with a suffix of `b` (bytes), `Ki` (kilobytes), `Mi` (megabytes), `Gi` (gigabytes). Default unit is `Mi` (megabytes). -#### Resource Reservations +If unspecified for any task, the default reservation will be .1 CPU and 128Mi RAM, with no limit set for either CPU or RAM. This is to avoid issues with overscheduling pods on a cluster. To avoid issues, set more specific values for at least resource reservations. > [!NOTE] -> Cron tasks retrieve resource reservations based on the computed cron task ID. If unspecified, the default will be 1 CPU and 512m RAM. - -- cpu: is specified in number of CPUs a process can access. -- memory: should be specified with a suffix of `b` (bytes), `k` (kilobytes), `m` (megabytes), `g` (gigabytes). Default unit is `m` (megabytes). +> Cron tasks retrieve resource limits based on the computed cron task ID. diff --git a/plugins/scheduler-k3s/functions.go b/plugins/scheduler-k3s/functions.go index e2756cf23..4c8b17355 100644 --- a/plugins/scheduler-k3s/functions.go +++ b/plugins/scheduler-k3s/functions.go @@ -824,13 +824,10 @@ func getProcessHealtchecks(healthchecks []appjson.Healthcheck, primaryPort int32 func getProcessResources(appName string, processType string) (ProcessResourcesMap, error) { processResources := ProcessResourcesMap{ - Limits: ProcessResources{ - CPU: "1000m", - Memory: "512Mi", - }, + Limits: ProcessResources{}, Requests: ProcessResources{ - CPU: "1000m", - Memory: "512Mi", + CPU: "100m", + Memory: "128Mi", }, } From 93323bd0ff880c421e121f5d274cca54b933a2bd Mon Sep 17 00:00:00 2001 From: Jose Diaz-Gonzalez Date: Thu, 22 Feb 2024 06:55:02 -0500 Subject: [PATCH 6/6] feat: add ability to clear out default values Also add some tests --- docs/deployment/schedulers/k3s.md | 2 +- plugins/scheduler-k3s/functions.go | 14 +++-- tests/unit/scheduler-k3s.bats | 89 ++++++++++++++++++++++++------ 3 files changed, 83 insertions(+), 22 deletions(-) diff --git a/docs/deployment/schedulers/k3s.md b/docs/deployment/schedulers/k3s.md index 573a2eb2d..a63ca3e3a 100644 --- a/docs/deployment/schedulers/k3s.md +++ b/docs/deployment/schedulers/k3s.md @@ -366,7 +366,7 @@ The `k3s` scheduler supports a minimal list of resource _limits_ and _reservatio - cpu: is specified in number of CPUs a process can access. - memory: should be specified with a suffix of `b` (bytes), `Ki` (kilobytes), `Mi` (megabytes), `Gi` (gigabytes). Default unit is `Mi` (megabytes). -If unspecified for any task, the default reservation will be .1 CPU and 128Mi RAM, with no limit set for either CPU or RAM. This is to avoid issues with overscheduling pods on a cluster. To avoid issues, set more specific values for at least resource reservations. +If unspecified for any task, the default reservation will be `.1` CPU and `128Mi` RAM, with no limit set for either CPU or RAM. This is to avoid issues with overscheduling pods on a cluster. To avoid issues, set more specific values for at least resource reservations. If unbounded utilization is desired, set CPU and Memory to `0m` and `0Mi`, respectively. > [!NOTE] > Cron tasks retrieve resource limits based on the computed cron task ID. diff --git a/plugins/scheduler-k3s/functions.go b/plugins/scheduler-k3s/functions.go index 4c8b17355..365d1268d 100644 --- a/plugins/scheduler-k3s/functions.go +++ b/plugins/scheduler-k3s/functions.go @@ -832,10 +832,8 @@ func getProcessResources(appName string, processType string) (ProcessResourcesMa } emptyValues := map[string]bool{ - "": true, - "0": true, - "0m": true, - "0Mi": true, + "": true, + "0": true, } result, err := common.CallPlugnTrigger(common.PlugnTriggerInput{ @@ -849,6 +847,8 @@ func getProcessResources(appName string, processType string) (ProcessResourcesMa } if quantity.MilliValue() != 0 { processResources.Limits.CPU = quantity.String() + } else { + processResources.Limits.CPU = "" } } nvidiaGpuLimit, err := common.PlugnTriggerOutputAsString("resource-get-property", []string{appName, processType, "limit", "nvidia-gpu"}...) @@ -870,6 +870,8 @@ func getProcessResources(appName string, processType string) (ProcessResourcesMa } if quantity != "0Mi" { processResources.Limits.Memory = quantity + } else { + processResources.Limits.Memory = "" } } @@ -884,6 +886,8 @@ func getProcessResources(appName string, processType string) (ProcessResourcesMa } if quantity.MilliValue() != 0 { processResources.Requests.CPU = quantity.String() + } else { + processResources.Requests.CPU = "" } } result, err = common.CallPlugnTrigger(common.PlugnTriggerInput{ @@ -897,6 +901,8 @@ func getProcessResources(appName string, processType string) (ProcessResourcesMa } if quantity != "0Mi" { processResources.Requests.Memory = quantity + } else { + processResources.Requests.Memory = "" } } diff --git a/tests/unit/scheduler-k3s.bats b/tests/unit/scheduler-k3s.bats index 6b0c9b8c1..c78e1f785 100644 --- a/tests/unit/scheduler-k3s.bats +++ b/tests/unit/scheduler-k3s.bats @@ -70,22 +70,6 @@ uninstall_k3s() { assert_success } -@test "(scheduler-k3s) install traefik" { - if [[ -z "$DOCKERHUB_USERNAME" ]] || [[ -z "$DOCKERHUB_TOKEN" ]]; then - skip "skipping due to missing docker.io credentials DOCKERHUB_USERNAME:DOCKERHUB_TOKEN" - fi - - install_k3s -} - -@test "(scheduler-k3s) install nginx" { - if [[ -z "$DOCKERHUB_USERNAME" ]] || [[ -z "$DOCKERHUB_TOKEN" ]]; then - skip "skipping due to missing docker.io credentials DOCKERHUB_USERNAME:DOCKERHUB_TOKEN" - fi - - INGRESS_CLASS=nginx install_k3s -} - @test "(scheduler-k3s) install traefik with taint" { if [[ -z "$DOCKERHUB_USERNAME" ]] || [[ -z "$DOCKERHUB_TOKEN" ]]; then skip "skipping due to missing docker.io credentials DOCKERHUB_USERNAME:DOCKERHUB_TOKEN" @@ -102,7 +86,7 @@ uninstall_k3s() { INGRESS_CLASS=nginx TAINT_SCHEDULING=true install_k3s } -@test "(scheduler-k3s) deploy traefik" { +@test "(scheduler-k3s) deploy traefik [resource]" { if [[ -z "$DOCKERHUB_USERNAME" ]] || [[ -z "$DOCKERHUB_TOKEN" ]]; then skip "skipping due to missing docker.io credentials DOCKERHUB_USERNAME:DOCKERHUB_TOKEN" fi @@ -130,6 +114,77 @@ uninstall_k3s() { assert_success assert_http_localhost_response "http" "$TEST_APP.dokku.me" "80" "" "python/http.server" + + # include resource tests + run /bin/bash -c "kubectl get pods -o=jsonpath='{.items[*]..resources.requests.cpu}'" + echo "output: $output" + echo "status: $status" + assert_success + assert_output "100m" + + run /bin/bash -c "kubectl get pods -o=jsonpath='{.items[*]..resources.requests.memory}'" + echo "output: $output" + echo "status: $status" + assert_success + assert_output "128Mi" + + run /bin/bash -c "kubectl get pods -o=jsonpath='{.items[*]..resources.limits.cpu}'" + echo "output: $output" + echo "status: $status" + assert_success + assert_output "" + + run /bin/bash -c "kubectl get pods -o=jsonpath='{.items[*]..resources.limits.memory}'" + echo "output: $output" + echo "status: $status" + assert_success + assert_output "" + + run /bin/bash -c "dokku resource:reserve $TEST_APP --memory 300 --cpu 0m --process-type web" + echo "output: $output" + echo "status: $status" + assert_success + + run /bin/bash -c "dokku resource:limit $TEST_APP --memory 512 --process-type web" + echo "output: $output" + echo "status: $status" + assert_success + + run /bin/bash -c "dokku ps:rebuild $TEST_APP" + echo "output: $output" + echo "status: $status" + assert_success + + run /bin/bash -c "sleep 20" + echo "output: $output" + echo "status: $status" + assert_success + + assert_http_localhost_response "http" "$TEST_APP.dokku.me" "80" "" "python/http.server" + + run /bin/bash -c "kubectl get pods -o=jsonpath='{.items[*]..resources.requests.cpu}'" + echo "output: $output" + echo "status: $status" + assert_success + assert_output "" + + run /bin/bash -c "kubectl get pods -o=jsonpath='{.items[*]..resources.requests.memory}'" + echo "output: $output" + echo "status: $status" + assert_success + assert_output "300Mi" + + run /bin/bash -c "kubectl get pods -o=jsonpath='{.items[*]..resources.limits.cpu}'" + echo "output: $output" + echo "status: $status" + assert_success + assert_output "" + + run /bin/bash -c "kubectl get pods -o=jsonpath='{.items[*]..resources.limits.memory}'" + echo "output: $output" + echo "status: $status" + assert_success + assert_output "512Mi" } @test "(scheduler-k3s) deploy nginx" {