add missing coverage for controller: (#2482)

- allow to define service type

Co-authored-by: taskbot <[email protected]>

Author: yrobla

cmd/thv-operator/api/v1alpha1/virtualmcpserver_types.go

@@ -42,6 +42,12 @@ type VirtualMCPServerSpec struct {
 	// +optional
 	Operational *OperationalConfig `json:"operational,omitempty"`
 
+	// ServiceType specifies the Kubernetes service type for the Virtual MCP server
+	// +kubebuilder:validation:Enum=ClusterIP;NodePort;LoadBalancer
+	// +kubebuilder:default=ClusterIP
+	// +optional
+	ServiceType string `json:"serviceType,omitempty"`
+
 	// PodTemplateSpec defines the pod template to use for the Virtual MCP server
 	// This allows for customizing the pod configuration beyond what is provided by the other fields.
 	// Note that to modify the specific container the Virtual MCP server runs in, you must specify

cmd/thv-operator/controllers/virtualmcpserver_controller.go

@@ -647,6 +647,15 @@ func (*VirtualMCPServerReconciler) serviceNeedsUpdate(
 		return true
 	}
 
+	// Check if service type has changed
+	expectedServiceType := corev1.ServiceTypeClusterIP
+	if vmcp.Spec.ServiceType != "" {
+		expectedServiceType = corev1.ServiceType(vmcp.Spec.ServiceType)
+	}
+	if service.Spec.Type != expectedServiceType {
+		return true
+	}
+
 	// Check if service metadata has changed
 	expectedLabels := labelsForVirtualMCPServer(vmcp.Name)
 	expectedAnnotations := make(map[string]string)

cmd/thv-operator/controllers/virtualmcpserver_controller_test.go

@@ -393,6 +393,156 @@ func TestVirtualMCPServerEnsureService(t *testing.T) {
 	assert.Equal(t, "http", service.Spec.Ports[0].Name)
 }
 
+// TestVirtualMCPServerServiceType tests Service creation with different service types
+func TestVirtualMCPServerServiceType(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		name                string
+		serviceType         string
+		expectedServiceType corev1.ServiceType
+	}{
+		{
+			name:                "default to ClusterIP",
+			serviceType:         "",
+			expectedServiceType: corev1.ServiceTypeClusterIP,
+		},
+		{
+			name:                "explicit ClusterIP",
+			serviceType:         "ClusterIP",
+			expectedServiceType: corev1.ServiceTypeClusterIP,
+		},
+		{
+			name:                "LoadBalancer",
+			serviceType:         "LoadBalancer",
+			expectedServiceType: corev1.ServiceTypeLoadBalancer,
+		},
+		{
+			name:                "NodePort",
+			serviceType:         "NodePort",
+			expectedServiceType: corev1.ServiceTypeNodePort,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+
+			vmcp := &mcpv1alpha1.VirtualMCPServer{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "test-vmcp",
+					Namespace: "default",
+				},
+				Spec: mcpv1alpha1.VirtualMCPServerSpec{
+					GroupRef: mcpv1alpha1.GroupRef{
+						Name: "test-group",
+					},
+					ServiceType: tt.serviceType,
+				},
+			}
+
+			scheme := runtime.NewScheme()
+			_ = mcpv1alpha1.AddToScheme(scheme)
+			_ = corev1.AddToScheme(scheme)
+
+			r := &VirtualMCPServerReconciler{
+				Scheme: scheme,
+			}
+
+			// Test serviceForVirtualMCPServer
+			service := r.serviceForVirtualMCPServer(context.Background(), vmcp)
+			require.NotNil(t, service)
+			assert.Equal(t, tt.expectedServiceType, service.Spec.Type)
+		})
+	}
+}
+
+// TestVirtualMCPServerServiceNeedsUpdate tests service update detection
+func TestVirtualMCPServerServiceNeedsUpdate(t *testing.T) {
+	t.Parallel()
+
+	baseVmcp := &mcpv1alpha1.VirtualMCPServer{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "test-vmcp",
+			Namespace: "default",
+		},
+		Spec: mcpv1alpha1.VirtualMCPServerSpec{
+			GroupRef: mcpv1alpha1.GroupRef{
+				Name: "test-group",
+			},
+			ServiceType: "ClusterIP",
+		},
+	}
+
+	baseService := &corev1.Service{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      vmcpServiceName(baseVmcp.Name),
+			Namespace: baseVmcp.Namespace,
+			Labels:    labelsForVirtualMCPServer(baseVmcp.Name),
+		},
+		Spec: corev1.ServiceSpec{
+			Type: corev1.ServiceTypeClusterIP,
+			Ports: []corev1.ServicePort{{
+				Port: vmcpDefaultPort,
+			}},
+		},
+	}
+
+	tests := []struct {
+		name        string
+		service     *corev1.Service
+		vmcp        *mcpv1alpha1.VirtualMCPServer
+		needsUpdate bool
+	}{
+		{
+			name:        "no update needed",
+			service:     baseService.DeepCopy(),
+			vmcp:        baseVmcp.DeepCopy(),
+			needsUpdate: false,
+		},
+		{
+			name:    "service type changed to LoadBalancer",
+			service: baseService.DeepCopy(),
+			vmcp: func() *mcpv1alpha1.VirtualMCPServer {
+				v := baseVmcp.DeepCopy()
+				v.Spec.ServiceType = "LoadBalancer"
+				return v
+			}(),
+			needsUpdate: true,
+		},
+		{
+			name:    "service type changed to NodePort",
+			service: baseService.DeepCopy(),
+			vmcp: func() *mcpv1alpha1.VirtualMCPServer {
+				v := baseVmcp.DeepCopy()
+				v.Spec.ServiceType = "NodePort"
+				return v
+			}(),
+			needsUpdate: true,
+		},
+		{
+			name: "port changed",
+			service: func() *corev1.Service {
+				s := baseService.DeepCopy()
+				s.Spec.Ports[0].Port = 9999
+				return s
+			}(),
+			vmcp:        baseVmcp.DeepCopy(),
+			needsUpdate: true,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+
+			r := &VirtualMCPServerReconciler{}
+			result := r.serviceNeedsUpdate(tt.service, tt.vmcp)
+			assert.Equal(t, tt.needsUpdate, result)
+		})
+	}
+}
+
 // TestVirtualMCPServerUpdateStatus tests status update logic
 func TestVirtualMCPServerUpdateStatus(t *testing.T) {
 	t.Parallel()

cmd/thv-operator/controllers/virtualmcpserver_deployment.go

@@ -322,9 +322,11 @@ func (r *VirtualMCPServerReconciler) serviceForVirtualMCPServer(
 	// Build service metadata
 	serviceLabels, serviceAnnotations := r.buildServiceMetadataForVmcp(ls, vmcp)
 
-	// Determine service type (ClusterIP by default, LoadBalancer if specified)
+	// Determine service type from spec (defaults to ClusterIP if not specified)
 	serviceType := corev1.ServiceTypeClusterIP
-	// TODO: Add configuration option for LoadBalancer service type
+	if vmcp.Spec.ServiceType != "" {
+		serviceType = corev1.ServiceType(vmcp.Spec.ServiceType)
+	}
 
 	svc := &corev1.Service{
 		ObjectMeta: metav1.ObjectMeta{

deploy/charts/operator-crds/Chart.yaml

@@ -2,5 +2,5 @@ apiVersion: v2
 name: toolhive-operator-crds
 description: A Helm chart for installing the ToolHive Operator CRDs into Kubernetes.
 type: application
-version: 0.0.51
+version: 0.0.52
 appVersion: "0.0.1"

deploy/charts/operator-crds/README.md

@@ -1,7 +1,7 @@
 
 # ToolHive Operator CRDs Helm Chart
 
-![Version: 0.0.51](https://img.shields.io/badge/Version-0.0.51-informational?style=flat-square)
+![Version: 0.0.52](https://img.shields.io/badge/Version-0.0.52-informational?style=flat-square)
 ![Type: application](https://img.shields.io/badge/Type-application-informational?style=flat-square)
 
 A Helm chart for installing the ToolHive Operator CRDs into Kubernetes.

deploy/charts/operator-crds/crds/toolhive.stacklok.dev_virtualmcpservers.yaml

@@ -725,6 +725,15 @@ spec:
                   This field accepts a PodTemplateSpec object as JSON/YAML.
                 type: object
                 x-kubernetes-preserve-unknown-fields: true
+              serviceType:
+                default: ClusterIP
+                description: ServiceType specifies the Kubernetes service type for
+                  the Virtual MCP server
+                enum:
+                - ClusterIP
+                - NodePort
+                - LoadBalancer
+                type: string
               tokenCache:
                 description: TokenCache configures token caching behavior
                 properties: