Update service API to include only necessary parameters

Signed-off-by: Shiva Krishna, Merla <smerla@nvidia.com>

Author: shivamerla

api/apps/v1alpha1/common_types.go

@@ -21,15 +21,14 @@ import (
 	autoscalingv2 "k8s.io/api/autoscaling/v2"
 	corev1 "k8s.io/api/core/v1"
 	networkingv1 "k8s.io/api/networking/v1"
-	"k8s.io/apimachinery/pkg/util/intstr"
 )
 
 const (
-	// DefaultAPIPort is the default API  port
+	// DefaultAPIPort is the default api  port
 	DefaultAPIPort = 8000
-	// DefaultNamedPortAPI is the default named API port
+	// DefaultNamedPortAPI is the default name for api port
 	DefaultNamedPortAPI = "api"
-	// DefaultNamedPortMetrics is the default named Metrics port
+	// DefaultNamedPortMetrics is the default name for metrics port
 	DefaultNamedPortMetrics = "metrics"
 )
 
@@ -44,29 +43,11 @@ type Service struct {
 	Type corev1.ServiceType `json:"type,omitempty"`
 	// override the default service name
 	Name string `json:"name,omitempty"`
-	// Deprecated: Use Ports instead.
-	// +kubebuilder:deprecatedversion
-	Port int32 `json:"port,omitempty"`
-	// Defines multiple ports for the service
-	Ports       []ServicePort     `json:"ports,omitempty"`
-	Annotations map[string]string `json:"annotations,omitempty"`
-}
-
-// ServicePort defines attributes to setup the service ports
-type ServicePort struct {
-	// The name of this port within the service.
-	Name string `json:"name,omitempty"`
-
-	// The IP protocol for this port. Supports "TCP", "UDP", and "SCTP".
-	// Default is TCP.
-	// +kubebuilder:validation:Enum=TCP;UDP;SCTP
-	// +kubebuilder:default="TCP"
-	Protocol corev1.Protocol `json:"protocol,omitempty"`
-
-	// The port that will be exposed by this service.
-	// +kubebuilder:validation:Minimum=1
-	// +kubebuilder:validation:Maximum=65535
+	// Port is the main api serving port
 	Port int32 `json:"port"`
+	// MetricsPort is the port to be used for the metrics collection (optional)
+	MetricsPort int32             `json:"metricsPort,omitempty"`
+	Annotations map[string]string `json:"annotations,omitempty"`
 }
 
 // Metrics defines attributes to setup metrics collection
@@ -141,59 +122,3 @@ type CertConfig struct {
 	// MountPath is the path where the certificates should be mounted in the container.
 	MountPath string `json:"mountPath"`
 }
-
-// selectNamedPort returns the first occurrence of a given named port, or an empty string if not found.
-func selectNamedPort(serviceSpec Service, portNames ...string) string {
-	for _, name := range portNames {
-		for _, port := range serviceSpec.Ports {
-			if port.Name == name {
-				return name
-			}
-		}
-	}
-	return ""
-}
-
-// getProbePort determines the appropriate port for probes based on the service spec.
-func getProbePort(serviceSpec Service) intstr.IntOrString {
-	switch len(serviceSpec.Ports) {
-	case 1:
-		port := serviceSpec.Ports[0]
-		if port.Name != "" {
-			return intstr.FromString(port.Name)
-		}
-		return intstr.FromInt(int(port.Port))
-	case 0:
-		// Default to "api" as the operator always adds a default named port with 8000
-		return intstr.FromString(DefaultNamedPortAPI)
-	default:
-		// Multiple ports: Prefer "api"
-		if portName := selectNamedPort(serviceSpec, DefaultNamedPortAPI); portName != "" {
-			return intstr.FromString(portName)
-		}
-		// Default when multiple ports exist
-		return intstr.FromString(DefaultNamedPortAPI)
-	}
-}
-
-// getMetricsPort determines the appropriate port for metrics based on the service spec.
-func getMetricsPort(serviceSpec Service) intstr.IntOrString {
-	switch len(serviceSpec.Ports) {
-	case 1:
-		port := serviceSpec.Ports[0]
-		if port.Name != "" {
-			return intstr.FromString(port.Name)
-		}
-		return intstr.FromInt(int(port.Port))
-	case 0:
-		// Default to "api" as the operator always adds a default named port with 8000
-		return intstr.FromString(DefaultNamedPortAPI)
-	default:
-		// Multiple ports: Prefer "metrics", fallback to "api"
-		if portName := selectNamedPort(serviceSpec, DefaultNamedPortMetrics, DefaultNamedPortAPI); portName != "" {
-			return intstr.FromString(portName)
-		}
-		// Default when multiple ports exist
-		return intstr.FromString(DefaultNamedPortMetrics)
-	}
-}

