[cozy-lib] Introduce instance-type resource presets (s1/u1/m1)#1959
[cozy-lib] Introduce instance-type resource presets (s1/u1/m1)#1959matthieu-robin wants to merge 6 commits intocozystack:mainfrom
Conversation
matthieu-robin
requested review from
kvaps,
lllamnyp and
nbykov0
as code owners
dosubot
bot
added
size:XL
Summary of ChangesHello @matthieu-robin, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the resource allocation system by introducing a more structured and descriptive instance-type naming convention for resource presets. This change provides greater flexibility and clarity in defining resource requirements for different applications by categorizing them into specific series based on CPU to memory ratios, alongside a standardized ephemeral storage configuration. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughSystematic migration of resource preset naming across 15+ applications and system components. ResourcesPreset enums are expanded from flat values (nano, micro, small, medium, large, xlarge, 2xlarge) to hierarchical prefixed families (s1., u1., m1.*), with corresponding default value updates in configuration files, schemas, and documentation. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a new, well-defined instance-type naming convention for resource presets, which significantly improves clarity and consistency. The implementation in cozy-lib is a major improvement, as it fixes a critical bug in the previous preset definition logic. The updates to the default presets across the various applications are largely consistent with the new scheme. I have provided a few suggestions to improve consistency for some services based on the new instance type definitions. Overall, this is an excellent and necessary refactoring.
packages/apps/mysql/values.yaml
Outdated
| ## @param {ResourcesPreset} resourcesPreset="nano" - Default sizing preset used when `resources` is omitted. | ||
| resourcesPreset: "nano" | ||
| ## @param {ResourcesPreset} resourcesPreset="s1.nano" - Default sizing preset used when `resources` is omitted. | ||
| resourcesPreset: "s1.nano" |
There was a problem hiding this comment.
The PR description states that the u1 series is for "general purpose databases and messaging". MySQL is a general-purpose database, but it's configured with an s1 series preset. For consistency with other databases in this repository (like PostgreSQL and MongoDB) and the new naming convention's intent, consider using a preset from the u1 series.
resourcesPreset: "u1.nano"
packages/apps/nats/values.yaml
Outdated
| ## @param {ResourcesPreset} resourcesPreset="nano" - Default sizing preset used when `resources` is omitted. | ||
| resourcesPreset: "nano" | ||
| ## @param {ResourcesPreset} resourcesPreset="s1.nano" - Default sizing preset used when `resources` is omitted. | ||
| resourcesPreset: "s1.nano" |
There was a problem hiding this comment.
The PR description specifies the u1 series for "general purpose databases and messaging". NATS is a messaging system, but it's using an s1 preset. To align with the stated purpose of the new instance types and for consistency with other messaging systems (like Kafka), it would be better to use a u1 series preset.
resourcesPreset: "u1.nano"
packages/extra/seaweedfs/values.yaml
Outdated
| storageClass: "" | ||
| resources: {} | ||
| resourcesPreset: "small" | ||
| resourcesPreset: "s1.small" |
There was a problem hiding this comment.
The db component of SeaweedFS appears to be a database for metadata. The PR description indicates that the u1 series is for "general purpose databases". To maintain consistency with how other databases are configured in this PR, it would be more appropriate to use a u1 series preset for this component.
resourcesPreset: "u1.small"![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/library/cozy-lib/templates/_resourcepresets.tpl (1)
59-63:⚠️ Potential issue | 🔴 CriticalPass the preset key directly instead of wrapping in a list.
Line 62 incorrectly passes
list ($args $global)tocozy-lib.resources.unsanitizedPreset, which expects.to be a string preset key. This breaks thehasKeyandindexlookups on line 48-51. The correct usage is shown in_resources.tpl, which passes the preset string directly.🔧 Proposed fix
-{{- $unsanitizedPreset := include "cozy-lib.resources.unsanitizedPreset" list ($args $global) | fromYaml }} +{{- $unsanitizedPreset := include "cozy-lib.resources.unsanitizedPreset" $args | fromYaml }}
🧹 Nitpick comments (1)
packages/extra/seaweedfs/values.yaml (1)
66-126: Uniforms1.smallpreset applied across all SeaweedFS components.All five components (db, master, filer, volume, s3) use the same
s1.smallpreset. This provides consistency, but you may want to consider whether thedbcomponent would benefit fromu1.small(1:4 CPU:memory ratio) given it's a database workload per the PR's series definitions.That said, if the current uniform approach is intentional for simplicity or based on SeaweedFS-specific characteristics, this is fine as-is.
dosubot
bot
added
size:XXL
matthieu-robin
force-pushed
the
resourcepreset
branch
3 times, most recently
from
d3be393 to
86ce4f8
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/apps/mysql/README.md (1)
129-137:⚠️ Potential issue | 🟡 MinorOutdated preset reference table.
The preset table still documents the old flat naming convention (
nano,micro,small, etc.) without the new series prefixes (s1.*,u1.*,m1.*). This is inconsistent with the newu1.nanodefault shown in the parameters table at line 78 and will confuse users.Update this table to document all three series with their CPU:memory ratios as defined in the PR objectives (s1 = 1:2, u1 = 1:4, m1 = 1:8).
packages/extra/seaweedfs/README.md (1)
1-1:⚠️ Potential issue | 🟡 MinorIncorrect service name in title.
The README title says "Managed NATS Service" but this file documents SeaweedFS. This appears to be a copy-paste error.
📝 Proposed fix
-# Managed NATS Service +# Managed SeaweedFS Service
🤖 Fix all issues with AI agents
In `@packages/library/cozy-lib/templates/_resourcepresets.tpl`:
- Around line 54-57: The example usage of the Go template helper is invalid:
change the include invocation so the list call is evaluated as a single argument
by wrapping it in parentheses; update the example that currently shows include
"cozy-lib.resources.preset" list ("s1.nano" $) to call include
"cozy-lib.resources.preset" (list "s1.nano" $) so the (list ...) expression is
passed correctly to the cozy-lib.resources.preset template.
- Around line 48-50: The error message uses (join "," (keys $presets)) which is
non-deterministic because keys iteration order is arbitrary; change it to join
over a sorted key list (e.g. (join "," (sort (keys $presets)))) so the output is
deterministic — update the template expression that composes the error (the
printf call using $presets, keys, join, and fail) to sort the keys before
joining.
🧹 Nitpick comments (1)
packages/apps/rabbitmq/values.yaml (1)
41-42: Consider usingu1.nanofor consistency with messaging workload classification.According to the PR description, the u1 (Universal, 1:4 CPU:memory ratio) series is designated for "general purpose databases and messaging" workloads. However, RabbitMQ defaults to
s1.nanowhile NATS (also a messaging system) correctly defaults tou1.nano.For consistency and to align with the intended resource ratios for messaging workloads, consider changing to:
Suggested change
-## `@param` {ResourcesPreset} resourcesPreset="s1.nano" - Default sizing preset used when `resources` is omitted. -resourcesPreset: "s1.nano" +## `@param` {ResourcesPreset} resourcesPreset="u1.nano" - Default sizing preset used when `resources` is omitted. +resourcesPreset: "u1.nano"
matthieu-robin
force-pushed
the
resourcepreset
branch
from
8587ea6 to
cfb0935
Compare
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/apps/vpn/README.md (1)
58-67:⚠️ Potential issue | 🟠 MajorUpdate preset table to match schema — documentation shows old flat names instead of current series-prefixed names.
The preset reference table documents outdated flat names (
nano,micro, etc.), but the schema validates only prefixed names (s1.nano,u1.nano, etc.). Users following the README will encounter validation errors.The PR introduces three series with different CPU:memory ratios:
- s1 (Standard, 1:2)
- u1 (Universal, 1:4)
- m1 (Memory, 1:8)
Update the table to document all 24 presets across the three series and include the new
4xlargesize. Group by series for clarity.
🤖 Fix all issues with AI agents
In `@packages/apps/clickhouse/README.md`:
- Around line 26-34: Update the resourcesPreset reference table in the
ClickHouse README to use the new preset family names (u1.*, s1.*, m1.*) instead
of the legacy flat names: change the `resourcesPreset` examples and any listed
presets in the table rows that currently show legacy names to the new u1/s1/m1
scheme, and mirror the same replacements in the second occurrence (the other
table at lines ~66-72). Ensure the `resourcesPreset` column value and any
explanatory text referencing presets are consistent with the new family naming
convention.
In `@packages/apps/ferretdb/README.md`:
- Around line 11-18: The presets table in the README documents legacy names but
resourcesPreset and the default value use the new naming (resourcesPreset,
default `u1.nano`); update the presets section to replace legacy entries (e.g.,
`nano`, `micro`, ...) with the new 24 preset names using the s1/u1/m1 families
across all sizes (s1.xxx, u1.xxx, m1.xxx) so the table lists every preset
variant and matches the default `u1.nano` naming; ensure the updated table
entries reference the `resourcesPreset` field and any examples reflect the new
names.
In `@packages/apps/foundationdb/README.md`:
- Line 170: The Resources example uses legacy preset names (e.g., "medium") but
the default and new presets use the s1/u1/m1 family (e.g., "u1.small"); update
the example to use the new naming scheme by replacing legacy values in the
example's resourcesPreset field with the corresponding new preset (for example
change resourcesPreset: "medium" to resourcesPreset: "u1.small") and ensure any
referenced sizing labels in that example match the new s1/u1/m1 presets so the
example aligns with the table entry for resourcesPreset.
In `@packages/apps/kafka/README.md`:
- Around line 25-34: Update the README table entry and preset list so the preset
names match the new naming scheme (s1/u1/m1) instead of legacy names
(nano/micro/…); specifically update the `kafka.resourcesPreset` description and
the preset table below to list the new families and sizes (s1, u1, m1) and their
CPU/memory ratios or replace the table with a link to the canonical cozy-lib
presets doc so `kafka.resourcesPreset` and related `kafka.resources` users see
the correct current defaults.
In `@packages/apps/kubernetes/README.md`:
- Around line 148-172: The README's legacy resourcesPreset names are
inconsistent with the new defaults; update the documented preset names for the
control plane entries (controlPlane.apiServer.resourcesPreset,
controlPlane.controllerManager.resourcesPreset,
controlPlane.scheduler.resourcesPreset,
controlPlane.konnectivity.server.resourcesPreset) and the later resourcesPreset
table to use the new s1/u1/m1 family names (or replace that whole table with a
single link to the shared preset documentation), ensuring the example default
values (e.g., u1.small, s1.micro) and explanatory text match the new naming
scheme.
In `@packages/apps/postgres/README.md`:
- Around line 69-79: Update the presets reference table so it uses the new
preset families (s1/u1/m1) instead of the legacy names (nano/micro/…) and ensure
the default shown for resourcesPreset remains `u1.nano`; modify the table rows
that list preset names and examples to the new family naming, update any
explanatory text that mentions legacy presets, and verify the `resourcesPreset`
entry still displays `u1.nano` as the default.
In `@packages/apps/rabbitmq/README.md`:
- Around line 16-25: The presets matrix in the RabbitMQ README uses legacy flat
names (e.g., "nano", "micro") but the default resourcesPreset is "s1.nano";
update the presets table to use the new names (s1.*, u1.*, m1.*) so they match
resourcesPreset; locate the presets table in the README and replace legacy keys
with the corresponding new keys (e.g., nano -> s1.nano, micro -> s1.micro or the
correct s/u/m series) and ensure any references to those preset keys
(resourcesPreset, resources, resources.cpu, resources.memory) and example values
reflect the new naming.
In `@packages/apps/redis/README.md`:
- Around line 16-26: Update the preset reference for resourcesPreset to use the
new family naming scheme (s1/u1/m1) instead of the legacy flat names: replace
legacy preset names in the table and any examples with the new family-format
values (e.g., s1.nano, s1.micro, u1.small, m1.large as applicable) and ensure
the resourcesPreset row and any preset list below reference the new s1/u1/m1
family consistently so that the resourcesPreset documentation matches the rest
of the README.
In `@packages/apps/tcp-balancer/README.md`:
- Around line 15-22: Update the resourcesPreset documentation to use the new
s1/u1/m1 family names instead of legacy nano/micro/etc.: find the
resourcesPreset reference/table (look for the symbol "resourcesPreset" and the
legacy presets list) and replace the old preset names with the new mapping
(e.g., s1.nano -> s1.small-style name as per new scheme) or collapse the
duplicated table into a single authoritative reference that matches the
top-of-file example showing s1/u1/m1; ensure all occurrences of legacy preset
names in the README are updated to the new family names or point to the single
source-of-truth.
packages/apps/ferretdb/README.md
Outdated
| | Name | Description | Type | Value | | ||
| | ------------------ | ---------------------------------------------------------------------------------------------------------------------------------- | ---------- | --------- | | ||
| | `replicas` | Number of replicas. | `int` | `2` | | ||
| | `resources` | Explicit CPU and memory configuration for each FerretDB replica. When omitted, the preset defined in `resourcesPreset` is applied. | `object` | `{}` | | ||
| | `resources.cpu` | CPU available to each replica. | `quantity` | `""` | | ||
| | `resources.memory` | Memory (RAM) available to each replica. | `quantity` | `""` | | ||
| | `resourcesPreset` | Default sizing preset used when `resources` is omitted. | `string` | `u1.nano` | | ||
| | `size` | Persistent Volume Claim size available for application data. | `quantity` | `10Gi` | |
There was a problem hiding this comment.
Update the preset table to the new s1/u1/m1 naming.
The default is now u1.nano, but the preset list below still documents legacy names (nano, micro, …). Please update that table to list the new 24 presets (s1/u1/m1 × sizes) to avoid confusing users.
🤖 Prompt for AI Agents
In `@packages/apps/ferretdb/README.md` around lines 11 - 18, The presets table in
the README documents legacy names but resourcesPreset and the default value use
the new naming (resourcesPreset, default `u1.nano`); update the presets section
to replace legacy entries (e.g., `nano`, `micro`, ...) with the new 24 preset
names using the s1/u1/m1 families across all sizes (s1.xxx, u1.xxx, m1.xxx) so
the table lists every preset variant and matches the default `u1.nano` naming;
ensure the updated table entries reference the `resourcesPreset` field and any
examples reflect the new names.
cfb0935 to
01b61b4
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/apps/clickhouse/README.md (1)
91-99:⚠️ Potential issue | 🟡 MinorUpdate preset reference table to reflect current naming convention and values.
The table in the README currently lists legacy preset names (
nano,micro,small, etc.) without thes1.*,u1.*, andm1.*series prefixes that are actually used by the system. The actual resource presets inpackages/library/cozy-lib/templates/_resourcepresets.tpldefine three series with different CPU:memory ratios, but this documentation is not reflected in the README.Replace the single preset table with three separate tables documenting the s1 (1:2 ratio), u1 (1:4 ratio), and m1 (1:8 ratio) series with their correct CPU and memory values.
📝 Suggested replacement for the preset reference table
-| Preset name | CPU | memory | -|-------------|--------|---------| -| `nano` | `250m` | `128Mi` | -| `micro` | `500m` | `256Mi` | -| `small` | `1` | `512Mi` | -| `medium` | `1` | `1Gi` | -| `large` | `2` | `2Gi` | -| `xlarge` | `4` | `4Gi` | -| `2xlarge` | `8` | `8Gi` | +#### s1 (Standard) — 1:2 CPU:memory ratio + +| Preset name | CPU | memory | +|-------------|--------|---------| +| `s1.nano` | `250m` | `512Mi` | +| `s1.micro` | `500m` | `1Gi` | +| `s1.small` | `1` | `2Gi` | +| `s1.medium` | `2` | `4Gi` | +| `s1.large` | `4` | `8Gi` | +| `s1.xlarge` | `8` | `16Gi` | +| `s1.2xlarge`| `16` | `32Gi` | +| `s1.4xlarge`| `32` | `64Gi` | + +#### u1 (Universal) — 1:4 CPU:memory ratio + +| Preset name | CPU | memory | +|-------------|--------|---------| +| `u1.nano` | `250m` | `1Gi` | +| `u1.micro` | `500m` | `2Gi` | +| `u1.small` | `1` | `4Gi` | +| `u1.medium` | `2` | `8Gi` | +| `u1.large` | `4` | `16Gi` | +| `u1.xlarge` | `8` | `32Gi` | +| `u1.2xlarge`| `16` | `64Gi` | +| `u1.4xlarge`| `32` | `128Gi` | + +#### m1 (Memory) — 1:8 CPU:memory ratio + +| Preset name | CPU | memory | +|-------------|--------|---------| +| `m1.nano` | `250m` | `2Gi` | +| `m1.micro` | `500m` | `4Gi` | +| `m1.small` | `1` | `8Gi` | +| `m1.medium` | `2` | `16Gi` | +| `m1.large` | `4` | `32Gi` | +| `m1.xlarge` | `8` | `64Gi` | +| `m1.2xlarge`| `16` | `128Gi` | +| `m1.4xlarge`| `32` | `256Gi` |packages/apps/vpn/README.md (1)
58-67:⚠️ Potential issue | 🟡 MinorUpdate the preset reference table to match new naming and values.
The preset table still documents legacy names and values that no longer match the actual presets. For example,
nanoshows128Mimemory, buts1.nanoactually provides512Mi. This will mislead users about resource allocations.Consider updating to document the three series with their actual values, or link to centralized preset documentation.
Suggested table structure
-| Preset name | CPU | memory | -|-------------|--------|---------| -| `nano` | `250m` | `128Mi` | -| `micro` | `500m` | `256Mi` | -| `small` | `1` | `512Mi` | -| `medium` | `1` | `1Gi` | -| `large` | `2` | `2Gi` | -| `xlarge` | `4` | `4Gi` | -| `2xlarge` | `8` | `8Gi` | +| Preset name | CPU | memory | Ratio | +|--------------|--------|---------|-------| +| `s1.nano` | `250m` | `512Mi` | 1:2 | +| `s1.micro` | `500m` | `1Gi` | 1:2 | +| `s1.small` | `1` | `2Gi` | 1:2 | +| ... | ... | ... | ... |
🤖 Fix all issues with AI agents
In `@packages/apps/rabbitmq/values.yaml`:
- Around line 41-42: Change the RabbitMQ default resources preset from s1.nano
to u1.nano to align with other messaging services; update the value assigned to
the resourcesPreset key (the RabbitMQ chart's resourcesPreset) so it reads
"u1.nano" instead of "s1.nano" to match the u1 series used by NATS and other
messaging/datastore services.
🧹 Nitpick comments (1)
packages/apps/kafka/values.schema.json (1)
59-88: Consider upgrade guidance for removed legacy presets.These enums drop the old flat names (nano/micro/...), so existing resources using them will fail validation after upgrade. Please ensure a migration/alias strategy or release note covers this.
Also applies to: 193-222
matthieu-robin
force-pushed
the
resourcepreset
branch
from
b260d4f to
31da97d
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/apps/foundationdb/README.md (1)
47-55:⚠️ Potential issue | 🟠 MajorUpdate the example to reflect the current preset naming scheme.
The Resources example (lines 47–55) shows
resourcesPreset: "medium"with the old preset list (small, medium, large, xlarge, 2xlarge), but the parameter table correctly documents the new defaultu1.smalland the actual valid presets (e.g.,s1.small,u1.small,m1.small). This contradicts the documentation and would mislead users into using invalid preset values.Since this README is auto-generated by cozyvalues-gen from values.yaml, regenerate it to ensure the example reflects the current enum values. If the example code block comes from a manual source, update it directly to use a valid preset name like
u1.small.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/apps/foundationdb/README.md` around lines 47 - 55, The resources example uses the old preset names (e.g., "medium"); update the example to use the current preset enum (e.g., "u1.small") or regenerate the README from values.yaml via cozyvalues-gen so the code block matches the documented presets (s1.small, u1.small, m1.small); locate the YAML example block in packages/apps/foundationdb/README.md and either replace resourcesPreset: "medium" with resourcesPreset: "u1.small" or re-run the auto-generation to sync the example with the values.yaml enums.
♻️ Duplicate comments (1)
packages/apps/redis/README.md (1)
16-26:⚠️ Potential issue | 🟡 MinorRegenerate the preset reference table from the new preset source.
The common-parameters table now advertises
s1.nano, but the preset matrix later in this README still lists legacy unqualified keys (nano…2xlarge). Under the new<series>.<size>scheme those names are no longer valid, so the doc is now internally inconsistent and points users at values the chart/schema will reject. Based on learnings, readme files within packages that are auto-generated by cozyvalues-gen should not be edited manually; update the upstream source or generator config and regenerate.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/apps/redis/README.md` around lines 16 - 26, The README's common-parameters table advertises resourcesPreset `s1.nano` while the preset matrix still contains legacy keys like `nano` and `2xlarge`; update the upstream preset source or the cozyvalues-gen generator config to emit the new `<series>.<size>` keys (e.g., `s1.nano`) and then re-run cozyvalues-gen to regenerate the package README so the `resourcesPreset`/preset matrix are consistent; modify the preset source entries (or generator mapping) that produce legacy names and ensure the generator output replaces keys referenced in the README (look for references to resourcesPreset, the preset matrix, and `s1.nano`) before committing the regenerated file.
🧹 Nitpick comments (1)
packages/apps/clickhouse/values.yaml (1)
44-45: Consider usingm1series for ClickHouse analytics workloads.Per the PR objectives, the
m1(Memory, 1:8 CPU:memory ratio) series is designed for "memory-intensive workloads (search, analytics)." ClickHouse is an OLAP/analytics database that typically benefits from higher memory allocation for columnar operations and query caching.The current
u1.microdefault may work for small deployments, butm1.micromight better match ClickHouse's memory-intensive nature. Was this a deliberate choice to keep a conservative default?🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/apps/clickhouse/values.yaml` around lines 44 - 45, The default resourcesPreset value "u1.micro" is conservative for ClickHouse; change the resourcesPreset setting in values.yaml from "u1.micro" to the memory-optimized series "m1.micro" (update the resourcesPreset key) so ClickHouse deployments use the m1 (memory-optimized) sizing by default; if the conservative default was intentional, add a short comment above resourcesPreset explaining why you'd keep "u1.micro" and how to override to "m1.micro" for analytics workloads.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/apps/mariadb/README.md`:
- Around line 72-79: The README's presets reference is inconsistent: change the
legacy preset keys (nano, micro, etc.) in the generator/source that produces the
resourcesPreset reference so they match the new schema (examples like u1.nano
and the s1.*, u1.*, m1.* families), update any examples and the reference table
entry for resourcesPreset and presets list to use the new token names, then
regenerate the README via the cozyvalues-gen/source generator rather than
editing the rendered file directly so that resourcesPreset, resources, and any
examples all consistently show the u1.nano default and the new preset families.
---
Outside diff comments:
In `@packages/apps/foundationdb/README.md`:
- Around line 47-55: The resources example uses the old preset names (e.g.,
"medium"); update the example to use the current preset enum (e.g., "u1.small")
or regenerate the README from values.yaml via cozyvalues-gen so the code block
matches the documented presets (s1.small, u1.small, m1.small); locate the YAML
example block in packages/apps/foundationdb/README.md and either replace
resourcesPreset: "medium" with resourcesPreset: "u1.small" or re-run the
auto-generation to sync the example with the values.yaml enums.
---
Duplicate comments:
In `@packages/apps/redis/README.md`:
- Around line 16-26: The README's common-parameters table advertises
resourcesPreset `s1.nano` while the preset matrix still contains legacy keys
like `nano` and `2xlarge`; update the upstream preset source or the
cozyvalues-gen generator config to emit the new `<series>.<size>` keys (e.g.,
`s1.nano`) and then re-run cozyvalues-gen to regenerate the package README so
the `resourcesPreset`/preset matrix are consistent; modify the preset source
entries (or generator mapping) that produce legacy names and ensure the
generator output replaces keys referenced in the README (look for references to
resourcesPreset, the preset matrix, and `s1.nano`) before committing the
regenerated file.
---
Nitpick comments:
In `@packages/apps/clickhouse/values.yaml`:
- Around line 44-45: The default resourcesPreset value "u1.micro" is
conservative for ClickHouse; change the resourcesPreset setting in values.yaml
from "u1.micro" to the memory-optimized series "m1.micro" (update the
resourcesPreset key) so ClickHouse deployments use the m1 (memory-optimized)
sizing by default; if the conservative default was intentional, add a short
comment above resourcesPreset explaining why you'd keep "u1.micro" and how to
override to "m1.micro" for analytics workloads.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: faa1ed5a-99e6-4a7c-9a12-28fd03ff2b73
📒 Files selected for processing (39)
packages/apps/clickhouse/README.mdpackages/apps/clickhouse/values.schema.jsonpackages/apps/clickhouse/values.yamlpackages/apps/foundationdb/README.mdpackages/apps/foundationdb/values.schema.jsonpackages/apps/foundationdb/values.yamlpackages/apps/http-cache/README.mdpackages/apps/http-cache/values.schema.jsonpackages/apps/http-cache/values.yamlpackages/apps/kafka/README.mdpackages/apps/kafka/values.schema.jsonpackages/apps/kafka/values.yamlpackages/apps/kubernetes/README.mdpackages/apps/kubernetes/values.schema.jsonpackages/apps/kubernetes/values.yamlpackages/apps/mariadb/README.mdpackages/apps/mariadb/values.schema.jsonpackages/apps/mariadb/values.yamlpackages/apps/mongodb/README.mdpackages/apps/mongodb/values.schema.jsonpackages/apps/mongodb/values.yamlpackages/apps/nats/README.mdpackages/apps/nats/values.schema.jsonpackages/apps/nats/values.yamlpackages/apps/postgres/README.mdpackages/apps/postgres/values.schema.jsonpackages/apps/postgres/values.yamlpackages/apps/rabbitmq/README.mdpackages/apps/rabbitmq/values.schema.jsonpackages/apps/rabbitmq/values.yamlpackages/apps/redis/README.mdpackages/apps/redis/values.schema.jsonpackages/apps/redis/values.yamlpackages/apps/tcp-balancer/README.mdpackages/apps/tcp-balancer/values.schema.jsonpackages/apps/tcp-balancer/values.yamlpackages/apps/vpn/README.mdpackages/apps/vpn/values.schema.jsonpackages/apps/vpn/values.yaml
🚧 Files skipped from review as they are similar to previous changes (16)
- packages/apps/kubernetes/values.schema.json
- packages/apps/mongodb/README.md
- packages/apps/http-cache/README.md
- packages/apps/postgres/values.schema.json
- packages/apps/tcp-balancer/README.md
- packages/apps/kubernetes/README.md
- packages/apps/postgres/README.md
- packages/apps/tcp-balancer/values.yaml
- packages/apps/mongodb/values.yaml
- packages/apps/clickhouse/README.md
- packages/apps/vpn/README.md
- packages/apps/rabbitmq/README.md
- packages/apps/kafka/README.md
- packages/apps/nats/README.md
- packages/apps/kafka/values.schema.json
- packages/apps/redis/values.yaml
kvaps
left a comment
kvaps
left a comment
There was a problem hiding this comment.
Thanks for working on this!
A couple of things before this can be merged:
1. Rebase onto main
The branch is behind main. Since then, OpenSearch was added (#1953), which also uses resourcesPreset — it'll need the same preset rename treatment.
2. Migration script is required
Existing deployments have HelmReleases with old preset names (nano, small, micro, etc.). After upgrade, the new JSON schemas will reject these values and helm rendering will break.
You need to add a migration script at packages/core/platform/images/migrations/migrations/36 that:
- Finds all HelmReleases across all namespaces
- Maps old
resourcesPresetvalues to new ones (e.g.small→u1.small,micro→u1.micro) - Patches each HelmRelease with
kubectl patch - Stamps the new version in the
cozystack-versionConfigMap
See migration 24 (MongoDB users format conversion) as a good reference — it follows the same pattern of iterating HelmReleases and transforming values with jq.
Also bump targetVersion in packages/core/platform/values.yaml from 36 to 37.
Replace flat resource presets (nano, micro, small...) with an instance-type naming convention: <series>.<size>. Three series with different CPU:memory ratios: s1 (Standard, 1:2), u1 (Universal, 1:4), m1 (Memory, 1:8). Fix cozy-lib.resources.preset passing a list instead of a string to unsanitizedPreset. Use u1 series for databases and messaging systems (MySQL, NATS, SeaweedFS db) for consistency with postgres, mongodb, kafka. Co-Authored-By: Claude Opus 4.5 <[email protected]> Signed-off-by: Matthieu <[email protected]>
Signed-off-by: Matthieu Robin <[email protected]> Co-Authored-By: Claude Opus 4.5 <[email protected]> Signed-off-by: Matthieu <[email protected]>
- Change RabbitMQ default preset from s1.nano to u1.nano (messaging belongs to Universal series) - Sort preset keys in error message for deterministic output - Fix invalid Go template syntax in usage example comment Signed-off-by: Matthieu <[email protected]> Co-Authored-By: Claude Opus 4.5 <[email protected]>
…ration script Extend s1/u1/m1 resource presets to all remaining packages that were still using old flat names: harbor, openbao, qdrant, external-dns, opensearch. Also fix redis default from s1.nano to m1.nano to match its memory-intensive workload profile. Add migration script (36→37) that renames resourcesPreset values in existing HelmReleases, selecting the appropriate series per app type: m1 for memory-intensive (redis, opensearch, qdrant), s1 for stateless services (harbor, vpn, openbao, etc.), u1 for databases and messaging. Update all schemas, cozyrds, READMEs, e2e tests, and opensearch unit tests to use the new naming convention consistently. Remove unused $cpuAllocationRatio variable from cozy-lib preset helper. Signed-off-by: Matthieu Robin <[email protected]> Signed-off-by: Matthieu <[email protected]>
matthieu-robin
force-pushed
the
resourcepreset
branch
from
d598320 to
9e27184
Compare
Run make generate on all packages modified in the previous commit to ensure generated files match the output of cozyvalues-gen exactly. Signed-off-by: Matthieu Robin <[email protected]> Signed-off-by: Matthieu <[email protected]>
Previous run failed due to Keycloak OIDC timeout (flaky infra). Signed-off-by: Matthieu Robin <[email protected]> Signed-off-by: Matthieu <[email protected]>
2 participants
Summary
nano,micro,small...) with an instance-type naming convention inspired by KubeVirt/cloud providers:<series>.<size>nano,micro,small,medium,large,xlarge,2xlarge,4xlarge(24 presets total)Test plan
helm templaterenders correctly for each app with default presetresourcesoverride still takes priority over presetmake unit-teststo validate existing Helm unit tests🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
New Features
Changes