Skip to content

feat(helm): worker concurrency and extraArgs support #16

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
lewisjared merged 1 commit into from
Jul 1, 2026
+22 −0
Merged

feat(helm): worker concurrency and extraArgs support #16

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog/16.feature.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Add first-class ``concurrency`` and free-form ``extraArgs`` values to provider workers, rendered as Celery worker options after the ``--`` separator, and pin the esmvaltool worker to ``concurrency: 1`` to prevent node OOM kills when its Dask cluster fans out.
5 changes: 5 additions & 0 deletions helm/templates/providers/deployment.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,11 @@
{{- $spec := merge (deepCopy $.Values.defaults) $spec -}}
{{- $args := list "celery" "start-worker" "--loglevel" "DEBUG" -}}
{{- if ne $provider "orchestrator" -}} {{- $args = concat $args (list "--provider" $provider) -}} {{- end -}}
{{- /* Celery worker options passed after the `--` separator (e.g. --concurrency, --pool). */ -}}
{{- $workerArgs := list -}}
{{- if $spec.concurrency -}} {{- $workerArgs = append $workerArgs (printf "--concurrency=%v" $spec.concurrency) -}} {{- end -}}
{{- with $spec.extraArgs -}} {{- $workerArgs = concat $workerArgs . -}} {{- end -}}
{{- if $workerArgs -}} {{- $args = concat $args (list "--") $workerArgs -}} {{- end -}}
{{- /* Chart-managed esmvaltool config: auto-mount when providers.esmvaltool.config is set. */ -}}
{{- $volumes := $spec.volumes | default list -}}
{{- $volumeMounts := $spec.volumeMounts | default list -}}
Expand Down
16 changes: 16 additions & 0 deletions helm/values.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,16 @@ defaults:

replicaCount: 1

# Number of Celery worker child processes per pod (passed as `--concurrency`).
# Leave unset to use Celery's default (one per CPU core). Set to 1 to pin a
# provider to a single worker process, e.g. for memory-heavy diagnostics that
# OOM when several run concurrently on the same node.
concurrency: null

# Additional arguments appended to the Celery worker after the `--` separator,
# for options not exposed as first-class values (e.g. ["--pool=solo"]).
extraArgs: []
Comment on lines +180 to +184

@coderabbitai coderabbitai Bot Jul 1, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Verify provider overrides render after defaults define the same keys.
set -eu

if ! command -v helm >/dev/null 2>&1; then
  echo "helm is not installed; run this locally in an environment with Helm."
  exit 0
fi

helm template verify ./helm \
  --set providers.esmvaltool.concurrency=1 \
  --set 'providers.esmvaltool.extraArgs[0]=--pool=solo' |
  awk '
    /name: .*esmvaltool/ { in_esm=1 }
    in_esm && /args:/ { in_args=1 }
    in_args { print }
    in_args && /envFrom:/ { exit }
  '
# Expect the esmvaltool container args to include:
# - --
# - --concurrency=1
# - --pool=solo

Repository: Climate-REF/climate-ref-aft

Length of output: 234

🏁 Script executed:

#!/bin/bash
set -eu

printf '\n## helm/values.yaml (around the changed lines)\n'
sed -n '160,210p' helm/values.yaml

printf '\n## helm/templates/providers/deployment.yaml\n'
sed -n '1,240p' helm/templates/providers/deployment.yaml

printf '\n## any other merge/mergeOverwrite usage in helm/\n'
rg -n '\bmergeOverwrite\b|\bmerge\b' helm -g '*.yaml' -g '*.tpl' -g '*.yml'

Repository: Climate-REF/climate-ref-aft

Length of output: 6153

🌐 Web query:

Sprig merge mergeOverwrite precedence destination precedence Helm docs

💡 Result:

In Helm and the underlying Sprig library, the merge and mergeOverwrite functions handle dictionary precedence differently [1][2]. Both functions perform a deep merge, meaning they recursively merge nested dictionaries, but they do not perform a deep copy; nested objects remain shared instances across the merged dictionaries [1][3]. merge The merge function gives precedence to the destination (the first) dictionary [1][4]. When merging multiple dictionaries, if a key exists in both the destination dictionary and the source dictionaries, the value in the destination dictionary is preserved [1][2]. mergeOverwrite The mergeOverwrite function gives precedence from right to left [1][4]. In this operation, values in the source dictionaries overwrite values in the destination dictionary [1][3]. This is particularly useful for overriding default configurations, as it ensures that values provided in later arguments take priority over those in earlier ones [5]. Key Differences in Behavior A notable quirk often encountered is that merge may not overwrite boolean fields set to true in the destination dictionary, whereas mergeOverwrite correctly overwrites these values [5][6]. Because these functions mutate the destination dictionary (the first argument), it is common practice to pass an empty dictionary (dict) as the first argument or to use deepCopy to avoid unintended side effects on source objects [5][7]. Example Usage To merge with overwrite precedence: {{ $newdict:= mergeOverwrite (dict) $defaultValues $userOverrides }} To ensure a deep copy during the merge: {{ $newdict:= mergeOverwrite (dict) (deepCopy $defaultValues) $userOverrides }}

Citations:

Switch this merge to right-precedence.

merge (deepCopy $.Values.defaults) $spec keeps the new default concurrency: null and extraArgs: [] values, so provider overrides like providers.esmvaltool.concurrency=1 and extraArgs never make it into the worker args. Use mergeOverwrite here so provider-specific settings win.


image:
repository: ghcr.io/climate-ref/climate-ref
pullPolicy: IfNotPresent
Expand Down Expand Up @@ -235,6 +245,12 @@ defaults:
providers:
orchestrator: {}
esmvaltool:
# Pin the esmvaltool worker to a single Celery child process. Each esmvaltool
# execution already fans out via its own Dask cluster (see `config` below);
# running multiple Celery workers per pod multiplies that memory footprint and
# OOM-kills the node.
# See https://climate-ref.readthedocs.io/en/latest/how-to-guides/control-memory-use/
concurrency: 1
# Esmvalcore configuration written to a ConfigMap and mounted at
# /etc/esmvaltool/config.yaml. Without this, esmvalcore auto-detects all
# available cores and runs unbounded dask workers, which routinely OOMs
Expand Down
Loading