Ensure we don't reconcile an invalid pod template spec.

Signed-off-by: Juan Antonio Osorio <[email protected]>

Author: JAORMX

cmd/thv-operator/controllers/mcpserver_controller.go

@@ -298,7 +298,12 @@ func (r *MCPServerReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
 	err = r.Get(ctx, types.NamespacedName{Name: mcpServer.Name, Namespace: mcpServer.Namespace}, deployment)
 	if err != nil && errors.IsNotFound(err) {
 		// Validate PodTemplateSpec and update status
-		r.validateAndUpdatePodTemplateStatus(ctx, mcpServer)
+		if !r.validateAndUpdatePodTemplateStatus(ctx, mcpServer) {
+			// Invalid PodTemplateSpec - return without error to avoid infinite retries
+			// The user must fix the spec and the next reconciliation will retry
+			ctxLogger.Info("Skipping deployment creation due to invalid PodTemplateSpec")
+			return ctrl.Result{}, nil
+		}
 
 		// Define a new deployment
 		dep := r.deploymentForMCPServer(ctx, mcpServer)
@@ -370,7 +375,12 @@ func (r *MCPServerReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
 	// Check if the deployment spec changed
 	if r.deploymentNeedsUpdate(ctx, deployment, mcpServer) {
 		// Validate PodTemplateSpec and update status
-		r.validateAndUpdatePodTemplateStatus(ctx, mcpServer)
+		if !r.validateAndUpdatePodTemplateStatus(ctx, mcpServer) {
+			// Invalid PodTemplateSpec - return without error to avoid infinite retries
+			// The user must fix the spec and the next reconciliation will retry
+			ctxLogger.Info("Skipping deployment update due to invalid PodTemplateSpec")
+			return ctrl.Result{}, nil
+		}
 
 		// Update the deployment
 		newDeployment := r.deploymentForMCPServer(ctx, mcpServer)
@@ -416,43 +426,55 @@ func setImageValidationCondition(mcpServer *mcpv1alpha1.MCPServer, status metav1
 
 // validateAndUpdatePodTemplateStatus validates the PodTemplateSpec and updates the MCPServer status
 // with appropriate conditions and events
-func (r *MCPServerReconciler) validateAndUpdatePodTemplateStatus(ctx context.Context, mcpServer *mcpv1alpha1.MCPServer) {
+func (r *MCPServerReconciler) validateAndUpdatePodTemplateStatus(ctx context.Context, mcpServer *mcpv1alpha1.MCPServer) bool {
 	ctxLogger := log.FromContext(ctx)
 
 	// Only validate if PodTemplateSpec is provided
 	if mcpServer.Spec.PodTemplateSpec == nil || mcpServer.Spec.PodTemplateSpec.Raw == nil {
-		return
+		// No PodTemplateSpec provided, validation passes
+		return true
 	}
 
 	_, err := NewMCPServerPodTemplateSpecBuilder(mcpServer.Spec.PodTemplateSpec)
 	if err != nil {
 		// Record event for invalid PodTemplateSpec
 		r.Recorder.Eventf(mcpServer, corev1.EventTypeWarning, "InvalidPodTemplateSpec",
-			"Failed to parse PodTemplateSpec: %v. Deployment will continue without pod customizations.", err)
+			"Failed to parse PodTemplateSpec: %v. Deployment blocked until PodTemplateSpec is fixed.", err)
 
 		// Set condition for invalid PodTemplateSpec
 		meta.SetStatusCondition(&mcpServer.Status.Conditions, metav1.Condition{
 			Type:               "PodTemplateValid",
 			Status:             metav1.ConditionFalse,
 			ObservedGeneration: mcpServer.Generation,
 			Reason:             "InvalidPodTemplateSpec",
-			Message:            fmt.Sprintf("Failed to parse PodTemplateSpec: %v. Deployment continues without customizations.", err),
-		})
-	} else {
-		// Set condition for valid PodTemplateSpec
-		meta.SetStatusCondition(&mcpServer.Status.Conditions, metav1.Condition{
-			Type:               "PodTemplateValid",
-			Status:             metav1.ConditionTrue,
-			ObservedGeneration: mcpServer.Generation,
-			Reason:             "ValidPodTemplateSpec",
-			Message:            "PodTemplateSpec is valid",
+			Message:            fmt.Sprintf("Failed to parse PodTemplateSpec: %v. Deployment blocked until fixed.", err),
 		})
+
+		// Update status with the condition
+		if statusErr := r.Status().Update(ctx, mcpServer); statusErr != nil {
+			ctxLogger.Error(statusErr, "Failed to update MCPServer status with PodTemplateSpec validation")
+		}
+
+		ctxLogger.Error(err, "PodTemplateSpec validation failed")
+		return false
 	}
 
+	// Set condition for valid PodTemplateSpec
+	meta.SetStatusCondition(&mcpServer.Status.Conditions, metav1.Condition{
+		Type:               "PodTemplateValid",
+		Status:             metav1.ConditionTrue,
+		ObservedGeneration: mcpServer.Generation,
+		Reason:             "ValidPodTemplateSpec",
+		Message:            "PodTemplateSpec is valid",
+	})
+
 	// Update status with the condition
 	if statusErr := r.Status().Update(ctx, mcpServer); statusErr != nil {
 		ctxLogger.Error(statusErr, "Failed to update MCPServer status with PodTemplateSpec validation")
 	}
+
+	ctxLogger.V(1).Info("PodTemplateSpec validation completed successfully")
+	return true
 }
 
 // handleRestartAnnotation checks if the restart annotation has been updated and triggers a restart if needed

cmd/thv-operator/controllers/mcpserver_invalid_podtemplate_integration_test.go

@@ -132,8 +132,8 @@ func TestMCPServerReconciler_InvalidPodTemplateSpec(t *testing.T) {
 
 			// Reconcile
 			_, err := r.Reconcile(ctx, req)
-			// We expect the reconciliation to succeed even with invalid PodTemplateSpec
-			// as it should continue without customizations
+			// We expect the reconciliation to succeed (no error) even with invalid PodTemplateSpec
+			// to avoid infinite retries. The deployment should not be created though.
 			require.NoError(t, err)
 
 			// Check the MCPServer status conditions
@@ -150,7 +150,7 @@ func TestMCPServerReconciler_InvalidPodTemplateSpec(t *testing.T) {
 
 				if tt.expectConditionStatus == metav1.ConditionFalse {
 					assert.Contains(t, condition.Message, "Failed to parse PodTemplateSpec")
-					assert.Contains(t, condition.Message, "Deployment continues without customizations")
+					assert.Contains(t, condition.Message, "Deployment blocked until fixed")
 				}
 			}