Skip to content

Commit 8112cf4

Browse files
sjbermansjberman
committed
Add unit test; fix some other issues
1 parent b8e035b commit 8112cf4

File tree

7 files changed

+118
-41
lines changed

7 files changed

+118
-41
lines changed

‎apis/v1alpha2/nginxproxy_types.go‎

Lines changed: 16 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -431,30 +431,30 @@ const (
431431

432432
// Deployment is the configuration for the NGINX Deployment.
433433
type DeploymentSpec struct {
434-
// Container defines container fields for the NGINX container.
435-
//
436-
// +optional
437-
Container ContainerSpec `json:"container"`
438-
439434
// Number of desired Pods.
440435
//
441436
// +optional
442437
Replicas *int32 `json:"replicas,omitempty"`
443438

444-
// Autoscaling defines the configuration for Horizontal Pod Autoscaling.
439+
// Pod defines Pod-specific fields.
445440
//
446441
// +optional
447-
Autoscaling HPASpec `json:"autoscaling"`
442+
Pod PodSpec `json:"pod"`
448443

449-
// Pod defines Pod-specific fields.
444+
// Container defines container fields for the NGINX container.
450445
//
451446
// +optional
452-
Pod PodSpec `json:"pod"`
447+
Container ContainerSpec `json:"container"`
453448

454449
// Patches are custom patches to apply to the NGINX Deployment.
455450
//
456451
// +optional
457452
Patches []Patch `json:"patches,omitempty"`
453+
454+
// Autoscaling defines the configuration for Horizontal Pod Autoscaling.
455+
//
456+
// +optional
457+
Autoscaling HPASpec `json:"autoscaling"`
458458
}
459459

460460
// DaemonSet is the configuration for the NGINX DaemonSet.
@@ -489,11 +489,6 @@ type HPASpec struct {
489489
// +optional
490490
Behavior *autoscalingv2.HorizontalPodAutoscalerBehavior `json:"behavior,omitempty"`
491491

492-
// AutoscalingTemplate configures the additional scaling option.
493-
//
494-
// +optional
495-
AutoscalingTemplate []autoscalingv2.MetricSpec `json:"autoscalingTemplate,omitempty"`
496-
497492
// Target cpu utilization percentage of HPA.
498493
//
499494
// +optional
@@ -514,11 +509,17 @@ type HPASpec struct {
514509
// +kubebuilder:validation:Minimum=1
515510
MinReplicas *int32 `json:"minReplicas,omitempty"`
516511

512+
// AutoscalingTemplate configures the additional scaling option.
513+
//
514+
// +optional
515+
AutoscalingTemplate []autoscalingv2.MetricSpec `json:"autoscalingTemplate,omitempty"`
516+
517517
// Maximum number of replicas.
518+
//
518519
// +kubebuilder:validation:Minimum=1
519520
MaxReplicas int32 `json:"maxReplicas"`
520521

521-
// Enable or disable Horizontal Pod Autoscaler
522+
// Enable or disable Horizontal Pod Autoscaler.
522523
Enabled bool `json:"enabled"`
523524
}
524525

‎apis/v1alpha2/zz_generated.deepcopy.go‎

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

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ rules:
88
- apiGroups:
99
- ""
1010
- apps
11-
{{- if or .Values.nginx.autoscaling.enabled .Values.nginxGateway.autoscaling.enabled }}
11+
{{- if .Values.nginx.autoscaling.enabled }}
1212
- autoscaling
1313
{{- end }}
1414
resources:
@@ -18,7 +18,7 @@ rules:
1818
- services
1919
- deployments
2020
- daemonsets
21-
{{- if or .Values.nginx.autoscaling.enabled .Values.nginxGateway.autoscaling.enabled }}
21+
{{- if .Values.nginx.autoscaling.enabled }}
2222
- horizontalpodautoscalers
2323
{{- end }}
2424
verbs:

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4179,7 +4179,7 @@ spec:
41794179
type: object
41804180
type: object
41814181
enabled:
4182-
description: Enable or disable Horizontal Pod Autoscaler
4182+
description: Enable or disable Horizontal Pod Autoscaler.
41834183
type: boolean
41844184
maxReplicas:
41854185
description: Maximum number of replicas.

‎deploy/crds.yaml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4764,7 +4764,7 @@ spec:
47644764
type: object
47654765
type: object
47664766
enabled:
4767-
description: Enable or disable Horizontal Pod Autoscaler
4767+
description: Enable or disable Horizontal Pod Autoscaler.
47684768
type: boolean
47694769
maxReplicas:
47704770
description: Maximum number of replicas.

‎internal/controller/provisioner/eventloop.go‎

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -79,17 +79,6 @@ func newEventLoop(
7979
),
8080
},
8181
},
82-
{
83-
objectType: &autoscalingv2.HorizontalPodAutoscaler{},
84-
options: []controller.Option{
85-
controller.WithK8sPredicate(
86-
k8spredicate.And(
87-
k8spredicate.GenerationChangedPredicate{},
88-
nginxResourceLabelPredicate,
89-
),
90-
),
91-
},
92-
},
9382
{
9483
objectType: &appsv1.DaemonSet{},
9584
options: []controller.Option{
@@ -148,6 +137,16 @@ func newEventLoop(
148137
),
149138
},
150139
},
140+
{
141+
objectType: &autoscalingv2.HorizontalPodAutoscaler{},
142+
options: []controller.Option{
143+
controller.WithK8sPredicate(
144+
k8spredicate.And(
145+
nginxResourceLabelPredicate,
146+
),
147+
),
148+
},
149+
},
151150
}
152151

