Clean up and fix templates

Author: sjberman

apis/v1alpha2/nginxproxy_types.go

@@ -477,8 +477,7 @@ type DaemonSetSpec struct {
 
 // HPASpec is the configuration for the Horizontal Pod Autoscaling.
 //
-// +kubebuilder:validation:XValidation:message="at least one metric must be specified when autoscaling is enabled",rule="!self.enable || (has(self.targetCPUUtilizationPercentage) || has(self.targetMemoryUtilizationPercentage) || (has(self.autoscalingTemplate) && size(self.autoscalingTemplate) > 0))"
-// +kubebuilder:validation:XValidation:message="minReplicas must be less than or equal to maxReplicas",rule="self.minReplicas <= self.maxReplicas"
+// +kubebuilder:validation:XValidation:message="minReplicas must be less than or equal to maxReplicas",rule="(!has(self.minReplicas)) || (self.minReplicas <= self.maxReplicas)"
 //
 //nolint:lll
 type HPASpec struct {
@@ -509,10 +508,10 @@ type HPASpec struct {
 	// +kubebuilder:validation:Minimum=1
 	MinReplicas *int32 `json:"minReplicas,omitempty"`
 
-	// AutoscalingTemplate configures the additional scaling option.
+	// Templates configures additional metrics options.
 	//
 	// +optional
-	AutoscalingTemplate []autoscalingv2.MetricSpec `json:"autoscalingTemplate,omitempty"`
+	Templates []autoscalingv2.MetricSpec `json:"templates,omitempty"`
 
 	// Maximum number of replicas.
 	//

apis/v1alpha2/zz_generated.deepcopy.go

@@ -38,7 +38,7 @@ spec:
         type: Utilization
         averageUtilization: {{ . }}
   {{- end }}
-  {{- with .Values.nginxGateway.autoscalingTemplate }}
+  {{- with .Values.nginxGateway.autoscaling.templates }}
   {{- toYaml . | nindent 2 }}
   {{- end }}
   {{- with .Values.nginxGateway.autoscaling.behavior }}

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

@@ -32,9 +32,9 @@ spec:
         behavior:
           {{- toYaml .Values.nginx.autoscaling.behavior | nindent 10 }}
         {{- end }}
-        {{- if .Values.nginx.autoscalingTemplate }}
-        autoscalingTemplate:
-          {{- toYaml .Values.nginx.autoscalingTemplate | nindent 8 }}
+        {{- if .Values.nginx.autoscaling.templates }}
+        templates:
+          {{- toYaml .Values.nginx.autoscaling.templates | nindent 8 }}
         {{- end }}
       {{- end }}
       {{- if .Values.nginx.pod }}

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

@@ -179,16 +179,15 @@ nginxGateway:
       #   - type: Pods
       #     value: 2
       #     periodSeconds: 60
-    # Custom or additional autoscaling metrics
-    # autoscalingTemplate: []
-      # ref: https://kubernetes.io/docs/tasks/run-application/horizontal-pod-autoscale/#support-for-custom-metrics
-      # - type: Pods
-      #   pods:
-      #     metric:
-      #       name: container_memory_usage_bytes
-      #     target:
-      #       type: AverageValue
-      #       averageValue: "400Mi"
+    # Custom or additional autoscaling metrics. https://kubernetes.io/docs/tasks/run-application/horizontal-pod-autoscale/#support-for-custom-metrics
+    # templates: []
+    # - type: Pods
+    #   pods:
+    #     metric:
+    #       name: container_memory_usage_bytes
+    #     target:
+    #       type: AverageValue
+    #       averageValue: "400Mi"
 
   metrics:
     # -- Enable exposing metrics in the Prometheus format.
@@ -251,16 +250,15 @@ nginx:
       #   - type: Pods
       #     value: 2
       #     periodSeconds: 60
-    # Custom or additional autoscaling metrics
-    # autoscalingTemplate: []
-      # ref: https://kubernetes.io/docs/tasks/run-application/horizontal-pod-autoscale/#support-for-custom-metrics
-      # - type: Pods
-      #   pods:
-      #     metric:
-      #       name: nginx_gateway_fabric_nginx_process_requests_total
-      #     target:
-      #       type: nginx_http_requests_total
-      #       averageValue: 400
+    # Custom or additional autoscaling metrics. https://kubernetes.io/docs/tasks/run-application/horizontal-pod-autoscale/#support-for-custom-metrics
+    # templates: []
+    # - type: Pods
+    #   pods:
+    #     metric:
+    #       name: nginx_gateway_fabric_nginx_process_requests_total
+    #     target:
+    #       type: AverageValue
+    #       averageValue: 400
   image:
     # -- The NGINX image to use.
     repository: ghcr.io/nginx/nginx-gateway-fabric/nginx

charts/nginx-gateway-fabric/values.yaml

@@ -1117,40 +1117,26 @@ func (p *NginxProvisioner) buildImage(nProxyCfg *graph.EffectiveNginxProxy) (str
 	return fmt.Sprintf("%s:%s", image, tag), pullPolicy
 }
 
-func getMetricTargetByType(target autoscalingv2.MetricTarget) autoscalingv2.MetricTarget {
-	switch target.Type {
-	case autoscalingv2.UtilizationMetricType:
-		return autoscalingv2.MetricTarget{
-			Type:               autoscalingv2.UtilizationMetricType,
-			AverageUtilization: target.AverageUtilization,
-		}
-	case autoscalingv2.AverageValueMetricType:
-		return autoscalingv2.MetricTarget{
-			Type:         autoscalingv2.AverageValueMetricType,
-			AverageValue: target.AverageValue,
-		}
-	case autoscalingv2.ValueMetricType:
-		return autoscalingv2.MetricTarget{
-			Type:  autoscalingv2.ValueMetricType,
-			Value: target.Value,
-		}
-	default:
-		return autoscalingv2.MetricTarget{}
-	}
-}
-
 func buildNginxDeploymentHPA(
 	objectMeta metav1.ObjectMeta,
 	autoScaling *ngfAPIv1alpha2.HPASpec,
 ) *autoscalingv2.HorizontalPodAutoscaler {
 	if !autoScaling.Enable {
 		return nil
 	}
-	var metrics []autoscalingv2.MetricSpec
 
 	cpuUtil := autoScaling.TargetCPUUtilizationPercentage
 	memUtil := autoScaling.TargetMemoryUtilizationPercentage
-	autoscalingTemplate := autoScaling.AutoscalingTemplate
+
+	metricsLen := len(autoScaling.Templates)
+	if cpuUtil != nil {
+		metricsLen++
+	}
+	if memUtil != nil {
+		metricsLen++
+	}
+
+	metrics := make([]autoscalingv2.MetricSpec, 0, metricsLen)
 
 	if cpuUtil != nil {
 		metrics = append(metrics, autoscalingv2.MetricSpec{
@@ -1178,17 +1164,7 @@ func buildNginxDeploymentHPA(
 		})
 	}
 
-	for _, additionalAutoscaling := range autoscalingTemplate {
-		metric := buildAdditionalMetric(additionalAutoscaling)
-		if metric != nil {
-			metrics = append(metrics, *metric)
-		}
-	}
-
-	if len(metrics) == 0 {
-		// No metrics configured, skip HPA creation
-		return nil
-	}
+	metrics = append(metrics, autoScaling.Templates...)
 
 	return &autoscalingv2.HorizontalPodAutoscaler{
 		ObjectMeta: objectMeta,
@@ -1206,61 +1182,6 @@ func buildNginxDeploymentHPA(
 	}
 }
 
-func buildAdditionalMetric(additionalAutoscaling autoscalingv2.MetricSpec) *autoscalingv2.MetricSpec {
-	switch additionalAutoscaling.Type {
-	case autoscalingv2.ResourceMetricSourceType:
-		return &autoscalingv2.MetricSpec{
-			Type: additionalAutoscaling.Type,
-			Resource: &autoscalingv2.ResourceMetricSource{
-				Name:   additionalAutoscaling.Resource.Name,
-				Target: getMetricTargetByType(additionalAutoscaling.Resource.Target),
-			},
-		}
-	case autoscalingv2.PodsMetricSourceType:
-		return &autoscalingv2.MetricSpec{
-			Type: additionalAutoscaling.Type,
-			Pods: &autoscalingv2.PodsMetricSource{
-				Metric: additionalAutoscaling.Pods.Metric,
-				Target: getMetricTargetByType(additionalAutoscaling.Pods.Target),
-			},
-		}
-	case autoscalingv2.ContainerResourceMetricSourceType:
-		return &autoscalingv2.MetricSpec{
-			Type: additionalAutoscaling.Type,
-			ContainerResource: &autoscalingv2.ContainerResourceMetricSource{
-				Name:      additionalAutoscaling.ContainerResource.Name,
-				Target:    getMetricTargetByType(additionalAutoscaling.ContainerResource.Target),
-				Container: additionalAutoscaling.ContainerResource.Container,
-			},
-		}
-	case autoscalingv2.ObjectMetricSourceType:
-		return &autoscalingv2.MetricSpec{
-			Type: additionalAutoscaling.Type,
-			Object: &autoscalingv2.ObjectMetricSource{
-				DescribedObject: additionalAutoscaling.Object.DescribedObject,
-				Target:          getMetricTargetByType(additionalAutoscaling.Object.Target),
-				Metric: autoscalingv2.MetricIdentifier{
-					Name:     additionalAutoscaling.Object.Metric.Name,
-					Selector: additionalAutoscaling.Object.Metric.Selector,
-				},
-			},
-		}
-	case autoscalingv2.ExternalMetricSourceType:
-		return &autoscalingv2.MetricSpec{
-			Type: additionalAutoscaling.Type,
-			External: &autoscalingv2.ExternalMetricSource{
-				Metric: autoscalingv2.MetricIdentifier{
-					Name:     additionalAutoscaling.External.Metric.Name,
-					Selector: additionalAutoscaling.External.Metric.Selector,
-				},
-				Target: getMetricTargetByType(additionalAutoscaling.External.Target),
-			},
-		}
-	default:
-		return nil
-	}
-}
-
 // TODO(sberman): see about how this can be made more elegant. Maybe create some sort of Object factory
 // that can better store/build all the objects we need, to reduce the amount of duplicate object lists that we
 // have everywhere.