add extraContainers support #252
Conversation
WalkthroughAdds a new Helm values key Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Helm as Helm template engine
participant K8s as Kubernetes API
User->>Helm: helm install/upgrade (values may include extraContainers)
Helm->>Helm: For each scope (main/worker/webhook) check .Values.<scope>.extraContainers
alt extraContainers provided
Helm->>Helm: tpl(toYaml(extraContainers)) → render additional container YAML
note right of Helm `#E6F7FF`: Insert rendered blocks into\n.spec.template.spec.containers
else not provided
Helm->>Helm: Render default containers only
end
Helm->>K8s: Apply rendered Deployment manifests
K8s-->>User: Deployments created/updated
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (6)
🚧 Files skipped from review as they are similar to previous changes (5)
🔇 Additional comments (1)
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 |
obsidian33
force-pushed
the
support-extraContainers
branch
2 times, most recently
from
086f9aa to
04bedfa
Compare
![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 (8)
charts/n8n/templates/deployment.worker.yaml (1)
118-120: Worker sidecars block is correct and consistentGood use of tpl(toYaml(...)) with proper indentation.
Optional: allow merging in a shared .Values.extraContainers to reduce duplication when the same sidecars are needed on all pods.
charts/n8n/templates/deployment.webhook.yaml (1)
122-124: Webhook sidecars block is correctMatches the pattern used for main/worker and will append valid container specs.
Optional: consider supporting a shared .Values.extraContainers for common sidecars.
README.md (3)
358-364: Clarify what’s allowed in extraContainers entriesSuggest adding a note that each item is a full Kubernetes Container spec and Helm templating is supported (tpl), plus a brief example including resources and a probe.
550-556: Worker docs: include a sidecar example tailored to workersAdd an example showing envFrom/secretKeyRef and ports typical for worker‑side runners, as mentioned in the PR description.
742-748: Webhook docs: add a minimal HTTP sidecar exampleConsider adding a webhook‑specific sidecar example (e.g., tiny HTTP helper) and remind users to set any needed volumeMounts within the sidecar.
charts/n8n/values.yaml (3)
267-273: Document that items are full Container specs and can use templatingAdd a one‑liner: entries accept the standard K8s Container fields; Helm templating is supported; include a short example with resources.
459-465: Worker: add example with envFrom/portsProvide a worker‑focused example (envFrom with secretKeyRef, ports, readinessProbe) to guide typical runner sidecars.
650-656: Webhook: add example and note about volume mountsInclude a brief example and a note that sidecars can declare their own volumeMounts which must match pod volumes defined under extraVolumes.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
README.md(3 hunks)charts/n8n/Chart.yaml(2 hunks)charts/n8n/templates/deployment.webhook.yaml(1 hunks)charts/n8n/templates/deployment.worker.yaml(1 hunks)charts/n8n/templates/deployment.yaml(1 hunks)charts/n8n/values.yaml(3 hunks)
🧰 Additional context used
🧠 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:
charts/n8n/Chart.yaml
🔇 Additional comments (3)
charts/n8n/Chart.yaml (2)
3-4: Verify appVersion is a stable releaseVersion bumps look fine. Please double‑check n8n 1.114.3 isn’t a pre‑release before publishing.
Based on learnings
37-38: Changelog entry LGTMAccurate “added” entry for extraContainers support.
charts/n8n/templates/deployment.yaml (1)
103-105: Sidecar rendering under containers looks correcttpl(toYaml .Values.main.extraContainers) | nindent 8 correctly appends sidecars to the containers list.
Optional: consider a shared .Values.extraContainers to DRY identical sidecars across main/worker/webhook.
Please verify the rendered output in your local Helm environment.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
charts/n8n/templates/deployment.worker.yaml (1)
118-120: Placement/indentation LGTM; consider merging global + worker extras.Rendering with tpl(toYaml ...) at nindent 8 correctly appends sidecars under containers:. To align with the PR’s “top‑level extraContainers” intent and reduce duplication, merge .Values.extraContainers with .Values.worker.extraContainers before rendering.
Apply this diff to support both sources:
- {{- if .Values.worker.extraContainers }} - {{ tpl (toYaml .Values.worker.extraContainers) . | nindent 8 }} - {{- end }} + {{- $extra := list }} + {{- if .Values.extraContainers }}{{- $extra = concat $extra .Values.extraContainers }}{{- end }} + {{- if .Values.worker.extraContainers }}{{- $extra = concat $extra .Values.worker.extraContainers }}{{- end }} + {{- if $extra }} + {{ tpl (toYaml $extra) . | nindent 8 }} + {{- end }}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
README.md(3 hunks)charts/n8n/Chart.yaml(2 hunks)charts/n8n/templates/deployment.webhook.yaml(1 hunks)charts/n8n/templates/deployment.worker.yaml(1 hunks)charts/n8n/templates/deployment.yaml(1 hunks)charts/n8n/values.yaml(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- README.md
- charts/n8n/Chart.yaml
- charts/n8n/values.yaml
- charts/n8n/templates/deployment.webhook.yaml
- charts/n8n/templates/deployment.yaml
|
looks like n8n is gearing up for v2 changes where task runners will be the default execution, this PR will be needed to run external tasks runners in sidecar. |
|
@obsidian33 It's good feature, but PR conflict and need to resolve. |
obsidian33
force-pushed
the
support-extraContainers
branch
from
04bedfa to
92eb41c
Compare
|
@FannWuCircle PR conflict resolved. |
|
This will be in the next release |
4 participants
What this PR does / why we need it
This PR adds basic support for specifying
extraContainers(sidecar) which can be used in main, worker, and webhook deployments.I want this feature added so I can add external n8n runner sidecar with the workers but it is generic enough to allow users to setup any sidecar they need. I tested this locally using:
Special notes for your reviewer
This PR was originally submitted by ArnaudTA in PR #180 but it has sat a month without addressing reviewer concerns.
Checklist
Version and Documentation
Chart.yamlfollowing semantic versioningChart.yamlif applicableartifacthub.io/changessection updated inChart.yaml(see ArtifactHub annotation reference)README.mdTesting and Validation
ah lintlocally without errorsct lint --chart-dirs charts/n8n --charts charts/n8n --validate-maintainers=false/examplesdirectorySummary by CodeRabbit
New Features
Documentation
Chores
✏️ Tip: You can customize this high-level summary in your review settings.