[marcleblanc2]

Open issues

Found in review on 2026-11-13 (review thread). This PR stays a draft until the first two are settled.

1. storageClass.allowVolumeExpansion renders empty when the key is missing or null. helm upgrade --reuse-values from a release made by an older chart renders allowVolumeExpansion: with no value, where main renders true. Applying that render to an existing StorageClass on EKS changed it from true to false. Tested fix: allowVolumeExpansion: {{ ne (toString .Values.storageClass.allowVolumeExpansion) "false" }}, plus a test for the null case
2. dind goes from 1 job to 10 per pod, and the sizing advice does not work. Jobs run in the dind container, which has no resources value; executor.resources only reaches the executor container. Pick one: default executor.maximumNumJobs to 1 (no change for existing releases), or keep 10, mark the changelog entry **BREAKING:**, and correct the advice here and in values.yaml. The load test under 10 concurrent dind jobs under load shows what 10 costs: one over-sized batch of jobs takes all 10 down, and no pod resources value can bound them
3. Small ones:
   • executor.maximumNumJobs: 1000000 in a values file renders "1e+06"
   • the k8s executor chart renders EXECUTOR_MAXIMUM_NUM_JOBS: "" for a null executor.maximumNumJobs, as it does on main

Fixed since the review:

• The subchart example and the changelog now say sg.nameOverride is for new installs only (5ba343a).
• Added the missing tests (a805a0d): nodeSelector, affinity, and tolerations precedence in both directions for the migrator, k8s executor, dind executor, and private-docker-registry; the migrator's omitted pod securityContext; dind's default of 10 jobs on a single executor; and the executor.env allow / deny rules. helm unittest now passes 263, 12, 9, and 13 cases (sourcegraph, dind, k8s, migrator). The new cases fail 9 of 34 against main's templates, and each of 16 one-line template mutations fails at least one case. If issue 2 settles on a default of 1, the dind default case changes with it.
• The dind chart now refuses to render a null or empty executor.maximumNumJobs (c205543), where it rendered an empty variable and the executor crash-looped with invalid int "". helm upgrade --reuse-values from a chart older than 7.4.0 gets there without anyone writing null. Tested on m5 with a temporary release made from main's chart minus that default: --reuse-values to the fixed chart failed with executor.maximumNumJobs is required, for example 10 and left the release at revision 1 and the pod untouched; adding --set executor.maximumNumJobs=1 upgraded it, 2/2 ready with 0 restarts. 0 still renders, since it pauses the executor.
• Each dind queue can set its own job limit with queues[].maximumNumJobs (a521e88). Before, the chart ignored the field and refused EXECUTOR_MAXIMUM_NUM_JOBS in a queue's env, so every queue rendered executor.maximumNumJobs. A queue that sets none, null, or an empty string still uses executor.maximumNumJobs; 0 is kept and pauses that queue. Two new cases; each of 6 one-line template mutations fails one of them. Tested on m5 with a temporary two-queue release and executor.maximumNumJobs: 3: the codeintel executor, set to 2, ran with EXECUTOR_MAXIMUM_NUM_JOBS=2 and the batches executor, set to nothing, with 3; both pods were 2/2 ready and sent heartbeats. Changing only the codeintel limit to 5, then 0, replaced only the codeintel pod. helm unittest now passes 263, 16, 9, and 13 cases.
• The nameOverride docs no longer say every selector uses app.kubernetes.io/name (2d074c5). Rendered with nameOverride: custom to count: in sourcegraph, 38 of 39 selectors carry it (not the grafana StatefulSet's, nor jaeger's when enabled, which are fixed to jaeger), and none with sourcegraph.selectorLabelsOverride; in dind, the Deployment and StatefulSet do and the private-docker-registry Service does not; in k8s both selectors do, so that string is unchanged. The migrator has no selector, so its docs now say the label is on the Job and its pod and can change on an installed release, which the m5 migrator test above already did (revision 3, new Job, label custom).
• "replace it when set" is now "replaces it when set" in the k8s executor and migrator values.yaml and READMEs (a4a9dfb).
• The k8s chart's docs no longer call sourcegraph.nodeSelector, affinity, and tolerations the default for "every pod" (538a002). They are for the executor pod; the executor creates job pods itself from executor.kubernetesJob.node and executor.kubernetesJob.pod. This is a docs fix, not a chart fix: a fallback would move the job pods of any release that shares a values file with the sourcegraph chart, and sourcegraph.affinity cannot be expressed in the executor's settings, which take one required node term and required pod terms only. Tested on m5 with the live executor release and a batch job per step: with only sourcegraph.nodeSelector and sourcegraph.tolerations set, the executor pod got both and the job pod got neither; with executor.kubernetesJob.node.selector and .tolerations added, the job pod got those and nothing from sourcegraph.*. Rolled back to revision 30's manifest afterward, 0 diff lines.
• Restored the blank line before ## 8.0.0 in the executor CHANGELOG.md (55c0ec9, 26e9c54).

