Skip to content

Commit b432b06

Browse files
GujiasshPangjiping
authored andcommitted
fix(k8s): propagate pvc readonly flag
1 parent 34653f7 commit b432b06

2 files changed

Lines changed: 71 additions & 28 deletions

File tree

server/opensandbox_server/services/k8s/volume_helper.py

Lines changed: 18 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -54,12 +54,15 @@ def apply_volumes_to_pod_spec(
5454
pvc_claim_name = vol.pvc.claim_name
5555

5656
if pvc_claim_name not in pvc_to_volume_name:
57-
pod_volumes.append({
58-
"name": vol_name,
59-
"persistentVolumeClaim": {
60-
"claimName": pvc_claim_name,
61-
},
62-
})
57+
pod_volumes.append(
58+
{
59+
"name": vol_name,
60+
"persistentVolumeClaim": {
61+
"claimName": pvc_claim_name,
62+
"readOnly": vol.read_only,
63+
},
64+
}
65+
)
6366
pvc_to_volume_name[pvc_claim_name] = vol_name
6467
existing_volume_names.add(vol_name)
6568

@@ -78,13 +81,15 @@ def apply_volumes_to_pod_spec(
7881
elif vol.host is not None:
7982
host_path = vol.host.path
8083

81-
pod_volumes.append({
82-
"name": vol_name,
83-
"hostPath": {
84-
"path": host_path,
85-
"type": "DirectoryOrCreate",
86-
},
87-
})
84+
pod_volumes.append(
85+
{
86+
"name": vol_name,
87+
"hostPath": {
88+
"path": host_path,
89+
"type": "DirectoryOrCreate",
90+
},
91+
}
92+
)
8893

8994
mount = {
9095
"name": vol_name,

server/tests/k8s/test_batchsandbox_provider.py

Lines changed: 53 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -140,9 +140,14 @@ def test_create_workload_builds_correct_manifest(self, mock_k8s_client):
140140
expires_at=expires_at,
141141
execd_image="execd:latest",
142142
)
143-
144-
assert result == {"name": "test-id", "uid": "test-uid", "apiVersion": "sandbox.opensandbox.io/v1alpha1", "kind": "BatchSandbox"}
145-
143+
144+
assert result == {
145+
"name": "test-id",
146+
"uid": "test-uid",
147+
"apiVersion": "sandbox.opensandbox.io/v1alpha1",
148+
"kind": "BatchSandbox",
149+
}
150+
146151
# Verify API call
147152
call_args = mock_k8s_client.create_custom_object.call_args
148153
body = call_args.kwargs["body"]
@@ -869,8 +874,13 @@ def test_create_workload_updates_informer_cache(self, mock_k8s_client):
869874
execd_image="execd:latest",
870875
)
871876

872-
assert result == {"name": "test-id", "uid": "test-uid", "apiVersion": "sandbox.opensandbox.io/v1alpha1", "kind": "BatchSandbox"}
873-
877+
assert result == {
878+
"name": "test-id",
879+
"uid": "test-uid",
880+
"apiVersion": "sandbox.opensandbox.io/v1alpha1",
881+
"kind": "BatchSandbox",
882+
}
883+
874884
# ===== Workload List Tests =====
875885

876886
def test_list_workloads_returns_items(self, mock_k8s_client, mock_batchsandbox_list_response):
@@ -1324,8 +1334,13 @@ def test_create_workload_poolref_ignores_image_spec(self, mock_k8s_client):
13241334
)
13251335

13261336
# Should succeed and return workload info
1327-
assert result == {"name": "sandbox-test-id", "uid": "test-uid", "apiVersion": "sandbox.opensandbox.io/v1alpha1", "kind": "BatchSandbox"}
1328-
1337+
assert result == {
1338+
"name": "sandbox-test-id",
1339+
"uid": "test-uid",
1340+
"apiVersion": "sandbox.opensandbox.io/v1alpha1",
1341+
"kind": "BatchSandbox",
1342+
}
1343+
13291344
# Verify poolRef is used
13301345
body = mock_k8s_client.create_custom_object.call_args.kwargs["body"]
13311346
assert body["spec"]["poolRef"] == "my-pool"
@@ -1356,8 +1371,13 @@ def test_create_workload_poolref_ignores_resource_limits(self, mock_k8s_client):
13561371
)
13571372

