[Docker] Point uv at the shipped environment in the kit-less image - #7628
Conversation
The kit-less image is the only multi-stage Dockerfile, and ENV does not cross stages: the builder's uv configuration was never repeated in the runtime stage. uv resolves the project environment from UV_PROJECT_ENVIRONMENT alone, so any in-container `uv run` ignored VIRTUAL_ENV, targeted /workspace/isaaclab/.venv and reinstalled the whole locked dependency set. Where Compose or CI bind-mounts the host checkout over source/, the editable rebuild then wrote egg-info into files the image's uid cannot touch and the command died with "Permission denied". Repeat the four uv settings in the runtime stage. UV_NO_SYNC is what keeps `uv run` on the environment the image shipped; the venv is already synced from the lock, so re-deriving it can only diverge from it. Every existing validation step calls `python` directly, which is why nothing caught this. Add a `uv run` step to the kit-less validation action.
|
run-ci |
|
run-ci |
The stage split existed to keep a C toolchain out of the runtime, but nothing
in the lock builds a compiled source distribution: 0 of 222 installed
distributions are locally built on x86_64. Dropping build-essential, cmake and
python3-dev removes the toolchain outright, and with one stage the uv settings
are declared once instead of being repeated across a stage boundary the
runtime never inherited.
Install through a uv cache mount as Dockerfile.base and Dockerfile.curobo do.
The mount targets ${HOME}/.cache/uv, not root's, because this image sets HOME
for the whole build; UV_CACHE_DIR pins the two together so an unmatched mount
cannot bake the cache into the layer.
Assert the fix on the built image rather than on the Dockerfile text: the new
image invariant runs uv run inside the image and checks it resolves the shipped
environment. kitless-docker.yml never passed verify-test-path, so image
invariants had never run against this image at all.
Measured on desk, RTX PRO 6000: 15.46 GB image, 109 s cold build, OVRTX
renderer constructs on GPU, warp and newton import on CUDA, uv run resolves
/opt/isaaclab-venv with source mounted read-only.
|
run-ci |
The verify-test-path input added in the previous commit runs the image invariants with ``uv run`` on the runner itself, and the GPU runners have no uv on PATH, so the step failed with exit 127 before the tests ran. build.yaml already carries the same setup-uv step ahead of its own verify-test-path for this reason; kitless-docker.yml only had one in its ubuntu-latest changes job.
|
run-ci |
| ENV HOME=${DOCKER_USER_HOME} | ||
| ENV VIRTUAL_ENV=${VENV_PATH_ARG} | ||
| ENV PATH=${VENV_PATH_ARG}/bin:/usr/local/bin:${PATH} | ||
| ENV UV_NO_SYNC=1 |
There was a problem hiding this comment.
No-sync breaks optional extras
Setting UV_NO_SYNC=1 globally prevents uv run --extra ... from installing extras that are not baked into the image. For example, the documented kit-less command uv run --extra leapp ... reaches code that imports leapp, but the image installs only the all, importers, and test extras, and leapp is not part of all. The command therefore fails because the dependency is missing. Preserve no-sync behavior for the preinstalled workflow without disabling documented extra selection for every container command.
There was a problem hiding this comment.
Fixed in 5174febe85c. Confirmed the failure first — in the built image uv run --extra leapp python -c "import leapp" gave ModuleNotFoundError: No module named 'leapp', and leapp is indeed not in --extra all.
UV_NO_SYNC is no longer set in the image. It moved to the only caller that needs it, the validate action, which is the one place that bind-mounts source/ read-only:
docker run --rm -e UV_NO_SYNC=1 "${mount_args[@]}" "$IMAGE_TAG" ...
Checked the blast radius before moving it: run_tests.sh (multi-GPU smoke, arm-ci) drives the image through ./isaaclab.sh -p, never uv run, so it is unaffected.
After the change, in the image: uv run --extra leapp installs 16 packages and imports cleanly, and with the source mounted read-only plus caller-side UV_NO_SYNC=1, uv run still resolves /opt/isaaclab-venv with no stray .venv.
|
|
||
| # The GPU runners have no uv on PATH; the invariant check below needs it. | ||
| - name: Set up uv | ||
| uses: astral-sh/setup-uv@v6 |
There was a problem hiding this comment.
The new astral-sh/setup-uv execution uses the mutable v6 tag on a self-hosted GPU runner. If that upstream tag is moved or compromised, replacement action code can run on a host that subsequently accesses AWS-backed ECR credentials. Pin this third-party action to a full commit SHA so the selected implementation is immutable.
How this was verified: The action executes on the self-hosted ECR build runner before the same job invokes AWS ECR authentication.
There was a problem hiding this comment.
Not changing this here, and flagging why rather than silently skipping it.
The concern is legitimate — a mutable tag on a self-hosted runner that later authenticates to ECR is real supply-chain exposure. But astral-sh/setup-uv@v6 is unpinned in 8 places across 6 workflows today:
.github/workflows/build.yaml:180,206
.github/workflows/kitless-docker.yml:72,109
.github/workflows/docs.yaml:62,94
.github/workflows/wheel.yml:150
.github/workflows/license-check.yaml:53
Pinning only the line this PR happens to add would leave the same exposure on every other job while making this file inconsistent with its neighbours — including build.yaml:206, which is the step this one was copied from. Repository-wide action pinning is worth doing, but as its own change with a policy decision behind it, not as a side effect of a kit-less Docker fix.
The line itself was added because verify-test-path runs uv run on the runner and the GPU runners have no uv on PATH — build.yaml:204 documents the same requirement.
There was a problem hiding this comment.
That rationale is sound. Pinning only this newly added invocation would leave the same mutable-tag exposure elsewhere and make the workflow inconsistent with the existing setup-uv usage. Repository-wide action pinning should be handled separately with an agreed policy and pinned SHAs; I have no further concern with this PR’s change.
| isaacsim-version: "24.04" | ||
| dockerfile-path: docker/Dockerfile.kitless | ||
| cache-tag: cache-kitless | ||
| verify-test-path: docker/test/test_image_invariants.py |
There was a problem hiding this comment.
Invariant changes skip validation
This adds test_image_invariants.py as a kit-less image verification input without adding that file to the workflow's change-detection patterns. Future changes to these invariants therefore will not trigger the dedicated kit-less build, so updates to the verification contract may not be exercised against the image they validate. Add docker/test/test_image_invariants.py to the patterns.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
There was a problem hiding this comment.
Fixed in 5174febe85c. Added to the change patterns in kitless-docker.yml, alongside the two test files already selected there:
^docker/test/test_image_invariants\.py$ :: Built-image invariant tests
There was a problem hiding this comment.
Isaac Lab Review Bot
The kit-less image now directs uv run to the shipped /opt/isaaclab-venv and adds CI coverage for that behavior. One filesystem-ownership issue remains with the new BuildKit cache mount under the runtime user’s home directory.
- Design and architecture: Collapsing the builder/runtime split prevents the uv environment variables from being lost between stages and aligns runtime
uv runbehavior with the environment installed during the image build. However, placing a root-created cache mount at/home/isaaclab/.cache/uvconflicts with the image’s non-root runtime design unless ownership is restored afterward. - API: The container-facing environment contract is intentionally changed so
uv runreusesVIRTUAL_ENVthroughUV_PROJECT_ENVIRONMENT, whileUV_NO_SYNCprevents runtime editable-package rebuilds. Existing non-root identity, paths, working directory, and command remain preserved. - Implementation: The dependency installation and regression checks cover the intended uv environment resolution, but the BuildKit cache mount can leave
${HOME}/.cache/uvroot-owned. Runtime uv commands executed asisaaclabmay then be unable to use their default cache. Move the build cache outside${HOME}or restore ownership after the cache-mounted RUN.
Minor fixes needed. Posted 1 actionable finding inline.
Automated review; human maintainers own approval decisions.
| # whole cache into the layer. UV_CACHE_DIR pins it so the two cannot drift. | ||
| # ovrtx writes shader caches inside its own site-packages, which a volume-mounted run reaches as | ||
| # the runner's uid rather than the image's. | ||
| RUN --mount=type=cache,target=/home/isaaclab/.cache/uv \ |
There was a problem hiding this comment.
🟡 Warning · Implementation — Cache mount leaves root-owned ${HOME}/.cache
The cache mount target sits inside the runtime user's home, and ${HOME} was already chowned and chmod 750 at lines 70-72 with no later chown. BuildKit creates missing mount-point directories as root, so the image ships a root-owned /home/isaaclab/.cache/uv; runtime uv calls as isaaclab (e.g. the validate action's uv pip install ... pytest) then cannot write their default cache. Point the mount outside ${HOME}, or re-chown ${HOME} after this RUN.
There was a problem hiding this comment.
Fixed in 5174febe85c. Confirmed the mechanism in the built image before changing anything — /home/isaaclab/.cache was root:root 755 while /home/isaaclab is isaaclab:isaaclab 750, exactly as described.
${HOME}/.cache is now pre-created in the runtime-user RUN so BuildKit finds the mount-point parent already present and owned:
&& mkdir -p "${HOME}/.cache" "${VIRTUAL_ENV}" "${ISAACLAB_PATH}" \
&& chown -R isaaclab:isaaclab "${HOME}" \
After the change it is isaaclab:isaaclab 755.
One correction to the impact: runtime uv was not actually broken before this, because .cache was mode 755 and .cache/uv itself was already isaaclab-owned — uv pip install returned rc=0. The defect was real but the consequence was a root-owned directory inside a 750 home rather than a write failure.
Restore the build toolchain for arm64 only. psutil 5.9.8 and pyopengl-accelerate
3.1.10 publish no aarch64 wheels, so uv builds them from sdist there; the earlier
claim that nothing in the lock compiles held only on x86_64, which is the one
architecture it was measured on. Guarded by dpkg --print-architecture as
Dockerfile.base does, so the x86_64 image keeps shipping no compiler.
Stop setting UV_NO_SYNC in the image. It suppressed the editable rebuild that
fails on a bind-mounted source tree, but it also stopped uv run --extra <name>
installing an extra the image does not carry, which breaks the documented
--extra leapp flow. The setting now travels with the caller that mounts the
source read-only, which is the validate action.
Retarget UV_PYTHON at the venv after the sync, as Dockerfile.base and
Dockerfile.curobo already do, so a runtime uv pip install does not resolve to
the externally-managed system interpreter.
Pre-create ${HOME}/.cache so BuildKit cannot leave the cache-mount parent
root-owned inside a 750 home, and select the image invariants in the workflow's
change patterns so an edit to them rebuilds the image they assert against.
|
run-ci |
|
@greptileai review |
| # UV_NO_SYNC is deliberately NOT set here: it would stop ``uv run --extra <name>`` installing an | ||
| # extra the image does not carry, such as the documented ``--extra leapp``. Callers that mount | ||
| # the source read-only pass it themselves. |
There was a problem hiding this comment.
Compose still rebuilds sources
Moving UV_NO_SYNC exclusively into the validation action leaves the documented Compose workflow unprotected. Compose also bind-mounts the repository source tree, but neither its environment nor its container launcher sets UV_NO_SYNC, so uv run attempts to rebuild the editable workspace packages in the host checkout. If that checkout is not writable by the image's fixed uid 1000, the command still fails with the same egg-info: Permission denied error this PR is intended to fix. Preserve optional-extra synchronization while ensuring every bind-mounted workflow suppresses the editable rebuild.
There was a problem hiding this comment.
Correct — confirmed docker/docker-compose.yaml bind-mounts ../source for the kit-less service, and without UV_NO_SYNC a uid-1000 container on a foreign-owned checkout reproduces Cannot update time stamp of directory 'isaaclab_visualizers.egg-info'. 569bd8b4844 restores it in the image; the extras trade-off moves to #7671.
kellyguo11
left a comment
There was a problem hiding this comment.
The root-cause analysis looks sound, but this has expanded beyond the NVBug and the current validation injects a condition absent from the normal user workflow. I left focused inline comments on scope and regression coverage.
| FROM ghcr.io/astral-sh/uv:0.12.9@sha256:8b940d3a9d65bed080436972241af2e21c84b5e8c9193f7014ed71479ee795ff AS uv | ||
|
|
||
| FROM ${BASE_IMAGE_ARG} AS builder | ||
| FROM ${BASE_IMAGE_ARG} |
There was a problem hiding this comment.
Could we keep NVBug 6732972 scoped to propagating the runtime uv environment and split this single-stage rebuild into a follow-up? Converting the image architecture also changes toolchain retention, COPY ownership, caching, and aarch64 build behavior. Those changes are not needed to prove the reported uv run regression is fixed, and they substantially increase the review and backport surface.
There was a problem hiding this comment.
Done — reverted in 569bd8b4844. Back to the two-stage image with the four uv settings and the uv run check: 3 files, +38 lines. Single stage, cache mount, arm64 deps and image invariants moved to #7671 (draft). Agreed on backport surface especially.
|
|
||
| # uv reads UV_PROJECT_ENVIRONMENT, never VIRTUAL_ENV. UV_PYTHON overrides pyproject's | ||
| # ``python-preference = "only-managed"``, which would fetch a CPython and rebuild the venv. | ||
| # UV_NO_SYNC is deliberately NOT set here: it would stop ``uv run --extra <name>`` installing an |
There was a problem hiding this comment.
The PR description says UV_NO_SYNC is required and reports that the other three settings alone still reproduced the permission error, but the current image deliberately omits it. Please reconcile the implementation and description and demonstrate that the normal kit-less user path succeeds without an injected variable. Otherwise this revision may still allow uv run to rebuild editable packages in a bind-mounted source tree.
There was a problem hiding this comment.
Resolved by the revert: ENV UV_NO_SYNC=1 is back at docker/Dockerfile.kitless:121, so "all four are required" is accurate again.
Omitting it was a real defect. Measured with no injected variable, source bind-mounted, container uid 1000 against a host checkout owned by another uid:
error: Cannot update time stamp of directory 'isaaclab_visualizers.egg-info'
docker/docker-compose.yaml bind-mounts ../source for this service, so Compose users would have hit it. With the setting restored, the same call resolves /opt/isaaclab-venv and creates no .venv.
| # UV_NO_SYNC belongs to the caller that mounts the source read-only, not to the image: in | ||
| # the image it would also stop ``uv run --extra <name>`` installing an extra the image does | ||
| # not carry. Without it the editable rebuild fails on the read-only mount. | ||
| docker run --rm -e UV_NO_SYNC=1 "${mount_args[@]}" "$IMAGE_TAG" \ |
There was a problem hiding this comment.
Injecting UV_NO_SYNC=1 here makes the check pass under a condition the image and Compose user path do not provide, so it can mask the original regression. Please exercise QA's exact uv run --extra ov isaaclab train ... --max_iterations 5 command under the shipped environment without this override, or add the setting to the actual user path. The regression test should fail whenever the documented workflow would still attempt editable rebuilds.
There was a problem hiding this comment.
Agreed — the injection was in the reverted commits, so the action runs uv run --extra ov under the shipped environment with no override, and fails if the image stops suppressing the rebuild.
Your QA-command point stands: the check asserts sys.prefix and the absence of ${ISAACLAB_PATH}/.venv, not a full isaaclab train ... --max_iterations 5. Say if you want that here rather than #7671.
Reverts 4093547, 0757d7c and 5174feb, leaving the four runtime uv settings and the uv run regression check that nvbugs 6732972 needs. Converting the image to a single stage also changed toolchain retention, COPY ownership, build caching and aarch64 behaviour. None of that is required to fix the reported regression, and it widened the review and backport surface of a change that is meant to land on the active release branch. The single-stage work, the uv cache mount, the arm64 build dependencies and the built-image invariants move to a follow-up PR.
|
@greptileai review |
|
run-ci |
|
Backported to |
…7628) # Description `uv run` inside the kit-less container ignored the environment the image ships and built a second one from scratch. Where the source tree is bind-mounted from the host — Compose, CI, and the documented developer workflow all do this — the editable rebuild wrote `egg-info` into files the image's uid cannot touch and the command died with `Permission denied`. Reported as NVBug 6732972 against Isaac Sim 6.1.0 rc26 / Isaac Lab 3.0.0 EA, on both x86_64 and aarch64. ## 1. Summary * `uv run isaaclab train …` works inside the kit-less image again, instead of failing with `could not create '<package>.egg-info': Permission denied`. * Removes a silent 188-package reinstall (plus a downloaded CPython) from every in-container `uv` invocation. ## 2. Cause `docker/Dockerfile.kitless` is the only multi-stage image in the repo, and `ENV` does not cross stages. Its `builder` stage sets `UV_PROJECT_ENVIRONMENT`, `UV_PYTHON` and `UV_PYTHON_PREFERENCE`; the `runtime` stage sets neither. `Dockerfile.base` and `Dockerfile.curobo` are single-stage, so their copies survive to runtime — which is why this reproduces only on the kit-less image. uv resolves the project environment from `UV_PROJECT_ENVIRONMENT` alone and does not honour `VIRTUAL_ENV`, so every in-container `uv run` emitted ``` warning: `VIRTUAL_ENV=/opt/isaaclab-venv` does not match the project environment path `.venv` and will be ignored; use `--active` to target the active environment instead ``` and then built `/workspace/isaaclab/.venv` from the lock, rebuilding all 12 editable path packages in place under `source/`. Introduced by #6946, which moved the runtime venv from `${ISAACLAB_PATH}/.venv` — uv's own default project path, which is why this worked before — to `/opt/isaaclab-venv`. ## 3. Fix Four `ENV` lines in the runtime stage: | Variable | Without it | |---|---| | `UV_PROJECT_ENVIRONMENT` | uv targets `${ISAACLAB_PATH}/.venv` and installs 188 packages | | `UV_PYTHON`, `UV_PYTHON_PREFERENCE` | `pyproject` pins `python-preference = "only-managed"`, so uv downloads its own CPython and **replaces** the venv rather than reusing it | | `UV_NO_SYNC` | `uv run` still rebuilds the 12 editable packages in place, so the `egg-info` write still fails on a mounted source tree | All four are required: the first three alone leave the reported failure intact (§4). ## 4. Validation Measured against a rebuilt image, with `source/` bind-mounted read-only from the host as CI does. QA's verbatim command, on an 8x NVIDIA L40 node: ``` uv run --extra ov isaaclab train --rl_library rsl_rl --task Isaac-Cartpole-Camera-Direct \ physics=ovphysx renderer=ovrtx presets=rgb --max_iterations 5 ``` rc=0, 0 tracebacks, 0 `egg-info` errors, 0 uv project-environment warnings, `Training time: 85.08 seconds`. This is QA's SMOKE TEST at their `--max_iterations 5`, kept verbatim as the bug repro; its reward and success-rate numbers are artifacts of a 5-iteration run and are not policy results. Arm-by-arm, same image and mounts: | Image | `uv run --extra ov …` | |---|---| | unfixed | fails — `egg-info … Permission denied` | | + `UV_PROJECT_ENVIRONMENT`, `UV_PYTHON`, `UV_PYTHON_PREFERENCE` | fails — same error | | + `UV_NO_SYNC` (this PR) | rc=0, `sys.prefix=/opt/isaaclab-venv`, no `.venv` created | `pre-commit run --all-files` clean; `docker/test` 38 passed, 1 skipped. ## 5. Test added Every step in `.github/actions/validate-kitless-image/action.yml` invoked `python` directly or `uv pip install --python "$VIRTUAL_ENV/bin/python"`; nothing ran `uv run`, which is why CI stayed green while the documented user command was broken. This PR adds a `uv run` step asserting `sys.prefix == $VIRTUAL_ENV` and that no `${ISAACLAB_PATH}/.venv` appears. Verified to fail on the unfixed image and pass on the fixed one. `kitless-docker.yml`'s change gate already lists both `docker/Dockerfile.kitless` and `.github/actions/validate-kitless-image/`, so this PR builds and validates the image with no workflow changes. ## Type of change - Bug fix (non-breaking change which fixes an issue) ## Release backport - [x] <!-- backport-active-release --> Backport this pull request to the active release branch after it merges into `develop` ## Checklist Docker and GPU tests run on demand. Push the commits you want tested, then comment `run-ci` on the pull request. - [x] I have read and understood the [contribution guidelines](https://isaac-sim.github.io/IsaacLab/main/source/refs/contributing.html) - [x] I have run the [`pre-commit` checks](https://pre-commit.com/) with `./isaaclab.sh --format` - [x] No documentation change is required — the documented commands are unchanged; this restores them working inside the image - [x] My changes generate no new warnings — it removes one uv emitted on every in-container run - [x] I have added tests that prove my fix is effective or that my feature works - [x] I have added a changelog fragment under `source/<pkg>/changelog.d/` for every touched package (do **not** edit `CHANGELOG.rst` or bump `extension.toml` — CI handles that) - [x] I have added my name to the `CONTRIBUTORS.md` or my name already exists there (cherry picked from commit 28d3527)
Description
uv runinside the kit-less container ignored the environment the image ships and built asecond one from scratch. Where the source tree is bind-mounted from the host — Compose,
CI, and the documented developer workflow all do this — the editable rebuild wrote
egg-infointo files the image's uid cannot touch and the command died withPermission denied. Reported as NVBug 6732972 against Isaac Sim 6.1.0 rc26 /Isaac Lab 3.0.0 EA, on both x86_64 and aarch64.
1. Summary
uv run isaaclab train …works inside the kit-less image again, instead of failing withcould not create '<package>.egg-info': Permission denied.uvinvocation.2. Cause
docker/Dockerfile.kitlessis the only multi-stage image in the repo, andENVdoes notcross stages. Its
builderstage setsUV_PROJECT_ENVIRONMENT,UV_PYTHONandUV_PYTHON_PREFERENCE; theruntimestage sets neither.Dockerfile.baseandDockerfile.curoboare single-stage, so their copies survive to runtime — which is why thisreproduces only on the kit-less image.
uv resolves the project environment from
UV_PROJECT_ENVIRONMENTalone and does not honourVIRTUAL_ENV, so every in-containeruv runemittedand then built
/workspace/isaaclab/.venvfrom the lock, rebuilding all 12 editable pathpackages in place under
source/.Introduced by #6946, which moved the runtime venv from
${ISAACLAB_PATH}/.venv— uv's owndefault project path, which is why this worked before — to
/opt/isaaclab-venv.3. Fix
Four
ENVlines in the runtime stage:UV_PROJECT_ENVIRONMENT${ISAACLAB_PATH}/.venvand installs 188 packagesUV_PYTHON,UV_PYTHON_PREFERENCEpyprojectpinspython-preference = "only-managed", so uv downloads its own CPython and replaces the venv rather than reusing itUV_NO_SYNCuv runstill rebuilds the 12 editable packages in place, so theegg-infowrite still fails on a mounted source treeAll four are required: the first three alone leave the reported failure intact (§4).
4. Validation
Measured against a rebuilt image, with
source/bind-mounted read-only from the host as CIdoes. QA's verbatim command, on an 8x NVIDIA L40 node:
rc=0, 0 tracebacks, 0
egg-infoerrors, 0 uv project-environment warnings,Training time: 85.08 seconds. This is QA's SMOKE TEST at their--max_iterations 5, keptverbatim as the bug repro; its reward and success-rate numbers are artifacts of a 5-iteration
run and are not policy results.
Arm-by-arm, same image and mounts:
uv run --extra ov …egg-info … Permission deniedUV_PROJECT_ENVIRONMENT,UV_PYTHON,UV_PYTHON_PREFERENCEUV_NO_SYNC(this PR)sys.prefix=/opt/isaaclab-venv, no.venvcreatedpre-commit run --all-filesclean;docker/test38 passed, 1 skipped.5. Test added
Every step in
.github/actions/validate-kitless-image/action.ymlinvokedpythondirectly oruv pip install --python "$VIRTUAL_ENV/bin/python"; nothing ranuv run, which is why CIstayed green while the documented user command was broken. This PR adds a
uv runstepasserting
sys.prefix == $VIRTUAL_ENVand that no${ISAACLAB_PATH}/.venvappears. Verifiedto fail on the unfixed image and pass on the fixed one.
kitless-docker.yml's change gate already lists bothdocker/Dockerfile.kitlessand.github/actions/validate-kitless-image/, so this PR builds and validates the image with noworkflow changes.
Type of change
Release backport
developChecklist
Docker and GPU tests run on demand. Push the commits you want tested, then
comment
run-cion the pull request.pre-commitchecks with./isaaclab.sh --formatsource/<pkg>/changelog.d/for every touched package (do not editCHANGELOG.rstor bumpextension.toml— CI handles that)CONTRIBUTORS.mdor my name already exists there