-
Notifications
You must be signed in to change notification settings - Fork 119
[cozy-lib] refactor resources #1155
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.
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
Conversation
WalkthroughResource preset and sanitization templates were refactored to use flat resource maps and allocation ratios for CPU, memory, and ephemeral storage. Resource values in several system component Helm charts were updated to reflect new defaults and ratios. The resource preset template signature was updated to accept a global context argument. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Template
participant SanitizeFunc
participant GlobalContext
User->>Template: Call cozy-lib.resources.preset(presetKey, global)
Template->>Template: Lookup flat preset map for presetKey
Template->>SanitizeFunc: Pass preset map and global context
SanitizeFunc->>SanitizeFunc: Validate flat map (no nested requests/limits)
SanitizeFunc->>GlobalContext: Retrieve allocation ratios (CPU, memory, ephemeral storage)
SanitizeFunc->>SanitizeFunc: Compute requests = limits / ratio
SanitizeFunc->>Template: Return resource map with requests and limits
Template->>User: Output sanitized resource YAML
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 (
|
<!-- This is an auto-generated comment: release notes by coderabbit.ai --> * **Refactor** * Updated the structure of resource presets for improved clarity and processing. * Adjusted template logic to streamline resource handling and removed previous resource limit calculations. * Modified template parameters to enhance flexibility in resource processing. * **Chores** * Improved internal template invocation for better compatibility with resource data. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Signed-off-by: Andrei Kvapil <kvapss@gmail.com>
Signed-off-by: Andrei Kvapil <kvapss@gmail.com>
…llocation-ratio options Signed-off-by: Andrei Kvapil <kvapss@gmail.com>
Signed-off-by: Andrei Kvapil <kvapss@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (3)
packages/library/cozy-lib/templates/_resources.tpl (2)
7-9
: Consider the implications of the high ephemeral storage allocation ratio.An allocation ratio of 40 for ephemeral storage means requests will be 1/40th of limits. This allows for significant overcommitment which could lead to node disk pressure if multiple pods consume their full ephemeral storage limits simultaneously.
87-118
: Bug fix correctly implemented, but consider precision loss in int conversion.The division by allocation ratios correctly fixes the bug from issue #1125. However, converting float results to int for memory and ephemeral-storage (lines 105, 110) can cause precision loss.
Consider preserving precision by keeping the float value or rounding appropriately:
-{{- $_ := set $output.requests $k ($memoryRequestF64 | int) }} +{{- $_ := set $output.requests $k ($memoryRequestF64 | printf "%.0f") }}-{{- $_ := set $output.requests $k ($ephemeralStorageRequestF64 | int) }} +{{- $_ := set $output.requests $k ($ephemeralStorageRequestF64 | printf "%.0f") }}packages/library/cozy-lib/templates/_resourcepresets.tpl (1)
30-38
: Consider scaling ephemeral storage with preset size.All presets from "nano" to "2xlarge" have the same 2Gi ephemeral storage. Larger workloads typically need more ephemeral storage proportional to their CPU and memory.
Consider scaling ephemeral storage similar to memory:
{{- $baseEphemeralStorage := dict - "nano" (dict "ephemeral-storage" "2Gi" ) - "micro" (dict "ephemeral-storage" "2Gi" ) - "small" (dict "ephemeral-storage" "2Gi" ) - "medium" (dict "ephemeral-storage" "2Gi" ) - "large" (dict "ephemeral-storage" "2Gi" ) - "xlarge" (dict "ephemeral-storage" "2Gi" ) - "2xlarge" (dict "ephemeral-storage" "2Gi" ) + "nano" (dict "ephemeral-storage" "1Gi" ) + "micro" (dict "ephemeral-storage" "2Gi" ) + "small" (dict "ephemeral-storage" "4Gi" ) + "medium" (dict "ephemeral-storage" "8Gi" ) + "large" (dict "ephemeral-storage" "16Gi" ) + "xlarge" (dict "ephemeral-storage" "32Gi" ) + "2xlarge" (dict "ephemeral-storage" "64Gi" ) }}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
packages/library/cozy-lib/templates/_resourcepresets.tpl
(1 hunks)packages/library/cozy-lib/templates/_resources.tpl
(3 hunks)packages/system/dashboard/values.yaml
(1 hunks)packages/system/fluxcd-operator/values.yaml
(1 hunks)packages/system/vertical-pod-autoscaler/values.yaml
(2 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 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/library/cozy-lib/templates/_resourcepresets.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.
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 (2)
- GitHub Check: Build
- GitHub Check: pre-commit
🔇 Additional comments (4)
packages/system/dashboard/values.yaml (1)
27-32
: LGTM! Memory allocation adjusted to match request and limit.The change to equal memory request and limit (128Mi) eliminates overcommit and ensures Redis gets exactly the memory it needs. This aligns with the new resource management approach.
packages/system/vertical-pod-autoscaler/values.yaml (1)
6-11
: Verify the 10x memory reduction is sufficient for VPA components.The memory allocations have been reduced by 90%:
- updater: 1100Mi → 110Mi
- recommender: 1600Mi → 160Mi
While reducing resource usage is good, such dramatic reductions could impact VPA performance, especially for the recommender component which analyzes metrics.
Have these reduced values been tested under typical workload conditions? Consider monitoring memory usage after deployment to ensure VPA components don't experience OOM issues.
Also applies to: 26-31
packages/system/fluxcd-operator/values.yaml (1)
8-14
: Resource allocation properly adjusted for guaranteed QoS.The changes set request equal to limit for both CPU (100m) and memory (350Mi), which:
- Guarantees the pod gets exactly these resources
- Places it in the Guaranteed QoS class
- Prevents resource overcommit
The values appear reasonable for a Flux operator workload.
packages/library/cozy-lib/templates/_resourcepresets.tpl (1)
40-43
: Resource preset refactoring looks good.The changes correctly:
- Merge all resource types into a single presets dictionary
- Pass the flat resource map to sanitize function
- Include global context for accessing allocation ratios
This aligns perfectly with the new flat resource map approach.
- Change wording for `resources` and `resourcesPreset` variables. - Explain and give exampls of other object-type variables, if their child fields are not annotated. - Fix a few typos, improve wording. - Bump all application charts to ensure that new texts are shown immediately after updating Cozystack. Incorporate changes in resourcesPreset values from #1155
Merge after #1117 and #1155 <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Documentation** * Improved clarity and detail in parameter descriptions across multiple app documentation files, especially for resource configuration options. * Expanded explanations for `resources` and `resourcesPreset` parameters, including explicit usage, allowed values, and fallback behavior. * Added new sections with YAML configuration examples and reference tables for resource presets in several app READMEs. * Corrected typos, improved formatting, and updated terminology for better readability and consistency. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
Add. missing commits from #1127, which were skipped by mistake
What this PR does
Release note
Summary by CodeRabbit
New Features
Refactor
Bug Fixes
Chores