Skip to content
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.

Sign up for GitHub

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

Jump to bottom

✨ feat: worker and webhook autosacling #112

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
+123 −2
Open

✨ feat: worker and webhook autosacling #112

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a5ca708
✨ feat: worker and webhook autosacling
mhkarimi1383 Aug 5, 2024
101e568
✏️ typo: also check if scaling is enabled in worker and webhook hpa
mhkarimi1383 Aug 5, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion charts/n8n/templates/deployment.webhooks.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ metadata:
labels:
{{- include "n8n.labels" . | nindent 4 }}
spec:
{{- if not .Values.autoscaling.enabled }}
{{- if not .Values.webhookAutoscaling.enabled }}
replicas: {{ .Values.scaling.webhook.count }}
{{- end }}
strategy:
Expand Down
2 changes: 1 addition & 1 deletion charts/n8n/templates/deployment.worker.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ metadata:
labels:
{{- include "n8n.labels" . | nindent 4 }}
spec:
{{- if not .Values.autoscaling.enabled }}
{{- if not .Values.workerAutoscaling.enabled }}
replicas: {{ .Values.scaling.worker.count }}
{{- end }}
strategy:
Expand Down
53 changes: 53 additions & 0 deletions charts/n8n/templates/hpa.webhook.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
{{- if .Values.scaling.enabled }}
Copy link

@coderabbitai coderabbitai bot Aug 5, 2024 •
edited
Loading

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.

Suggested change
{{- if .Values.scaling.enabled }}
{{- if .Values.scaling.enabled -}}
Tools
yamllint

[error] 1-1: syntax error: expected the node content, but found '-'

(syntax)

Copy link
Author

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

Copy link

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

{{- if .Values.webhookAutoscaling.enabled }}
{{- if semverCompare ">=1.25-0" .Capabilities.KubeVersion.GitVersion -}}
apiVersion: autoscaling/v2
{{- else -}}
apiVersion: autoscaling/v2beta1
{{- end }}
kind: HorizontalPodAutoscaler
metadata:
name: {{ include "n8n.fullname" . }}-webhook
labels:
{{- include "n8n.labels" . | nindent 4 }}
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 }}
Comment on lines +13 to +51
Copy link

@coderabbitai coderabbitai bot Aug 5, 2024 •
edited
Loading

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.

Suggested change
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 }}

Copy link
Author

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

Copy link

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.

{{- end }}
{{- end }}
53 changes: 53 additions & 0 deletions charts/n8n/templates/hpa.worker.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
{{- if .Values.scaling.enabled }}
Copy link

@coderabbitai coderabbitai bot Aug 5, 2024 •
edited
Loading

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.

Suggested change
{{- if .Values.scaling.enabled }}
{{- if .Values.scaling.enabled -}}
Tools
yamllint

[error] 1-1: syntax error: expected the node content, but found '-'

(syntax)

Copy link
Author

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

Copy link

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

{{- if .Values.workerAutoscaling.enabled }}
{{- if semverCompare ">=1.25-0" .Capabilities.KubeVersion.GitVersion -}}
apiVersion: autoscaling/v2
{{- else -}}
apiVersion: autoscaling/v2beta1
{{- end }}
kind: HorizontalPodAutoscaler
metadata:
name: {{ include "n8n.fullname" . }}-worker
labels:
{{- include "n8n.labels" . | nindent 4 }}
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 }}
Comment on lines +13 to +51
Copy link

@coderabbitai coderabbitai bot Aug 5, 2024 •
edited
Loading

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.

Suggested change
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 }}

Copy link
Author

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

Copy link

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.

{{- end }}
{{- end }}
15 changes: 15 additions & 0 deletions charts/n8n/values.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -277,9 +277,24 @@ ingress:
workerResources:
{}

workerAutoscaling:
enabled: false
minReplicas: 1
maxReplicas: 100
targetCPUUtilizationPercentage: 80
# targetMemoryUtilizationPercentage: 80

webhookResources:
{}

webhookAutoscaling:
enabled: false
minReplicas: 1
maxReplicas: 100
targetCPUUtilizationPercentage: 80
# targetMemoryUtilizationPercentage: 80


resources:
{}
# We usually recommend not to specify default resources and to leave this as a conscious
Expand Down