-
Notifications
You must be signed in to change notification settings - Fork 220
Fix chart behaviour when Redis/Valkey Cluster is used #275
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
WalkthroughThe n8n Helm chart version is bumped to 1.1.1, and the Valkey/Redis validation logic is updated to require both missing host and missing cluster nodes to trigger failure when webhooks are enabled. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
Tip 📝 Customizable high-level summaries are now available in beta!You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.
Example instruction:
Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later. 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
charts/n8n/templates/_helpers.tpl (1)
101-101: Consider updating the error message to mention cluster configuration.The error message currently says "Please set a Redis/Valkey host" but doesn't mention the cluster nodes option. This could confuse users who need cluster configuration.
Apply this diff to improve clarity:
-{{- fail "Webhook processes rely on Valkey. Please set a Redis/Valkey host when webhook.enabled=true" -}} +{{- fail "Webhook processes rely on Valkey. Please set either QUEUE_BULL_REDIS_HOST or QUEUE_BULL_REDIS_CLUSTER_NODES when webhook.enabled=true" -}}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
charts/n8n/Chart.yaml(1 hunks)charts/n8n/templates/_helpers.tpl(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: tsch157
Repo: 8gears/n8n-helm-chart PR: 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.
📚 Learning: 2025-09-02T08:05:39.372Z
Learnt from: tsch157
Repo: 8gears/n8n-helm-chart PR: 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-3: LGTM! Version bump is appropriate for this bug fix.The patch version increment from 1.1.0 to 1.1.1 correctly follows semantic versioning for a bug fix.
4-4: appVersion 1.116.2 is a stable release — no concerns identified.Verification confirms that n8n v1.116.2 was released as a stable version on October 21, 2025. It is safe for production deployments and does not require changes.
charts/n8n/templates/_helpers.tpl (1)
100-100: Excellent fix! The validation logic now correctly handles both standalone and cluster configurations.The updated condition allows webhooks to work with either:
- Standalone Redis/Valkey (via
QUEUE_BULL_REDIS_HOST), or- Cluster mode (via
QUEUE_BULL_REDIS_CLUSTER_NODES)The validation now only fails when both are missing, which correctly addresses the PR objective.
What this PR does / why we need it
When using Redis/Valkey Cluster instead of standalone (for HA purposes), the chart throws an error because the
QUEUE_BULL_REDIS_HOSTenvironnement variable stays empty, whileQUEUE_BULL_REDIS_CLUSTER_NODESis not.Ref.: https://docs.n8n.io/hosting/configuration/environment-variables/queue-mode/
Summary by CodeRabbit
Bug Fixes
Chores
✏️ Tip: You can customize this high-level summary in your review settings.