Skip to content

Commit 9cf56fb

Browse files
skhedimskhedim
committed
fix: make default MCP server timeout configurable via Helm chart
Add a new --default-mcp-server-timeout flag that controls the default timeout applied to MCP server connections.
1 parent 391c73c commit 9cf56fb

21 files changed

Lines changed: 116 additions & 51 deletions

‎go/internal/controller/reconciler/mcp_server_reconciler_test.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,7 @@ func TestReconcileKagentMCPServer_ErrorPropagation(t *testing.T) {
9999
types.NamespacedName{Namespace: "test", Name: "default-model"},
100100
nil,
101101
"",
102+
0,
102103
)
103104
reconciler := NewKagentReconciler(
104105
translator,

‎go/internal/controller/translator/agent/adk_api_translator.go‎

Lines changed: 32 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import (
1414
"slices"
1515
"strconv"
1616
"strings"
17+
"time"
1718

1819
"github.com/kagent-dev/kagent/go/api/v1alpha2"
1920
"github.com/kagent-dev/kagent/go/internal/adk"
@@ -95,20 +96,36 @@ type AdkApiTranslator interface {
9596

9697
type TranslatorPlugin = translator.TranslatorPlugin
9798

98-
func NewAdkApiTranslator(kube client.Client, defaultModelConfig types.NamespacedName, plugins []TranslatorPlugin, globalProxyURL string) AdkApiTranslator {
99+
// DefaultMCPServerTimeout is the default timeout for MCP server connections
100+
// when no explicit timeout is configured on the RemoteMCPServer resource.
101+
//
102+
// MCP servers deployed via the MCPServer CRD use a sidecar gateway that
103+
// spawns a new stdio process (e.g. via uvx/npx) for each session. Process
104+
// startup typically takes 2-8 seconds depending on package cache state,
105+
// which exceeds the default 5-second timeout used by some ADK clients
106+
// (e.g. Python ADK StreamableHTTPConnectionParams). A 30-second default
107+
// provides sufficient headroom for cold starts while remaining responsive.
108+
const DefaultMCPServerTimeout = 30 * time.Second
109+
110+
func NewAdkApiTranslator(kube client.Client, defaultModelConfig types.NamespacedName, plugins []TranslatorPlugin, globalProxyURL string, defaultMCPServerTimeout time.Duration) AdkApiTranslator {
111+
if defaultMCPServerTimeout <= 0 {
112+
defaultMCPServerTimeout = DefaultMCPServerTimeout
113+
}
99114
return &adkApiTranslator{
100-
kube: kube,
101-
defaultModelConfig: defaultModelConfig,
102-
plugins: plugins,
103-
globalProxyURL: globalProxyURL,
115+
kube: kube,
116+
defaultModelConfig: defaultModelConfig,
117+
plugins: plugins,
118+
globalProxyURL: globalProxyURL,
119+
defaultMCPServerTimeout: defaultMCPServerTimeout,
104120
}
105121
}
106122

107123
type adkApiTranslator struct {
108-
kube client.Client
109-
defaultModelConfig types.NamespacedName
110-
plugins []TranslatorPlugin
111-
globalProxyURL string
124+
kube client.Client
125+
defaultModelConfig types.NamespacedName
126+
plugins []TranslatorPlugin
127+
globalProxyURL string
128+
defaultMCPServerTimeout time.Duration
112129
}
113130

114131
const MAX_DEPTH = 10
@@ -1074,12 +1091,16 @@ func (a *adkApiTranslator) translateStreamableHttpTool(ctx context.Context, serv
10741091
}
10751092
if server.Spec.Timeout != nil {
10761093
params.Timeout = ptr.To(server.Spec.Timeout.Seconds())
1094+
} else {
1095+
params.Timeout = ptr.To(a.defaultMCPServerTimeout.Seconds())
10771096
}
10781097
if server.Spec.SseReadTimeout != nil {
10791098
params.SseReadTimeout = ptr.To(server.Spec.SseReadTimeout.Seconds())
10801099
}
10811100
if server.Spec.TerminateOnClose != nil {
10821101
params.TerminateOnClose = server.Spec.TerminateOnClose
1102+
} else {
1103+
params.TerminateOnClose = ptr.To(true)
10831104
}
10841105

10851106
return params, nil
@@ -1108,6 +1129,8 @@ func (a *adkApiTranslator) translateSseHttpTool(ctx context.Context, server *v1a
11081129
}
11091130
if server.Spec.Timeout != nil {
11101131
params.Timeout = ptr.To(server.Spec.Timeout.Seconds())
1132+
} else {
1133+
params.Timeout = ptr.To(a.defaultMCPServerTimeout.Seconds())
11111134
}
11121135
if server.Spec.SseReadTimeout != nil {
11131136
params.SseReadTimeout = ptr.To(server.Spec.SseReadTimeout.Seconds())

‎go/internal/controller/translator/agent/adk_api_translator_test.go‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -175,7 +175,7 @@ func Test_AdkApiTranslator_CrossNamespaceAgentTool(t *testing.T) {
175175
Name: "test-model",
176176
}
177177

178-
trans := translator.NewAdkApiTranslator(kubeClient, defaultModel, nil, "")
178+
trans := translator.NewAdkApiTranslator(kubeClient, defaultModel, nil, "", 0)
179179

180180
_, err := trans.TranslateAgent(context.Background(), tt.sourceAgent)
181181

@@ -338,7 +338,7 @@ func Test_AdkApiTranslator_CrossNamespaceRemoteMCPServer(t *testing.T) {
338338
Name: "test-model",
339339
}
340340

341-
trans := translator.NewAdkApiTranslator(kubeClient, defaultModel, nil, "")
341+
trans := translator.NewAdkApiTranslator(kubeClient, defaultModel, nil, "", 0)
342342

343343
_, err := trans.TranslateAgent(context.Background(), tt.agent)
344344

@@ -412,7 +412,7 @@ func Test_AdkApiTranslator_OllamaOptions(t *testing.T) {
412412
Name: modelName,
413413
}
414414

415-
trans := translator.NewAdkApiTranslator(kubeClient, defaultModel, nil, "")
415+
trans := translator.NewAdkApiTranslator(kubeClient, defaultModel, nil, "", 0)
416416

417417
outputs, err := trans.TranslateAgent(context.Background(), agent)
418418
require.NoError(t, err)
@@ -531,7 +531,7 @@ func Test_AdkApiTranslator_ServiceAccountNameOverride(t *testing.T) {
531531
Name: "test-model",
532532
}
533533

534-
trans := translator.NewAdkApiTranslator(kubeClient, defaultModel, nil, "")
534+
trans := translator.NewAdkApiTranslator(kubeClient, defaultModel, nil, "", 0)
535535

536536
outputs, err := trans.TranslateAgent(context.Background(), tt.agent)
537537
require.NoError(t, err)

‎go/internal/controller/translator/agent/adk_translator_golden_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -160,7 +160,7 @@ func runGoldenTest(t *testing.T, inputFile, outputsDir, testName string, updateG
160160

161161
// Use proxy URL from test input if provided
162162
proxyURL := testInput.ProxyURL
163-
result, err = translator.NewAdkApiTranslator(kubeClient, defaultModel, nil, proxyURL).TranslateAgent(ctx, agent)
163+
result, err = translator.NewAdkApiTranslator(kubeClient, defaultModel, nil, proxyURL, 0).TranslateAgent(ctx, agent)
164164
require.NoError(t, err)
165165

166166
default:

‎go/internal/controller/translator/agent/mcp_validation_test.go‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,7 @@ func TestMCPServerValidation_InvalidPort(t *testing.T) {
9494
types.NamespacedName{Namespace: "test", Name: "default-model"},
9595
nil,
9696
"",
97+
0,
9798
)
9899

99100
// TranslateAgent should fail with error about invalid port
@@ -179,6 +180,7 @@ func TestMCPServerValidation_ValidPort(t *testing.T) {
179180
types.NamespacedName{Namespace: "test", Name: "default-model"},
180181
nil,
181182
"",
183+
0,
182184
)
183185

184186
// TranslateAgent should succeed
@@ -249,6 +251,7 @@ func TestMCPServerValidation_NotFound(t *testing.T) {
249251
types.NamespacedName{Namespace: "test", Name: "default-model"},
250252
nil,
251253
"",
254+
0,
252255
)
253256

254257
// TranslateAgent should fail with not found error
@@ -310,6 +313,7 @@ func TestMCPServerValidation_NoMCPServerReference(t *testing.T) {
310313
types.NamespacedName{Namespace: "test", Name: "default-model"},
311314
nil,
312315
"",
316+
0,
313317
)
314318

315319
// TranslateAgent should fail with provider or tool server error
@@ -388,6 +392,7 @@ func TestMCPServerValidation_RemoteMCPServer(t *testing.T) {
388392
types.NamespacedName{Namespace: "test", Name: "default-model"},
389393
nil,
390394
"",
395+
0,
391396
)
392397

393398
// TranslateAgent should succeed - RemoteMCPServer doesn't have port validation
@@ -546,6 +551,7 @@ func TestMCPServerValidation_MultipleTools(t *testing.T) {
546551
types.NamespacedName{Namespace: "test", Name: "default-model"},
547552
nil,
548553
"",
554+
0,
549555
)
550556

551557
// TranslateAgent should fail because one of the MCPServers is invalid

‎go/internal/controller/translator/agent/proxy_test.go‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,7 @@ func TestProxyConfiguration_ThroughTranslateAgent(t *testing.T) {
116116
types.NamespacedName{Name: "default-model", Namespace: "test"},
117117
nil,
118118
"http://proxy.kagent.svc.cluster.local:8080",
119+
0,
119120
)
120121

121122
result, err := translator.TranslateAgent(ctx, agent)
@@ -145,6 +146,7 @@ func TestProxyConfiguration_ThroughTranslateAgent(t *testing.T) {
145146
types.NamespacedName{Name: "default-model", Namespace: "test"},
146147
nil,
147148
"", // No proxy
149+
0,
148150
)
149151

150152
result, err := translator.TranslateAgent(ctx, agent)
@@ -246,6 +248,7 @@ func TestProxyConfiguration_RemoteMCPServer_ExternalURL(t *testing.T) {
246248
types.NamespacedName{Name: "default-model", Namespace: "test"},
247249
nil,
248250
"http://proxy.kagent.svc.cluster.local:8080",
251+
0,
249252
)
250253

251254
result, err := translator.TranslateAgent(ctx, agent)
@@ -338,6 +341,7 @@ func TestProxyConfiguration_MCPServer(t *testing.T) {
338341
types.NamespacedName{Name: "default-model", Namespace: "test"},
339342
nil,
340343
"http://proxy.kagent.svc.cluster.local:8080",
344+
0,
341345
)
342346

343347
result, err := translator.TranslateAgent(ctx, agent)
@@ -435,6 +439,7 @@ func TestProxyConfiguration_Service(t *testing.T) {
435439
types.NamespacedName{Name: "default-model", Namespace: "test"},
436440
nil,
437441
"http://proxy.kagent.svc.cluster.local:8080",
442+
0,
438443
)
439444

440445
result, err := translator.TranslateAgent(ctx, agent)

‎go/internal/controller/translator/agent/security_context_test.go‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ func TestSecurityContext_AppliedToPodSpec(t *testing.T) {
8484
Namespace: "test",
8585
Name: "test-model",
8686
}
87-
translatorInstance := translator.NewAdkApiTranslator(kubeClient, defaultModel, nil, "")
87+
translatorInstance := translator.NewAdkApiTranslator(kubeClient, defaultModel, nil, "", 0)
8888

8989
// Translate agent
9090
result, err := translatorInstance.TranslateAgent(ctx, agent)
@@ -175,7 +175,7 @@ func TestSecurityContext_OnlyPodSecurityContext(t *testing.T) {
175175
Namespace: "test",
176176
Name: "test-model",
177177
}
178-
translatorInstance := translator.NewAdkApiTranslator(kubeClient, defaultModel, nil, "")
178+
translatorInstance := translator.NewAdkApiTranslator(kubeClient, defaultModel, nil, "", 0)
179179

180180
result, err := translatorInstance.TranslateAgent(ctx, agent)
181181
require.NoError(t, err)
@@ -250,7 +250,7 @@ func TestSecurityContext_OnlyContainerSecurityContext(t *testing.T) {
250250
Namespace: "test",
251251
Name: "test-model",
252252
}
253-
translatorInstance := translator.NewAdkApiTranslator(kubeClient, defaultModel, nil, "")
253+
translatorInstance := translator.NewAdkApiTranslator(kubeClient, defaultModel, nil, "", 0)
254254

255255
result, err := translatorInstance.TranslateAgent(ctx, agent)
256256
require.NoError(t, err)
@@ -328,7 +328,7 @@ func TestSecurityContext_WithSandbox(t *testing.T) {
328328
Namespace: "test",
329329
Name: "test-model",
330330
}
331-
translatorInstance := translator.NewAdkApiTranslator(kubeClient, defaultModel, nil, "")
331+
translatorInstance := translator.NewAdkApiTranslator(kubeClient, defaultModel, nil, "", 0)
332332

333333
result, err := translatorInstance.TranslateAgent(ctx, agent)
334334
require.NoError(t, err)

‎go/internal/controller/translator/agent/testdata/outputs/agent_with_allowed_headers.json‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,8 @@
2727
],
2828
"params": {
2929
"headers": {},
30+
"terminate_on_close": true,
31+
"timeout": 30,
3032
"url": "http://mcp-server.test:8080/mcp"
3133
},
3234
"tools": [
@@ -72,7 +74,7 @@
7274
},
7375
"stringData": {
7476
"agent-card.json": "{\"name\":\"agent\",\"description\":\"\",\"url\":\"http://agent.test:8080\",\"version\":\"\",\"capabilities\":{\"streaming\":true,\"pushNotifications\":false,\"stateTransitionHistory\":true},\"defaultInputModes\":[\"text\"],\"defaultOutputModes\":[\"text\"],\"skills\":[]}",
75-
"config.json": "{\"model\":{\"type\":\"openai\",\"model\":\"gpt-4o\",\"base_url\":\"\"},\"description\":\"\",\"instruction\":\"You are a helpful assistant.\",\"http_tools\":[{\"params\":{\"url\":\"http://mcp-server.test:8080/mcp\",\"headers\":{}},\"tools\":[\"tool1\",\"tool2\"],\"allowed_headers\":[\"x-user-email\",\"x-tenant-id\"]}],\"sse_tools\":null,\"remote_agents\":null,\"stream\":false}"
77+
"config.json": "{\"model\":{\"type\":\"openai\",\"model\":\"gpt-4o\",\"base_url\":\"\"},\"description\":\"\",\"instruction\":\"You are a helpful assistant.\",\"http_tools\":[{\"params\":{\"url\":\"http://mcp-server.test:8080/mcp\",\"headers\":{},\"timeout\":30,\"terminate_on_close\":true},\"tools\":[\"tool1\",\"tool2\"],\"allowed_headers\":[\"x-user-email\",\"x-tenant-id\"]}],\"sse_tools\":null,\"remote_agents\":null,\"stream\":false}"
7678
}
7779
},
7880
{
@@ -141,7 +143,7 @@
141143
"template": {
142144
"metadata": {
143145
"annotations": {
144-
"kagent.dev/config-hash": "10932872605308481917"
146+
"kagent.dev/config-hash": "15838967152682650808"
145147
},
146148
"labels": {
147149
"app": "kagent",

‎go/internal/controller/translator/agent/testdata/outputs/agent_with_cross_namespace_tools.json‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
"headers": {
2626
"Authorization": "tool-secret-token"
2727
},
28+
"terminate_on_close": true,
2829
"timeout": 30,
2930
"url": "http://tools.tools-ns.svc:8080/mcp"
3031
},
@@ -77,7 +78,7 @@
7778
},
7879
"stringData": {
7980
"agent-card.json": "{\"name\":\"source_agent\",\"description\":\"An agent that uses cross-namespace tools\",\"url\":\"http://source-agent.source-ns:8080\",\"version\":\"\",\"capabilities\":{\"streaming\":true,\"pushNotifications\":false,\"stateTransitionHistory\":true},\"defaultInputModes\":[\"text\"],\"defaultOutputModes\":[\"text\"],\"skills\":[]}",
80-
"config.json": "{\"model\":{\"type\":\"openai\",\"model\":\"gpt-4o\",\"base_url\":\"\"},\"description\":\"An agent that uses cross-namespace tools\",\"instruction\":\"You are an assistant with access to shared tools.\",\"http_tools\":[{\"params\":{\"url\":\"http://tools.tools-ns.svc:8080/mcp\",\"headers\":{\"Authorization\":\"tool-secret-token\"},\"timeout\":30},\"tools\":[\"list_resources\",\"get_resource\"]}],\"sse_tools\":null,\"remote_agents\":[{\"name\":\"tools_ns__NS__tools_agent\",\"url\":\"http://tools-agent.tools-ns:8080\",\"description\":\"An agent that can be used as a cross-namespace tool\"}],\"stream\":false}"
81+
"config.json": "{\"model\":{\"type\":\"openai\",\"model\":\"gpt-4o\",\"base_url\":\"\"},\"description\":\"An agent that uses cross-namespace tools\",\"instruction\":\"You are an assistant with access to shared tools.\",\"http_tools\":[{\"params\":{\"url\":\"http://tools.tools-ns.svc:8080/mcp\",\"headers\":{\"Authorization\":\"tool-secret-token\"},\"timeout\":30,\"terminate_on_close\":true},\"tools\":[\"list_resources\",\"get_resource\"]}],\"sse_tools\":null,\"remote_agents\":[{\"name\":\"tools_ns__NS__tools_agent\",\"url\":\"http://tools-agent.tools-ns:8080\",\"description\":\"An agent that can be used as a cross-namespace tool\"}],\"stream\":false}"
8182
}
8283
},
8384
{
@@ -146,7 +147,7 @@
146147
"template": {
147148
"metadata": {
148149
"annotations": {
149-
"kagent.dev/config-hash": "5519576411735538994"
150+
"kagent.dev/config-hash": "15656908531094268482"
150151
},
151152
"labels": {
152153
"app": "kagent",

‎go/internal/controller/translator/agent/testdata/outputs/agent_with_http_toolserver.json‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
"MATH": "sk-test-api-key"
2727
},
2828
"sse_read_timeout": 300,
29+
"terminate_on_close": true,
2930
"timeout": 30,
3031
"url": "http://localhost:8084/mcp"
3132
},
@@ -71,7 +72,7 @@
7172
},
7273
"stringData": {
7374
"agent-card.json": "{\"name\":\"agent\",\"description\":\"\",\"url\":\"http://agent.test:8080\",\"version\":\"\",\"capabilities\":{\"streaming\":true,\"pushNotifications\":false,\"stateTransitionHistory\":true},\"defaultInputModes\":[\"text\"],\"defaultOutputModes\":[\"text\"],\"skills\":[]}",
74-
"config.json": "{\"model\":{\"type\":\"openai\",\"model\":\"gpt-4o\",\"base_url\":\"\"},\"description\":\"\",\"instruction\":\"You are a math toolserver. Focus on solving mathematical problems step by step.\",\"http_tools\":[{\"params\":{\"url\":\"http://localhost:8084/mcp\",\"headers\":{\"MATH\":\"sk-test-api-key\"},\"timeout\":30,\"sse_read_timeout\":300},\"tools\":[\"k8s_get_resources\"]}],\"sse_tools\":null,\"remote_agents\":null,\"stream\":false}"
75+
"config.json": "{\"model\":{\"type\":\"openai\",\"model\":\"gpt-4o\",\"base_url\":\"\"},\"description\":\"\",\"instruction\":\"You are a math toolserver. Focus on solving mathematical problems step by step.\",\"http_tools\":[{\"params\":{\"url\":\"http://localhost:8084/mcp\",\"headers\":{\"MATH\":\"sk-test-api-key\"},\"timeout\":30,\"sse_read_timeout\":300,\"terminate_on_close\":true},\"tools\":[\"k8s_get_resources\"]}],\"sse_tools\":null,\"remote_agents\":null,\"stream\":false}"
7576
}
7677
},
7778
{
@@ -140,7 +141,7 @@
140141
"template": {
141142
"metadata": {
142143
"annotations": {
143-
"kagent.dev/config-hash": "11246224890029939632"
144+
"kagent.dev/config-hash": "3112593142832707551"
144145
},
145146
"labels": {
146147
"app": "kagent",

0 commit comments

Comments
 (0)