[Docker] Make kit/data writable by the non-root user#6122
Conversation
Add a named volume for ${DOCKER_ISAACSIM_ROOT_PATH}/kit/data so the
non-root isaaclab user can write the Kit kernel config there.
PR isaac-sim#6082 made every docker-compose named-volume mount point writable by
the non-root user, but kit/data was never a declared volume: pre-migration
it was writable only because the container ran as root. Under the non-root
image it stays root-owned, so the Kit kernel fails to write
kit/data/Kit/IsaacLab/3.0/user.config.json ("unable to save the dictionary")
on every docker run, independent of the launch flow.
Declaring it as a named volume routes it through the existing
volume_mounts.py + chown machinery (docker-compose.yaml is the single
source of truth), so the build pre-creates and chowns it to isaaclab.
This fixes the user.config.json write error for all docker users and
persists Kit config across runs, consistent with kit/cache.
There was a problem hiding this comment.
Review: [Docker] Make kit/data writable by the non-root user
Overall Assessment: Clean, minimal fix that correctly addresses the permission issue. The approach is consistent with the existing volume management pattern and properly leverages the volume_mounts.py + chown machinery. No concerns with correctness.
✅ Strengths
- Root cause properly identified — The PR description clearly explains why
kit/datawas never an issue pre-migration (container ran as root) and why it is now. - Minimal, focused change — Only 4 lines added, no unnecessary refactoring.
- Consistent with existing patterns — The
isaac-data-kitvolume follows the same pattern asisaac-cache-kit,isaac-carb-logs, etc. - Leverages existing infrastructure —
volume_mounts.pywill automatically discover this new volume target since it iterates alltype: volumeentries underx-default-isaac-lab-volumes. No code changes needed there.
💬 Minor Observations (non-blocking)
-
Volume naming adjacency (nit, informational):
isaac-data-kit(this PR) → targetskit/dataisaac-data(existing) → targets~/.local/share/ov/data
The names are distinguishable, but at a glance
isaac-data-kitvsisaac-datacould confuse someone skimming the volumes list. A comment like# kit/dataabove the volume declaration would help, consistent with the# isaac-sim/# isaac-labsection comments already present. Purely optional. -
Existing users with stale containers: Users who already have running containers/volumes from before this fix will not automatically benefit — they'd need to recreate. This is standard Docker behavior and not actionable in the PR itself, but might be worth a note in release notes or migration docs if not already covered by #6082's documentation.
🔍 CI Status
Some checks are still pending (Docker image builds, license check). The content-relevant checks (pre-commit, changelog, broken links, wheel build) have all passed.
Verdict: LGTM. Straightforward infrastructure fix, no concerns.
Greptile SummaryThis PR adds a single named volume (
Confidence Score: 5/5Safe to merge — two-line yaml addition that mirrors a well-established existing pattern. The change is minimal and self-consistent: the volume declaration is added in both the mount anchor and the top-level No files require special attention. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[docker-compose.yaml\nadd isaac-data-kit volume] --> B[Dockerfile.base build step\nvolume_mounts.py parses compose]
B --> C[mkdir -p kit/data\nin image layer]
C --> D[chown -R isaaclab:isaaclab kit/data\nin image layer]
D --> E{Container start}
E -->|Fresh named volume| F[Docker copies image dir\ninto volume, owner = isaaclab]
E -->|Existing named volume| G[Volume already chowned\nfrom prior run]
F --> H[isaaclab user writes\nkit/data/.../user.config.json ✓]
G --> H
Reviews (1): Last reviewed commit: "[Docker] Make kit/data writable by the n..." | Re-trigger Greptile |
…6122) (#6123) Cherry-pick of #6122 to `release/3.0.0-beta2` for nvbug 6288406. Makes `/isaac-sim/kit/data` writable by the non-root `isaaclab` user by declaring it as a named volume, so the Kit kernel can write `user.config.json` (the residual that #6082/#6095 did not cover — `kit/data` was never a declared volume). Routes through the existing `volume_mounts.py` + build-time `chown`; `docker-compose.yaml` is the single source of truth. Clean cherry-pick (release already has the #6095 volume-prep mechanism). Validated on develop in #6122: on a clean rebuild, `/isaac-sim/kit/data` is owned by uid 1000 and `user.config.json` writes succeed (pre-fix: `Permission denied`).
Summary
Adds a named volume for
${DOCKER_ISAACSIM_ROOT_PATH}/kit/dataso the non-rootisaaclabuser can write the Kit kernel config there.Fixes the residual
omni.kit.app.pluginerror reported in nvbug 6288406:Root cause
PR #6082 made every docker-compose named-volume mount point writable by the non-root user (driven by
docker/utils/volume_mounts.py+ the build-timechown). Butkit/datawas never a declared volume — pre–non-root migration it was writable simply because the container ran asroot. Under the non-root image it staysroot:root, so the Kit kernel cannot writekit/data/.../user.config.json. This is universal to every docker user (the image bakesUSER isaaclab;kit/datais neither mounted nor chowned), independent of the launch flow.(The sibling
omni.datastorelock errors onkit/cachein the original report were a QA workflow artifact of a rawdocker runthat bypasses the compose volumes; the documentedcontainer.py/compose flow already handles those via #6082.)Fix
Declare
kit/dataas a named volume.docker-compose.yamlis the single source of truth, so this routes it through the existingvolume_mounts.py+chownmachinery — the build pre-creates and chowns/isaac-sim/kit/datatoisaaclab. Consistent withkit/cache/kit/logs, and it benignly persists Kit config across runs.Validation
Built the image (
nvcr.io/nvidian/isaac-sim:latest-release-6-0base) and verified as the non-root runtime user (uid 1000):/isaac-sim/kit/datauser.config.jsonvolume_mounts.pynow emits/isaac-sim/kit/data, and the running container no longer logs theuser.config.json/omni.datastore/DerivedDataCacheerrors.