Skip to content

[Docker] Make kit/data writable by the non-root user (cherry-pick of #6122)#6123

Merged
ooctipus merged 1 commit into
isaac-sim:release/3.0.0-beta2from
hujc7:jichuanh/cherrypick-6122-release-3.0.0-beta2
Jun 11, 2026
Merged

[Docker] Make kit/data writable by the non-root user (cherry-pick of #6122)#6123
ooctipus merged 1 commit into
isaac-sim:release/3.0.0-beta2from
hujc7:jichuanh/cherrypick-6122-release-3.0.0-beta2

Conversation

@hujc7

@hujc7 hujc7 commented Jun 11, 2026

Copy link
Copy Markdown
Collaborator

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).

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.

@isaaclab-review-bot isaaclab-review-bot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review: [Docker] Make kit/data writable by the non-root user (cherry-pick of #6122)

Commit reviewed: 2d7e3e9

Summary

Clean cherry-pick of #6122 to release/3.0.0-beta2. Adds a named volume declaration for ${DOCKER_ISAACSIM_ROOT_PATH}/kit/data so the non-root isaaclab user can write Kit kernel config (user.config.json). This follows the same established pattern as the existing isaac-cache-kit volume for kit/cache.

Analysis

Aspect Assessment
Correctness ✅ The volume mount is correctly placed in the x-default-isaac-lab-volumes anchor (alphabetical ordering with respect to kit/cachekit/data), and the named volume isaac-data-kit is declared in the top-level volumes: section.
Consistency ✅ Naming follows the existing convention: isaac-{purpose}-{component} (e.g., isaac-cache-kit, isaac-cache-ov). The isaac-data-kit name is consistent.
Integration ✅ Routes through the existing volume_mounts.py + build-time chown machinery that was introduced in #6082/#6095. No additional plumbing needed.
Scope ✅ Minimal — 4 lines added to a single file (docker/docker-compose.yaml). No risk of unintended side effects.
CI ✅ Pre-commit and core checks pass. Docker image builds are pending (expected for infra changes).

Minor Observations

  1. Volume naming: isaac-data-kit places the qualifier before the component, which is slightly different from a strict isaac-kit-data pattern one might expect. However, it matches the existing isaac-cache-kit convention in this file, so this is correct as-is.

  2. Persistence implication: Declaring kit/data as a named volume means Kit configuration will now persist across container recreations (consistent with kit/cache). This is desirable behavior as noted in the PR description.

Verdict

This is a straightforward, well-scoped fix. The change correctly addresses the permission issue for kit/data under the non-root user model, using the established volume mechanism. No concerns.


Automated review by IsaacLab Review Bot

@hujc7 hujc7 marked this pull request as ready for review June 11, 2026 00:12
@hujc7 hujc7 requested a review from pascal-roth as a code owner June 11, 2026 00:12
@greptile-apps

greptile-apps Bot commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

Declares ${DOCKER_ISAACSIM_ROOT_PATH}/kit/data as a named Docker volume (isaac-data-kit) so the non-root isaaclab user can write user.config.json under the Kit kernel's data directory — the last path not covered by the earlier kit/cache fix. The build-time volume_mounts.py + chown mechanism in Dockerfile.base automatically picks up the new entry from docker-compose.yaml, requiring no Dockerfile changes.

  • Added isaac-data-kit volume entry to both x-default-isaac-lab-volumes (shared anchor used by all services) and the top-level volumes: block, consistent with how every other named volume is declared in this file.

Confidence Score: 5/5

Single-volume declaration change that follows the established pattern exactly; no logic or control flow is altered.

Both the compose anchor and the top-level volumes block are updated consistently. The volume_mounts.py + chown mechanism in Dockerfile.base automatically handles the new entry without any Dockerfile changes needed. The change is minimal and targeted, matching how every other named volume in this file is wired up.

No files require special attention.

Important Files Changed

Filename Overview
docker/docker-compose.yaml Adds isaac-data-kit named volume targeting ${DOCKER_ISAACSIM_ROOT_PATH}/kit/data in both the shared volume anchor and the top-level volumes declaration, enabling the non-root isaaclab user to write user.config.json under kit/data.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["docker-compose.yaml\nx-default-isaac-lab-volumes"] -->|new entry: isaac-data-kit| B["Named Volume\nisaac-data-kit"]
    A -->|parsed at build time| C["volume_mounts.py\nresolves target paths"]
    C -->|mkdir + chown| D["Dockerfile.base\nownership set to isaaclab uid 1000"]
    D -->|volume inherits directory ownership| B
    B -->|mounted at kit/data| E["isaaclab user\nwrites user.config.json"]
Loading

Reviews (1): Last reviewed commit: "[Docker] Make kit/data writable by the n..." | Re-trigger Greptile

@ooctipus ooctipus merged commit 1ef11e5 into isaac-sim:release/3.0.0-beta2 Jun 11, 2026
96 of 98 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants