Skip to content

Commit ac538ce

Browse files
YehongPanxuyaqist
authored andcommitted
šŸ› Bugfix: Fixed an issue where the MCP service failed to start in a Kubernetes container. (#3254)
[Specification Details] 1. Modify the pod naming logic to convert all non-compliant characters to -. 2. Modify test cases.
1 parent 392bf5b commit ac538ce

2 files changed

Lines changed: 210 additions & 8 deletions

File tree

ā€Žsdk/nexent/container/k8s_client.pyā€Ž

Lines changed: 46 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
import asyncio
99
import logging
1010
import socket
11+
import re
1112
import uuid
1213

1314
import kubernetes
@@ -23,6 +24,47 @@
2324

2425
logger = logging.getLogger("nexent.container.kubernetes")
2526

27+
# Kubernetes naming constraints: lowercase alphanumeric or dash, cannot start/end with dash,
28+
# cannot have consecutive dashes, max 253 characters
29+
K8S_NAME_PATTERN = re.compile(r"[^a-z0-9-]+")
30+
K8S_CONSECUTIVE_DASHES = re.compile(r"-+")
31+
32+
33+
def _sanitize_k8s_name(name: str) -> str:
34+
"""Convert arbitrary string to valid Kubernetes resource name.
35+
36+
Rules:
37+
- Convert to lowercase
38+
- Replace invalid characters with dash
39+
- Collapse consecutive dashes
40+
- Remove leading/trailing dashes
41+
- Must start with alphanumeric
42+
43+
Args:
44+
name: Input string to sanitize
45+
46+
Returns:
47+
Valid Kubernetes name (lowercase alphanumeric and dashes only)
48+
"""
49+
if not name:
50+
return "unknown"
51+
52+
# Lowercase and replace invalid chars with dash
53+
sanitized = K8S_NAME_PATTERN.sub("-", name.lower())
54+
55+
# Collapse consecutive dashes
56+
sanitized = K8S_CONSECUTIVE_DASHES.sub("-", sanitized)
57+
58+
# Remove leading/trailing dashes
59+
sanitized = sanitized.strip("-")
60+
61+
# Ensure it starts with alphanumeric
62+
if sanitized and not sanitized[0].isalnum():
63+
sanitized = "x" + sanitized
64+
65+
# Fallback if empty
66+
return sanitized if sanitized else "unknown"
67+
2668

2769
class ContainerError(Exception):
2870
"""Raised when container operation fails"""
@@ -77,9 +119,9 @@ def __init__(self, config: KubernetesContainerConfig):
77119

78120
def _generate_pod_name(self, service_name: str, tenant_id: str, user_id: str) -> str:
79121
"""Generate unique pod name with service, tenant, and user segments."""
80-
safe_name = "".join(c if c.isalnum() or c == "-" else "-" for c in service_name)
81-
tenant_part = (tenant_id or "")[:8]
82-
user_part = (user_id or "")[:8]
122+
safe_name = _sanitize_k8s_name(service_name)
123+
tenant_part = _sanitize_k8s_name(tenant_id)[:8]
124+
user_part = _sanitize_k8s_name(user_id)[:8]
83125
uuid_part = uuid.uuid4().hex[:8]
84126
return f"mcp-{safe_name}-{tenant_part}-{user_part}-{uuid_part}"
85127

@@ -486,7 +528,7 @@ def list_containers(
486528

487529
# Filter by service_name if provided
488530
if service_name:
489-
safe_name = "".join(c if c.isalnum() or c == "-" else "-" for c in service_name)
531+
safe_name = _sanitize_k8s_name(service_name)
490532
pod_component = labels.get(self.LABEL_COMPONENT, "")
491533
if safe_name not in pod_component:
492534
continue

ā€Žtest/sdk/container/test_k8s_client.pyā€Ž

Lines changed: 164 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
KubernetesContainerClient,
1212
ContainerError,
1313
ContainerConnectionError,
14+
_sanitize_k8s_name,
1415
)
1516
from nexent.container.k8s_config import KubernetesContainerConfig
1617

@@ -90,6 +91,79 @@ def mock_pod():
9091
return pod
9192

9293

94+
# ---------------------------------------------------------------------------
95+
# Test _sanitize_k8s_name
96+
# ---------------------------------------------------------------------------
97+
98+
99+
class TestSanitizeK8sName:
100+
"""Test _sanitize_k8s_name helper function"""
101+
102+
def test_sanitize_basic_alphanumeric(self):
103+
"""Test basic alphanumeric string passes through"""
104+
assert _sanitize_k8s_name("test-service") == "test-service"
105+
assert _sanitize_k8s_name("abc123") == "abc123"
106+
107+
def test_sanitize_lowercase_conversion(self):
108+
"""Test uppercase letters are converted to lowercase"""
109+
assert _sanitize_k8s_name("TestService") == "testservice"
110+
assert _sanitize_k8s_name("UPPERCASE") == "uppercase"
111+
112+
def test_sanitize_special_characters_replaced(self):
113+
"""Test special characters are replaced with dash"""
114+
assert _sanitize_k8s_name("test@service") == "test-service"
115+
assert _sanitize_k8s_name("foo#bar") == "foo-bar"
116+
assert _sanitize_k8s_name("test$123") == "test-123"
117+
118+
def test_sanitize_consecutive_special_chars(self):
119+
"""Test consecutive special characters are collapsed to single dash"""
120+
assert _sanitize_k8s_name("foo@@bar") == "foo-bar"
121+
assert _sanitize_k8s_name("test@#$service") == "test-service"
122+
assert _sanitize_k8s_name("a!!b") == "a-b"
123+
124+
def test_sanitize_leading_special_chars(self):
125+
"""Test leading special characters are removed"""
126+
assert _sanitize_k8s_name("@test") == "test"
127+
assert _sanitize_k8s_name("#foo") == "foo"
128+
assert _sanitize_k8s_name("!test@service") == "test-service"
129+
130+
def test_sanitize_trailing_special_chars(self):
131+
"""Test trailing special characters are removed"""
132+
assert _sanitize_k8s_name("test@") == "test"
133+
assert _sanitize_k8s_name("test-service!") == "test-service"
134+
135+
def test_sanitize_mixed_case_with_specials(self):
136+
"""Test mixed case with special characters"""
137+
assert _sanitize_k8s_name("Foo@Bar!Test") == "foo-bar-test"
138+
139+
def test_sanitize_empty_string(self):
140+
"""Test empty string returns 'unknown'"""
141+
assert _sanitize_k8s_name("") == "unknown"
142+
143+
def test_sanitize_only_special_chars(self):
144+
"""Test string with only special characters returns 'unknown'"""
145+
assert _sanitize_k8s_name("@@@") == "unknown"
146+
assert _sanitize_k8s_name("!@#") == "unknown"
147+
148+
def test_sanitize_none(self):
149+
"""Test None returns 'unknown'"""
150+
assert _sanitize_k8s_name(None) == "unknown"
151+
152+
def test_sanitize_with_dots(self):
153+
"""Test dots are converted to dashes"""
154+
assert _sanitize_k8s_name("foo.bar") == "foo-bar"
155+
assert _sanitize_k8s_name("foo...bar") == "foo-bar"
156+
157+
def test_sanitize_underscore_replaced(self):
158+
"""Test underscores are replaced with dash"""
159+
assert _sanitize_k8s_name("foo_bar") == "foo-bar"
160+
161+
def test_sanitize_spaces_replaced(self):
162+
"""Test spaces are replaced with dash"""
163+
assert _sanitize_k8s_name("foo bar") == "foo-bar"
164+
assert _sanitize_k8s_name("foo bar") == "foo-bar"
165+
166+
93167
# ---------------------------------------------------------------------------
94168
# Test KubernetesContainerClient.__init__
95169
# ---------------------------------------------------------------------------
@@ -192,6 +266,72 @@ def test_generate_pod_name_with_special_chars(self, k8s_container_client):
192266
assert "@" not in name
193267
assert "#" not in name
194268

269+
def test_generate_pod_name_consecutive_special_chars(self, k8s_container_client):
270+
"""Test pod name generation with consecutive special characters"""
271+
with patch("nexent.container.k8s_client.uuid.uuid4") as mock_uuid:
272+
mock_uuid.return_value.hex = "a1b2c3d4"
273+
name = k8s_container_client._generate_pod_name(
274+
"foo@@bar", "tenant123", "user12345")
275+
assert name == "mcp-foo-bar-tenant12-user1234-a1b2c3d4"
276+
assert "--" not in name
277+
278+
def test_generate_pod_name_leading_special_chars(self, k8s_container_client):
279+
"""Test pod name generation with leading special characters"""
280+
with patch("nexent.container.k8s_client.uuid.uuid4") as mock_uuid:
281+
mock_uuid.return_value.hex = "a1b2c3d4"
282+
name = k8s_container_client._generate_pod_name(
283+
"@test-service", "tenant123", "user12345")
284+
# "@test-service" -> "test-service" (leading @ stripped)
285+
assert name.startswith("mcp-test")
286+
assert not name.startswith("mcp-@")
287+
288+
def test_generate_pod_name_trailing_special_chars(self, k8s_container_client):
289+
"""Test pod name generation with trailing special characters"""
290+
with patch("nexent.container.k8s_client.uuid.uuid4") as mock_uuid:
291+
mock_uuid.return_value.hex = "a1b2c3d4"
292+
name = k8s_container_client._generate_pod_name(
293+
"test-service@", "tenant123", "user12345")
294+
assert name == "mcp-test-service-tenant12-user1234-a1b2c3d4"
295+
assert name.endswith("-a1b2c3d4")
296+
297+
def test_generate_pod_name_uppercase(self, k8s_container_client):
298+
"""Test pod name generation with uppercase letters"""
299+
with patch("nexent.container.k8s_client.uuid.uuid4") as mock_uuid:
300+
mock_uuid.return_value.hex = "a1b2c3d4"
301+
name = k8s_container_client._generate_pod_name(
302+
"TestService", "tenant123", "user12345")
303+
assert name == "mcp-testservice-tenant12-user1234-a1b2c3d4"
304+
305+
def test_generate_pod_name_underscores(self, k8s_container_client):
306+
"""Test pod name generation with underscores"""
307+
with patch("nexent.container.k8s_client.uuid.uuid4") as mock_uuid:
308+
mock_uuid.return_value.hex = "a1b2c3d4"
309+
name = k8s_container_client._generate_pod_name(
310+
"test_service", "tenant_123", "user_12345")
311+
# tenant_123 -> tenant-123 (9 chars), truncated to 8 -> tenant-1
312+
# user_12345 -> user-12345 (10 chars), truncated to 8 -> user-123
313+
assert name == "mcp-test-service-tenant-1-user-123-a1b2c3d4"
314+
315+
def test_generate_pod_name_dots(self, k8s_container_client):
316+
"""Test pod name generation with dots"""
317+
with patch("nexent.container.k8s_client.uuid.uuid4") as mock_uuid:
318+
mock_uuid.return_value.hex = "a1b2c3d4"
319+
name = k8s_container_client._generate_pod_name(
320+
"test.service", "tenant.123", "user.12345")
321+
# tenant.123 -> tenant.123 (9 chars), truncated to 8 -> tenant.1
322+
# user.12345 -> user.12345 (10 chars), truncated to 8 -> user.123
323+
assert name == "mcp-test-service-tenant-1-user-123-a1b2c3d4"
324+
325+
def test_generate_pod_name_spaces(self, k8s_container_client):
326+
"""Test pod name generation with spaces"""
327+
with patch("nexent.container.k8s_client.uuid.uuid4") as mock_uuid:
328+
mock_uuid.return_value.hex = "a1b2c3d4"
329+
name = k8s_container_client._generate_pod_name(
330+
"test service", "tenant 123", "user 12345")
331+
# tenant 123 -> tenant 123 (9 chars), truncated to 8 -> tenant 1
332+
# user 12345 -> user 12345 (10 chars), truncated to 8 -> user 123
333+
assert name == "mcp-test-service-tenant-1-user-123-a1b2c3d4"
334+
195335
def test_generate_pod_name_long_user_id(self, k8s_container_client):
196336
"""Test pod name generation with long user ID"""
197337
long_user_id = "a" * 20
@@ -216,31 +356,31 @@ def test_generate_pod_name_empty_tenant(self, k8s_container_client):
216356
mock_uuid.return_value.hex = "a1b2c3d4"
217357
name = k8s_container_client._generate_pod_name(
218358
"test-service", "", "user12345")
219-
assert name == "mcp-test-service--user1234-a1b2c3d4"
359+
assert name == "mcp-test-service-unknown-user1234-a1b2c3d4"
220360

221361
def test_generate_pod_name_empty_user(self, k8s_container_client):
222362
"""Test pod name generation with empty user_id"""
223363
with patch("nexent.container.k8s_client.uuid.uuid4") as mock_uuid:
224364
mock_uuid.return_value.hex = "a1b2c3d4"
225365
name = k8s_container_client._generate_pod_name(
226366
"test-service", "tenant123", "")
227-
assert name == "mcp-test-service-tenant12--a1b2c3d4"
367+
assert name == "mcp-test-service-tenant12-unknown-a1b2c3d4"
228368

229369
def test_generate_pod_name_none_tenant(self, k8s_container_client):
230370
"""Test pod name generation with None tenant_id"""
231371
with patch("nexent.container.k8s_client.uuid.uuid4") as mock_uuid:
232372
mock_uuid.return_value.hex = "a1b2c3d4"
233373
name = k8s_container_client._generate_pod_name(
234374
"test-service", None, "user12345")
235-
assert name == "mcp-test-service--user1234-a1b2c3d4"
375+
assert name == "mcp-test-service-unknown-user1234-a1b2c3d4"
236376

237377
def test_generate_pod_name_none_user(self, k8s_container_client):
238378
"""Test pod name generation with None user_id"""
239379
with patch("nexent.container.k8s_client.uuid.uuid4") as mock_uuid:
240380
mock_uuid.return_value.hex = "a1b2c3d4"
241381
name = k8s_container_client._generate_pod_name(
242382
"test-service", "tenant123", None)
243-
assert name == "mcp-test-service-tenant12--a1b2c3d4"
383+
assert name == "mcp-test-service-tenant12-unknown-a1b2c3d4"
244384

245385

246386
# ---------------------------------------------------------------------------
@@ -1265,6 +1405,26 @@ def test_list_containers_service_filter_special_chars(self, k8s_container_client
12651405

12661406
assert len(result) == 0
12671407

1408+
def test_list_containers_service_filter_consecutive_special_chars(self, k8s_container_client, mock_pod):
1409+
"""Test listing containers with service filter containing consecutive special characters"""
1410+
k8s_container_client.core_v1.list_namespaced_pod.return_value = MagicMock(items=[mock_pod])
1411+
1412+
# The sanitized version of "test@@service" is "test-service"
1413+
# Since mock_pod's component is "test-service", it should match
1414+
result = k8s_container_client.list_containers(service_name="test@@service")
1415+
1416+
assert len(result) == 1
1417+
1418+
def test_list_containers_service_filter_leading_special_chars(self, k8s_container_client, mock_pod):
1419+
"""Test listing containers with service filter containing leading special characters"""
1420+
k8s_container_client.core_v1.list_namespaced_pod.return_value = MagicMock(items=[mock_pod])
1421+
1422+
# The sanitized version of "@test-service" is "test-service" (leading @ stripped)
1423+
# Since mock_pod's component is "test-service", it should match
1424+
result = k8s_container_client.list_containers(service_name="@test-service")
1425+
1426+
assert len(result) == 1
1427+
12681428
def test_list_containers_pod_no_ports(self, k8s_container_client):
12691429
"""Test listing containers when pod has no ports configured"""
12701430
mock_pod_no_ports = MagicMock()

0 commit comments

Comments
Ā (0)