Repository navigation
[cozy-lib] Introduce instance-type resource presets (s1/u1/m1) - #1959
Matthieu ROBIN (matthieu-robin) wants to merge 11 commits into
Conversation
dosubot
Bot
added
size/XL
Summary of ChangesHello Matthieu ROBIN (@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 (@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)
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.
| ## @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"| ## @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"| 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
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"
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.
| | 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
b260d4f to
31da97d
Compare
31da97d to
d598320
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
Andrei Kvapil (kvaps)
left a comment
Andrei Kvapil (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.
d598320 to
9e27184
Compare
|
Hi, Matthieu ROBIN (@matthieu-robin)! I'm here to merge this finally. Could you please update this PR? |
dosubot
Bot
added
the
size/L
github-actions
Bot
added
the
area/platform
Aleksei Sviridkin (lexfrei)
left a comment
Aleksei Sviridkin (lexfrei)
left a comment
There was a problem hiding this comment.
The PR is a clean rename overall — _resourcepresets.tpl is sound, the 24-preset enum is consistent across all 17 api/apps/v1alpha1/*/types.go files, e2e suites are updated, and CodeRabbit's earlier asks (deterministic error message, list parens) and the maintainer's three March blockers (rebase, migration, opensearch) are addressed.
One blocker on the migration script: two of its case-statement prefixes are dead code (inline). Plus two non-blocking inline notes on uniform-series application across nested fields and a stale README table.
This is also a breaking schema change worth flagging in the PR description: the new dynamic CRD enum drops legacy nano…2xlarge values, so any user-applied kind: <App> manifest with a flat preset name will be rejected after upgrade. The migration only rewrites HelmReleases — it does not touch user-managed CR manifests / GitOps repos / Argo apps.
Resource budget impact (non-blocking, project-wide)
The migration produces sharp memory upsizes for existing deployments because the new s1/u1/m1 ratios decouple memory from the size-name. Old preset memory was roughly 1:1 to CPU; new u1.* is 1:4 and m1.* is 1:8. So when a user-set resourcesPreset: small is rewritten to u1.small or m1.small:
| Old preset (~mem) | After u1 migration | After m1 migration |
|---|---|---|
| nano (128Mi) | u1.nano (1Gi) | m1.nano (2Gi) |
| small (512Mi) | u1.small (4Gi) | m1.small (8Gi) |
| large (2Gi) | u1.large (16Gi) | m1.large (32Gi) |
That is 8x-16x memory across the cluster on platform upgrade. Defensible per the PR's stated philosophy (m1 for memory-intensive workloads), but unannounced — operators with tight node capacity, ResourceQuotas, or cost ceilings may see scheduling failures or surprise spend. Worth pre-flagging in the PR body and the eventual release note so they can right-size before the migration runs.
| case "$name" in | ||
| redis-*|opensearch-*|qdrant-*) | ||
| echo "m1" ;; | ||
| harbor-*|vpn-*|tcp-balancer-*|http-cache-*|openbao-*|ingress-*|external-dns-*) |
There was a problem hiding this comment.
The case patterns ingress-* and external-dns-* here are dead code. Both Applications set release.prefix: "" in their cozyrds (packages/system/ingress-rd/cozyrds/ingress.yaml:13, packages/system/external-dns-rd/cozyrds/external-dns.yaml:13), and pkg/registry/apps/application/rest.go:237 builds the HelmRelease name as releaseConfig.Prefix + name. With prefix empty, the HR name is whatever the user named the CR — default, main, edge, etc. — none of which match ingress-* or external-dns-*. Users with explicit resourcesPreset overrides on these apps fall through to the u1 default branch and end up with u1.<size> (e.g. 2Gi for u1.micro), whereas the chart's new default and the apparent intent here is s1.<size> (1Gi for s1.micro).
Suggested fix: stop matching by HR name regex and key off the kind label that cozystack-api already sets on the HR. The label-based approach is also more durable against future renames — the prefix list here duplicates state that already lives in each ApplicationDefinition's release.prefix.
A quick sanity check: of the patterns in this case (redis-*|opensearch-*|qdrant-*|harbor-*|vpn-*|tcp-balancer-*|http-cache-*|openbao-*|ingress-*|external-dns-*), the first eight match because their cozyrds set the matching release.prefix. The last two never match.
| end; | ||
|
|
||
| walk( | ||
| if type == "object" and has("resourcesPreset") and (.resourcesPreset | type) == "string" then |
There was a problem hiding this comment.
Observation (non-blocking): the walk(...) correctly visits every nested resourcesPreset, but the SAME series is applied uniformly across the whole values tree. For apps whose new chart defaults mix series per sub-component, this drifts users with explicit nested overrides:
- clickhouse: top
u1.micro+clickhouseKeeper: s1.microinpackages/apps/clickhouse/values.yaml. Migration falls tou1default forclickhouse-*, so an explicitclickhouseKeeper.resourcesPreset: microbecomesu1.micro(2Gi) vs the chart's news1.micro(1Gi). - kafka: top
u1.micro+zookeeper: s1.small. Same drift pattern. - kubernetes: mixed
u1.small(apiServer) ands1.micro(controllerManager, scheduler, konnectivity); migration uniformly usesu1. - opensearch: top
m1.large+dashboards: s1.mediuminpackages/apps/opensearch/values.yaml. Migration appliesm1, so an explicitdashboards.resourcesPreset: mediumbecomesm1.medium(16Gi) vs chart defaults1.medium(4Gi).
This affects only users who set resourcesPreset on nested fields explicitly — not the common case where the chart default applies — but it silently oversizes sub-components. Switching to label-driven selection (see other comment) plus consulting the chart's per-field default for fields the user did not override would resolve this.
| | `replicas` | Number of RabbitMQ replicas. | `int` | `3` | | ||
| | `resources` | Explicit CPU and memory configuration for each RabbitMQ 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` | `""` | |
There was a problem hiding this comment.
Observation (non-blocking): the preset reference table later in this README (around line 57, outside the diff window) still shows the legacy 7-row layout (nano | 100m | 128Mi, etc.); every other regenerated README in this PR (postgres, redis, mariadb, kafka, …) shows the new 24-row s1/u1/m1 table. The make generate / cozyvalues-gen pass appears to have skipped this package — re-run it for packages/apps/rabbitmq and replace the table.
Side note: the values shown there for the legacy presets (nano | 100m | 128Mi) don't match what the old _resourcepresets.tpl actually defined (nano | 250m | 128Mi). The README was drifted from reality before this PR — worth aligning while you regenerate.
|
Thanks for the careful review Aleksei Sviridkin (@lexfrei)! All three points addressed in Blocker —
|
Aleksei Sviridkin (lexfrei)
left a comment
Aleksei Sviridkin (lexfrei)
left a comment
There was a problem hiding this comment.
Prior blocker (migration script using dead HR-name patterns) is resolved in c3ac843 — verified the apps.cozystack.io/application.kind label is set on every cozystack-api-managed HelmRelease (pkg/registry/apps/application/rest.go:199, 531, 1439), and HRs without the label safely fall through to the u1 default. The uniform-series limitation is honestly documented in the migration comment. Rabbitmq README is regenerated in 115e8eb. LGTM. Worth expanding the release-note block at release-cut time to flag the upsize on existing deployments (u1 1:4 / m1 1:8 vs old ~1:1).
There was a problem hiding this comment.
NOT LGTM (same blocker as last round) — mergeable_state is still dirty. Recent commits show the author rebased and regenerated schemas/CRDs/types, but main has moved further since that rebase, so conflicts are back. Please rebase onto current main once more and force-push; I'll re-check the diff after that.
The PR-internal work continues to look clean (10 commits, 117 files, ~+2435/-1020 across the s1/u1/m1 preset rollout); this is purely a branch-hygiene blocker.
115e8eb to
6ea24d5
Compare
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]>
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]>
…ename - harbor portal: "nano" → "s1.nano" (was hardcoded fallback) - opensearch dashboards: default fallback "medium" → "s1.medium" (matches values.yaml) - keycloak STS: "small" → "s1.small" (only existing direct cozy-lib caller) - cozy-lib _resources.tpl docstring: update example to use "s1.nano" preset Without these the new ResourcesPreset enum rejects the legacy keys at render time. Signed-off-by: mattia-eleuteri <[email protected]>
Re-runs cozyvalues-gen + update-crd.sh across all apps/extra packages that ship a resourcesPreset enum. Picks up the new s1/u1/m1 enum and default values together with the unrelated upstream additions (seaweedfs.host, kubernetes.images.waitForKubeconfig, etc.). Signed-off-by: mattia-eleuteri <[email protected]>
Match HelmReleases by the apps.cozystack.io/application.kind label that cozystack-api sets on every HR it creates, instead of regexing the HR name. The previous patterns 'ingress-*' and 'external-dns-*' were dead code: both ApplicationDefinitions set release.prefix="", so HRs are named after the user-chosen CR (e.g. 'default'), which never matched, and explicit resourcesPreset overrides on those apps fell through to the u1 default instead of s1. Document the known limitation that a single series is applied uniformly across nested resourcesPreset fields — affects only users with explicit nested overrides on apps with mixed-series chart defaults (ClickHouse, Kafka, OpenSearch, Kubernetes). Signed-off-by: Matthieu <[email protected]>
Replace the legacy 7-row preset table with the new 24-row s1/u1/m1 layout produced for the other apps in this PR (postgres, redis, mariadb, kafka, …). The cozyvalues-gen pass had skipped this README. Signed-off-by: Matthieu <[email protected]>
6ea24d5 to
53d7fbd
Compare
Aleksei Sviridkin (lexfrei)
left a comment
Aleksei Sviridkin (lexfrei)
left a comment
There was a problem hiding this comment.
LGTM — the merge-conflict blocker from my last round is gone (mergeable: true, force-push at 53d7fbd). The rebase brought in slightly more files (123 vs 117) due to regenerated CRDs/schemas/Go types, which is expected. E2E, Build, pre-commit, and the codegen-drift gate are all green.
Substantively this is the same s1/u1/m1 instance-type preset rollout I LGTM'd on 115e8eb before the branch went dirty — no new logic introduced by the rebase, just the regenerated artifacts catching up to current main.
The cluster.yaml template tests in packages/apps/kubernetes/tests/values/common.yaml still passed the legacy preset names (`large`, `micro`) to controlPlane.{apiServer,controllerManager,scheduler,konnectivity.server}.resourcesPreset. After the s1/u1/m1 schema landed in this PR, those names are no longer in the enum, so helm-unittest fails with schema-validation errors before any test assertion runs.
Update the fixture to mirror the new chart defaults (apiServer: u1.small, others: s1.micro). The tests don't depend on specific preset values — they exercise disk/storage/branding wiring on the worker side — so this is a pure rename to keep the fixtures schema-valid.
Reproduced locally: `cd packages/apps/kubernetes && helm unittest .` now passes 46/46 instead of failing on cluster_test.yaml.
Assisted-By: Claude <[email protected]>
Signed-off-by: Aleksei Sviridkin <[email protected]>
Aleksei Sviridkin (lexfrei)
left a comment
Aleksei Sviridkin (lexfrei)
left a comment
There was a problem hiding this comment.
Reverting my prior LGTM. The repeated E2E failures on this branch are not flakes — they are the symptom of a behaviour change this PR makes silently, and the same change will hit production users on upgrade.
Concrete CI evidence (run 25159603079, attempts 1 and 4, 13h apart, identical signature):
Status: Failed
Reason: OutOfmemory
Message: Pod was rejected: Node didn't have enough resource: memory, requested: 4429185024, used: 21829788946, capacity: 24533819392
Five tests fail at the same stage in both attempts: foundationdb, mongodb, redis, kubernetes-latest, kubernetes-previous, plus vminstance cumulatively. All workloads create themselves successfully but pods are then OOM-rejected by the scheduler. Concurrent runs on main and other PRs are green — the e2e infrastructure is fine, this branch is not.
Why it happens — the new presets are not behaviour-equivalent to the old ones they replace:
| old size | old mem | s1.* | u1.* | m1.* |
|---|---|---|---|---|
| nano | 128Mi | 512Mi (4×) | 1Gi (8×) | 2Gi (16×) |
| small | 512Mi | 2Gi (4×) | 4Gi (8×) | 8Gi (16×) |
| medium | 1Gi | 4Gi (4×) | 8Gi (8×) | 16Gi (16×) |
| large | 2Gi | 8Gi (4×) | 16Gi (8×) | 32Gi (16×) |
CPU also moves at medium and above: medium 1→2, large 2→4, xlarge 4→8, 2xlarge 8→16. With memoryAllocationRatio: 1 the memory request equals the limit, so every multiplier above is paid by the scheduler at request time, not just at OOM time.
Per-app default change (chart's own values.yaml, not user input):
| app | mem | CPU |
|---|---|---|
| redis: nano → m1.nano | 16× | same |
| qdrant: small → m1.small | 16× | same |
| opensearch: large → m1.large | 16× | 2× |
| mariadb / nats / rabbitmq: nano → u1.nano | 8× | same |
| foundationdb: medium → u1.small | 4× | same |
| mongodb: small → u1.micro | 4× | 0.5× ↓ |
| postgres: micro → u1.nano | 4× | 0.5× ↓ |
| clickhouse: small → u1.micro | 4× | 0.5× ↓ |
| openbao / vpn / tcp-balancer / ingress / external-dns | 4× | same |
Three of these (mongodb, postgres, clickhouse) silently halve CPU on the chart-default path even as they 4× memory. That is a separate decision worth making consciously, not as a side effect of the series choice.
On upgrade — migration 39 walks every cozystack-managed HelmRelease and rewrites resourcesPreset according to the apps.cozystack.io/application.kind label. This runs automatically, with no dry-run, no per-HR diff log, no node-capacity check, no opt-out. Operators with tight node capacity, ResourceQuotas, or cost ceilings will hit one of:
- pods stuck
Pending(scheduler can't fit the new request), - ResourceQuota rejection at HelmRelease reconcile,
- autoscaler scale-out → unannounced cost increase,
- on cloud nodes that can't grow: outage.
On user-managed CRs — the enum for resourcesPreset in values.schema.json drops the legacy nano/small/medium/etc values entirely. Any GitOps-managed kind: Postgres / kind: Redis / etc with a flat preset name will be rejected by API validation post-upgrade. Migration 39 only rewrites HRs — it does not touch the user's source manifests. Flux/ArgoCD reconcile loops will fight back against the migration.
The PR description does not communicate any of this. The title carries no !. The body says "Updated default presets in all app/extra packages to use the new naming" — phrased as a rename. The test plan covers helm template rendering, override priority, unit tests, and the invalid-key error message — none of these test upgrade impact, schema compatibility, or capacity. There are no release notes, no upgrade guide, no warning. CodeRabbit's auto-summary line "to optimize performance and resource utilization" does not help.
I flagged a softer version of this in my 28 Apr review (the "resource budget impact, non-blocking" table) and downgraded to LGTM. The CI failures since then have moved this from "may surprise users" to "is currently breaking the e2e cluster, will break tight production clusters on upgrade".
To unblock, in priority order:
- Make the rename behaviour-preserving by default. Map old preset names to the new namespace at their old sizes (so a HelmRelease that had
resourcesPreset: smallends up ats1.small = 1 CPU / 512Miafter migration, notu1.small = 1 CPU / 4Gi). The new s1 series is closest to the old shapes already; aligning the size column makes the rename a no-op for resource budget. Larger u1 / m1 sizes remain available as additive choices. - OR explicitly mark this as breaking. Add
!to the title, replace the "Updated default presets" line with a prominent breaking-change block, audit each per-app default change for intentional vs accidental (call out the three CPU halvings), write an upgrade guide that includes a node-capacity check, and add an upgrade smoke test (install on a tight cluster, run migration, verify scheduling). - Make migration 39 emit a per-HR before/after log and consider gating it behind a cozystack-config flag so admins run it after a capacity audit.
- Address the CR-level schema-incompat case explicitly — either keep the old enum values as deprecated aliases that map to the new ones at chart-render time, or document the GitOps-repo update path clearly in the upgrade guide.
## What this PR does Closes #1959. Continuation of @matthieu-robin's instance-type resource presets work, addressing the migration gaps and resource regressions surfaced during the review of #1959. Resource presets now follow a cloud-style `<series>.<size>` naming convention. Five series cover the full CPU-to-memory ratio range, each with eight sizes (`nano` through `4xlarge`): | Series | Ratio | Use case | | ------ | ----- | -------- | | `t1` | 1:0.5 | Tiny / burstable, low memory | | `c1` | 1:1 | Compute-balanced | | `s1` | 1:2 | Standard — proxies, caches | | `u1` | 1:4 | Universal — databases, messaging | | `m1` | 1:8 | Memory — search, analytics | 40 instance-type presets in total. The seven legacy flat names (`nano`, `micro`, `small`, `medium`, `large`, `xlarge`, `2xlarge`) remain accepted as deprecated aliases. ### Upgrade path The combination of three mechanisms makes the rename a no-op for existing deployments: 1. **`cozy-lib` legacy aliases** — preserve the exact pre-rename CPU and memory for every legacy name. `helm template ... --set resourcesPreset=small` continues to render `1 CPU / 512Mi`, identical to before. No HelmRelease that already lives on the cluster breaks. 2. **Migration 39** — a pre-upgrade hook walks every `HelmRelease.spec.values` and every app CR under `apps.cozystack.io/v1alpha1`, rewriting legacy values to their 1:1 instance-type equivalents (`nano → t1.nano`, `medium → c1.small`, etc). Idempotent, best-effort, dry-runnable via `MIGRATION_DRY_RUN=1`, distinguishes "no items" from "kubectl errored" so a broken cluster cannot silently stamp the version. 3. **`cozystack-api` deprecation warning** — every `Create` and `Update` on an app CR with a legacy value emits a `klog` warning naming the JSON path and the 1:1 replacement, so operators discover any remaining legacy data after the platform upgrade. The legacy-to-instance-type mapping is 1:1 by CPU and memory: | Legacy | CPU | Memory | Instance-type | | -------- | ---- | ------ | ------------- | | `nano` | 250m | 128Mi | `t1.nano` | | `micro` | 500m | 256Mi | `t1.micro` | | `small` | 1 | 512Mi | `t1.small` | | `medium` | 1 | 1Gi | `c1.small` | | `large` | 2 | 2Gi | `c1.medium` | | `xlarge` | 4 | 4Gi | `c1.large` | | `2xlarge`| 8 | 8Gi | `c1.xlarge` | Mirrored in four places (cozy-lib template, Go package, migration jq, docs) with cross-reference comments so drift is visible. ### Why not just #1959 The implementation in #1959 had three blockers that motivated this rewrite: - **`cozy-lib` lost the legacy names entirely**, so any existing HelmRelease with `resourcesPreset: small` failed render with `Preset key 'small' invalid`. Aliases close this hole. - **Migration only touched HelmReleases**, leaving app CRs unmigrated; the next `kubectl edit` would fail enum validation. The new migration walks both. - **Defaults silently inflated memory**, e.g. Redis `nano → m1.nano` jumped from 128Mi to 2Gi (×16). With two replicas plus sentinel the e2e VM ran out of memory, producing the Redis 90s timeout that blocked the original PR. New defaults preserve resources 1:1. ### Deprecation timeline Legacy aliases are scheduled for removal in a future cozystack release. The combination of migration + operator warnings is expected to converge production clusters before then. Removal will land as a separate PR with its own migration cleanup pass. ### Test plan - `make unit-tests` — Go tests for the preset walker + warning helper, `helm unittest` legacy↔instance-type parity suites for `postgres`/`clickhouse`/`opensearch`, RD schema invariance check via `hack/check-rd-presets.sh`. - `make generate` is idempotent. - `helm template ./packages/apps/redis` with the new default renders 250m / 128Mi (matches pre-rename Redis exactly — closes the e2e blocker). - `helm template ./packages/apps/postgres --set resourcesPreset=small` renders 1 / 512Mi (legacy alias works). - Migration dry-run is exercised via `MIGRATION_DRY_RUN=1`. ### Documentation See [`docs/operations/resource-presets.md`](docs/operations/resource-presets.md) for the full size matrix and migration table. ### Release note ```release-note feat(cozy-lib): introduce `<series>.<size>` instance-type resource presets (t1/c1/s1/u1/m1 × 8 sizes). Legacy flat names (nano…2xlarge) remain accepted as deprecated 1:1 aliases; migration 39 rewrites stored values to the new form. CPU and memory are preserved exactly across the rename. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Expanded resource preset options to instance-style names (e.g., t1.*, c1.*, m1.*); legacy flat names remain supported as deprecated aliases. * Deprecation warnings now surface when legacy presets are detected. * **Documentation** * Added a comprehensive resource-presets guide and updated chart/app docs to the new naming convention. * **Tests** * Added compatibility tests ensuring legacy ↔ instance-type presets render identical resources. * **Chores** * Migration and CI checks to detect/upgrade legacy preset values. [](https://app.coderabbit.ai/change-stack/cozystack/cozystack/pull/2607) <!-- end of auto-generated comment: release notes by coderabbit.ai -->
%5D(https%3A%2F%2Fapp.coderabbit.ai%2Fchange-stack%2Fcozystack%2Fcozystack%2Fpull%2F2607){kind=link}
4 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