NE-2488: Add OpenShift router tools to NetworkEdge toolset#98
NE-2488: Add OpenShift router tools to NetworkEdge toolset#98alebedev87 wants to merge 1 commit intoopenshift:mainfrom
Conversation
|
Skipping CI for Draft Pull Request. |
openshift-ci
bot
added
the
do-not-merge/work-in-progress
|
Should we consider having this as a part of the "openshift" toolgroup? |
alebedev87
force-pushed
the
router-toolset
branch
from
899262a to
66029cc
Compare
openshift-ci
bot
removed
the
do-not-merge/work-in-progress
@swghosh : That crossed my mind too. In my case "router" doesn't make much sense in Kubernetes context. However I didn't want to complicate things at this stage either. |
alebedev87
force-pushed
the
router-toolset
branch
from
66029cc to
ca4989f
Compare
|
Wherever this ends up we'll likely end up using it as part of our NIDS MCP tooling. https://issues.redhat.com/browse/NE-2278 |
@bentito @alebedev87 any updates on this? or still on discussion. |
openshift-merge-robot
added
the
needs-rebase
|
alebedev87
force-pushed
the
router-toolset
branch
from
ca4989f to
b9d7ed9
Compare
openshift-merge-robot
removed
the
needs-rebase
alebedev87
changed the title
openshift-ci-robot
added
the
jira/valid-reference
|
@alebedev87: This pull request references NE-2488 which is a valid jira issue. 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. |
alebedev87
force-pushed
the
router-toolset
branch
from
b9d7ed9 to
7871a5b
Compare
|
@alebedev87: This pull request references NE-2488 which is a valid jira issue. 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. |
| Title: "Get Router Config", | ||
| ReadOnlyHint: ptr.To(true), | ||
| DestructiveHint: ptr.To(false), | ||
| OpenWorldHint: ptr.To(true), |
There was a problem hiding this comment.
@bentito : I used false here before but then I copied true as in get_coredns_config tool. I'm not quite sure whether it's a good decision for router configs/sessions/info. Can you please advice?
There was a problem hiding this comment.
Yes true is correct for all three tools, since all three router tools exec into a live, running router pod on the cluster. False would be for a tool that had all the data locally already, not the case here.
(Sorry I missed this comment for quite awhile)
|
I've reviewed this PR in the context of the NIDS MCP strategy and found a few critical areas for improvement, particularly regarding offline analysis. Review Summary: Router Tools vs Offline StrategyContext:
1. Critical Refactoring Required: Offline CompatibilityThe current implementation of
2. Scope Clarification: "Live Only" ToolsThe tools
3. Consistency: Client UsagePR #98 uses |
alebedev87
force-pushed
the
router-toolset
branch
from
7871a5b to
7ce7684
Compare
|
Regarding the first 2 points ( |
|
/assign @bentito |
|
I
I've added haproxy-gather to the list of offline artifacts. So then it might make sense, right? |
|
/assign @matzew |
|
@matzew I think this can merge, I will track for any refactoring these tools might need with a follow-up PR if that's okay? |
alebedev87
force-pushed
the
router-toolset
branch
from
99a95dd to
f62a70b
Compare
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: alebedev87 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 |
📝 WalkthroughWalkthroughAdds a NetEdge documentation page, three evaluation tasks, and implements three new netedge router tools that query HAProxy data from OpenShift router pods and are registered into the netedge toolset. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as MCP Client
participant Server as MCP Server
participant K8sAPI as Kubernetes API
participant RouterPod as OpenShift Router Pod
Client->>Server: Invoke router tool (optional pod param)
alt pod provided
Server->>RouterPod: Exec command in specified pod's router container
else pod omitted
Server->>K8sAPI: List pods in openshift-ingress with deployment-ingresscontroller=default and status.phase=Running
K8sAPI-->>Server: Return pod list
Server->>Server: Select first running router pod
Server->>RouterPod: Exec command in selected pod's router container
end
RouterPod-->>Server: Command output
Server->>Server: Wrap output in fenced code block
Server-->>Client: Return tool result with formatted output
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
@alebedev87: This pull request references NE-2488 which is a valid jira issue. 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. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/openshift/NETEDGE.md`:
- Line 37: The code fences in NETEDGE.md are missing language identifiers
causing markdownlint warnings; update each opening triple-backtick fence at the
four example blocks to include the "text" language tag (i.e., change ``` to
```text) so the blocks at the mentioned example locations render and lint
cleanly.
- Around line 27-29: The note claiming "All tools have an optional `pod`
parameter" is too broad; update the text to state that the optional `pod`
parameter applies only to the router-related tools (e.g., the router command
group / functions) and not to DNS commands like get_coredns_config which have no
parameters; revise the sentence to explicitly mention "router tools" (or list
the router commands) and remove or clarify the implication that
get_coredns_config accepts a `pod` argument so readers won't expect `pod` on DNS
commands.
In `@evals/tasks/netedge/get-router-info/task.yaml`:
- Around line 5-6: The verify step asserts the prose string "HAProxy Version"
but pkg/toolsets/netedge/router.go returns raw "show info" output (fields like
"Name: HAProxy" and "Version:"), so update the task verification to match the
tool contract—replace or broaden the contains check to assert for the raw fields
such as "Name: HAProxy" and/or "Version:" (or a regex that matches
"Name:\s*HAProxy" and "Version:") so valid raw responses are accepted.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 368622e6-27f5-4fde-ac33-c34624214f7c
📒 Files selected for processing (6)
docs/openshift/NETEDGE.mdevals/tasks/netedge/get-router-config/task.yamlevals/tasks/netedge/get-router-info/task.yamlevals/tasks/netedge/get-router-sessions/task.yamlpkg/toolsets/netedge/router.gopkg/toolsets/netedge/toolset.go
Add three new MCP tools for inspecting OpenShift router pods (HAProxy): - get_router_config: retrieves the HAProxy configuration file - get_router_info: retrieves HAProxy runtime information via admin socket - get_router_sessions: retrieves all active HAProxy sessions Each tool accepts an optional pod parameter. When omitted, a Running router pod is automatically selected from the default ingress controller. Also includes evaluation tasks and documentation for the new tools.
alebedev87
force-pushed
the
router-toolset
branch
from
f62a70b to
eb2e6d7
Compare
|
@alebedev87: This pull request references NE-2488 which is a valid jira issue. 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. |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
pkg/toolsets/netedge/router.go (1)
104-129: Consider extracting common handler logic.All three handlers share an identical pattern: extract pod argument → resolve pod → exec command → format result. A helper function could reduce duplication:
func execRouterCommand(params api.ToolHandlerParams, header string, command []string) (*api.ToolCallResult, error) { pod, ok := params.GetArguments()["pod"].(string) if !ok || pod == "" { p, err := getAnyRouterPod(params, defaultIngressControllerName) if err != nil { return api.NewToolCallResult(fmt.Sprintf("# %s\nError getting router pod: %v", header, err), nil), nil } pod = p } out, err := kubernetes.NewCore(params).PodsExec(params.Context, ingressNamespace, pod, routerContainerName, command) if err != nil { return api.NewToolCallResult(fmt.Sprintf("# %s (pod: %s)\nError: %v", header, pod, err), nil), nil } return api.NewToolCallResult(fmt.Sprintf("# %s (pod: %s)\n```\n%s\n```", header, pod, out), nil), nil }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/toolsets/netedge/router.go` around lines 104 - 129, The getRouterConfig handler duplicates the common pattern of resolving a pod arg, falling back to getAnyRouterPod, running kubernetes.NewCore(...).PodsExec and formatting the output; extract that shared logic into a helper (e.g., execRouterCommand) that accepts params, a header string and command []string, calls params.GetArguments() to resolve "pod" (using getAnyRouterPod(defaultIngressControllerName) when empty), executes the command via kubernetes.NewCore(params).PodsExec(params.Context, ingressNamespace, pod, routerContainerName, command), and returns an api.ToolCallResult with either an error block or a fenced output block; then simplify getRouterConfig to call this helper with the header "Router configuration" and the command ["cat", "/var/lib/haproxy/conf/haproxy.config"].
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@pkg/toolsets/netedge/router.go`:
- Around line 104-129: The getRouterConfig handler duplicates the common pattern
of resolving a pod arg, falling back to getAnyRouterPod, running
kubernetes.NewCore(...).PodsExec and formatting the output; extract that shared
logic into a helper (e.g., execRouterCommand) that accepts params, a header
string and command []string, calls params.GetArguments() to resolve "pod" (using
getAnyRouterPod(defaultIngressControllerName) when empty), executes the command
via kubernetes.NewCore(params).PodsExec(params.Context, ingressNamespace, pod,
routerContainerName, command), and returns an api.ToolCallResult with either an
error block or a fenced output block; then simplify getRouterConfig to call this
helper with the header "Router configuration" and the command ["cat",
"/var/lib/haproxy/conf/haproxy.config"].
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 3a50b86a-a3bc-40ee-b1b7-965ea2a701c9
📒 Files selected for processing (6)
docs/openshift/NETEDGE.mdevals/tasks/netedge/get-router-config/task.yamlevals/tasks/netedge/get-router-info/task.yamlevals/tasks/netedge/get-router-sessions/task.yamlpkg/toolsets/netedge/router.gopkg/toolsets/netedge/toolset.go
✅ Files skipped from review due to trivial changes (4)
- evals/tasks/netedge/get-router-sessions/task.yaml
- evals/tasks/netedge/get-router-config/task.yaml
- docs/openshift/NETEDGE.md
- evals/tasks/netedge/get-router-info/task.yaml
🚧 Files skipped from review as they are similar to previous changes (1)
- pkg/toolsets/netedge/toolset.go
|
/retest |
|
@alebedev87: 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. |
6 participants
This PR introduces new tools for inspecting OpenShift router pods through the Kubernetes MCP Server.
Added tools:
get_router_config: View the router's configurationget_router_info: Get router runtime information and statisticsget_router_sessions: View all active sessions in the routerDocumentation: Added
NETEDGE.mdcovering the NetworkEdge toolset, including both router and CoreDNS tools.Tests: Added evaluation tasks for the added tools.
Summary by CodeRabbit
New Features
Documentation
Tests