[keycloak, cozy-lib] Calculate Java heap params - #1157
Conversation
This patch passes Java heap parameters to Keycloak to prevent OOM errors when the JVM lacks compatibility with cgroups v2 and fails to recognize container memory requests and limits. A new function is introduced in cozy-lib to calculate the heap parameters from requests and limits, setting Xmx to 75% of the memory limit and Xms to the lesser of the memory request or 25% of the memory limits. Change log: [keycloak] Calculate and pass Java heap parameters explicitly to prevent OOM errors. [cozy-lib] Introduce helper function to calculate Java heap params based on memory requests and limits. Signed-off-by: Timofei Larkin <lllamnyp@gmail.com>
WalkthroughThis change updates resource sanitization logic to ensure all resource limits are converted to strings and introduces a new template function to compute Java heap settings based on resource allocations. It also adds a Changes
Sequence Diagram(s)sequenceDiagram
participant HelmChart
participant CozyLib
participant K8sStatefulSet
HelmChart->>CozyLib: Call resources.defaultingSanitize(profile, resources)
CozyLib-->>HelmChart: Return sanitized resources
HelmChart->>CozyLib: Call resources.javaHeap(merged resources)
CozyLib-->>HelmChart: Return Java heap string
HelmChart->>K8sStatefulSet: Inject JAVA_OPTS_KC_HEAP env var with heap string
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
packages/system/keycloak/templates/sts.yaml (1)
48-58: Default pod resources to match JVM heap settingsThe JVM heap (
JAVA_OPTS_KC_HEAP) is always calculated from the sanitized preset (e.g. “small”), but theresources:block is only emitted when the user explicitly sets.Values.resources.requestsor.Values.resources.limits. If neither is set, the container will have no memory limit even though the JVM thinks it’s capped (e.g. 512Mi), defeating the purpose of the preset.Let’s unify both the resource stanza and the heap calculation on the same sanitized values:
• In
packages/system/keycloak/templates/sts.yamlaround lines 48–58, replace the raw.Values.resourcescheck/blocks with a precomputed$effectivemap based oncozy-lib.resources.defaultingSanitize+mergeOverwrite.
• Do the same in the env block around lines 71–76 so that the heap calc and theresources:stanza stay in lock-step.Suggested patch:
--- a/packages/system/keycloak/templates/sts.yaml +++ b/packages/system/keycloak/templates/sts.yaml @@ containers: - {{- if or .Values.resources.requests .Values.resources.limits }} + {{- $sanitized := fromYaml (include "cozy-lib.resources.defaultingSanitize" (list "small" (default dict .Values.resources.limits) $)) }} + {{- $effective := mergeOverwrite $sanitized $.Values.resources }} + {{- if or $effective.requests $effective.limits }} resources: - {{- if .Values.resources.limits }} - limits: - {{- toYaml .Values.resources.limits | nindent 14 }} - {{- end }} - {{- if .Values.resources.requests }} - requests: - {{- toYaml .Values.resources.requests | nindent 14 }} - {{- end }} + {{- if $effective.limits }} + limits: + {{- toYaml $effective.limits | nindent 14 }} + {{- end }} + {{- if $effective.requests }} + requests: + {{- toYaml $effective.requests | nindent 14 }} + {{- end }} {{- end }} @@ env: - {{- with (fromYaml (include "cozy-lib.resources.defaultingSanitize" (list "small" (default dict .Values.resources.limits) $))) }} - {{- with (mergeOverwrite . $.Values.resources ) }} - - name: JAVA_OPTS_KC_HEAP - value: {{ include "cozy-lib.resources.javaHeap" . }} - {{- end }} - {{- end }} + {{- /* reuse the same $effective above */}} + - name: JAVA_OPTS_KC_HEAP + value: {{ include "cozy-lib.resources.javaHeap" $effective }}Also apply this change to the identical snippet at lines 71–76.
This ensures the Kubernetes memory limit always aligns with the heap size assumed by the JVM.
🧹 Nitpick comments (1)
packages/system/keycloak/templates/sts.yaml (1)
71-76: Quote the generated heap flags to avoid YAML edge-casesThe
javaHeaphelper returns a value that starts with a dash and contains spaces (e.g.-Xms256m -Xmx768m). Although unquoted scalars are usually parsed correctly, a leading-can be mis-interpreted by YAML linters or future editors; quoting removes any ambiguity.- value: {{ include "cozy-lib.resources.javaHeap" . }} + value: "{{ include "cozy-lib.resources.javaHeap" . }}"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/library/cozy-lib/templates/_resources.tpl(2 hunks)packages/system/keycloak/charts/cozy-lib(1 hunks)packages/system/keycloak/templates/sts.yaml(1 hunks)
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: NickVolynkin
PR: cozystack/cozystack#1120
File: packages/apps/clickhouse/README.md:60-67
Timestamp: 2025-07-03T05:54:50.989Z
Learning: The `cozy-lib.resources.sanitize` function in packages/library/cozy-lib/templates/_resources.tpl supports both standard Kubernetes resource format (with limits:/requests: sections) and flat format (direct resource specifications). The flat format takes priority over nested values. CozyStack apps include cozy-lib as a chart dependency through symlinks in packages/apps/*/charts/cozy-lib directories.
packages/system/keycloak/charts/cozy-lib (2)
undefined
<retrieved_learning>
Learnt from: lllamnyp
PR: #1025
File: packages/apps/kafka/charts/cozy-lib:1-1
Timestamp: 2025-06-04T06:22:17.306Z
Learning: Files in packages/apps/*/charts/cozy-lib that contain the path "../../../library/cozy-lib" are symbolic links, not regular files. Git diffs may display symlinks as regular files with their target path as content, which can be misleading in code review.
</retrieved_learning>
<retrieved_learning>
Learnt from: NickVolynkin
PR: #1120
File: packages/apps/clickhouse/README.md:60-67
Timestamp: 2025-07-03T05:54:50.989Z
Learning: The cozy-lib.resources.sanitize function in packages/library/cozy-lib/templates/_resources.tpl supports both standard Kubernetes resource format (with limits:/requests: sections) and flat format (direct resource specifications). The flat format takes priority over nested values. CozyStack apps include cozy-lib as a chart dependency through symlinks in packages/apps/*/charts/cozy-lib directories.
</retrieved_learning>
packages/system/keycloak/templates/sts.yaml (1)
Learnt from: NickVolynkin
PR: cozystack/cozystack#1120
File: packages/apps/clickhouse/README.md:60-67
Timestamp: 2025-07-03T05:54:50.989Z
Learning: The `cozy-lib.resources.sanitize` function in packages/library/cozy-lib/templates/_resources.tpl supports both standard Kubernetes resource format (with limits:/requests: sections) and flat format (direct resource specifications). The flat format takes priority over nested values. CozyStack apps include cozy-lib as a chart dependency through symlinks in packages/apps/*/charts/cozy-lib directories.
packages/library/cozy-lib/templates/_resources.tpl (1)
Learnt from: NickVolynkin
PR: cozystack/cozystack#1120
File: packages/apps/clickhouse/README.md:60-67
Timestamp: 2025-07-03T05:54:50.989Z
Learning: The `cozy-lib.resources.sanitize` function in packages/library/cozy-lib/templates/_resources.tpl supports both standard Kubernetes resource format (with limits:/requests: sections) and flat format (direct resource specifications). The flat format takes priority over nested values. CozyStack apps include cozy-lib as a chart dependency through symlinks in packages/apps/*/charts/cozy-lib directories.
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Build
🔇 Additional comments (2)
packages/library/cozy-lib/templates/_resources.tpl (1)
101-101: LGTM: Consistent string conversion for resource limitsThe explicit string conversion for all resource limits ensures consistency with the existing pattern used for requests. This change aligns with Kubernetes resource specifications that expect string values.
Also applies to: 105-106, 110-111
packages/system/keycloak/charts/cozy-lib (1)
1-1: Symlink target is correct – no change neededThe symlink at
packages/system/keycloak/charts/cozy-libpoints to../../../library/cozy-lib/. Frompackages/system/keycloak/charts, three levels up lands inpackages/, making the full pathpackages/library/cozy-lib/, which matches the existing directory. You can ignore the suggested adjustment.Likely an incorrect or invalid review comment.
| {{- define "cozy-lib.resources.javaHeap" }} | ||
| {{- $memoryRequestInt := include "cozy-lib.resources.toFloat" .requests.memory | float64 | int64 }} | ||
| {{- $memoryLimitInt := include "cozy-lib.resources.toFloat" .limits.memory | float64 | int64 }} | ||
| {{- /* 4194304 is 4Mi */}} | ||
| {{- $xmxMi := div (mul $memoryLimitInt 3) 4194304 }} | ||
| {{- $xmsMi := min (div $memoryLimitInt 4194304) (div $memoryRequestInt 1048576) }} | ||
| {{- printf `-Xms%dm -Xmx%dm` $xmsMi $xmxMi }} | ||
| {{- end }} |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Review the Java heap calculation function for potential edge cases
The mathematical logic appears correct:
-Xmx= 75% of memory limit:(memoryLimit * 3) / (4 * 1048576)= 75% in MiB-Xms= min(memory request, 25% of memory limit) in MiB
However, consider these potential improvements:
- Missing error handling: The function assumes
.requests.memoryand.limits.memoryexist but doesn't validate this - Input validation: No validation that memory values are positive
- Edge case: Very small memory values could result in 0 heap sizes
Consider adding defensive checks:
{{- define "cozy-lib.resources.javaHeap" }}
+{{- if not (and .requests.memory .limits.memory) }}
+{{- fail "ERROR: Both memory requests and limits must be specified for Java heap calculation" }}
+{{- end }}
{{- $memoryRequestInt := include "cozy-lib.resources.toFloat" .requests.memory | float64 | int64 }}
{{- $memoryLimitInt := include "cozy-lib.resources.toFloat" .limits.memory | float64 | int64 }}
{{- /* 4194304 is 4Mi */}}
{{- $xmxMi := div (mul $memoryLimitInt 3) 4194304 }}
{{- $xmsMi := min (div $memoryLimitInt 4194304) (div $memoryRequestInt 1048576) }}
+{{- if or (lt $xmxMi 1) (lt $xmsMi 1) }}
+{{- fail "ERROR: Memory allocation too small for Java heap calculation" }}
+{{- end }}
{{- printf `-Xms%dm -Xmx%dm` $xmsMi $xmxMi }}
{{- end }}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| {{- define "cozy-lib.resources.javaHeap" }} | |
| {{- $memoryRequestInt := include "cozy-lib.resources.toFloat" .requests.memory | float64 | int64 }} | |
| {{- $memoryLimitInt := include "cozy-lib.resources.toFloat" .limits.memory | float64 | int64 }} | |
| {{- /* 4194304 is 4Mi */}} | |
| {{- $xmxMi := div (mul $memoryLimitInt 3) 4194304 }} | |
| {{- $xmsMi := min (div $memoryLimitInt 4194304) (div $memoryRequestInt 1048576) }} | |
| {{- printf `-Xms%dm -Xmx%dm` $xmsMi $xmxMi }} | |
| {{- end }} | |
| {{- define "cozy-lib.resources.javaHeap" }} | |
| {{- if not (and .requests.memory .limits.memory) }} | |
| {{- fail "ERROR: Both memory requests and limits must be specified for Java heap calculation" }} | |
| {{- end }} | |
| {{- $memoryRequestInt := include "cozy-lib.resources.toFloat" .requests.memory | float64 | int64 }} | |
| {{- $memoryLimitInt := include "cozy-lib.resources.toFloat" .limits.memory | float64 | int64 }} | |
| {{- /* 4194304 is 4Mi */}} | |
| {{- $xmxMi := div (mul $memoryLimitInt 3) 4194304 }} | |
| {{- $xmsMi := min (div $memoryLimitInt 4194304) (div $memoryRequestInt 1048576) }} | |
| {{- if or (lt $xmxMi 1) (lt $xmsMi 1) }} | |
| {{- fail "ERROR: Memory allocation too small for Java heap calculation" }} | |
| {{- end }} | |
| {{- printf `-Xms%dm -Xmx%dm` $xmsMi $xmxMi }} | |
| {{- end }} |
🤖 Prompt for AI Agents
In packages/library/cozy-lib/templates/_resources.tpl around lines 167 to 174,
the Java heap calculation function lacks validation for the presence and
positivity of .requests.memory and .limits.memory, and does not handle very
small memory values that could lead to zero heap sizes. Add checks to ensure
these memory values exist and are positive before performing calculations, and
set minimum heap sizes to avoid zero values in the -Xms and -Xmx parameters.
Implement fallback or default values if inputs are missing or invalid to make
the function more robust.
What this PR does
This patch passes Java heap parameters to Keycloak to prevent OOM errors when the JVM lacks compatibility with cgroups v2 and fails to recognize container memory requests and limits. A new function is introduced in cozy-lib to calculate the heap parameters from requests and limits, setting Xmx to 75% of the memory limit and Xms to the lesser of the memory request or 25% of the memory limits.
Release note
Summary by CodeRabbit