Problem

• Customers (and their AI assistants) read our charts' Values.yaml files, and configure values in their override files, and nothing changes, because the values we document are very different from the values the charts consume

• Some values worked but weren't documented

• Some values were read incorrectly, eg. storageClass.allowVolumeExpansion: false was ignored because true was an inline default which took precedence

• sourcegraph.nameOverride, in all four charts; the charts read a top-level nameOverride

• dind's executor.maximumNumJobs: dind never rendered it, and refused EXECUTOR_MAXIMUM_NUM_JOBS in executor.env, so executors always ran 1 job, the executor's default, while the chart documented 10

• Per-component nodeSelector, affinity, tolerations, and podSecurityContext that the dind and migrator READMEs list, and global ones the k8s chart documents

• dind's storageClass.create, provisioner, type, parameters, and allowedTopologies; dind has no StorageClass template

• The migrator's generic values block, copied from the sourcegraph chart, and the k8s chart's executor.securityContext.privileged

• Example override files: basic (wrong key names), custom-security-context (container fields under the pod), openshift (two serviceAccount keys), opentelemetry-exporter (the removed jaeger exporter)

Solution

• This PR fixes the documentation which was incorrect, and fixes the configs which were broken

• Amp rendered every one of the 730 values in the four charts, and checked the examples, READMEs, and doc site against the templates

• nameOverride: document the top-level key, which is the Helm convention and what a parent chart sets for a subchart (<alias>.nameOverride). An alias replaces the chart name in the selectors, so the subchart example now sets sg.nameOverride: sourcegraph

• dind job limit: render executor.maximumNumJobs, with the documented default of 10, as in the k8s chart. EXECUTOR_DOCKER_ADD_HOST_GATEWAY and EXECUTOR_KEEP_WORKSPACES, which have no value, can now be set in executor.env

• Scheduling: each pod uses its own nodeSelector, affinity, and tolerations, then the global sourcegraph.* ones: the k8s executor, the dind executor and registry, and the migrator Job. The dind registry gets podSecurityContext. These are now in values.yaml, instead of README prose

• Not done, on purpose: changing the effective top-level nameOverride key work the way sourcegraph.nameOverride was documented; anyone who set it has default selectors today, and would get new ones, so their next upgrade would fail

Customer-impacting changes on their next upgrade:

• dind executors that don't set executor.maximumNumJobs go from 1 job at a time to 10, and their pods restart once. Before upgrading, size executor.resources and executor.storage for 10 concurrent jobs, or set executor.maximumNumJobs: 1

• A release that set values that were ignored now gets them. For example, a k8s executor release with sourcegraph.nodeSelector gets its executor pod rescheduled; job pods are not affected

• Otherwise default renders don't change: sourcegraph and k8s are byte-identical to main. The migrator Job drops a securityContext: null

Testing

• helm unittest and helm lint pass on all four charts. Each new case fails on main, except nameOverride, which passes there too because no template changed: dind maximumNumJobs on every queue, per-component scheduling in dind and the migrator, global fallback in k8s, the registry's podSecurityContext, and allowVolumeExpansion: false

• Rendered every chart with defaults and with each example, before and after. The only changes are the intended ones above; the openshift example renders byte-identical, so the keys removed from it were unused

• Reran the per-value render audit on the three smaller charts. Every documented value now changes the render, except ones that need another value set first (executor.storage.class with storage.type: ephemeral, sourcegraph.image.defaultTag with useGlobalTagAsDefault), and the k8s chart's sourcegraph.labels, which Render sourcegraph.labels on every object, and add storage and ServiceAccount labels #1008 fixes

• The subchart example, against this branch's chart:

