diff --git a/AGENTS.md b/AGENTS.md index f4ab91ba4c..1801ec84af 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -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 ``` diff --git a/README.md b/README.md index d70421ff9d..c17415eee8 100644 --- a/README.md +++ b/README.md @@ -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). diff --git a/backend/tests/test_provisioner_pvc_volumes.py b/backend/tests/test_provisioner_pvc_volumes.py index ba9c895c8d..bea72af39b 100644 --- a/backend/tests/test_provisioner_pvc_volumes.py +++ b/backend/tests/test_provisioner_pvc_volumes.py @@ -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): diff --git a/docker/provisioner/README.md b/docker/provisioner/README.md index 2ffde764e2..fe1e4ec0f9 100644 --- a/docker/provisioner/README.md +++ b/docker/provisioner/README.md @@ -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 | diff --git a/docker/provisioner/app.py b/docker/provisioner/app.py index 5a62eea5a3..d4c12578f6 100644 --- a/docker/provisioner/app.py +++ b/docker/provisioner/app.py @@ -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, )