feature/add-presets - #695
Conversation
WalkthroughThis pull request adds resource management options to multiple Helm charts. New parameters, Changes
Sequence Diagram(s)sequenceDiagram
participant U as User Values File
participant T as Template Engine
participant P as resources.preset Template
U->>T: Provide values (resources & resourcesPreset)
T->>T: Check if `.Values.resources` is defined
alt resources provided
T->>T: Render resources using provided values
else resources not provided
T->>T: Check if `.Values.resourcesPreset` ≠ "none"
alt Preset condition met
T->>P: Call preset template to generate resource config
P-->>T: Return YAML snippet for resources
else
T->>T: Omit resources configuration
end
end
T-->>Final: Merge resource configuration into final YAML output
Suggested reviewers
Poem
Tip ⚡🧪 Multi-step agentic review comment chat (experimental)
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (27)
packages/system/kubeovn/values.yaml (1)
21-21: Registry Address Update Consistency CheckThe registry address is now updated to
ghcr.io/cozystack/cozystackas intended in the PR. Please verify that this new address is consistent with your image repository configuration and that any related documentation or references are updated accordingly.packages/core/installer/Makefile (1)
39-40: Update IMAGE variable to reflect installer image naming
The updated assignment on line 39 now uses the "installer" tag instead of "cozystack", which aligns with the new naming convention for the built image. However, the subsequent YAML update on line 40 still writes to the key.cozystack.image. Please verify whether this is intentional or if the YAML key should also be updated (e.g., to.installer.image) to maintain consistency across configurations.packages/apps/redis/templates/_resources.tpl (1)
1-50: Well-structured resource presets templateThis template provides a comprehensive set of resource presets with good error handling and clear documentation.
Consider the following refinements:
CPU requests for xlarge and 2xlarge are the same as large (1.0), while limits increase significantly. This might cause scheduling inefficiencies.
The comment on line 14 states "limits are the requests increased by 50%", but for xlarge/2xlarge, CPU limits are 3x and 6x the requests.
"xlarge" (dict - "requests" (dict "cpu" "1.0" "memory" "3072Mi" "ephemeral-storage" "50Mi") + "requests" (dict "cpu" "2.0" "memory" "3072Mi" "ephemeral-storage" "50Mi") "limits" (dict "cpu" "3.0" "memory" "6144Mi" "ephemeral-storage" "2Gi") ) "2xlarge" (dict - "requests" (dict "cpu" "1.0" "memory" "3072Mi" "ephemeral-storage" "50Mi") + "requests" (dict "cpu" "4.0" "memory" "3072Mi" "ephemeral-storage" "50Mi") "limits" (dict "cpu" "6.0" "memory" "12288Mi" "ephemeral-storage" "2Gi") )packages/apps/ferretdb/templates/_resources.tpl (1)
1-50: Template appears to be duplicated across applicationsThis template is identical to the one in
packages/apps/rabbitmq/templates/_resources.tpl. While having application-specific templates gives flexibility, it introduces maintenance overhead if preset definitions need to change in the future.Consider whether it might be better to have a shared template in a common location, or use a configuration management approach to keep these templates in sync.
packages/apps/kafka/templates/_resources.tpl (1)
36-43: Consider adjusting CPU requests for larger presetsFor xlarge and 2xlarge presets, the CPU request remains at 1.0 while the limits are significantly higher (3.0 and 6.0). This large gap between requested and limit CPU could potentially lead to CPU throttling during peak usage, as the container might try to use more resources than what was requested from the scheduler.
Consider adjusting the CPU requests to better match the expected usage pattern for these larger workloads.
packages/apps/ferretdb/values.schema.json (1)
85-94: Consider adding enum validation for resourcesPresetThe schema properties are well-defined with good descriptions. However, consider adding an "enum" property to validate the resourcesPreset values at the schema level, which would provide better validation and documentation:
"resourcesPreset": { "type": "string", "description": "Set container resources according to one common preset (allowed values: none, nano, micro, small, medium, large, xlarge, 2xlarge). This is ignored if resources is set (resources is recommended for production).", "default": "nano", + "enum": ["none", "nano", "micro", "small", "medium", "large", "xlarge", "2xlarge"] }packages/apps/clickhouse/templates/_resources.tpl (1)
1-50: Consider a common library approach for resource presetsThe implementation is consistent with other applications, which is excellent. However, since this exact template appears in multiple applications (kafka, postgres, etc.), consider whether these resource presets could be refactored into a common library template that all applications reference. This would reduce duplication and make maintenance easier when changes to the presets are needed.
The same consideration about CPU requests for xlarge and 2xlarge presets applies here as well.
packages/apps/ferretdb/README.md (1)
24-35: Refine Resource Preset Description GrammarThe new table rows introducing the
resourcesandresourcesPresetparameters look good overall. However, the description forresourcesPresetcontains a minor grammatical inconsistency. Consider revising the text from:"... This is ignored if resources is set (resources is recommended for production)."
to something like:
"... This is ignored if
resourcesis specified (usingresourcesis recommended for production)."This change will improve clarity and ensure proper subject–verb agreement.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~35-~35: This verb does not appear to agree with the subject. Consider using a different form.
Context: ... 2xlarge). This is ignored if resources is set (resources is recommended for produ...(AI_EN_LECTOR_REPLACEMENT_VERB_AGREEMENT)
[uncategorized] ~35-~35: This verb does not appear to agree with the subject. Consider using a different form.
Context: ... ignored if resources is set (resources is recommended for production). |nano...(AI_EN_LECTOR_REPLACEMENT_VERB_AGREEMENT)
packages/apps/rabbitmq/values.yaml (2)
52-52: Remove Trailing SpacesThere appear to be trailing spaces on line 52. Please remove these to comply with YAML linting standards.
🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 52-52: trailing spaces
(trailing-spaces)
42-54: Improve Resource Preset DescriptionThe introduction of the
resourcesandresourcesPresetparameters is well integrated into the documentation. However, similar to the FerretDB README, the description forresourcesPresetwould benefit from a slight rewording for clarity. Consider changing:"... This is ignored if resources is set (resources is recommended for production)."
to
"... This is ignored if
resourcesis specified (usingresourcesis recommended for production)."🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 52-52: trailing spaces
(trailing-spaces)
packages/apps/redis/values.yaml (1)
15-26: Resource configuration parameters look good.The addition of resource management options with both explicit configuration and preset-based options provides flexibility for different deployment scenarios.
There's a trailing space at the end of line 24 that should be removed:
# cpu: 100m # memory: 512Mi - +🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 24-24: trailing spaces
(trailing-spaces)
packages/apps/kafka/values.yaml (1)
43-54: Resource configuration parameters look good.The addition of resource management options with both explicit configuration and preset-based options provides flexibility for different deployment scenarios.
There's a trailing space at the end of line 52 that should be removed:
# cpu: 100m # memory: 512Mi - +🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 52-52: trailing spaces
(trailing-spaces)
packages/apps/clickhouse/values.yaml (2)
49-58: Resource Parameters Documentation in ClickHouse Values
The new block introducing theresourcesparameter—with its empty object default and commented-out examples for limits and requests—is clear and useful for guiding users on how to override defaults when needed.
59-59: Trailing Whitespace Detected
Static analysis has flagged trailing whitespace on this line. Please remove the extra spaces to maintain YAML style compliance.🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 59-59: trailing spaces
(trailing-spaces)
packages/apps/postgres/values.yaml (2)
79-88: Resource Parameters Documentation for Postgres
The addition of theresourcesparameter along with the commented examples gives users a clear starting point for configuring resource limits and requests. This mirrors the approach taken in other charts, ensuring consistency across the repository.
89-89: Trailing Whitespace Detected
Static analysis has detected trailing whitespace on this line. Please remove the extra spaces to conform with YAML linting standards.🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 89-89: trailing spaces
(trailing-spaces)
packages/apps/nats/README.md (1)
7-19: Updated Documentation with New Resource Parameters
The README table now includes rows forresourcesandresourcesPreset, each with clear descriptions and default values. Note that the explanation forresourcesPreset(e.g., "This is ignored if resources is set (resources is recommended for production)") could be rephrased for improved grammatical clarity. Consider revising the verb agreement to enhance readability.🧰 Tools
🪛 LanguageTool
[uncategorized] ~18-~18: This verb does not appear to agree with the subject. Consider using a different form.
Context: ... 2xlarge). This is ignored if resources is set (resources is recommended for produ...(AI_EN_LECTOR_REPLACEMENT_VERB_AGREEMENT)
[uncategorized] ~18-~18: This verb does not appear to agree with the subject. Consider using a different form.
Context: ... ignored if resources is set (resources is recommended for production). |nano...(AI_EN_LECTOR_REPLACEMENT_VERB_AGREEMENT)
packages/apps/mysql/values.yaml (2)
57-66: Resource Parameters Documentation for MySQL
The newresourcessection—including the empty object default and commented examples—is consistent with the changes made in other services. This clear documentation will help users understand how to override resource defaults when needed.
67-67: Trailing Whitespace Detected
Static analysis has flagged trailing whitespace on this line. Removing the extra spaces will help maintain a clean YAML style.🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 67-67: trailing spaces
(trailing-spaces)
packages/apps/clickhouse/README.md (1)
39-50: Clarify Resource Parameter DescriptionsThe new
resourcesandresourcesPresetentries enhance configuration flexibility. To improve clarity and grammatical correctness, consider rephrasing theresourcesPresettext. For example, modify the phrase from:This is ignored if resources is set (resources is recommended for production).to something like:
- This is ignored if resources is set (resources is recommended for production). + This parameter is ignored if `resources` is explicitly specified (using `resources` is recommended for production).🧰 Tools
🪛 LanguageTool
[uncategorized] ~50-~50: This verb does not appear to agree with the subject. Consider using a different form.
Context: ... 2xlarge). This is ignored if resources is set (resources is recommended for produ...(AI_EN_LECTOR_REPLACEMENT_VERB_AGREEMENT)
[uncategorized] ~50-~50: This verb does not appear to agree with the subject. Consider using a different form.
Context: ... ignored if resources is set (resources is recommended for production). |nano...(AI_EN_LECTOR_REPLACEMENT_VERB_AGREEMENT)
packages/apps/nats/values.yaml (1)
65-76: Enhance Resource Parameters and Remove Trailing SpacesThe addition of the
resourcesandresourcesPresetparameters is clear and consistent with the changes in other applications. Consider revising theresourcesPresetdescription for improved readability and grammatical accuracy. For instance, change it from:Set container resources according to one common preset (allowed values: none, nano, micro, small, medium, large, xlarge, 2xlarge). This is ignored if resources is set (resources is recommended for production).to:
- Set container resources according to one common preset (allowed values: none, nano, micro, small, medium, large, xlarge, 2xlarge). This is ignored if resources is set (resources is recommended for production). + Set container resources according to a common preset (allowed values: none, nano, micro, small, medium, large, xlarge, 2xlarge). This parameter is ignored if `resources` is specified (using `resources` is recommended for production).Additionally, static analysis flagged trailing whitespace at line 74. Please remove any extraneous spaces to ensure YAML lint compliance.
🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 74-74: trailing spaces
(trailing-spaces)
packages/apps/mysql/README.md (1)
86-98: Fix Typo and Clarify Descriptions in Backup ParametersIn the backup parameters table, there is a small typo—"pereiodic" should be corrected to "periodic." Also, similar to other files, consider updating the
resourcesPresetdescription for consistency and clarity. For example, update it from:- Set container resources according to one common preset (allowed values: none, nano, micro, small, medium, large, xlarge, 2xlarge). This is ignored if resources is set (resources is recommended for production). + Set container resources according to a common preset (allowed values: none, nano, micro, small, medium, large, xlarge, 2xlarge). This parameter is ignored if `resources` is specified (using `resources` is recommended for production).and
- | `backup.enabled` | Enable pereiodic backups | `false` | + | `backup.enabled` | Enable periodic backups | `false` |🧰 Tools
🪛 LanguageTool
[uncategorized] ~97-~97: This verb does not appear to agree with the subject. Consider using a different form.
Context: ... 2xlarge). This is ignored if resources is set (resources is recommended for produ...(AI_EN_LECTOR_REPLACEMENT_VERB_AGREEMENT)
[uncategorized] ~97-~97: This verb does not appear to agree with the subject. Consider using a different form.
Context: ... ignored if resources is set (resources is recommended for production). |nano...(AI_EN_LECTOR_REPLACEMENT_VERB_AGREEMENT)
packages/apps/rabbitmq/README.md (1)
25-30: ImproveresourcesPresetDescriptionThe new
resourcesandresourcesPresetparameters are a valuable addition. However, the description forresourcesPresetcan be made clearer. Consider rephrasing it from:- Set container resources according to one common preset (allowed values: none, nano, micro, small, medium, large, xlarge, 2xlarge). This is ignored if resources is set (resources is recommended for production). + Set container resources according to a common preset (allowed values: none, nano, micro, small, medium, large, xlarge, 2xlarge). This parameter is ignored if `resources` is specified (using `resources` is recommended for production).🧰 Tools
🪛 LanguageTool
[uncategorized] ~30-~30: This verb does not appear to agree with the subject. Consider using a different form.
Context: ... 2xlarge). This is ignored if resources is set (resources is recommended for produ...(AI_EN_LECTOR_REPLACEMENT_VERB_AGREEMENT)
[uncategorized] ~30-~30: This verb does not appear to agree with the subject. Consider using a different form.
Context: ... ignored if resources is set (resources is recommended for production). |nano|...(AI_EN_LECTOR_REPLACEMENT_VERB_AGREEMENT)
packages/apps/redis/values.schema.json (1)
35-38: Refine Schema Description forresourcesPresetThe JSON schema now includes the new resource management properties. To improve clarity and maintain consistency with other documentation, consider updating the
resourcesPresetdescription as follows:- "Set container resources according to one common preset (allowed values: none, nano, micro, small, medium, large, xlarge, 2xlarge). This is ignored if resources is set (resources is recommended for production)." + "Set container resources according to a common preset (allowed values: none, nano, micro, small, medium, large, xlarge, 2xlarge). This parameter is ignored if `resources` is specified (using `resources` is recommended for production)."packages/apps/ferretdb/values.yaml (1)
61-61: Fix trailing whitespaceThere is trailing whitespace on this line that should be removed.
-resourcesPreset: "nano" - +resourcesPreset: "nano"🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 61-61: trailing spaces
(trailing-spaces)
packages/apps/redis/README.md (1)
16-24: Resource management parameters look good.The addition of resource management parameters (
resourcesandresourcesPreset) follows best practices for Kubernetes deployments. This standardization will help users configure resource limits and requests consistently across different services.Small grammar suggestion in line 24: Consider changing "if resources is set (resources is recommended..." to "if resources is set (the resources parameter is recommended..." to improve clarity.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~24-~24: This verb does not appear to agree with the subject. Consider using a different form.
Context: ... 2xlarge). This is ignored if resources is set (resources is recommended for produ...(AI_EN_LECTOR_REPLACEMENT_VERB_AGREEMENT)
[uncategorized] ~24-~24: This verb does not appear to agree with the subject. Consider using a different form.
Context: ... ignored if resources is set (resources is recommended for production). |nano...(AI_EN_LECTOR_REPLACEMENT_VERB_AGREEMENT)
packages/apps/postgres/README.md (1)
61-72: Resource management parameters correctly added to backup section.The addition of resource management parameters is consistent with the changes in other services. These parameters will allow users to appropriately size the backup container resources.
Small grammar suggestion in line 72: Consider changing "if resources is set (resources is recommended..." to "if resources is set (the resources parameter is recommended..." to improve clarity.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~72-~72: This verb does not appear to agree with the subject. Consider using a different form.
Context: ... 2xlarge). This is ignored if resources is set (resources is recommended for produ...(AI_EN_LECTOR_REPLACEMENT_VERB_AGREEMENT)
[uncategorized] ~72-~72: This verb does not appear to agree with the subject. Consider using a different form.
Context: ... ignored if resources is set (resources is recommended for production). |nano...(AI_EN_LECTOR_REPLACEMENT_VERB_AGREEMENT)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (53)
packages/apps/clickhouse/Chart.yaml(1 hunks)packages/apps/clickhouse/README.md(1 hunks)packages/apps/clickhouse/templates/_resources.tpl(1 hunks)packages/apps/clickhouse/templates/clickhouse.yaml(1 hunks)packages/apps/clickhouse/values.schema.json(1 hunks)packages/apps/clickhouse/values.yaml(1 hunks)packages/apps/ferretdb/Chart.yaml(1 hunks)packages/apps/ferretdb/README.md(1 hunks)packages/apps/ferretdb/templates/_resources.tpl(1 hunks)packages/apps/ferretdb/templates/postgres.yaml(1 hunks)packages/apps/ferretdb/values.schema.json(1 hunks)packages/apps/ferretdb/values.yaml(1 hunks)packages/apps/kafka/Chart.yaml(1 hunks)packages/apps/kafka/README.md(1 hunks)packages/apps/kafka/templates/_resources.tpl(1 hunks)packages/apps/kafka/templates/kafka.yaml(1 hunks)packages/apps/kafka/values.schema.json(1 hunks)packages/apps/kafka/values.yaml(1 hunks)packages/apps/mysql/Chart.yaml(1 hunks)packages/apps/mysql/README.md(1 hunks)packages/apps/mysql/templates/_resources.tpl(1 hunks)packages/apps/mysql/templates/mariadb.yaml(1 hunks)packages/apps/mysql/values.schema.json(1 hunks)packages/apps/mysql/values.yaml(1 hunks)packages/apps/nats/Chart.yaml(1 hunks)packages/apps/nats/README.md(1 hunks)packages/apps/nats/templates/_resources.tpl(1 hunks)packages/apps/nats/templates/nats.yaml(1 hunks)packages/apps/nats/values.schema.json(1 hunks)packages/apps/nats/values.yaml(1 hunks)packages/apps/postgres/Chart.yaml(1 hunks)packages/apps/postgres/README.md(1 hunks)packages/apps/postgres/templates/_resources.tpl(1 hunks)packages/apps/postgres/templates/db.yaml(1 hunks)packages/apps/postgres/values.schema.json(1 hunks)packages/apps/postgres/values.yaml(1 hunks)packages/apps/rabbitmq/Chart.yaml(1 hunks)packages/apps/rabbitmq/README.md(1 hunks)packages/apps/rabbitmq/templates/_resources.tpl(1 hunks)packages/apps/rabbitmq/templates/rabbitmq.yaml(1 hunks)packages/apps/rabbitmq/values.schema.json(1 hunks)packages/apps/rabbitmq/values.yaml(1 hunks)packages/apps/redis/Chart.yaml(1 hunks)packages/apps/redis/README.md(1 hunks)packages/apps/redis/templates/_resources.tpl(1 hunks)packages/apps/redis/templates/redisfailover.yaml(1 hunks)packages/apps/redis/values.schema.json(1 hunks)packages/apps/redis/values.yaml(1 hunks)packages/apps/tenant/README.md(1 hunks)packages/apps/tenant/values.schema.json(1 hunks)packages/apps/versions_map(4 hunks)packages/core/installer/Makefile(1 hunks)packages/system/kubeovn/values.yaml(1 hunks)
🧰 Additional context used
🪛 YAMLlint (1.35.1)
packages/apps/clickhouse/values.yaml
[error] 59-59: trailing spaces
(trailing-spaces)
packages/apps/redis/values.yaml
[error] 24-24: trailing spaces
(trailing-spaces)
packages/apps/postgres/templates/db.yaml
[error] 9-9: syntax error: could not find expected ':'
(syntax)
packages/apps/kafka/templates/kafka.yaml
[error] 12-12: syntax error: could not find expected ':'
(syntax)
packages/apps/mysql/values.yaml
[error] 67-67: trailing spaces
(trailing-spaces)
packages/apps/kafka/values.yaml
[error] 52-52: trailing spaces
(trailing-spaces)
packages/apps/ferretdb/values.yaml
[error] 61-61: trailing spaces
(trailing-spaces)
packages/apps/postgres/values.yaml
[error] 89-89: trailing spaces
(trailing-spaces)
packages/apps/rabbitmq/values.yaml
[error] 52-52: trailing spaces
(trailing-spaces)
packages/apps/nats/values.yaml
[error] 74-74: trailing spaces
(trailing-spaces)
🪛 LanguageTool
packages/apps/ferretdb/README.md
[uncategorized] ~35-~35: This verb does not appear to agree with the subject. Consider using a different form.
Context: ... 2xlarge). This is ignored if resources is set (resources is recommended for produ...
(AI_EN_LECTOR_REPLACEMENT_VERB_AGREEMENT)
[uncategorized] ~35-~35: This verb does not appear to agree with the subject. Consider using a different form.
Context: ... ignored if resources is set (resources is recommended for production). | nano ...
(AI_EN_LECTOR_REPLACEMENT_VERB_AGREEMENT)
packages/apps/redis/README.md
[uncategorized] ~24-~24: This verb does not appear to agree with the subject. Consider using a different form.
Context: ... 2xlarge). This is ignored if resources is set (resources is recommended for produ...
(AI_EN_LECTOR_REPLACEMENT_VERB_AGREEMENT)
[uncategorized] ~24-~24: This verb does not appear to agree with the subject. Consider using a different form.
Context: ... ignored if resources is set (resources is recommended for production). | nano ...
(AI_EN_LECTOR_REPLACEMENT_VERB_AGREEMENT)
packages/apps/nats/README.md
[uncategorized] ~18-~18: This verb does not appear to agree with the subject. Consider using a different form.
Context: ... 2xlarge). This is ignored if resources is set (resources is recommended for produ...
(AI_EN_LECTOR_REPLACEMENT_VERB_AGREEMENT)
[uncategorized] ~18-~18: This verb does not appear to agree with the subject. Consider using a different form.
Context: ... ignored if resources is set (resources is recommended for production). | nano ...
(AI_EN_LECTOR_REPLACEMENT_VERB_AGREEMENT)
packages/apps/rabbitmq/README.md
[uncategorized] ~30-~30: This verb does not appear to agree with the subject. Consider using a different form.
Context: ... 2xlarge). This is ignored if resources is set (resources is recommended for produ...
(AI_EN_LECTOR_REPLACEMENT_VERB_AGREEMENT)
[uncategorized] ~30-~30: This verb does not appear to agree with the subject. Consider using a different form.
Context: ... ignored if resources is set (resources is recommended for production). | nano |...
(AI_EN_LECTOR_REPLACEMENT_VERB_AGREEMENT)
packages/apps/clickhouse/README.md
[uncategorized] ~50-~50: This verb does not appear to agree with the subject. Consider using a different form.
Context: ... 2xlarge). This is ignored if resources is set (resources is recommended for produ...
(AI_EN_LECTOR_REPLACEMENT_VERB_AGREEMENT)
[uncategorized] ~50-~50: This verb does not appear to agree with the subject. Consider using a different form.
Context: ... ignored if resources is set (resources is recommended for production). | nano ...
(AI_EN_LECTOR_REPLACEMENT_VERB_AGREEMENT)
packages/apps/mysql/README.md
[uncategorized] ~97-~97: This verb does not appear to agree with the subject. Consider using a different form.
Context: ... 2xlarge). This is ignored if resources is set (resources is recommended for produ...
(AI_EN_LECTOR_REPLACEMENT_VERB_AGREEMENT)
[uncategorized] ~97-~97: This verb does not appear to agree with the subject. Consider using a different form.
Context: ... ignored if resources is set (resources is recommended for production). | nano ...
(AI_EN_LECTOR_REPLACEMENT_VERB_AGREEMENT)
packages/apps/postgres/README.md
[uncategorized] ~72-~72: This verb does not appear to agree with the subject. Consider using a different form.
Context: ... 2xlarge). This is ignored if resources is set (resources is recommended for produ...
(AI_EN_LECTOR_REPLACEMENT_VERB_AGREEMENT)
[uncategorized] ~72-~72: This verb does not appear to agree with the subject. Consider using a different form.
Context: ... ignored if resources is set (resources is recommended for production). | nano ...
(AI_EN_LECTOR_REPLACEMENT_VERB_AGREEMENT)
🔇 Additional comments (36)
packages/apps/tenant/README.md (1)
60-60: Updated Default for theisolatedParameterThe default value for the
isolatedparameter has been updated fromfalsetotrueto enforce tenant namespace isolation by default. This change aligns well with the intended security posture and is clearly reflected in the documentation through the updated table entry.packages/apps/tenant/values.schema.json (1)
33-33: Synchronized Default forisolatedin JSON SchemaThe JSON schema now sets the default value for the
isolatedproperty totrue, ensuring consistency with the documentation update in the README. This update enforces tenant isolation by default, which is crucial for maintaining secure network policies.packages/apps/redis/Chart.yaml (1)
19-19: Version Update for Redis Chart
The chart version is correctly updated to 0.6.0, aligning with the coordinated version bump across applications.packages/apps/postgres/Chart.yaml (1)
19-19: Postgres Version Bump Confirmation
The version field is bumped to 0.10.0, which reflects the updates and anticipated changes (such as the addition of resource management options) in this chart.packages/apps/nats/Chart.yaml (1)
19-19: NATS Chart Version Updated
The version increment to 0.5.0 is in line with similar changes across the charts. Please ensure that any resource management presets (if applied in related configuration or README files) are consistent with this version update.packages/apps/rabbitmq/Chart.yaml (1)
19-19: RabbitMQ Chart Version Adjustment
The version update to 0.5.0 is appropriate. Since this chart now integrates new parameters for resource management (as mentioned in the related PR objectives and documentation updates), verify that the corresponding documentation (e.g., README.md) correctly reflects these new options.packages/apps/mysql/Chart.yaml (1)
19-19: MySQL Chart Version Increment
The version update to 0.6.0 is properly applied. Ensure that the new resource management features (i.e., theresourcesandresourcesPresetparameters described in the PR objectives) are consistently configured within the associated values and documentations.packages/apps/clickhouse/Chart.yaml (1)
19-19: Version number updated appropriatelyThe chart version has been incremented from 0.6.2 to 0.7.0, which correctly follows semantic versioning practices for adding new functionality (resource management capabilities) without breaking changes.
packages/apps/ferretdb/Chart.yaml (1)
19-19: Version number updated appropriatelyThe chart version has been incremented from 0.4.2 to 0.5.0, correctly following semantic versioning practices for the addition of the new resource management features.
packages/apps/kafka/Chart.yaml (1)
19-19: Version number updated appropriatelyThe chart version has been incremented from 0.3.3 to 0.4.0, which correctly follows semantic versioning practices and maintains consistency with the versioning approach used in other charts in this PR.
packages/apps/mysql/templates/mariadb.yaml (1)
76-80: Resource configuration logic looks goodThe conditional logic for resource management is well-implemented, using either explicit values or preset definitions. The code handles two scenarios: when custom resources are defined and when a preset is specified.
Note: There's an implicit third case where both
.Values.resourcesis undefined AND.Values.resourcesPresetis "none". In this case, no resources would be defined. Please verify this is the intended behavior.packages/apps/rabbitmq/templates/_resources.tpl (2)
14-44: Resource preset definitions look well-structured with graduated sizesThe preset structure provides a good range of resource options from nano to 2xlarge. I notice a few patterns:
- For smaller sizes (nano through large), limits are consistently 150% of requests for CPU and memory
- For xlarge and 2xlarge, the CPU limits scale more aggressively (3x and 6x respectively)
- Ephemeral storage has a consistent 50Mi request but much larger 2Gi limit across all sizes
This implementation should provide good flexibility for different workload requirements.
45-49: Good error handling for invalid preset typesThe template appropriately validates that the provided preset type exists in the dictionary and fails with a clear error message listing allowed values if an invalid type is specified.
packages/apps/clickhouse/templates/clickhouse.yaml (1)
124-128:Details
✅ Verification successful
Resource configuration looks good with proper indentation
The conditional logic for resource configuration is implemented correctly with proper indentation (16 spaces) for the container spec context. The implementation provides flexibility by allowing either custom resources or predefined presets.
Note that this implementation directly uses
.Values.resourceswhile the RabbitMQ template uses.Values.podTemplate.resources. Ensure this difference is intentional based on your values schema structure.
🏁 Script executed:
#!/bin/bash # Verify if this resources implementation difference is intentional by checking the values schema # Check clickhouse values.yaml for resources structure echo "Checking ClickHouse values.yaml structure:" cat packages/apps/clickhouse/values.yaml | grep -A 10 "resources\|resourcesPreset" # Compare with RabbitMQ values structure echo -e "\nComparing with RabbitMQ values.yaml structure:" cat packages/apps/rabbitmq/values.yaml | grep -A 10 "resources\|resourcesPreset"Length of output: 1217
Resource configuration verified with values schema consistency
- The conditional logic in the ClickHouse template is implemented correctly with proper indentation (16 spaces) and flexibility for custom or preset resources.
- Verification of both
packages/apps/clickhouse/values.yamlandpackages/apps/rabbitmq/values.yamlconfirms they share an identical structure with a top-levelresourceskey.- The original note mentioned that the RabbitMQ template uses
.Values.podTemplate.resources, but the values file for RabbitMQ does not define apodTemplatekey. Please verify that the RabbitMQ template reference is intentional or update it accordingly if it’s using legacy logic.packages/apps/kafka/templates/_resources.tpl (3)
1-12: Well-structured template with clear documentationThe header documentation clearly explains the purpose and usage of the template, which helps other developers understand how to use it correctly.
14-14: Good explanation of the resource allocation strategyThe comment explaining the general pattern for setting limits (requests + 50%) helps maintain consistency and makes the implementation more understandable.
45-49: Effective error handling implementationThe error handling is well-implemented, providing clear feedback when an invalid preset type is provided, including listing the allowed values. This will help users quickly identify and fix configuration issues.
packages/apps/postgres/templates/_resources.tpl (1)
1-50: Consistent implementation across applicationsThe resource presets template is consistent with the implementation in other applications (kafka, clickhouse, etc.), which is excellent for maintainability and user experience. Users will have a predictable experience when configuring resources across different components.
The same consideration about CPU requests for xlarge and 2xlarge presets applies here as well - consider whether the 1.0 CPU request with much higher limits (3.0/6.0) is the intended pattern for these workload sizes.
packages/apps/redis/templates/redisfailover.yaml (2)
28-32: Conditional Resource Configuration for SentinelThe conditional block for the Sentinel resource configuration is well implemented by checking for
.Values.resourcesand falling back to a preset when it is not specified. Please ensure that the referencedresources.presettemplate returns valid YAML and that the indentation (withnindent 6) remains correct in all cases.
35-39: Conditional Resource Configuration for RedisThis block mirrors the Sentinel logic and maintains consistency for the Redis resource configuration. As with the Sentinel block, confirm that the generated YAML from the
resources.presettemplate is correct and that the indentation meets expectations.packages/apps/postgres/templates/db.yaml (1)
8-12: Ensure Valid YAML Output for Resource ConfigurationThe conditional resource configuration introduced here is a good approach for offering both custom resource definitions and preset-based configurations. However, YAMLlint has flagged a potential syntax error on line 9 (e.g., "could not find expected ':'"). Please verify the rendered YAML output after templating to ensure that the
toYamlconversion and subsequent indentation produce valid YAML.🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 9-9: syntax error: could not find expected ':'
(syntax)
packages/apps/nats/values.schema.json (1)
50-59: JSON Schema Update: Resource PropertiesThe addition of the
resources(object) andresourcesPreset(string) fields is well documented and aligns with the unified resource management strategy. The default values (an empty object forresourcesand"nano"forresourcesPreset) seem appropriate. Please verify that these defaults integrate smoothly with downstream templating and schema validation.packages/apps/mysql/values.schema.json (1)
70-79: MySQL JSON Schema: New Resource FieldsThe new
resourcesandresourcesPresetproperties are correctly added and documented. Their definitions and default values are consistent with similar changes elsewhere in the project. This unified approach should improve configurability across deployments.packages/apps/postgres/values.schema.json (1)
105-113: Postgres JSON Schema: Consistent Resource PropertiesThe additions of
resourcesandresourcesPresetare clear and maintain consistency with the other applications’ schemas. Ensure that these changes integrate seamlessly with the rest of the schema definitions and that validations pass without conflicts in your CI pipeline.packages/apps/rabbitmq/values.schema.json (1)
30-39: Good addition of resource management options.The new
resourcesandresourcesPresetproperties provide flexible resource configuration options for RabbitMQ deployments. The comprehensive description forresourcesPresetclearly explains its relationship with theresourcesproperty and provides guidance on production usage.packages/apps/nats/templates/_resources.tpl (1)
1-50: Well-structured resource presets template.The template provides a comprehensive set of resource presets with appropriate scaling across different sizes. The implementation is robust with:
- Clear documentation and attribution
- Well-defined resource tiers from nano to 2xlarge
- Proper error handling for invalid preset types
- Consistent pattern of setting limits at approximately 150% of requests
packages/apps/clickhouse/values.yaml (1)
60-60: Resource Preset Parameter Implementation
The newresourcesPresetparameter is defined with a default value of"nano". Its documentation clearly states that it is ignored if explicit resource settings are provided—a design decision that aligns with best practices for production environments.packages/apps/postgres/values.yaml (1)
90-90: Resource Preset Parameter Implementation
TheresourcesPresetparameter is appropriately set to"nano"by default and its purpose—using a common preset unless explicit resources are specified—is clearly documented.packages/apps/nats/templates/nats.yaml (1)
41-51: Conditional Resource Configuration in NATS Template
The newpodTemplatesection undernatsintroduces dynamic resource configuration. The conditional logic properly checks for an explicit.Values.resourcesvalue and falls back to using a preset—via theresources.presetMustache template—if.Values.resourcesPresetis not set to"none". This implementation provides the flexibility intended by the PR objectives.packages/apps/mysql/values.yaml (1)
68-68: Resource Preset Parameter Implementation
TheresourcesPresetparameter is defined with a default value of"nano"and comes with a clear explanation that it will be ignored whenresourcesis explicitly configured. This aligned approach ensures consistency across the charts.packages/apps/ferretdb/values.yaml (1)
51-63: Appropriate resource management parameters addedThe addition of
resourcesandresourcesPresetparameters provides useful configuration options for managing container resources. This follows good Helm chart practices by allowing both explicit resource definitions and simplified preset-based configurations.🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 61-61: trailing spaces
(trailing-spaces)
packages/apps/kafka/values.schema.json (1)
55-65: Schema properly configured for new resource parametersThe schema correctly defines the new
resourcesandresourcesPresetproperties with appropriate types, descriptions, and default values. This ensures proper validation of user input and provides clear documentation about parameter usage.packages/apps/mysql/templates/_resources.tpl (2)
1-50: Well-structured resource presets templateThis template provides a robust implementation of resource presets with:
- Clear documentation and purpose
- Well-defined preset configurations for different resource sizes
- Proper error handling for invalid preset types
- Consistent pattern of setting limits as requests + 50%
The warning about presets being for testing rather than production is a good practice to set appropriate expectations.
46-49: Excellent error handling implementationThe error reporting includes both the invalid key and all allowed values, which provides actionable information to users. This follows best practices for user-friendly error messages.
packages/apps/kafka/README.md (1)
19-23: Documentation properly updated with new parametersThe README has been correctly updated to document the new
resourcesandresourcesPresetparameters with clear descriptions and default values. The table formatting is consistent with the rest of the documentation.packages/apps/versions_map (1)
10-11: Version mapping updates look appropriate.The changes follow a consistent pattern of:
- Updating previous versions from
HEADto specific commit hashes- Adding new versions with
HEADreferenceThis is good practice for version management and ensures that the versions properly track the development history.
Also applies to: 18-19, 32-33, 63-64, 70-71, 84-85, 93-94, 100-101
2 participants
Summary by CodeRabbit
New Features
Version Updates
Infrastructure Updates