From 077aa0d0981aa94fea75abecb5b52edb89321fc9 Mon Sep 17 00:00:00 2001 From: shanurwan Date: Mon, 16 Nov 2026 03:01:30 +0800 Subject: [PATCH] executor-k8s: render job scheduling values as JSON The executor parses EXECUTOR_KUBERNETES_NODE_TOLERATIONS, the node affinity match expressions and fields, and the pod affinity and anti-affinity with json.Unmarshal. The ConfigMap rendered them as "{{ .Values... }}", which broke both ways of setting them: - A JSON encoded string, the documented form, put unescaped double quotes inside a double-quoted YAML scalar, so the render failed with "did not find expected key". - A YAML list was printed with Go formatting, e.g. "[map[effect:NoSchedule key:foo operator:Equal value:bar]]", so the executor failed with "invalid character 'm' looking for beginning of value". Add an executor.jsonEnv helper that quotes a string unchanged and encodes a list or map with toJson, and use it for all five values. Empty values still render as "". The values.yaml examples escaped their quotes inside single-quoted YAML, so copied as written they were not valid JSON; they now are. Fixes #349 --- charts/sourcegraph-executor/CHANGELOG.md | 2 + charts/sourcegraph-executor/k8s/README.md | 10 +- .../k8s/templates/_helpers.tpl | 15 +++ .../k8s/templates/executor.ConfigMap.yaml | 10 +- .../k8s/tests/executor_test.yaml | 100 ++++++++++++++++++ charts/sourcegraph-executor/k8s/values.yaml | 10 +- 6 files changed, 132 insertions(+), 15 deletions(-) diff --git a/charts/sourcegraph-executor/CHANGELOG.md b/charts/sourcegraph-executor/CHANGELOG.md index 5e2f4fb18..446e8e12a 100644 --- a/charts/sourcegraph-executor/CHANGELOG.md +++ b/charts/sourcegraph-executor/CHANGELOG.md @@ -3,6 +3,8 @@ ## Unreleased +- Fixed `executor.kubernetesJob.node.tolerations`, `node.requiredAffinityMatchExpressions`, `node.requiredAffinityMatchFields`, `pod.affinity`, and `pod.antiAffinity` in the `sourcegraph-executor-k8s` chart. A JSON encoded string, the documented form, failed the render with a YAML parse error, and a YAML list rendered as `[map[effect:NoSchedule key:foo ...]]`, so the executor failed to start with `invalid character 'm' looking for beginning of value`. Both forms now render as JSON. The examples in `values.yaml` are now valid JSON when copied as written. [#349](https://github.com/sourcegraph/deploy-sourcegraph-helm/issues/349) + ## 8.0.0 * Added a `network-policy` example to the `sourcegraph-executor-k8s` and `sourcegraph-executor-dind` charts, with `CiliumNetworkPolicy` resources which limit Executor and job pods to the frontend API. diff --git a/charts/sourcegraph-executor/k8s/README.md b/charts/sourcegraph-executor/k8s/README.md index 67bae6cc3..5465b707f 100644 --- a/charts/sourcegraph-executor/k8s/README.md +++ b/charts/sourcegraph-executor/k8s/README.md @@ -72,12 +72,12 @@ In addition to the documented values, the `executor` and `private-docker-registr | executor.kubernetesJob.deadline | string | `"1200"` | The number of seconds after which a Kubernetes job will be terminated. | | executor.kubernetesJob.fsGroup | string | `"1000"` | The group ID which is set on the job PVC file system. | | executor.kubernetesJob.node.name | string | `""` | The name of the Kubernetes Node to create job pods on. If not specified, the pods are created on the first available node. | -| executor.kubernetesJob.node.requiredAffinityMatchExpressions | string | `""` | The JSON encoded required affinity match expressions for Kubernetes Jobs. e.g. '[{\"key\":\"foo\",\"operator\":\"In\",\"values\":[\"bar\"]}]' | -| executor.kubernetesJob.node.requiredAffinityMatchFields | string | `""` | The JSON encoded required affinity match fields for Kubernetes Jobs. e.g. '[{\"key\":\"foo\",\"operator\":\"In\",\"values\":[\"bar\"]}]' | +| executor.kubernetesJob.node.requiredAffinityMatchExpressions | string | `""` | The required affinity match expressions for Kubernetes Jobs, as a YAML list or a JSON encoded string. e.g. '[{"key":"foo","operator":"In","values":["bar"]}]' | +| executor.kubernetesJob.node.requiredAffinityMatchFields | string | `""` | The required affinity match fields for Kubernetes Jobs, as a YAML list or a JSON encoded string. e.g. '[{"key":"foo","operator":"In","values":["bar"]}]' | | executor.kubernetesJob.node.selector | string | `""` | A comma separated list of values to use as a node selector for Kubernetes Jobs. e.g. `foo=bar,app=my-app` | -| executor.kubernetesJob.node.tolerations | string | `""` | The JSON encoded tolerations for Kubernetes Jobs. e.g. '[{\"key\":\"foo\",\"operator\":\"Equal\",\"value\":\"bar\",\"effect\":\"NoSchedule\"}]' | -| executor.kubernetesJob.pod.affinity | string | `""` | The JSON encoded pod affinity for Kubernetes Jobs. e.g. '[{\"labelSelector\": {\"matchExpressions\": [{\"key\": \"foo\",\"operator\": \"In\",\"values\": [\"bar\"]}]},\"topologyKey\": \"kubernetes.io/hostname\"}]' | -| executor.kubernetesJob.pod.antiAffinity | string | `""` | The JSON encoded pod anti-affinity for Kubernetes Jobs. e.g. '[{\"labelSelector\": {\"matchExpressions\": [{\"key\": \"foo\",\"operator\": \"In\",\"values\": [\"bar\"]}]},\"topologyKey\": \"kubernetes.io/hostname\"}]' | +| executor.kubernetesJob.node.tolerations | string | `""` | The tolerations for Kubernetes Jobs, as a YAML list or a JSON encoded string. e.g. '[{"key":"foo","operator":"Equal","value":"bar","effect":"NoSchedule"}]' | +| executor.kubernetesJob.pod.affinity | string | `""` | The pod affinity for Kubernetes Jobs, as a YAML list or a JSON encoded string. e.g. '[{"labelSelector": {"matchExpressions": [{"key": "foo","operator": "In","values": ["bar"]}]},"topologyKey": "kubernetes.io/hostname"}]' | +| executor.kubernetesJob.pod.antiAffinity | string | `""` | The pod anti-affinity for Kubernetes Jobs, as a YAML list or a JSON encoded string. e.g. '[{"labelSelector": {"matchExpressions": [{"key": "foo","operator": "In","values": ["bar"]}]},"topologyKey": "kubernetes.io/hostname"}]' | | executor.kubernetesJob.resources.limits.cpu | string | `""` | The maximum CPU for a job. | | executor.kubernetesJob.resources.limits.memory | string | `"12Gi"` | The maximum memory for a job. | | executor.kubernetesJob.resources.requests.cpu | string | `""` | The requested CPU for a job. | diff --git a/charts/sourcegraph-executor/k8s/templates/_helpers.tpl b/charts/sourcegraph-executor/k8s/templates/_helpers.tpl index 70f75b28c..460bd8194 100644 --- a/charts/sourcegraph-executor/k8s/templates/_helpers.tpl +++ b/charts/sourcegraph-executor/k8s/templates/_helpers.tpl @@ -112,3 +112,18 @@ deploy: sourcegraph sourcegraph-resource-requires: no-cluster-admin app.kubernetes.io/component: executor {{- end}} + +{{/* +Render a value as a quoted JSON string for an executor environment variable. +Accepts a JSON-encoded string (passed through unchanged) or a YAML list or map +(encoded as JSON). Empty values render as "". +*/}} +{{- define "executor.jsonEnv" -}} +{{- if kindIs "string" . -}} +{{- . | quote -}} +{{- else if . -}} +{{- toJson . | quote -}} +{{- else -}} +"" +{{- end -}} +{{- end }} diff --git a/charts/sourcegraph-executor/k8s/templates/executor.ConfigMap.yaml b/charts/sourcegraph-executor/k8s/templates/executor.ConfigMap.yaml index 6e773607b..1122f1333 100644 --- a/charts/sourcegraph-executor/k8s/templates/executor.ConfigMap.yaml +++ b/charts/sourcegraph-executor/k8s/templates/executor.ConfigMap.yaml @@ -21,12 +21,12 @@ data: EXECUTOR_KUBERNETES_NODE_NAME: "{{ .Values.executor.kubernetesJob.node.name }}" EXECUTOR_KUBERNETES_NODE_SELECTOR: "{{ .Values.executor.kubernetesJob.node.selector }}" - EXECUTOR_KUBERNETES_NODE_TOLERATIONS: "{{ .Values.executor.kubernetesJob.node.tolerations }}" - EXECUTOR_KUBERNETES_NODE_REQUIRED_AFFINITY_MATCH_EXPRESSIONS: "{{ .Values.executor.kubernetesJob.node.requiredAffinityMatchExpressions }}" - EXECUTOR_KUBERNETES_NODE_REQUIRED_AFFINITY_MATCH_FIELDS: "{{ .Values.executor.kubernetesJob.node.requiredAffinityMatchFields }}" + EXECUTOR_KUBERNETES_NODE_TOLERATIONS: {{ include "executor.jsonEnv" .Values.executor.kubernetesJob.node.tolerations }} + EXECUTOR_KUBERNETES_NODE_REQUIRED_AFFINITY_MATCH_EXPRESSIONS: {{ include "executor.jsonEnv" .Values.executor.kubernetesJob.node.requiredAffinityMatchExpressions }} + EXECUTOR_KUBERNETES_NODE_REQUIRED_AFFINITY_MATCH_FIELDS: {{ include "executor.jsonEnv" .Values.executor.kubernetesJob.node.requiredAffinityMatchFields }} - EXECUTOR_KUBERNETES_POD_AFFINITY: "{{ .Values.executor.kubernetesJob.pod.affinity }}" - EXECUTOR_KUBERNETES_POD_ANTI_AFFINITY: "{{ .Values.executor.kubernetesJob.pod.antiAffinity }}" + EXECUTOR_KUBERNETES_POD_AFFINITY: {{ include "executor.jsonEnv" .Values.executor.kubernetesJob.pod.affinity }} + EXECUTOR_KUBERNETES_POD_ANTI_AFFINITY: {{ include "executor.jsonEnv" .Values.executor.kubernetesJob.pod.antiAffinity }} EXECUTOR_KUBERNETES_RESOURCE_REQUEST_CPU: "{{ .Values.executor.kubernetesJob.resources.requests.cpu }}" EXECUTOR_KUBERNETES_RESOURCE_REQUEST_MEMORY: "{{ .Values.executor.kubernetesJob.resources.requests.memory }}" diff --git a/charts/sourcegraph-executor/k8s/tests/executor_test.yaml b/charts/sourcegraph-executor/k8s/tests/executor_test.yaml index 256638deb..3f1046beb 100644 --- a/charts/sourcegraph-executor/k8s/tests/executor_test.yaml +++ b/charts/sourcegraph-executor/k8s/tests/executor_test.yaml @@ -125,3 +125,103 @@ tests: path: spec.template.spec.securityContext.runAsUser - isNull: path: spec.template.spec.securityContext.runAsGroup + + - it: should render empty Kubernetes job scheduling values as empty strings by default + template: executor.ConfigMap.yaml + set: + executor: + queueName: "test" + asserts: + - equal: + path: data.EXECUTOR_KUBERNETES_NODE_TOLERATIONS + value: "" + - equal: + path: data.EXECUTOR_KUBERNETES_NODE_REQUIRED_AFFINITY_MATCH_EXPRESSIONS + value: "" + - equal: + path: data.EXECUTOR_KUBERNETES_NODE_REQUIRED_AFFINITY_MATCH_FIELDS + value: "" + - equal: + path: data.EXECUTOR_KUBERNETES_POD_AFFINITY + value: "" + - equal: + path: data.EXECUTOR_KUBERNETES_POD_ANTI_AFFINITY + value: "" + + - it: should pass JSON encoded Kubernetes job scheduling strings through unchanged + template: executor.ConfigMap.yaml + set: + executor: + queueName: "test" + kubernetesJob: + node: + tolerations: '[{"key":"foo","operator":"Equal","value":"bar","effect":"NoSchedule"}]' + requiredAffinityMatchExpressions: '[{"key":"foo","operator":"In","values":["bar"]}]' + requiredAffinityMatchFields: '[{"key":"metadata.name","operator":"In","values":["node-1"]}]' + pod: + affinity: '[{"labelSelector":{"matchLabels":{"app":"foo"}},"topologyKey":"kubernetes.io/hostname"}]' + antiAffinity: '[{"labelSelector":{"matchLabels":{"app":"bar"}},"topologyKey":"kubernetes.io/hostname"}]' + asserts: + - equal: + path: data.EXECUTOR_KUBERNETES_NODE_TOLERATIONS + value: '[{"key":"foo","operator":"Equal","value":"bar","effect":"NoSchedule"}]' + - equal: + path: data.EXECUTOR_KUBERNETES_NODE_REQUIRED_AFFINITY_MATCH_EXPRESSIONS + value: '[{"key":"foo","operator":"In","values":["bar"]}]' + - equal: + path: data.EXECUTOR_KUBERNETES_NODE_REQUIRED_AFFINITY_MATCH_FIELDS + value: '[{"key":"metadata.name","operator":"In","values":["node-1"]}]' + - equal: + path: data.EXECUTOR_KUBERNETES_POD_AFFINITY + value: '[{"labelSelector":{"matchLabels":{"app":"foo"}},"topologyKey":"kubernetes.io/hostname"}]' + - equal: + path: data.EXECUTOR_KUBERNETES_POD_ANTI_AFFINITY + value: '[{"labelSelector":{"matchLabels":{"app":"bar"}},"topologyKey":"kubernetes.io/hostname"}]' + + - it: should encode Kubernetes job scheduling YAML lists as JSON + template: executor.ConfigMap.yaml + set: + executor: + queueName: "test" + kubernetesJob: + node: + tolerations: + - key: foo + operator: Equal + value: bar + effect: NoSchedule + requiredAffinityMatchExpressions: + - key: foo + operator: In + values: ["bar"] + requiredAffinityMatchFields: + - key: metadata.name + operator: In + values: ["node-1"] + pod: + affinity: + - labelSelector: + matchLabels: + app: foo + topologyKey: kubernetes.io/hostname + antiAffinity: + - labelSelector: + matchLabels: + app: bar + topologyKey: kubernetes.io/hostname + asserts: + - equal: + path: data.EXECUTOR_KUBERNETES_NODE_TOLERATIONS + value: '[{"effect":"NoSchedule","key":"foo","operator":"Equal","value":"bar"}]' + - equal: + path: data.EXECUTOR_KUBERNETES_NODE_REQUIRED_AFFINITY_MATCH_EXPRESSIONS + value: '[{"key":"foo","operator":"In","values":["bar"]}]' + - equal: + path: data.EXECUTOR_KUBERNETES_NODE_REQUIRED_AFFINITY_MATCH_FIELDS + value: '[{"key":"metadata.name","operator":"In","values":["node-1"]}]' + - equal: + path: data.EXECUTOR_KUBERNETES_POD_AFFINITY + value: '[{"labelSelector":{"matchLabels":{"app":"foo"}},"topologyKey":"kubernetes.io/hostname"}]' + - equal: + path: data.EXECUTOR_KUBERNETES_POD_ANTI_AFFINITY + value: '[{"labelSelector":{"matchLabels":{"app":"bar"}},"topologyKey":"kubernetes.io/hostname"}]' diff --git a/charts/sourcegraph-executor/k8s/values.yaml b/charts/sourcegraph-executor/k8s/values.yaml index b55767725..c2daa3c9f 100644 --- a/charts/sourcegraph-executor/k8s/values.yaml +++ b/charts/sourcegraph-executor/k8s/values.yaml @@ -120,17 +120,17 @@ executor: name: "" # -- A comma separated list of values to use as a node selector for Kubernetes Jobs. e.g. `foo=bar,app=my-app` selector: "" - # -- The JSON encoded tolerations for Kubernetes Jobs. e.g. '[{\"key\":\"foo\",\"operator\":\"Equal\",\"value\":\"bar\",\"effect\":\"NoSchedule\"}]' + # -- The tolerations for Kubernetes Jobs, as a YAML list or a JSON encoded string. e.g. '[{"key":"foo","operator":"Equal","value":"bar","effect":"NoSchedule"}]' tolerations: "" - # -- The JSON encoded required affinity match expressions for Kubernetes Jobs. e.g. '[{\"key\":\"foo\",\"operator\":\"In\",\"values\":[\"bar\"]}]' + # -- The required affinity match expressions for Kubernetes Jobs, as a YAML list or a JSON encoded string. e.g. '[{"key":"foo","operator":"In","values":["bar"]}]' requiredAffinityMatchExpressions: "" - # -- The JSON encoded required affinity match fields for Kubernetes Jobs. e.g. '[{\"key\":\"foo\",\"operator\":\"In\",\"values\":[\"bar\"]}]' + # -- The required affinity match fields for Kubernetes Jobs, as a YAML list or a JSON encoded string. e.g. '[{"key":"foo","operator":"In","values":["bar"]}]' requiredAffinityMatchFields: "" pod: - # -- The JSON encoded pod affinity for Kubernetes Jobs. e.g. '[{\"labelSelector\": {\"matchExpressions\": [{\"key\": \"foo\",\"operator\": \"In\",\"values\": [\"bar\"]}]},\"topologyKey\": \"kubernetes.io/hostname\"}]' + # -- The pod affinity for Kubernetes Jobs, as a YAML list or a JSON encoded string. e.g. '[{"labelSelector": {"matchExpressions": [{"key": "foo","operator": "In","values": ["bar"]}]},"topologyKey": "kubernetes.io/hostname"}]' affinity: "" - # -- The JSON encoded pod anti-affinity for Kubernetes Jobs. e.g. '[{\"labelSelector\": {\"matchExpressions\": [{\"key\": \"foo\",\"operator\": \"In\",\"values\": [\"bar\"]}]},\"topologyKey\": \"kubernetes.io/hostname\"}]' + # -- The pod anti-affinity for Kubernetes Jobs, as a YAML list or a JSON encoded string. e.g. '[{"labelSelector": {"matchExpressions": [{"key": "foo","operator": "In","values": ["bar"]}]},"topologyKey": "kubernetes.io/hostname"}]' antiAffinity: "" debug: