-
Notifications
You must be signed in to change notification settings - Fork 106
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
✨ feat: worker and webhook autosacling #112
base: main
Are you sure you want to change the base?
Conversation
closes 8gears#80, closes 8gears#87 Signed-off-by: Muhammed Hussein Karimi <[email protected]>
WalkthroughThe changes introduce more granular autoscaling configurations for both webhook and worker deployments in the n8n Helm chart. By creating dedicated settings for webhook and worker autoscaling, the modifications allow better management of pod replicas based on specific metrics, enhancing the control over resource allocation and scaling behavior. Additionally, new Horizontal Pod Autoscaler templates improve scalability and responsiveness to resource utilization. Changes
Assessment against linked issues
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
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: 2
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (5)
- charts/n8n/templates/deployment.webhooks.yaml (1 hunks)
- charts/n8n/templates/deployment.worker.yaml (1 hunks)
- charts/n8n/templates/hpa.webhook.yaml (1 hunks)
- charts/n8n/templates/hpa.worker.yaml (1 hunks)
- charts/n8n/values.yaml (1 hunks)
Additional context used
yamllint
charts/n8n/templates/hpa.worker.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
charts/n8n/templates/hpa.webhook.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
Additional comments not posted (10)
charts/n8n/templates/hpa.worker.yaml (3)
8-18
: LGTM! Metadata and spec sections are correct.The metadata and spec sections are well-defined, with appropriate use of labels and replica settings.
20-36
: LGTM! Metrics section for Kubernetes version >=1.25 is correct.The metrics section is well-defined, with appropriate conditional logic for CPU and memory utilization.
37-49
: LGTM! Metrics section for Kubernetes version <1.25 is correct.The metrics section is well-defined, with appropriate conditional logic for CPU and memory utilization.
charts/n8n/templates/hpa.webhook.yaml (3)
8-18
: LGTM! Metadata and spec sections are correct.The metadata and spec sections are well-defined, with appropriate use of labels and replica settings.
20-36
: LGTM! Metrics section for Kubernetes version >=1.25 is correct.The metrics section is well-defined, with appropriate conditional logic for CPU and memory utilization.
37-49
: LGTM! Metrics section for Kubernetes version <1.25 is correct.The metrics section is well-defined, with appropriate conditional logic for CPU and memory utilization.
charts/n8n/templates/deployment.webhooks.yaml (1)
9-9
: LGTM! Conditional statement for webhook autoscaling is correct.The conditional statement is well-defined and ensures that the replicas are set correctly based on the webhook autoscaling setting.
charts/n8n/templates/deployment.worker.yaml (1)
9-11
: LGTM! Verify consistency across the codebase.The change to check
workerAutoscaling.enabled
instead ofautoscaling.enabled
aligns with the new autoscaling settings. Ensure that similar changes are consistently applied throughout the codebase.Verification successful
Verification successful.
The change to check
workerAutoscaling.enabled
instead ofautoscaling.enabled
is consistent within the scope of its intended use. No further inconsistencies were found across the codebase.
workerAutoscaling
settings are properly referenced invalues.yaml
andhpa.worker.yaml
.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify consistency of `workerAutoscaling.enabled` usage across the codebase. # Test: Search for the usage of `workerAutoscaling.enabled`. Expect: Consistent usage across the codebase. rg --type yaml 'workerAutoscaling.enabled'Length of output: 136
Script:
#!/bin/bash # Description: Verify broader usage of `workerAutoscaling` across the codebase. # Test: Search for the usage of `workerAutoscaling`. Expect: Consistent usage across the codebase. rg --type yaml 'workerAutoscaling'Length of output: 1334
charts/n8n/values.yaml (2)
280-286
: LGTM! TheworkerAutoscaling
section is well-configured.The default values for the
workerAutoscaling
parameters are reasonable, with autoscaling disabled by default. This allows users to enable and configure autoscaling as needed.
290-296
: LGTM! ThewebhookAutoscaling
section is well-configured.The default values for the
webhookAutoscaling
parameters are reasonable, with autoscaling disabled by default. This allows users to enable and configure autoscaling as needed.
charts/n8n/templates/hpa.worker.yaml
Outdated
@@ -0,0 +1,51 @@ | |||
{{- if .Values.autoscaling.enabled }} |
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.
Fix the syntax error in the conditional block.
The static analysis tool flagged a syntax error on line 1. The correct syntax should be:
- {{- if .Values.autoscaling.enabled }}
+ {{- if .Values.autoscaling.enabled -}}
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.
{{- if .Values.autoscaling.enabled }} | |
{{- if .Values.autoscaling.enabled -}} |
Tools
yamllint
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
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.
Original HPA made by the same syntax
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.
@mhkarimi1383 The search results confirm that the original HPA file uses the same syntax {{- if .Values.autoscaling.enabled }}
, so the current syntax is consistent with the existing codebase and does not need to be changed. Thank you for pointing that out!
Analysis chain
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for the original HPA files and inspect their syntax
rg '{{- if .Values.autoscaling.enabled }}' -A 5
Length of output: 162
Script:
#!/bin/bash
# Search for the original HPA files and inspect their syntax with escaped curly braces
rg '\{\{- if \.Values\.autoscaling\.enabled \}\}' -A 5
Length of output: 430
@@ -0,0 +1,51 @@ | |||
{{- if .Values.autoscaling.enabled }} |
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.
Fix the syntax error in the conditional block.
The static analysis tool flagged a syntax error on line 1. The correct syntax should be:
- {{- if .Values.autoscaling.enabled }}
+ {{- if .Values.autoscaling.enabled -}}
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.
{{- if .Values.autoscaling.enabled }} | |
{{- if .Values.autoscaling.enabled -}} |
Tools
yamllint
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
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.
Original HPA made by the same syntax
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.
@mhkarimi1383, I checked the original HPA file (charts/n8n/templates/hpa.yaml
), and it indeed uses the same syntax ({{- if .Values.autoscaling.enabled }}
). It seems that the static analysis tool might have flagged a false positive in this context. Based on this, the existing syntax should be acceptable.
Analysis chain
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for original HPA files in the repository to verify their syntax.
fd -e yaml -e yml -x grep -Hn '{{- if .Values.autoscaling.enabled }}' {}
Length of output: 148
Signed-off-by: Muhammed Hussein Karimi <[email protected]>
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: 4
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- charts/n8n/templates/hpa.webhook.yaml (1 hunks)
- charts/n8n/templates/hpa.worker.yaml (1 hunks)
Additional context used
yamllint
charts/n8n/templates/hpa.worker.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
charts/n8n/templates/hpa.webhook.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
Additional comments not posted (2)
charts/n8n/templates/hpa.worker.yaml (1)
52-53
: LGTM!The closing conditional blocks are correct and necessary for the autoscaling settings.
charts/n8n/templates/hpa.webhook.yaml (1)
52-53
: LGTM!The closing conditional blocks are correct and necessary for the autoscaling settings.
@@ -0,0 +1,53 @@ | |||
{{- if .Values.scaling.enabled }} |
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.
Fix the syntax error in the conditional block.
The static analysis tool flagged a syntax error on line 1. The correct syntax should be:
- {{- if .Values.scaling.enabled }}
+ {{- if .Values.scaling.enabled -}}
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.
{{- if .Values.scaling.enabled }} | |
{{- if .Values.scaling.enabled -}} |
Tools
yamllint
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
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.
Original HPA made by the same syntax
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.
@mhkarimi1383, you are correct. The syntax {{- if .Values.scaling.enabled }}
is consistently used across multiple files in the project, including hpa.worker.yaml
, hpa.webhook.yaml
, and deployment.worker.yaml
. It appears that the static analysis tool might have flagged a false positive. Thank you for pointing this out!
Analysis chain
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for the original HPA configuration files in the repository to verify the syntax.
fd -e yaml -e yml -x grep -Hn '{{- if .Values.scaling.enabled }}' {}
Length of output: 305
spec: | ||
scaleTargetRef: | ||
apiVersion: apps/v1 | ||
kind: Deployment | ||
name: {{ include "n8n.fullname" . }}-worker | ||
minReplicas: {{ .Values.workerAutoscaling.minReplicas }} | ||
maxReplicas: {{ .Values.workerAutoscaling.maxReplicas }} | ||
metrics: | ||
{{- if semverCompare ">=1.25-0" .Capabilities.KubeVersion.GitVersion -}} | ||
{{- if .Values.workerAutoscaling.targetCPUUtilizationPercentage }} | ||
- type: Resource | ||
resource: | ||
name: cpu | ||
target: | ||
type: Utilization | ||
averageUtilization: {{ .Values.workerAutoscaling.targetCPUUtilizationPercentage }} | ||
{{- end }} | ||
{{- if .Values.workerAutoscaling.targetMemoryUtilizationPercentage }} | ||
- type: Resource | ||
resource: | ||
name: memory | ||
target: | ||
type: Utilization | ||
averageUtilization: {{ .Values.workerAutoscaling.targetMemoryUtilizationPercentage }} | ||
{{- end }} | ||
{{- else -}} | ||
{{- if .Values.workerAutoscaling.targetCPUUtilizationPercentage }} | ||
- type: Resource | ||
resource: | ||
name: cpu | ||
targetAverageUtilization: {{ .Values.workerAutoscaling.targetCPUUtilizationPercentage }} | ||
{{- end }} | ||
{{- if .Values.workerAutoscaling.targetMemoryUtilizationPercentage }} | ||
- type: Resource | ||
resource: | ||
name: memory | ||
targetAverageUtilization: {{ .Values.workerAutoscaling.targetMemoryUtilizationPercentage }} | ||
{{- end }} | ||
{{- end }} |
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.
Improve readability of the metrics section.
The metrics section uses nested conditional blocks, which can be simplified for better readability. Consider refactoring to reduce nesting.
- {{- if semverCompare ">=1.25-0" .Capabilities.KubeVersion.GitVersion -}}
- {{- if .Values.workerAutoscaling.targetCPUUtilizationPercentage }}
- - type: Resource
- resource:
- name: cpu
- target:
- type: Utilization
- averageUtilization: {{ .Values.workerAutoscaling.targetCPUUtilizationPercentage }}
- {{- end }}
- {{- if .Values.workerAutoscaling.targetMemoryUtilizationPercentage }}
- - type: Resource
- resource:
- name: memory
- target:
- type: Utilization
- averageUtilization: {{ .Values.workerAutoscaling.targetMemoryUtilizationPercentage }}
- {{- end }}
- {{- else -}}
- {{- if .Values.workerAutoscaling.targetCPUUtilizationPercentage }}
- - type: Resource
- resource:
- name: cpu
- targetAverageUtilization: {{ .Values.workerAutoscaling.targetCPUUtilizationPercentage }}
- {{- end }}
- {{- if .Values.workerAutoscaling.targetMemoryUtilizationPercentage }}
- - type: Resource
- resource:
- name: memory
- targetAverageUtilization: {{ .Values.workerAutoscaling.targetMemoryUtilizationPercentage }}
- {{- end }}
- {{- end }}
+ {{- if semverCompare ">=1.25-0" .Capabilities.KubeVersion.GitVersion -}}
+ {{- $cpuTarget := "Utilization" }}
+ {{- $memoryTarget := "Utilization" }}
+ {{- else -}}
+ {{- $cpuTarget := "targetAverageUtilization" }}
+ {{- $memoryTarget := "targetAverageUtilization" }}
+ {{- end }}
+ {{- if .Values.workerAutoscaling.targetCPUUtilizationPercentage }}
+ - type: Resource
+ resource:
+ name: cpu
+ target:
+ type: {{ $cpuTarget }}
+ averageUtilization: {{ .Values.workerAutoscaling.targetCPUUtilizationPercentage }}
+ {{- end }}
+ {{- if .Values.workerAutoscaling.targetMemoryUtilizationPercentage }}
+ - type: Resource
+ resource:
+ name: memory
+ target:
+ type: {{ $memoryTarget }}
+ averageUtilization: {{ .Values.workerAutoscaling.targetMemoryUtilizationPercentage }}
+ {{- 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.
spec: | |
scaleTargetRef: | |
apiVersion: apps/v1 | |
kind: Deployment | |
name: {{ include "n8n.fullname" . }}-worker | |
minReplicas: {{ .Values.workerAutoscaling.minReplicas }} | |
maxReplicas: {{ .Values.workerAutoscaling.maxReplicas }} | |
metrics: | |
{{- if semverCompare ">=1.25-0" .Capabilities.KubeVersion.GitVersion -}} | |
{{- if .Values.workerAutoscaling.targetCPUUtilizationPercentage }} | |
- type: Resource | |
resource: | |
name: cpu | |
target: | |
type: Utilization | |
averageUtilization: {{ .Values.workerAutoscaling.targetCPUUtilizationPercentage }} | |
{{- end }} | |
{{- if .Values.workerAutoscaling.targetMemoryUtilizationPercentage }} | |
- type: Resource | |
resource: | |
name: memory | |
target: | |
type: Utilization | |
averageUtilization: {{ .Values.workerAutoscaling.targetMemoryUtilizationPercentage }} | |
{{- end }} | |
{{- else -}} | |
{{- if .Values.workerAutoscaling.targetCPUUtilizationPercentage }} | |
- type: Resource | |
resource: | |
name: cpu | |
targetAverageUtilization: {{ .Values.workerAutoscaling.targetCPUUtilizationPercentage }} | |
{{- end }} | |
{{- if .Values.workerAutoscaling.targetMemoryUtilizationPercentage }} | |
- type: Resource | |
resource: | |
name: memory | |
targetAverageUtilization: {{ .Values.workerAutoscaling.targetMemoryUtilizationPercentage }} | |
{{- end }} | |
{{- end }} | |
spec: | |
scaleTargetRef: | |
apiVersion: apps/v1 | |
kind: Deployment | |
name: {{ include "n8n.fullname" . }}-worker | |
minReplicas: {{ .Values.workerAutoscaling.minReplicas }} | |
maxReplicas: {{ .Values.workerAutoscaling.maxReplicas }} | |
metrics: | |
{{- if semverCompare ">=1.25-0" .Capabilities.KubeVersion.GitVersion -}} | |
{{- $cpuTarget := "Utilization" }} | |
{{- $memoryTarget := "Utilization" }} | |
{{- else -}} | |
{{- $cpuTarget := "targetAverageUtilization" }} | |
{{- $memoryTarget := "targetAverageUtilization" }} | |
{{- end }} | |
{{- if .Values.workerAutoscaling.targetCPUUtilizationPercentage }} | |
- type: Resource | |
resource: | |
name: cpu | |
target: | |
type: {{ $cpuTarget }} | |
averageUtilization: {{ .Values.workerAutoscaling.targetCPUUtilizationPercentage }} | |
{{- end }} | |
{{- if .Values.workerAutoscaling.targetMemoryUtilizationPercentage }} | |
- type: Resource | |
resource: | |
name: memory | |
target: | |
type: {{ $memoryTarget }} | |
averageUtilization: {{ .Values.workerAutoscaling.targetMemoryUtilizationPercentage }} | |
{{- end }} |
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.
Original HPA made by the same syntax
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.
@mhkarimi1383, understood. We'll retain the current syntax to ensure consistency with the original HPA.
@@ -0,0 +1,53 @@ | |||
{{- if .Values.scaling.enabled }} |
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.
Fix the syntax error in the conditional block.
The static analysis tool flagged a syntax error on line 1. The correct syntax should be:
- {{- if .Values.scaling.enabled }}
+ {{- if .Values.scaling.enabled -}}
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.
{{- if .Values.scaling.enabled }} | |
{{- if .Values.scaling.enabled -}} |
Tools
yamllint
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
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.
Original HPA made by the same syntax
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.
@mhkarimi1383, you are correct. The original HPA files use the same syntax for the conditional blocks. Maintaining consistency across all HPA files is important. Thank you for pointing that out!
Analysis chain
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Find and display the contents of the original HPA files to check the syntax
fd -e yaml 'hpa.*.yaml' --exec cat {}
Length of output: 5302
spec: | ||
scaleTargetRef: | ||
apiVersion: apps/v1 | ||
kind: Deployment | ||
name: {{ include "n8n.fullname" . }}-webhook | ||
minReplicas: {{ .Values.webhookAutoscaling.minReplicas }} | ||
maxReplicas: {{ .Values.webhookAutoscaling.maxReplicas }} | ||
metrics: | ||
{{- if semverCompare ">=1.25-0" .Capabilities.KubeVersion.GitVersion -}} | ||
{{- if .Values.webhookAutoscaling.targetCPUUtilizationPercentage }} | ||
- type: Resource | ||
resource: | ||
name: cpu | ||
target: | ||
type: Utilization | ||
averageUtilization: {{ .Values.webhookAutoscaling.targetCPUUtilizationPercentage }} | ||
{{- end }} | ||
{{- if .Values.webhookAutoscaling.targetMemoryUtilizationPercentage }} | ||
- type: Resource | ||
resource: | ||
name: memory | ||
target: | ||
type: Utilization | ||
averageUtilization: {{ .Values.webhookAutoscaling.targetMemoryUtilizationPercentage }} | ||
{{- end }} | ||
{{- else -}} | ||
{{- if .Values.webhookAutoscaling.targetCPUUtilizationPercentage }} | ||
- type: Resource | ||
resource: | ||
name: cpu | ||
targetAverageUtilization: {{ .Values.webhookAutoscaling.targetCPUUtilizationPercentage }} | ||
{{- end }} | ||
{{- if .Values.webhookAutoscaling.targetMemoryUtilizationPercentage }} | ||
- type: Resource | ||
resource: | ||
name: memory | ||
targetAverageUtilization: {{ .Values.webhookAutoscaling.targetMemoryUtilizationPercentage }} | ||
{{- end }} | ||
{{- end }} |
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.
Improve readability of the metrics section.
The metrics section uses nested conditional blocks, which can be simplified for better readability. Consider refactoring to reduce nesting.
- {{- if semverCompare ">=1.25-0" .Capabilities.KubeVersion.GitVersion -}}
- {{- if .Values.webhookAutoscaling.targetCPUUtilizationPercentage }}
- - type: Resource
- resource:
- name: cpu
- target:
- type: Utilization
- averageUtilization: {{ .Values.webhookAutoscaling.targetCPUUtilizationPercentage }}
- {{- end }}
- {{- if .Values.webhookAutoscaling.targetMemoryUtilizationPercentage }}
- - type: Resource
- resource:
- name: memory
- target:
- type: Utilization
- averageUtilization: {{ .Values.webhookAutoscaling.targetMemoryUtilizationPercentage }}
- {{- end }}
- {{- else -}}
- {{- if .Values.webhookAutoscaling.targetCPUUtilizationPercentage }}
- - type: Resource
- resource:
- name: cpu
- targetAverageUtilization: {{ .Values.webhookAutoscaling.targetCPUUtilizationPercentage }}
- {{- end }}
- {{- if .Values.webhookAutoscaling.targetMemoryUtilizationPercentage }}
- - type: Resource
- resource:
- name: memory
- targetAverageUtilization: {{ .Values.webhookAutoscaling.targetMemoryUtilizationPercentage }}
- {{- end }}
- {{- end }}
+ {{- if semverCompare ">=1.25-0" .Capabilities.KubeVersion.GitVersion -}}
+ {{- $cpuTarget := "Utilization" }}
+ {{- $memoryTarget := "Utilization" }}
+ {{- else -}}
+ {{- $cpuTarget := "targetAverageUtilization" }}
+ {{- $memoryTarget := "targetAverageUtilization" }}
+ {{- end }}
+ {{- if .Values.webhookAutoscaling.targetCPUUtilizationPercentage }}
+ - type: Resource
+ resource:
+ name: cpu
+ target:
+ type: {{ $cpuTarget }}
+ averageUtilization: {{ .Values.webhookAutoscaling.targetCPUUtilizationPercentage }}
+ {{- end }}
+ {{- if .Values.webhookAutoscaling.targetMemoryUtilizationPercentage }}
+ - type: Resource
+ resource:
+ name: memory
+ target:
+ type: {{ $memoryTarget }}
+ averageUtilization: {{ .Values.webhookAutoscaling.targetMemoryUtilizationPercentage }}
+ {{- 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.
spec: | |
scaleTargetRef: | |
apiVersion: apps/v1 | |
kind: Deployment | |
name: {{ include "n8n.fullname" . }}-webhook | |
minReplicas: {{ .Values.webhookAutoscaling.minReplicas }} | |
maxReplicas: {{ .Values.webhookAutoscaling.maxReplicas }} | |
metrics: | |
{{- if semverCompare ">=1.25-0" .Capabilities.KubeVersion.GitVersion -}} | |
{{- if .Values.webhookAutoscaling.targetCPUUtilizationPercentage }} | |
- type: Resource | |
resource: | |
name: cpu | |
target: | |
type: Utilization | |
averageUtilization: {{ .Values.webhookAutoscaling.targetCPUUtilizationPercentage }} | |
{{- end }} | |
{{- if .Values.webhookAutoscaling.targetMemoryUtilizationPercentage }} | |
- type: Resource | |
resource: | |
name: memory | |
target: | |
type: Utilization | |
averageUtilization: {{ .Values.webhookAutoscaling.targetMemoryUtilizationPercentage }} | |
{{- end }} | |
{{- else -}} | |
{{- if .Values.webhookAutoscaling.targetCPUUtilizationPercentage }} | |
- type: Resource | |
resource: | |
name: cpu | |
targetAverageUtilization: {{ .Values.webhookAutoscaling.targetCPUUtilizationPercentage }} | |
{{- end }} | |
{{- if .Values.webhookAutoscaling.targetMemoryUtilizationPercentage }} | |
- type: Resource | |
resource: | |
name: memory | |
targetAverageUtilization: {{ .Values.webhookAutoscaling.targetMemoryUtilizationPercentage }} | |
{{- end }} | |
{{- end }} | |
spec: | |
scaleTargetRef: | |
apiVersion: apps/v1 | |
kind: Deployment | |
name: {{ include "n8n.fullname" . }}-webhook | |
minReplicas: {{ .Values.webhookAutoscaling.minReplicas }} | |
maxReplicas: {{ .Values.webhookAutoscaling.maxReplicas }} | |
metrics: | |
{{- if semverCompare ">=1.25-0" .Capabilities.KubeVersion.GitVersion -}} | |
{{- $cpuTarget := "Utilization" }} | |
{{- $memoryTarget := "Utilization" }} | |
{{- else -}} | |
{{- $cpuTarget := "targetAverageUtilization" }} | |
{{- $memoryTarget := "targetAverageUtilization" }} | |
{{- end }} | |
{{- if .Values.webhookAutoscaling.targetCPUUtilizationPercentage }} | |
- type: Resource | |
resource: | |
name: cpu | |
target: | |
type: {{ $cpuTarget }} | |
averageUtilization: {{ .Values.webhookAutoscaling.targetCPUUtilizationPercentage }} | |
{{- end }} | |
{{- if .Values.webhookAutoscaling.targetMemoryUtilizationPercentage }} | |
- type: Resource | |
resource: | |
name: memory | |
target: | |
type: {{ $memoryTarget }} | |
averageUtilization: {{ .Values.webhookAutoscaling.targetMemoryUtilizationPercentage }} | |
{{- end }} |
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.
Original HPA made by the same syntax
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.
@mhkarimi1383, understood. Maintaining consistency with the existing HPA syntax is important. We'll keep the current structure as it aligns with the rest of the codebase.
@Vad1mo |
Thank you for the contribution, I'll test it out next week, when I am back. Maybe some other community members can give this PR a try too. |
@Vad1mo |
@mhkarimi1383 how does this relate to #110? |
Any updates on this @Vad1mo? |
I would need to test it, did you guys use it? How does the values file look like? |
I have done See two parts are added for auto scaling worker and webhook deployments (webhookAutoscaling and workerAutoscaling) config looks like older autoScaling |
Testing this PR in our development environment, and I'll provide feedback after testing |
Looks good to me! ❯ helm unittest .
### Chart [ n8n ] .
Charts: 1 passed, 1 total
Test Suites: 0 passed, 0 total
Tests: 0 passed, 0 total
Snapshot: 0 passed, 0 total
Time: 1.944917ms NAME REFERENCE TARGETS MINPODS MAXPODS REPLICAS AGE
n8n-webhook Deployment/n8n-webhook 60% / 80% 2 10 2 5m
n8n-worker Deployment/n8n-worker 75% / 80% 1 10 3 5m All of the values populate as intended as well |
By adding
webhookAutoscaling
andworkerAutoscaling
HPA will work for webhook and worker also fixes the problem where when autoscaling is enabled replicaCount will not work for webhook and worker
closes #80
closes #87
Summary by CodeRabbit
New Features
Bug Fixes