(Feat) Disable mcpServer reconciliation via kagent.dev/discovery-disabled annotation

[jmhbh]

This PR enables disabling reconciliation of mcpServer CRDs that explicitly set the kagent.dev/discovery=disabled label.

Resolves #853

[Copilot]

Pull Request Overview

This PR implements a feature to disable reconciliation of mcpServer CRDs by introducing a kagent.dev/discovery-disabled annotation. When set to "true", the controller will skip processing these resources.

• Added a new predicate to filter out resources with the discovery disabled annotation
• Integrated the predicate into the MCPServerController setup
• Added comprehensive tests covering all event types and annotation scenarios

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File	Description
go/internal/controller/predicates/discovery_disabled.go	New predicate implementation that filters resources based on discovery-disabled annotation
go/internal/controller/predicates/discovery_disabled_test.go	Test coverage for the discovery disabled predicate functionality
go/internal/controller/mcp_server_controller.go	Integration of the new predicate into the controller setup

---

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

[onematchfox]

Just a comment here. I'd love to see consistency in the way that secondary resources are discovered. It feels weird to me that Services are opt-in via the label kagent.dev/mcp-service=true whereas MCPServer resources are opt-out via the kagent.dev/discovery-disabled annotation. IMO, all secondary resources should be opt-in using the same mechanism.

[EItanya]

Just a comment here. I'd love to see consistency in the way that secondary resources are discovered. It feels weird to me that Services are opt-in via the label kagent.dev/mcp-service=true whereas MCPServer resources are opt-out via the kagent.dev/discovery-disabled annotation. IMO, all secondary resources should be opt-in using the same mechanism.

Interesting. I think the concept here is that this is in fact a primary resource, but I do see what you’re saying. Why would you consider this a secondary resource?

[onematchfox]

Why would you consider this a secondary resource?

Hopefully, the contributor's meeting cleared this up. But for the sake of posterity and a wider audience, I'll comment here as well...

I see this as a secondary resource because the CRD is not owned by this project. It belongs to KMCP as does the controller that reconciles it (related to #822).

[linsun]

Why would you consider this a secondary resource?

Hopefully, the contributor's meeting cleared this up. But for the sake of posterity and a wider audience, I'll comment here as well...

I see this as a secondary resource because the CRD is not owned by this project. It belongs to KMCP as does the controller that reconciles it (related to #822).

Great thoughts @onematchfox! We are also interested in looking at merging kmcp as an optional component in kagent which would make MCPServer as first citizen resource in kagent.

[onematchfox]

What happens in the case of a resource that has already been discovered? Will this prevent it from being "undiscovered"?

[EItanya]

That is a great question. There's really no surefire way to catch this without checking every single time which feels like overkill. What do you think about a doc explaining how to remove the old entries for now?

[onematchfox]

There's really no surefire way to catch this without checking every single time which feels like overkill.

Actually, I think there is a better way of handling this. The update handler on the predicate has access to both the old (e.ObjectOld) and new (e.ObjectNew) versions of the object. So, rather than only checking the discovery configuration on the new object (!isDiscoveryDisabled(e.ObjectNew)) the controller should check both the old and the new versions and process any object where either of them do not have discovery disabled. That way, the controller is able to handle objects where discovery is disabled on an existing object and ensure that the tool server is removed.