From 443fc6ff034bbb93be13bfa0970f45449e6ae05b Mon Sep 17 00:00:00 2001 From: Evan Nemerson Date: Tue, 29 Sep 2026 11:50:42 -0400 Subject: [PATCH] CP-48316: Let chart pods reserve ephemeral storage The chart accepted any Kubernetes resource settings but rendered only CPU and memory, silently dropping ephemeral-storage requests and limits. Pods could therefore land on nodes without enough allocatable ephemeral storage, even when the aggregator emptyDir had a sizeLimit. Resource blocks now pass through every key they are given. The aggregator collector, which writes the metric files to the shared emptyDir, also requests aggregator.database.emptyDir.sizeLimit as ephemeral storage when it is set, unless the collector requests ephemeral storage explicitly. No limit is derived, and default renders are unchanged because sizeLimit defaults to empty. Helm unit tests cover pass-through, the derived request, the explicit override, and the unset and disabled cases; existing template baselines are unchanged. Co-Authored-By: Claude Opus 5.5 --- app/functions/helmless/default-values.yaml | 6 +++ helm/templates/_helpers.tpl | 29 +++-------- helm/templates/aggregator-deploy.yaml | 18 ++++++- .../aggregator_resources_fallback_test.yaml | 51 +++++++++++++++++++ helm/values.yaml | 6 +++ 5 files changed, 87 insertions(+), 23 deletions(-) diff --git a/app/functions/helmless/default-values.yaml b/app/functions/helmless/default-values.yaml index e9e3dc587..ff1fc060f 100644 --- a/app/functions/helmless/default-values.yaml +++ b/app/functions/helmless/default-values.yaml @@ -1749,6 +1749,12 @@ aggregator: # Whether to enable the emptyDir volume for the aggregator. enabled: true # Size limit for the emptyDir volume. If not set, no limit is applied. + # + # When set, this is also used as the collector container's + # ephemeral-storage request, so the pod is only scheduled onto a node + # with enough allocatable ephemeral storage. Set + # components.aggregator.collector.resources.requests.ephemeral-storage + # to override it. sizeLimit: "" # Configuration for the collector component of the aggregator. collector: diff --git a/helm/templates/_helpers.tpl b/helm/templates/_helpers.tpl index 1fa6d0670..5b3f72669 100644 --- a/helm/templates/_helpers.tpl +++ b/helm/templates/_helpers.tpl @@ -1345,28 +1345,15 @@ Example usage: {{- if . -}} {{- $resources := . -}} {{- $cleanResources := dict -}} - {{- if $resources.requests -}} - {{- $cleanRequests := dict -}} - {{- if and $resources.requests.cpu (ne $resources.requests.cpu "") -}} - {{- $_ := set $cleanRequests "cpu" $resources.requests.cpu -}} - {{- end -}} - {{- if and $resources.requests.memory (ne $resources.requests.memory "") -}} - {{- $_ := set $cleanRequests "memory" $resources.requests.memory -}} - {{- end -}} - {{- if $cleanRequests -}} - {{- $_ := set $cleanResources "requests" $cleanRequests -}} - {{- end -}} - {{- end -}} - {{- if $resources.limits -}} - {{- $cleanLimits := dict -}} - {{- if and $resources.limits.cpu (ne $resources.limits.cpu "") -}} - {{- $_ := set $cleanLimits "cpu" $resources.limits.cpu -}} - {{- end -}} - {{- if and $resources.limits.memory (ne $resources.limits.memory "") -}} - {{- $_ := set $cleanLimits "memory" $resources.limits.memory -}} + {{- range $section := list "requests" "limits" -}} + {{- $clean := dict -}} + {{- range $name, $value := (index $resources $section | default dict) -}} + {{- if and $value (ne (toString $value) "") -}} + {{- $_ := set $clean $name $value -}} + {{- end -}} {{- end -}} - {{- if $cleanLimits -}} - {{- $_ := set $cleanResources "limits" $cleanLimits -}} + {{- if $clean -}} + {{- $_ := set $cleanResources $section $clean -}} {{- end -}} {{- end -}} {{- if $cleanResources -}} diff --git a/helm/templates/aggregator-deploy.yaml b/helm/templates/aggregator-deploy.yaml index 4edf7b430..f0965c0a2 100644 --- a/helm/templates/aggregator-deploy.yaml +++ b/helm/templates/aggregator-deploy.yaml @@ -115,10 +115,24 @@ spec: initialDelaySeconds: {{ $collectorLiveness.initialDelaySeconds }} periodSeconds: {{ $collectorLiveness.periodSeconds }} failureThreshold: {{ $collectorLiveness.failureThreshold }} - {{- include "cloudzero-agent.generateResources" (include "cloudzero-agent.mergeStringOverwrite" (list + {{- $collectorResources := include "cloudzero-agent.mergeStringOverwrite" (list (.Values.components.aggregator.collector.resources | default (dict)) (.Values.aggregator.collector.resources | default (dict)) - ) | fromYaml) | nindent 10 }} + ) | fromYaml -}} + {{- /* + The collector writes the metric files to the shared emptyDir, so when a + sizeLimit is set, reserve that much ephemeral storage for scheduling + unless the collector requests ephemeral storage explicitly. + */}} + {{- $emptyDir := .Values.aggregator.database.emptyDir }} + {{- if and $emptyDir.enabled $emptyDir.sizeLimit }} + {{- $requests := $collectorResources.requests | default (dict) }} + {{- if not (index $requests "ephemeral-storage") }} + {{- $_ := set $requests "ephemeral-storage" $emptyDir.sizeLimit }} + {{- $_ := set $collectorResources "requests" $requests }} + {{- end }} + {{- end }} + {{- include "cloudzero-agent.generateResources" $collectorResources | nindent 10 }} {{- include "cloudzero-agent.generateContainerSecurityContext" (mergeOverwrite (.Values.defaults.securityContext | default (dict)) (.Values.aggregator.collector.securityContext | default (dict)) diff --git a/helm/tests/aggregator_resources_fallback_test.yaml b/helm/tests/aggregator_resources_fallback_test.yaml index 705fa9d5e..12d9e0ed4 100644 --- a/helm/tests/aggregator_resources_fallback_test.yaml +++ b/helm/tests/aggregator_resources_fallback_test.yaml @@ -87,3 +87,54 @@ tests: - equal: path: spec.template.spec.containers[0].resources.limits.cpu value: "4000m" + + - it: should render ephemeral-storage requests and limits + set: + components.aggregator.collector.resources: + requests: + ephemeral-storage: "1Gi" + limits: + ephemeral-storage: "2Gi" + asserts: + - equal: + path: spec.template.spec.containers[0].resources.requests.ephemeral-storage + value: "1Gi" + - equal: + path: spec.template.spec.containers[0].resources.limits.ephemeral-storage + value: "2Gi" + + - it: should request the emptyDir sizeLimit as collector ephemeral-storage + set: + aggregator.database.emptyDir.sizeLimit: "5Gi" + asserts: + - equal: + path: spec.template.spec.containers[0].resources.requests.ephemeral-storage + value: "5Gi" + - notExists: + path: spec.template.spec.containers[0].resources.limits.ephemeral-storage + - notExists: + path: spec.template.spec.containers[1].resources.requests.ephemeral-storage + + - it: should prefer an explicit collector ephemeral-storage request over the sizeLimit + set: + aggregator.database.emptyDir.sizeLimit: "5Gi" + components.aggregator.collector.resources: + requests: + ephemeral-storage: "1Gi" + asserts: + - equal: + path: spec.template.spec.containers[0].resources.requests.ephemeral-storage + value: "1Gi" + + - it: should not request ephemeral-storage when no sizeLimit is set + asserts: + - notExists: + path: spec.template.spec.containers[0].resources.requests.ephemeral-storage + + - it: should not request ephemeral-storage when the emptyDir is disabled + set: + aggregator.database.emptyDir.enabled: false + aggregator.database.emptyDir.sizeLimit: "5Gi" + asserts: + - notExists: + path: spec.template.spec.containers[0].resources.requests.ephemeral-storage diff --git a/helm/values.yaml b/helm/values.yaml index d1c4d140d..8961d952d 100644 --- a/helm/values.yaml +++ b/helm/values.yaml @@ -1749,6 +1749,12 @@ aggregator: # Whether to enable the emptyDir volume for the aggregator. enabled: true # Size limit for the emptyDir volume. If not set, no limit is applied. + # + # When set, this is also used as the collector container's + # ephemeral-storage request, so the pod is only scheduled onto a node + # with enough allocatable ephemeral storage. Set + # components.aggregator.collector.resources.requests.ephemeral-storage + # to override it. sizeLimit: "" # Configuration for the collector component of the aggregator. collector: