Skip to content

Commit f2b69d0

Browse files
committed
Reduce MCPServer CRD size by using runtime.RawExtension for PodTemplateSpec
The MCPServer CRD was too large (~9500 lines) to apply without server-side apply due to the embedded PodTemplateSpec taking up ~8500 lines. This was causing issues as reported in GitHub issue #2013. Changed the PodTemplateSpec field from a strongly-typed corev1.PodTemplateSpec to runtime.RawExtension, which stores the raw JSON without schema validation at the CRD level. This reduces the CRD size from ~9500 lines to 651 lines (93% reduction). The solution maintains full backwards compatibility - users can still use the same YAML structure. Validation now happens at runtime in the operator, with proper error handling via Kubernetes events and status conditions to notify users when invalid PodTemplateSpec data is provided. Key changes: - Modified MCPServer type to use runtime.RawExtension for PodTemplateSpec - Updated PodTemplateSpecBuilder to unmarshal and validate at runtime - Added event recording and status conditions for validation errors - Added comprehensive tests for invalid PodTemplateSpec scenarios - Fixed race conditions in parallel tests Fixes #2013
1 parent 84041cc commit f2b69d0

13 files changed

+600
-8524
lines changed

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

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
package v1alpha1
22

33
import (
4-
corev1 "k8s.io/api/core/v1"
54
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
5+
"k8s.io/apimachinery/pkg/runtime"
66
)
77

88
// Condition types for MCPServer
@@ -85,8 +85,11 @@ type MCPServerSpec struct {
8585
// This allows for customizing the pod configuration beyond what is provided by the other fields.
8686
// Note that to modify the specific container the MCP server runs in, you must specify
8787
// the `mcp` container name in the PodTemplateSpec.
88+
// This field accepts a PodTemplateSpec object as JSON/YAML.
8889
// +optional
89-
PodTemplateSpec *corev1.PodTemplateSpec `json:"podTemplateSpec,omitempty"`
90+
// +kubebuilder:pruning:PreserveUnknownFields
91+
// +kubebuilder:validation:Type=object
92+
PodTemplateSpec *runtime.RawExtension `json:"podTemplateSpec,omitempty"`
9093

9194
// ResourceOverrides allows overriding annotations and labels for resources created by the operator
9295
// +optional

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

Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
package controllers
2+
3+
import (
4+
"encoding/json"
5+
"testing"
6+
7+
"github.com/stretchr/testify/require"
8+
corev1 "k8s.io/api/core/v1"
9+
"k8s.io/apimachinery/pkg/runtime"
10+
)
11+
12+
// podTemplateSpecToRawExtension is a test helper to convert PodTemplateSpec to RawExtension
13+
func podTemplateSpecToRawExtension(t *testing.T, pts *corev1.PodTemplateSpec) *runtime.RawExtension {
14+
t.Helper()
15+
if pts == nil {
16+
return nil
17+
}
18+
raw, err := json.Marshal(pts)
19+
require.NoError(t, err, "Failed to marshal PodTemplateSpec")
20+
return &runtime.RawExtension{Raw: raw}
21+
}

cmd/thv-operator/controllers/mcpserver_controller.go

Lines changed: 126 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import (
2626
"k8s.io/apimachinery/pkg/types"
2727
"k8s.io/apimachinery/pkg/util/intstr"
2828
"k8s.io/client-go/rest"
29+
"k8s.io/client-go/tools/record"
2930
ctrl "sigs.k8s.io/controller-runtime"
3031
"sigs.k8s.io/controller-runtime/pkg/client"
3132
"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
@@ -40,6 +41,7 @@ import (
4041
type MCPServerReconciler struct {
4142
client.Client
4243
Scheme *runtime.Scheme
44+
Recorder record.EventRecorder
4345
platformDetector kubernetes.PlatformDetector
4446
detectedPlatform kubernetes.Platform
4547
platformOnce sync.Once
@@ -295,6 +297,14 @@ func (r *MCPServerReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
295297
deployment := &appsv1.Deployment{}
296298
err = r.Get(ctx, types.NamespacedName{Name: mcpServer.Name, Namespace: mcpServer.Namespace}, deployment)
297299
if err != nil && errors.IsNotFound(err) {
300+
// Validate PodTemplateSpec and update status
301+
if !r.validateAndUpdatePodTemplateStatus(ctx, mcpServer) {
302+
// Invalid PodTemplateSpec - return without error to avoid infinite retries
303+
// The user must fix the spec and the next reconciliation will retry
304+
ctxLogger.Info("Skipping deployment creation due to invalid PodTemplateSpec")
305+
return ctrl.Result{}, nil
306+
}
307+
298308
// Define a new deployment
299309
dep := r.deploymentForMCPServer(ctx, mcpServer)
300310
if dep == nil {
@@ -364,6 +374,14 @@ func (r *MCPServerReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
364374

365375
// Check if the deployment spec changed
366376
if r.deploymentNeedsUpdate(ctx, deployment, mcpServer) {
377+
// Validate PodTemplateSpec and update status
378+
if !r.validateAndUpdatePodTemplateStatus(ctx, mcpServer) {
379+
// Invalid PodTemplateSpec - return without error to avoid infinite retries
380+
// The user must fix the spec and the next reconciliation will retry
381+
ctxLogger.Info("Skipping deployment update due to invalid PodTemplateSpec")
382+
return ctrl.Result{}, nil
383+
}
384+
367385
// Update the deployment
368386
newDeployment := r.deploymentForMCPServer(ctx, mcpServer)
369387
deployment.Spec = newDeployment.Spec
@@ -406,6 +424,59 @@ func setImageValidationCondition(mcpServer *mcpv1alpha1.MCPServer, status metav1
406424
})
407425
}
408426

427+
// validateAndUpdatePodTemplateStatus validates the PodTemplateSpec and updates the MCPServer status
428+
// with appropriate conditions and events
429+
func (r *MCPServerReconciler) validateAndUpdatePodTemplateStatus(ctx context.Context, mcpServer *mcpv1alpha1.MCPServer) bool {
430+
ctxLogger := log.FromContext(ctx)
431+
432+
// Only validate if PodTemplateSpec is provided
433+
if mcpServer.Spec.PodTemplateSpec == nil || mcpServer.Spec.PodTemplateSpec.Raw == nil {
434+
// No PodTemplateSpec provided, validation passes
435+
return true
436+
}
437+
438+
_, err := NewMCPServerPodTemplateSpecBuilder(mcpServer.Spec.PodTemplateSpec)
439+
if err != nil {
440+
// Record event for invalid PodTemplateSpec
441+
r.Recorder.Eventf(mcpServer, corev1.EventTypeWarning, "InvalidPodTemplateSpec",
442+
"Failed to parse PodTemplateSpec: %v. Deployment blocked until PodTemplateSpec is fixed.", err)
443+
444+
// Set condition for invalid PodTemplateSpec
445+
meta.SetStatusCondition(&mcpServer.Status.Conditions, metav1.Condition{
446+
Type: "PodTemplateValid",
447+
Status: metav1.ConditionFalse,
448+
ObservedGeneration: mcpServer.Generation,
449+
Reason: "InvalidPodTemplateSpec",
450+
Message: fmt.Sprintf("Failed to parse PodTemplateSpec: %v. Deployment blocked until fixed.", err),
451+
})
452+
453+
// Update status with the condition
454+
if statusErr := r.Status().Update(ctx, mcpServer); statusErr != nil {
455+
ctxLogger.Error(statusErr, "Failed to update MCPServer status with PodTemplateSpec validation")
456+
}
457+
458+
ctxLogger.Error(err, "PodTemplateSpec validation failed")
459+
return false
460+
}
461+
462+
// Set condition for valid PodTemplateSpec
463+
meta.SetStatusCondition(&mcpServer.Status.Conditions, metav1.Condition{
464+
Type: "PodTemplateValid",
465+
Status: metav1.ConditionTrue,
466+
ObservedGeneration: mcpServer.Generation,
467+
Reason: "ValidPodTemplateSpec",
468+
Message: "PodTemplateSpec is valid",
469+
})
470+
471+
// Update status with the condition
472+
if statusErr := r.Status().Update(ctx, mcpServer); statusErr != nil {
473+
ctxLogger.Error(statusErr, "Failed to update MCPServer status with PodTemplateSpec validation")
474+
}
475+
476+
ctxLogger.V(1).Info("PodTemplateSpec validation completed successfully")
477+
return true
478+
}
479+
409480
// handleRestartAnnotation checks if the restart annotation has been updated and triggers a restart if needed
410481
// Returns true if a restart was triggered and the reconciliation should be requeued
411482
func (r *MCPServerReconciler) handleRestartAnnotation(ctx context.Context, mcpServer *mcpv1alpha1.MCPServer) (bool, error) {
@@ -756,17 +827,22 @@ func (r *MCPServerReconciler) deploymentForMCPServer(ctx context.Context, m *mcp
756827
args = append(args, fmt.Sprintf("--from-configmap=%s", configMapRef))
757828

758829
// Also add pod template patch for secrets (same as regular flags approach)
759-
finalPodTemplateSpec := NewMCPServerPodTemplateSpecBuilder(m.Spec.PodTemplateSpec).
760-
WithSecrets(m.Spec.Secrets).
761-
Build()
762-
// Add pod template patch if we have one
763-
if finalPodTemplateSpec != nil {
764-
podTemplatePatch, err := json.Marshal(finalPodTemplateSpec)
765-
if err != nil {
766-
ctxLogger := log.FromContext(ctx)
767-
ctxLogger.Error(err, "Failed to marshal pod template spec")
768-
} else {
769-
args = append(args, fmt.Sprintf("--k8s-pod-patch=%s", string(podTemplatePatch)))
830+
builder, err := NewMCPServerPodTemplateSpecBuilder(m.Spec.PodTemplateSpec)
831+
if err != nil {
832+
ctxLogger := log.FromContext(ctx)
833+
ctxLogger.Error(err, "Invalid PodTemplateSpec in MCPServer spec, continuing without customizations")
834+
// Continue without pod template patch - the deployment will still work
835+
} else {
836+
finalPodTemplateSpec := builder.WithSecrets(m.Spec.Secrets).Build()
837+
// Add pod template patch if we have one
838+
if finalPodTemplateSpec != nil {
839+
podTemplatePatch, err := json.Marshal(finalPodTemplateSpec)
840+
if err != nil {
841+
ctxLogger := log.FromContext(ctx)
842+
ctxLogger.Error(err, "Failed to marshal pod template spec")
843+
} else {
844+
args = append(args, fmt.Sprintf("--k8s-pod-patch=%s", string(podTemplatePatch)))
845+
}
770846
}
771847
}
772848
} else {
@@ -794,18 +870,26 @@ func (r *MCPServerReconciler) deploymentForMCPServer(ctx context.Context, m *mcp
794870
defaultSA := mcpServerServiceAccountName(m.Name)
795871
serviceAccount = &defaultSA
796872
}
797-
finalPodTemplateSpec := NewMCPServerPodTemplateSpecBuilder(m.Spec.PodTemplateSpec).
798-
WithServiceAccount(serviceAccount).
799-
WithSecrets(m.Spec.Secrets).
800-
Build()
801-
// Add pod template patch if we have one
802-
if finalPodTemplateSpec != nil {
803-
podTemplatePatch, err := json.Marshal(finalPodTemplateSpec)
804-
if err != nil {
805-
ctxLogger := log.FromContext(ctx)
806-
ctxLogger.Error(err, "Failed to marshal pod template spec")
807-
} else {
808-
args = append(args, fmt.Sprintf("--k8s-pod-patch=%s", string(podTemplatePatch)))
873+
874+
builder, err := NewMCPServerPodTemplateSpecBuilder(m.Spec.PodTemplateSpec)
875+
if err != nil {
876+
ctxLogger := log.FromContext(ctx)
877+
ctxLogger.Error(err, "Invalid PodTemplateSpec in MCPServer spec, continuing without customizations")
878+
// Continue without pod template patch - the deployment will still work
879+
} else {
880+
finalPodTemplateSpec := builder.
881+
WithServiceAccount(serviceAccount).
882+
WithSecrets(m.Spec.Secrets).
883+
Build()
884+
// Add pod template patch if we have one
885+
if finalPodTemplateSpec != nil {
886+
podTemplatePatch, err := json.Marshal(finalPodTemplateSpec)
887+
if err != nil {
888+
ctxLogger := log.FromContext(ctx)
889+
ctxLogger.Error(err, "Failed to marshal pod template spec")
890+
} else {
891+
args = append(args, fmt.Sprintf("--k8s-pod-patch=%s", string(podTemplatePatch)))
892+
}
809893
}
810894
}
811895

@@ -1379,21 +1463,19 @@ func (r *MCPServerReconciler) deploymentNeedsUpdate(
13791463
// TODO: Add more comprehensive checks for PodTemplateSpec changes beyond just the args
13801464
// This would involve comparing the actual pod template spec fields with what would be
13811465
// generated by the operator, rather than just checking the command-line arguments.
1382-
if mcpServer.Spec.PodTemplateSpec != nil {
1383-
podTemplatePatch, err := json.Marshal(mcpServer.Spec.PodTemplateSpec)
1384-
if err == nil {
1385-
podTemplatePatchArg := fmt.Sprintf("--k8s-pod-patch=%s", string(podTemplatePatch))
1386-
found := false
1387-
for _, arg := range container.Args {
1388-
if arg == podTemplatePatchArg {
1389-
found = true
1390-
break
1391-
}
1392-
}
1393-
if !found {
1394-
return true
1466+
if mcpServer.Spec.PodTemplateSpec != nil && mcpServer.Spec.PodTemplateSpec.Raw != nil {
1467+
// Use the raw bytes directly since PodTemplateSpec is now a RawExtension
1468+
podTemplatePatchArg := fmt.Sprintf("--k8s-pod-patch=%s", string(mcpServer.Spec.PodTemplateSpec.Raw))
1469+
found := false
1470+
for _, arg := range container.Args {
1471+
if arg == podTemplatePatchArg {
1472+
found = true
1473+
break
13951474
}
13961475
}
1476+
if !found {
1477+
return true
1478+
}
13971479
}
13981480

13991481
// Check if the container port has changed
@@ -1447,7 +1529,14 @@ func (r *MCPServerReconciler) deploymentNeedsUpdate(
14471529
defaultSA := mcpServerServiceAccountName(mcpServer.Name)
14481530
serviceAccount = &defaultSA
14491531
}
1450-
expectedPodTemplateSpec := NewMCPServerPodTemplateSpecBuilder(mcpServer.Spec.PodTemplateSpec).
1532+
1533+
builder, err := NewMCPServerPodTemplateSpecBuilder(mcpServer.Spec.PodTemplateSpec)
1534+
if err != nil {
1535+
// If we can't parse the PodTemplateSpec, consider it as needing update
1536+
return true
1537+
}
1538+
1539+
expectedPodTemplateSpec := builder.
14511540
WithServiceAccount(serviceAccount).
14521541
WithSecrets(mcpServer.Spec.Secrets).
14531542
Build()
@@ -1513,7 +1602,6 @@ func (r *MCPServerReconciler) deploymentNeedsUpdate(
15131602
if !equalOpenTelemetryArgs(otelConfig, container.Args) {
15141603
return true
15151604
}
1516-
15171605
}
15181606

15191607
// Check if the service account name has changed

0 commit comments

Comments
 (0)