api/apps/v1alpha1/common_types_test.go

@@ -31,11 +31,14 @@ import (
 	networkingv1 "k8s.io/api/networking/v1"
 	rbacv1 "k8s.io/api/rbac/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/util/intstr"
 )
 
 const (
 	// CustomizerAPIPort is the default port that customizer serves on
 	CustomizerAPIPort = 8000
+	// DefaultNamedPortInternal is the default name for customizer internal port
+	DefaultNamedPortInternal = "internal"
 	// CustomizerInternalPort is the default port used for syncing training progress
 	CustomizerInternalPort = 9009
 	// NemoCustomizerConditionReady indicates that the NEMO CustomizerService is ready.
@@ -180,7 +183,7 @@ func (n *NemoCustomizer) GetStandardEnv() []corev1.EnvVar {
 		},
 		{
 			Name:  "CUSTOMIZATIONS_CALLBACK_URL",
-			Value: fmt.Sprintf("http://%s.%s.svc.cluster.local:%d", n.GetName(), n.GetNamespace(), n.GetInternalServicePort()),
+			Value: fmt.Sprintf("http://%s.%s.svc.cluster.local:%d", n.GetName(), n.GetNamespace(), CustomizerInternalPort),
 		},
 		{
 			Name:  "LOG_LEVEL",
@@ -446,7 +449,7 @@ func (n *NemoCustomizer) GetDefaultStartupProbe() *corev1.Probe {
 		ProbeHandler: corev1.ProbeHandler{
 			HTTPGet: &corev1.HTTPGetAction{
 				Path: "/v1/health/ready",
-				Port: getProbePort(n.Spec.Expose.Service),
+				Port: intstr.FromString(DefaultNamedPortAPI),
 			},
 		},
 	}
@@ -473,7 +476,7 @@ func (n *NemoCustomizer) GetDefaultLivenessProbe() *corev1.Probe {
 		ProbeHandler: corev1.ProbeHandler{
 			HTTPGet: &corev1.HTTPGetAction{
 				Path: "/v1/health/live",
-				Port: getProbePort(n.Spec.Expose.Service),
+				Port: intstr.FromString(DefaultNamedPortAPI),
 			},
 		},
 	}
@@ -499,7 +502,7 @@ func (n *NemoCustomizer) GetDefaultReadinessProbe() *corev1.Probe {
 		ProbeHandler: corev1.ProbeHandler{
 			HTTPGet: &corev1.HTTPGetAction{
 				Path: "/v1/health/ready",
-				Port: getProbePort(n.Spec.Expose.Service),
+				Port: intstr.FromString(DefaultNamedPortAPI),
 			},
 		},
 	}
@@ -560,23 +563,6 @@ func (n *NemoCustomizer) IsServiceMonitorEnabled() bool {
 	return n.Spec.Metrics.Enabled != nil && *n.Spec.Metrics.Enabled
 }
 
-// GetInternalServicePort returns the internal service port number for the NemoCustomizer deployment
-func (n *NemoCustomizer) GetInternalServicePort() int32 {
-	for _, port := range n.Spec.Expose.Service.Ports {
-		if port.Name == "internal" {
-			return port.Port
-		}
-	}
-
-	// Default to 9009 if no internal port is found
-	return CustomizerInternalPort
-}
-
-// GetServicePorts returns the service ports for the NemoCustomizer deployment
-func (n *NemoCustomizer) GetServicePorts() []corev1.ServicePort {
-	return n.Spec.Expose.Service.Ports
-}
-
 // GetServiceType returns the service type for the NemoCustomizer deployment
 func (n *NemoCustomizer) GetServiceType() string {
 	return string(n.Spec.Expose.Service.Type)
@@ -657,21 +643,27 @@ func (n *NemoCustomizer) GetDeploymentParams() *rendertypes.DeploymentParams {
 	params.RuntimeClassName = n.GetRuntimeClass()
 
 	// Setup container ports for customizer
-	// TODO: set these to use defined values in the service spec
-	// once that is allowed by the customizer through env variables
-	containerPorts := []corev1.ContainerPort{
+	params.Ports = []corev1.ContainerPort{
 		{
 			Name:          DefaultNamedPortAPI,
 			Protocol:      corev1.ProtocolTCP,
 			ContainerPort: CustomizerAPIPort,
 		},
 		{
-			Name:          "internal",
+			Name:          DefaultNamedPortInternal,
 			Protocol:      corev1.ProtocolTCP,
 			ContainerPort: CustomizerInternalPort,
 		},
 	}
-	params.Ports = containerPorts
+
+	if n.GetMetricsPort() != 0 {
+		metricsPort := corev1.ContainerPort{
+			Name:          DefaultNamedPortMetrics,
+			Protocol:      corev1.ProtocolTCP,
+			ContainerPort: n.GetMetricsPort(),
+		}
+		params.Ports = append(params.Ports, metricsPort)
+	}
 
 	return params
 }
@@ -738,23 +730,30 @@ func (n *NemoCustomizer) GetServiceParams() *rendertypes.ServiceParams {
 	// Set service type
 	params.Type = n.GetServiceType()
 
-	servicePorts := n.GetServicePorts()
-	if len(servicePorts) != 0 {
-		params.Ports = servicePorts
-	} else {
-		// Set default ports
-		params.Ports = []corev1.ServicePort{
-			{
-				Name:     DefaultNamedPortAPI,
-				Port:     CustomizerAPIPort,
-				Protocol: corev1.ProtocolTCP,
-			},
-			{
-				Name:     "internal",
-				Port:     CustomizerInternalPort,
-				Protocol: corev1.ProtocolTCP,
-			},
+	// Set service ports
+	params.Ports = []corev1.ServicePort{
+		{
+			Name:       DefaultNamedPortAPI,
+			Port:       n.GetServicePort(),
+			TargetPort: intstr.FromString((DefaultNamedPortAPI)),
+			Protocol:   corev1.ProtocolTCP,
+		},
+		{
+			Name:       DefaultNamedPortInternal,
+			Port:       CustomizerInternalPort,
+			TargetPort: intstr.FromString((DefaultNamedPortInternal)),
+			Protocol:   corev1.ProtocolTCP,
+		},
+	}
+
+	if n.GetMetricsPort() != 0 {
+		metricsPort := corev1.ServicePort{
+			Name:       DefaultNamedPortMetrics,
+			Port:       n.GetMetricsPort(),
+			TargetPort: intstr.FromString((DefaultNamedPortMetrics)),
+			Protocol:   corev1.ProtocolTCP,
 		}
+		params.Ports = append(params.Ports, metricsPort)
 	}
 
 	return params
@@ -892,15 +891,19 @@ func (n *NemoCustomizer) GetServiceMonitorParams() *rendertypes.ServiceMonitorPa
 	params.Annotations = n.GetServiceMonitorAnnotations()
 
 	// Determine the appropriate port for monitoring
-	metricsPort := getMetricsPort(n.Spec.Expose.Service)
+	namedMetricsPort := DefaultNamedPortAPI
+	if n.GetMetricsPort() != 0 {
+		// Use the named port for metrics
+		namedMetricsPort = DefaultNamedPortMetrics
+	}
 
 	// Set Service Monitor spec
 	smSpec := monitoringv1.ServiceMonitorSpec{
 		NamespaceSelector: monitoringv1.NamespaceSelector{MatchNames: []string{n.Namespace}},
 		Selector:          metav1.LabelSelector{MatchLabels: n.GetServiceLabels()},
 		Endpoints: []monitoringv1.Endpoint{
 			{
-				Port:          metricsPort.StrVal,
+				Port:          namedMetricsPort,
 				ScrapeTimeout: serviceMonitor.ScrapeTimeout,
 				Interval:      serviceMonitor.Interval,
 			},
@@ -910,6 +913,19 @@ func (n *NemoCustomizer) GetServiceMonitorParams() *rendertypes.ServiceMonitorPa
 	return params
 }
 
+// GetServicePort returns the service port for the NemoCustomizer deployment or default port
+func (n *NemoCustomizer) GetServicePort() int32 {
+	if n.Spec.Expose.Service.Port == 0 {
+		return DefaultAPIPort
+	}
+	return n.Spec.Expose.Service.Port
+}
+
+// GetMetricsPort returns the metrics port for the NemoCustomizer deployment
+func (n *NemoCustomizer) GetMetricsPort() int32 {
+	return n.Spec.Expose.Service.MetricsPort
+}
+
 // GetIngressAnnotations return standard and customized ingress annotations
 func (n *NemoCustomizer) GetIngressAnnotations() map[string]string {
 	NemoCustomizerAnnotations := n.GetNemoCustomizerAnnotations()