feat: Support container resize policy for runtime resource updates #232
Conversation
…replacement * Reference: https://kubernetes.io/docs/tasks/configure-pod-container/resize-container-resources/ Signed-off-by: younsl <[email protected]>
* artifacthub.io annotations in Chart.yaml are based on https://artifacthub.io/docs/topics/annotations/helm/. * Bump chart version minor. See: https://github.com/8gears/n8n-helm-chart\?tab\=readme-ov-file\#chart-versioning-schema
WalkthroughBump chart version to 1.0.15 and add support for container Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Values as values.yaml
participant Helm as Helm renderer
participant K8s as Kubernetes API
User->>Values: Optionally set *.resizePolicy
Values->>Helm: Provide values
Helm->>Helm: If set, render containers[*].resizePolicy via toYaml
Helm->>K8s: Apply rendered Deployment manifests
note right of Helm: resizePolicy rendered only when value provided
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ Finishing Touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type 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: 0
♻️ Duplicate comments (2)
charts/n8n/values.yaml (2)
450-456: Repeat of main: add the same guidance for worker.Mirror the comments from the main section so users get consistent hints across roles.
Apply the same comment block as suggested for Lines 258-264.
644-650: Repeat of main: add the same guidance for webhook.Mirror the comments from the main section for consistency.
Apply the same comment block as suggested for Lines 258-264.
🧹 Nitpick comments (6)
charts/n8n/Chart.yaml (1)
37-41: Polish Artifact Hub change entry and remove trailing space.
- Fix grammar: “Add support for spec.containers.resizePolicy …” and tighten the second sentence.
- Remove the trailing space flagged by yamllint on Line 41.
Apply this diff:
artifacthub.io/changes: | - kind: added - description: Add support spec.containers.resizePolicy for all containers. This allows to control the behavior when a container is resized. + description: Add support for spec.containers.resizePolicy for all containers. This allows controlling the behavior when a container is resized. links: - name: GitHub PR - url: https://github.com/8gears/n8n-helm-chart/pull/232 + url: https://github.com/8gears/n8n-helm-chart/pull/232charts/n8n/values.yaml (2)
258-264: Document version support and allowed values for resizePolicy (main).Add quick guidance for users to avoid applying on unsupported clusters and to constrain inputs to the K8s enum values.
Apply this diff to enrich comments:
# Container resize policy allows runtime resource updates without pod restart + # Requires Kubernetes version that supports in-place resources resize (v1.27+ recommended). + # Allowed values per entry: + # resourceName: cpu | memory + # restartPolicy: NotRequired | RestartContainer resizePolicy: [] # - resourceName: cpu # restartPolicy: NotRequired # - resourceName: memory # restartPolicy: RestartContainerOptionally, add a values.schema.json to validate the enum values (see suggestion below).
258-264: Optional: add values.schema.json to validate resizePolicy entries.This prevents typos and invalid values at chart render time.
Proposed schema snippet (new file: charts/n8n/values.schema.json):
{ "$schema": "https://json-schema.org/draft/2019-09/schema", "type": "object", "properties": { "main": { "type": "object", "properties": { "resizePolicy": { "type": "array", "items": { "type": "object", "required": ["resourceName", "restartPolicy"], "properties": { "resourceName": { "type": "string", "enum": ["cpu", "memory"] }, "restartPolicy": { "type": "string", "enum": ["NotRequired", "RestartContainer"] } }, "additionalProperties": false } } } }, "worker": { "$ref": "#/properties/main" }, "webhook": { "$ref": "#/properties/main" } } }If you want, I can open a follow-up PR to add this schema.
Also applies to: 450-456, 644-650
charts/n8n/templates/deployment.yaml (1)
96-99: Gate resizePolicy on Kubernetes version to avoid apply failures on older clusters.On unsupported clusters, emitting resizePolicy can cause server-side validation errors. Keep default behavior untouched, but render only when K8s version is compatible, or fail early with a clear message.
Apply this diff:
- {{- with .Values.main.resizePolicy }} - resizePolicy: - {{- toYaml . | nindent 12 }} - {{- end }} + {{- if and (.Values.main.resizePolicy) (semverCompare ">=1.27-0" .Capabilities.KubeVersion.Version) }} + resizePolicy: + {{- toYaml .Values.main.resizePolicy | nindent 12 }} + {{- else if .Values.main.resizePolicy }} + {{- fail (printf "main.resizePolicy requires Kubernetes >= 1.27; detected %s" .Capabilities.KubeVersion.Version) }} + {{- end }}If you prefer not to hard-fail, replace fail with a comment or emit nothing and document the version requirement in values.yaml (as suggested).
charts/n8n/templates/deployment.worker.yaml (1)
111-114: Apply the same Kubernetes version guard for worker.Keep behavior consistent and user-friendly across all roles.
Apply this diff:
- {{- with .Values.worker.resizePolicy }} - resizePolicy: - {{- toYaml . | nindent 12 }} - {{- end }} + {{- if and (.Values.worker.resizePolicy) (semverCompare ">=1.27-0" .Capabilities.KubeVersion.Version) }} + resizePolicy: + {{- toYaml .Values.worker.resizePolicy | nindent 12 }} + {{- else if .Values.worker.resizePolicy }} + {{- fail (printf "worker.resizePolicy requires Kubernetes >= 1.27; detected %s" .Capabilities.KubeVersion.Version) }} + {{- end }}charts/n8n/templates/deployment.webhook.yaml (1)
115-118: Apply the same Kubernetes version guard for webhook.Aligns behavior with main and worker to prevent surprises.
Apply this diff:
- {{- with .Values.webhook.resizePolicy }} - resizePolicy: - {{- toYaml . | nindent 12 }} - {{- end }} + {{- if and (.Values.webhook.resizePolicy) (semverCompare ">=1.27-0" .Capabilities.KubeVersion.Version) }} + resizePolicy: + {{- toYaml .Values.webhook.resizePolicy | nindent 12 }} + {{- else if .Values.webhook.resizePolicy }} + {{- fail (printf "webhook.resizePolicy requires Kubernetes >= 1.27; detected %s" .Capabilities.KubeVersion.Version) }} + {{- end }}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
charts/n8n/Chart.yaml(2 hunks)charts/n8n/templates/deployment.webhook.yaml(1 hunks)charts/n8n/templates/deployment.worker.yaml(1 hunks)charts/n8n/templates/deployment.yaml(1 hunks)charts/n8n/values.yaml(3 hunks)
🧰 Additional context used
🪛 YAMLlint (1.37.1)
charts/n8n/Chart.yaml
[error] 41-41: trailing spaces
(trailing-spaces)
🔇 Additional comments (1)
charts/n8n/Chart.yaml (1)
3-3: SemVer bump looks right for a feature addition.Minor version increment to 1.1.0 is appropriate for introducing optional functionality without breaking defaults.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
charts/n8n/Chart.yaml (2)
3-3: Consider minor version bump for a feature addition (SemVer).Adding a new user-facing capability (container resizePolicy) is a feature, not a bug fix. Per SemVer, consider bumping the chart minor version instead of patch.
-version: 1.0.13 +version: 1.1.0If the project intentionally treats such changes as patch-level, feel free to keep 1.0.13—just confirm your versioning policy.
37-40: Polish Artifact Hub change entry for clarity and correctness.Minor grammar fix and more precise field path. Optionally add a link to Kubernetes docs for context.
- - kind: added - description: Add support spec.containers.resizePolicy for all containers. This allows to control the behavior when a container is resized. - links: - - name: GitHub PR - url: https://github.com/8gears/n8n-helm-chart/pull/232 + - kind: added + description: Add support for spec.containers[].resizePolicy for all containers. Controls behavior when a container's CPU/memory resources are resized in-place. + links: + - name: GitHub PR + url: https://github.com/8gears/n8n-helm-chart/pull/232 + - name: Kubernetes docs + url: https://kubernetes.io/docs/tasks/configure-pod-container/resize-container-resources/
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
charts/n8n/Chart.yaml(2 hunks)
🧰 Additional context used
🪛 YAMLlint (1.37.1)
charts/n8n/Chart.yaml
[error] 41-41: trailing spaces
(trailing-spaces)
|
@Vad1mo @RoseSecurity PTAL. I need a this feature in my production n8n chart. |
|
Testing this today. Thanks for contributing |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
charts/n8n/Chart.yaml (2)
37-41: Polish Artifact Hub change note grammar for clarity.Improve wording and be explicit about the field path.
artifacthub.io/changes: | - - kind: added - description: Add support spec.containers.resizePolicy for all containers. This allows to control the behavior when a container is resized. - links: + - kind: added + description: Add support for spec.containers.resizePolicy on all containers. This lets you control restart behavior when CPU/memory resources are resized. + links: - name: GitHub PR url: https://github.com/8gears/n8n-helm-chart/pull/232
37-41: Add a short compatibility hint (docs/values) for resizePolicy.Consider noting that container resizePolicy is supported from Kubernetes 1.27 (alpha) and appears in the API reference by 1.29; in-place resize is enabled by default starting 1.33 (Beta). This helps users know when the new values work as intended. (kubernetes.io)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
charts/n8n/Chart.yaml(2 hunks)charts/n8n/templates/deployment.webhook.yaml(1 hunks)charts/n8n/templates/deployment.worker.yaml(1 hunks)charts/n8n/templates/deployment.yaml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- charts/n8n/templates/deployment.worker.yaml
- charts/n8n/templates/deployment.yaml
- charts/n8n/templates/deployment.webhook.yaml
🧰 Additional context used
🪛 YAMLlint (1.37.1)
charts/n8n/Chart.yaml
[error] 41-41: no new line character at the end of file
(new-line-at-end-of-file)
🔇 Additional comments (1)
charts/n8n/Chart.yaml (1)
3-3: Version bump looks good.Patch version increase to 1.0.14 is consistent with a backward-compatible feature addition.
|
@RoseSecurity Any updates? |
|
@RoseSecurity Thx. Ready to merge after resolving conflicts caused by d06fe8b. |
* This rebump is caused by PR 8gears#239 Signed-off-by: younsl <[email protected]>
|
@Vad1mo @RoseSecurity Please merge if no blockers. |
|
@Vad1mo @RoseSecurity Gentle ping for merging |
2 participants
Changes
Reference
Summary by CodeRabbit
New Features
Chores