Skip to content

Allow setting no limits and requests for Brainstore #70

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
Jeff McCollum (jeffmccollum) merged 1 commit into from
Mar 20, 2026
+15 −27
Merged

Allow setting no limits and requests for Brainstore #70

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
14 changes: 5 additions & 9 deletions braintrust/templates/brainstore-fastreader-deployment.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,15 +71,11 @@ spec:
ports:
- containerPort: {{ .Values.brainstore.fastreader.service.port }}
resources:
requests:
cpu: {{ .Values.brainstore.fastreader.resources.requests.cpu | quote }}
memory: {{ .Values.brainstore.fastreader.resources.requests.memory | quote }}
{{- if and (eq .Values.cloud "google") (eq .Values.google.mode "autopilot") .Values.brainstore.fastreader.volume.size }}
ephemeral-storage: {{ .Values.brainstore.fastreader.volume.size | quote }}
{{- end }}
limits:
cpu: {{ .Values.brainstore.fastreader.resources.limits.cpu | quote }}
memory: {{ .Values.brainstore.fastreader.resources.limits.memory | quote }}
{{- $resources := .Values.brainstore.fastreader.resources }}
{{- if and (eq .Values.cloud "google") (eq .Values.google.mode "autopilot") .Values.brainstore.fastreader.volume.size }}
{{- $resources = merge (dict "requests" (merge $resources.requests (dict "ephemeral-storage" .Values.brainstore.fastreader.volume.size))) $resources }}
{{- end }}
{{- toYaml $resources | nindent 12 }}
{{- with .Values.brainstore.livenessProbe }}
livenessProbe:
{{- toYaml . | nindent 12 }}
Expand Down
14 changes: 5 additions & 9 deletions braintrust/templates/brainstore-reader-deployment.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,15 +71,11 @@ spec:
ports:
- containerPort: {{ .Values.brainstore.reader.service.port }}
resources:
requests:
cpu: {{ .Values.brainstore.reader.resources.requests.cpu | quote }}
memory: {{ .Values.brainstore.reader.resources.requests.memory | quote }}
{{- if and (eq .Values.cloud "google") (eq .Values.google.mode "autopilot") .Values.brainstore.reader.volume.size }}
ephemeral-storage: {{ .Values.brainstore.reader.volume.size | quote }}
{{- end }}
limits:
cpu: {{ .Values.brainstore.reader.resources.limits.cpu | quote }}
memory: {{ .Values.brainstore.reader.resources.limits.memory | quote }}
{{- $resources := .Values.brainstore.reader.resources }}
{{- if and (eq .Values.cloud "google") (eq .Values.google.mode "autopilot") .Values.brainstore.reader.volume.size }}
{{- $resources = merge (dict "requests" (merge $resources.requests (dict "ephemeral-storage" .Values.brainstore.reader.volume.size))) $resources }}
Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot Mar 20, 2026

Choose a reason for hiding this comment

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

P2 Badge Deep-copy resources before merge

merge mutates its destination map, and here the destination is $resources.requests, which is a direct reference into .Values. That means rendering this template can mutate chart values in-place; if a deploy uses shared resource maps (for example via YAML anchors or generated values reused across reader/writer/fastreader), the injected ephemeral-storage request can bleed into other components unexpectedly. Wrap the merged maps in deepCopy before merging to keep template rendering isolated.

Useful? React with 👍 / 👎.

Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot Mar 20, 2026

Choose a reason for hiding this comment

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

P2 Badge Enforce autopilot ephemeral-storage with overwrite

This merge expression does not guarantee that volume.size wins: merge gives precedence to the first map, so an existing resources.requests.ephemeral-storage value is kept and the autopilot-required value from volume.size is ignored. In the autopilot branch this can render a pod request that no longer matches the configured volume size, which can lead to scheduling/rejection issues. Use mergeOverwrite (or explicit set) for ephemeral-storage in this path.

Useful? React with 👍 / 👎.

{{- end }}
{{- toYaml $resources | nindent 12 }}
{{- with .Values.brainstore.livenessProbe }}
livenessProbe:
{{- toYaml . | nindent 12 }}
Expand Down
14 changes: 5 additions & 9 deletions braintrust/templates/brainstore-writer-deployment.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,15 +71,11 @@ spec:
ports:
- containerPort: {{ .Values.brainstore.writer.service.port }}
resources:
requests:
cpu: {{ .Values.brainstore.writer.resources.requests.cpu | quote }}
memory: {{ .Values.brainstore.writer.resources.requests.memory | quote }}
{{- if and (eq .Values.cloud "google") (eq .Values.google.mode "autopilot") .Values.brainstore.writer.volume.size }}
ephemeral-storage: {{ .Values.brainstore.writer.volume.size | quote }}
{{- end }}
limits:
cpu: {{ .Values.brainstore.writer.resources.limits.cpu | quote }}
memory: {{ .Values.brainstore.writer.resources.limits.memory | quote }}
{{- $resources := .Values.brainstore.writer.resources }}
{{- if and (eq .Values.cloud "google") (eq .Values.google.mode "autopilot") .Values.brainstore.writer.volume.size }}
{{- $resources = merge (dict "requests" (merge $resources.requests (dict "ephemeral-storage" .Values.brainstore.writer.volume.size))) $resources }}
{{- end }}
{{- toYaml $resources | nindent 12 }}
{{- with .Values.brainstore.livenessProbe }}
livenessProbe:
{{- toYaml . | nindent 12 }}
Expand Down
Loading