Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions cmd/thv-operator/api/v1alpha1/toolconfig_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -96,17 +96,17 @@ type MCPToolConfigStatus struct {
// +optional
ConfigHash string `json:"configHash,omitempty"`

// ReferencingServers is a list of MCPServer resources that reference this MCPToolConfig
// This helps track which servers need to be reconciled when this config changes
// ReferencingWorkloads is a list of workload resources that reference this MCPToolConfig.
// Each entry identifies the workload by kind and name.
// +optional
ReferencingServers []string `json:"referencingServers,omitempty"`
ReferencingWorkloads []WorkloadReference `json:"referencingWorkloads,omitempty"`
}

// +kubebuilder:object:root=true
// +kubebuilder:subresource:status
// +kubebuilder:resource:shortName=tc;toolconfig,categories=toolhive
// +kubebuilder:printcolumn:name="Ready",type=string,JSONPath=`.status.conditions[?(@.type=='Valid')].status`
// +kubebuilder:printcolumn:name="References",type=string,JSONPath=`.status.referencingServers`
// +kubebuilder:printcolumn:name="References",type=string,JSONPath=`.status.referencingWorkloads`
// +kubebuilder:printcolumn:name="Age",type=date,JSONPath=`.metadata.creationTimestamp`

// MCPToolConfig is the Schema for the mcptoolconfigs API.
Expand Down
6 changes: 3 additions & 3 deletions cmd/thv-operator/api/v1alpha1/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

178 changes: 96 additions & 82 deletions cmd/thv-operator/controllers/toolconfig_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,15 @@ func (r *ToolConfigReconciler) Reconcile(ctx context.Context, req ctrl.Request)
return r.handleConfigHashChange(ctx, toolConfig, configHash)
}

// Refresh ReferencingWorkloads list
referencingWorkloads, err := r.findReferencingWorkloads(ctx, toolConfig)
if err != nil {
logger.Error(err, "Failed to find referencing workloads")
} else if !slices.Equal(toolConfig.Status.ReferencingWorkloads, referencingWorkloads) {
toolConfig.Status.ReferencingWorkloads = referencingWorkloads
conditionChanged = true
}

// Update condition if it changed (even without hash change)
if conditionChanged {
if err := r.Status().Update(ctx, toolConfig); err != nil {
Expand All @@ -106,9 +115,7 @@ func (r *ToolConfigReconciler) Reconcile(ctx context.Context, req ctrl.Request)
}
}

// Even when hash hasn't changed, update referencing servers list.
// This ensures ReferencingServers is updated when MCPServers are created or deleted.
return r.updateReferencingServers(ctx, toolConfig)
return ctrl.Result{}, nil
}

