diff --git a/plugins/scheduler-k3s/cron_job_template_test.go b/plugins/scheduler-k3s/cron_job_template_test.go new file mode 100644 index 000000000..4770f7cb8 --- /dev/null +++ b/plugins/scheduler-k3s/cron_job_template_test.go @@ -0,0 +1,149 @@ +package scheduler_k3s + +import ( + "errors" + "io" + "os" + "path/filepath" + "strings" + "testing" + + "gopkg.in/yaml.v3" + "helm.sh/helm/v3/pkg/chart/loader" + "helm.sh/helm/v3/pkg/chartutil" + "helm.sh/helm/v3/pkg/engine" +) + +// TestCronIDLabelValue asserts the hashed cron ID fits inside Kubernetes' 63 +// byte label cap and is deterministic. Without this the label can exceed the +// cap and the Kubernetes API server rejects the manifest. +func TestCronIDLabelValue(t *testing.T) { + cases := []string{ + "short", + "app===echo hello-from-cron===5 5 5 5 5", + strings.Repeat("a-very-long-cron-id-that-would-far-exceed-the-label-cap-", 10), + } + for _, in := range cases { + out := cronIDLabelValue(in) + if len(out) > 63 { + t.Errorf("cronIDLabelValue(%q) = %q (len %d); must be <= 63", in, out, len(out)) + } + if out != cronIDLabelValue(in) { + t.Errorf("cronIDLabelValue(%q) is not deterministic", in) + } + } +} + +// TestCronJobTemplateQuotesAllDigitSuffix asserts that the rendered cron-job +// manifest produces string-typed annotation values even when the suffix and +// cron-id consist entirely of digits. Without `| quote` in the template, YAML +// would coerce these to numbers and the manifest would be rejected by the +// Kubernetes API server. +func TestCronJobTemplateQuotesAllDigitSuffix(t *testing.T) { + chartDir := t.TempDir() + if err := os.MkdirAll(filepath.Join(chartDir, "templates"), 0o755); err != nil { + t.Fatalf("mkdir: %v", err) + } + + chartYAML := []byte("apiVersion: v2\nname: test\nversion: 0.0.1\n") + if err := os.WriteFile(filepath.Join(chartDir, "Chart.yaml"), chartYAML, 0o644); err != nil { + t.Fatalf("write Chart.yaml: %v", err) + } + + cronJobTpl, err := templates.ReadFile("templates/chart/cron-job.yaml") + if err != nil { + t.Fatalf("read cron-job template: %v", err) + } + if err := os.WriteFile(filepath.Join(chartDir, "templates", "cron-job.yaml"), cronJobTpl, 0o644); err != nil { + t.Fatalf("write cron-job template: %v", err) + } + + helpersTpl, err := templates.ReadFile("templates/chart/_helpers.tpl") + if err != nil { + t.Fatalf("read _helpers: %v", err) + } + if err := os.WriteFile(filepath.Join(chartDir, "templates", "_helpers.tpl"), helpersTpl, 0o644); err != nil { + t.Fatalf("write _helpers: %v", err) + } + + loaded, err := loader.Load(chartDir) + if err != nil { + t.Fatalf("load chart: %v", err) + } + + values := map[string]interface{}{ + "global": map[string]interface{}{ + "app_name": "myapp", + "deployment_id": "1", + "namespace": "myapp", + "image": map[string]interface{}{ + "name": "myapp:latest", + "type": "dockerfile", + }, + }, + "processes": map[string]interface{}{ + "cron-id-123": map[string]interface{}{ + "args": []interface{}{"echo", "hello"}, + "cron": map[string]interface{}{ + "id": "1234567890", + "hash": "abc123def", + "schedule": "5 5 5 5 5", + "suffix": "1234567890", + "suspend": false, + "concurrency_policy": "Allow", + }, + }, + }, + } + + renderValues, err := chartutil.ToRenderValues(loaded, values, chartutil.ReleaseOptions{Name: "test", Namespace: "default"}, nil) + if err != nil { + t.Fatalf("ToRenderValues: %v", err) + } + + rendered, err := engine.Render(loaded, renderValues) + if err != nil { + t.Fatalf("render: %v", err) + } + + var manifest string + for name, content := range rendered { + if filepath.Base(name) == "cron-job.yaml" { + manifest = content + break + } + } + if manifest == "" { + t.Fatalf("cron-job.yaml not rendered; got: %v", rendered) + } + + decoder := yaml.NewDecoder(strings.NewReader(manifest)) + for { + var doc map[string]interface{} + if err := decoder.Decode(&doc); err != nil { + if errors.Is(err, io.EOF) { + break + } + t.Fatalf("yaml decode failed (would also fail in Kubernetes API): %v\nrendered:\n%s", err, manifest) + } + if doc == nil { + continue + } + metadata, ok := doc["metadata"].(map[string]interface{}) + if !ok { + continue + } + annotations, _ := metadata["annotations"].(map[string]interface{}) + for key, value := range annotations { + if _, isString := value.(string); !isString { + t.Errorf("annotation %q has non-string value %v (type %T); helm template must apply | quote", key, value, value) + } + } + labels, _ := metadata["labels"].(map[string]interface{}) + for key, value := range labels { + if _, isString := value.(string); !isString { + t.Errorf("label %q has non-string value %v (type %T); helm template must apply | quote", key, value, value) + } + } + } +} diff --git a/plugins/scheduler-k3s/functions.go b/plugins/scheduler-k3s/functions.go index 4970ef4e7..a183efe7b 100644 --- a/plugins/scheduler-k3s/functions.go +++ b/plugins/scheduler-k3s/functions.go @@ -3,6 +3,7 @@ package scheduler_k3s import ( "bytes" "context" + "crypto/sha256" "encoding/base64" "errors" "fmt" @@ -23,6 +24,7 @@ import ( nginxvhosts "github.com/dokku/dokku/plugins/nginx-vhosts" resty "github.com/go-resty/resty/v2" kedav1alpha1 "github.com/kedacore/keda/v2/apis/keda/v1alpha1" + "github.com/multiformats/go-base36" "golang.org/x/sync/errgroup" "gopkg.in/yaml.v3" "helm.sh/helm/v3/pkg/strvals" @@ -2081,6 +2083,14 @@ func kubernetesNodeToNode(node v1.Node) Node { } } +// cronIDLabelValue returns a Kubernetes-label-safe hash of a cron ID. +// The raw cron ID can exceed the 63-byte label cap, so we keep it as an +// annotation and use this short hash for selectors. +func cronIDLabelValue(cronID string) string { + sum := sha256.Sum256([]byte(cronID)) + return base36.EncodeToStringLc(sum[:16]) +} + // parseMemoryQuantity parses a string into a valid memory quantity func parseMemoryQuantity(input string) (string, error) { if _, err := strconv.ParseInt(input, 10, 64); err == nil { diff --git a/plugins/scheduler-k3s/go.mod b/plugins/scheduler-k3s/go.mod index 0451e6e08..532446756 100644 --- a/plugins/scheduler-k3s/go.mod +++ b/plugins/scheduler-k3s/go.mod @@ -22,6 +22,7 @@ require ( github.com/gosimple/slug v1.15.0 github.com/kballard/go-shellquote v0.0.0-20180428030007-95032a82bc51 github.com/kedacore/keda/v2 v2.18.3 + github.com/multiformats/go-base36 v0.2.0 github.com/ryanuber/columnize v2.1.2+incompatible github.com/spf13/pflag v1.0.10 github.com/traefik/traefik/v2 v2.11.45 @@ -115,7 +116,6 @@ require ( github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd // indirect github.com/modern-go/reflect2 v1.0.3-0.20250322232337-35a7c28c31ee // indirect github.com/monochromegane/go-gitignore v0.0.0-20200626010858-205db1a8cc00 // indirect - github.com/multiformats/go-base36 v0.2.0 // indirect github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 // indirect github.com/opencontainers/go-digest v1.0.0 // indirect github.com/opencontainers/image-spec v1.1.1 // indirect diff --git a/plugins/scheduler-k3s/template.go b/plugins/scheduler-k3s/template.go index 9d2b2e63a..554bde5f4 100644 --- a/plugins/scheduler-k3s/template.go +++ b/plugins/scheduler-k3s/template.go @@ -305,6 +305,7 @@ const ( type ProcessCron struct { ID string `yaml:"id"` + Hash string `yaml:"hash"` Schedule string `yaml:"schedule"` Suffix string `yaml:"suffix"` Suspend bool `yaml:"suspend"` diff --git a/plugins/scheduler-k3s/templates/chart/cron-job.yaml b/plugins/scheduler-k3s/templates/chart/cron-job.yaml index c3b9e30b4..3d4ee9b79 100644 --- a/plugins/scheduler-k3s/templates/chart/cron-job.yaml +++ b/plugins/scheduler-k3s/templates/chart/cron-job.yaml @@ -9,18 +9,18 @@ kind: CronJob metadata: annotations: app.kubernetes.io/version: {{ $.Values.global.deployment_id | quote }} - dokku.com/builder-type: {{ $.Values.global.image.type }} - dokku.com/cron-id: {{ $config.cron.id }} - dokku.com/job-suffix: {{ $config.cron.suffix }} + dokku.com/builder-type: {{ $.Values.global.image.type | quote }} + dokku.com/cron-id: {{ $config.cron.id | quote }} + dokku.com/job-suffix: {{ $config.cron.suffix | quote }} dokku.com/managed: "true" - kubectl.kubernetes.io/default-container: {{ $.Values.global.app_name }}-cron + kubectl.kubernetes.io/default-container: {{ printf "%s-cron" $.Values.global.app_name | quote }} {{ include "print.annotations" (dict "config" $.Values.global "key" "cronjob") | indent 4 }} {{ include "print.annotations" (dict "config" $config "key" "cronjob") | indent 4 }} labels: - app.kubernetes.io/instance: {{ $.Values.global.app_name }}-cron-{{ $config.cron.suffix }} + app.kubernetes.io/instance: {{ printf "%s-cron-%s" $.Values.global.app_name $config.cron.suffix | quote }} app.kubernetes.io/name: cron - app.kubernetes.io/part-of: {{ $.Values.global.app_name }} - dokku.com/cron-id: {{ $config.cron.id }} + app.kubernetes.io/part-of: {{ $.Values.global.app_name | quote }} + dokku.com/cron-hash: {{ $config.cron.hash | quote }} {{ include "print.labels" (dict "config" $.Values.global "key" "cronjob") | indent 4 }} {{ include "print.labels" (dict "config" $config "key" "cronjob") | indent 4 }} name: {{ $.Values.global.app_name }}-cron-{{ $config.cron.suffix }} @@ -32,18 +32,18 @@ spec: metadata: annotations: app.kubernetes.io/version: {{ $.Values.global.deployment_id | quote }} - dokku.com/builder-type: {{ $.Values.global.image.type }} - dokku.com/cron-id: {{ $config.cron.id }} - dokku.com/job-suffix: {{ $config.cron.suffix }} + dokku.com/builder-type: {{ $.Values.global.image.type | quote }} + dokku.com/cron-id: {{ $config.cron.id | quote }} + dokku.com/job-suffix: {{ $config.cron.suffix | quote }} dokku.com/managed: "true" - kubectl.kubernetes.io/default-container: {{ $.Values.global.app_name }}-cron + kubectl.kubernetes.io/default-container: {{ printf "%s-cron" $.Values.global.app_name | quote }} {{ include "print.annotations" (dict "config" $.Values.global "key" "job") | indent 8 }} {{ include "print.annotations" (dict "config" $config "key" "job") | indent 8 }} labels: - app.kubernetes.io/instance: {{ $.Values.global.app_name }}-cron-{{ $config.cron.suffix }} + app.kubernetes.io/instance: {{ printf "%s-cron-%s" $.Values.global.app_name $config.cron.suffix | quote }} app.kubernetes.io/name: cron - app.kubernetes.io/part-of: {{ $.Values.global.app_name }} - dokku.com/cron-id: {{ $config.cron.id }} + app.kubernetes.io/part-of: {{ $.Values.global.app_name | quote }} + dokku.com/cron-hash: {{ $config.cron.hash | quote }} {{ include "print.labels" (dict "config" $.Values.global "key" "job") | indent 8 }} {{ include "print.labels" (dict "config" $config "key" "job") | indent 8 }} spec: @@ -55,18 +55,18 @@ spec: metadata: annotations: app.kubernetes.io/version: {{ $.Values.global.deployment_id | quote }} - dokku.com/builder-type: {{ $.Values.global.image.type }} - dokku.com/cron-id: {{ $config.cron.id }} - dokku.com/job-suffix: {{ $config.cron.suffix }} + dokku.com/builder-type: {{ $.Values.global.image.type | quote }} + dokku.com/cron-id: {{ $config.cron.id | quote }} + dokku.com/job-suffix: {{ $config.cron.suffix | quote }} dokku.com/managed: "true" - kubectl.kubernetes.io/default-container: {{ $.Values.global.app_name }}-cron + kubectl.kubernetes.io/default-container: {{ printf "%s-cron" $.Values.global.app_name | quote }} {{ include "print.annotations" (dict "config" $.Values.global "key" "pod") | indent 12 }} {{ include "print.annotations" (dict "config" $config "key" "pod") | indent 12 }} labels: - app.kubernetes.io/instance: {{ $.Values.global.app_name }}-cron-{{ $config.cron.suffix }} + app.kubernetes.io/instance: {{ printf "%s-cron-%s" $.Values.global.app_name $config.cron.suffix | quote }} app.kubernetes.io/name: cron - app.kubernetes.io/part-of: {{ $.Values.global.app_name }} - dokku.com/cron-id: {{ $config.cron.id }} + app.kubernetes.io/part-of: {{ $.Values.global.app_name | quote }} + dokku.com/cron-hash: {{ $config.cron.hash | quote }} {{ include "print.labels" (dict "config" $.Values.global "key" "pod") | indent 12 }} {{ include "print.labels" (dict "config" $config "key" "pod") | indent 12 }} spec: diff --git a/plugins/scheduler-k3s/templates/chart/deployment.yaml b/plugins/scheduler-k3s/templates/chart/deployment.yaml index 84c97dfcf..34312777e 100644 --- a/plugins/scheduler-k3s/templates/chart/deployment.yaml +++ b/plugins/scheduler-k3s/templates/chart/deployment.yaml @@ -16,15 +16,15 @@ kind: Deployment metadata: annotations: app.kubernetes.io/version: {{ $.Values.global.deployment_id | quote }} - dokku.com/builder-type: {{ $.Values.global.image.type }} + dokku.com/builder-type: {{ $.Values.global.image.type | quote }} dokku.com/managed: "true" - kubectl.kubernetes.io/default-container: {{ $.Values.global.app_name }}-{{ $processName }} + kubectl.kubernetes.io/default-container: {{ printf "%s-%s" $.Values.global.app_name $processName | quote }} {{ include "print.annotations" (dict "config" $.Values.global "key" "deployment") | indent 4 }} {{ include "print.annotations" (dict "config" $config "key" "deployment") | indent 4 }} labels: - app.kubernetes.io/instance: {{ $.Values.global.app_name }}-{{ $processName }} - app.kubernetes.io/name: {{ $processName }} - app.kubernetes.io/part-of: {{ $.Values.global.app_name }} + app.kubernetes.io/instance: {{ printf "%s-%s" $.Values.global.app_name $processName | quote }} + app.kubernetes.io/name: {{ $processName | quote }} + app.kubernetes.io/part-of: {{ $.Values.global.app_name | quote }} {{ include "print.labels" (dict "config" $.Values.global "key" "deployment") | indent 4 }} {{ include "print.labels" (dict "config" $config "key" "deployment") | indent 4 }} name: {{ $.Values.global.app_name }}-{{ $processName }} diff --git a/plugins/scheduler-k3s/triggers.go b/plugins/scheduler-k3s/triggers.go index 0a0182966..d1e01f360 100644 --- a/plugins/scheduler-k3s/triggers.go +++ b/plugins/scheduler-k3s/triggers.go @@ -282,7 +282,7 @@ func TriggerSchedulerCronWrite(scheduler string, appName string) error { for _, cronTask := range cronTasks { labelSelector := []string{ fmt.Sprintf("app.kubernetes.io/part-of=%s", appName), - fmt.Sprintf("dokku.com/cron-id=%s", cronTask.ID), + fmt.Sprintf("dokku.com/cron-hash=%s", cronIDLabelValue(cronTask.ID)), } if cronTask.Maintenance { @@ -790,7 +790,7 @@ func TriggerSchedulerDeploy(scheduler string, appName string, imageTag string) e // todo: implement pod annotations suffix := "" for _, cronJob := range cronJobs { - if cronJob.Labels["dokku.com/cron-id"] == cronTask.ID { + if cronJob.Annotations["dokku.com/cron-id"] == cronTask.ID { var ok bool suffix, ok = cronJob.Annotations["dokku.com/job-suffix"] if !ok { @@ -843,6 +843,7 @@ func TriggerSchedulerDeploy(scheduler string, appName string, imageTag string) e Annotations: annotations, Cron: ProcessCron{ ID: cronTask.ID, + Hash: cronIDLabelValue(cronTask.ID), Schedule: cronTask.Schedule, Suffix: suffix, Suspend: cronTask.Maintenance, @@ -1368,26 +1369,25 @@ func TriggerSchedulerRun(scheduler string, appName string, envCount int, args [] processType := "run" if os.Getenv("DOKKU_CRON_ID") != "" { processType = "cron" - labels["dokku.com/cron-id"] = os.Getenv("DOKKU_CRON_ID") + cronHash := cronIDLabelValue(os.Getenv("DOKKU_CRON_ID")) + labels["dokku.com/cron-hash"] = cronHash concurrencyPolicy := strings.ToUpper(os.Getenv("DOKKU_CONCURRENCY_POLICY")) switch concurrencyPolicy { case "forbid": - // check if there is a running pod with the same dokku.com/cron-id label pods, err := clientset.ListPods(context.Background(), ListPodsInput{ Namespace: namespace, - LabelSelector: fmt.Sprintf("dokku.com/cron-id=%s", os.Getenv("DOKKU_CRON_ID")), + LabelSelector: fmt.Sprintf("dokku.com/cron-hash=%s", cronHash), }) if err != nil { return fmt.Errorf("Error listing pods: %w", err) } if len(pods) > 0 { - return fmt.Errorf("There is a running pod with the same dokku.com/cron-id label") + return fmt.Errorf("There is a running pod with the same dokku.com/cron-hash label") } case "replace": - // delete any existing pod with the same dokku.com/cron-id label err := clientset.DeletePod(context.Background(), DeletePodInput{ Namespace: namespace, - LabelSelector: fmt.Sprintf("dokku.com/cron-id=%s", os.Getenv("DOKKU_CRON_ID")), + LabelSelector: fmt.Sprintf("dokku.com/cron-hash=%s", cronHash), }) if err != nil { return fmt.Errorf("Error deleting pod: %w", err) @@ -1572,7 +1572,7 @@ func TriggerSchedulerRun(scheduler string, appName string, envCount int, args [] Clientset: clientset, Namespace: namespace, LabelSelector: batchJobSelector, - Timeout: 10, + Timeout: 30, Waiter: isPodReady, }) if streamLogsOnly && (err == nil || errors.Is(err, conditions.ErrPodCompleted)) { @@ -1748,9 +1748,9 @@ func TriggerSchedulerRunList(scheduler string, appName string, format string) er } } - cronID, ok := cronJob.Labels["dokku.com/cron-id"] + cronID, ok := cronJob.Annotations["dokku.com/cron-id"] if !ok { - common.LogWarn(fmt.Sprintf("Cron job %s does not have a cron ID label", cronJob.Name)) + common.LogWarn(fmt.Sprintf("Cron job %s does not have a cron ID annotation", cronJob.Name)) continue }