Values	Selector app.kubernetes.io/name
Standalone, default	sourcegraph
Standalone, nameOverride: custom	custom
Subchart alias sg, default	sg
Subchart alias sg, sg.nameOverride: sourcegraph	sourcegraph
sourcegraph.nameOverride, sg.sourcegraph.nameOverride, or global.nameOverride	ignored

On a cluster

Tested at 4549ed9 on the m5 test instance (EKS, Sourcegraph 8.1.4873), with temporary releases next to the live one, plus one real upgrade and rollback of the k8s executor release. The live sourcegraph release was upgraded later, see Upgrade of the live sourcegraph release.

Change	Result
storageClass.allowVolumeExpansion: false	main creates the class with true, this branch with false; the default is still true
k8s executor, global fallback	With only sourcegraph.nodeSelector set, the pod gets it; executor.nodeSelector replaces it. The pod came back Ready and sent heartbeats
dind executor.maximumNumJobs	EXECUTOR_MAXIMUM_NUM_JOBS=10 is in the pod, which restarts once and sends heartbeats
dind scheduling, registry podSecurityContext, executor.env	Per-component values land on the executor and registry pods; the two newly allowed variables are set once each; EXECUTOR_MAXIMUM_NUM_JOBS in executor.env is still refused
Migrator	migrator.nodeSelector replaces the global one, no securityContext: null, and the Job completed with args: ["validate"]
opentelemetry-exporter examples	On main, the collector rejects all three with unknown type: "jaeger". On this branch all three validate and the collector starts. Real traces: see Traces through the otlp exporter
custom-security-context example	On main, the API server rejects the render (strict server dry run); on this branch it passes
nameOverride	Changing it on an installed release fails with spec.selector ... field is immutable, as the changelog warns
helm unittest	263, 7, 8, and 10 cases pass (sourcegraph, dind, k8s, migrator)

Upgrade of the live sourcegraph release

Real upgrades of the m5 release at 26e9c54, 2026-11-14, with the instance's own values file. The release was deployed from a test branch, so its manifest differed from main's by 358 lines before this PR is counted. After each step: all 28 pods ready, the API and a search answered, and a server-side batch job completed through the executor. Log: logs/live-upgrade-m5.log in the name-override worktree.

Step	Result
As found (revision 75) to this branch	Upgraded. 27 of 28 pods were replaced, which is the 358-line difference, not this PR: the resulting manifest is identical to main's. No object was recreated (same UIDs, ClusterIPs, and volumes)
This branch to main	Revision bumped, 0 manifest lines changed, 28 of 28 pods kept, 0 objects changed
main to this branch	The same: 0 lines, 28 of 28 pods kept, 0 container restarts
main to this branch with --reuse-values	The same. The release then stores no storageClass.allowVolumeExpansion, the condition of open issue 1, but m5 sets storageClass.create: false, so no StorageClass is rendered and the shared one stayed allowVolumeExpansion: true
Rolled back to revision 75	Manifest and object UIDs identical to before the test

So for a release whose values touch none of the changed templates, upgrading from main to this branch changes nothing on the cluster. Not tested on the live release: storageClass.create: true (tested with a temporary release above), because m5 shares its StorageClass with other instances.

Traces through the otlp exporter

Tested at 26e9c54 on m5, 2026-11-14: the live release upgraded to this branch with each example's values, and real Sourcegraph traces followed from the frontend to a test Jaeger (frontend, otel-agent, otel-collector, otlp exporter, Jaeger). Each request was a GraphQL search sent with X-Sourcegraph-Should-Trace; its trace ID, from the X-Trace response header, was then looked up in Jaeger. Log: logs/otlp-traces-m5.log in the name-override worktree.

Values	Result
override.yaml	20 of 20 traced searches found in Jaeger, 59 to 67 spans each, from frontend and zoekt-webserver. 0 of 20 searches sent without the header
override.yaml and override-processor.yaml	20 of 200 found, the example's 10%. Each kept trace was whole (61 to 67 spans)
override-tls.yaml as written, to a server whose certificate is from a private CA	0 of 10. The exporter logs authentication handshake failed: with no ca_file, it checks the server against the public roots
override-tls.yaml plus tls.ca_file: /tls/ca.crt, with the CA added to the same Secret	20 of 20, through a collector that requires a client certificate (a handshake without one fails)

What the test needed that the examples do not say:

