Skip to content
Open
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
10 changes: 7 additions & 3 deletions apiclient/types/mcpserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -235,6 +235,9 @@ type MCPServer struct {
// NeedsUpdate indicates whether the configuration in this server's catalog entry has drift from this server's configuration.
NeedsUpdate bool `json:"needsUpdate,omitempty"`

// NeedsK8sUpdate indicates whether this server needs redeployment with new K8s settings
NeedsK8sUpdate bool `json:"needsK8sUpdate,omitempty"`

// NeedsURL indicates whether the server's URL needs to be updated to match the catalog entry.
NeedsURL bool `json:"needsURL,omitempty"`

Expand Down Expand Up @@ -313,9 +316,10 @@ type ProjectMCPServer struct {
Runtime Runtime `json:"runtime,omitempty"`

// The following status fields are always copied from the MCPServer that this points to.
Configured bool `json:"configured"`
NeedsURL bool `json:"needsURL"`
NeedsUpdate bool `json:"needsUpdate"`
Configured bool `json:"configured"`
NeedsURL bool `json:"needsURL"`
NeedsUpdate bool `json:"needsUpdate"`
NeedsK8sUpdate bool `json:"needsK8sUpdate"`
}

type ProjectMCPServerList List[ProjectMCPServer]
Expand Down
1 change: 1 addition & 0 deletions pkg/api/handlers/mcp.go
Original file line number Diff line number Diff line change
Expand Up @@ -2463,6 +2463,7 @@ func ConvertMCPServer(server v1.MCPServer, credEnv map[string]string, serverURL,
MCPCatalogID: server.Spec.MCPCatalogID,
ConnectURL: connectURL,
NeedsUpdate: server.Status.NeedsUpdate,
NeedsK8sUpdate: server.Status.NeedsK8sUpdate,
NeedsURL: server.Spec.NeedsURL,
PreviousURL: server.Spec.PreviousURL,
MCPServerInstanceUserCount: server.Status.MCPServerInstanceUserCount,
Expand Down
8 changes: 5 additions & 3 deletions pkg/api/handlers/projectmcp.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,9 +51,10 @@ func convertProjectMCPServer(projectServer *v1.ProjectMCPServer, mcpServer *v1.M
Runtime: mcpServer.Spec.Manifest.Runtime,

// Default values to show to the user for shared servers:
Configured: true,
NeedsURL: false,
NeedsUpdate: false,
Configured: true,
NeedsURL: false,
NeedsUpdate: false,
NeedsK8sUpdate: false,
}
pmcp.Alias = mcpServer.Spec.Alias

Expand All @@ -66,6 +67,7 @@ func convertProjectMCPServer(projectServer *v1.ProjectMCPServer, mcpServer *v1.M
pmcp.Configured = convertedServer.Configured
pmcp.NeedsURL = convertedServer.NeedsURL
pmcp.NeedsUpdate = convertedServer.NeedsUpdate
pmcp.NeedsK8sUpdate = convertedServer.NeedsK8sUpdate
}

return pmcp
Expand Down
30 changes: 30 additions & 0 deletions pkg/controller/handlers/deployment/deployment.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ import (

"github.com/obot-platform/nah/pkg/apply"
"github.com/obot-platform/nah/pkg/router"
"github.com/obot-platform/obot/apiclient/types"
"github.com/obot-platform/obot/pkg/mcp"
v1 "github.com/obot-platform/obot/pkg/storage/apis/obot.obot.ai/v1"
"github.com/obot-platform/obot/pkg/system"
appsv1 "k8s.io/api/apps/v1"
Expand Down Expand Up @@ -84,13 +86,41 @@ func (h *Handler) UpdateMCPServerStatus(req router.Request, _ router.Response) e
mcpServer.Status.DeploymentConditions = conditions
needsUpdate = true
}

// Update K8s settings hash if it changed
// Note: k8sSettingsHash will be empty string for non-K8s runtimes or if annotation is missing
if mcpServer.Status.K8sSettingsHash != k8sSettingsHash {
mcpServer.Status.K8sSettingsHash = k8sSettingsHash
needsUpdate = true
}

// Manage NeedsK8sUpdate flag for K8s-compatible runtimes
isK8sRuntime := mcpServer.Spec.Manifest.Runtime == types.RuntimeContainerized ||
mcpServer.Spec.Manifest.Runtime == types.RuntimeUVX ||
mcpServer.Spec.Manifest.Runtime == types.RuntimeNPX

if isK8sRuntime {
// Get current K8s settings to compare
var k8sSettings v1.K8sSettings
if err := h.storageClient.Get(req.Ctx, kclient.ObjectKey{
Namespace: h.mcpNamespace,
Name: system.K8sSettingsName,
}, &k8sSettings); err == nil {
currentHash := mcp.ComputeK8sSettingsHash(k8sSettings.Spec)

// Only SET to true if drift detected, never clear it
// The flag gets cleared by the RedeployWithK8sSettings API endpoint after successful redeploy
hasHash := k8sSettingsHash != ""
hasDrift := k8sSettingsHash != currentHash
flagNotSet := !mcpServer.Status.NeedsK8sUpdate

if hasHash && hasDrift && flagNotSet {
mcpServer.Status.NeedsK8sUpdate = true
needsUpdate = true
}
}
}

// Update the MCPServer status if needed
if needsUpdate {
return h.storageClient.Status().Update(req.Ctx, &mcpServer)
Expand Down
51 changes: 51 additions & 0 deletions pkg/controller/handlers/k8ssettings/k8ssettings.go
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file should be deleted.

The way our controller framework works, the two controllers you wrote (one for servers and one for catalog entries) will do the correct thing when the k8s settings change.

Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
package k8ssettings

import (
"github.com/obot-platform/nah/pkg/router"
"github.com/obot-platform/obot/pkg/mcp"
v1 "github.com/obot-platform/obot/pkg/storage/apis/obot.obot.ai/v1"
kclient "sigs.k8s.io/controller-runtime/pkg/client"
)

// UpdateAllServerK8sSettingsDrift updates the NeedsK8sUpdate status on all MCP servers when K8s settings change
func UpdateAllServerK8sSettingsDrift(req router.Request, _ router.Response) error {
k8sSettings := req.Object.(*v1.K8sSettings)

// Compute the new hash
currentHash := mcp.ComputeK8sSettingsHash(k8sSettings.Spec)

// List all MCP servers
var servers v1.MCPServerList
if err := req.List(&servers, &kclient.ListOptions{
Namespace: req.Object.GetNamespace(),
}); err != nil {
return err
}

// Update each server's NeedsK8sUpdate status
for i := range servers.Items {
server := &servers.Items[i]
// Skip servers without K8s settings hash (not deployed or non-K8s runtimes)
if server.Status.K8sSettingsHash == "" {
if server.Status.NeedsK8sUpdate {
server.Status.NeedsK8sUpdate = false
if err := req.Client.Status().Update(req.Ctx, server); err != nil {
return err
}
}
continue
}

// Check if the server needs update
needsK8sUpdate := server.Status.K8sSettingsHash != currentHash

if server.Status.NeedsK8sUpdate != needsK8sUpdate {
server.Status.NeedsK8sUpdate = needsK8sUpdate
if err := req.Client.Status().Update(req.Ctx, server); err != nil {
return err
}
}
}

return nil
}
38 changes: 38 additions & 0 deletions pkg/controller/handlers/mcpserver/mcpserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,44 @@ func (h *Handler) DetectDrift(req router.Request, _ router.Response) error {
return nil
}

// DetectK8sSettingsDrift detects when a server needs redeployment with new K8s settings
func (h *Handler) DetectK8sSettingsDrift(req router.Request, _ router.Response) error {
server := req.Object.(*v1.MCPServer)

// Only check for containerized or uvx/npx runtimes that might run in K8s
if server.Spec.Manifest.Runtime != types.RuntimeContainerized &&
server.Spec.Manifest.Runtime != types.RuntimeUVX &&
server.Spec.Manifest.Runtime != types.RuntimeNPX {
return nil
}

// Skip if server doesn't have K8s settings hash (not yet deployed)
if server.Status.K8sSettingsHash == "" {
return nil
}

// Get current K8s settings
var k8sSettings v1.K8sSettings
if err := req.Get(&k8sSettings, server.Namespace, system.K8sSettingsName); err != nil {
if apierrors.IsNotFound(err) {
return nil
}
return fmt.Errorf("failed to get K8s settings: %w", err)
}

// Compute current K8s settings hash
currentHash := mcp.ComputeK8sSettingsHash(k8sSettings.Spec)

// Only SET to true when drift detected, never clear it
// The flag gets cleared by the RedeployWithK8sSettings API endpoint after successful redeploy
if server.Status.K8sSettingsHash != currentHash && !server.Status.NeedsK8sUpdate {
server.Status.NeedsK8sUpdate = true
return req.Client.Status().Update(req.Ctx, server)
}

return nil
}

func configurationHasDrifted(serverManifest types.MCPServerManifest, entryManifest types.MCPServerCatalogEntryManifest) (bool, error) {
// Check if runtime types differ
if serverManifest.Runtime != entryManifest.Runtime {
Expand Down
5 changes: 5 additions & 0 deletions pkg/controller/routes.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"github.com/obot-platform/obot/pkg/controller/handlers/auditlogexport"
"github.com/obot-platform/obot/pkg/controller/handlers/cleanup"
"github.com/obot-platform/obot/pkg/controller/handlers/cronjob"
"github.com/obot-platform/obot/pkg/controller/handlers/k8ssettings"
"github.com/obot-platform/obot/pkg/controller/handlers/knowledgefile"
"github.com/obot-platform/obot/pkg/controller/handlers/knowledgeset"
"github.com/obot-platform/obot/pkg/controller/handlers/knowledgesource"
Expand Down Expand Up @@ -236,6 +237,7 @@ func (c *Controller) setupRoutes() {
root.Type(&v1.MCPServer{}).HandlerFunc(mcpserver.DeleteServersForAnonymousUser)
root.Type(&v1.MCPServer{}).HandlerFunc(mcpserver.CleanupNestedCompositeServers)
root.Type(&v1.MCPServer{}).HandlerFunc(mcpserver.DetectDrift)
root.Type(&v1.MCPServer{}).HandlerFunc(mcpserver.DetectK8sSettingsDrift)
root.Type(&v1.MCPServer{}).HandlerFunc(mcpserver.EnsureMCPServerInstanceUserCount)
root.Type(&v1.MCPServer{}).HandlerFunc(mcpserver.EnsureMCPServerSecretInfo)
root.Type(&v1.MCPServer{}).HandlerFunc(mcpserver.EnsureCompositeComponents)
Expand Down Expand Up @@ -310,6 +312,9 @@ func (c *Controller) setupRoutes() {
// ScheduledAuditLogExport
root.Type(&v1.ScheduledAuditLogExport{}).HandlerFunc(scheduledAuditLogExportHandler.ScheduleExports)

// K8sSettings
root.Type(&v1.K8sSettings{}).HandlerFunc(k8ssettings.UpdateAllServerK8sSettingsDrift)

c.toolRefHandler = toolRef
c.mcpCatalogHandler = mcpCatalog
c.adminWorkspaceHandler = adminWorkspaceHandler
Expand Down
30 changes: 30 additions & 0 deletions pkg/mcp/details.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,17 @@ package mcp

import (
"context"
"errors"
"fmt"
"io"
"slices"

"github.com/obot-platform/obot/apiclient/types"
)

// ErrServerNotRunning is returned when an MCP server is not running
var ErrServerNotRunning = errors.New("mcp server is not running")

// GetServerDetails will get the details of a specific MCP server based on its configuration, if the backend supports it.
// If the server is remote, it will return an error as remote servers do not support this operation.
// If the backend does not support the operation, it will return an [ErrNotSupportedByBackend] error.
Expand All @@ -17,6 +21,19 @@ func (sm *SessionManager) GetServerDetails(ctx context.Context, serverConfig Ser
return types.MCPServerDetails{}, fmt.Errorf("getting server details is not supported for remote servers")
}

// Try to get details first - only deploy if server doesn't exist
// This prevents unnecessary redeployments that would update K8s settings and clear the NeedsK8sUpdate flag
details, err := sm.backend.getServerDetails(ctx, serverConfig.MCPServerName)
if err == nil {
return details, nil
}

// Only deploy if server is not running - for any other error, return it
if !errors.Is(err, ErrServerNotRunning) {
return types.MCPServerDetails{}, err
}

// Server not running - deploy it
if err := sm.deployServer(ctx, serverConfig); err != nil {
return types.MCPServerDetails{}, err
}
Expand All @@ -32,6 +49,19 @@ func (sm *SessionManager) StreamServerLogs(ctx context.Context, serverConfig Ser
return nil, fmt.Errorf("streaming logs is not supported for remote servers")
}

// Check if server exists first - only deploy if it doesn't
// This prevents unnecessary redeployments that would update K8s settings and clear the NeedsK8sUpdate flag
_, err := sm.backend.getServerDetails(ctx, serverConfig.MCPServerName)
if err == nil {
return sm.backend.streamServerLogs(ctx, serverConfig.MCPServerName)
}

// Only deploy if server is not running - for any other error, return it
if !errors.Is(err, ErrServerNotRunning) {
return nil, err
}

// Server not running - deploy it
if err := sm.deployServer(ctx, serverConfig); err != nil {
return nil, err
}
Expand Down
2 changes: 2 additions & 0 deletions pkg/storage/apis/obot.obot.ai/v1/mcpserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,8 @@ type MCPServerStatus struct {
// This field is only populated for servers running in Kubernetes runtime.
// For Docker, local, or remote runtimes, this field is omitted entirely.
K8sSettingsHash string `json:"k8sSettingsHash,omitempty"`
// NeedsK8sUpdate indicates whether this server needs redeployment with new K8s settings
NeedsK8sUpdate bool `json:"needsK8sUpdate,omitempty"`
// AuditLogTokenHash is the hash of the token used to submit audit logs.
AuditLogTokenHash string `json:"auditLogTokenHash,omitempty"`
// ObservedCompositeManifestHash is the hash of the server's manifest the last time all component servers were updated to match the composite server.
Expand Down
23 changes: 22 additions & 1 deletion pkg/storage/openapi/generated/openapi_generated.go

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

Loading
Loading