-
Notifications
You must be signed in to change notification settings - Fork 127
Hpa #3492
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?
Hpa #3492
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -1,6 +1,7 @@ | ||||||||||||||
package v1alpha2 | ||||||||||||||
|
||||||||||||||
import ( | ||||||||||||||
autoscalingv2 "k8s.io/api/autoscaling/v2" | ||||||||||||||
corev1 "k8s.io/api/core/v1" | ||||||||||||||
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" | ||||||||||||||
|
||||||||||||||
|
@@ -388,6 +389,11 @@ type DeploymentSpec struct { | |||||||||||||
// +optional | ||||||||||||||
Replicas *int32 `json:"replicas,omitempty"` | ||||||||||||||
|
||||||||||||||
// Horizontal Pod Autoscaling. | ||||||||||||||
// | ||||||||||||||
// +optional | ||||||||||||||
Autoscaling HPASpec `json:"autoscaling"` | ||||||||||||||
|
||||||||||||||
// Pod defines Pod-specific fields. | ||||||||||||||
// | ||||||||||||||
// +optional | ||||||||||||||
|
@@ -412,6 +418,47 @@ type DaemonSetSpec struct { | |||||||||||||
Container ContainerSpec `json:"container"` | ||||||||||||||
} | ||||||||||||||
|
||||||||||||||
type HPASpec struct { | ||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sorry - this comment went missing in my last review! This enables validation to ensure we have at least one metrics source, requires the percentages are between 1 and 100, and ensures min replicas is less than max replicas
Suggested change
|
||||||||||||||
// behavior configures the scaling behavior of the target | ||||||||||||||
// in both Up and Down directions (scaleUp and scaleDown fields respectively). | ||||||||||||||
// If not set, the default HPAScalingRules for scale up and scale down are used. | ||||||||||||||
// | ||||||||||||||
// +optional | ||||||||||||||
Behavior *autoscalingv2.HorizontalPodAutoscalerBehavior `json:"behavior,omitempty"` | ||||||||||||||
|
||||||||||||||
// AutoscalingTemplate configures the additional scaling option. | ||||||||||||||
// | ||||||||||||||
// +optional | ||||||||||||||
AutoscalingTemplate *[]autoscalingv2.MetricSpec `json:"autoscalingTemplate,omitempty"` | ||||||||||||||
|
||||||||||||||
// Target cpu utilization percentage of HPA. | ||||||||||||||
// | ||||||||||||||
// +optional | ||||||||||||||
TargetCPUUtilizationPercentage *int32 `json:"targetCPUUtilizationPercentage,omitempty"` | ||||||||||||||
|
||||||||||||||
// Target memory utilization percentage of HPA. | ||||||||||||||
// | ||||||||||||||
// +optional | ||||||||||||||
TargetMemoryUtilizationPercentage *int32 `json:"targetMemoryUtilizationPercentage,omitempty"` | ||||||||||||||
|
||||||||||||||
// Annotation for Horizontal Pod Autoscaler | ||||||||||||||
// Annotations is an unstructured key value map stored with a resource that may be | ||||||||||||||
// set by external tools to store and retrieve arbitrary metadata. They are not | ||||||||||||||
// queryable and should be preserved when modifying objects. | ||||||||||||||
// More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/annotations | ||||||||||||||
// +optional | ||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||
HPAAnnotations map[string]string `json:"hpaAnnotations,omitempty"` | ||||||||||||||
|
||||||||||||||
// Minimun of replicas. | ||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should enable validation here
Suggested change
|
||||||||||||||
MinReplicas int32 `json:"minReplicas"` | ||||||||||||||
|
||||||||||||||
// Minimun of replicas. | ||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should enable validation here
Suggested change
|
||||||||||||||
MaxReplicas int32 `json:"maxReplicas"` | ||||||||||||||
|
||||||||||||||
// Enable or disable Horizontal Pod Autoscaler | ||||||||||||||
Enabled bool `json:"enabled"` | ||||||||||||||
} | ||||||||||||||
|
||||||||||||||
// PodSpec defines Pod-specific fields. | ||||||||||||||
type PodSpec struct { | ||||||||||||||
// TerminationGracePeriodSeconds is the optional duration in seconds the pod needs to terminate gracefully. | ||||||||||||||
|
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,46 @@ | ||
{{- if and (eq .Values.nginxGateway.kind "deployment") .Values.nginxGateway.autoscaling.enabled -}} | ||
apiVersion: {{ ternary "autoscaling/v2" "autoscaling/v2beta2" (.Capabilities.APIVersions.Has "autoscaling/v2") }} | ||
kind: HorizontalPodAutoscaler | ||
metadata: | ||
{{- with .Values.nginxGateway.autoscaling.annotations }} | ||
annotations: {{ toYaml . | nindent 4 }} | ||
{{- end }} | ||
labels: | ||
{{- include "nginx-gateway.labels" . | nindent 4 }} | ||
{{- with .Values.nginxGateway.labels }} | ||
{{- toYaml . | nindent 4 }} | ||
{{- end }} | ||
name: {{ include "nginx-gateway.fullname" . }} | ||
namespace: {{ .Release.Namespace }} | ||
spec: | ||
scaleTargetRef: | ||
apiVersion: apps/v1 | ||
kind: Deployment | ||
name: {{ include "nginx-gateway.fullname" . }} | ||
minReplicas: {{ .Values.nginxGateway.autoscaling.minReplicas }} | ||
maxReplicas: {{ .Values.nginxGateway.autoscaling.maxReplicas }} | ||
metrics: | ||
{{- with .Values.nginxGateway.autoscaling.targetMemoryUtilizationPercentage }} | ||
- type: Resource | ||
resource: | ||
name: memory | ||
target: | ||
type: Utilization | ||
averageUtilization: {{ . }} | ||
{{- end }} | ||
{{- with .Values.nginxGateway.autoscaling.targetCPUUtilizationPercentage }} | ||
- type: Resource | ||
resource: | ||
name: cpu | ||
target: | ||
type: Utilization | ||
averageUtilization: {{ . }} | ||
{{- end }} | ||
{{- with .Values.autoscalingTemplate }} | ||
{{- toYaml . | nindent 2 }} | ||
{{- end }} | ||
{{- with .Values.nginxGateway.autoscaling.behavior }} | ||
behavior: | ||
{{- toYaml . | nindent 4 }} | ||
{{- end }} | ||
{{- end }} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -12,7 +12,27 @@ spec: | |
kubernetes: | ||
{{- if eq .Values.nginx.kind "deployment" }} | ||
deployment: | ||
{{- if .Values.nginx.replicas }} | ||
replicas: {{ .Values.nginx.replicas }} | ||
{{- end }} | ||
autoscaling: | ||
enabled: {{ .Values.nginx.autoscaling.enabled }} | ||
{{- if .Values.nginx.autoscaling.hpaAnnotations }} | ||
hpaAnnotations: | ||
{{- toYaml .Values.nginx.autoscaling.hpaAnnotations | nindent 10 }} | ||
{{- end }} | ||
minReplicas: {{ .Values.nginx.autoscaling.minReplicas }} | ||
maxReplicas: {{ .Values.nginx.autoscaling.maxReplicas }} | ||
targetCPUUtilizationPercentage: {{ .Values.nginx.autoscaling.targetCPUUtilizationPercentage }} | ||
targetMemoryUtilizationPercentage: {{ .Values.nginx.autoscaling.targetMemoryUtilizationPercentage }} | ||
{{- if .Values.nginx.autoscaling.behavior }} | ||
behavior: | ||
{{- toYaml .Values.nginx.autoscaling.behavior | nindent 10 }} | ||
{{- end }} | ||
{{- if .Values.nginx.autoscalingTemplate }} | ||
autoscalingTemplate: | ||
{{- toYaml .Values.nginx.autoscalingTemplate | nindent 8 }} | ||
{{- end }} | ||
Comment on lines
+18
to
+35
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should really only be generating the autoscaling spec in the NginxProxy resource if it's enabled. Similarly,
|
||
{{- if .Values.nginx.pod }} | ||
pod: | ||
{{- toYaml .Values.nginx.pod | nindent 8 }} | ||
|
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.
As mentioned in the overall review comment, please remove this for now and regenerate the CRDs