feat: improve deployment modes and persistence validation #175
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughIntroduces component-specific StatefulSet support and PVCs for main/worker/webhook, adds validation for replica/persistence combinations, refactors Helm values into main/worker/webhook, adds README deployment/migration guidance, and updates chart version and CI linting to validate StatefulSet rendering. (49 words) Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Helm
participant Validator as n8n.validateValues
participant Kubernetes
User->>Helm: helm install/upgrade (values.yaml)
Helm->>Validator: run validation checks
alt validation fails
Validator-->>Helm: error -> abort render
else validation succeeds
Helm->>Kubernetes: render templates (Deployment/StatefulSet, PVCs, HPA)
alt main.statefulSet.enabled
Kubernetes->>Kubernetes: create StatefulSet (main) + PVC (-main)
else
Kubernetes->>Kubernetes: create Deployment (main) + PVC/emptyDir
end
alt worker.statefulSet.enabled
Kubernetes->>Kubernetes: create StatefulSet (worker) + PVC (-worker)
else
Kubernetes->>Kubernetes: create Deployment (worker) + PVC/emptyDir
end
Kubernetes->>Kubernetes: create Webhook Deployment (+/- webhook PVC)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20–30 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks (3 passed)✅ Passed checks (3 passed)
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. ✨ 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. Comment |
![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
🧹 Nitpick comments (4)
charts/n8n/values.yaml (2)
263-266: Main Component StatefulSet Configuration
The newstatefulSetblock under themainsection is clearly introduced withenabled: false. This addition gives users the option to switch from a Deployment to a StatefulSet while preserving the default behavior. Please ensure that the documentation (and any validation functions in the templates) inform users that using a StatefulSet may require an n8n Enterprise license when scaling beyond one replica.
281-284: Commented Queue Health Check Examples
The commented-out environment variable examples for queue health checks in the worker’sextraEnvare helpful as in-code documentation. Consider adding a brief note in the README or inline comment explaining when to enable these settings.charts/n8n/templates/statefulset.yaml (1)
1-15: StatefulSet for Main Component
This new template for a StatefulSet is conditionally rendered whenmain.statefulSet.enabledis true. The metadata (including labels, deployment labels, and annotations) is well structured. Be mindful that some YAML lint tools might flag indentation issues (e.g. on lines 9–10) due to templating; if these are false positives after Helm rendering, they can be safely ignored.🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
[warning] 9-9: wrong indentation: expected 2 but found 4
(indentation)
[warning] 10-10: wrong indentation: expected 2 but found 4
(indentation)
charts/n8n/templates/statefulset.worker.yaml (1)
8-14: Worker Deployment Metadata
The metadata section for the worker StatefulSet, including labels and annotations, is consistent with the main component template. Again, if YAML lint warnings about indentation occur, ensure they are verified post-templating output.🧰 Tools
🪛 YAMLlint (1.35.1)
[warning] 9-9: wrong indentation: expected 2 but found 4
(indentation)
[warning] 10-10: wrong indentation: expected 2 but found 4
(indentation)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
README.md(6 hunks)charts/n8n/templates/_helpers.tpl(3 hunks)charts/n8n/templates/deployment.webhook.yaml(5 hunks)charts/n8n/templates/deployment.worker.yaml(3 hunks)charts/n8n/templates/deployment.yaml(3 hunks)charts/n8n/templates/pvc.yaml(2 hunks)charts/n8n/templates/statefulset.worker.yaml(1 hunks)charts/n8n/templates/statefulset.yaml(1 hunks)charts/n8n/values.yaml(2 hunks)
🧰 Additional context used
🪛 YAMLlint (1.35.1)
charts/n8n/templates/deployment.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
charts/n8n/templates/statefulset.worker.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
[warning] 9-9: wrong indentation: expected 2 but found 4
(indentation)
[warning] 10-10: wrong indentation: expected 2 but found 4
(indentation)
charts/n8n/templates/statefulset.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
[warning] 9-9: wrong indentation: expected 2 but found 4
(indentation)
[warning] 10-10: wrong indentation: expected 2 but found 4
(indentation)
charts/n8n/templates/deployment.worker.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
🪛 markdownlint-cli2 (0.17.2)
README.md
639-639: Multiple spaces after hash on atx style heading
null
(MD019, no-multiple-space-atx)
639-639: Headings must start at the beginning of the line
null
(MD023, heading-start-left)
640-640: Multiple spaces after hash on atx style heading
null
(MD019, no-multiple-space-atx)
640-640: Headings must start at the beginning of the line
null
(MD023, heading-start-left)
641-641: Multiple spaces after hash on atx style heading
null
(MD019, no-multiple-space-atx)
641-641: Headings must start at the beginning of the line
null
(MD023, heading-start-left)
642-642: Multiple spaces after hash on atx style heading
null
(MD019, no-multiple-space-atx)
642-642: Headings must start at the beginning of the line
null
(MD023, heading-start-left)
642-642: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
643-643: Multiple spaces after hash on atx style heading
null
(MD019, no-multiple-space-atx)
643-643: Headings must start at the beginning of the line
null
(MD023, heading-start-left)
644-644: Multiple spaces after hash on atx style heading
null
(MD019, no-multiple-space-atx)
644-644: Headings must start at the beginning of the line
null
(MD023, heading-start-left)
🔇 Additional comments (28)
charts/n8n/values.yaml (1)
286-289: Worker StatefulSet Option
The addition of thestatefulSetconfiguration under theworkersection is consistent with the main component changes. This gives users the flexibility to choose a StatefulSet for worker pods as needed. Please double-check that any usage of this setting is also documented, especially regarding persistence constraints.charts/n8n/templates/deployment.yaml (3)
1-2: Conditional Validation for Deployment
The insertion of a conditional check to include then8n.validateValuesfunction whenmain.statefulSet.enabledis false is a smart move to enforce configuration constraints. Ensure that the helper function is comprehensive in checking persistence and replica-related conditions.🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
99-102: Conditional Volume Mount for Persistence
The block that conditionally adds the volume mount for/home/node/.n8nwhenmain.persistence.enabledis true is clear and concise. This properly limits the mount to when persistent storage is in use.
122-126: Conditional Volume Definition
Including a conditional block to add the volume (usinginclude "n8n.pvc" .) when persistence is enabled (and appending any extra volumes if defined) enhances modularity. Please verify that then8n.pvchelper consistently renders the expected PVC spec.charts/n8n/templates/statefulset.yaml (2)
91-98: Container Volume Mounts and Extra Mounts
The conditional logic ensuring that the container only defines thedatavolume mount when persistence is enabled—and appending any extra volume mounts—is well executed.
115-134: VolumeClaimTemplates Block
The PVC template undervolumeClaimTemplatesis clearly defined. It correctly iterates over the defined access modes and conditionally sets thestorageClassNamebased on whether a hyphen ("-") is specified. Please verify in practice that these conditions work as intended when users supply custom values.charts/n8n/templates/statefulset.worker.yaml (4)
1-4: Conditional Rendering of Worker StatefulSet
The template is rendered only when bothworker.enabledandworker.statefulSet.enabledare true, which aligns with the PR objectives. This conditional check avoids conflicts in PVC management for workers that are deployed as StatefulSets.🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
75-87: Worker Container Command and Args
The logic to conditionally set the container’s command (defaulting to"n8n") and args (defaulting to"worker"with a concurrency flag) is clear and provides sensible defaults.
103-110: Worker Container Persistence Volume Mount
The conditional volume mount under the worker container for the persistent storage is correctly applied whenworker.persistence.enabledis true.
127-146: Worker VolumeClaimTemplates
The PVC specification for the worker component is correctly gated by the condition that persistence is enabled and the worker is not deployed via StatefulSet. This avoids duplicate PVC definitions when using StatefulSet deployments.charts/n8n/templates/pvc.yaml (3)
1-29: Main Component PVC Definition
The updated PVC for the main component now appends a-mainsuffix to its name. The annotations, labels, access modes, and resource requests are defined in a consistent manner. This conditionally creates a PVC only whenmain.persistence.enabledis true and no existing claim is provided.🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
31-59: Webhook PVC Definition
The new PVC for the webhook component is clearly defined with a-webhooksuffix. The conditional ensures that the PVC is only created when persistence is enabled and no existing claim is used.
61-89: Worker PVC Definition
The PVC for the worker component is conditionally created only if the worker is enabled, persistence is enabled, no existing claim is provided, and the worker is not using a StatefulSet. This logic matches the PR objective of preventing persistent deployments with multiple replicas where not allowed.charts/n8n/templates/deployment.worker.yaml (3)
1-1: Refined worker deployment condition to support StatefulSet alternativeThe condition now checks both if the worker is enabled AND if the StatefulSet mode is not enabled, ensuring the deployment is only created when the appropriate deployment mode is selected.
🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
111-115: Improved persistence configuration with conditional volume mountsGood implementation of conditional volume mounts that are only applied when persistence is enabled, making the template more flexible and robust.
135-139: Enhanced persistence handling with worker-specific PVCThe volumes section is now conditional and uses the worker-specific PVC template function instead of the generic one, aligning with the modular approach for component-specific persistence configurations.
charts/n8n/templates/deployment.webhook.yaml (5)
4-4: Added validation for deployment configurationIncluding the validation function ensures that invalid configurations (like multiple replicas with persistent storage) are caught early, preventing potential data loss scenarios.
36-36: Improved configuration checksum calculationThe checksum now includes all values instead of just webhook and main values, ensuring that any configuration change will trigger a pod restart.
60-61: Added explicit security context for webhook containerExplicitly defining the security context at the container level is a good security practice that ensures the container runs with the appropriate permissions.
115-119: Added conditional volume mounts for persistenceSimilar to the worker component, the webhook now has conditional volume mounts that are only applied when persistence is enabled, improving template flexibility.
140-143: Enhanced persistence with webhook-specific PVCThe volumes section now uses the webhook-specific PVC template and is conditional based on persistence being enabled, providing better separation of concerns.
README.md (3)
4-4: Fixed typo in popularity wordSimple typo correction improves the document's professionalism.
14-16: Added important licensing note for high availability setupThis critical information clearly states the licensing requirements for using StatefulSet with multiple replicas, helping users make informed decisions.
364-485: Comprehensive documentation of deployment modes and high availabilityThe new sections provide detailed explanations of the various deployment options for main, worker, and webhook components, along with examples and limitations. This documentation is essential for users to understand the configuration options and constraints of each deployment mode.
Especially valuable is the clear distinction between:
- Deployment (default) mode limitations with persistent storage
- StatefulSet requirements for high availability
- Enterprise licensing requirements
- The different persistence options and their implications
The YAML examples make it easy for users to implement the configurations correctly.
charts/n8n/templates/_helpers.tpl (4)
63-63: Improved PVC template comment clarityThe updated comment clearly specifies that this PVC definition is for the main component, enhancing template readability.
72-72: Enhanced PVC claim naming for main componentAdding the "-main" suffix to the claim name improves clarity and consistency with the component-specific naming convention used for the webhook and worker components.
76-100: Added dedicated PVC template functions for webhook and worker componentsThese new template functions follow the same pattern as the main component's PVC function, providing component-specific persistence configurations. This modular approach allows for more flexible configuration and better separation of concerns.
129-139: Added robust validation for deployment configurationsThe validation function enforces important constraints:
- Deployment mode (non-StatefulSet) with persistence can only have one replica
- Webhook with persistence can only have one replica
This validation is crucial to prevent data loss scenarios and provides clear, actionable error messages to guide users toward valid configurations.
|
Very nice thank you |
aleksandrovpa
force-pushed
the
feature/main-statefulset-support
branch
from
845fdf1 to
b958325
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
README.md (1)
639-644: Markdown formatting issue in init container example.There are multiple spaces after hash characters in the init container example section, which might cause rendering issues in some markdown processors.
- # - name: init-data-dir - # image: "{{ .Values.image.repository }}:{{ .Values.image.tag | default .Chart.AppVersion }}" - # command: [ "/bin/sh", "-c", "mkdir -p /home/node/.n8n/" ] - # volumeMounts: - # - name: data - # mountPath: /home/node/.n8n +# - name: init-data-dir +# image: "{{ .Values.image.repository }}:{{ .Values.image.tag | default .Chart.AppVersion }}" +# command: [ "/bin/sh", "-c", "mkdir -p /home/node/.n8n/" ] +# volumeMounts: +# - name: data +# mountPath: /home/node/.n8n🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
639-639: Multiple spaces after hash on atx style heading
null(MD019, no-multiple-space-atx)
639-639: Headings must start at the beginning of the line
null(MD023, heading-start-left)
640-640: Multiple spaces after hash on atx style heading
null(MD019, no-multiple-space-atx)
640-640: Headings must start at the beginning of the line
null(MD023, heading-start-left)
641-641: Multiple spaces after hash on atx style heading
null(MD019, no-multiple-space-atx)
641-641: Headings must start at the beginning of the line
null(MD023, heading-start-left)
642-642: Multiple spaces after hash on atx style heading
null(MD019, no-multiple-space-atx)
642-642: Headings must start at the beginning of the line
null(MD023, heading-start-left)
642-642: Trailing punctuation in heading
Punctuation: ':'(MD026, no-trailing-punctuation)
643-643: Multiple spaces after hash on atx style heading
null(MD019, no-multiple-space-atx)
643-643: Headings must start at the beginning of the line
null(MD023, heading-start-left)
644-644: Multiple spaces after hash on atx style heading
null(MD019, no-multiple-space-atx)
644-644: Headings must start at the beginning of the line
null(MD023, heading-start-left)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
README.md(6 hunks)charts/n8n/Chart.yaml(1 hunks)charts/n8n/templates/_helpers.tpl(3 hunks)charts/n8n/templates/deployment.webhook.yaml(5 hunks)charts/n8n/templates/deployment.yaml(3 hunks)
✅ Files skipped from review due to trivial changes (1)
- charts/n8n/Chart.yaml
🧰 Additional context used
🪛 LanguageTool
README.md
[uncategorized] ~16-~16: You might be missing the article “an” here.
Context: ...uires n8n Enterprise license. > Without Enterprise license, use Deployment with a single m...
(AI_EN_LECTOR_MISSING_DETERMINER_AN)
🪛 markdownlint-cli2 (0.17.2)
README.md
639-639: Multiple spaces after hash on atx style heading
null
(MD019, no-multiple-space-atx)
639-639: Headings must start at the beginning of the line
null
(MD023, heading-start-left)
640-640: Multiple spaces after hash on atx style heading
null
(MD019, no-multiple-space-atx)
640-640: Headings must start at the beginning of the line
null
(MD023, heading-start-left)
641-641: Multiple spaces after hash on atx style heading
null
(MD019, no-multiple-space-atx)
641-641: Headings must start at the beginning of the line
null
(MD023, heading-start-left)
642-642: Multiple spaces after hash on atx style heading
null
(MD019, no-multiple-space-atx)
642-642: Headings must start at the beginning of the line
null
(MD023, heading-start-left)
642-642: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
643-643: Multiple spaces after hash on atx style heading
null
(MD019, no-multiple-space-atx)
643-643: Headings must start at the beginning of the line
null
(MD023, heading-start-left)
644-644: Multiple spaces after hash on atx style heading
null
(MD019, no-multiple-space-atx)
644-644: Headings must start at the beginning of the line
null
(MD023, heading-start-left)
🪛 YAMLlint (1.35.1)
charts/n8n/templates/deployment.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
🔇 Additional comments (16)
charts/n8n/templates/deployment.yaml (4)
1-2: Good addition of validation check for non-StatefulSet deployments.The conditional check ensures that validation is only performed when StatefulSet mode is not enabled, which is appropriate since different validation rules apply to different deployment types.
🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
99-105: Well-structured conditional volume mounting.The conditional volume mount for data is correctly implemented to only be included when persistence is enabled. This prevents unnecessary volume mounts when persistence is disabled.
122-129: Good implementation of conditional volumes.Similar to the volume mounts, the volumes section is correctly conditionally included only when persistence is enabled. This maintains consistency between volume mounts and volumes.
130-130: Properly closed conditional block.The closing of the
{{- if not .Values.main.statefulSet.enabled }}block ensures the template renders correctly only for Deployment mode, not for StatefulSet mode.charts/n8n/templates/_helpers.tpl (4)
63-63: Improved specificity in comment.The comment now clarifies that this PVC definition is specifically for the main component, which enhances code documentation.
72-72: Appropriate claim name suffix.Adding "-main" to the claim name improves clarity and prevents naming conflicts between different component PVCs.
76-100: Well-structured PVC definitions for webhook and worker components.The new PVC templates for webhook and worker components follow the same pattern as the main component, promoting consistency while allowing component-specific PVC configurations. This is essential for managing persistent storage across different components independently.
129-139: Comprehensive validation to prevent data loss scenarios.The new validation function enforces important constraints:
- Deployments with persistent storage can only have one replica
- Webhooks with persistent storage can only have one replica
This prevents potential data corruption issues that could occur with multiple replicas using the same persistent volume.
The error messages are clear and helpful, providing instructions on how to resolve the invalid configuration.
charts/n8n/templates/deployment.webhook.yaml (5)
4-4: Good addition of validation for webhook deployments.Including the validation function ensures webhook deployments adhere to the same constraints as main deployments, preventing invalid configurations.
36-36: More comprehensive checksum calculation.The updated checksum calculation now includes all values instead of just webhook and main values. This is more robust as it will trigger pod restarts when any relevant configuration changes.
60-61: Added missing security context for webhook container.Adding the security context to the webhook container ensures consistent security settings across all deployment types. This is important for maintaining proper pod security.
115-119: Well-structured conditional volume mounting for webhook.The conditional volume mount configuration ensures that volume mounts are only included when persistence is enabled, which is consistent with the approach used in the main deployment.
140-143: Component-specific PVC template.Using the webhook-specific PVC template function (
n8n.webhook.pvc) instead of the generic one improves clarity and ensures webhook deployments use their dedicated persistent volumes.README.md (3)
4-4: Fixed typo in popularity.The typo has been corrected to improve readability and professionalism of the documentation.
14-16: Important licensing information added.This warning clearly communicates the licensing requirements for using StatefulSet with multiple replicas, which is crucial information for users planning their deployment strategy.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~16-~16: You might be missing the article “an” here.
Context: ...uires n8n Enterprise license. > Without Enterprise license, use Deployment with a single m...(AI_EN_LECTOR_MISSING_DETERMINER_AN)
364-485: Excellent documentation of deployment modes and high availability setup.The new sections provide comprehensive information about:
- The various deployment modes for main, worker, and webhook components
- The constraints and requirements for each mode
- YAML examples for different configurations
- High availability options and license requirements
This documentation significantly improves the usability of the chart by clearly explaining options and limitations.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
README.md (1)
15-17: Well-structured note about Enterprise license requirements.This important note provides critical information for users about licensing requirements for different deployment configurations. It helps prevent potential misconfigurations.
However, there's a minor grammatical issue - you're missing the article "an" before "Enterprise license" in line 17.
- Without Enterprise license, use Deployment with a single main replica. + Without an Enterprise license, use Deployment with a single main replica.🧰 Tools
🪛 LanguageTool
[uncategorized] ~17-~17: You might be missing the article “an” here.
Context: ...uires n8n Enterprise license. > Without Enterprise license, use Deployment with a single m...(AI_EN_LECTOR_MISSING_DETERMINER_AN)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
README.md(5 hunks)charts/n8n/Chart.yaml(1 hunks)charts/n8n/templates/_helpers.tpl(3 hunks)charts/n8n/templates/deployment.worker.yaml(3 hunks)charts/n8n/values.yaml(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- charts/n8n/Chart.yaml
- charts/n8n/values.yaml
🧰 Additional context used
🪛 LanguageTool
README.md
[uncategorized] ~17-~17: You might be missing the article “an” here.
Context: ...uires n8n Enterprise license. > Without Enterprise license, use Deployment with a single m...
(AI_EN_LECTOR_MISSING_DETERMINER_AN)
🪛 markdownlint-cli2 (0.17.2)
README.md
640-640: Multiple spaces after hash on atx style heading
null
(MD019, no-multiple-space-atx)
640-640: Headings must start at the beginning of the line
null
(MD023, heading-start-left)
641-641: Multiple spaces after hash on atx style heading
null
(MD019, no-multiple-space-atx)
641-641: Headings must start at the beginning of the line
null
(MD023, heading-start-left)
642-642: Multiple spaces after hash on atx style heading
null
(MD019, no-multiple-space-atx)
642-642: Headings must start at the beginning of the line
null
(MD023, heading-start-left)
643-643: Multiple spaces after hash on atx style heading
null
(MD019, no-multiple-space-atx)
643-643: Headings must start at the beginning of the line
null
(MD023, heading-start-left)
643-643: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
644-644: Multiple spaces after hash on atx style heading
null
(MD019, no-multiple-space-atx)
644-644: Headings must start at the beginning of the line
null
(MD023, heading-start-left)
645-645: Multiple spaces after hash on atx style heading
null
(MD019, no-multiple-space-atx)
645-645: Headings must start at the beginning of the line
null
(MD023, heading-start-left)
🪛 YAMLlint (1.35.1)
charts/n8n/templates/deployment.worker.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
🔇 Additional comments (8)
README.md (1)
365-487: Excellent documentation of deployment modes and high availability setup.This comprehensive addition significantly improves the documentation by:
- Clearly explaining the different deployment modes for each component
- Providing configuration examples for each scenario
- Documenting the limitations of each approach
- Explaining the enterprise licensing requirements
The YAML examples are particularly helpful for users to understand the practical implementation of each deployment mode.
charts/n8n/templates/deployment.worker.yaml (3)
1-1: Improved conditional logic for worker deployment.The condition has been enhanced to only create the worker deployment when not using StatefulSet mode, which aligns with the documented deployment modes in the README.
🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
111-115: Added conditional volume mounts for worker persistence.This change ensures that volume mounts are only defined when persistence is enabled, improving resource management and configuration clarity.
135-139: Updated persistent volume configuration for worker component.The volume definition now uses the worker-specific PVC template function and is properly conditioned on persistence being enabled. This ensures correct resource allocation and naming for the worker's persistent storage.
charts/n8n/templates/_helpers.tpl (4)
63-74: Improved clarity for main component PVC definition.The comment has been updated to specify this is for the "main component" and the claim name now includes "-main" suffix for better identification. This change enhances clarity and prevents potential naming conflicts between different components.
76-87: Added PVC definition for webhook component.This new template function follows the same pattern as the main component's PVC definition but is specific to the webhook component. This supports the deployment modes described in the README and ensures proper resource management.
89-100: Added PVC definition for worker component.This template function creates a dedicated PVC for the worker component, following the same pattern as the other components. This separation of concerns ensures proper resource isolation between components.
130-140: Added valuable validation function to prevent misconfiguration.This validation function is an excellent addition that:
- Prevents invalid configurations that could lead to data loss
- Provides clear, informative error messages
- Offers alternative configurations when an invalid setup is detected
The validation covers two key scenarios:
- Main component with persistence using Deployment (non-StatefulSet) must have only one replica
- Webhook component with persistence must have only one replica
This helps users avoid potential data corruption issues and improves the overall user experience.
|
@Vad1mo what do you think about this PR? |
|
I need some more time to review this.. |
|
Isn't this an anti-pattern to n8n high availability setup? According to n8n official documentation about scaling it suggests to use redis instead, are there any benefits of using persistent storage instead of redis for queuing ? Persistence could be useful if you are using one pod IMO |
|
@dominykasn This is just to persist community nodes between pod restarts. Redis is still used as the queue backend in queue mode — nothing changes there. |
There was a problem hiding this comment.
Pull Request Overview
This PR improves the deployment configuration by adding advanced validation and documentation to prevent misconfigurations that could lead to data loss when using persistent storage. Key changes include adding StatefulSet options for main and worker components, updating PVC naming conventions and validation logic, and enhancing the README with detailed deployment mode examples.
Reviewed Changes
Copilot reviewed 9 out of 10 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| charts/n8n/values.yaml | Added StatefulSet options for main and worker configurations |
| charts/n8n/templates/statefulset*.yaml | Introduced StatefulSet templates for main and worker components |
| charts/n8n/templates/pvc.yaml | Updated pvc names and added conditions for webhook and worker PVCs |
| charts/n8n/templates/deployment*.yaml | Applied persistence validations and adjusted volume mounts/volumes |
| charts/n8n/Chart.yaml | Bumped chart version and updated change annotations |
| README.md | Expanded deployment documentation with clear examples |
Files not reviewed (1)
- charts/n8n/templates/_helpers.tpl: Language not supported
There was a problem hiding this comment.
Pull Request Overview
This PR improves deployment configuration options for n8n by introducing StatefulSet support, enhancing persistence validation, and updating the documentation accordingly.
- Added StatefulSet configuration options for main and worker components.
- Integrated enhanced validation and PVC naming for main, webhook, and worker components.
- Expanded documentation to detail deployment modes and persistence limitations.
Reviewed Changes
Copilot reviewed 9 out of 10 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| charts/n8n/values.yaml | Added StatefulSet options for main and worker; commented extra env vars. |
| charts/n8n/templates/statefulset.yaml | Introduced a new StatefulSet template for the main component. |
| charts/n8n/templates/statefulset.worker.yaml | Introduced a new StatefulSet template for the worker component. |
| charts/n8n/templates/pvc.yaml | Updated PVC naming conventions and added new blocks for webhook and worker. |
| charts/n8n/templates/deployment.yaml | Wrapped deployment configuration with conditions for persistence. |
| charts/n8n/templates/deployment.worker.yaml | Added worker deployment support with statefulSet conditional check and worker PVC inclusion. |
| charts/n8n/templates/deployment.webhook.yaml | Updated webhook validation and persistence conditionals. |
| charts/n8n/Chart.yaml | Bumped chart version and updated annotations. |
| README.md | Expanded documentation with detailed deployment mode examples. |
Files not reviewed (1)
- charts/n8n/templates/_helpers.tpl: Language not supported
Comments suppressed due to low confidence (1)
charts/n8n/templates/statefulset.worker.yaml:53
- [nitpick] In the statefulSet worker template, environment variables are sourced from '.Values.main.config' and '.Values.main.secret' which differs from the deployment.worker.yaml template that uses worker-specific config. Consider reviewing if this behavior is intentional, or if worker-specific config values should be used to improve consistency.
envFrom:
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
charts/n8n/templates/deployment.worker.yaml (2)
33-33: Refine Pod annotation checksum scope
Hashing the entire.Values.workerobject can trigger unnecessary rollouts on any change (e.g. resource tweaks or extraEnv). Consider limiting the checksum to only the configuration data that should drive a restart, for example the content ofworker.configor a dedicated subset:- checksum/config: {{ print .Values.worker | sha256sum }} + checksum/worker-config: {{ toYaml .Values.worker.config | sha256sum }}
111-115: Add explicit readOnly flag and subPath for data mount
The persistence mount is in place, but it may be safer to:
- Declare
readOnly: falseexplicitly.- Use
subPathif only a specific directory needs to be mounted, avoiding conflicts when the underlying PVC has extra files.- name: data mountPath: /home/node/.n8n readOnly: false # subPath: <optional-directory>charts/n8n/templates/statefulset.worker.yaml (3)
25-25: Refine StatefulSet Pod annotation checksum scope
Similar to the Deployment, consider narrowing the checksum input to only the relevant config block rather than the entireworkerobject to avoid spurious rolling updates:- checksum/config: {{ print .Values.worker | sha256sum }} + checksum/worker-config: {{ toYaml .Values.worker.config | sha256sum }}
103-107: Add explicit readOnly flag and subPath for StatefulSet mounts
Mirroring the Deployment, you may want to specifyreadOnly: falseand optionally usesubPathfor the volumeMount in the StatefulSet to prevent overwriting unrelated paths:- name: data mountPath: /home/node/.n8n readOnly: false # subPath: <optional-directory>
127-146: Enhance PVC templates for better traceability
The volumeClaimTemplates block is solid, but adding component labels can help with PVC management and filtering in Kubernetes:volumeClaimTemplates: - metadata: name: data + labels: + app.kubernetes.io/component: worker + app.kubernetes.io/instance: {{ include "n8n.fullname" . }} spec: accessModes: {{- range .Values.worker.persistence.accessModes }}This improves visibility when inspecting PVCs in multi-component charts.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
charts/n8n/templates/deployment.webhook.yaml(5 hunks)charts/n8n/templates/deployment.worker.yaml(4 hunks)charts/n8n/templates/statefulset.worker.yaml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- charts/n8n/templates/deployment.webhook.yaml
🧰 Additional context used
🪛 YAMLlint (1.35.1)
charts/n8n/templates/deployment.worker.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
charts/n8n/templates/statefulset.worker.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
[warning] 9-9: wrong indentation: expected 2 but found 4
(indentation)
[warning] 10-10: wrong indentation: expected 2 but found 4
(indentation)
🔇 Additional comments (3)
charts/n8n/templates/deployment.worker.yaml (2)
1-1: Conditional rendering of worker Deployment is correct
Theifguard ensures the worker Deployment is only created when StatefulSet mode is disabled. This aligns with the intended single–mode activation per component.🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
135-139: Leverage PVC helper for worker Deployment
Great use of then8n.worker.pvchelper here to keep PVC definitions consistent. This ensures that any PVC spec changes propagate uniformly across templates.charts/n8n/templates/statefulset.worker.yaml (1)
1-3: Conditional rendering of worker StatefulSet is correct
The chart will only render this StatefulSet when bothworker.enabledandworker.statefulSet.enabledare true. This cleanly separates Deployment vs. StatefulSet modes.🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
charts/n8n/templates/statefulset.worker.yaml (3)
62-74: Simplify extra environment variables handling
Theenv: {{ not (empty .Values.worker.extraEnv) | ternary nil "[]" }}expression is clever but hard to read. Consider using a more straightforward block:env: {{- if .Values.worker.extraEnv }} {{ toYaml .Values.worker.extraEnv | indent 12 }} {{- else }} [] {{- end }}
89-99: Reuse of main probes—consider worker-specific probes
Currently, liveness and readiness probes use.Values.main.*Probe. If users need different health checks for worker pods, adding.Values.worker.livenessProbeand.Values.worker.readinessProbecould be beneficial.
127-146: Add metadata labels to PVC templates for consistency
To make PersistentVolumeClaims easier to identify and manage, consider adding standard labels undermetadata. For example:volumeClaimTemplates: - metadata: name: data + labels: + app.kubernetes.io/name: {{ include "n8n.name" . }} + app.kubernetes.io/instance: {{ .Release.Name }} ...
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
charts/n8n/templates/statefulset.worker.yaml(1 hunks)
🧰 Additional context used
🪛 YAMLlint (1.35.1)
charts/n8n/templates/statefulset.worker.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
[warning] 9-9: wrong indentation: expected 2 but found 4
(indentation)
[warning] 10-10: wrong indentation: expected 2 but found 4
(indentation)
🔇 Additional comments (9)
charts/n8n/templates/statefulset.worker.yaml (9)
1-4: Conditional rendering for StatefulSet is correct
Usingand .Values.worker.enabled .Values.worker.statefulSet.enabledensures this resource is only created when both flags are set.🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
5-14: Metadata labels and annotations structure is well-formed
The use ofn8n.labels, optionaldeploymentLabels, anddeploymentAnnotationsis consistent with other templates.🧰 Tools
🪛 YAMLlint (1.35.1)
[warning] 9-9: wrong indentation: expected 2 but found 4
(indentation)
[warning] 10-10: wrong indentation: expected 2 but found 4
(indentation)
15-22: Replica count key is consistent
replicas: {{ .Values.worker.replicaCount }}aligns with the Deployment template, avoiding the previous mismatch.
23-34: Pod template annotations and labels look good
Checksum annotation will trigger rolling updates on config changes, and selector labels are applied correctly.
35-46: Pull secrets and init containers handling is appropriate
Leveragingwithandtplfor dynamic definitions keeps the code DRY and flexible.
75-88: Default command and args are appropriate
Providing sensible defaults (["n8n", "worker", "--concurrency=..."]) while allowing overrides balances usability and flexibility.
100-110: Resource limits and persistence volume mounts are correct
Resources,volumeMounts, andextraVolumeMountsfollow the established pattern.
111-126: Node scheduling and tolerations are properly supported
The inclusion ofhostAliases,nodeSelector,affinity, andtolerationsgives users full control over pod scheduling.
147-148: Conditional block closed correctly
The final{{- end }}properly terminates the initialif.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
charts/n8n/templates/statefulset.worker.yaml (2)
75-88: Factor command/args logic into a helper
The templating forcommandandargsis duplicated between Deployment and StatefulSet templates. Extracting this into a partial (e.g.,_command.worker.tpl) would reduce duplication and simplify maintenance.
127-146: Allow customizing StatefulSet update strategy and pod management
Currently, the StatefulSet uses defaultRollingUpdateandOrderedReadypolicies. You may want to exposeupdateStrategyandpodManagementPolicyoptions invalues.yamlfor advanced use cases. For example:worker: statefulSet: updateStrategy: type: RollingUpdate rollingUpdate: partition: 0 podManagementPolicy: Parallel
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
charts/n8n/templates/statefulset.worker.yaml(1 hunks)
🧰 Additional context used
🪛 YAMLlint (1.35.1)
charts/n8n/templates/statefulset.worker.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
[warning] 9-9: wrong indentation: expected 2 but found 4
(indentation)
[warning] 10-10: wrong indentation: expected 2 but found 4
(indentation)
🔇 Additional comments (3)
charts/n8n/templates/statefulset.worker.yaml (3)
16-16: Consistent use ofreplicaCountkey
Good catch aligning the StatefulSet to use.Values.worker.replicaCount, matching the Deployment. This resolves the previous mismatch and avoids confusion.
17-17: Verify headless Service exists for StatefulSet
TheserviceNamereferences{{ include "n8n.fullname" . }}-worker. Ensure there’s a corresponding headless Service defined (i.e.,ClusterIP: None) with the same name, so the StatefulSet pods have stable network identities.
93-99: Confirm probe configuration scope
This template usesmain.livenessProbeandmain.readinessProbefor the worker container. If you intended separate probes for workers, you might want.Values.worker.livenessProbeand.Values.worker.readinessProbe. Please verify.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (6)
charts/n8n/templates/deployment.worker.yaml (2)
34-34: Consider expanding checksum to include main config
Rolling restarts trigger only on changes to.Values.worker. If updates to.Values.main.configor.Values.main.secretshould also restart worker pods, combine both before hashing:checksum/config: {{ print (dict "main" .Values.main "worker" .Values.worker) | sha256sum }}
112-116: Conditional volume mount for worker persistence
The mount at/home/node/.n8nis rendered only whenworker.persistence.enabled, which is correct. To reduce duplication across templates, consider extracting this mount snippet into a Helm helper.charts/n8n/templates/pvc.yaml (2)
2-7: Distinct naming for main PVC
Appending-mainto the PVC name avoids claim collisions across components. Ensure that any examples invalues.yamlandREADME.mdreference this updated claim name.
32-60: Add component enable check to webhook PVC
The webhook PVC is gated bywebhook.persistence.enabled, but doesn’t check if the webhook component is actually enabled. To prevent orphan PVCs when a user disables the webhook, consider:{{- if and .Values.webhook.enabled .Values.webhook.persistence.enabled (not .Values.webhook.persistence.existingClaim) }}charts/n8n/templates/statefulset.worker.yaml (2)
26-26: Consider expanding checksum to include main config
Thechecksum/configannotation covers only.Values.worker. To ensure changes in main config or secret also trigger rolling updates, you could do:checksum/config: {{ print (dict "main" .Values.main "worker" .Values.worker) | sha256sum }}
104-108: Conditional volume mount for StatefulSet worker
The volume mount for persistence is correctly scoped underworker.persistence.enabled. For consistency, you might reuse the same helper or snippet as in the Deployment template.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
charts/n8n/templates/deployment.worker.yaml(4 hunks)charts/n8n/templates/hpa.yaml(1 hunks)charts/n8n/templates/pvc.yaml(2 hunks)charts/n8n/templates/statefulset.worker.yaml(1 hunks)charts/n8n/templates/statefulset.yaml(1 hunks)
🧰 Additional context used
🪛 YAMLlint (1.37.1)
charts/n8n/templates/statefulset.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
[warning] 10-10: wrong indentation: expected 2 but found 4
(indentation)
[warning] 11-11: wrong indentation: expected 2 but found 4
(indentation)
charts/n8n/templates/pvc.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
charts/n8n/templates/deployment.worker.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
charts/n8n/templates/statefulset.worker.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
[warning] 10-10: wrong indentation: expected 2 but found 4
(indentation)
[warning] 11-11: wrong indentation: expected 2 but found 4
(indentation)
🔇 Additional comments (10)
charts/n8n/templates/hpa.yaml (1)
1-3: Validation integration in HPA template
You've correctly added{{ include "n8n.validateValues" . }}to enforce the new configuration rules. Since this HPA targets a Deployment, please verify thatn8n.validateValuesrejects combinations ofstatefulSet.enabledwithmain.autoscaling.enabled(HPA isn’t wired to scale StatefulSets).🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
charts/n8n/templates/deployment.worker.yaml (2)
1-2: Invoke configuration validation for worker Deployment
Adding{{ include "n8n.validateValues" . }}under the Deployment guard ensures invalid combos (e.g., PVC with multi-replica) are caught early.🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
136-140: Render worker PVC using helper
Including{{ include "n8n.worker.pvc" . }}ensures consistent claim specs. Please verify the indentation aligns with the surroundingvolumes:block to avoid YAML parse issues.charts/n8n/templates/pvc.yaml (2)
1-1: Invoke validation for PVC resources
Adding{{ include "n8n.validateValues" . }}at the top ensures invalid persistence or replica configurations are flagged before PVC creation.🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
62-90: Worker PVC condition respects deployment mode
The guard ensures a PVC is created only when the worker is enabled, persistence is enabled, and StatefulSet is disabled—aligning with your new validation rules.charts/n8n/templates/statefulset.yaml (2)
1-3: Invoke validation for main StatefulSet
Placing{{ include "n8n.validateValues" . }}at the top catches misconfigurations (e.g., StatefulSet with disallowed replica/persistence combos) before rendering.🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
17-19: Conditional replica count for StatefulSet
You correctly emitreplicas: .Values.main.replicaCountonly when autoscaling is disabled, since HPA isn’t wired to StatefulSets. Ensure thatn8n.validateValuesalso prohibitsmain.autoscaling.enabledwhenstatefulSet.enabledis true to prevent silent scaling mismatches.charts/n8n/templates/statefulset.worker.yaml (3)
1-2: Invoke validation for worker StatefulSet
Adding{{ include "n8n.validateValues" . }}at the very top enforces your deployment constraints early.🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
17-17: Consistent replica count key
Using.Values.worker.replicaCountmatches the Deployment template and avoids confusion with alternate keys.
128-147: Define volumeClaimTemplates for worker StatefulSet persistence
ThevolumeClaimTemplatesblock accurately mirrors the main StatefulSet’s PVC spec, including storage class handling and size defaults.
|
@Vad1mo up |
sorry, a bit delayed, can you rebase and I'll have a look at this tonight. |
aleksandrovpa
force-pushed
the
feature/main-statefulset-support
branch
from
f614ebb to
8db7f80
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
charts/n8n/templates/deployment.worker.yaml (1)
1-2: Ensure valid YAML after validation include.This template also starts with
{{- include "n8n.validateValues" . }}. As above, verify withhelm lintthat no YAML syntax issues arise due to trimming.🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
🧹 Nitpick comments (5)
charts/n8n/templates/deployment.worker.yaml (1)
34-34: Scope hashed config annotation.Currently,
checksum/configuses the entire.Values.workerobject, which may trigger pod restarts on unrelated value changes. Consider narrowing the hash input to only the keys affecting pod configuration (e.g.,config,secret,lifecycle,extraEnv,persistence).charts/n8n/templates/statefulset.yaml (2)
27-31: Scope hashed config annotation.The
checksum/configannotation uses the full.Values.mainobject. To avoid unnecessary rollouts on unrelated changes, consider hashing only the subset of values that actually influence the pod spec.
116-135: Extract volumeClaimTemplates duplication.The
volumeClaimTemplatesblock duplicates PVC spec logic. Consider abstracting this into a helper or usingtplto reduce redundancy and improve maintainability.README.md (1)
14-16: Grammar: include missing article.Change
requires n8n Enterprise licensetorequires an n8n Enterprise licensefor clarity.🧰 Tools
🪛 LanguageTool
[uncategorized] ~16-~16: You might be missing the article “an” here.
Context: ...uires n8n Enterprise license. > Without Enterprise license, use Deployment with a single m...(AI_EN_LECTOR_MISSING_DETERMINER_AN)
charts/n8n/templates/statefulset.worker.yaml (1)
24-35: Enhance rolling restart triggers by expanding checksum scopeCurrently only
.Values.workeris hashed for pod restarts. Updates to the main config or secrets won’t trigger a rollout. Consider:- checksum/config: {{ print .Values.worker | sha256sum }} + checksum/config: {{ print (dict "main" .Values.main "worker" .Values.worker) | sha256sum }}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
README.md(5 hunks)charts/n8n/Chart.yaml(1 hunks)charts/n8n/templates/_helpers.tpl(3 hunks)charts/n8n/templates/deployment.webhook.yaml(5 hunks)charts/n8n/templates/deployment.worker.yaml(4 hunks)charts/n8n/templates/deployment.yaml(3 hunks)charts/n8n/templates/hpa.yaml(1 hunks)charts/n8n/templates/pvc.yaml(2 hunks)charts/n8n/templates/statefulset.worker.yaml(1 hunks)charts/n8n/templates/statefulset.yaml(1 hunks)charts/n8n/values.yaml(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- charts/n8n/Chart.yaml
🚧 Files skipped from review as they are similar to previous changes (4)
- charts/n8n/templates/hpa.yaml
- charts/n8n/values.yaml
- charts/n8n/templates/deployment.webhook.yaml
- charts/n8n/templates/_helpers.tpl
🧰 Additional context used
🪛 YAMLlint (1.37.1)
charts/n8n/templates/deployment.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
charts/n8n/templates/deployment.worker.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
charts/n8n/templates/pvc.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
charts/n8n/templates/statefulset.worker.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
[warning] 10-10: wrong indentation: expected 2 but found 4
(indentation)
[warning] 11-11: wrong indentation: expected 2 but found 4
(indentation)
charts/n8n/templates/statefulset.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
[warning] 10-10: wrong indentation: expected 2 but found 4
(indentation)
[warning] 11-11: wrong indentation: expected 2 but found 4
(indentation)
🪛 LanguageTool
README.md
[uncategorized] ~16-~16: You might be missing the article “an” here.
Context: ...uires n8n Enterprise license. > Without Enterprise license, use Deployment with a single m...
(AI_EN_LECTOR_MISSING_DETERMINER_AN)
🪛 markdownlint-cli2 (0.17.2)
README.md
639-639: Multiple spaces after hash on atx style heading
null
(MD019, no-multiple-space-atx)
639-639: Headings must start at the beginning of the line
null
(MD023, heading-start-left)
640-640: Multiple spaces after hash on atx style heading
null
(MD019, no-multiple-space-atx)
640-640: Headings must start at the beginning of the line
null
(MD023, heading-start-left)
641-641: Multiple spaces after hash on atx style heading
null
(MD019, no-multiple-space-atx)
641-641: Headings must start at the beginning of the line
null
(MD023, heading-start-left)
642-642: Multiple spaces after hash on atx style heading
null
(MD019, no-multiple-space-atx)
642-642: Headings must start at the beginning of the line
null
(MD023, heading-start-left)
642-642: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
643-643: Multiple spaces after hash on atx style heading
null
(MD019, no-multiple-space-atx)
643-643: Headings must start at the beginning of the line
null
(MD023, heading-start-left)
644-644: Multiple spaces after hash on atx style heading
null
(MD019, no-multiple-space-atx)
644-644: Headings must start at the beginning of the line
null
(MD023, heading-start-left)
🔇 Additional comments (24)
charts/n8n/templates/deployment.yaml (3)
1-2: Ensure valid YAML after validation include.The
{{- include "n8n.validateValues" . }}call at the top may strip necessary newlines, potentially causing YAML parse errors. Please verify the generated template withhelm lintand consider ensuring a blank line after the include.🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
99-105: Verify conditional volumeMounts indentation.The
volumeMountsandextraVolumeMountsblocks are conditionally rendered. Please runhelm templateto confirm the indentation aligns correctly under the container spec and that the extra mounts merge as expected.
122-126: Verify volumes block indentation.Ensure the
volumessection (withn8n.pvc) is properly indented underspec.template.specwhen.Values.main.persistence.enabledis true. A quickhelm templatecheck should catch any misalignment.charts/n8n/templates/deployment.worker.yaml (2)
112-116: Verify worker volumeMounts indentation.The conditional
volumeMountsfor worker persistence must align under the container spec. Please confirm viahelm templatethat the mount is correctly rendered.
136-140: Verify worker volumes block indentation.Ensure the worker
volumessection (usingn8n.worker.pvc) is indented correctly underspecwhen persistence is enabled. Ahelm templatepass will validate this.charts/n8n/templates/pvc.yaml (4)
1-2: Validation helper inclusion is correctly placed.The call to
n8n.validateValuesat the top ensures constraints are enforced before any PVC creation. This aligns with other templates.🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
6-6: Main PVC naming updated.Appending
-mainto the PVC name matches the naming convention in your StatefulSet and Deployment templates.
32-42: Webhook PVC conditional block is correct.The webhook PVC is created only when persistence is enabled and no existing claim is specified. This matches the documented behavior.
62-75: Worker PVC conditional block is correct.The worker PVC is gated on
worker.enabled,worker.persistence.enabled, no existing claim, and no StatefulSet. This aligns with your worker StatefulSet implementation.charts/n8n/templates/statefulset.yaml (2)
1-2: Validation helper inclusion is correctly placed.Calling
n8n.validateValuesbefore rendering the StatefulSet enforces your deployment constraints upfront.🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
92-99: Verify StatefulSet volumeMounts indentation.Confirm via
helm templatethat thevolumeMountsfor persistence are rendered correctly within the container spec, especially whenextraVolumeMountsare provided.charts/n8n/templates/statefulset.worker.yaml (13)
1-2: Validation invocation placement is correctThe inclusion of
{{ include "n8n.validateValues" . }}at the top ensures invalid PVC+replica configurations are caught before rendering.🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
3-6: StatefulSet API and metadata name are accurate
apiVersion,kind, andmetadata.namecorrectly derive from chart helpers and follow Kubernetes conventions.
7-15: Metadata labels and annotations handling is solidOptional
deploymentLabelsunderlabelsanddeploymentAnnotationsat the correct metadata level are conditionally rendered with proper indentation.🧰 Tools
🪛 YAMLlint (1.37.1)
[warning] 10-10: wrong indentation: expected 2 but found 4
(indentation)
[warning] 11-11: wrong indentation: expected 2 but found 4
(indentation)
16-23: Replica count and selector configuration is consistentUsing
.Values.worker.replicaCountaligns with the deployment template, and the label selector correctly matches the worker type.
36-47: Pod spec security and init containers are well-defined
imagePullSecrets,serviceAccountName,securityContext, and optionalinitContainersare correctly templated and scoped.
48-70: Container spec and envFrom configuration is correctThe main and worker config maps and secrets are conditionally mounted via
envFrom, ensuring environment variables are sourced appropriately.
71-79: Skip: env and command args are standardConfiguration of
extraEnv,command, andargsfollows established patterns and requires no adjustments.
80-89: Default command and arguments are correctly setThe fallback
["n8n"]command and theworker --concurrencyargs align with expected behavior for the worker process.
90-101: Networking and health probes are properly configuredPort settings, livenessProbe, and readinessProbe correctly reference values from both worker and main sections.
102-111: Resource and volume mount definitions look goodResource requests/limits and conditional mounts for persistence and extra volumes are implemented correctly.
112-127: Additional pod scheduling and affinity rules are well-handled
hostAliases,nodeSelector,affinity, andtolerationsblocks are conditionally rendered according to Helm best practices.
128-147: Persistent Volume Claim templates are properly defined
volumeClaimTemplatescorrectly useaccessModes,resources.requests, and handlestorageClassedge cases.
148-149: Conditional block closure is correctThe closing
{{- end }}properly matches the initial statefulSet-enabled conditional.
|
@Vad1mo done, please check this out |
|
@Vad1mo up 😊 |
|
You have broken the README.md file, can you take a closer look into it again?
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
README.md (5)
361-401: Consolidate overlapping sectionsThe "Deployment Modes" (lines 361–401) and "High Availability Setup" (lines 455–485) largely repeat the same configuration guidance. To avoid duplication and simplify maintenance, consider merging these into a single section or cross-linking between them.
Also applies to: 455-485
681-682: Convert heading to link text and remove trailing punctuationAvoid using a bare URL and ending a heading with a period. Change this to a normal paragraph with a Markdown link, for example:
See [Issue #39 comment](https://github.com/8gears/n8n-helm-chart/issues/39#issuecomment-1579991754) for the full explanation
683-683: Remove trailing period in headingHeadings should not end with punctuation. Update this to:
### Webhook processes rely on Valkey/Redis too
866-866: Hyphenate compound adjectiveFor grammatical accuracy, change the heading to:
### User-defined supplementary K8s manifests
902-902: Use descriptive link instead of bare URLReplace the bare URL with a Markdown link, for example:
See [Bitnami Valkey Helm chart](https://artifacthub.io/packages/helm/bitnami/valkey) for more details.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
README.md(6 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
README.md
681-681: Trailing punctuation in heading
Punctuation: '.'
(MD026, no-trailing-punctuation)
681-681: Bare URL used
null
(MD034, no-bare-urls)
683-683: Trailing punctuation in heading
Punctuation: '.'
(MD026, no-trailing-punctuation)
902-902: Bare URL used
null
(MD034, no-bare-urls)
🪛 LanguageTool
README.md
[uncategorized] ~866-~866: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ... tolerations: [] affinity: {} ``` ### User defined supplementary K8s manifests Takes a li...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
aleksandrovpa
force-pushed
the
feature/main-statefulset-support
branch
from
7ca0cff to
b39617c
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
README.md (1)
407-409: Inconsistent replica key (countvsreplicaCount) remains unresolvedPrevious feedback pointed out that using
counthere while the rest of the chart (and docs below) usereplicaCountis confusing. The mismatch still exists and may lead to mis-configuration.- count: 2 + replicaCount: 2
🧹 Nitpick comments (2)
README.md (2)
683-683: Remove trailing period from heading to satisfy markdownlint (MD026)The period at the end of the heading violates MD026 and breaks some TOC generators.
-#### See https://github.com/8gears/n8n-helm-chart/issues/39#issuecomment-1579991754 for the full explanation. +#### See https://github.com/8gears/n8n-helm-chart/issues/39#issuecomment-1579991754 for the full explanation
901-903: Convert bare URL to Markdown link to silence MD034Wrapping the Valkey URL in a proper Markdown link improves readability and lints cleanly.
-### Bitnami Valkey configuration - https://artifacthub.io/packages/helm/bitnami/valkey +### Bitnami Valkey configuration +[Bitnami Valkey chart](https://artifacthub.io/packages/helm/bitnami/valkey)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
README.md(6 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
README.md
681-681: Trailing punctuation in heading
Punctuation: '.'
(MD026, no-trailing-punctuation)
681-681: Bare URL used
(MD034, no-bare-urls)
683-683: Trailing punctuation in heading
Punctuation: '.'
(MD026, no-trailing-punctuation)
902-902: Bare URL used
(MD034, no-bare-urls)
@aleksandrovpa Thanks for the context! @Vad1mo The current implementation in main doesn't support that at all (see #204) and leads to a bunch of errors reported in #189 and #204, but I see it is actually addressed by this PR. |
|
@aleksandrovpa can you rebase this once more? Maybe this gets merged then? :) |
aleksandrovpa
force-pushed
the
feature/main-statefulset-support
branch
from
b39617c to
3010024
Compare
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (2)
README.md (1)
407-410: Inconsistent key name (countvsreplicaCount) – still unresolved
This was flagged earlier; please switch the example toreplicaCountor explicitly documentcountas an alias to avoid confusing users.charts/n8n/templates/statefulset.worker.yaml (1)
26-26: Checksum still ignoresmain.*values
Earlier feedback suggested hashing bothmainandworkersettings so that changes in the common config/secrets trigger a rolling restart. That recommendation is still applicable.
🧹 Nitpick comments (1)
charts/n8n/templates/statefulset.yaml (1)
92-100: Extra volume mounts disappear when persistence is off
volumeMounts:(and therefore anyextraVolumeMounts) are only rendered ifmain.persistence.enabledis true.
Users may want to mount config-only volumes while keeping the main container stateless. Consider movingvolumeMounts:outside of that condition, mirroring the fix proposed for the Deployment template.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
README.md(6 hunks)charts/n8n/templates/_helpers.tpl(3 hunks)charts/n8n/templates/deployment.webhook.yaml(5 hunks)charts/n8n/templates/deployment.worker.yaml(4 hunks)charts/n8n/templates/deployment.yaml(3 hunks)charts/n8n/templates/hpa.yaml(1 hunks)charts/n8n/templates/pvc.yaml(2 hunks)charts/n8n/templates/statefulset.worker.yaml(1 hunks)charts/n8n/templates/statefulset.yaml(1 hunks)charts/n8n/values.yaml(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- charts/n8n/templates/hpa.yaml
- charts/n8n/templates/deployment.webhook.yaml
- charts/n8n/templates/_helpers.tpl
- charts/n8n/values.yaml
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
README.md
681-681: Trailing punctuation in heading
Punctuation: '.'
(MD026, no-trailing-punctuation)
681-681: Bare URL used
(MD034, no-bare-urls)
683-683: Trailing punctuation in heading
Punctuation: '.'
(MD026, no-trailing-punctuation)
902-902: Bare URL used
(MD034, no-bare-urls)
🪛 YAMLlint (1.37.1)
charts/n8n/templates/deployment.worker.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
charts/n8n/templates/deployment.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
charts/n8n/templates/pvc.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
charts/n8n/templates/statefulset.worker.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
[warning] 10-10: wrong indentation: expected 2 but found 4
(indentation)
[warning] 11-11: wrong indentation: expected 2 but found 4
(indentation)
charts/n8n/templates/statefulset.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
[warning] 10-10: wrong indentation: expected 2 but found 4
(indentation)
[warning] 11-11: wrong indentation: expected 2 but found 4
(indentation)
🔇 Additional comments (1)
charts/n8n/templates/statefulset.worker.yaml (1)
17-18: Replica key may be out-of-sync withvalues.yamlThe template now expects
.Values.worker.replicaCount, whereas older versions (and previous reviews) referred toworker.count.
Double-check thatvalues.yaml, README examples and validation logic were updated accordingly; otherwise a default install will rendernulland Helm will fail.
|
@aleksandrovpa I just deployed your updated branch, works! :) |
It's awesome! |
Vad1mo
force-pushed
the
feature/main-statefulset-support
branch
from
b64554b to
f1e7f14
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (9)
charts/n8n/templates/deployment.yaml (2)
98-106:volumeMounts:key is emitted even when the list is empty → invalid manifestIf
main.persistence.enabled = falseand nomain.extraVolumeMountsare supplied, Helm still produces an emptyvolumeMounts:key, which K8s rejects.- volumeMounts: - {{- if .Values.main.persistence.enabled }} - - name: data - mountPath: /home/node/.n8n - {{- end }} - {{- if .Values.main.extraVolumeMounts }} - {{- toYaml .Values.main.extraVolumeMounts | nindent 12 }} - {{- end }} + {{- if or .Values.main.persistence.enabled .Values.main.extraVolumeMounts }} + volumeMounts: + {{- if .Values.main.persistence.enabled }} + - name: data + mountPath: /home/node/.n8n + {{- end }} + {{- if .Values.main.extraVolumeMounts }} +{{ toYaml .Values.main.extraVolumeMounts | nindent 12 }} + {{- end }} + {{- end }}
122-129:extraVolumesorphaned when persistence disabled
volumes:is created only inside the persistenceif, so with persistence disabled butmain.extraVolumesspecified, the items render without the parent key → broken YAML.- {{- if .Values.main.persistence.enabled }} - volumes: - - name: data -{{ include "n8n.pvc" . | nindent 10 }} - {{- end }} - {{- if .Values.main.extraVolumes }} - {{- toYaml .Values.main.extraVolumes | nindent 8 }} - {{- end }} + {{- if or .Values.main.persistence.enabled .Values.main.extraVolumes }} + volumes: + {{- if .Values.main.persistence.enabled }} + - name: data +{{ include "n8n.pvc" . | nindent 10 }} + {{- end }} + {{- if .Values.main.extraVolumes }} +{{ toYaml .Values.main.extraVolumes | nindent 8 }} + {{- end }} + {{- end }}charts/n8n/templates/deployment.worker.yaml (2)
112-119: EmptyvolumeMounts:when persistence off ➜ invalid YAMLSame condition/indentation issue as for the main deployment.
- {{- if .Values.worker.persistence.enabled }} - volumeMounts: - - name: data - mountPath: /home/node/.n8n - {{- end }} - {{- if .Values.worker.extraVolumeMounts }} - {{- toYaml .Values.worker.extraVolumeMounts | nindent 12 }} - {{- end }} + {{- if or .Values.worker.persistence.enabled .Values.worker.extraVolumeMounts }} + volumeMounts: + {{- if .Values.worker.persistence.enabled }} + - name: data + mountPath: /home/node/.n8n + {{- end }} + {{- if .Values.worker.extraVolumeMounts }} +{{ toYaml .Values.worker.extraVolumeMounts | nindent 12 }} + {{- end }} + {{- end }}
136-143:extraVolumesrendered without parent key when persistence disabledSame fix pattern as for the main deployment:
- {{- if .Values.worker.persistence.enabled }} - volumes: - - name: data -{{ include "n8n.worker.pvc" . | nindent 10 }} - {{- end }} - {{- if .Values.worker.extraVolumes }} - {{- toYaml .Values.worker.extraVolumes | nindent 8 }} - {{- end }} + {{- if or .Values.worker.persistence.enabled .Values.worker.extraVolumes }} + volumes: + {{- if .Values.worker.persistence.enabled }} + - name: data +{{ include "n8n.worker.pvc" . | nindent 10 }} + {{- end }} + {{- if .Values.worker.extraVolumes }} +{{ toYaml .Values.worker.extraVolumes | nindent 8 }} + {{- end }} + {{- end }}charts/n8n/templates/pvc.yaml (2)
30-34: Missing---separator before the second PVC → multi-document YAML invalid{{- end }} + +---
60-64: Add separator before the third PVC as well{{- end }} + +---README.md (1)
407-411: Replica key inconsistency:countvsreplicaCountThe worker example still shows
count: 2, all other sections usereplicaCount.
Align to avoid user confusion.- count: 2 + replicaCount: 2charts/n8n/templates/statefulset.worker.yaml (2)
86-89: Guard the--concurrencyflag to avoid an empty valueThis was raised earlier but is still present.
Ifworker.concurrencyis unset the rendered arg becomes--concurrency=which n8n treats as an error.- - "--concurrency={{ .Values.worker.concurrency }}" + {{- if .Values.worker.concurrency }} + - "--concurrency={{ .Values.worker.concurrency }}" + {{- end }}
104-111:extraVolumeMountscan render at the wrong level when persistence is disabledThe
volumeMounts:key is emitted only under the persistence guard, yet
extraVolumeMountsare appended unconditionally.
If users disable persistence but supply extra mounts, Helm will output a list
without its parent key and the manifest will be invalid.See previous suggestion; an abbreviated fix:
- {{- if .Values.worker.persistence.enabled }} - volumeMounts: - - name: data - mountPath: /home/node/.n8n - {{- end }} - {{- if .Values.worker.extraVolumeMounts }} - {{- toYaml .Values.worker.extraVolumeMounts | nindent 12 }} - {{- end }} + {{- if or .Values.worker.persistence.enabled .Values.worker.extraVolumeMounts }} + volumeMounts: + {{- if .Values.worker.persistence.enabled }} + - name: data + mountPath: /home/node/.n8n + {{- end }} + {{- if .Values.worker.extraVolumeMounts }} + {{- toYaml .Values.worker.extraVolumeMounts | nindent 12 }} + {{- end }} + {{- end }}
🧹 Nitpick comments (2)
charts/n8n/templates/deployment.yaml (1)
69-77: Minor indentation drift insideenvFromblockLines 73–76 are indented by one space less than the preceding list-items.
Not fatal, but Helm’s rendered YAML is harder to read and can trip YAML linters.charts/n8n/templates/statefulset.worker.yaml (1)
26-29: Includemain.*values in the checksum so workers roll after global config/secret changesRolling restarts are currently triggered only when
.Values.workerchanges.
Because the pod also consumesmain.configandmain.secret, edits there will not propagate until the StatefulSet is manually restarted.- checksum/config: {{ print .Values.worker | sha256sum }} + # include both main & worker scopes so any of them triggers a rollout + checksum/config: {{ print (dict "main" .Values.main "worker" .Values.worker) | sha256sum }}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
README.md(6 hunks)charts/n8n/Chart.yaml(1 hunks)charts/n8n/templates/_helpers.tpl(3 hunks)charts/n8n/templates/deployment.webhook.yaml(5 hunks)charts/n8n/templates/deployment.worker.yaml(4 hunks)charts/n8n/templates/deployment.yaml(3 hunks)charts/n8n/templates/hpa.yaml(1 hunks)charts/n8n/templates/pvc.yaml(2 hunks)charts/n8n/templates/statefulset.worker.yaml(1 hunks)charts/n8n/templates/statefulset.yaml(1 hunks)charts/n8n/values.yaml(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- charts/n8n/templates/hpa.yaml
- charts/n8n/Chart.yaml
- charts/n8n/templates/_helpers.tpl
- charts/n8n/values.yaml
- charts/n8n/templates/deployment.webhook.yaml
🧰 Additional context used
🪛 YAMLlint (1.37.1)
charts/n8n/templates/statefulset.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
[warning] 10-10: wrong indentation: expected 2 but found 4
(indentation)
[warning] 11-11: wrong indentation: expected 2 but found 4
(indentation)
charts/n8n/templates/deployment.worker.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
charts/n8n/templates/deployment.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
charts/n8n/templates/pvc.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
charts/n8n/templates/statefulset.worker.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
[warning] 10-10: wrong indentation: expected 2 but found 4
(indentation)
[warning] 11-11: wrong indentation: expected 2 but found 4
(indentation)
🪛 markdownlint-cli2 (0.17.2)
README.md
681-681: Trailing punctuation in heading
Punctuation: '.'
(MD026, no-trailing-punctuation)
681-681: Bare URL used
(MD034, no-bare-urls)
683-683: Trailing punctuation in heading
Punctuation: '.'
(MD026, no-trailing-punctuation)
902-902: Bare URL used
(MD034, no-bare-urls)
| {{- if .Values.main.persistence.enabled }} | ||
| volumeMounts: | ||
| - name: data | ||
| mountPath: /home/node/.n8n | ||
| {{- end }} | ||
| {{- if .Values.main.extraVolumeMounts }} | ||
| {{- toYaml .Values.main.extraVolumeMounts | nindent 12 }} | ||
| {{- end }} |
There was a problem hiding this comment.
🛠️ Refactor suggestion
volumeMounts: key gated by persistence but extraVolumeMounts not → breakage
Providing main.extraVolumeMounts while main.persistence.enabled=false yields dangling items.
Apply the same guard pattern used in the Deployment fix.
🤖 Prompt for AI Agents
In charts/n8n/templates/statefulset.yaml around lines 92 to 99, the volumeMounts
key is conditionally included only if persistence is enabled, but
extraVolumeMounts is included unconditionally, causing dangling volumeMount
entries when persistence is disabled. To fix this, wrap the extraVolumeMounts
block with the same conditional check for .Values.main.persistence.enabled to
ensure extraVolumeMounts are only added when persistence is enabled, preventing
invalid YAML structure.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.github/workflows/lint-test.yaml (1)
61-64: Great safeguard—consider extending the template check.The step correctly asserts that
main.deploymentMode=statefulsetrenders aStatefulSet. You may also want to cover:
worker.deploymentMode=statefulset- Ensuring only one StatefulSet is produced when persistence is disabled/enabled.
A quick enhancement:
- helm template n8n ./charts/n8n --set main.deploymentMode=statefulset | grep -q 'kind: StatefulSet' + helm template n8n ./charts/n8n --set main.deploymentMode=statefulset | grep -q 'kind: StatefulSet' + helm template n8n ./charts/n8n --set worker.deploymentMode=statefulset | grep -q 'kind: StatefulSet'Not critical, but it tightens coverage of the new modes.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/lint-test.yaml(2 hunks)
🔇 Additional comments (1)
.github/workflows/lint-test.yaml (1)
34-34: Why downgradehelm/chart-testing-actiontov2.7.0?The workflow previously used a
v3.xrelease, which contains numerous bug-fixes (e.g. OCI registry support, Go 1.20 runtime, flaky test fixes). Reverting tov2.7.0may silently drop those improvements and expose the pipeline to already-resolved issues.- uses: helm/[email protected] + uses: helm/[email protected] # or the latest v3 tagUnless you hit a regression in
v3.x, please keep the latest major version or pin to a specific patch ofv3to avoid unexpected behaviour.
|
@Vad1mo do you know who can ask to merge this? |
|
Hi @aleksandrovpa and @till , I attempted to do this using a custom n8n Docker image based on n8nio/n8n:1.103.0. Here's what I tried: However, it seems that the n8n-helm-chart overrides the /home/node/.n8n directory during deployment, so I’m unable to find the @mbakgun/n8n-nodes-slack-socket-mode package there afterward. n8n-helm-chart config |
Hi @viktym, I’ve tested this recently by adding a new community node via the Web UI, and I can confirm that it appears on both the main and worker instances when I check My setup is pretty straightforward without custom Docker images or init containers. Each pod has its own PVC mounted at |
|
@Vad1mo can we finally merge this PR? or you still have some concerns about that? |
|
@Vad1mo @RoseSecurity can we nudge this along? |
|
Gentle ping 🫣 |
|
I'll do my best to test this change this week. Just a crazy time of the year with vacations. Thanks for your patience and sorry for the delay! |
| {{- if .Values.worker.persistence.enabled }} | ||
| volumeMounts: | ||
| - name: data | ||
| mountPath: /home/node/.n8n |
There was a problem hiding this comment.
The mounting point should be in '/home/node/"
I encountered issues when I tried to install community npm packages; it was missing different folders, for example: .cache, .npm, etc
The same was in the main and webhook deployment
|
@aleksandrovpa would you kindly rebase one more time? and thank you for this feature 🙌🏼 @Vad1mo & @RoseSecurity, I hope you can find time to review this PR, as it is a much needed feature for anyone who wants to scale n8n components in prod 🙏🏼 |
aleksandrovpa
force-pushed
the
feature/main-statefulset-support
branch
from
cb9a69f to
f810e9b
Compare
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
README.md (1)
43-44: Fix heading: “Configurating” → “Configuring”Minor grammar improvement in a prominent heading.
-## Configurating N8n via Values and Environment Variables +## Configuring n8n via values and environment variables
♻️ Duplicate comments (10)
charts/n8n/templates/deployment.yaml (2)
99-106: Guard volumeMounts to avoid empty key and malformed YAMLIf both persistence and extraVolumeMounts are disabled/empty,
volumeMounts:renders with no items. Wrap the whole section behind a single condition and keep items inside, so the key is emitted only when needed.- volumeMounts: - {{- if .Values.main.persistence.enabled }} - - name: data - mountPath: /home/node/.n8n - {{- end }} - {{- if .Values.main.extraVolumeMounts }} - {{- toYaml .Values.main.extraVolumeMounts | nindent 12 }} - {{- end }} + {{- if or .Values.main.persistence.enabled .Values.main.extraVolumeMounts }} + volumeMounts: + {{- if .Values.main.persistence.enabled }} + - name: data + mountPath: /home/node/.n8n + {{- end }} + {{- if .Values.main.extraVolumeMounts }} +{{ toYaml .Values.main.extraVolumeMounts | nindent 12 }} + {{- end }} + {{- end }}
123-131: Fix volumes block; extraVolumes currently render without parent keyWhen persistence is disabled but
main.extraVolumesare set, list items are emitted without avolumes:parent. Emitvolumes:once and populate it conditionally.- {{- if .Values.main.persistence.enabled }} - volumes: - - name: "data" - {{ include "n8n.pvc" . }} - {{- end }} - {{- if .Values.main.extraVolumes }} - {{- toYaml .Values.main.extraVolumes | nindent 8 }} - {{- end }} + {{- if or .Values.main.persistence.enabled .Values.main.extraVolumes }} + volumes: + {{- if .Values.main.persistence.enabled }} + - name: data +{{ include "n8n.pvc" . | nindent 10 }} + {{- end }} + {{- if .Values.main.extraVolumes }} +{{ toYaml .Values.main.extraVolumes | nindent 8 }} + {{- end }} + {{- end }}charts/n8n/templates/deployment.worker.yaml (2)
113-120: Unify volumeMounts rendering to prevent invalid YAML when persistence is offCurrently
volumeMounts:is gated by persistence, butextraVolumeMountsare rendered independently. Emit the parent key only if one of the sources provides items.- {{- if .Values.worker.persistence.enabled }} - volumeMounts: - - name: data - mountPath: /home/node/.n8n - {{- end }} - {{- if .Values.worker.extraVolumeMounts }} - {{- toYaml .Values.worker.extraVolumeMounts | nindent 12 }} - {{- end }} + {{- if or .Values.worker.persistence.enabled .Values.worker.extraVolumeMounts }} + volumeMounts: + {{- if .Values.worker.persistence.enabled }} + - name: data + mountPath: /home/node/.n8n + {{- end }} + {{- if .Values.worker.extraVolumeMounts }} +{{ toYaml .Values.worker.extraVolumeMounts | nindent 12 }} + {{- end }} + {{- end }}
137-144: Emit a single volumes block; avoid dangling items without parent
extraVolumescan render without avolumes:key when persistence is disabled. Guard and populate one block.- {{- if .Values.worker.persistence.enabled }} - volumes: - - name: "data" - {{ include "n8n.worker.pvc" . }} - {{- end }} - {{- if .Values.worker.extraVolumes }} - {{- toYaml .Values.worker.extraVolumes | nindent 8 }} - {{- end }} + {{- if or .Values.worker.persistence.enabled .Values.worker.extraVolumes }} + volumes: + {{- if .Values.worker.persistence.enabled }} + - name: data +{{ include "n8n.worker.pvc" . | nindent 10 }} + {{- end }} + {{- if .Values.worker.extraVolumes }} +{{ toYaml .Values.worker.extraVolumes | nindent 8 }} + {{- end }} + {{- end }}charts/n8n/templates/deployment.webhook.yaml (1)
37-37: Scoped config checksum looks goodHashing only
.Values.webhookreduces unnecessary rollouts. Nice.charts/n8n/templates/pvc.yaml (1)
30-31: Separate multiple PVCs with YAML document delimitersWithout
---, K8s may reject combined objects. Insert separators before the 2nd and 3rd PVCs.{{- end }} +--- {{- if and .Values.webhook.persistence.enabled (not .Values.webhook.persistence.existingClaim) }} @@ {{- end }} +--- {{- if and .Values.worker.enabled .Values.worker.persistence.enabled (not .Values.worker.persistence.existingClaim) (not .Values.worker.statefulSet.enabled) }}Also applies to: 60-61, 90-91
charts/n8n/templates/statefulset.yaml (1)
92-99: Fix volumeMounts/extraVolumeMounts compositionPrevent orphaned
extraVolumeMountswhen persistence is disabled by emitting a single guardedvolumeMounts:block.- {{- if .Values.main.persistence.enabled }} - volumeMounts: - - name: data - mountPath: /home/node/.n8n - {{- end }} - {{- if .Values.main.extraVolumeMounts }} - {{- toYaml .Values.main.extraVolumeMounts | nindent 12 }} - {{- end }} + {{- if or .Values.main.persistence.enabled .Values.main.extraVolumeMounts }} + volumeMounts: + {{- if .Values.main.persistence.enabled }} + - name: data + mountPath: /home/node/.n8n + {{- end }} + {{- if .Values.main.extraVolumeMounts }} +{{ toYaml .Values.main.extraVolumeMounts | nindent 12 }} + {{- end }} + {{- end }}charts/n8n/templates/statefulset.worker.yaml (2)
86-89: Skip empty --concurrency flagAvoid rendering
--concurrency=when unset.- - "worker" - - "--concurrency={{ .Values.worker.concurrency }}" + - "worker" + {{- if .Values.worker.concurrency }} + - "--concurrency={{ .Values.worker.concurrency }}" + {{- end }}
104-111: Guard volumeMounts and include extra mounts under one keyPrevent orphaned mounts when persistence is off.
- {{- if .Values.worker.persistence.enabled }} - volumeMounts: - - name: data - mountPath: /home/node/.n8n - {{- end }} - {{- if .Values.worker.extraVolumeMounts }} - {{- toYaml .Values.worker.extraVolumeMounts | nindent 12 }} - {{- end }} + {{- if or .Values.worker.persistence.enabled .Values.worker.extraVolumeMounts }} + volumeMounts: + {{- if .Values.worker.persistence.enabled }} + - name: data + mountPath: /home/node/.n8n + {{- end }} + {{- if .Values.worker.extraVolumeMounts }} +{{ toYaml .Values.worker.extraVolumeMounts | nindent 12 }} + {{- end }} + {{- end }}README.md (1)
408-416: Standardize on replicaCount for worker example
countis inconsistent withreplicaCountused elsewhere and in values. Use one key to avoid misconfiguration.worker: enabled: true - count: 2 + replicaCount: 2 statefulSet: enabled: true # or false for Deployment
🧹 Nitpick comments (4)
charts/n8n/templates/deployment.worker.yaml (1)
35-35: Consider hashing both main and worker values for restart intentWorker also consumes
main.config/secret. Include both to trigger expected rollouts on config changes.- checksum/config: {{ print .Values.worker | sha256sum }} + checksum/config: {{ print (dict "main" .Values.main "worker" .Values.worker) | sha256sum }}charts/n8n/templates/_helpers.tpl (1)
133-140: Tighten main replica/persistence validationAdd
main.persistence.enabledto prevent false positives whentype != emptyDirbut persistence is disabled.-{{- if and (not .Values.main.statefulSet.enabled) (gt (int .Values.main.replicaCount) 1) (ne .Values.main.persistence.type "emptyDir") -}} +{{- if and (not .Values.main.statefulSet.enabled) (gt (int .Values.main.replicaCount) 1) .Values.main.persistence.enabled (ne .Values.main.persistence.type "emptyDir") -}}charts/n8n/templates/statefulset.yaml (1)
79-81: Align port lookup with DeploymentStatefulSet reads
main.config.port, while Deployment readsmain.config.n8n.port. Use the same source for consistency.- containerPort: {{ get (default (dict) .Values.main.config) "port" | default 5678 }} + containerPort: {{ get (default (dict) .Values.main.config.n8n) "port" | default 5678 }}charts/n8n/templates/statefulset.worker.yaml (1)
26-26: Optionally include main values in checksumWorker consumes main config/secret via envFrom; include both in the rollout hash to match operator expectations.
- checksum/config: {{ print .Values.worker | sha256sum }} + checksum/config: {{ print (dict "main" .Values.main "worker" .Values.worker) | sha256sum }}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
.github/workflows/lint-test.yaml(1 hunks)README.md(7 hunks)charts/n8n/Chart.yaml(1 hunks)charts/n8n/templates/_helpers.tpl(3 hunks)charts/n8n/templates/deployment.webhook.yaml(5 hunks)charts/n8n/templates/deployment.worker.yaml(4 hunks)charts/n8n/templates/deployment.yaml(3 hunks)charts/n8n/templates/hpa.yaml(1 hunks)charts/n8n/templates/pvc.yaml(2 hunks)charts/n8n/templates/statefulset.worker.yaml(1 hunks)charts/n8n/templates/statefulset.yaml(1 hunks)charts/n8n/values.yaml(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- charts/n8n/templates/hpa.yaml
- charts/n8n/Chart.yaml
- .github/workflows/lint-test.yaml
- charts/n8n/values.yaml
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-02T08:05:39.372Z
Learnt from: tsch157
PR: 8gears/n8n-helm-chart#239
File: charts/n8n/Chart.yaml:3-4
Timestamp: 2025-09-02T08:05:39.372Z
Learning: n8n 1.110.0 is a pre-release version and should not be used in production deployments. Always verify if a version is stable before recommending upgrades in the n8n Helm chart.
Applied to files:
README.md
🪛 YAMLlint (1.37.1)
charts/n8n/templates/statefulset.worker.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
[warning] 10-10: wrong indentation: expected 2 but found 4
(indentation)
[warning] 11-11: wrong indentation: expected 2 but found 4
(indentation)
charts/n8n/templates/deployment.worker.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
charts/n8n/templates/deployment.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
charts/n8n/templates/pvc.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
charts/n8n/templates/statefulset.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
[warning] 10-10: wrong indentation: expected 2 but found 4
(indentation)
[warning] 11-11: wrong indentation: expected 2 but found 4
(indentation)
🪛 markdownlint-cli2 (0.17.2)
README.md
682-682: Trailing punctuation in heading
Punctuation: '.'
(MD026, no-trailing-punctuation)
682-682: Bare URL used
(MD034, no-bare-urls)
684-684: Trailing punctuation in heading
Punctuation: '.'
(MD026, no-trailing-punctuation)
903-903: Bare URL used
(MD034, no-bare-urls)
🔇 Additional comments (1)
charts/n8n/templates/_helpers.tpl (1)
63-75: Main PVC name suffix is a good change
-mainsuffix avoids collisions across components. LGTM.
| {{- if .Values.webhook.persistence.enabled }} | ||
| volumeMounts: | ||
| - name: data | ||
| mountPath: /home/node/.n8n | ||
| {{- end }} | ||
| {{- if .Values.webhook.extraVolumeMounts }} | ||
| {{- toYaml .Values.webhook.extraVolumeMounts | nindent 12 }} | ||
| {{- end }} |
There was a problem hiding this comment.
Render volumeMounts only when there is content
Avoid emitting an empty volumeMounts: or orphaned mount items when persistence is disabled.
- {{- if .Values.webhook.persistence.enabled }}
- volumeMounts:
- - name: data
- mountPath: /home/node/.n8n
- {{- end }}
- {{- if .Values.webhook.extraVolumeMounts }}
- {{- toYaml .Values.webhook.extraVolumeMounts | nindent 12 }}
- {{- end }}
+ {{- if or .Values.webhook.persistence.enabled .Values.webhook.extraVolumeMounts }}
+ volumeMounts:
+ {{- if .Values.webhook.persistence.enabled }}
+ - name: data
+ mountPath: /home/node/.n8n
+ {{- end }}
+ {{- if .Values.webhook.extraVolumeMounts }}
+{{ toYaml .Values.webhook.extraVolumeMounts | nindent 12 }}
+ {{- end }}
+ {{- end }}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In charts/n8n/templates/deployment.webhook.yaml around lines 116–123, the
template can emit an empty or orphaned volumeMounts block when persistence is
disabled but extraVolumeMounts is absent; update the conditional to render the
entire volumeMounts section only when there is at least one mount by combining
the checks (e.g., if .Values.webhook.persistence.enabled or
.Values.webhook.extraVolumeMounts) and inside that block include the persistence
mount when enabled and render extraVolumeMounts via toYaml with the correct
nindent so no empty list or orphaned header is produced.
| volumes: | ||
| {{- if .Values.webhook.persistence.enabled }} | ||
| - name: "data" | ||
| {{ include "n8n.pvc" . }} | ||
| {{ include "n8n.webhook.pvc" . }} | ||
| {{- end }} | ||
| {{- if .Values.webhook.extraVolumes }} | ||
| {{- toYaml .Values.webhook.extraVolumes | nindent 8 }} | ||
| {{- end }} |
There was a problem hiding this comment.
Guard the volumes section to avoid an empty key
volumes: is always emitted; if both persistence and extraVolumes are off, YAML is invalid. Emit only when required.
- volumes:
- {{- if .Values.webhook.persistence.enabled }}
- - name: "data"
- {{ include "n8n.webhook.pvc" . }}
- {{- end }}
- {{- if .Values.webhook.extraVolumes }}
- {{- toYaml .Values.webhook.extraVolumes | nindent 8 }}
- {{- end }}
+ {{- if or .Values.webhook.persistence.enabled .Values.webhook.extraVolumes }}
+ volumes:
+ {{- if .Values.webhook.persistence.enabled }}
+ - name: data
+{{ include "n8n.webhook.pvc" . | nindent 10 }}
+ {{- end }}
+ {{- if .Values.webhook.extraVolumes }}
+{{ toYaml .Values.webhook.extraVolumes | nindent 8 }}
+ {{- end }}
+ {{- end }}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In charts/n8n/templates/deployment.webhook.yaml around lines 140 to 147, the
template always emits the "volumes:" key which becomes an empty mapping when
both .Values.webhook.persistence.enabled and .Values.webhook.extraVolumes are
false; wrap the entire "volumes:" block in a conditional that checks whether
either .Values.webhook.persistence.enabled or .Values.webhook.extraVolumes is
true and only render "volumes:" and its children when at least one is present,
preserving the existing children logic and indentation.
| {{- with .Values.main.livenessProbe }} | ||
| livenessProbe: | ||
| {{- toYaml . | nindent 12 }} | ||
| {{- end }} | ||
| {{- with .Values.main.readinessProbe }} | ||
| readinessProbe: | ||
| {{- toYaml . | nindent 12 }} | ||
| {{- end }} |
There was a problem hiding this comment.
Use worker probes, not main
The worker StatefulSet references .Values.main.livenessProbe/readinessProbe. Point to worker settings.
- {{- with .Values.main.livenessProbe }}
+ {{- with .Values.worker.livenessProbe }}
livenessProbe:
{{- toYaml . | nindent 12 }}
{{- end }}
- {{- with .Values.main.readinessProbe }}
+ {{- with .Values.worker.readinessProbe }}
readinessProbe:
{{- toYaml . | nindent 12 }}
{{- end }}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| {{- with .Values.main.livenessProbe }} | |
| livenessProbe: | |
| {{- toYaml . | nindent 12 }} | |
| {{- end }} | |
| {{- with .Values.main.readinessProbe }} | |
| readinessProbe: | |
| {{- toYaml . | nindent 12 }} | |
| {{- end }} | |
| {{- with .Values.worker.livenessProbe }} | |
| livenessProbe: | |
| {{- toYaml . | nindent 12 }} | |
| {{- end }} | |
| {{- with .Values.worker.readinessProbe }} | |
| readinessProbe: | |
| {{- toYaml . | nindent 12 }} | |
| {{- end }} |
🤖 Prompt for AI Agents
In charts/n8n/templates/statefulset.worker.yaml around lines 94 to 101, the
StatefulSet is incorrectly pulling livenessProbe and readinessProbe from
.Values.main; change both references to use .Values.worker.livenessProbe and
.Values.worker.readinessProbe (preserving the same toYaml + nindent formatting)
so the worker uses its own probe settings.
| {{- with .Values.hostAliases }} | ||
| hostAliases: | ||
| {{- toYaml . | nindent 8 }} | ||
| {{- end }} | ||
| {{- with .Values.worker.nodeSelector }} | ||
| nodeSelector: | ||
| {{- toYaml . | nindent 8 }} | ||
| {{- end }} | ||
| {{- with .Values.worker.affinity }} | ||
| affinity: | ||
| {{- toYaml . | nindent 8 }} | ||
| {{- end }} | ||
| {{- with .Values.worker.tolerations }} | ||
| tolerations: | ||
| {{- toYaml . | nindent 8 }} | ||
| {{- end }} |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add volumes for worker.extraVolumes
Without this, extra mounts reference undefined volumes.
{{- with .Values.worker.tolerations }}
tolerations:
{{- toYaml . | nindent 8 }}
{{- end }}
+ {{- if .Values.worker.extraVolumes }}
+ volumes:
+{{ toYaml .Values.worker.extraVolumes | nindent 8 }}
+ {{- end }}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| {{- with .Values.hostAliases }} | |
| hostAliases: | |
| {{- toYaml . | nindent 8 }} | |
| {{- end }} | |
| {{- with .Values.worker.nodeSelector }} | |
| nodeSelector: | |
| {{- toYaml . | nindent 8 }} | |
| {{- end }} | |
| {{- with .Values.worker.affinity }} | |
| affinity: | |
| {{- toYaml . | nindent 8 }} | |
| {{- end }} | |
| {{- with .Values.worker.tolerations }} | |
| tolerations: | |
| {{- toYaml . | nindent 8 }} | |
| {{- end }} | |
| {{- with .Values.hostAliases }} | |
| hostAliases: | |
| {{- toYaml . | nindent 8 }} | |
| {{- end }} | |
| {{- with .Values.worker.nodeSelector }} | |
| nodeSelector: | |
| {{- toYaml . | nindent 8 }} | |
| {{- end }} | |
| {{- with .Values.worker.affinity }} | |
| affinity: | |
| {{- toYaml . | nindent 8 }} | |
| {{- end }} | |
| {{- with .Values.worker.tolerations }} | |
| tolerations: | |
| {{- toYaml . | nindent 8 }} | |
| {{- end }} | |
| {{- if .Values.worker.extraVolumes }} | |
| volumes: | |
| {{ toYaml .Values.worker.extraVolumes | nindent 8 }} | |
| {{- end }} |
🤖 Prompt for AI Agents
In charts/n8n/templates/statefulset.worker.yaml around lines 112 to 127, the
template adds extraVolumeMounts for worker.extraVolumes but does not declare
corresponding volumes, causing mounts to reference undefined volumes; add a
volumes block under spec.template.spec that renders .Values.worker.extraVolumes
(or .Values.extraVolumes if that’s the intended key) using toYaml and nindent so
each declared volume name matches the volumeMount names, and guard it with a
with/.Values check (similar to the existing extraVolumeMounts) so volumes are
only rendered when provided.
| {{- with .Values.hostAliases }} | ||
| hostAliases: | ||
| {{- toYaml . | nindent 8 }} | ||
| {{- end }} | ||
| {{- with .Values.main.nodeSelector }} | ||
| nodeSelector: | ||
| {{- toYaml . | nindent 8 }} | ||
| {{- end }} | ||
| {{- with .Values.main.affinity }} | ||
| affinity: | ||
| {{- toYaml . | nindent 8 }} | ||
| {{- end }} | ||
| {{- with .Values.main.tolerations }} | ||
| tolerations: | ||
| {{- toYaml . | nindent 8 }} | ||
| {{- end }} |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Missing volumes for main.extraVolumes in StatefulSet
extraVolumes aren’t rendered at all; add a conditional volumes: block so mounts can resolve.
{{- with .Values.main.tolerations }}
tolerations:
{{- toYaml . | nindent 8 }}
{{- end }}
+ {{- if .Values.main.extraVolumes }}
+ volumes:
+{{ toYaml .Values.main.extraVolumes | nindent 8 }}
+ {{- end }}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In charts/n8n/templates/statefulset.yaml around lines 100 to 115, the template
renders hostAliases/nodeSelector/affinity/tolerations but does not render a
conditional volumes: block for .Values.main.extraVolumes, so any extraVolumes
referenced by container volumeMounts will be unresolved; add a conditional block
similar to the others: if .Values.main.extraVolumes is set, emit a volumes:
section under the pod spec and render the values with toYaml and the same
indentation level used for the other spec keys (nindent 8) so supplied
extraVolumes are present and mountable.
|
Up |
|
I understand that people are busy. But is this ever happening? |
10 participants
This PR improves the validation and documentation of deployment modes in the n8n Helm chart.
Key changes:
Note: This PR fixes potential issues with data loss and improves user experience by providing clear error messages and documentation.
Summary by CodeRabbit
New Features
Documentation
Bug Fixes
Tests