Skip to content

Commit 2820b32

Browse files
committed
Add default resource limits and separate proxy runner resource configuration
Implements default resource limits for MCP server and VirtualMCP server containers to prevent resource monopolization, with intelligent merging of user-provided values and separate resource configuration for proxy runner containers. Per issue #2873, vmcp and MCP server containers lacked default resource limits, creating security and operational risks where misbehaving containers could exhaust node resources and affect other workloads. Additionally, MCPServer deployments have two containers (MCP server + proxy runner) but only had one resource field (`spec.resources`), creating ambiguity about which container the resources applied to. - MCP server container: 500m/100m CPU, 512Mi/128Mi memory - Proxy runner container: 200m/50m CPU, 256Mi/64Mi memory (lighter weight) - VirtualMCP server container: 500m/100m CPU, 512Mi/128Mi memory - Defaults are intelligently merged with user-provided values - Add `spec.proxyRunnerResources` field for proxy runner container configuration - Existing `spec.resources` applies to MCP server container - Clear separation eliminates ambiguity between the two containers MCP server resources are composed from three sources: - Defaults → `spec.resources` → `spec.podTemplateSpec` (highest precedence) Proxy runner resources are composed from: - Defaults → `spec.proxyRunnerResources` - Add `resourceRequirementsForMCPServer()`: Merges defaults with spec.resources - Add `resourceRequirementsForProxyRunner()`: Merges defaults with spec.proxyRunnerResources - Update `deploymentNeedsUpdate()`: Checks both MCP and proxy resources separately - Add `PodTemplateSpecBuilder.WithResources()`: Merges resources intelligently - Add shared utility functions in `pkg/controllerutil/resources.go` - Add regression tests for `deploymentNeedsUpdate()` resource changes (4 tests) - Add tests for resource merging in PodTemplateSpecBuilder (2 tests) - Add tests for default resources and merging (3 tests) - Add VirtualMCPServer deployment tests (10 tests) - Total: 19 new tests ensuring no reconciliation loops - Add `proxyRunnerResources` field to MCPServer CRD - Regenerate deepcopy methods and CRD manifests - Bump Helm chart version: 0.0.75 → 0.0.76 - Update example YAML with clear comments - Document default values and composition precedence **MCPServer (2 containers):** - `spec.resources` → MCP server container (via pod template patch) - `spec.proxyRunnerResources` → Proxy runner container (in deployment) **VirtualMCPServer (1 container):** - `spec.resources` → VirtualMCP server container - Prevents resource exhaustion by providing sensible defaults - Clear separation: separate fields for separate containers in MCPServer - User can customize each container independently - Intelligent merging allows partial overrides - Prevents reconciliation loops through proper update detection Fixes #2873 Signed-off-by: 4t8dd <wanger.xyz@gmail.com>
1 parent 23bf796 commit 2820b32

16 files changed

