Skip to content

Commit 29d48e3

Browse files
nowjeannowjean
committed
Code Review
1 parent 444740c commit 29d48e3

File tree

7 files changed

+18
-54
lines changed

7 files changed

+18
-54
lines changed

‎apis/v1alpha2/nginxproxy_types.go‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -466,13 +466,13 @@ type DaemonSetSpec struct {
466466
Patches []Patch `json:"patches,omitempty"`
467467
}
468468

469-
// +kubebuilder:validation:XValidation:message="at least one metric must be specified when autoscaling is enabled",rule="!self.enabled || (has(self.targetCPUUtilizationPercentage) || has(self.targetMemoryUtilizationPercentage) || (has(self.autoscalingTemplate) && size(self.autoscalingTemplate) > 0))"
470-
// +kubebuilder:validation:XValidation:message="minReplicas must be less than or equal to maxReplicas",rule="self.minReplicas <= self.maxReplicas"
471-
// +kubebuilder:validation:XValidation:message="CPU utilization must be between 1 and 100",rule="!has(self.targetCPUUtilizationPercentage) || (self.targetCPUUtilizationPercentage >= 1 && self.targetCPUUtilizationPercentage <= 100)"
472-
// +kubebuilder:validation:XValidation:message="memory utilization must be between 1 and 100",rule="!has(self.targetMemoryUtilizationPercentage) || (self.targetMemoryUtilizationPercentage >= 1 && self.targetMemoryUtilizationPercentage <= 100)"
473-
//
474469
// HPASpec is the configuration for the Horizontal Pod Autoscaling.
475470
//
471+
// +kubebuilder:message="at least one metric must be specified when autoscaling is enabled",rule="!self.enabled || (has(self.targetCPUUtilizationPercentage) || has(self.targetMemoryUtilizationPercentage) || (has(self.autoscalingTemplate) && size(self.autoscalingTemplate) > 0))"
472+
// +kubebuilder:message="minReplicas must be less than or equal to maxReplicas",rule="self.minReplicas <= self.maxReplicas"
473+
// +kubebuilder:message="CPU utilization must be between 1 and 100",rule="!has(self.targetCPUUtilizationPercentage) || (self.targetCPUUtilizationPercentage >= 1 && self.targetCPUUtilizationPercentage <= 100)"
474+
// +kubebuilder:message="memory utilization must be between 1 and 100",rule="!has(self.targetMemoryUtilizationPercentage) || (self.targetMemoryUtilizationPercentage >= 1 && self.targetMemoryUtilizationPercentage <= 100)"
475+
//
476476
//nolint:lll
477477
type HPASpec struct {
478478
// Behavior configures the scaling behavior of the target
@@ -485,7 +485,7 @@ type HPASpec struct {
485485
// AutoscalingTemplate configures the additional scaling option.
486486
//
487487
// +optional
488-
AutoscalingTemplate *[]autoscalingv2.MetricSpec `json:"autoscalingTemplate,omitempty"`
488+
AutoscalingTemplate []autoscalingv2.MetricSpec `json:"autoscalingTemplate,omitempty"`
489489

490490
// Target cpu utilization percentage of HPA.
491491
//

‎apis/v1alpha2/zz_generated.deepcopy.go‎

Lines changed: 3 additions & 7 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎charts/nginx-gateway-fabric/templates/hpa.yaml‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,9 @@ spec:
1717
apiVersion: apps/v1
1818
kind: Deployment
1919
name: {{ include "nginx-gateway.fullname" . }}
20+
{{- if .Values.nginxGateway.autoscaling.minReplicas }}
2021
minReplicas: {{ .Values.nginxGateway.autoscaling.minReplicas }}
22+
{{- end }}
2123
maxReplicas: {{ .Values.nginxGateway.autoscaling.maxReplicas }}
2224
metrics:
2325
{{- with .Values.nginxGateway.autoscaling.targetMemoryUtilizationPercentage }}

‎charts/nginx-gateway-fabric/templates/nginxproxy.yaml‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,9 @@ spec:
1818
{{- if .Values.nginx.autoscaling.enabled }}
1919
autoscaling:
2020
enabled: {{ .Values.nginx.autoscaling.enabled }}
21+
{{- if .Values.nginx.autoscaling.minReplicas }}
2122
minReplicas: {{ .Values.nginx.autoscaling.minReplicas }}
23+
{{- end }}
2224
maxReplicas: {{ .Values.nginx.autoscaling.maxReplicas }}
2325
{{- if .Values.nginx.autoscaling.targetCPUUtilizationPercentage }}
2426
targetCPUUtilizationPercentage: {{ .Values.nginx.autoscaling.targetCPUUtilizationPercentage }}

‎config/crd/bases/gateway.nginx.org_nginxproxies.yaml‎

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -4192,20 +4192,6 @@ spec:
41924192
- enabled
41934193
- maxReplicas
41944194
type: object
4195-
x-kubernetes-validations:
4196-
- message: at least one metric must be specified when autoscaling
4197-
is enabled
4198-
rule: '!self.enabled || (has(self.targetCPUUtilizationPercentage)
4199-
|| has(self.targetMemoryUtilizationPercentage) || (has(self.autoscalingTemplate)
4200-
&& size(self.autoscalingTemplate) > 0))'
4201-
- message: minReplicas must be less than or equal to maxReplicas
4202-
rule: self.minReplicas <= self.maxReplicas
4203-
- message: CPU utilization must be between 1 and 100
4204-
rule: '!has(self.targetCPUUtilizationPercentage) || (self.targetCPUUtilizationPercentage
4205-
>= 1 && self.targetCPUUtilizationPercentage <= 100)'
4206-
- message: memory utilization must be between 1 and 100
4207-
rule: '!has(self.targetMemoryUtilizationPercentage) || (self.targetMemoryUtilizationPercentage
4208-
>= 1 && self.targetMemoryUtilizationPercentage <= 100)'
42094195
container:
42104196
description: Container defines container fields for the NGINX
42114197
container.

‎deploy/crds.yaml‎

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -4777,20 +4777,6 @@ spec:
47774777
- enabled
47784778
- maxReplicas
47794779
type: object
4780-
x-kubernetes-validations:
4781-
- message: at least one metric must be specified when autoscaling
4782-
is enabled
4783-
rule: '!self.enabled || (has(self.targetCPUUtilizationPercentage)
4784-
|| has(self.targetMemoryUtilizationPercentage) || (has(self.autoscalingTemplate)
4785-
&& size(self.autoscalingTemplate) > 0))'
4786-
- message: minReplicas must be less than or equal to maxReplicas
4787-
rule: self.minReplicas <= self.maxReplicas
4788-
- message: CPU utilization must be between 1 and 100
4789-
rule: '!has(self.targetCPUUtilizationPercentage) || (self.targetCPUUtilizationPercentage
4790-
>= 1 && self.targetCPUUtilizationPercentage <= 100)'
4791-
- message: memory utilization must be between 1 and 100
4792-
rule: '!has(self.targetMemoryUtilizationPercentage) || (self.targetMemoryUtilizationPercentage
4793-
>= 1 && self.targetMemoryUtilizationPercentage <= 100)'
47944780
container:
47954781
description: Container defines container fields for the NGINX
47964782
container.

‎internal/controller/provisioner/objects.go‎

Lines changed: 5 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1101,12 +1101,10 @@ func buildNginxDeploymentHPA(
11011101
})
11021102
}
11031103

1104-
if autoscalingTemplate != nil {
1105-
for _, additionalAutoscaling := range *autoscalingTemplate {
1106-
metric := buildAdditionalMetric(additionalAutoscaling)
1107-
if metric != nil {
1108-
metrics = append(metrics, *metric)
1109-
}
1104+
for _, additionalAutoscaling := range autoscalingTemplate {
1105+
metric := buildAdditionalMetric(additionalAutoscaling)
1106+
if metric != nil {
1107+
metrics = append(metrics, *metric)
11101108
}
11111109
}
11121110

@@ -1193,11 +1191,11 @@ func (p *NginxProvisioner) buildNginxResourceObjectsForDeletion(deploymentNSName
11931191
// order to delete:
11941192
// deployment/daemonset
11951193
// service
1194+
// hpa
11961195
// role/binding (if openshift)
11971196
// serviceaccount
11981197
// configmaps
11991198
// secrets
1200-
// hpa
12011199

12021200
objectMeta := metav1.ObjectMeta{
12031201
Name: deploymentNSName.Name,
@@ -1219,12 +1217,6 @@ func (p *NginxProvisioner) buildNginxResourceObjectsForDeletion(deploymentNSName
12191217

12201218
objects := []client.Object{deployment, daemonSet, service, hpa}
12211219

1222-
// objects := []client.Object{deployment, daemonSet, service}
1223-
1224-
// // if hpa := p.buildHPA(objectMeta, nProxyCfg); hpa != nil {
1225-
// // objects = append(objects, hpa)
1226-
// // }
1227-
12281220
if p.isOpenshift {
12291221
role := &rbacv1.Role{
12301222
ObjectMeta: objectMeta,

0 commit comments

Comments
 (0)