mirror of
https://github.com/bytedance/deer-flow.git
synced 2026-10-26 21:26:39 +00:00
fix(provisioner): reuse userdata PVC across sandbox mounts (#7274)
* fix(provisioner): reuse userdata PVC across sandbox mounts * fix(provisioner): keep shared volume names in sync and document skills PVC caveat * test(provisioner): pin skills PVC caveat and hostPath broker manifest
This commit is contained in:
parent
1792e4e432
commit
3113c150cd
@ -44,6 +44,10 @@ redirect `make dev`'s port wait. Nginx's IPv4+IPv6 `default_server` and Gateway'
|
||||
Require explicit bind addresses for new published ports;
|
||||
`backend/tests/test_compose_default_bind_host.py` pins every service in both files.
|
||||
|
||||
Provisioner PVC mode declares `USERDATA_PVC_NAME` once as `user-data`; extra and
|
||||
Lark broker credential mounts reuse it with their existing subPaths and read-only
|
||||
flags. Keep credential mounts broker-only and keep distinct volumes in hostPath mode.
|
||||
|
||||
## Repository Map
|
||||
|
||||
```
|
||||
|
||||
@ -514,6 +514,7 @@ make docker-logs # View logs
|
||||
```
|
||||
|
||||
`make docker-start` starts `provisioner` only when `config.yaml` uses provisioner mode (`sandbox.use: deerflow.community.aio_sandbox:AioSandboxProvider` with `provisioner_url`). Direct Compose users can add `--profile provisioner` to the dev command for the same mode; the default stack does not require Kubernetes.
|
||||
With `USERDATA_PVC_NAME`, sandbox data, extra mounts and Lark broker credentials share one `user-data` volume with separate `subPath` mounts, avoiding duplicate references to the same claim on NFS CSI drivers.
|
||||
|
||||
Docker builds use the upstream `uv` registry by default. The gateway image installs with `uv sync --locked`, and `uv.lock` pins `https://pypi.org/simple`, so `UV_INDEX_URL` has to keep that value: exporting a mirror such as `https://pypi.tuna.tsinghua.edu.cn/simple` fails the build with ``The lockfile at `uv.lock` needs to be updated, but `--locked` was provided`` (reproduce in an already-synced environment with `cd backend && UV_INDEX_URL=https://pypi.tuna.tsinghua.edu.cn/simple uv sync --locked --check`; if the environment itself is stale, `--check` reports that instead for either index, so use `uv lock --check` to see it without syncing). Installing from a mirror requires regenerating the lock or dropping `--locked`, and both change what the image installs, so keep the default index for a checkout you intend to contribute back. `NPM_REGISTRY=https://registry.npmmirror.com` is independent of this: it applies to the frontend image's `pnpm` install and to the gateway image's Lark CLI install (`@larksuite/cli`, which `backend/Dockerfile` installs with npm).
|
||||
|
||||
|
||||
@ -18,6 +18,108 @@ def _thread_skill_mounts(
|
||||
]
|
||||
|
||||
|
||||
@pytest.mark.parametrize("volume_name", ["user-data", "shared-userdata"])
|
||||
@pytest.mark.parametrize("broker", [False, True])
|
||||
def test_pod_reuses_one_userdata_claim_for_all_subpaths(provisioner_module, broker, volume_name):
|
||||
"""One PVC volume serves sandbox projections and broker-only credentials."""
|
||||
provisioner_module.USERDATA_VOLUME_NAME = volume_name
|
||||
provisioner_module.USERDATA_PVC_NAME = "userdata-pvc"
|
||||
provisioner_module.SKILLS_PVC_NAME = "skills-pvc"
|
||||
provisioner_module.DEER_FLOW_HOST_BASE_DIR = "/state"
|
||||
provisioner_module.LARK_CLI_BROKER_IMAGE = "deer-flow/lark-cli-broker:test"
|
||||
extra_mounts = [
|
||||
*_thread_skill_mounts(provisioner_module),
|
||||
*TestLarkCliBrokerSidecar._credential_mounts(provisioner_module),
|
||||
]
|
||||
pod = provisioner_module._build_pod(
|
||||
"sandbox-1",
|
||||
"thread-1",
|
||||
user_id="alice",
|
||||
extra_mounts=extra_mounts,
|
||||
provision_lark_cli_broker=broker,
|
||||
)
|
||||
spec = provisioner_module.k8s_client.ApiClient().sanitize_for_serialization(pod)["spec"]
|
||||
claim_volumes = [volume for volume in spec["volumes"] if "persistentVolumeClaim" in volume]
|
||||
assert claim_volumes == [{"name": volume_name, "persistentVolumeClaim": {"claimName": "userdata-pvc"}}]
|
||||
declared_names = {volume["name"] for volume in spec["volumes"]}
|
||||
for container in [*spec["containers"], *spec.get("initContainers", [])]:
|
||||
assert all(mount["name"] in declared_names for mount in container.get("volumeMounts", []))
|
||||
|
||||
sandbox_mounts = {mount["mountPath"]: mount for mount in spec["containers"][0]["volumeMounts"]}
|
||||
for category in ("public", "custom", "legacy", "integrations"):
|
||||
mount = sandbox_mounts[f"/mnt/skills/{category}"]
|
||||
assert mount["name"] == volume_name
|
||||
assert mount["subPath"] == f"deer-flow/users/alice/threads/thread-1/skills_view/{category}"
|
||||
assert mount["readOnly"] is True
|
||||
assert sandbox_mounts["/mnt/user-data"]["subPath"] == "deer-flow/users/alice/threads/thread-1/user-data"
|
||||
|
||||
if broker:
|
||||
assert provisioner_module.LARK_CLI_CONFIG_CONTAINER_PATH not in sandbox_mounts
|
||||
container = next(item for item in spec["containers"] if item["name"] == "lark-cli-broker")
|
||||
mounts = {mount["mountPath"]: mount for mount in container["volumeMounts"]}
|
||||
for mount in mounts.values():
|
||||
assert mount["name"] == volume_name
|
||||
assert mounts[provisioner_module.LARK_BROKER_SIDECAR_CONFIG_PATH]["subPath"] == "deer-flow/users/alice/integrations/lark-cli/config"
|
||||
assert mounts[provisioner_module.LARK_BROKER_SIDECAR_CONFIG_PATH]["readOnly"] is True
|
||||
assert mounts[provisioner_module.LARK_BROKER_SIDECAR_LOCKS_PATH]["readOnly"] is False
|
||||
assert mounts[provisioner_module.LARK_BROKER_SIDECAR_DATA_PATH]["subPath"] == "deer-flow/users/alice/integrations/lark-cli/data"
|
||||
|
||||
|
||||
@pytest.mark.parametrize("skills_claim", ["skills-pvc", "userdata-pvc"])
|
||||
def test_unrestricted_pod_keeps_separate_skills_claim(provisioner_module, skills_claim):
|
||||
"""Pin the documented exception when both settings point at one claim."""
|
||||
provisioner_module.USERDATA_PVC_NAME = "userdata-pvc"
|
||||
provisioner_module.SKILLS_PVC_NAME = skills_claim
|
||||
pod = provisioner_module._build_pod("sandbox-1", "thread-1", user_id="alice")
|
||||
spec = provisioner_module.k8s_client.ApiClient().sanitize_for_serialization(pod)["spec"]
|
||||
volumes = {volume["name"]: volume["persistentVolumeClaim"] for volume in spec["volumes"]}
|
||||
assert volumes == {
|
||||
"skills": {"claimName": skills_claim, "readOnly": True},
|
||||
"user-data": {"claimName": "userdata-pvc"},
|
||||
}
|
||||
mounts = {mount["mountPath"]: mount for mount in spec["containers"][0]["volumeMounts"]}
|
||||
assert mounts["/mnt/skills"]["name"] == "skills"
|
||||
assert mounts["/mnt/skills"]["readOnly"] is True
|
||||
assert mounts["/mnt/user-data"]["name"] == "user-data"
|
||||
|
||||
|
||||
def test_hostpath_pod_keeps_broker_credential_volumes(provisioner_module):
|
||||
"""HostPath broker credentials remain declared and isolated from sandbox."""
|
||||
provisioner_module.USERDATA_PVC_NAME = ""
|
||||
provisioner_module.SKILLS_PVC_NAME = ""
|
||||
provisioner_module.DEER_FLOW_HOST_BASE_DIR = "/state"
|
||||
provisioner_module.LARK_CLI_BROKER_IMAGE = "deer-flow/lark-cli-broker:test"
|
||||
credentials = TestLarkCliBrokerSidecar._credential_mounts(provisioner_module)
|
||||
pod = provisioner_module._build_pod(
|
||||
"sandbox-1",
|
||||
"thread-1",
|
||||
user_id="alice",
|
||||
extra_mounts=credentials,
|
||||
provision_lark_cli_broker=True,
|
||||
)
|
||||
spec = provisioner_module.k8s_client.ApiClient().sanitize_for_serialization(pod)["spec"]
|
||||
volumes = {volume["name"]: volume for volume in spec["volumes"]}
|
||||
expected = {
|
||||
"lark-cli-config": ("/state/users/alice/integrations/lark-cli/config", "Directory"),
|
||||
"lark-cli-locks": ("/state/users/alice/integrations/lark-cli/config/locks", "DirectoryOrCreate"),
|
||||
"lark-cli-data": ("/state/users/alice/integrations/lark-cli/data", "DirectoryOrCreate"),
|
||||
}
|
||||
for name, (path, directory_type) in expected.items():
|
||||
assert volumes[name]["hostPath"] == {"path": path, "type": directory_type}
|
||||
assert not any("persistentVolumeClaim" in volume for volume in volumes.values())
|
||||
for container in [*spec["containers"], *spec.get("initContainers", [])]:
|
||||
assert all(mount["name"] in volumes for mount in container.get("volumeMounts", []))
|
||||
sandbox = spec["containers"][0]
|
||||
assert not any(mount["name"] in expected for mount in sandbox["volumeMounts"])
|
||||
broker = next(container for container in spec["containers"] if container["name"] == "lark-cli-broker")
|
||||
mounts = {mount["name"]: mount for mount in broker["volumeMounts"]}
|
||||
assert set(mounts) == set(expected)
|
||||
assert mounts["lark-cli-config"]["readOnly"] is True
|
||||
assert mounts["lark-cli-locks"]["readOnly"] is False
|
||||
assert mounts["lark-cli-data"]["readOnly"] is False
|
||||
assert all("subPath" not in mount for mount in mounts.values())
|
||||
|
||||
|
||||
# ── _build_volumes ─────────────────────────────────────────────────────
|
||||
|
||||
|
||||
@ -168,13 +270,12 @@ class TestBuildVolumes:
|
||||
|
||||
volumes = provisioner_module._build_volumes("thread-1", extra_mounts=extra_mounts)
|
||||
|
||||
# Three skill projections + user-data (4 base) + 1 extra volume.
|
||||
assert len(volumes) == 5
|
||||
extra_vol = volumes[-1]
|
||||
assert extra_vol.name == "extra-0"
|
||||
assert extra_vol.persistent_volume_claim is not None
|
||||
assert extra_vol.persistent_volume_claim.claim_name == "userdata-pvc"
|
||||
assert extra_vol.host_path is None
|
||||
# Three hostPath skill projections + one shared user-data PVC.
|
||||
assert len(volumes) == 4
|
||||
userdata_vol = next(volume for volume in volumes if volume.name == "user-data")
|
||||
assert userdata_vol.persistent_volume_claim.claim_name == "userdata-pvc"
|
||||
assert userdata_vol.host_path is None
|
||||
assert not any(volume.name.startswith("extra-") for volume in volumes)
|
||||
|
||||
def test_extra_mount_rejects_paths_outside_deerflow_state(self, provisioner_module):
|
||||
"""Provisioner must not accept arbitrary hostPath mounts from clients."""
|
||||
@ -255,8 +356,8 @@ class TestBuildVolumes:
|
||||
|
||||
assert all(volume.name != "skills" for volume in volumes)
|
||||
extra_volumes = [volume for volume in volumes if volume.name.startswith("extra-")]
|
||||
assert len(extra_volumes) == 4
|
||||
assert all(volume.persistent_volume_claim.claim_name == "userdata-pvc" for volume in extra_volumes)
|
||||
assert extra_volumes == []
|
||||
assert [volume.name for volume in volumes] == ["user-data"]
|
||||
|
||||
|
||||
# ── _build_volume_mounts ───────────────────────────────────────────────
|
||||
@ -411,7 +512,7 @@ class TestBuildVolumeMounts:
|
||||
mounts = provisioner_module._build_volume_mounts("thread-1", extra_mounts=extra_mounts)
|
||||
|
||||
extra_mount = mounts[-1]
|
||||
assert extra_mount.name == "extra-0"
|
||||
assert extra_mount.name == "user-data"
|
||||
assert extra_mount.sub_path == "deer-flow/users/alice/integrations/lark-cli/config"
|
||||
|
||||
def test_extra_mount_rejects_unknown_container_path(self, provisioner_module):
|
||||
|
||||
@ -146,6 +146,17 @@ List all sandboxes currently managed.
|
||||
|
||||
## Configuration
|
||||
|
||||
When `USERDATA_PVC_NAME` is set, user-data and extra mounts share the `user-data` volume.
|
||||
User data, extra mounts and Lark broker credentials use separate `subPath` mounts
|
||||
on that volume. Read-only flags remain per mount; broker credentials are mounted
|
||||
only in the broker container. This avoids repeated claim publication on NFS CSI
|
||||
drivers that cannot handle multiple volume declarations for one claim. Without
|
||||
`USERDATA_PVC_NAME`, each hostPath keeps its own volume.
|
||||
|
||||
If `SKILLS_PVC_NAME` is also configured, use a different claim name from
|
||||
`USERDATA_PVC_NAME`: unrestricted threads still declare a separate `skills`
|
||||
volume, so using the same claim for both can reintroduce duplicate publication.
|
||||
|
||||
The provisioner is configured via environment variables (set in [docker-compose-dev.yaml](../docker-compose-dev.yaml)):
|
||||
|
||||
| Variable | Default | Description |
|
||||
|
||||
@ -103,6 +103,7 @@ THREADS_HOST_PATH = os.environ.get("THREADS_HOST_PATH", "/.deer-flow/threads")
|
||||
DEER_FLOW_HOST_BASE_DIR = os.environ.get("DEER_FLOW_HOST_BASE_DIR", "/.deer-flow")
|
||||
SKILLS_PVC_NAME = os.environ.get("SKILLS_PVC_NAME", "")
|
||||
USERDATA_PVC_NAME = os.environ.get("USERDATA_PVC_NAME", "")
|
||||
USERDATA_VOLUME_NAME = "user-data"
|
||||
SKILLS_PVC_SUBPATH_TEMPLATE = os.environ.get("SKILLS_PVC_SUBPATH_TEMPLATE", "")
|
||||
SANDBOX_CONTAINER_PORT_RAW = os.environ.get("SANDBOX_CONTAINER_PORT", "8080")
|
||||
SANDBOX_SERVICE_TYPE = os.environ.get("SANDBOX_SERVICE_TYPE", "NodePort")
|
||||
@ -567,6 +568,7 @@ def _build_extra_volumes(
|
||||
*,
|
||||
skills_container_path: str = DEFAULT_SKILLS_CONTAINER_PATH,
|
||||
) -> list[k8s_client.V1Volume]:
|
||||
"""Build hostPath extras; _build_volumes alone declares the user-data PVC."""
|
||||
volumes: list[k8s_client.V1Volume] = []
|
||||
for index, mount in enumerate(
|
||||
_validated_extra_mounts(
|
||||
@ -575,14 +577,8 @@ def _build_extra_volumes(
|
||||
)
|
||||
):
|
||||
if USERDATA_PVC_NAME:
|
||||
volumes.append(
|
||||
k8s_client.V1Volume(
|
||||
name=_extra_mount_volume_name(index),
|
||||
persistent_volume_claim=k8s_client.V1PersistentVolumeClaimVolumeSource(
|
||||
claim_name=USERDATA_PVC_NAME,
|
||||
),
|
||||
)
|
||||
)
|
||||
# _build_volumes already declares this claim as user-data. Reuse
|
||||
# that volume with per-mount subPaths instead of republishing it.
|
||||
continue
|
||||
|
||||
volumes.append(
|
||||
@ -602,6 +598,7 @@ def _build_extra_volume_mounts(
|
||||
*,
|
||||
skills_container_path: str = DEFAULT_SKILLS_CONTAINER_PATH,
|
||||
) -> list[k8s_client.V1VolumeMount]:
|
||||
"""Reference the shared PVC declared by _build_volumes, or hostPath extras."""
|
||||
mounts: list[k8s_client.V1VolumeMount] = []
|
||||
for index, mount in enumerate(
|
||||
_validated_extra_mounts(
|
||||
@ -610,7 +607,7 @@ def _build_extra_volume_mounts(
|
||||
)
|
||||
):
|
||||
volume_mount = k8s_client.V1VolumeMount(
|
||||
name=_extra_mount_volume_name(index),
|
||||
name=USERDATA_VOLUME_NAME if USERDATA_PVC_NAME else _extra_mount_volume_name(index),
|
||||
mount_path=mount.container_path,
|
||||
read_only=mount.read_only,
|
||||
)
|
||||
@ -721,14 +718,14 @@ def _build_volumes(
|
||||
|
||||
if USERDATA_PVC_NAME:
|
||||
userdata_vol = k8s_client.V1Volume(
|
||||
name="user-data",
|
||||
name=USERDATA_VOLUME_NAME,
|
||||
persistent_volume_claim=k8s_client.V1PersistentVolumeClaimVolumeSource(
|
||||
claim_name=USERDATA_PVC_NAME,
|
||||
),
|
||||
)
|
||||
else:
|
||||
userdata_vol = k8s_client.V1Volume(
|
||||
name="user-data",
|
||||
name=USERDATA_VOLUME_NAME,
|
||||
host_path=k8s_client.V1HostPathVolumeSource(
|
||||
path=join_host_path(THREADS_HOST_PATH, thread_id, "user-data"),
|
||||
type="DirectoryOrCreate",
|
||||
@ -770,24 +767,18 @@ def _build_volumes(
|
||||
if mount is None:
|
||||
continue
|
||||
if USERDATA_PVC_NAME:
|
||||
volumes.append(
|
||||
k8s_client.V1Volume(
|
||||
name=volume_name,
|
||||
persistent_volume_claim=k8s_client.V1PersistentVolumeClaimVolumeSource(
|
||||
claim_name=USERDATA_PVC_NAME,
|
||||
),
|
||||
)
|
||||
)
|
||||
else:
|
||||
volumes.append(
|
||||
k8s_client.V1Volume(
|
||||
name=volume_name,
|
||||
host_path=k8s_client.V1HostPathVolumeSource(
|
||||
path=mount.host_path,
|
||||
type="Directory" if mount.read_only else "DirectoryOrCreate",
|
||||
),
|
||||
)
|
||||
# Broker credentials use their own subPaths on user-data;
|
||||
# only the broker container receives these mounts.
|
||||
continue
|
||||
volumes.append(
|
||||
k8s_client.V1Volume(
|
||||
name=volume_name,
|
||||
host_path=k8s_client.V1HostPathVolumeSource(
|
||||
path=mount.host_path,
|
||||
type="Directory" if mount.read_only else "DirectoryOrCreate",
|
||||
),
|
||||
)
|
||||
)
|
||||
return volumes
|
||||
|
||||
|
||||
@ -857,7 +848,7 @@ def _build_volume_mounts(
|
||||
mounts.extend(mount for mount in default_skill_mounts if mount.mount_path not in skill_overrides)
|
||||
|
||||
userdata_mount = k8s_client.V1VolumeMount(
|
||||
name="user-data",
|
||||
name=USERDATA_VOLUME_NAME,
|
||||
mount_path="/mnt/user-data",
|
||||
read_only=False,
|
||||
)
|
||||
@ -946,6 +937,7 @@ def _build_lark_cli_broker_sidecars(
|
||||
container), so the plaintext app secret / OAuth tokens stay out of the
|
||||
sandbox filesystem. The config root is read-only and its nested locks mount
|
||||
is writable. The broker serves the command surface on loopback.
|
||||
PVC mounts reuse the user-data volume declared solely by _build_volumes.
|
||||
"""
|
||||
if not _lark_cli_broker_enabled(provision_lark_cli_broker):
|
||||
return []
|
||||
@ -963,7 +955,7 @@ def _build_lark_cli_broker_sidecars(
|
||||
if mount is None:
|
||||
continue
|
||||
sidecar_mount = k8s_client.V1VolumeMount(
|
||||
name=volume_name,
|
||||
name=USERDATA_VOLUME_NAME if USERDATA_PVC_NAME else volume_name,
|
||||
mount_path=sidecar_path,
|
||||
read_only=mount.read_only,
|
||||
)
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user