Added stateful deployment for main #187
Conversation
|
Warning Rate limit exceeded@fank has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 17 minutes and 52 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (11)
WalkthroughThe changes introduce support for deploying the n8n application as a Kubernetes StatefulSet, allowing each replica to have its own persistent volume. This includes new configuration options, updated documentation, Helm template logic for StatefulSet creation and validation, conditional rendering of resources, and an example configuration for stateful deployments with Redis. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Helm
participant Kubernetes
User->>Helm: Install/Upgrade chart with values.yaml
Helm->>Helm: Check main.useStatefulSet value
alt useStatefulSet = true
Helm->>Helm: Validate persistence settings for StatefulSet
Helm->>Kubernetes: Render and apply StatefulSet manifest
Helm->>Kubernetes: Render and apply volumeClaimTemplates
Helm->>Kubernetes: Render and apply headless Service
else useStatefulSet = false
Helm->>Kubernetes: Render and apply Deployment manifest
Helm->>Kubernetes: Render and apply PVC if needed
Helm->>Kubernetes: Render and apply regular Service
end
Kubernetes->>User: Deploys n8n pods with appropriate persistent storage
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~15 minutes Suggested reviewers
✨ Finishing Touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (7)
examples/values_stateful.yaml (4)
1-1: Add space after comment marker to improve readability
YAML conventions recommend a space after the#. Change line 1 from:#stateful deployment for multiple replicas with persistent state per replicato:
# stateful deployment for multiple replicas with persistent state per replica🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 1-1: wrong new line character: expected \n
(new-lines)
7-7: Remove trailing whitespace
Line 7 contains trailing spaces. Please remove them to satisfy YAML lint rules and avoid unnecessary diff noise.🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 7-7: trailing spaces
(trailing-spaces)
18-18: Remove trailing whitespace
Line 18 contains trailing spaces. Removing them keeps the file clean and lint-compliant.🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 18-18: trailing spaces
(trailing-spaces)
32-38: AddaccessModesfor Valkey persistence
For consistency withmain.persistence, consider specifying:valkey: primary: persistence: enabled: true size: 2Gi accessModes: - ReadWriteOnceThis makes storage requirements explicit.
README.md (2)
50-50: Add missing comma for clarity
Insert a comma after “persistent volume” in line 50 so it reads:“…all pods share the same persistent volume, which can lead to data corruption…”
🧰 Tools
🪛 LanguageTool
[uncategorized] ~50-~50: A comma might be missing here.
Context: ...ent, all pods share the same persistent volume which can lead to data corruption as mu...(AI_EN_LECTOR_MISSING_PUNCTUATION_COMMA)
63-66: Clarify requiredpersistence.typesetting
The warning mentionspersistence.enabledandpersistence.storageClass, but for StatefulSets you must also set:persistence: type: dynamicPlease update the note to include
persistence.typefor completeness.charts/n8n/templates/statefulset.yaml (1)
71-75: Refineenvempty-list logic
The current| ternary nil "[]"may emitenv: nullwhenextraEnvis non-empty, leading to invalid YAML. Consider refactoring to:env: {{- if .Values.main.extraEnv }} {{- range $key, $value := .Values.main.extraEnv }} - name: {{ $key }} {{ toYaml $value | nindent 4 }} {{- end }} {{- else }} [] {{- end }}This ensures
envis always a proper array.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
README.md(2 hunks)charts/n8n/templates/_helpers.tpl(2 hunks)charts/n8n/templates/deployment.yaml(2 hunks)charts/n8n/templates/statefulset.yaml(1 hunks)charts/n8n/values.yaml(1 hunks)examples/README.md(1 hunks)examples/values_stateful.yaml(1 hunks)
🧰 Additional context used
🪛 YAMLlint (1.37.1)
charts/n8n/templates/deployment.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
examples/values_stateful.yaml
[error] 1-1: wrong new line character: expected \n
(new-lines)
[error] 7-7: trailing spaces
(trailing-spaces)
[error] 18-18: trailing spaces
(trailing-spaces)
[error] 24-24: trailing spaces
(trailing-spaces)
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)
[warning] 11-11: wrong indentation: expected 2 but found 4
(indentation)
🪛 LanguageTool
README.md
[uncategorized] ~50-~50: A comma might be missing here.
Context: ...ent, all pods share the same persistent volume which can lead to data corruption as mu...
(AI_EN_LECTOR_MISSING_PUNCTUATION_COMMA)
🔇 Additional comments (7)
charts/n8n/values.yaml (1)
51-53: LGTM: Well-documented StatefulSet configuration flagThe addition of the
useStatefulSetparameter is well-documented and appropriately defaulted tofalseto maintain backward compatibility with existing deployments. This flag enables users to choose between Deployment and StatefulSet for their n8n instances.examples/README.md (1)
12-12: LGTM: Clear documentation of the new exampleThe README has been properly updated to reference the new StatefulSet example configuration file, which will help users understand how to deploy n8n with individual persistent volumes per replica.
charts/n8n/templates/deployment.yaml (1)
1-1: LGTM: Appropriate conditional rendering of DeploymentThe conditional wrapping ensures that the Deployment resource is only created when
useStatefulSetis false, preventing resource conflicts when using StatefulSets. This implementation allows the seamless transition between deployment types.The static analysis tool flagging a YAML syntax error is a false positive due to the Helm template syntax being processed before the YAML is rendered.
Also applies to: 125-125
🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
charts/n8n/templates/_helpers.tpl (2)
70-70: LGTM: Proper PVC handling for different deployment typesThe PVC logic has been correctly modified to only create dynamic PVCs when not using StatefulSets. This prevents duplicate volume claim creation since StatefulSets will handle PVC creation through volumeClaimTemplates.
105-110: LGTM: Important validation to prevent misconfigurationThe new validation function is an excellent safeguard that prevents users from configuring incompatible options. By failing early with a clear error message when both
useStatefulSet=trueandpersistence.existingClaimis specified, you help users avoid confusing runtime errors.charts/n8n/templates/statefulset.yaml (2)
1-3: Conditional rendering and validation are applied correctly
Theif .Values.main.useStatefulSetguard andinclude "n8n.validateStatefulSet"call ensure the StatefulSet is only rendered when intended and that the configuration is validated.🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
62-70:❓ Verification inconclusive
Verify
envFromindentation and optionality
Indentation errors in templated blocks can cause unexpected rendering. Usehelm templateto confirm thatenvFrom:and its child entries for ConfigMapRef and SecretRef appear only when defined and are indented correctly.
🏁 Script executed:
#!/bin/bash # Render only the StatefulSet template and lint it helm template . \ --values examples/values_stateful.yaml \ --set main.useStatefulSet=true \ -s charts/n8n/templates/statefulset.yaml | yamllint -Length of output: 236
Manual Verification Required: Confirm
envFromIndentationOur environment couldn’t run the Helm CLI, so automated linting didn’t execute. Please verify locally that:
envFrom:only appears when.Values.main.configor.Values.main.secretare set- The child entries (
configMapRef/secretRef) are indented correctly underenvFrom:Run this command in your local setup and ensure no errors are reported:
helm template . \ --values examples/values_stateful.yaml \ --set main.useStatefulSet=true \ -s charts/n8n/templates/statefulset.yaml \ | yamllint -
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
charts/n8n/templates/pvc.yaml(1 hunks)charts/n8n/templates/statefulset.yaml(1 hunks)
🧰 Additional context used
🪛 YAMLlint (1.37.1)
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] 9-9: wrong indentation: expected 2 but found 4
(indentation)
[warning] 10-10: wrong indentation: expected 2 but found 4
(indentation)
[warning] 11-11: wrong indentation: expected 2 but found 4
(indentation)
🔇 Additional comments (4)
charts/n8n/templates/pvc.yaml (2)
1-29: Conditional PVC creation aligns with StatefulSet support.
The addition of(not .Values.main.useStatefulSet)to theifguard on line 1 correctly prevents standalone PVC resources from being rendered whenuseStatefulSetis enabled, since StatefulSets manage volumes viavolumeClaimTemplates. This change avoids PVC conflicts and is consistent with the new StatefulSet template logic.🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
1-29: Ignore YAMLLint false positive on Helm templating.
The YAMLLint errorsyntax error: expected the node content, but found '-'arises from Helm's Go templating syntax, not an actual YAML issue. You can safely disregard this warning.🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
charts/n8n/templates/statefulset.yaml (2)
1-2: Validate StatefulSet configuration at the top-level.
Wrapping the template in{{- if .Values.main.useStatefulSet }}and invokingn8n.validateStatefulSetensures that StatefulSet-specific preconditions (persistence enabled, no existing claims) are enforced early. This promotes fail-fast behavior for misconfigurations.🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
9-11: Indentation warnings are false positives.
The static analysis warnings at lines 9–11 (wrong indentation) stem from Helm template directives, not actual YAML formatting errors. The existingnindentcalls correctly handle indentation for conditional blocks and can be ignored.🧰 Tools
🪛 YAMLlint (1.37.1)
[warning] 9-9: wrong indentation: expected 2 but found 4
(indentation)
[warning] 10-10: wrong indentation: expected 2 but found 4
(indentation)
[warning] 11-11: wrong indentation: expected 2 but found 4
(indentation)
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
examples/values_stateful.yaml (2)
1-1: Nitpick: Standardize header formatting.
There’s no newline before the first line and missing space after#. Consider revising to:# Stateful deployment for multiple replicas with persistent state per replica🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 1-1: wrong new line character: expected \n
(new-lines)
9-9: Remove trailing whitespace.
Lines 9, 20, and 26 contain trailing spaces. Cleaning these improves YAML linting and readability.Also applies to: 20-20, 26-26
🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 9-9: trailing spaces
(trailing-spaces)
README.md (1)
50-50: Typographical nitpick: missing comma.
Add a comma after "persistent volume" for clarity:- all pods share the same persistent volume which can lead to data corruption as multiple replicas try to access the same files simultaneously. + all pods share the same persistent volume, which can lead to data corruption as multiple replicas try to access the same files simultaneously.🧰 Tools
🪛 LanguageTool
[uncategorized] ~50-~50: A comma might be missing here.
Context: ...ent, all pods share the same persistent volume which can lead to data corruption as mu...(AI_EN_LECTOR_MISSING_PUNCTUATION_COMMA)
charts/n8n/templates/statefulset.yaml (1)
17-21: Expose StatefulSet updateStrategy & podManagementPolicy.
Currently the default update strategy and pod management policy are used. Consider adding configurableupdateStrategyandpodManagementPolicyinvalues.yamlto enable controlled rolling updates and parallel pod startups:main: statefulset: updateStrategy: type: RollingUpdate partition: 0 podManagementPolicy: Parallel
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
README.md(2 hunks)charts/n8n/templates/service-headless.yaml(1 hunks)charts/n8n/templates/service.yaml(1 hunks)charts/n8n/templates/statefulset.yaml(1 hunks)examples/values_stateful.yaml(1 hunks)
🧰 Additional context used
🪛 YAMLlint (1.37.1)
charts/n8n/templates/service.yaml
[error] 4-4: syntax error: expected , but found '{'
(syntax)
charts/n8n/templates/service-headless.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] 9-9: wrong indentation: expected 2 but found 4
(indentation)
[warning] 10-10: wrong indentation: expected 2 but found 4
(indentation)
[warning] 11-11: wrong indentation: expected 2 but found 4
(indentation)
examples/values_stateful.yaml
[error] 1-1: wrong new line character: expected \n
(new-lines)
[error] 9-9: trailing spaces
(trailing-spaces)
[error] 20-20: trailing spaces
(trailing-spaces)
[error] 26-26: trailing spaces
(trailing-spaces)
🪛 LanguageTool
README.md
[uncategorized] ~50-~50: A comma might be missing here.
Context: ...ent, all pods share the same persistent volume which can lead to data corruption as mu...
(AI_EN_LECTOR_MISSING_PUNCTUATION_COMMA)
🔇 Additional comments (4)
charts/n8n/templates/service.yaml (1)
4-4:⚠️ Potential issueFix default Service type value.
The default for.Values.main.service.typeis defined as"ClusterIP_"(with a trailing underscore) which is invalid. It should be"ClusterIP".
Apply this diff:- type: {{ default "ClusterIP_" .Values.main.service.type }} + type: {{ default "ClusterIP" .Values.main.service.type }}Likely an incorrect or invalid review comment.
🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 4-4: syntax error: expected , but found '{'
(syntax)
charts/n8n/templates/service-headless.yaml (1)
1-5: The headless Service template correctly conditionally renders whenuseStatefulSetis enabled and aligns with the StatefulSet’sserviceName.🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
README.md (1)
44-54: Documentation clear and comprehensive.
The new "Deployment Types" section effectively explains when to use Deployments vs StatefulSets and references the example configuration.🧰 Tools
🪛 LanguageTool
[uncategorized] ~50-~50: A comma might be missing here.
Context: ...ent, all pods share the same persistent volume which can lead to data corruption as mu...(AI_EN_LECTOR_MISSING_PUNCTUATION_COMMA)
charts/n8n/templates/statefulset.yaml (1)
1-3: StatefulSet conditional inclusion & validation.
Including then8n.validateStatefulSethelper ensures invalid PVC configurations (e.g.,existingClaimwith StatefulSets) are caught early.🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
|
@fank This is a useful feature. Do you have the time to wrap it up? Thanks! |
|
@joshuabaird I am still actively using it in production, I will come back to this to clean it up! |
- Fixed YAML formatting in examples/values_stateful.yaml (removed trailing spaces, added space after comment marker) - Fixed grammar in README.md (added missing comma) - Fixed service type default value from 'ClusterIP_' to 'ClusterIP' in charts/n8n/templates/service.yaml
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
charts/n8n/templates/service.yaml (2)
4-4: Quote the generated service name to avoid parsing issuesAlthough Helm usually renders the value without problems, quoting the entire
namestring guards against accidental colon/space characters sneaking in from template helpers and makes the YAML friendlier to external linters (they often choke on the first{they see, as hinted by the YAML-lint warning).-name: {{ include "n8n.fullname" . }}{{ if .Values.main.useStatefulSet }}-svc{{ end }} +name: "{{ include "n8n.fullname" . }}{{ if .Values.main.useStatefulSet }}-svc{{ end }}"
12-12: Quote the service type for consistencyUpstream Kubernetes examples normally quote the value of
type, and quoting also sidesteps edge cases where users pass a lower-case value that is later upper-cased by templating logic.-type: {{ default "ClusterIP" .Values.main.service.type }} +type: {{ default "ClusterIP" .Values.main.service.type | quote }}examples/values_stateful.yaml (2)
1-1: Convert CRLF to LF line endingsThe file is committed with
\r\nline endings (YAML-lint “wrong new line character”). This causes noisy diffs and can break tooling on Unix environments. Re-save the file with LF endings only.
20-24: Remind users to externalise secrets
encryption_keyis hard-coded in the example. Even though it’s a placeholder string, beginners often copy the file verbatim. Adding a brief comment recommending a Kubernetes secret (or at least an environment variable) avoids accidental plaintext keys in Git.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
README.md(2 hunks)charts/n8n/templates/service.yaml(1 hunks)examples/values_stateful.yaml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- README.md
🧰 Additional context used
🪛 YAMLlint (1.37.1)
charts/n8n/templates/service.yaml
[error] 4-4: syntax error: expected , but found '{'
(syntax)
examples/values_stateful.yaml
[error] 1-1: wrong new line character: expected \n
(new-lines)
🔇 Additional comments (2)
charts/n8n/templates/service.yaml (1)
18-20: Confirm selector labels still match the StatefulSet podsWhen
main.useStatefulSetis true, the workload switches from Deployment to StatefulSet. Please double-check that the StatefulSet template still setsapp.kubernetes.io/type: master; otherwise this service will not pick up the pods and traffic will break.examples/values_stateful.yaml (1)
12-18: Validatepersistence.type: dynamicmatches the chart enumThe base
values.yamlfor the chart historically acceptedtype: pvc/type: hostPathor similar. “dynamic” is new; if the helper templates don’t recognise this value the StatefulSet will not render thevolumeClaimTemplates. Please verify the helpers in_helpers.tplhandle"dynamic".
|
Thanks so much, @fank! |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
charts/n8n/templates/deployment.yaml (2)
1-1: Guard clause is correct but should defensively handle a missing value
useStatefulSetis a newly-introduced flag, so oldervalues.yamlfiles may omit it. Helm treats an undefined value asnil, which is falsy, but thenotoperator will returntruefornil.
If that implicit behaviour is acceptable, leave as-is; otherwise wrap withdefaultfor explicitness and future-proofing:-{{- if not .Values.main.useStatefulSet }} +{{- if not (default false .Values.main.useStatefulSet) }}
125-125: Closing directive matches, but keep a blank line after itHelm’s
{{- end }}already trims the preceding newline. Adding one explicit newline after the closing tag avoids concatenating with whatever follows in concatenated manifests and keepshelm templateoutput tidy.-{{- end }} +{{- end }}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
.github/workflows/lint-test.yaml(1 hunks)README.md(2 hunks)charts/n8n/templates/deployment.yaml(2 hunks)charts/n8n/templates/service.yaml(1 hunks)charts/n8n/values.yaml(1 hunks)examples/README.md(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- examples/README.md
- .github/workflows/lint-test.yaml
🚧 Files skipped from review as they are similar to previous changes (3)
- charts/n8n/templates/service.yaml
- charts/n8n/values.yaml
- README.md
🧰 Additional context used
🪛 YAMLlint (1.37.1)
charts/n8n/templates/deployment.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
- Add main.useStatefulSet flag to toggle between Deployment and StatefulSet - Create StatefulSet template with volumeClaimTemplates for individual PVCs per replica - Add headless service for StatefulSet DNS management - Add validation to prevent incompatible persistence configurations - Update PVC creation logic to skip when using StatefulSet - Add example configuration for StatefulSet deployment - Document StatefulSet vs Deployment use cases in README - Bump chart version to 1.0.12 - Fix CI workflow chart-testing-action version to v2.7.0 This enables horizontal scaling with individual persistent volumes per replica, useful for multi-instance deployments where each instance needs its own state. Closes 8gears#187
- Add main.useStatefulSet flag to toggle between Deployment and StatefulSet - Create StatefulSet template with volumeClaimTemplates for individual PVCs per replica - Add headless service for StatefulSet DNS management - Add validation to prevent incompatible persistence configurations - Update PVC creation logic to skip when using StatefulSet - Add example configuration for StatefulSet deployment - Document StatefulSet vs Deployment use cases in README - Bump chart version to 1.0.12 - Fix CI workflow chart-testing-action version to v2.7.0 This enables horizontal scaling with individual persistent volumes per replica, useful for multi-instance deployments where each instance needs its own state. Closes 8gears#187
- Add main.useStatefulSet flag to toggle between Deployment and StatefulSet - Create StatefulSet template with volumeClaimTemplates for individual PVCs per replica - Add headless service for StatefulSet DNS management - Add validation to prevent incompatible persistence configurations - Update PVC creation logic to skip when using StatefulSet - Add example configuration for StatefulSet deployment - Document StatefulSet vs Deployment use cases in README - Bump chart version to 1.0.12 - Fix CI workflow chart-testing-action version to v2.7.0 This enables horizontal scaling with individual persistent volumes per replica, useful for multi-instance deployments where each instance needs its own state. Closes 8gears#187
|
@Vad1mo feel free to review |
There was a problem hiding this comment.
Actionable comments posted: 3
🔭 Outside diff range comments (1)
charts/n8n/templates/pvc.yaml (1)
15-18: Incorrect use of{{-strips indentation → PVC YAML becomes invalidBecause
{{- range ... }}trims the leading four spaces, list items are emitted flush-left:accessModes: - "ReadWriteOnce"This breaks YAML parsing.
- {{- range .Values.main.persistence.accessModes }} - - {{ . | quote }} - {{- end }} + {{- range .Values.main.persistence.accessModes }} + - {{ . | quote }} + {{- end }}or switch to
nindent:- {{- range .Values.main.persistence.accessModes }} - - {{ . | quote }} - {{- end }} +{{- range .Values.main.persistence.accessModes }} + - {{ . | quote }} +{{- end }}
🧹 Nitpick comments (4)
charts/n8n/templates/deployment.yaml (1)
125-125: Minor: closing{{- end }}is easy to overlookConsider adding a short comment so future edits don’t accidentally delete the opening guard:
{{- end }}{{/* useStatefulSet guard */}}.github/workflows/lint-test.yaml (1)
50-50: Missing newline at EOFAdd a trailing newline to satisfy YAML-lint and keep Git diffs clean.
examples/values_stateful.yaml (1)
1-38: CRLF line endings & placeholder secrets
- File currently has Windows CRLF endings; convert to LF to avoid helm-lint noise.
encryption_key: "<shared-encryption-key>"is a good reminder, but users often forget. Consider referencing a Kubernetes Secret instead.Example:
- encryption_key: "<shared-encryption-key>" + # encryption_key: "<shared-encryption-key>" + encryption_key__existingSecret: n8n-encryptioncharts/n8n/templates/statefulset.yaml (1)
1-16: YAML front-matter & indentation warnings
yamllintflags a top-level syntax error and indentation issues (see pipeline hints).
While templating often confuses linters, adding the standard document delimiter and consistent two-space indentation silences most false positives and improves readability:+--- {{- if .Values.main.useStatefulSet }} {{- include "n8n.validateStatefulSet" . -}}Consider running
helm templatethroughyamllint -d relaxedin CI to catch real YAML mistakes early.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
.github/workflows/lint-test.yaml(2 hunks)README.md(2 hunks)charts/n8n/Chart.yaml(2 hunks)charts/n8n/templates/_helpers.tpl(2 hunks)charts/n8n/templates/deployment.yaml(2 hunks)charts/n8n/templates/pvc.yaml(1 hunks)charts/n8n/templates/service-headless.yaml(1 hunks)charts/n8n/templates/service.yaml(1 hunks)charts/n8n/templates/statefulset.yaml(1 hunks)charts/n8n/values.yaml(1 hunks)examples/README.md(1 hunks)examples/values_stateful.yaml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (6)
- charts/n8n/Chart.yaml
- examples/README.md
- charts/n8n/templates/service.yaml
- charts/n8n/values.yaml
- README.md
- charts/n8n/templates/_helpers.tpl
🧰 Additional context used
🪛 actionlint (1.7.7)
.github/workflows/lint-test.yaml
10-10: the runner of "actions/checkout@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🪛 YAMLlint (1.37.1)
.github/workflows/lint-test.yaml
[error] 50-50: no new line character at the end of file
(new-line-at-end-of-file)
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/service-headless.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] 9-9: wrong indentation: expected 2 but found 4
(indentation)
[warning] 10-10: wrong indentation: expected 2 but found 4
(indentation)
[warning] 11-11: wrong indentation: expected 2 but found 4
(indentation)
examples/values_stateful.yaml
[error] 1-1: wrong new line character: expected \n
(new-lines)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: lint-test
🔇 Additional comments (4)
charts/n8n/templates/pvc.yaml (1)
1-1: ✅ Condition correctly disables PVC when StatefulSet is enabledWrapping the PVC manifest with
(not .Values.main.useStatefulSet)cleanly prevents duplicate/conflicting PVCs in StatefulSet mode.charts/n8n/templates/deployment.yaml (1)
1-1: Deployment correctly gated behind!useStatefulSetThis prevents multiple workload controllers for the same pod spec. Nice.
.github/workflows/lint-test.yaml (1)
10-26: Downgrading core actions may break future CI runners
actions/checkout@v3,azure/[email protected], and[email protected]are all superseded. action-lint is already warning aboutcheckout@v3.Unless you have a reproducibility reason, upgrade to the latest major versions:
-uses: actions/checkout@v3 +uses: actions/checkout@v4 -uses: azure/[email protected] +uses: azure/setup-helm@v4 +with: + version: v3.14.4 # keep pin if really required -uses: helm/[email protected] +uses: helm/chart-testing-action@v3charts/n8n/templates/statefulset.yaml (1)
20-21: Ensure matching headless Service name
serviceNameis set to{{ include "n8n.fullname" . }}-headless, but no headless Service template is included in this PR snippet. Pods will fail DNS resolution (<pod-ordinal>.<serviceName>) unless a Service with the exact same name (clusterIP: None) is rendered.Please verify the chart renders a
Serviceresource named{{ include "n8n.fullname" . }}-headless.
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
charts/n8n/templates/pvc.yaml(2 hunks)charts/n8n/templates/service-headless.yaml(1 hunks)charts/n8n/templates/statefulset.yaml(1 hunks)
🧰 Additional context used
🪛 YAMLlint (1.37.1)
charts/n8n/templates/pvc.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
charts/n8n/templates/service-headless.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] 9-9: wrong indentation: expected 2 but found 4
(indentation)
[warning] 10-10: wrong indentation: expected 2 but found 4
(indentation)
[warning] 11-11: wrong indentation: expected 2 but found 4
(indentation)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: lint-test
🔇 Additional comments (2)
charts/n8n/templates/pvc.yaml (1)
16-18:accessModesblock can render with zero-indent → invalid YAMLBecause
{{- range ...}}trims the entire leading whitespace, the list items may be emitted starting at column 0 instead of the required indent underaccessModes:. Some Kubernetes validators (andyamllint) will reject the manifest.- accessModes: -{{- range .Values.main.persistence.accessModes }} - - {{ . | quote }} -{{- end }} + accessModes: +{{- range .Values.main.persistence.accessModes }} + - {{ . | quote }} +{{- end }}Removing the dash from
{{- rangeor usingnindent 2after the range are both acceptable alternatives.Likely an incorrect or invalid review comment.
charts/n8n/templates/service-headless.yaml (1)
17-18: Port lookup now matches Deployment — good catchSwitching to
get (default (dict) .Values.main.config.n8n) "port"
eliminates the earlier drift between Service and Pod ports.
This PR introduces StatefulSet support for n8n deployments, enabling better handling of persistent storage when running multiple replicas. ## Key Changes - Add new StatefulSet template with volumeClaimTemplates for individual PVCs per replica - Add headless Service for StatefulSet DNS management and pod identity - Add useStatefulSet flag to toggle between Deployment and StatefulSet modes - Update PVC template to skip creation when using StatefulSet (handled by volumeClaimTemplates) - Add validation to prevent incompatible persistence configurations (existingClaim with StatefulSet) - Include example configuration file for StatefulSet deployments - Add comprehensive documentation explaining when and how to use StatefulSets ## Implementation Details - StatefulSet creates individual PVCs for each replica, preventing data corruption from shared volumes - Headless service provides stable network identity for StatefulSet pods - Regular service (-svc suffix) provides load balancing for external access - Validation helper ensures users don't misconfigure persistence settings - All templates include namespace field for proper scoping ## Use Cases StatefulSets are recommended when: - Running multiple n8n replicas that need individual persistent storage - Each instance needs to maintain its own workflow data - Implementing high availability with proper data isolation For proper multi-replica operation, users should: - Enable Redis/Valkey for session state management - Use external database instead of SQLite - Configure shared encryption key across all replicas Fixes 8gears#186
fank
force-pushed
the
fk_statefulmain
branch
from
b478511 to
e30824e
Compare
|
@Vad1mo changes and commit got cleaned up, based on latest changes in master. |
|
@RoseSecurity any change for you to review this? |
2 participants
This pull request introduces support for deploying n8n as a StatefulSet in Kubernetes, enabling better handling of persistent volumes for multi-replica setups. It includes updates to the Helm chart, configuration files, and documentation to guide users on when and how to use StatefulSets instead of Deployments.
StatefulSet Support
Documentation Updates:
README.mdexplaining the differences between StatefulSets and Deployments, their use cases, and configuration requirements. Includes a warning about incompatibility withpersistence.existingClaimwhen using StatefulSets.examples/values_stateful.yaml) for deploying n8n as a StatefulSet with individual persistent volumes per replica.examples/README.mdto reference the new example configuration.Helm Chart Enhancements:
main.useStatefulSetflag invalues.yamlto toggle between StatefulSet and Deployment._helpers.tplto ensurepersistence.existingClaimis not used with StatefulSets.deployment.yamlto skip Deployment creation whenmain.useStatefulSetis true. [1] [2]statefulset.yamltemplate for StatefulSet deployments, including support for volumeClaimTemplates and other StatefulSet-specific configurations.Persistence Configuration:
_helpers.tplto ensure dynamic PVC provisioning is only used for Deployments whenmain.useStatefulSetis false.Summary by CodeRabbit
New Features
Documentation