• Sourcegraph sends no traces until the site config has an "observability.tracing" object. m5's had none, and the first run found 0 of 20, with no span reaching otel-agent
• A config-only helm upgrade does not restart the otel-collector pod; it kept the old config until the pod was deleted. Render sourcegraph.labels on every object, and add storage and ServiceAccount labels #1008 adds the checksum annotation that fixes this
• A custom openTelemetry.gateway.command keeps its own --config, so the exporters are ignored. m5 sets one; the test reset it to the chart's command

The release was rolled back to its starting manifest afterward, and the site config, test Jaeger, collector, and Secrets were removed.

10 concurrent dind jobs under load

Tested at 26e9c54 on m5, 2026-11-14, with a temporary dind release on its own node (m5a.xlarge: 4 vCPU, 16 GiB, 80 GiB disk), the chart's default of 10 jobs, and no resources values, so the pod is BestEffort. Each job was a server-side batch spec whose step holds memory, writes a file inside its container, and spins one core. The live k8s executor was scaled to 0 for the test, so every job went to the dind executor. Log: logs/load-test-m5.log in the name-override worktree.

Load	Result
12 jobs, each 512 MiB memory, 1 GiB disk, 150 s	10 ran at once, 2 waited, all 12 completed. No restart, no eviction, no pressure condition. Node CPU at 106%, available memory 14.3 to 9.1 GiB, disk used 3.8 to 16.8 GiB
10 jobs, each growing to 2 GiB memory (20 GiB on a 16 GiB node)	After 55 s the kernel OOM killer killed dockerd, its containerd, and the container shims, not the jobs. The dind container restarted (exit 137), all 10 jobs failed with exit code 125, and the pulled images were lost. No eviction and no MemoryPressure condition
10 jobs, each writing 8 GiB (80 GiB on an 80 GiB disk)	Writes ran at the volume's 125 MiB/s limit with I/O pressure at 99%. After 5.5 minutes the kubelet set DiskPressure and evicted the pod. The replacement pod started on another node, the 10 jobs were reset and started again there, and began filling that node too

What the test showed about the chart, all of it also true on main:

• Job containers are not in the pod's cgroup. They run in /docker/<id> at the node's cgroup root, with oom_score_adj 0, while dockerd has 1000. kubectl top pod showed 22m CPU for the dind container while the node ran at 4000m. So a resources value on executor or dind cannot bound the jobs, and under memory pressure the kernel kills the Docker daemon before the jobs
• The kubelet counted 948 KiB of disk for the dind container when it evicted the pod, while the jobs had written over 40 GiB through it. Docker's disk use is not counted against the pod
• With the chart's default user, every batch job fails after its step runs: step.docker.step.0.post exits 1 with git's detected dubious ownership. The executor container clones as uid 100 and the batcheshelper container runs as root. The load tiers above ran with executor.podSecurityContext.runAsUser: 0, which makes the job complete
• Canceling the 10 retried disk jobs marked them canceled and removed their workspaces, but their 10 step containers kept running and writing until killed by hand

Not tested: the same loads on a node shared with other pods, and a codeintel job.

Linked PRs

• Render sourcegraph.labels on every object, and add storage and ServiceAccount labels #1008 renders sourcegraph.labels in the k8s chart. Conflicts with this PR in dind's _helpers.tpl and README, and the executor CHANGELOG.md; whichever merges second needs a rebase

• The docs repo has five related fixes, not in this PR (trace sampling, Grafana snippet, $JAEGER_HOST, max_traces, EKS StorageClass provisioner)

Amp threads

• The audit and these fixes

• Review and cluster tests

Changelog

• nameOverride is a top-level value in all four charts; sourcegraph.nameOverride never had an effect. Set it only on a new install

• dind: executor.maximumNumJobs now works, with a default of 10. Executors that don't set it go from 1 job at a time to 10. EXECUTOR_DOCKER_ADD_HOST_GATEWAY and EXECUTOR_KEEP_WORKSPACES can be set in executor.env. Each entry in queues can set its own maximumNumJobs

• Per-component nodeSelector, affinity, and tolerations work in the k8s, dind, and migrator charts, falling back to the global sourcegraph.* ones. Releases that had set these values now get them on upgrade.

• Removed values that never had an effect: dind storageClass.* except name, k8s executor.securityContext.privileged, and the migrator's generic block

• storageClass.allowVolumeExpansion: false now works

[github-actions]

Open the Diff Tour for this PR in Sourcegraph