153152
if isOpenshift {
@@ -201,6 +200,7 @@ func newEventLoop(
201200
// to provision or deprovision any nginx resources.
202201
&gatewayv1.GatewayList{},
203202
&appsv1.DeploymentList{},
203+
&appsv1.DaemonSetList{},
204204
&autoscalingv2.HorizontalPodAutoscalerList{},
205205
&corev1.ServiceList{},
206206
&corev1.ServiceAccountList{},

‎internal/controller/provisioner/objects_test.go‎

Lines changed: 78 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -383,6 +383,82 @@ func TestBuildNginxResourceObjects_NginxProxyConfig(t *testing.T) {
383383
g.Expect(hpa.Spec.MaxReplicas).To(Equal(int32(5)))
384384
}
385385

386+
func TestBuildNginxResourceObjects_DeploymentReplicasFromHPA(t *testing.T) {
387+
t.Parallel()
388+
g := NewWithT(t)
389+
390+
// Create a fake HPA with status.desiredReplicas set
391+
hpa := &autoscalingv2.HorizontalPodAutoscaler{
392+
ObjectMeta: metav1.ObjectMeta{
393+
Name: "gw-nginx",
394+
Namespace: "default",
395+
},
396+
Status: autoscalingv2.HorizontalPodAutoscalerStatus{
397+
DesiredReplicas: 7,
398+
},
399+
}
400+
401+
agentTLSSecret := &corev1.Secret{
402+
ObjectMeta: metav1.ObjectMeta{
403+
Name: agentTLSTestSecretName,
404+
Namespace: ngfNamespace,
405+
},
406+
Data: map[string][]byte{"tls.crt": []byte("tls")},
407+
}
408+
409+
fakeClient := fake.NewFakeClient(agentTLSSecret, hpa)
410+
411+
provisioner := &NginxProvisioner{
412+
cfg: Config{
413+
GatewayPodConfig: &config.GatewayPodConfig{
414+
Namespace: ngfNamespace,
415+
Version: "1.0.0",
416+
Image: "ngf-image",
417+
},
418+
AgentTLSSecretName: agentTLSTestSecretName,
419+
},
420+
baseLabelSelector: metav1.LabelSelector{
421+
MatchLabels: map[string]string{"app": "nginx"},
422+
},
423+
k8sClient: fakeClient,
424+
}
425+
426+
gateway := &gatewayv1.Gateway{
427+
ObjectMeta: metav1.ObjectMeta{
428+
Name: "gw",
429+
Namespace: "default",
430+
},
431+
Spec: gatewayv1.GatewaySpec{
432+
Listeners: []gatewayv1.Listener{{Port: 80}},
433+
},
434+
}
435+
436+
resourceName := "gw-nginx"
437+
nProxyCfg := &graph.EffectiveNginxProxy{
438+
Kubernetes: &ngfAPIv1alpha2.KubernetesSpec{
439+
Deployment: &ngfAPIv1alpha2.DeploymentSpec{
440+
Replicas: nil, // Should be overridden by HPA
441+
Autoscaling: ngfAPIv1alpha2.HPASpec{Enabled: true},
442+
},
443+
},
444+
}
445+
446+
objects, err := provisioner.buildNginxResourceObjects(resourceName, gateway, nProxyCfg)
447+
g.Expect(err).ToNot(HaveOccurred())
448+
449+
// Find the deployment object
450+
var deployment *appsv1.Deployment
451+
for _, obj := range objects {
452+
if d, ok := obj.(*appsv1.Deployment); ok {
453+
deployment = d
454+
break
455+
}
456+
}
457+
g.Expect(deployment).ToNot(BeNil())
458+
g.Expect(deployment.Spec.Replicas).ToNot(BeNil())
459+
g.Expect(*deployment.Spec.Replicas).To(Equal(int32(7)))
460+
}
461+
386462
func TestBuildNginxResourceObjects_Plus(t *testing.T) {
387463
t.Parallel()
388464
g := NewWithT(t)
@@ -1121,8 +1197,8 @@ func TestBuildNginxResourceObjectsForDeletion_DataplaneKeySecret(t *testing.T) {
11211197
objects := provisioner.buildNginxResourceObjectsForDeletion(deploymentNSName)
11221198

11231199
// Should include the dataplane key secret in the objects list
1124-
// Default: deployment, daemonset, service, serviceaccount, 2 configmaps, agentTLSSecret, dataplaneKeySecret
1125-
g.Expect(objects).To(HaveLen(8))
1200+
// Default: deployment, daemonset, service, hpa, serviceaccount, 2 configmaps, agentTLSSecret, dataplaneKeySecret
1201+
g.Expect(objects).To(HaveLen(9))
11261202

11271203
validateMeta := func(obj client.Object, name string) {
11281204
g.Expect(obj.GetName()).To(Equal(name))

0 commit comments

Comments
 (0)