Skip to content

Commit 3298ffe

Browse files
committed
Code Review
1 parent bacd371 commit 3298ffe

File tree

23 files changed

+10773
-209
lines changed

23 files changed

+10773
-209
lines changed

Makefile

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,7 @@ generate: ## Run go generate
121121

122122
.PHONY: generate-crds
123123
generate-crds: ## Generate CRDs and Go types using kubebuilder
124-
go run sigs.k8s.io/controller-tools/cmd/controller-gen@$(CONTROLLER_TOOLS_VERSION) crd:maxDescLen=0 object paths=./apis/... output:crd:artifacts:config=config/crd/bases
124+
go run sigs.k8s.io/controller-tools/cmd/controller-gen@$(CONTROLLER_TOOLS_VERSION) crd object paths=./apis/... output:crd:artifacts:config=config/crd/bases
125125
kubectl kustomize config/crd >deploy/crds.yaml
126126

127127
.PHONY: install-crds

apis/v1alpha2/nginxproxy_types.go

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -418,6 +418,12 @@ type DaemonSetSpec struct {
418418
Container ContainerSpec `json:"container"`
419419
}
420420

421+
// +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))"
422+
// +kubebuilder:validation:XValidation:message="minReplicas must be less than or equal to maxReplicas",rule="self.minReplicas <= self.maxReplicas"
423+
// +kubebuilder:validation:XValidation:message="CPU utilization must be between 1 and 100",rule="!has(self.targetCPUUtilizationPercentage) || (self.targetCPUUtilizationPercentage >= 1 && self.targetCPUUtilizationPercentage <= 100)"
424+
// +kubebuilder:validation:XValidation:message="memory utilization must be between 1 and 100",rule="!has(self.targetMemoryUtilizationPercentage) || (self.targetMemoryUtilizationPercentage >= 1 && self.targetMemoryUtilizationPercentage <= 100)"
425+
//
426+
//nolint:lll
421427
type HPASpec struct {
422428
// behavior configures the scaling behavior of the target
423429
// in both Up and Down directions (scaleUp and scaleDown fields respectively).
@@ -446,13 +452,14 @@ type HPASpec struct {
446452
// set by external tools to store and retrieve arbitrary metadata. They are not
447453
// queryable and should be preserved when modifying objects.
448454
// More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/annotations
455+
//
449456
// +optional
450457
HPAAnnotations map[string]string `json:"hpaAnnotations,omitempty"`
451458

452-
// Minimun of replicas.
459+
// Minimum number of replicas.
453460
MinReplicas int32 `json:"minReplicas"`
454461

455-
// Minimun of replicas.
462+
// Maximum number of replicas.
456463
MaxReplicas int32 `json:"maxReplicas"`
457464

458465
// Enable or disable Horizontal Pod Autoscaler

charts/nginx-gateway-fabric/README.md

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -264,18 +264,16 @@ The following table lists the configurable parameters of the NGINX Gateway Fabri
264264
| `certGenerator.ttlSecondsAfterFinished` | How long to wait after the cert generator job has finished before it is removed by the job controller. | int | `30` |
265265
| `clusterDomain` | The DNS cluster domain of your Kubernetes cluster. | string | `"cluster.local"` |
266266
| `gateways` | A list of Gateway objects. View https://gateway-api.sigs.k8s.io/reference/spec/#gateway for full Gateway reference. | list | `[]` |
267-
| `nginx` | The nginx section contains the configuration for all NGINX data plane deployments installed by the NGINX Gateway Fabric control plane. | object | `{"autoscaling":{"behavior":{},"enabled":true,"hpaAnnotations":{},"maxReplicas":11,"minReplicas":1,"targetCPUUtilizationPercentage":50,"targetMemoryUtilizationPercentage":50},"autoscalingTemplate":[],"config":{},"container":{"resources":{}},"debug":false,"image":{"pullPolicy":"Always","repository":"ghcr.io/nginx/nginx-gateway-fabric/nginx","tag":"edge"},"imagePullSecret":"","imagePullSecrets":[],"kind":"deployment","plus":false,"pod":{"tolerations":[]},"replicas":1,"service":{"externalTrafficPolicy":"Local","loadBalancerClass":"","loadBalancerIP":"","loadBalancerSourceRanges":[],"nodePorts":[],"type":"LoadBalancer"},"usage":{"caSecretName":"","clientSSLSecretName":"","endpoint":"","resolver":"","secretName":"nplus-license","skipVerify":false}}` |
267+
| `nginx` | The nginx section contains the configuration for all NGINX data plane deployments installed by the NGINX Gateway Fabric control plane. | object | `{"autoscaling":{"enabled":false},"autoscalingTemplate":[],"config":{},"container":{},"debug":false,"image":{"pullPolicy":"Always","repository":"ghcr.io/nginx/nginx-gateway-fabric/nginx","tag":"edge"},"imagePullSecret":"","imagePullSecrets":[],"kind":"deployment","plus":false,"pod":{},"replicas":1,"service":{"externalTrafficPolicy":"Local","loadBalancerClass":"","loadBalancerIP":"","loadBalancerSourceRanges":[],"nodePorts":[],"type":"LoadBalancer"},"usage":{"caSecretName":"","clientSSLSecretName":"","endpoint":"","resolver":"","secretName":"nplus-license","skipVerify":false}}` |
268268
| `nginx.config` | The configuration for the data plane that is contained in the NginxProxy resource. This is applied globally to all Gateways managed by this instance of NGINX Gateway Fabric. | object | `{}` |
269-
| `nginx.container` | The container configuration for the NGINX container. This is applied globally to all Gateways managed by this instance of NGINX Gateway Fabric. | object | `{"resources":{}}` |
270-
| `nginx.container.resources` | The resource requirements of the NGINX container. You should be set this value, If you want to use dataplane Autoscaling(HPA). | object | `{}` |
269+
| `nginx.container` | The container configuration for the NGINX container. This is applied globally to all Gateways managed by this instance of NGINX Gateway Fabric. | object | `{}` |
271270
| `nginx.debug` | Enable debugging for NGINX. Uses the nginx-debug binary. The NGINX error log level should be set to debug in the NginxProxy resource. | bool | `false` |
272271
| `nginx.image.repository` | The NGINX image to use. | string | `"ghcr.io/nginx/nginx-gateway-fabric/nginx"` |
273272
| `nginx.imagePullSecret` | The name of the secret containing docker registry credentials. Secret must exist in the same namespace as the helm release. The control plane will copy this secret into any namespace where NGINX is deployed. | string | `""` |
274273
| `nginx.imagePullSecrets` | A list of secret names containing docker registry credentials. Secrets must exist in the same namespace as the helm release. The control plane will copy these secrets into any namespace where NGINX is deployed. | list | `[]` |
275274
| `nginx.kind` | The kind of NGINX deployment. | string | `"deployment"` |
276275
| `nginx.plus` | Is NGINX Plus image being used. | bool | `false` |
277-
| `nginx.pod` | The pod configuration for the NGINX data plane pod. This is applied globally to all Gateways managed by this instance of NGINX Gateway Fabric. | object | `{"tolerations":[]}` |
278-
| `nginx.pod.tolerations` | Tolerations for the NGINX data plane pod. | list | `[]` |
276+
| `nginx.pod` | The pod configuration for the NGINX data plane pod. This is applied globally to all Gateways managed by this instance of NGINX Gateway Fabric. | object | `{}` |
279277
| `nginx.replicas` | The number of replicas of the NGINX Deployment. | int | `1` |
280278
| `nginx.service` | The service configuration for the NGINX data plane. This is applied globally to all Gateways managed by this instance of NGINX Gateway Fabric. | object | `{"externalTrafficPolicy":"Local","loadBalancerClass":"","loadBalancerIP":"","loadBalancerSourceRanges":[],"nodePorts":[],"type":"LoadBalancer"}` |
281279
| `nginx.service.externalTrafficPolicy` | The externalTrafficPolicy of the service. The value Local preserves the client source IP. | string | `"Local"` |
@@ -290,7 +288,7 @@ The following table lists the configurable parameters of the NGINX Gateway Fabri
290288
| `nginx.usage.resolver` | The nameserver used to resolve the NGINX Plus usage reporting endpoint. Used with NGINX Instance Manager. | string | `""` |
291289
| `nginx.usage.secretName` | The name of the Secret containing the JWT for NGINX Plus usage reporting. Must exist in the same namespace that the NGINX Gateway Fabric control plane is running in (default namespace: nginx-gateway). | string | `"nplus-license"` |
292290
| `nginx.usage.skipVerify` | Disable client verification of the NGINX Plus usage reporting server certificate. | bool | `false` |
293-
| `nginxGateway` | The nginxGateway section contains configuration for the NGINX Gateway Fabric control plane deployment. | object | `{"affinity":{},"autoscaling":{"annotations":{},"behavior":{},"enabled":false,"maxReplicas":11,"minReplicas":1,"targetCPUUtilizationPercentage":50,"targetMemoryUtilizationPercentage":50},"autoscalingTemplate":[],"config":{"logging":{"level":"info"}},"configAnnotations":{},"extraVolumeMounts":[],"extraVolumes":[],"gatewayClassAnnotations":{},"gatewayClassName":"nginx","gatewayControllerName":"gateway.nginx.org/nginx-gateway-controller","gwAPIExperimentalFeatures":{"enable":false},"image":{"pullPolicy":"Always","repository":"ghcr.io/nginx/nginx-gateway-fabric","tag":"edge"},"kind":"deployment","labels":{},"leaderElection":{"enable":true,"lockName":""},"lifecycle":{},"metrics":{"enable":true,"port":9113,"secure":false},"nodeSelector":{},"podAnnotations":{},"productTelemetry":{"enable":true},"readinessProbe":{"enable":true,"initialDelaySeconds":3,"port":8081},"replicas":1,"resources":{},"service":{"annotations":{},"labels":{}},"serviceAccount":{"annotations":{},"imagePullSecret":"","imagePullSecrets":[],"name":""},"snippetsFilters":{"enable":false},"terminationGracePeriodSeconds":30,"tolerations":[],"topologySpreadConstraints":[]}` |
291+
| `nginxGateway` | The nginxGateway section contains configuration for the NGINX Gateway Fabric control plane deployment. | object | `{"affinity":{},"autoscaling":{"enabled":false},"autoscalingTemplate":[],"config":{"logging":{"level":"info"}},"configAnnotations":{},"extraVolumeMounts":[],"extraVolumes":[],"gatewayClassAnnotations":{},"gatewayClassName":"nginx","gatewayControllerName":"gateway.nginx.org/nginx-gateway-controller","gwAPIExperimentalFeatures":{"enable":false},"image":{"pullPolicy":"Always","repository":"ghcr.io/nginx/nginx-gateway-fabric","tag":"edge"},"kind":"deployment","labels":{},"leaderElection":{"enable":true,"lockName":""},"lifecycle":{},"metrics":{"enable":true,"port":9113,"secure":false},"name":"","nodeSelector":{},"podAnnotations":{},"productTelemetry":{"enable":true},"readinessProbe":{"enable":true,"initialDelaySeconds":3,"port":8081},"replicas":1,"resources":{},"service":{"annotations":{},"labels":{}},"serviceAccount":{"annotations":{},"imagePullSecret":"","imagePullSecrets":[],"name":""},"snippetsFilters":{"enable":false},"terminationGracePeriodSeconds":30,"tolerations":[],"topologySpreadConstraints":[]}` |
294292
| `nginxGateway.affinity` | The affinity of the NGINX Gateway Fabric control plane pod. | object | `{}` |
295293
| `nginxGateway.config.logging.level` | Log level. | string | `"info"` |
296294
| `nginxGateway.configAnnotations` | Set of custom annotations for NginxGateway objects. | object | `{}` |
@@ -310,6 +308,7 @@ The following table lists the configurable parameters of the NGINX Gateway Fabri
310308
| `nginxGateway.metrics.enable` | Enable exposing metrics in the Prometheus format. | bool | `true` |
311309
| `nginxGateway.metrics.port` | Set the port where the Prometheus metrics are exposed. | int | `9113` |
312310
| `nginxGateway.metrics.secure` | Enable serving metrics via https. By default metrics are served via http. Please note that this endpoint will be secured with a self-signed certificate. | bool | `false` |
311+
| `nginxGateway.name` | The name of the NGINX Gateway Fabric deployment - if not present, then by default uses release name given during installation. | string | `""` |
313312
| `nginxGateway.nodeSelector` | The nodeSelector of the NGINX Gateway Fabric control plane pod. | object | `{}` |
314313
| `nginxGateway.podAnnotations` | Set of custom annotations for the NGINX Gateway Fabric pods. | object | `{}` |
315314
| `nginxGateway.productTelemetry.enable` | Enable the collection of product telemetry. | bool | `true` |

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ spec:
1515
{{- if .Values.nginx.replicas }}
1616
replicas: {{ .Values.nginx.replicas }}
1717
{{- end }}
18+
{{- if .Values.nginx.autoscaling.enabled }}
1819
autoscaling:
1920
enabled: {{ .Values.nginx.autoscaling.enabled }}
2021
{{- if .Values.nginx.autoscaling.hpaAnnotations }}
@@ -23,8 +24,12 @@ spec:
2324
{{- end }}
2425
minReplicas: {{ .Values.nginx.autoscaling.minReplicas }}
2526
maxReplicas: {{ .Values.nginx.autoscaling.maxReplicas }}
27+
{{- if .Values.nginx.autoscaling.targetCPUUtilizationPercentage }}
2628
targetCPUUtilizationPercentage: {{ .Values.nginx.autoscaling.targetCPUUtilizationPercentage }}
29+
{{- end }}
30+
{{- if .Values.nginx.autoscaling.targetMemoryUtilizationPercentage }}
2731
targetMemoryUtilizationPercentage: {{ .Values.nginx.autoscaling.targetMemoryUtilizationPercentage }}
32+
{{- end }}
2833
{{- if .Values.nginx.autoscaling.behavior }}
2934
behavior:
3035
{{- toYaml .Values.nginx.autoscaling.behavior | nindent 10 }}
@@ -33,6 +38,7 @@ spec:
3338
autoscalingTemplate:
3439
{{- toYaml .Values.nginx.autoscalingTemplate | nindent 8 }}
3540
{{- end }}
41+
{{- end }}
3642
{{- if .Values.nginx.pod }}
3743
pod:
3844
{{- toYaml .Values.nginx.pod | nindent 8 }}

charts/nginx-gateway-fabric/values.schema.json

Lines changed: 8 additions & 88 deletions
Original file line numberDiff line numberDiff line change
@@ -100,46 +100,12 @@
100100
"properties": {
101101
"autoscaling": {
102102
"properties": {
103-
"behavior": {
104-
"required": [],
105-
"title": "behavior",
106-
"type": "object"
107-
},
108103
"enabled": {
109-
"default": true,
104+
"default": false,
110105
"description": "Enable or disable Horizontal Pod Autoscaler",
111106
"required": [],
112107
"title": "enabled",
113108
"type": "boolean"
114-
},
115-
"hpaAnnotations": {
116-
"required": [],
117-
"title": "hpaAnnotations",
118-
"type": "object"
119-
},
120-
"maxReplicas": {
121-
"default": 11,
122-
"required": [],
123-
"title": "maxReplicas",
124-
"type": "integer"
125-
},
126-
"minReplicas": {
127-
"default": 1,
128-
"required": [],
129-
"title": "minReplicas",
130-
"type": "integer"
131-
},
132-
"targetCPUUtilizationPercentage": {
133-
"default": 50,
134-
"required": [],
135-
"title": "targetCPUUtilizationPercentage",
136-
"type": "integer"
137-
},
138-
"targetMemoryUtilizationPercentage": {
139-
"default": 50,
140-
"required": [],
141-
"title": "targetMemoryUtilizationPercentage",
142-
"type": "integer"
143109
}
144110
},
145111
"required": [],
@@ -369,14 +335,6 @@
369335
},
370336
"container": {
371337
"description": "The container configuration for the NGINX container. This is applied globally to all Gateways managed by this\ninstance of NGINX Gateway Fabric.",
372-
"properties": {
373-
"resources": {
374-
"description": "The resource requirements of the NGINX container. You should be set this value, If you want to use dataplane Autoscaling(HPA).",
375-
"required": [],
376-
"title": "resources",
377-
"type": "object"
378-
}
379-
},
380338
"required": [],
381339
"title": "container",
382340
"type": "object"
@@ -454,17 +412,6 @@
454412
},
455413
"pod": {
456414
"description": "The pod configuration for the NGINX data plane pod. This is applied globally to all Gateways managed by this\ninstance of NGINX Gateway Fabric.",
457-
"properties": {
458-
"tolerations": {
459-
"description": "Tolerations for the NGINX data plane pod.",
460-
"items": {
461-
"required": []
462-
},
463-
"required": [],
464-
"title": "tolerations",
465-
"type": "array"
466-
}
467-
},
468415
"required": [],
469416
"title": "pod",
470417
"type": "object"
@@ -618,46 +565,12 @@
618565
},
619566
"autoscaling": {
620567
"properties": {
621-
"annotations": {
622-
"required": [],
623-
"title": "annotations",
624-
"type": "object"
625-
},
626-
"behavior": {
627-
"required": [],
628-
"title": "behavior",
629-
"type": "object"
630-
},
631568
"enabled": {
632569
"default": false,
633570
"description": "Enable or disable Horizontal Pod Autoscaler",
634571
"required": [],
635572
"title": "enabled",
636573
"type": "boolean"
637-
},
638-
"maxReplicas": {
639-
"default": 11,
640-
"required": [],
641-
"title": "maxReplicas",
642-
"type": "integer"
643-
},
644-
"minReplicas": {
645-
"default": 1,
646-
"required": [],
647-
"title": "minReplicas",
648-
"type": "integer"
649-
},
650-
"targetCPUUtilizationPercentage": {
651-
"default": 50,
652-
"required": [],
653-
"title": "targetCPUUtilizationPercentage",
654-
"type": "integer"
655-
},
656-
"targetMemoryUtilizationPercentage": {
657-
"default": 50,
658-
"required": [],
659-
"title": "targetMemoryUtilizationPercentage",
660-
"type": "integer"
661574
}
662575
},
663576
"required": [],
@@ -858,6 +771,13 @@
858771
"title": "metrics",
859772
"type": "object"
860773
},
774+
"name": {
775+
"default": "",
776+
"description": "The name of the NGINX Gateway Fabric deployment - if not present, then by default uses release name given during installation.",
777+
"required": [],
778+
"title": "name",
779+
"type": "string"
780+
},
861781
"nodeSelector": {
862782
"description": "The nodeSelector of the NGINX Gateway Fabric control plane pod.",
863783
"required": [],

charts/nginx-gateway-fabric/values.yaml

Lines changed: 22 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,9 @@ nginxGateway:
1313
# -- The kind of the NGINX Gateway Fabric installation - currently, only deployment is supported.
1414
kind: deployment
1515

16+
# -- The name of the NGINX Gateway Fabric deployment - if not present, then by default uses release name given during installation.
17+
name: ""
18+
1619
# @schema
1720
# required: true
1821
# type: string
@@ -159,12 +162,12 @@ nginxGateway:
159162
autoscaling:
160163
# Enable or disable Horizontal Pod Autoscaler
161164
enabled: false
162-
annotations: {}
163-
minReplicas: 1
164-
maxReplicas: 11
165-
targetCPUUtilizationPercentage: 50
166-
targetMemoryUtilizationPercentage: 50
167-
behavior: {}
165+
# annotations: {}
166+
# minReplicas: 1
167+
# maxReplicas: 11
168+
# targetCPUUtilizationPercentage: 50
169+
# targetMemoryUtilizationPercentage: 50
170+
# behavior: {}
168171
# scaleDown:
169172
# stabilizationWindowSeconds: 300
170173
# policies:
@@ -230,13 +233,13 @@ nginx:
230233

231234
autoscaling:
232235
# Enable or disable Horizontal Pod Autoscaler
233-
enabled: true
234-
hpaAnnotations: {}
235-
minReplicas: 1
236-
maxReplicas: 11
237-
targetCPUUtilizationPercentage: 50
238-
targetMemoryUtilizationPercentage: 50
239-
behavior: {}
236+
enabled: false
237+
# hpaAnnotations: {}
238+
# minReplicas: 1
239+
# maxReplicas: 11
240+
# targetCPUUtilizationPercentage: 50
241+
# targetMemoryUtilizationPercentage: 50
242+
# behavior: {}
240243
# scaleDown:
241244
# stabilizationWindowSeconds: 300
242245
# policies:
@@ -440,12 +443,12 @@ nginx:
440443

441444
# -- The pod configuration for the NGINX data plane pod. This is applied globally to all Gateways managed by this
442445
# instance of NGINX Gateway Fabric.
443-
pod:
446+
pod: {}
444447
# -- The termination grace period of the NGINX data plane pod.
445448
# terminationGracePeriodSeconds: 30
446449

447450
# -- Tolerations for the NGINX data plane pod.
448-
tolerations: []
451+
# tolerations: []
449452

450453
# -- The nodeSelector of the NGINX data plane pod.
451454
# nodeSelector: {}
@@ -462,9 +465,10 @@ nginx:
462465

463466
# -- The container configuration for the NGINX container. This is applied globally to all Gateways managed by this
464467
# instance of NGINX Gateway Fabric.
465-
container:
466-
# -- The resource requirements of the NGINX container. You should be set this value, If you want to use dataplane Autoscaling(HPA).
467-
resources: {}
468+
container: {}
469+
# -- The resource requirements of the NGINX container. You should set this value if you want to use dataplane Autoscaling(HPA).
470+
# resources: {}
471+
468472
# -- The lifecycle of the NGINX container.
469473
# lifecycle: {}
470474

0 commit comments

Comments
 (0)