Skip to content

Commit 46824fb

Browse files
ngopalak-redhatngopalak-redhat
committed
Support for multiple log files in the Node logs query
Signed-off-by: Neeraj Krishna Gopalakrishna <[email protected]>
1 parent 44053f1 commit 46824fb

File tree

2 files changed

+260
-10
lines changed

2 files changed

+260
-10
lines changed

‎pkg/mcp/nodes_test.go‎

Lines changed: 180 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -199,6 +199,186 @@ func (s *NodesSuite) TestNodesLog() {
199199
}
200200
}
201201

202+
func (s *NodesSuite) TestNodesLogWithArrayQuery() {
203+
s.mockServer.Handle(http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
204+
// Get Node response
205+
if req.URL.Path == "/api/v1/nodes/existing-node" {
206+
w.Header().Set("Content-Type", "application/json")
207+
w.WriteHeader(http.StatusOK)
208+
_, _ = w.Write([]byte(`{
209+
"apiVersion": "v1",
210+
"kind": "Node",
211+
"metadata": {
212+
"name": "existing-node"
213+
}
214+
}`))
215+
return
216+
}
217+
// Get Proxy Logs
218+
if req.URL.Path == "/api/v1/nodes/existing-node/proxy/logs" {
219+
w.Header().Set("Content-Type", "text/plain")
220+
query := req.URL.Query().Get("query")
221+
var logContent string
222+
switch query {
223+
case "/kubelet.log":
224+
logContent = "Kubelet log line 1\nKubelet log line 2\n"
225+
case "/kube-proxy.log":
226+
logContent = "Kube-proxy log line 1\nKube-proxy log line 2\n"
227+
case "/empty.log":
228+
logContent = ""
229+
default:
230+
w.WriteHeader(http.StatusNotFound)
231+
return
232+
}
233+
w.WriteHeader(http.StatusOK)
234+
_, _ = w.Write([]byte(logContent))
235+
return
236+
}
237+
w.WriteHeader(http.StatusNotFound)
238+
}))
239+
s.InitMcpClient()
240+
241+
s.Run("nodes_log(name=existing-node, query=[/kubelet.log, /kube-proxy.log])", func() {
242+
toolResult, err := s.CallTool("nodes_log", map[string]interface{}{
243+
"name": "existing-node",
244+
"query": []interface{}{"/kubelet.log", "/kube-proxy.log"},
245+
})
246+
s.Require().NotNil(toolResult, "toolResult should not be nil")
247+
s.Run("no error", func() {
248+
s.Falsef(toolResult.IsError, "call tool should succeed")
249+
s.Nilf(err, "call tool should not return error object")
250+
})
251+
s.Run("returns logs from both files with separators", func() {
252+
result := toolResult.Content[0].(mcp.TextContent).Text
253+
s.Contains(result, "=== Logs for query: /kubelet.log ===", "should contain kubelet log header")
254+
s.Contains(result, "Kubelet log line 1\nKubelet log line 2\n", "should contain kubelet log content")
255+
s.Contains(result, "=== Logs for query: /kube-proxy.log ===", "should contain kube-proxy log header")
256+
s.Contains(result, "Kube-proxy log line 1\nKube-proxy log line 2\n", "should contain kube-proxy log content")
257+
})
258+
})
259+
260+
s.Run("nodes_log(name=existing-node, query=[])", func() {
261+
toolResult, err := s.CallTool("nodes_log", map[string]interface{}{
262+
"name": "existing-node",
263+
"query": []interface{}{},
264+
})
265+
s.Require().NotNil(toolResult, "toolResult should not be nil")
266+
s.Run("has error", func() {
267+
s.Truef(toolResult.IsError, "call tool should fail")
268+
s.Nilf(err, "call tool should not return error object")
269+
})
270+
s.Run("describes empty array", func() {
271+
expectedMessage := "failed to get node log, query array cannot be empty"
272+
s.Equalf(expectedMessage, toolResult.Content[0].(mcp.TextContent).Text,
273+
"expected descriptive error '%s', got %v", expectedMessage, toolResult.Content[0].(mcp.TextContent).Text)
274+
})
275+
})
276+
277+
s.Run("nodes_log(name=existing-node, query=[''])", func() {
278+
toolResult, err := s.CallTool("nodes_log", map[string]interface{}{
279+
"name": "existing-node",
280+
"query": []interface{}{""},
281+
})
282+
s.Require().NotNil(toolResult, "toolResult should not be nil")
283+
s.Run("has error", func() {
284+
s.Truef(toolResult.IsError, "call tool should fail")
285+
s.Nilf(err, "call tool should not return error object")
286+
})
287+
s.Run("describes empty string in array", func() {
288+
expectedMessage := "failed to get node log, query array element 0 cannot be empty"
289+
s.Equalf(expectedMessage, toolResult.Content[0].(mcp.TextContent).Text,
290+
"expected descriptive error '%s', got %v", expectedMessage, toolResult.Content[0].(mcp.TextContent).Text)
291+
})
292+
})
293+
294+
s.Run("nodes_log(name=existing-node, query=[123])", func() {
295+
toolResult, err := s.CallTool("nodes_log", map[string]interface{}{
296+
"name": "existing-node",
297+
"query": []interface{}{123},
298+
})
299+
s.Require().NotNil(toolResult, "toolResult should not be nil")
300+
s.Run("has error", func() {
301+
s.Truef(toolResult.IsError, "call tool should fail")
302+
s.Nilf(err, "call tool should not return error object")
303+
})
304+
s.Run("describes non-string element in array", func() {
305+
expectedMessage := "failed to get node log, query array element 0 is not a string"
306+
s.Equalf(expectedMessage, toolResult.Content[0].(mcp.TextContent).Text,
307+
"expected descriptive error '%s', got %v", expectedMessage, toolResult.Content[0].(mcp.TextContent).Text)
308+
})
309+
})
310+
311+
s.Run("nodes_log(name=existing-node, query=[/kubelet.log, /empty.log])", func() {
312+
toolResult, err := s.CallTool("nodes_log", map[string]interface{}{
313+
"name": "existing-node",
314+
"query": []interface{}{"/kubelet.log", "/empty.log"},
315+
})
316+
s.Require().NotNil(toolResult, "toolResult should not be nil")
317+
s.Run("no error", func() {
318+
s.Falsef(toolResult.IsError, "call tool should succeed")
319+
s.Nilf(err, "call tool should not return error object")
320+
})
321+
s.Run("handles empty log in array", func() {
322+
result := toolResult.Content[0].(mcp.TextContent).Text
323+
s.Contains(result, "=== Logs for query: /kubelet.log ===", "should contain kubelet log header")
324+
s.Contains(result, "Kubelet log line 1\nKubelet log line 2\n", "should contain kubelet log content")
325+
s.Contains(result, "=== Logs for query: /empty.log ===", "should contain empty log header")
326+
s.Contains(result, "The node existing-node has not logged any message yet for query '/empty.log' or the log file is empty", "should contain empty log message")
327+
})
328+
})
329+
330+
s.Run("nodes_log(name=existing-node, query=[/missing.log])", func() {
331+
toolResult, err := s.CallTool("nodes_log", map[string]interface{}{
332+
"name": "existing-node",
333+
"query": []interface{}{"/missing.log"},
334+
})
335+
s.Require().NotNil(toolResult, "toolResult should not be nil")
336+
s.Run("has error", func() {
337+
s.Truef(toolResult.IsError, "call tool should fail")
338+
s.Nilf(err, "call tool should not return error object")
339+
})
340+
s.Run("describes missing log file", func() {
341+
expectedMessage := "failed to get node log for existing-node: failed to get node logs: the server could not find the requested resource"
342+
s.Equalf(expectedMessage, toolResult.Content[0].(mcp.TextContent).Text,
343+
"expected descriptive error '%s', got %v", expectedMessage, toolResult.Content[0].(mcp.TextContent).Text)
344+
})
345+
})
346+
347+
s.Run("nodes_log(name=existing-node, query=[/missing1.log, /missing2.log])", func() {
348+
toolResult, err := s.CallTool("nodes_log", map[string]interface{}{
349+
"name": "existing-node",
350+
"query": []interface{}{"/missing1.log", "/missing2.log"},
351+
})
352+
s.Require().NotNil(toolResult, "toolResult should not be nil")
353+
s.Run("has error", func() {
354+
s.Truef(toolResult.IsError, "call tool should fail")
355+
s.Nilf(err, "call tool should not return error object")
356+
})
357+
s.Run("describes missing log file with query when multiple queries", func() {
358+
expectedMessage := "failed to get node log for existing-node (query: /missing1.log): failed to get node logs: the server could not find the requested resource"
359+
s.Equalf(expectedMessage, toolResult.Content[0].(mcp.TextContent).Text,
360+
"expected descriptive error '%s', got %v", expectedMessage, toolResult.Content[0].(mcp.TextContent).Text)
361+
})
362+
})
363+
364+
s.Run("nodes_log(name=existing-node, query='')", func() {
365+
toolResult, err := s.CallTool("nodes_log", map[string]interface{}{
366+
"name": "existing-node",
367+
"query": "",
368+
})
369+
s.Require().NotNil(toolResult, "toolResult should not be nil")
370+
s.Run("has error", func() {
371+
s.Truef(toolResult.IsError, "call tool should fail")
372+
s.Nilf(err, "call tool should not return error object")
373+
})
374+
s.Run("describes empty string query", func() {
375+
expectedMessage := "failed to get node log, query cannot be empty"
376+
s.Equalf(expectedMessage, toolResult.Content[0].(mcp.TextContent).Text,
377+
"expected descriptive error '%s', got %v", expectedMessage, toolResult.Content[0].(mcp.TextContent).Text)
378+
})
379+
})
380+
}
381+
202382
func (s *NodesSuite) TestNodesLogDenied() {
203383
s.Require().NoError(toml.Unmarshal([]byte(`
204384
denied_resources = [ { version = "v1", kind = "Node" } ]

‎pkg/toolsets/core/nodes.go‎

Lines changed: 80 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,20 @@ func initNodes() []api.ServerTool {
2323
Description: "Name of the node to get logs from",
2424
},
2525
"query": {
26-
Type: "string",
27-
Description: `query specifies services(s) or files from which to return logs (required). Example: "kubelet" to fetch kubelet logs, "/<log-file-name>" to fetch a specific log file from the node (e.g., "/var/log/kubelet.log" or "/var/log/kube-proxy.log")`,
26+
OneOf: []*jsonschema.Schema{
27+
{
28+
Type: "string",
29+
Description: `Single query specifying a service or file from which to return logs. Example: "kubelet" or "kubelet.log"`,
30+
},
31+
{
32+
Type: "array",
33+
Description: `Array of queries specifying multiple services or files from which to return logs`,
34+
Items: &jsonschema.Schema{
35+
Type: "string",
36+
},
37+
},
38+
},
39+
Description: `query specifies service(s) or files from which to return logs (required). Can be a single string or array of strings. Example: "kubelet" to fetch kubelet logs, "/<log-file-name>" to fetch a specific log file from the node (e.g., "kubelet.log" or "kube-proxy.log"), or ["kubelet", "kube-proxy.log"] for multiple sources`,
2840
},
2941
"tailLines": {
3042
Type: "integer",
@@ -51,10 +63,38 @@ func nodesLog(params api.ToolHandlerParams) (*api.ToolCallResult, error) {
5163
if !ok || name == "" {
5264
return api.NewToolCallResult("", errors.New("failed to get node log, missing argument name")), nil
5365
}
54-
query, ok := params.GetArguments()["query"].(string)
55-
if !ok || query == "" {
66+
67+
// Handle query parameter - can be string or array of strings
68+
var queries []string
69+
queryArg := params.GetArguments()["query"]
70+
if queryArg == nil {
5671
return api.NewToolCallResult("", errors.New("failed to get node log, missing argument query")), nil
5772
}
73+
74+
switch v := queryArg.(type) {
75+
case string:
76+
if v == "" {
77+
return api.NewToolCallResult("", errors.New("failed to get node log, query cannot be empty")), nil
78+
}
79+
queries = []string{v}
80+
case []interface{}:
81+
if len(v) == 0 {
82+
return api.NewToolCallResult("", errors.New("failed to get node log, query array cannot be empty")), nil
83+
}
84+
for i, item := range v {
85+
str, ok := item.(string)
86+
if !ok {
87+
return api.NewToolCallResult("", fmt.Errorf("failed to get node log, query array element %d is not a string", i)), nil
88+
}
89+
if str == "" {
90+
return api.NewToolCallResult("", fmt.Errorf("failed to get node log, query array element %d cannot be empty", i)), nil
91+
}
92+
queries = append(queries, str)
93+
}
94+
default:
95+
return api.NewToolCallResult("", fmt.Errorf("failed to get node log, query must be a string or array of strings, got %T", queryArg)), nil
96+
}
97+
5898
tailLines := params.GetArguments()["tailLines"]
5999
var tailInt int64
60100
if tailLines != nil {
@@ -69,11 +109,41 @@ func nodesLog(params api.ToolHandlerParams) (*api.ToolCallResult, error) {
69109
return api.NewToolCallResult("", fmt.Errorf("failed to parse tail parameter: expected integer, got %T", tailLines)), nil
70110
}
71111
}
72-
ret, err := params.NodesLog(params, name, query, tailInt)
73-
if err != nil {
74-
return api.NewToolCallResult("", fmt.Errorf("failed to get node log for %s: %v", name, err)), nil
75-
} else if ret == "" {
76-
ret = fmt.Sprintf("The node %s has not logged any message yet or the log file is empty", name)
112+
113+
// Fetch logs for each query and concatenate results
114+
var results string
115+
for i, query := range queries {
116+
ret, err := params.NodesLog(params, name, query, tailInt)
117+
if err != nil {
118+
// Only include query in error message if there are multiple queries
119+
if len(queries) > 1 {
120+
return api.NewToolCallResult("", fmt.Errorf("failed to get node log for %s (query: %s): %v", name, query, err)), nil
121+
}
122+
return api.NewToolCallResult("", fmt.Errorf("failed to get node log for %s: %v", name, err)), nil
123+
}
124+
125+
if len(queries) > 1 {
126+
// Add separator between multiple queries
127+
if i > 0 {
128+
results += "\n\n"
129+
}
130+
results += fmt.Sprintf("=== Logs for query: %s ===\n", query)
131+
}
132+
133+
if ret == "" {
134+
if len(queries) > 1 {
135+
results += fmt.Sprintf("The node %s has not logged any message yet for query '%s' or the log file is empty\n", name, query)
136+
} else {
137+
results = fmt.Sprintf("The node %s has not logged any message yet or the log file is empty", name)
138+
}
139+
} else {
140+
results += ret
141+
}
142+
}
143+
144+
if results == "" {
145+
results = fmt.Sprintf("The node %s has not logged any message yet or the log file is empty", name)
77146
}
78-
return api.NewToolCallResult(ret, nil), nil
147+
148+
return api.NewToolCallResult(results, nil), nil
79149
}

0 commit comments

Comments
 (0)