CORENET-7134: Add ovn-kubernetes toolset skeleton#315
Conversation
|
@mattedallo: This pull request references CORENET-7134 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "5.0.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis PR adds a new OVN-Kubernetes toolset to the OpenShift MCP server. It defines a toolset struct with network troubleshooting capabilities, registers it in the module system, and integrates it into the existing toolset test suite with snapshot validation. ChangesOVN-Kubernetes Toolset
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
Skipping CI for Draft Pull Request. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: mattedallo The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
/test all |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
@mattedallo: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
| @@ -0,0 +1,38 @@ | |||
| package ovnkubernetes | |||
There was a problem hiding this comment.
seems pretty empty
why not land this with actual tools (e.g. as go module)?
There was a problem hiding this comment.
Hi @matzew, thanks for reviewing!
Yep, we have a followup PRs coming for the tools.
But If you prefer I may add some of them into this PR directly.
There was a problem hiding this comment.
yes - I'd think we do not land empty toolsets here
There was a problem hiding this comment.
+1 let's at least try to land the first tool in this PR. If you want to then incrementally add some more tools in future PRs that is cool
There was a problem hiding this comment.
Hi @matzew and @Cali0707, While adding the tools I am considering this approach:
the upstream exports tool definitions using gosdk.Tool from the go-sdk (a type both repos already depend on), paired with a type-erased execution function. In here we just iterates and converts (From gosdk.Tool to api.Tool) , no manual schema duplication. When upstream adds a new tool, downstream would have the tool automatically with version bump.
The benefit is that we don't duplicate the schema definition and we get the new tools automatically with version bump.
Is this something desired or you see problems with this approach?
|
@mattedallo also, if this new toolset is directly mapping to "upstream" ovn-k8s, I think the right address for PRs like that might be the actual upstream repo. Here, on OCP downstream, we might want to have mostly OCP specifics. That said, see recent discussion on #122 that for some CRUD tools, it might be also just fine to provide evals and see if those are passing already w/ our |
Register an empty ovn-kubernetes toolset so sibling stories (CORENET-7135/7136) can land OVN/OVS tool handlers on top of this framework. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Matteo Dallaglio <mdallagl@redhat.com>
Import the ovn-kubernetes-mcp library which provides OVN network analysis tools (show, get, lflow-list, trace) as an MCP server. Signed-off-by: Matteo Dallaglio <mdallagl@redhat.com>
Prototype of upstream changes that export tool definitions via AllToolRegistrations() using *mcp.Tool as the shared interface: - Add ExecTarget type for role-based container routing - Add MakeExecute generic helper for type-erased method dispatch - Add TargetSelector per tool (dbTarget, staticTarget) - Export named tool vars (ShowTool, GetTool, LFlowListTool, TraceTool) - Refactor AddTools() to use exported tool vars This is a proof-of-concept — the actual change would be an upstream PR to github.com/ovn-kubernetes/ovn-kubernetes-mcp. Signed-off-by: Matteo Dallaglio <mdallagl@redhat.com>
Hey @matzew, one of the reasons not to add these tools to upstream is because some of the ovnk tools need the node-debug functionality and as per some existing conversations it seems that adding that tool upstream is not in current plans. Additionally, upstream k8s-mcp-server might want to be CNI agnostic whereas for openshift-mcp-server these tools will be very useful as most customers use ovnk as the CNI. We already have a separate upstream repo for ovnk mcp server (https://github.com/ovn-kubernetes/ovn-kubernetes-mcp) and thus adding these tools in k8s-mcp-server will mean that 2 separate upstream projects have the same tools, which probably is not ideal. I have created an EP (openshift/enhancements#2002) where we can have further discussions on this topic. PTAL. |
Consume ovn-kubernetes-mcp's AllToolRegistrations() in a generic loop that converts *mcp.Tool to api.Tool and wires handlers automatically. No per-tool knowledge in the downstream — new tools added upstream propagate on go mod vendor with zero code changes here. Signed-off-by: Matteo Dallaglio <mdallagl@redhat.com>
1124f8a to
39da71e
Compare
Summary
Registers a new
ovn-kubernetestoolset as scaffolding for OVN-Kubernetes networking diagnostics integration (enhancements#2002).pkg/toolsets/ovnkubernetes/toolset.go— empty toolset implementingapi.Toolset, registered viainit()pkg/mcp/modules.goovn-kubernetesto theTestGranularToolsetsToolssnapshot test with an empty[]snapshotThe toolset ships with zero tools — sibling stories CORENET-7135 (OVS tools) and CORENET-7136 (OVN tools) will add tool handlers on top of this framework.
Verification
make build— binary compiles with the new toolsetmake test— all tests pass (including the new snapshot test)make lint— no lint violationsWhat is NOT in this PR
ovn-kubernetes-mcpGo dependency (arrives with tool implementations)Summary by CodeRabbit
Release Notes
New Features
Tests