Conversation
…04ing DeleteToolServer resolves a server's kind by name lookup in the discovery projection, whose only writer is the reconciler and which only runs after it has tried to reach the server. A RemoteMCPServer or MCPServer created moments ago is live in Kubernetes with no row in the projection yet, so the lookup returns an empty groupKind and DeleteToolServer answers NotFound for an object that exists and is never actually removed. Fall back to a live Kubernetes lookup across the kinds CreateToolServer can produce before treating an empty groupKind as nonexistence. This covers the "undeletable" half of kagent-dev#2849 only; the "invisible in the list" half needs the larger List-authoritative-over-Kubernetes redesign the issue itself proposes (a proto change plus a UI column), left for a separate change. Refs kagent-dev#2849 Signed-off-by: Amir Fathi <amirfathi.me@gmail.com>
moezdil
reviewed
Sep 29, 2026
| if err := s.kubeClient.Get(ctx, ref, &kmcp.MCPServer{}); err == nil { | ||
| return mcpServerGVK.GroupKind().String() | ||
| } | ||
| if err := s.kubeClient.Get(ctx, ref, &corev1.Service{}); err == nil { |
Contributor
There was a problem hiding this comment.
createtoolserver never makes services, so this deletes any service
Contributor
Author
There was a problem hiding this comment.
Good catch, dropped the Service check entirely. CreateToolServer never creates one, so it was only ever going to match an unrelated Service sharing the name and delete that instead. Pushed in d0418ac.
| if err := s.kubeClient.Get(ctx, ref, &corev1.Service{}); err == nil { | ||
| return "Service" | ||
| } | ||
| return "" |
Contributor
There was a problem hiding this comment.
rbac or timeout errors on get become a 404
Contributor
Author
There was a problem hiding this comment.
Right, an RBAC or timeout error here isn't the same as not found. Both Get calls now check IsNotFound explicitly and propagate anything else as an internal error. Same commit.
…t errors liveToolServerGroupKind() had two gaps moezdil caught on review: - It matched a bare corev1.Service by name/namespace as a fallback kind. CreateToolServer never creates a Service for a ToolServer, so this only ever matches an unrelated Service that happens to share the ref, and DeleteToolServer would go on to delete it. Dropped the Service check entirely; there is no creation-lag race to fix for a kind this path never creates. - Every Get error, not just NotFound, was folded into "keep trying the next kind" and ultimately into a bare 404. An RBAC or timeout error now propagates as an internal error instead of reading as ToolServer not found. Signed-off-by: Amir Fathi <amirfathi.me@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
DeleteToolServerresolves a server's kind by looking its name up in thediscovery projection (the
toolservertable). That table's only writer isthe reconciler, and the reconciler runs only after it has already tried to
reach the server over the network. So a
RemoteMCPServerorMCPServercreated moments ago is live in Kubernetes with no row in the projection yet:
the lookup returns an empty
groupKind, andDeleteToolServeranswersNotFoundfor an object that is genuinely there and never gets removed.This adds a fallback: when the projection has no row for the name, look the
object up live in Kubernetes across the kinds
CreateToolServercan produce,before answering
NotFound. A name that is absent from both the projectionand Kubernetes still 404s.
This covers the "undeletable" half of #2849 only. The "invisible in the
list" half needs the bigger change the issue itself proposes, making
ListToolServersauthoritative about existence by listing Kubernetesdirectly and left-joining the projection for discovered tools, which is a
proto change plus a UI column, so I left it for a separate PR.
How I verified this
TestServiceDeleteToolServerBeforeReconcile, which fails onmain(the pre-reconcile object 404s and is left undeleted) and passes on this
branch, for both
RemoteMCPServerandMCPServer; a third subtest checksa name absent from Kubernetes still 404s.
go test -race -v ./core/internal/service/tool/...passes, including theexisting
TestServiceDeleteToolServer.go vetandgolangci-lint runreport no issues on the changed files.server missing from the list for a few seconds); it still needs the
redesign above.
Refs #2849.