// handleConfigHashChange handles the logic when the config hash changes
Expand All @@ -134,13 +141,12 @@ func (r *ToolConfigReconciler) handleConfigHashChange(
toolConfig.Status.ConfigHash = configHash
toolConfig.Status.ObservedGeneration = toolConfig.Generation

// Update the status with the list of referencing servers
serverNames := make([]string, 0, len(referencingServers))
// Update the status with the list of referencing workloads
refs := make([]mcpv1alpha1.WorkloadReference, 0, len(referencingServers))
for _, server := range referencingServers {
serverNames = append(serverNames, server.Name)
refs = append(refs, mcpv1alpha1.WorkloadReference{Kind: "MCPServer", Name: server.Name})
}
slices.Sort(serverNames)
toolConfig.Status.ReferencingServers = serverNames
toolConfig.Status.ReferencingWorkloads = refs

Comment thread
ChrisJBurns marked this conversation as resolved.
// Update the MCPToolConfig status
if err := r.Status().Update(ctx, toolConfig); err != nil {
Expand All @@ -166,46 +172,6 @@ func (r *ToolConfigReconciler) handleConfigHashChange(
return ctrl.Result{}, nil
}

// updateReferencingServers finds referencing MCPServers and updates the status if the list changed
func (r *ToolConfigReconciler) updateReferencingServers(
ctx context.Context,
toolConfig *mcpv1alpha1.MCPToolConfig,
) (ctrl.Result, error) {
referencingServers, err := r.findReferencingMCPServers(ctx, toolConfig)
if err != nil {
logger := log.FromContext(ctx)
logger.Error(err, "Failed to find referencing MCPServers")
meta.SetStatusCondition(&toolConfig.Status.Conditions, metav1.Condition{
Type: mcpv1alpha1.ConditionToolConfigValid,
Status: metav1.ConditionFalse,
Reason: mcpv1alpha1.ConditionReasonToolConfigValidationFailed,
Message: "Failed to find referencing MCPServers",
ObservedGeneration: toolConfig.Generation,
})
if updateErr := r.Status().Update(ctx, toolConfig); updateErr != nil {
logger.Error(updateErr, "Failed to update MCPToolConfig status after error")
}
return ctrl.Result{}, fmt.Errorf("failed to find referencing MCPServers: %w", err)
}

serverNames := make([]string, 0, len(referencingServers))
for _, server := range referencingServers {
serverNames = append(serverNames, server.Name)
}
slices.Sort(serverNames)

if !slices.Equal(toolConfig.Status.ReferencingServers, serverNames) {
toolConfig.Status.ReferencingServers = serverNames
if err := r.Status().Update(ctx, toolConfig); err != nil {
logger := log.FromContext(ctx)
logger.Error(err, "Failed to update MCPToolConfig status")
return ctrl.Result{}, err
}
}

return ctrl.Result{}, nil
}

// calculateConfigHash calculates a hash of the MCPToolConfig spec using Kubernetes utilities
func (*ToolConfigReconciler) calculateConfigHash(spec mcpv1alpha1.MCPToolConfigSpec) string {
return ctrlutil.CalculateConfigHash(spec)
Expand All @@ -216,31 +182,32 @@ func (r *ToolConfigReconciler) handleDeletion(ctx context.Context, toolConfig *m
logger := log.FromContext(ctx)

if controllerutil.ContainsFinalizer(toolConfig, ToolConfigFinalizerName) {
// Check if any MCPServers are still referencing this MCPToolConfig
referencingServers, err := r.findReferencingMCPServers(ctx, toolConfig)
// Check if any workloads still reference this MCPToolConfig
referencingWorkloads, err := r.findReferencingWorkloads(ctx, toolConfig)
if err != nil {
logger.Error(err, "Failed to find referencing MCPServers during deletion")
logger.Error(err, "Failed to check referencing workloads during deletion")
return ctrl.Result{}, err
}

if len(referencingServers) > 0 {
// Cannot delete - still referenced
serverNames := make([]string, 0, len(referencingServers))
for _, server := range referencingServers {
serverNames = append(serverNames, server.Name)
if len(referencingWorkloads) > 0 {
logger.Info("MCPToolConfig is still referenced by workloads, blocking deletion",
"toolconfig", toolConfig.Name,
"referencingWorkloads", referencingWorkloads)

meta.SetStatusCondition(&toolConfig.Status.Conditions, metav1.Condition{
Type: "DeletionBlocked",
Status: metav1.ConditionTrue,
Reason: "ReferencedByWorkloads",
Message: fmt.Sprintf("Cannot delete: referenced by workloads: %v", referencingWorkloads),
ObservedGeneration: toolConfig.Generation,
})
toolConfig.Status.ReferencingWorkloads = referencingWorkloads
if updateErr := r.Status().Update(ctx, toolConfig); updateErr != nil {
logger.Error(updateErr, "Failed to update status during deletion block")
}
logger.Info("Cannot delete MCPToolConfig - still referenced by MCPServers",
"toolconfig", toolConfig.Name, "referencingServers", serverNames)

// Update status to show it's still referenced
toolConfig.Status.ReferencingServers = serverNames
if err := r.Status().Update(ctx, toolConfig); err != nil {
logger.Error(err, "Failed to update MCPToolConfig status during deletion")
}

// Return an error to prevent deletion
return ctrl.Result{}, fmt.Errorf("MCPToolConfig %s is still referenced by MCPServers: %v",
toolConfig.Name, serverNames)
// Requeue to check again later
return ctrl.Result{RequeueAfter: 30 * time.Second}, nil
}

// No references, safe to remove finalizer and allow deletion
Expand All @@ -255,7 +222,28 @@ func (r *ToolConfigReconciler) handleDeletion(ctx context.Context, toolConfig *m
return ctrl.Result{}, nil
}

// findReferencingMCPServers finds all MCPServers that reference the given MCPToolConfig
// findReferencingWorkloads returns the workload resources (MCPServer)
// that reference this MCPToolConfig via their ToolConfigRef field.
func (r *ToolConfigReconciler) findReferencingWorkloads(
ctx context.Context,
toolConfig *mcpv1alpha1.MCPToolConfig,
) ([]mcpv1alpha1.WorkloadReference, error) {
mcpServerList := &mcpv1alpha1.MCPServerList{}
if err := r.List(ctx, mcpServerList, client.InNamespace(toolConfig.Namespace)); err != nil {
return nil, fmt.Errorf("failed to list MCPServers: %w", err)
}

var refs []mcpv1alpha1.WorkloadReference
for _, server := range mcpServerList.Items {
if server.Spec.ToolConfigRef != nil && server.Spec.ToolConfigRef.Name == toolConfig.Name {
refs = append(refs, mcpv1alpha1.WorkloadReference{Kind: "MCPServer", Name: server.Name})
}
}
return refs, nil
}

// findReferencingMCPServers finds all MCPServers that reference the given MCPToolConfig.
// Returns the full MCPServer objects, used by handleConfigHashChange to update server annotations.
func (r *ToolConfigReconciler) findReferencingMCPServers(
ctx context.Context,
toolConfig *mcpv1alpha1.MCPToolConfig,
Expand All @@ -270,33 +258,59 @@ func (r *ToolConfigReconciler) findReferencingMCPServers(
}

// SetupWithManager sets up the controller with the Manager.
// Watches MCPServer changes to maintain accurate ReferencingWorkloads status.
func (r *ToolConfigReconciler) SetupWithManager(mgr ctrl.Manager) error {
// Create a handler that maps MCPServer changes to MCPToolConfig reconciliation requests.
// When an MCPServer is created/updated/deleted, we need to reconcile the MCPToolConfig
// it references so that the ReferencingServers status field stays up to date.
// Watch MCPServer changes to update ReferencingWorkloads on referenced MCPToolConfigs.
// This handler enqueues both the currently-referenced MCPToolConfig AND any
// MCPToolConfig that still lists this server in ReferencingWorkloads (covers the
// case where a server removes its toolConfigRef — the previously-referenced
// config needs to reconcile and clean up the stale entry).
toolConfigHandler := handler.EnqueueRequestsFromMapFunc(
func(_ context.Context, obj client.Object) []reconcile.Request {
mcpServer, ok := obj.(*mcpv1alpha1.MCPServer)
func(ctx context.Context, obj client.Object) []reconcile.Request {
server, ok := obj.(*mcpv1alpha1.MCPServer)
if !ok {
return nil
}

if mcpServer.Spec.ToolConfigRef == nil {
return nil
seen := make(map[types.NamespacedName]struct{})
var requests []reconcile.Request

// Enqueue the currently-referenced MCPToolConfig (if any)
if server.Spec.ToolConfigRef != nil {
nn := types.NamespacedName{
Name: server.Spec.ToolConfigRef.Name,
Namespace: server.Namespace,
}
seen[nn] = struct{}{}
requests = append(requests, reconcile.Request{NamespacedName: nn})
}

// Also enqueue any MCPToolConfig that still lists this server in
// ReferencingWorkloads — handles ref-removal and server-deletion cases.
toolConfigList := &mcpv1alpha1.MCPToolConfigList{}
if err := r.List(ctx, toolConfigList, client.InNamespace(server.Namespace)); err != nil {
log.FromContext(ctx).Error(err, "Failed to list MCPToolConfigs for MCPServer watch")
return requests
}
for _, cfg := range toolConfigList.Items {
nn := types.NamespacedName{Name: cfg.Name, Namespace: cfg.Namespace}
if _, already := seen[nn]; already {
continue
}
for _, ref := range cfg.Status.ReferencingWorkloads {
if ref.Kind == "MCPServer" && ref.Name == server.Name {
requests = append(requests, reconcile.Request{NamespacedName: nn})
break
}
}
}

return []reconcile.Request{{
NamespacedName: types.NamespacedName{
Name: mcpServer.Spec.ToolConfigRef.Name,
Namespace: mcpServer.Namespace,
},
}}
return requests
},
)

return ctrl.NewControllerManagedBy(mgr).
For(&mcpv1alpha1.MCPToolConfig{}).
// Watch for MCPServers and reconcile the MCPToolConfig when they change
Watches(&mcpv1alpha1.MCPServer{}, toolConfigHandler).
Complete(r)
}
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,8 @@ func TestToolConfigReconciler_EdgeCases(t *testing.T) {
err = fakeClient.Get(ctx, req.NamespacedName, &updatedConfig)
require.NoError(t, err)
assert.NotEmpty(t, updatedConfig.Status.ConfigHash)
assert.Contains(t, updatedConfig.Status.ReferencingServers, "test-server")
assert.Contains(t, updatedConfig.Status.ReferencingWorkloads,
mcpv1alpha1.WorkloadReference{Kind: "MCPServer", Name: "test-server"})
})

t.Run("reconcile with changed spec", func(t *testing.T) {
Expand Down Expand Up @@ -350,10 +351,13 @@ func TestToolConfigReconciler_ComplexScenarios(t *testing.T) {
var updatedConfig mcpv1alpha1.MCPToolConfig
err = fakeClient.Get(ctx, req.NamespacedName, &updatedConfig)
require.NoError(t, err)
assert.Len(t, updatedConfig.Status.ReferencingServers, 3)
assert.Contains(t, updatedConfig.Status.ReferencingServers, "server1")
assert.Contains(t, updatedConfig.Status.ReferencingServers, "server2")
assert.Contains(t, updatedConfig.Status.ReferencingServers, "server3")
assert.Len(t, updatedConfig.Status.ReferencingWorkloads, 3)
assert.Contains(t, updatedConfig.Status.ReferencingWorkloads,
mcpv1alpha1.WorkloadReference{Kind: "MCPServer", Name: "server1"})
assert.Contains(t, updatedConfig.Status.ReferencingWorkloads,
mcpv1alpha1.WorkloadReference{Kind: "MCPServer", Name: "server2"})
assert.Contains(t, updatedConfig.Status.ReferencingWorkloads,
mcpv1alpha1.WorkloadReference{Kind: "MCPServer", Name: "server3"})
})

t.Run("empty MCPToolConfig spec", func(t *testing.T) {
Expand Down
Loading
Loading