13581373
# Should succeed and return workload info
1359-
assert result == {"name": "sandbox-test-id", "uid": "test-uid", "apiVersion": "sandbox.opensandbox.io/v1alpha1", "kind": "BatchSandbox"}
1360-
1374+
assert result == {
1375+
"name": "sandbox-test-id",
1376+
"uid": "test-uid",
1377+
"apiVersion": "sandbox.opensandbox.io/v1alpha1",
1378+
"kind": "BatchSandbox",
1379+
}
1380+
13611381
# Verify poolRef is used
13621382
body = mock_k8s_client.create_custom_object.call_args.kwargs["body"]
13631383
assert body["spec"]["poolRef"] == "my-pool"
@@ -1385,9 +1405,14 @@ def test_create_workload_poolref_allows_entrypoint_and_env(self, mock_k8s_client
13851405
execd_image="execd:latest",
13861406
extensions={"poolRef": "my-pool"},
13871407
)
1388-
1389-
assert result == {"name": "sandbox-test-id", "uid": "test-uid", "apiVersion": "sandbox.opensandbox.io/v1alpha1", "kind": "BatchSandbox"}
1390-
1408+
1409+
assert result == {
1410+
"name": "sandbox-test-id",
1411+
"uid": "test-uid",
1412+
"apiVersion": "sandbox.opensandbox.io/v1alpha1",
1413+
"kind": "BatchSandbox",
1414+
}
1415+
13911416
# Verify the call
13921417
body = mock_k8s_client.create_custom_object.call_args.kwargs["body"]
13931418
assert body["spec"]["poolRef"] == "my-pool"
@@ -1620,7 +1645,9 @@ def test_create_workload_poolref_default_entrypoint_with_env_includes_task_templ
16201645
task_template = body["spec"]["taskTemplate"]
16211646
assert task_template["spec"]["process"]["env"] == [{"name": "VERSION", "value": "11"}]
16221647

1623-
def test_create_workload_poolref_none_entrypoint_no_env_omits_task_template(self, mock_k8s_client):
1648+
def test_create_workload_poolref_none_entrypoint_no_env_omits_task_template(
1649+
self, mock_k8s_client
1650+
):
16241651
"""When entrypoint is None and env is empty, taskTemplate is omitted.
16251652
16261653
SDK pool mode callers omit entrypoint entirely (None), expecting the pool's
@@ -2637,7 +2664,12 @@ def test_create_workload_with_pvc_volume(self, mock_k8s_client):
26372664
volumes=volumes,
26382665
)
26392666

2640-
assert result == {"name": "test-id", "uid": "test-uid", "apiVersion": "sandbox.opensandbox.io/v1alpha1", "kind": "BatchSandbox"}
2667+
assert result == {
2668+
"name": "test-id",
2669+
"uid": "test-uid",
2670+
"apiVersion": "sandbox.opensandbox.io/v1alpha1",
2671+
"kind": "BatchSandbox",
2672+
}
26412673

26422674
def test_create_workload_poolref_rejects_platform(self, mock_k8s_client):
26432675
provider = BatchSandboxProvider(mock_k8s_client)
@@ -2698,8 +2730,11 @@ def test_create_workload_with_pvc_volume_readonly(self, mock_k8s_client):
26982730
main_container = pod_spec["containers"][0]
26992731
mounts = main_container.get("volumeMounts", [])
27002732
models_mount = next((m for m in mounts if m["name"] == "models-volume"), None)
2733+
models_volume = next((v for v in pod_spec["volumes"] if v["name"] == "models-volume"), None)
27012734
assert models_mount is not None
27022735
assert models_mount["readOnly"] is True
2736+
assert models_volume is not None
2737+
assert models_volume["persistentVolumeClaim"]["readOnly"] is True
27032738

27042739
def test_create_workload_with_pvc_volume_subpath(self, mock_k8s_client):
27052740
"""
@@ -2961,7 +2996,10 @@ def test_apply_volumes_to_pod_spec_same_pvc_multiple_mounts(self, mock_k8s_clien
29612996
# One volume definition for the shared PVC (first Volume name used)
29622997
assert len(pod_spec["volumes"]) == 1
29632998
assert pod_spec["volumes"][0]["name"] == "skills"
2964-
assert pod_spec["volumes"][0]["persistentVolumeClaim"]["claimName"] == "oss-pvc-r"
2999+
shared_volume = next((v for v in pod_spec["volumes"] if v["name"] == "skills"), None)
3000+
assert shared_volume is not None
3001+
assert shared_volume["persistentVolumeClaim"]["claimName"] == "oss-pvc-r"
3002+
assert shared_volume["persistentVolumeClaim"]["readOnly"] is True
29653003

29663004
# Two volumeMounts, both referencing the same volume name
29673005
mounts = pod_spec["containers"][0]["volumeMounts"]

0 commit comments

Comments
 (0)