refactor(provisioner): remove ignored legacy skills arguments (#7555)

* refactor(provisioner): remove ignored legacy skills arguments

* chore(provisioner): document legacy flag and align test durations
This commit is contained in:
Xinmin Zeng 2026-10-26 02:01:51 -07:00 committed by GitHub
parent 370da43119
commit 44c6077801
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 8 additions and 49 deletions

View File

@ -19529,8 +19529,6 @@
"tests/test_provisioner_mount_contract.py::test_gateway_and_provisioner_extra_mount_contracts_match": 0.020990583114326,
"tests/test_provisioner_pvc_volumes.py::TestBuildPodVolumes::test_pod_hostpath_has_four_mounts": 0.00571479182690382,
"tests/test_provisioner_pvc_volumes.py::TestBuildPodVolumes::test_pod_hostpath_has_four_volumes": 0.005259708035737276,
"tests/test_provisioner_pvc_volumes.py::TestBuildPodVolumes::test_pod_hostpath_with_legacy_has_four_mounts": 0.00440941727720201,
"tests/test_provisioner_pvc_volumes.py::TestBuildPodVolumes::test_pod_hostpath_with_legacy_has_four_volumes": 0.005018542055040598,
"tests/test_provisioner_pvc_volumes.py::TestBuildPodVolumes::test_pod_includes_extra_mounts": 0.005034750327467918,
"tests/test_provisioner_pvc_volumes.py::TestBuildPodVolumes::test_pod_pvc_has_two_mounts": 0.005018165102228522,
"tests/test_provisioner_pvc_volumes.py::TestBuildPodVolumes::test_pod_pvc_has_two_volumes": 0.004009666852653027,
@ -19545,10 +19543,10 @@
"tests/test_provisioner_pvc_volumes.py::TestBuildVolumeMounts::test_extra_mount_uses_pvc_subpath": 0.0031363321468234062,
"tests/test_provisioner_pvc_volumes.py::TestBuildVolumeMounts::test_hostpath_skills_custom_mount": 0.0032700421288609505,
"tests/test_provisioner_pvc_volumes.py::TestBuildVolumeMounts::test_hostpath_skills_legacy_mount": 0.0029997501987963915,
"tests/test_provisioner_pvc_volumes.py::TestBuildVolumeMounts::test_hostpath_skills_projection_mounts_are_stable": 0.0029834159649908543,
"tests/test_provisioner_pvc_volumes.py::TestBuildVolumeMounts::test_hostpath_skills_public_mount": 0.004140791948884726,
"tests/test_provisioner_pvc_volumes.py::TestBuildVolumeMounts::test_hostpath_userdata_read_write": 0.004402917111292481,
"tests/test_provisioner_pvc_volumes.py::TestBuildVolumeMounts::test_hostpath_uses_three_projection_mounts": 0.0030454988591372967,
"tests/test_provisioner_pvc_volumes.py::TestBuildVolumeMounts::test_hostpath_without_legacy_flag_still_has_legacy_mount": 0.0029834159649908543,
"tests/test_provisioner_pvc_volumes.py::TestBuildVolumeMounts::test_pvc_defaults_to_default_user_subpath": 0.00381799996830523,
"tests/test_provisioner_pvc_volumes.py::TestBuildVolumeMounts::test_pvc_no_subpath_on_userdata": 0.0029956260696053505,
"tests/test_provisioner_pvc_volumes.py::TestBuildVolumeMounts::test_pvc_returns_two_mounts": 0.003352042054757476,
@ -19574,10 +19572,10 @@
"tests/test_provisioner_pvc_volumes.py::TestBuildVolumes::test_extra_mount_uses_userdata_pvc_when_configured": 0.004007165785878897,
"tests/test_provisioner_pvc_volumes.py::TestBuildVolumes::test_hostpath_skills_custom_volume": 0.0037376671098172665,
"tests/test_provisioner_pvc_volumes.py::TestBuildVolumes::test_hostpath_skills_legacy_volume": 0.0033499987330287695,
"tests/test_provisioner_pvc_volumes.py::TestBuildVolumes::test_hostpath_skills_projection_volumes_are_stable": 0.0032199581619352102,
"tests/test_provisioner_pvc_volumes.py::TestBuildVolumes::test_hostpath_skills_public_volume": 0.004053332144394517,
"tests/test_provisioner_pvc_volumes.py::TestBuildVolumes::test_hostpath_userdata_includes_thread_id": 0.004025542875751853,
"tests/test_provisioner_pvc_volumes.py::TestBuildVolumes::test_hostpath_uses_three_projection_volumes": 0.004587041912600398,
"tests/test_provisioner_pvc_volumes.py::TestBuildVolumes::test_hostpath_without_legacy_flag_still_has_empty_capable_mount": 0.0032199581619352102,
"tests/test_provisioner_pvc_volumes.py::TestBuildVolumes::test_pvc_returns_two_volumes": 0.0033919168636202812,
"tests/test_provisioner_pvc_volumes.py::TestBuildVolumes::test_pvc_volume_names_are_stable": 0.00340524991042912,
"tests/test_provisioner_pvc_volumes.py::TestBuildVolumes::test_skills_pvc_overrides_hostpath": 0.0032209178898483515,

View File

@ -159,17 +159,14 @@ class TestBuildVolumes:
def test_hostpath_skills_legacy_volume(self, provisioner_module):
"""Legacy projection is per-user and stable regardless of visibility."""
provisioner_module.SKILLS_PVC_NAME = ""
volumes = provisioner_module._build_volumes(
"thread-1",
include_legacy_skills=True,
)
volumes = provisioner_module._build_volumes("thread-1")
legacy = volumes[2]
assert legacy.name == "skills-legacy"
assert legacy.host_path is not None
assert "users/default/skills_view/legacy" in posix_path(legacy.host_path.path)
assert legacy.host_path.type == "Directory"
def test_hostpath_without_legacy_flag_still_has_empty_capable_mount(self, provisioner_module):
def test_hostpath_skills_projection_volumes_are_stable(self, provisioner_module):
"""Visibility changes update contents without recreating the Pod."""
provisioner_module.SKILLS_PVC_NAME = ""
volumes = provisioner_module._build_volumes("thread-1")
@ -394,15 +391,12 @@ class TestBuildVolumeMounts:
def test_hostpath_skills_legacy_mount(self, provisioner_module):
"""Legacy skills mount at /mnt/skills/legacy, read-only."""
provisioner_module.SKILLS_PVC_NAME = ""
mounts = provisioner_module._build_volume_mounts(
"thread-1",
include_legacy_skills=True,
)
mounts = provisioner_module._build_volume_mounts("thread-1")
assert mounts[2].name == "skills-legacy"
assert mounts[2].mount_path == "/mnt/skills/legacy"
assert mounts[2].read_only is True
def test_hostpath_without_legacy_flag_still_has_legacy_mount(self, provisioner_module):
def test_hostpath_skills_projection_mounts_are_stable(self, provisioner_module):
"""Hidden legacy content is represented by an empty mounted view."""
provisioner_module.SKILLS_PVC_NAME = ""
mounts = provisioner_module._build_volume_mounts("thread-1")
@ -665,28 +659,6 @@ class TestBuildPodVolumes:
pod = provisioner_module._build_pod("sandbox-1", "thread-1")
assert len(pod.spec.containers[0].volume_mounts) == 4
def test_pod_hostpath_with_legacy_has_four_volumes(self, provisioner_module):
"""Legacy volume should be present when the backend requests it."""
provisioner_module.SKILLS_PVC_NAME = ""
provisioner_module.USERDATA_PVC_NAME = ""
pod = provisioner_module._build_pod(
"sandbox-1",
"thread-1",
include_legacy_skills=True,
)
assert len(pod.spec.volumes) == 4
def test_pod_hostpath_with_legacy_has_four_mounts(self, provisioner_module):
"""Legacy mount should be present when the backend requests it."""
provisioner_module.SKILLS_PVC_NAME = ""
provisioner_module.USERDATA_PVC_NAME = ""
pod = provisioner_module._build_pod(
"sandbox-1",
"thread-1",
include_legacy_skills=True,
)
assert len(pod.spec.containers[0].volume_mounts) == 4
def test_pod_pvc_has_two_volumes(self, provisioner_module):
"""PVC Pod spec should contain exactly 2 volumes."""
provisioner_module.SKILLS_PVC_NAME = "skills-pvc"
@ -746,11 +718,7 @@ class TestBuildPodVolumes:
"""Ensure public/custom/legacy mount paths are correct."""
provisioner_module.SKILLS_PVC_NAME = ""
provisioner_module.USERDATA_PVC_NAME = ""
pod = provisioner_module._build_pod(
"sandbox-1",
"thread-1",
include_legacy_skills=True,
)
pod = provisioner_module._build_pod("sandbox-1", "thread-1")
mount_paths = {m.name: m.mount_path for m in pod.spec.containers[0].volume_mounts}
assert mount_paths["skills-public"] == "/mnt/skills/public"
assert mount_paths["skills-custom"] == "/mnt/skills/custom"

View File

@ -510,6 +510,7 @@ class CreateSandboxRequest(BaseModel):
thread_id: str | None = Field(default=None, pattern=SAFE_THREAD_ID_PATTERN)
user_id: str = Field(default=DEFAULT_USER_ID, pattern=SAFE_USER_ID_PATTERN)
extra_mounts: list[ExtraMount] = Field(default_factory=list)
# Accepted and logged for older Gateway requests; no longer gates mounts.
include_legacy_skills: bool = False
# Sent explicitly by new Gateways; the default keeps old Gateways compatible.
skills_container_path: str = DEFAULT_SKILLS_CONTAINER_PATH
@ -621,7 +622,6 @@ def _build_volumes(
thread_id: str,
user_id: str = DEFAULT_USER_ID,
*,
include_legacy_skills: bool = False,
extra_mounts: list[ExtraMount] | None = None,
skills_container_path: str = DEFAULT_SKILLS_CONTAINER_PATH,
provision_lark_cli_runtime: bool = False,
@ -635,7 +635,6 @@ def _build_volumes(
``LocalSandboxProvider`` and ``AioSandboxProvider``.
"""
volumes: list[k8s_client.V1Volume] = []
del include_legacy_skills # retained for request compatibility
skills_root = _normalize_skills_container_path(skills_container_path)
managed_skill_paths = _managed_skill_category_mount_paths(skills_root)
validated_extra_mounts = _validated_extra_mounts(
@ -786,7 +785,6 @@ def _build_volume_mounts(
thread_id: str,
user_id: str = DEFAULT_USER_ID,
*,
include_legacy_skills: bool = False,
extra_mounts: list[ExtraMount] | None = None,
skills_container_path: str = DEFAULT_SKILLS_CONTAINER_PATH,
provision_lark_cli_runtime: bool = False,
@ -800,7 +798,6 @@ def _build_volume_mounts(
scope that mount with ``SKILLS_PVC_SUBPATH_TEMPLATE``.
"""
mounts: list[k8s_client.V1VolumeMount] = []
del include_legacy_skills # retained for request compatibility
skills_root = _normalize_skills_container_path(skills_container_path)
managed_skill_paths = _managed_skill_category_mount_paths(skills_root)
validated_extra_mounts = _validated_extra_mounts(
@ -996,7 +993,6 @@ def _build_pod(
thread_id: str,
user_id: str = DEFAULT_USER_ID,
*,
include_legacy_skills: bool = False,
extra_mounts: list[ExtraMount] | None = None,
skills_container_path: str = DEFAULT_SKILLS_CONTAINER_PATH,
provision_lark_cli_runtime: bool = False,
@ -1083,7 +1079,6 @@ def _build_pod(
volume_mounts=_build_volume_mounts(
thread_id,
user_id=user_id,
include_legacy_skills=include_legacy_skills,
extra_mounts=extra_mounts,
skills_container_path=skills_container_path,
provision_lark_cli_runtime=provision_lark_cli_runtime,
@ -1104,7 +1099,6 @@ def _build_pod(
volumes=_build_volumes(
thread_id,
user_id=user_id,
include_legacy_skills=include_legacy_skills,
extra_mounts=extra_mounts,
skills_container_path=skills_container_path,
provision_lark_cli_runtime=provision_lark_cli_runtime,
@ -1322,7 +1316,6 @@ def create_sandbox(req: CreateSandboxRequest):
sandbox_id,
thread_id,
user_id=user_id,
include_legacy_skills=include_legacy_skills,
extra_mounts=req.extra_mounts,
skills_container_path=skills_container_path,
provision_lark_cli_runtime=provision_lark_cli_runtime,