+1613
-63
lines changed

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,12 @@ type MCPServerSpec struct {
107107
// +optional
108108
Resources ResourceRequirements `json:"resources,omitempty"`
109109

110+
// ProxyRunnerResources defines the resource requirements for the ToolHive proxy runner container.
111+
// The proxy runner is the infrastructure container that manages the MCP server container.
112+
// If not specified, defaults to 50m/200m CPU and 64Mi/256Mi memory (lighter than MCP server defaults).
113+
// +optional
114+
ProxyRunnerResources ResourceRequirements `json:"proxyRunnerResources,omitempty"`
115+
110116
// Secrets are references to secrets to mount in the MCP server container
111117
// +optional
112118
Secrets []SecretRef `json:"secrets,omitempty"`

cmd/thv-operator/api/v1alpha1/zz_generated.deepcopy.go

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

cmd/thv-operator/controllers/mcpserver_controller.go

Lines changed: 31 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@ import (
1818
rbacv1 "k8s.io/api/rbac/v1"
1919
"k8s.io/apimachinery/pkg/api/errors"
2020
"k8s.io/apimachinery/pkg/api/meta"
21-
"k8s.io/apimachinery/pkg/api/resource"
2221
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2322
"k8s.io/apimachinery/pkg/runtime"
2423
"k8s.io/apimachinery/pkg/types"
@@ -928,9 +927,13 @@ func (r *MCPServerReconciler) deploymentForMCPServer(
928927
defaultSA := mcpServerServiceAccountName(m.Name)
929928
serviceAccount = &defaultSA
930929
}
930+
// Convert MCP server resources to Kubernetes format
931+
mcpResources := resourceRequirementsForMCPServer(m)
932+
931933
finalPodTemplateSpec := builder.
932934
WithServiceAccount(serviceAccount).
933935
WithSecrets(m.Spec.Secrets).
936+
WithResources(mcpResources).
934937
Build()
935938
// Add pod template patch if we have one
936939
if finalPodTemplateSpec != nil {
@@ -1062,26 +1065,9 @@ func (r *MCPServerReconciler) deploymentForMCPServer(
10621065
volumes = append(volumes, *authzVolume)
10631066
}
10641067

1065-
// Prepare container resources
1066-
resources := corev1.ResourceRequirements{}
1067-
if m.Spec.Resources.Limits.CPU != "" || m.Spec.Resources.Limits.Memory != "" {
1068-
resources.Limits = corev1.ResourceList{}
1069-
if m.Spec.Resources.Limits.CPU != "" {
1070-
resources.Limits[corev1.ResourceCPU] = resource.MustParse(m.Spec.Resources.Limits.CPU)
1071-
}
1072-
if m.Spec.Resources.Limits.Memory != "" {
1073-
resources.Limits[corev1.ResourceMemory] = resource.MustParse(m.Spec.Resources.Limits.Memory)
1074-
}
1075-
}
1076-
if m.Spec.Resources.Requests.CPU != "" || m.Spec.Resources.Requests.Memory != "" {
1077-
resources.Requests = corev1.ResourceList{}
1078-
if m.Spec.Resources.Requests.CPU != "" {
1079-
resources.Requests[corev1.ResourceCPU] = resource.MustParse(m.Spec.Resources.Requests.CPU)
1080-
}
1081-
if m.Spec.Resources.Requests.Memory != "" {
1082-
resources.Requests[corev1.ResourceMemory] = resource.MustParse(m.Spec.Resources.Requests.Memory)
1083-
}
1084-
}
1068+
// Prepare container resources for the proxy runner container
1069+
// Note: This is separate from MCP server container resources (which are in the pod template patch)
1070+
resources := resourceRequirementsForProxyRunner(m)
10851071

10861072
// Prepare deployment metadata with overrides
10871073
deploymentLabels := ls
@@ -1528,9 +1514,13 @@ func (r *MCPServerReconciler) deploymentNeedsUpdate(
15281514
return true
15291515
}
15301516

1517+
// Convert MCP server resources to Kubernetes format (same as in deploymentForMCPServer)
1518+
mcpResources := resourceRequirementsForMCPServer(mcpServer)
1519+
15311520
expectedPodTemplateSpec := builder.
15321521
WithServiceAccount(serviceAccount).
15331522
WithSecrets(mcpServer.Spec.Secrets).
1523+
WithResources(mcpResources).
15341524
Build()
15351525

15361526
// Find the current pod template patch in the container args
@@ -1560,8 +1550,8 @@ func (r *MCPServerReconciler) deploymentNeedsUpdate(
15601550
return true
15611551
}
15621552

1563-
// Check if the resource requirements have changed
1564-
if !reflect.DeepEqual(container.Resources, resourceRequirementsForMCPServer(mcpServer)) {
1553+
// Check if the proxy runner container resource requirements have changed
1554+
if !reflect.DeepEqual(container.Resources, resourceRequirementsForProxyRunner(mcpServer)) {
15651555
return true
15661556
}
15671557
}
@@ -1656,28 +1646,25 @@ func serviceNeedsUpdate(service *corev1.Service, mcpServer *mcpv1alpha1.MCPServe
16561646
return false
16571647
}
16581648

1659-
// resourceRequirementsForMCPServer returns the resource requirements for the MCPServer
1649+
// resourceRequirementsForMCPServer returns the resource requirements for the MCP server container
1650+
// with intelligent merging of defaults and user-provided values
16601651
func resourceRequirementsForMCPServer(m *mcpv1alpha1.MCPServer) corev1.ResourceRequirements {
1661-
resources := corev1.ResourceRequirements{}
1662-
if m.Spec.Resources.Limits.CPU != "" || m.Spec.Resources.Limits.Memory != "" {
1663-
resources.Limits = corev1.ResourceList{}
1664-
if m.Spec.Resources.Limits.CPU != "" {
1665-
resources.Limits[corev1.ResourceCPU] = resource.MustParse(m.Spec.Resources.Limits.CPU)
1666-
}
1667-
if m.Spec.Resources.Limits.Memory != "" {
1668-
resources.Limits[corev1.ResourceMemory] = resource.MustParse(m.Spec.Resources.Limits.Memory)
1669-
}
1670-
}
1671-
if m.Spec.Resources.Requests.CPU != "" || m.Spec.Resources.Requests.Memory != "" {
1672-
resources.Requests = corev1.ResourceList{}
1673-
if m.Spec.Resources.Requests.CPU != "" {
1674-
resources.Requests[corev1.ResourceCPU] = resource.MustParse(m.Spec.Resources.Requests.CPU)
1675-
}
1676-
if m.Spec.Resources.Requests.Memory != "" {
1677-
resources.Requests[corev1.ResourceMemory] = resource.MustParse(m.Spec.Resources.Requests.Memory)
1678-
}
1679-
}
1680-
return resources
1652+
defaultResources := ctrlutil.BuildDefaultResourceRequirements()
1653+
userResources := ctrlutil.BuildResourceRequirements(m.Spec.Resources)
1654+
return ctrlutil.MergeResourceRequirements(defaultResources, userResources)
1655+
}
1656+
1657+
// resourceRequirementsForProxyRunner returns the resource requirements for the proxy runner container
1658+
// with intelligent merging of defaults and user-provided values
1659+
func resourceRequirementsForProxyRunner(m *mcpv1alpha1.MCPServer) corev1.ResourceRequirements {
1660+
// Get default proxy runner resources (lighter than MCP server defaults)
1661+
defaultProxyResources := ctrlutil.BuildDefaultProxyRunnerResourceRequirements()
1662+
1663+
// Get user-defined proxy resources from the dedicated field
1664+
userProxyResources := ctrlutil.BuildResourceRequirements(m.Spec.ProxyRunnerResources)
1665+
1666+
// Merge defaults with user-defined values
1667+
return ctrlutil.MergeResourceRequirements(defaultProxyResources, userProxyResources)
16811668
}
16821669

16831670
// mcpServerServiceAccountName returns the service account name for the mcp server

0 commit comments

Comments
 (0)