Skip to content

Commit ab7cf51

Browse files
nowjeannowjean
committed
Code Review and add unit test
1 parent 1affd73 commit ab7cf51

File tree

5 files changed

+41
-27
lines changed

5 files changed

+41
-27
lines changed

‎apis/v1alpha2/nginxproxy_types.go‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -470,8 +470,6 @@ type DaemonSetSpec struct {
470470
//
471471
// +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))"
472472
// +kubebuilder:validation:XValidation:message="minReplicas must be less than or equal to maxReplicas",rule="self.minReplicas <= self.maxReplicas"
473-
// +kubebuilder:validation:XValidation:message="CPU utilization must be between 1 and 100",rule="!has(self.targetCPUUtilizationPercentage) || (self.targetCPUUtilizationPercentage >= 1 && self.targetCPUUtilizationPercentage <= 100)"
474-
// +kubebuilder:validation:XValidation:message="memory utilization must be between 1 and 100",rule="!has(self.targetMemoryUtilizationPercentage) || (self.targetMemoryUtilizationPercentage >= 1 && self.targetMemoryUtilizationPercentage <= 100)"
475473
//
476474
//nolint:lll
477475
type HPASpec struct {
@@ -490,19 +488,25 @@ type HPASpec struct {
490488
// Target cpu utilization percentage of HPA.
491489
//
492490
// +optional
491+
// +kubebuilder:validation:Minimum=1
492+
// +kubebuilder:validation:Maximum=100
493493
TargetCPUUtilizationPercentage *int32 `json:"targetCPUUtilizationPercentage,omitempty"`
494494

495495
// Target memory utilization percentage of HPA.
496496
//
497497
// +optional
498+
// +kubebuilder:validation:Minimum=1
499+
// +kubebuilder:validation:Maximum=100
498500
TargetMemoryUtilizationPercentage *int32 `json:"targetMemoryUtilizationPercentage,omitempty"`
499501

500502
// Minimum number of replicas.
501503
//
502504
// +optional
505+
// +kubebuilder:validation:Minimum=1
503506
MinReplicas *int32 `json:"minReplicas,omitempty"`
504507

505508
// Maximum number of replicas.
509+
// +kubebuilder:validation:Minimum=1
506510
MaxReplicas int32 `json:"maxReplicas"`
507511

508512
// Enable or disable Horizontal Pod Autoscaler

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

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4175,18 +4175,24 @@ spec:
41754175
maxReplicas:
41764176
description: Maximum number of replicas.
41774177
format: int32
4178+
minimum: 1
41784179
type: integer
41794180
minReplicas:
41804181
description: Minimum number of replicas.
41814182
format: int32
4183+
minimum: 1
41824184
type: integer
41834185
targetCPUUtilizationPercentage:
41844186
description: Target cpu utilization percentage of HPA.
41854187
format: int32
4188+
maximum: 100
4189+
minimum: 1
41864190
type: integer
41874191
targetMemoryUtilizationPercentage:
41884192
description: Target memory utilization percentage of HPA.
41894193
format: int32
4194+
maximum: 100
4195+
minimum: 1
41904196
type: integer
41914197
required:
41924198
- enabled
@@ -4200,12 +4206,6 @@ spec:
42004206
&& size(self.autoscalingTemplate) > 0))'
42014207
- message: minReplicas must be less than or equal to maxReplicas
42024208
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)'
42094209
container:
42104210
description: Container defines container fields for the NGINX
42114211
container.

‎deploy/crds.yaml‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4760,18 +4760,24 @@ spec:
47604760
maxReplicas:
47614761
description: Maximum number of replicas.
47624762
format: int32
4763+
minimum: 1
47634764
type: integer
47644765
minReplicas:
47654766
description: Minimum number of replicas.
47664767
format: int32
4768+
minimum: 1
47674769
type: integer
47684770
targetCPUUtilizationPercentage:
47694771
description: Target cpu utilization percentage of HPA.
47704772
format: int32
4773+
maximum: 100
4774+
minimum: 1
47714775
type: integer
47724776
targetMemoryUtilizationPercentage:
47734777
description: Target memory utilization percentage of HPA.
47744778
format: int32
4779+
maximum: 100
4780+
minimum: 1
47754781
type: integer
47764782
required:
47774783
- enabled
@@ -4785,12 +4791,6 @@ spec:
47854791
&& size(self.autoscalingTemplate) > 0))'
47864792
- message: minReplicas must be less than or equal to maxReplicas
47874793
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)'
47944794
container:
47954795
description: Container defines container fields for the NGINX
47964796
container.

‎internal/controller/provisioner/objects.go‎

Lines changed: 16 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -614,24 +614,27 @@ func (p *NginxProvisioner) buildNginxDeployment(
614614
}
615615
}
616616

617+
var replicas *int32
617618
if deploymentCfg.Replicas != nil {
618-
replicas := deploymentCfg.Replicas
619+
replicas = deploymentCfg.Replicas
620+
}
619621

620-
if isAutoscalingEnabled(&deploymentCfg) {
621-
ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
622-
defer cancel()
622+
if isAutoscalingEnabled(&deploymentCfg) {
623+
ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
624+
defer cancel()
623625

624-
hpa := &autoscalingv2.HorizontalPodAutoscaler{}
625-
err := p.k8sClient.Get(ctx, types.NamespacedName{
626-
Namespace: objectMeta.Namespace,
627-
Name: objectMeta.Name,
628-
}, hpa)
629-
if err == nil && hpa.Status.DesiredReplicas > 0 {
630-
// overwrite with HPA's desiredReplicas
631-
replicas = helpers.GetPointer(hpa.Status.DesiredReplicas)
632-
}
626+
hpa := &autoscalingv2.HorizontalPodAutoscaler{}
627+
err := p.k8sClient.Get(ctx, types.NamespacedName{
628+
Namespace: objectMeta.Namespace,
629+
Name: objectMeta.Name,
630+
}, hpa)
631+
if err == nil && hpa.Status.DesiredReplicas > 0 {
632+
// overwrite with HPA's desiredReplicas
633+
replicas = helpers.GetPointer(hpa.Status.DesiredReplicas)
633634
}
635+
}
634636

637+
if replicas != nil {
635638
deployment.Spec.Replicas = replicas
636639
}
637640

‎internal/controller/provisioner/objects_test.go‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -374,6 +374,13 @@ func TestBuildNginxResourceObjects_NginxProxyConfig(t *testing.T) {
374374
g.Expect(container.ReadinessProbe.HTTPGet.Path).To(Equal("/readyz"))
375375
g.Expect(container.ReadinessProbe.HTTPGet.Port).To(Equal(intstr.FromInt(9091)))
376376
g.Expect(container.ReadinessProbe.InitialDelaySeconds).To(Equal(int32(5)))
377+
378+
hpaObj := objects[6]
379+
hpa, ok := hpaObj.(*autoscalingv2.HorizontalPodAutoscaler)
380+
g.Expect(ok).To(BeTrue())
381+
g.Expect(hpa.Spec.MinReplicas).ToNot(BeNil())
382+
g.Expect(*hpa.Spec.MinReplicas).To(Equal(int32(1)))
383+
g.Expect(hpa.Spec.MaxReplicas).To(Equal(int32(5)))
377384
}
378385

379386
func TestBuildNginxResourceObjects_Plus(t *testing.T) {

0 commit comments

Comments
 (0)