From 00c3e48cf4192af3b014a87cc00b23a8aba17d69 Mon Sep 17 00:00:00 2001 From: Jose Diaz-Gonzalez Date: Sat, 24 Oct 2026 19:34:13 -0400 Subject: [PATCH] feat: add a whole-set form and clear for chart overrides The `scheduler-k3s:charts:set` command only changed one chart override at a time, so matching a chart's stored overrides to a declared set meant reading the report, computing the difference in both directions and issuing one command per added, changed and removed property, with a failure partway through leaving a mixture of the two sets. A `--replace` flag on `scheduler-k3s:charts:set` now takes a chart name followed by `property=value` pairs and writes the declared map for that chart in a single call. Every pair is parsed before anything is written, so a rejected pair leaves the stored overrides untouched, and a pair ending in `=` stores an empty value, which the per-property form cannot express. An empty pair list is rejected rather than treated as a request to remove everything, and the new `scheduler-k3s:charts:clear` command handles that case by removing every override for a chart. --- docs/deployment/schedulers/k3s.md | 33 ++++++ plugins/scheduler-k3s/Makefile | 2 +- plugins/scheduler-k3s/functions.go | 14 +++ plugins/scheduler-k3s/functions_test.go | 32 ++++++ .../scheduler-k3s/src/commands/commands.go | 2 + .../src/subcommands/subcommands.go | 22 +++- plugins/scheduler-k3s/subcommands.go | 106 ++++++++++++------ tests/unit/scheduler-k3s-charts-1.bats | 5 +- tests/unit/scheduler-k3s-charts-2.bats | 5 +- tests/unit/scheduler-k3s-charts-3.bats | 99 ++++++++++++++++ 10 files changed, 278 insertions(+), 42 deletions(-) create mode 100644 tests/unit/scheduler-k3s-charts-3.bats diff --git a/docs/deployment/schedulers/k3s.md b/docs/deployment/schedulers/k3s.md index 9883af10e..e51991d5b 100644 --- a/docs/deployment/schedulers/k3s.md +++ b/docs/deployment/schedulers/k3s.md @@ -11,8 +11,10 @@ scheduler-k3s:annotations:report [|--global] [--format stdout|json] [--proc scheduler-k3s:autoscaling-auth:set [<--metadata key=value>...] # Set or clear a scheduler-k3s autoscaling keda trigger authentication resource for an app scheduler-k3s:autoscaling-auth:set --replace <--metadata key=value> [<--metadata key=value>...] # Replace the entire metadata map for a scheduler-k3s autoscaling keda trigger authentication resource scheduler-k3s:autoscaling-auth:report [|--global] [--format stdout|json] [--include-metadata] # Displays a scheduler-k3s autoscaling auth report for one or more apps +scheduler-k3s:charts:clear # Clear all chart-specific helm values for a chart scheduler-k3s:charts:report [] [--format stdout|json] # Displays a scheduler-k3s chart override report scheduler-k3s:charts:set () # Set or clear a chart-specific helm value +scheduler-k3s:charts:set --replace [ ...] # Replace the entire helm value override map for a chart scheduler-k3s:cluster:add [--profile PROFILE] [--role ROLE] [--insecure-allow-unknown-hosts] [--server-ip SERVER_IP] [--taint-scheduling] [--kubelet-args KUBELET_ARGS] # Adds a server node to a Dokku-managed cluster scheduler-k3s:cluster:list [--format json|stdout] # Lists all nodes in a Dokku-managed cluster scheduler-k3s:cluster:remove [node-id] # Removes client node to a Dokku-managed cluster @@ -1174,6 +1176,37 @@ Alternatively, a comma separated list of chart names can be specified to only fo dokku scheduler-k3s:ensure-charts --charts cert-manager ``` +#### Replacing the entire chart override set + +The per-property form above changes one chart override at a time, so matching the stored overrides for a chart to a declared set means computing the difference and issuing one command per added, changed and removed property. The `--replace` flag writes the whole override map for a chart in a single call, taking the chart name followed by `property=value` pairs instead of a single `.` argument and value. + +```shell +dokku scheduler-k3s:charts:set --replace cert-manager installCRDs=false replicaCount=2 +``` + +Anything previously stored for that chart and not named in the call is removed. Overrides for other charts are left untouched. + +Every pair is parsed before anything is written, so a rejected pair leaves the stored overrides untouched. A pair without a `=`, a pair with an empty property, and a property named more than once are all rejected. Only the first `=` in a pair separates the property from the value, so values may themselves contain `=`. + +A pair ending in `=` stores an empty value, which the per-property form cannot express - there, an empty value deletes the property instead. + +```shell +dokku scheduler-k3s:charts:set --replace cert-manager installCRDs= +``` + +> [!NOTE] +> An empty pair list is rejected rather than treated as a request to remove everything, so a generated list that expands to nothing cannot silently drop a chart's overrides. Use `scheduler-k3s:charts:clear` for that. + +#### Clearing chart overrides + +The `scheduler-k3s:charts:clear` command removes every override configured for a chart. + +```shell +dokku scheduler-k3s:charts:clear cert-manager +``` + +As with any other chart property change, a `scheduler-k3s:ensure-charts --force` or `scheduler-k3s:ensure-charts --charts ` call is required after replacing or clearing chart overrides in order to have them apply. + ### Persistent storage K3s apps mount persistent volumes through the Dokku storage plugin. A `storage:create --scheduler k3s` call provisions a PersistentVolumeClaim through a small per-entry helm release; `storage:mount --container-dir ` then attaches it to an app. Because storage entries are global, the same PVC can be mounted into multiple apps (for `ReadWriteMany` workloads) or into the same app at multiple subpaths. diff --git a/plugins/scheduler-k3s/Makefile b/plugins/scheduler-k3s/Makefile index d175aa3ad..fd83593a9 100644 --- a/plugins/scheduler-k3s/Makefile +++ b/plugins/scheduler-k3s/Makefile @@ -1,4 +1,4 @@ -SUBCOMMANDS = subcommands/annotations:clear subcommands/annotations:set subcommands/annotations:report subcommands/autoscaling-auth:set subcommands/autoscaling-auth:report subcommands/charts:report subcommands/charts:set subcommands/cluster:add subcommands/cluster:list subcommands/cluster:remove subcommands/ensure-charts subcommands/initialize subcommands/labels:clear subcommands/labels:set subcommands/labels:report subcommands/node-sysctls:clear subcommands/node-sysctls:set subcommands/node-sysctls:report subcommands/preview subcommands/profiles:add subcommands/profiles:list subcommands/profiles:remove subcommands/report subcommands/set subcommands/show-kubeconfig subcommands/uninstall +SUBCOMMANDS = subcommands/annotations:clear subcommands/annotations:set subcommands/annotations:report subcommands/autoscaling-auth:set subcommands/autoscaling-auth:report subcommands/charts:clear subcommands/charts:report subcommands/charts:set subcommands/cluster:add subcommands/cluster:list subcommands/cluster:remove subcommands/ensure-charts subcommands/initialize subcommands/labels:clear subcommands/labels:set subcommands/labels:report subcommands/node-sysctls:clear subcommands/node-sysctls:set subcommands/node-sysctls:report subcommands/preview subcommands/profiles:add subcommands/profiles:list subcommands/profiles:remove subcommands/report subcommands/set subcommands/show-kubeconfig subcommands/uninstall TRIGGERS = triggers/core-post-deploy triggers/core-post-extract triggers/install triggers/post-app-clone-setup triggers/post-app-rename-setup triggers/post-certs-update triggers/post-certs-remove triggers/post-create triggers/post-delete triggers/report triggers/scheduler-app-status triggers/scheduler-deploy triggers/scheduler-enter triggers/scheduler-is-deployed triggers/scheduler-logs triggers/scheduler-proxy-config triggers/scheduler-proxy-logs triggers/scheduler-post-delete triggers/scheduler-run triggers/scheduler-run-list triggers/scheduler-stop triggers/scheduler-cron-write triggers/scheduler-uses-host-cron triggers/storage-create triggers/storage-destroy triggers/storage-status triggers/scheduler-storage-exec BUILD = commands subcommands triggers PLUGIN_NAME = scheduler-k3s diff --git a/plugins/scheduler-k3s/functions.go b/plugins/scheduler-k3s/functions.go index c996287b1..ff13cf359 100644 --- a/plugins/scheduler-k3s/functions.go +++ b/plugins/scheduler-k3s/functions.go @@ -820,6 +820,20 @@ func getComputedDeployTimeout(appName string) string { return deployTimeout } +// getHelmChart returns the helm chart managed by dokku with the given release name +func getHelmChart(chartName string) (HelmChart, error) { + knownCharts := []string{} + for _, chart := range HelmCharts { + if chart.ReleaseName == chartName { + return chart, nil + } + knownCharts = append(knownCharts, chart.ReleaseName) + } + + sort.Strings(knownCharts) + return HelmChart{}, fmt.Errorf("Invalid chart name %q, valid charts: %s", chartName, strings.Join(knownCharts, ", ")) +} + func getImagePullSecrets(appName string) string { return common.PropertyGet("scheduler-k3s", appName, "image-pull-secrets") } diff --git a/plugins/scheduler-k3s/functions_test.go b/plugins/scheduler-k3s/functions_test.go index 8e343e9a2..e17926fd6 100644 --- a/plugins/scheduler-k3s/functions_test.go +++ b/plugins/scheduler-k3s/functions_test.go @@ -433,6 +433,38 @@ func TestValidateCertIssuerName(t *testing.T) { } } +func TestGetHelmChart(t *testing.T) { + for _, chart := range HelmCharts { + got, err := getHelmChart(chart.ReleaseName) + if err != nil { + t.Fatalf("getHelmChart(%q) unexpected error: %v", chart.ReleaseName, err) + } + if got.ReleaseName != chart.ReleaseName { + t.Fatalf("getHelmChart(%q) returned chart %q", chart.ReleaseName, got.ReleaseName) + } + } + + for _, name := range []string{"", "not-a-chart", "Traefik"} { + _, err := getHelmChart(name) + if err == nil { + t.Fatalf("getHelmChart(%q) expected an error", name) + } + if !strings.HasPrefix(err.Error(), fmt.Sprintf("Invalid chart name %q, valid charts: ", name)) { + t.Fatalf("getHelmChart(%q) unexpected error: %v", name, err) + } + + listed := strings.Split(strings.SplitN(err.Error(), "valid charts: ", 2)[1], ", ") + if len(listed) != len(HelmCharts) { + t.Fatalf("getHelmChart(%q) listed %d charts, want %d", name, len(listed), len(HelmCharts)) + } + for i := 1; i < len(listed); i++ { + if listed[i-1] > listed[i] { + t.Fatalf("getHelmChart(%q) listed charts out of order: %v", name, listed) + } + } + } +} + func TestValidateLetsencryptServer(t *testing.T) { cases := []struct { name string diff --git a/plugins/scheduler-k3s/src/commands/commands.go b/plugins/scheduler-k3s/src/commands/commands.go index fa22ac357..32a39951a 100644 --- a/plugins/scheduler-k3s/src/commands/commands.go +++ b/plugins/scheduler-k3s/src/commands/commands.go @@ -24,8 +24,10 @@ Additional commands:` scheduler-k3s:autoscaling-auth:set [<--metadata key=value>...], Set or clear a scheduler-k3s autoscaling keda trigger authentication resource for an app scheduler-k3s:autoscaling-auth:set --replace <--metadata key=value> [<--metadata key=value>...], Replace the entire metadata map for a scheduler-k3s autoscaling keda trigger authentication resource scheduler-k3s:autoscaling-auth:report [|--global] [--format stdout|json] [--include-metadata], Displays a scheduler-k3s autoscaling auth report for one or more apps + scheduler-k3s:charts:clear , Clear all chart-specific helm values for a chart scheduler-k3s:charts:report [] [--format stdout|json], Displays a scheduler-k3s chart override report scheduler-k3s:charts:set (), Set or clear a chart-specific helm value + scheduler-k3s:charts:set --replace [ ...], Replace the entire helm value override map for a chart scheduler-k3s:cluster:add [--profile PROFILE] [--role ROLE] [--insecure-allow-unknown-hosts] [--server-ip SERVER_IP] [--taint-scheduling] [--kubelet-args KUBELET_ARGS] , Adds a server node to a Dokku-managed cluster scheduler-k3s:cluster:list [--format json|stdout], Lists all nodes in a Dokku-managed cluster scheduler-k3s:cluster:remove [node-id], Removes client node to a Dokku-managed cluster diff --git a/plugins/scheduler-k3s/src/subcommands/subcommands.go b/plugins/scheduler-k3s/src/subcommands/subcommands.go index 65d117521..15aa5d301 100644 --- a/plugins/scheduler-k3s/src/subcommands/subcommands.go +++ b/plugins/scheduler-k3s/src/subcommands/subcommands.go @@ -108,12 +108,28 @@ func main() { appName = "--global" } err = scheduler_k3s.CommandAutoscalingAuthReport(appName, *format, *includeMetadata, infoFlag) + case "charts:clear": + args := flag.NewFlagSet("scheduler-k3s:charts:clear", flag.ExitOnError) + args.Parse(os.Args[2:]) + chartName := args.Arg(0) + err = scheduler_k3s.CommandChartsClear(chartName) case "charts:set": args := flag.NewFlagSet("scheduler-k3s:charts:set", flag.ExitOnError) + replace := args.Bool("replace", false, "--replace: replace the entire override map for the chart") args.Parse(os.Args[2:]) - property := args.Arg(0) - value := args.Arg(1) - err = scheduler_k3s.CommandChartsSet(property, value) + input := scheduler_k3s.ChartsSetInput{ + Replace: *replace, + } + + if *replace { + input.ChartName = args.Arg(0) + input.Pairs = common.VarArgs(args.Args(), 1) + } else { + input.Property = args.Arg(0) + input.Value = args.Arg(1) + } + + err = scheduler_k3s.CommandChartsSet(input) case "charts:report": args := flag.NewFlagSet("scheduler-k3s:charts:report", flag.ExitOnError) format := args.String("format", "stdout", "format: [ stdout | json ]") diff --git a/plugins/scheduler-k3s/subcommands.go b/plugins/scheduler-k3s/subcommands.go index a6e506fb8..d654cdd4e 100644 --- a/plugins/scheduler-k3s/subcommands.go +++ b/plugins/scheduler-k3s/subcommands.go @@ -616,8 +616,44 @@ func CommandInitialize(ingressClass string, serverIP string, taintScheduling boo return nil } -// CommandChartsSet sets or clears a chart-specific helm value for the scheduler -func CommandChartsSet(propertyArg string, value string) error { +// ChartsSetInput captures the inputs accepted by charts:set. +// Property/Value and ChartName/Pairs are mutually exclusive: ChartName and +// Pairs are populated only under --replace, where the positional arguments are +// a chart name followed by property=value pairs rather than a single +// . argument and value. +type ChartsSetInput struct { + ChartName string + Property string + Value string + Pairs []string + Replace bool +} + +// CommandChartsClear clears every chart-specific helm value for a chart +func CommandChartsClear(chartName string) error { + if chartName == "" { + return fmt.Errorf("Missing chart name") + } + + chart, err := getHelmChart(chartName) + if err != nil { + return err + } + + common.LogInfo2Quiet(fmt.Sprintf("Clearing chart overrides for %s", chart.ReleaseName)) + if err := common.PropertyDelete("scheduler-k3s", "--global", "chart-overrides."+chart.ReleaseName); err != nil { + return fmt.Errorf("Unable to delete property: %w", err) + } + return nil +} + +// CommandChartsSet sets, clears or replaces chart-specific helm values for the scheduler +func CommandChartsSet(input ChartsSetInput) error { + if input.Replace { + return replaceChartOverrides(input.ChartName, input.Pairs) + } + + propertyArg := input.Property if propertyArg == "" { return fmt.Errorf("Invalid property, expected format: .") } @@ -629,25 +665,15 @@ func CommandChartsSet(propertyArg string, value string) error { chartName := propertyArg[:dotIndex] chartProperty := propertyArg[dotIndex+1:] - - knownCharts := []string{} - chartFound := false - for _, chart := range HelmCharts { - knownCharts = append(knownCharts, chart.ReleaseName) - if chart.ReleaseName == chartName { - chartFound = true - } - } - if !chartFound { - sort.Strings(knownCharts) - return fmt.Errorf("Invalid chart name %q, valid charts: %s", chartName, strings.Join(knownCharts, ", ")) + if _, err := getHelmChart(chartName); err != nil { + return err } key := fmt.Sprintf("chart.%s.%s", chartName, chartProperty) mapProperty := "chart-overrides." + chartName - if value != "" { - common.LogInfo2Quiet(fmt.Sprintf("Setting %s to %s", key, value)) - if err := common.PropertyMapSet("scheduler-k3s", "--global", mapProperty, chartProperty, value); err != nil { + if input.Value != "" { + common.LogInfo2Quiet(fmt.Sprintf("Setting %s to %s", key, input.Value)) + if err := common.PropertyMapSet("scheduler-k3s", "--global", mapProperty, chartProperty, input.Value); err != nil { return fmt.Errorf("Unable to write property: %w", err) } return nil @@ -660,6 +686,33 @@ func CommandChartsSet(propertyArg string, value string) error { return nil } +// replaceChartOverrides replaces every chart-specific helm value for a chart in a single write +func replaceChartOverrides(chartName string, pairs []string) error { + if chartName == "" { + return fmt.Errorf("Missing chart name") + } + + chart, err := getHelmChart(chartName) + if err != nil { + return err + } + + if len(pairs) == 0 { + return fmt.Errorf("Must specify at least one key=value pair, use scheduler-k3s:charts:clear to remove all chart overrides") + } + + parsed, err := parseMetadataPairs(pairs) + if err != nil { + return err + } + + common.LogInfo2Quiet(fmt.Sprintf("Replacing chart overrides for %s", chart.ReleaseName)) + if err := common.PropertyMapWrite("scheduler-k3s", "--global", "chart-overrides."+chart.ReleaseName, parsed); err != nil { + return fmt.Errorf("Unable to write property map: %w", err) + } + return nil +} + // CommandChartsReport displays a scheduler-k3s chart override report func CommandChartsReport(chartName string, format string, infoFlag string) error { if format != "stdout" && format != "json" { @@ -672,22 +725,11 @@ func CommandChartsReport(chartName string, format string, infoFlag string) error charts := []HelmChart{} if chartName != "" { - found := false - for _, chart := range HelmCharts { - if chart.ReleaseName == chartName { - charts = append(charts, chart) - found = true - break - } - } - if !found { - knownCharts := []string{} - for _, chart := range HelmCharts { - knownCharts = append(knownCharts, chart.ReleaseName) - } - sort.Strings(knownCharts) - return fmt.Errorf("Invalid chart name %q, valid charts: %s", chartName, strings.Join(knownCharts, ", ")) + chart, err := getHelmChart(chartName) + if err != nil { + return err } + charts = append(charts, chart) } else { charts = append(charts, HelmCharts...) } diff --git a/tests/unit/scheduler-k3s-charts-1.bats b/tests/unit/scheduler-k3s-charts-1.bats index a15c2326b..2e00bfaf6 100644 --- a/tests/unit/scheduler-k3s-charts-1.bats +++ b/tests/unit/scheduler-k3s-charts-1.bats @@ -7,9 +7,8 @@ setup() { } teardown() { - dokku scheduler-k3s:charts:set cert-manager.installCRDs >/dev/null 2>/dev/null || true - dokku scheduler-k3s:charts:set cert-manager.version >/dev/null 2>/dev/null || true - dokku scheduler-k3s:charts:set traefik.replicas >/dev/null 2>/dev/null || true + dokku scheduler-k3s:charts:clear cert-manager >/dev/null 2>/dev/null || true + dokku scheduler-k3s:charts:clear traefik >/dev/null 2>/dev/null || true global_teardown } diff --git a/tests/unit/scheduler-k3s-charts-2.bats b/tests/unit/scheduler-k3s-charts-2.bats index e56f3ea4a..f6dc8a845 100644 --- a/tests/unit/scheduler-k3s-charts-2.bats +++ b/tests/unit/scheduler-k3s-charts-2.bats @@ -7,9 +7,8 @@ setup() { } teardown() { - dokku scheduler-k3s:charts:set traefik.service.annotations.prometheus.io/scrape >/dev/null 2>/dev/null || true - dokku scheduler-k3s:charts:set traefik.controller.config >/dev/null 2>/dev/null || true - dokku scheduler-k3s:charts:set cert-manager.installCRDs >/dev/null 2>/dev/null || true + dokku scheduler-k3s:charts:clear cert-manager >/dev/null 2>/dev/null || true + dokku scheduler-k3s:charts:clear traefik >/dev/null 2>/dev/null || true global_teardown } diff --git a/tests/unit/scheduler-k3s-charts-3.bats b/tests/unit/scheduler-k3s-charts-3.bats new file mode 100644 index 000000000..f7749622b --- /dev/null +++ b/tests/unit/scheduler-k3s-charts-3.bats @@ -0,0 +1,99 @@ +#!/usr/bin/env bats + +load test_helper + +setup() { + global_setup +} + +teardown() { + dokku scheduler-k3s:charts:clear cert-manager >/dev/null 2>/dev/null || true + dokku scheduler-k3s:charts:clear traefik >/dev/null 2>/dev/null || true + global_teardown +} + +@test "(scheduler-k3s:charts:set) --replace writes the declared map in one call" { + run /bin/bash -c "dokku scheduler-k3s:charts:set traefik.replicas 2" + assert_success + run /bin/bash -c "dokku scheduler-k3s:charts:set traefik.foo bar" + assert_success + run /bin/bash -c "dokku scheduler-k3s:charts:set cert-manager.installCRDs false" + assert_success + + run /bin/bash -c "dokku scheduler-k3s:charts:set --replace traefik replicas=3 service.annotations.prometheus.io/scrape=true opt=a=b empty=" + assert_success + + run /bin/bash -c "dokku scheduler-k3s:charts:report traefik --format json | jq -c -S ." + assert_success + assert_output '{"traefik.empty":"","traefik.opt":"a=b","traefik.replicas":"3","traefik.service.annotations.prometheus.io/scrape":"true"}' + + run /bin/bash -c "dokku scheduler-k3s:charts:report cert-manager --format json | jq -c -S ." + assert_success + assert_output '{"cert-manager.installCRDs":"false"}' +} + +@test "(scheduler-k3s:charts:set) --replace rejects invalid input without writing" { + run /bin/bash -c "dokku scheduler-k3s:charts:set traefik.replicas 2" + assert_success + + run /bin/bash -c "dokku scheduler-k3s:charts:set --replace" + assert_failure + assert_output_contains "Missing chart name" + + run /bin/bash -c "dokku scheduler-k3s:charts:set --replace not-a-chart replicas=3" + assert_failure + assert_output_contains "Invalid chart name" + + run /bin/bash -c "dokku scheduler-k3s:charts:set --replace replicas=3" + assert_failure + assert_output_contains "Invalid chart name" + + run /bin/bash -c "dokku scheduler-k3s:charts:set --replace traefik" + assert_failure + assert_output_contains "scheduler-k3s:charts:clear" + + run /bin/bash -c "dokku scheduler-k3s:charts:set --replace traefik replicas=3 malformed" + assert_failure + assert_output_contains "Invalid key=value pair: malformed" + + run /bin/bash -c "dokku scheduler-k3s:charts:set --replace traefik replicas=3 =3" + assert_failure + assert_output_contains "Invalid key=value pair: =3" + + run /bin/bash -c "dokku scheduler-k3s:charts:set --replace traefik replicas=3 replicas=4" + assert_failure + assert_output_contains "Duplicate key specified: replicas" + + run /bin/bash -c "dokku scheduler-k3s:charts:report traefik --format json | jq -c -S ." + assert_success + assert_output '{"traefik.replicas":"2"}' +} + +@test "(scheduler-k3s:charts:clear) removes every override for a chart" { + run /bin/bash -c "dokku scheduler-k3s:charts:set --replace traefik replicas=2 foo=bar" + assert_success + run /bin/bash -c "dokku scheduler-k3s:charts:set cert-manager.installCRDs false" + assert_success + + run /bin/bash -c "dokku scheduler-k3s:charts:clear traefik" + assert_success + + run /bin/bash -c "dokku scheduler-k3s:charts:report traefik --format json" + assert_success + assert_output "{}" + + run /bin/bash -c "dokku scheduler-k3s:charts:report cert-manager --format json | jq -c -S ." + assert_success + assert_output '{"cert-manager.installCRDs":"false"}' + + run /bin/bash -c "dokku scheduler-k3s:charts:clear traefik" + assert_success + + run /bin/bash -c "dokku scheduler-k3s:charts:clear" + assert_failure + assert_output_contains "Missing chart name" + + run /bin/bash -c "dokku scheduler-k3s:charts:clear not-a-chart" + assert_failure + assert_output_contains "Invalid chart name" +}