fix(executor): render tolerations as json

charts/sourcegraph-executor/CHANGELOG.md

@@ -3,6 +3,7 @@
 <!-- START CHANGELOG -->
 ## Unreleased
 
+* Fixed `sourcegraph-executor-k8s` rendering of `executor.kubernetesJob.node.tolerations`, `requiredAffinityMatchExpressions`, `requiredAffinityMatchFields`, `pod.affinity` and `pod.antiAffinity`: YAML lists are now JSON-encoded for the executor instead of rendering as Go `map[...]` strings. Pre-escaped JSON strings keep working. [#349](https://github.com/sourcegraph/deploy-sourcegraph-helm/issues/349)
 * 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.
 * Added new chart `sourcegraph-executor-k8s` to deploy Sourcegraph executors that use Kubernetes jobs.
 * **BREAKING:** Renamed `sourcegraph-executor` chart to `sourcegraph-executor-dind` to indicate these are Docker in Docker executors. To update to newer versions of this chart, ensure the new Chart name is used.

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 | `""` | Required node affinity match expressions for Kubernetes Jobs, as a YAML list. e.g. `[{key: foo, operator: In, values: [bar]}]`. A pre-escaped JSON string is also accepted. |
+| executor.kubernetesJob.node.requiredAffinityMatchFields | string | `""` | Required node affinity match fields for Kubernetes Jobs, as a YAML list. e.g. `[{key: foo, operator: In, values: [bar]}]`. A pre-escaped JSON string is also accepted. |
 | 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 | `""` | Tolerations for Kubernetes Jobs, as a YAML list. e.g. `[{key: foo, operator: Equal, value: bar, effect: NoSchedule}]`. A pre-escaped JSON string is also accepted. |
+| executor.kubernetesJob.pod.affinity | string | `""` | Pod affinity terms for Kubernetes Jobs, as a YAML list. e.g. `[{labelSelector: {matchExpressions: [{key: foo, operator: In, values: [bar]}]}, topologyKey: kubernetes.io/hostname}]`. A pre-escaped JSON string is also accepted. |
+| executor.kubernetesJob.pod.antiAffinity | string | `""` | Pod anti-affinity terms for Kubernetes Jobs, as a YAML list. e.g. `[{labelSelector: {matchExpressions: [{key: foo, operator: In, values: [bar]}]}, topologyKey: kubernetes.io/hostname}]`. A pre-escaped JSON string is also accepted. |
 | 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. |

charts/sourcegraph-executor/k8s/templates/_helpers/_jsonValue.tpl

@@ -0,0 +1,9 @@
+{{/*
+Render a value the executor parses as JSON (tolerations, affinity, ...).
+A YAML list or map is JSON-encoded. A string is rendered as-is, so the
+pre-escaped JSON strings documented before YAML lists were supported, like
+'[{\"key\":\"foo\"}]', keep working.
+*/}}
+{{- define "executor.jsonValue" -}}
+{{- if kindIs "string" . -}}"{{ . }}"{{- else -}}{{ toJson . | quote }}{{- end -}}
+{{- end }}

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.jsonValue" .Values.executor.kubernetesJob.node.tolerations }}
+  EXECUTOR_KUBERNETES_NODE_REQUIRED_AFFINITY_MATCH_EXPRESSIONS: {{ include "executor.jsonValue" .Values.executor.kubernetesJob.node.requiredAffinityMatchExpressions }}
+  EXECUTOR_KUBERNETES_NODE_REQUIRED_AFFINITY_MATCH_FIELDS: {{ include "executor.jsonValue" .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.jsonValue" .Values.executor.kubernetesJob.pod.affinity }}
+  EXECUTOR_KUBERNETES_POD_ANTI_AFFINITY: {{ include "executor.jsonValue" .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 }}"

charts/sourcegraph-executor/k8s/tests/executor_test.yaml

@@ -125,3 +125,81 @@ tests:
           path: spec.template.spec.securityContext.runAsUser
       - isNull:
           path: spec.template.spec.securityContext.runAsGroup
+
+  - it: should JSON-encode kubernetesJob JSON fields given as YAML lists (issue #349)
+    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:
+                  matchExpressions:
+                    - key: foo
+                      operator: In
+                      values: [bar]
+                topologyKey: kubernetes.io/hostname
+            antiAffinity:
+              - labelSelector:
+                  matchExpressions:
+                    - key: app
+                      operator: In
+                      values: [executor]
+                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":{"matchExpressions":[{"key":"foo","operator":"In","values":["bar"]}]},"topologyKey":"kubernetes.io/hostname"}]'
+      - equal:
+          path: data.EXECUTOR_KUBERNETES_POD_ANTI_AFFINITY
+          value: '[{"labelSelector":{"matchExpressions":[{"key":"app","operator":"In","values":["executor"]}]},"topologyKey":"kubernetes.io/hostname"}]'
+
+  - it: should pass through kubernetesJob JSON fields given as pre-escaped JSON strings
+    template: executor.ConfigMap.yaml
+    set:
+      executor:
+        queueName: "test"
+        kubernetesJob:
+          node:
+            tolerations: '[{\"key\":\"foo\",\"operator\":\"Equal\",\"value\":\"bar\",\"effect\":\"NoSchedule\"}]'
+    asserts:
+      - equal:
+          path: data.EXECUTOR_KUBERNETES_NODE_TOLERATIONS
+          value: '[{"key":"foo","operator":"Equal","value":"bar","effect":"NoSchedule"}]'
+
+  - it: should render default kubernetesJob JSON fields as empty strings
+    template: executor.ConfigMap.yaml
+    set:
+      executor:
+        queueName: "test"
+    asserts:
+      - equal:
+          path: data.EXECUTOR_KUBERNETES_NODE_TOLERATIONS
+          value: ""
+      - equal:
+          path: data.EXECUTOR_KUBERNETES_POD_ANTI_AFFINITY
+          value: ""

charts/sourcegraph-executor/k8s/values.yaml

@@ -117,17 +117,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\"}]'
+      # -- Tolerations for Kubernetes Jobs, as a YAML list. e.g. `[{key: foo, operator: Equal, value: bar, effect: NoSchedule}]`. A pre-escaped JSON string is also accepted.
       tolerations: ""
-      # -- The JSON encoded required affinity match expressions for Kubernetes Jobs. e.g. '[{\"key\":\"foo\",\"operator\":\"In\",\"values\":[\"bar\"]}]'
+      # -- Required node affinity match expressions for Kubernetes Jobs, as a YAML list. e.g. `[{key: foo, operator: In, values: [bar]}]`. A pre-escaped JSON string is also accepted.
       requiredAffinityMatchExpressions: ""
-      # -- The JSON encoded required affinity match fields for Kubernetes Jobs. e.g. '[{\"key\":\"foo\",\"operator\":\"In\",\"values\":[\"bar\"]}]'
+      # -- Required node affinity match fields for Kubernetes Jobs, as a YAML list. e.g. `[{key: foo, operator: In, values: [bar]}]`. A pre-escaped JSON string is also accepted.
       requiredAffinityMatchFields: ""
 
     pod:
-      # -- The JSON encoded pod affinity for Kubernetes Jobs. e.g. '[{\"labelSelector\": {\"matchExpressions\": [{\"key\": \"foo\",\"operator\": \"In\",\"values\": [\"bar\"]}]},\"topologyKey\": \"kubernetes.io/hostname\"}]'
+      # -- Pod affinity terms for Kubernetes Jobs, as a YAML list. e.g. `[{labelSelector: {matchExpressions: [{key: foo, operator: In, values: [bar]}]}, topologyKey: kubernetes.io/hostname}]`. A pre-escaped JSON string is also accepted.
       affinity: ""
-      # -- The JSON encoded pod anti-affinity for Kubernetes Jobs. e.g. '[{\"labelSelector\": {\"matchExpressions\": [{\"key\": \"foo\",\"operator\": \"In\",\"values\": [\"bar\"]}]},\"topologyKey\": \"kubernetes.io/hostname\"}]'
+      # -- Pod anti-affinity terms for Kubernetes Jobs, as a YAML list. e.g. `[{labelSelector: {matchExpressions: [{key: foo, operator: In, values: [bar]}]}, topologyKey: kubernetes.io/hostname}]`. A pre-escaped JSON string is also accepted.
       antiAffinity: ""
 
   debug: