[Docker] Install the Docker images from uv.lock - #7405
Conversation
github-actions
Bot
added
documentation
|
@StafaH when you got time. |
|
run-ci |
isaaclab-bot
Bot
added
ci:run-docker
hujc7
changed the title
Greptile SummaryThe PR changes the Isaac Sim and kit-less Docker images to install reproducible dependency sets from
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains. Important Files Changed
Reviews (2): Last reviewed commit: "Contain the lock install to the Docker i..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| for name in optional_submodules: | ||
| if name not in LOCK_EXTRAS_BY_FEATURE: | ||
| continue |
There was a problem hiding this comment.
When a uv-managed environment runs the default/all install or requests mimic or teleop, _lock_install_extras silently skips these optional submodules because they are absent from LOCK_EXTRAS_BY_FEATURE. Their editable packages are installed without dependencies such as robomimic, ipywidgets, isaacteleop, and dex-retargeting, causing the selected workflows to fail on missing imports or functionality.
There was a problem hiding this comment.
Fixed in 77136b4d201, now moot — install.py is out of this PR.
| if not _lock_install_available(get_pip_command(python_exe)): | ||
| return None | ||
| extras = _lock_install_extras([name for name, _ in extra_features], optional_submodules, install_isaacsim) |
There was a problem hiding this comment.
When a uv-managed environment requests a selective token such as rl[rsl-rl], visualizer[rerun], ov[ovrtx], or contrib[rlinf], this call removes the selector before resolving lock extras. RL, visualizer, and OV requests therefore install every sibling implementation, while contrib[rlinf] installs none of the RLinF dependencies and leaves that requested workflow unusable.
There was a problem hiding this comment.
Correct; moot — install.py is out of this PR. Selector parity tracked for the follow-up.
![isaaclab-review-bot[bot]](https://avatars.githubusercontent.com/u/157846462?s=60&v=4){kind=small}
There was a problem hiding this comment.
Isaac Lab Review Bot
The Docker migration to a locked /opt/isaaclab-venv is internally consistent, but the new isaaclab.sh --install lock branch does not reliably target the repository and selected environment, and it drops the existing per-token selector semantics.
- Design and architecture: Separating the locked and pip installation flows is reasonable, and placing the Docker venv outside
ISAACLAB_PATHavoids bind-mount masking. However, the lock branch currently relies on uv's current-directory project discovery rather than the already-resolved Isaac Lab root and Python target, making the dispatch less robust than the preserved pip branch. - API: The existing
--install <tokens>interface still accepts selectors, but uv-managed environments no longer honor them consistently. For example,rl[skrl]expands to every mapped RL extra, while optional-submodule selectors are omitted entirely. Non-empty selectors should be translated exactly or delegated to the pip path. - Implementation:
_install_from_lockinvokes bareuv synceven though availability was established usingISAACLAB_ROOT / "uv.lock"and a resolvedpython_exe. It should explicitly select the Isaac Lab project and intended environment. The Dockerfiles already demonstrate the project-directory requirement by changing toISAACLAB_PATHbefore invoking the installer.
Minor fixes needed. Posted 2 actionable findings inline.
Automated review; human maintainers own approval decisions.
| leaves packages the lock does not name (a seeded pip, a source-built Isaac Sim) in place | ||
| instead of removing them. | ||
| """ | ||
| command = ["uv", "sync", "--frozen", "--inexact"] |
There was a problem hiding this comment.
🟡 Warning · Implementation — uv sync is not targeted at project or venv
_install_from_lock runs a bare uv sync with no --project/--directory and no interpreter targeting, although the gate keyed off ISAACLAB_ROOT / "uv.lock" and python_exe. uv discovers the project from the caller's cwd and syncs UV_PROJECT_ENVIRONMENT or <project>/.venv, so invoking the installer from another directory, or with a venv activated outside the repo, can sync the wrong project or environment. Dockerfile.base needed cd ${ISAACLAB_PATH} for this reason.
There was a problem hiding this comment.
Correct; moot — install.py is out of this PR, the Dockerfile runs uv sync directly.
| """ | ||
| if not _lock_install_available(get_pip_command(python_exe)): | ||
| return None | ||
| extras = _lock_install_extras([name for name, _ in extra_features], optional_submodules, install_isaacsim) |
There was a problem hiding this comment.
🟡 Warning · Api — Lock path discards install-token selectors
_resolve_lock_extras keeps only name from each (name, selector) pair and never receives optional_submodule_extra_dependencies. On a uv venv, --install rl[skrl] now installs sb3, rl-games and rsl-rl as well, and a submodule selector is dropped entirely, whereas _install_with_pip forwards each selector to _install_extra_feature. Map selectors to their lock extras, or return None so the pip path handles any non-empty selector.
There was a problem hiding this comment.
Correct; moot — install.py is out of this PR.
Adds a composable teleop-no-isaacsim extra so the lock path selects the teleop requirements without pulling the isaacsim wheel into an image that already ships Kit. Derives the submodule extras from OPTIONAL_SUBMODULE_ROOT_EXTRAS instead of a second hardcoded table, and falls back to pip rather than installing less than was asked for. Pins the uv pip calls to the venv: UV_PYTHON selects the target environment, so they were writing into Isaac Sim's site-packages.
|
[AI-generated review] I am concerned that this Docker-focused change expands into the behavior of the general Could the lock-based installation be contained to the Docker build, retaining only the supporting |
| interpreter or a bare system Python; those keep the pip path. ``get_pip_command`` | ||
| already resolves to uv only for a virtualenv target, so it is the gate here. | ||
| """ | ||
| return pip_cmd[0] == "uv" and (ISAACLAB_ROOT / "uv.lock").is_file() |
There was a problem hiding this comment.
[AI-generated review comment]
This gate is broader than the Docker use case. get_pip_command() also returns uv for Conda and arbitrary virtualenvs when uv is on PATH, so they enter the new lock path even though the changelog says other environments retain pip. Current x86 CI shows the consequence: uv warns that the selected environment is ignored, and the Conda/venv tests subsequently cannot import isaaclab or warp.
Given the Docker-focused goal, could this path require an explicitly configured Docker/project environment, leaving the existing pip dispatch unchanged elsewhere?
There was a problem hiding this comment.
Confirmed. utils.py:307 includes CONDA_PREFIX in in_venv, so the gate caught Conda; the docstring saying otherwise was wrong. Reverted.
| leaves packages the lock does not name (a seeded pip, a source-built Isaac Sim) in place | ||
| instead of removing them. | ||
| """ | ||
| command = ["uv", "sync", "--frozen", "--inexact"] |
There was a problem hiding this comment.
[AI-generated review comment]
This command relies on the caller already being in ISAACLAB_ROOT. That is true in the Docker RUN cd ... step, but isaaclab.sh itself does not change directories. Calling the absolute launcher from another directory now fails because uv cannot find pyproject.toml.
Keeping uv sync inside the Dockerfile avoids changing that public CLI contract. If the generic lock path is retained, pass an explicit --project/--directory and explicitly target the intended environment.
There was a problem hiding this comment.
Correct — the RUN cd ${ISAACLAB_PATH} was the tell. Reverted; the Dockerfile runs uv sync itself. --project/--python if the CLI path returns.
| """ | ||
| if not _lock_install_available(get_pip_command(python_exe)): | ||
| return None | ||
| extras = _lock_install_extras([name for name, _ in extra_features], optional_submodules, install_isaacsim) |
There was a problem hiding this comment.
[AI-generated review comment]
Dropping the selectors here changes existing CLI semantics in uv environments: contrib[rlinf] resolves to no extra, while rl[rsl-rl], visualizer[rerun], and ov[ovrtx] expand to every mapped sibling. This generalized selector translation is another source of complexity that is not needed if lock-based syncing remains Docker-specific. If the generic path stays, the mapper needs selector parity with the pip path and should fall back when an exact mapping is unavailable.
There was a problem hiding this comment.
Confirmed — the selector was dropped outright. Reverted; parity is a follow-up item.
Reverts the install CLI lock path: it also caught Conda environments, where uv sync targets the project environment rather than the active one, breaking the install_ci Conda tests. The images control VIRTUAL_ENV and UV_PROJECT_ENVIRONMENT themselves, so Dockerfile.base syncs the lock directly through a named kit-image extra. imageio-ffmpeg is excluded during the sync instead of uninstalled after.
…-docker-install # Conflicts: # docker/Dockerfile.kitless # uv.lock
|
run-ci |
isaaclab-bot
Bot
added
ci:run-docker
|
run-ci |
The downloaded-package guard rejects any active virtual environment, but the property that matters is which interpreter runs Kit's extension modules. A virtual environment reuses the interpreter it was created from, so one created on the package's own Python runs that exact binary; the conda breakages the guard exists for come from environments that supply their own interpreter and native libraries instead. Compare the interpreter against the Isaac Sim directory rather than testing for an active environment, in both the launcher and the CLI. Anything that cannot be resolved stays rejected, so conda and foreign-interpreter environments are unaffected. isaaclab.bat keeps the broader check: narrowing it needs junction resolution and case-insensitive prefix matching that cannot be exercised here.
The images installed with uv 0.9.25 while uv.lock is generated by 0.12.9, so a lock refresh inside a container rewrites the environment markers the refresh in isaac-sim#7492 removed. Move all three images to the pinned 0.12.9 digest. Assert the pin across every Dockerfile rather than inside the kit-less test, so the three copies cannot drift apart again.
… into jichuanh/uv-lock-docker-install
The merge text-combined uv.lock, which no resolution produces. Regenerate it with the pinned uv 0.12.9 so the workspace member versions match the bump.
isaac-sim#7405 landed the uv.lock-based image install this branch was stacked on, so the Docker, extras and CLI work now comes from develop. Take develop's version of every file that PR owned, keeping only the Newton pin. Regenerate uv.lock with the pinned uv 0.12.9 rather than resolving the merge by hand: the branch was locked with an older uv whose redundant environment markers made the lock read as 2241 changed lines. The relock changes one package.
Review feedback on isaac-sim#7405: the images went through ``isaaclab.sh -p`` for every Python invocation. Putting the environment on PATH makes the console script directly callable, so the call sites read ``isaaclab -p`` -- shorter than the launcher path, with no wrapper. ``uv run`` was the other option but needs ``--frozen --no-sync --project`` to match: without them it re-resolves the project on every call, and it warns when invoked outside the project directory. Measured 124 ms against 110 ms per call. ``install_deps.py apt`` moves below the workspace install. It ran before it, where the console script cannot import its own package, and nothing between the two steps needs apt first: the third-party packages are already installed and the workspace members are pure-Python editables. Two aliases go with it. ``isaaclab`` shadowed the console script now on PATH, and ``tensorboard`` was an explicit path into the environment. ``isaaclab.sh`` itself still ships and is unchanged; this only changes how the images reach it.
…m only in Kit images (#7540) ## Summary Every first CI run of a new commit on a dependency-stable branch has failed since #7405 landed (develop included): `Build Base Docker Image` dies in 20 s and skips the 17 test jobs behind it, and `Multi-GPU training smoke (kit-less)` exits before pytest. Both are CI plumbing, fixed here. # Description ## 1. Build Base Docker Image #7405 added a "Verify image invariants" step that runs `docker image inspect` on the commit tag after `ecr-build-push-pull`. The action leaves an image on the runner only after a full build or an exact-tag pull. On a deps-cache hit it just aliases the commit tag onto the `deps-<hash>` manifest in ECR and pulls nothing, so the inspect fails: ``` Error response from daemon: No such image: isaac-lab-ci:develop-8ca264bd8e5528d7d9b7b6f58a9d8cd051c5933a ``` develop runs 33733992373, 33740164999 and 33781460422 fail this way; PRs pass only after a deps change (full build) or on a re-run of an already built head (exact-tag pull). The step also ran too late to be a gate. The action pushes the commit tag and the deps tag inside itself, so an image that fails the check is already published. That is not hypothetical: on `newton-world-prefixes` (run 33735349483) `deps-6aebe721c56a91f7` was pushed at 09:05:07 and the invariant failed at 09:05:11, leaving an image whose Isaac Sim extensions cannot load in the shared cache. Any later run whose install inputs hash the same inherits it. Fix: move the assertion into the action, before every push, behind a `verify-command` input that the base image job passes. A failure now publishes nothing, so the next run rebuilds and fails again rather than serving the bad image from the cache. Skipping it on a cache hit is then correct by construction — every published image passed when it was built, and the deps hash covers exactly the install inputs the invariant asserts against. ## 2. Multi-GPU training smoke (kit-less) `run_tests.sh` bind-mounts nine writable runtime directories under `/isaac-sim/…` into every container, so `/isaac-sim` exists as a directory in the kit-less image too, and the container script then runs `ln -s /isaac-sim _isaac_sim` unconditionally. #7466 taught `isaaclab.sh` to treat a `_isaac_sim` directory without a source-build marker as a downloaded Isaac Sim and to refuse it next to an active venv; the kit-less image sets `VIRTUAL_ENV=/opt/isaaclab-venv`, so every run stops with: ``` [ERROR] Downloaded Isaac Sim packages cannot be combined with a Python virtual environment. ``` #7405 reworked the same check (venv-on-bundled-Python exemption) and the run-tests script without exempting the kit-less lane. Every PR's smoke run since 11:43Z on 2026-09-03 fails identically. Fix: plant the link only when `/isaac-sim/python.sh` exists. Kit images are unchanged; the kit-less image resolves its interpreter through `VIRTUAL_ENV`, as it did before #7466. ## 3. Audit of every `_isaac_sim` site Every place that creates the link, and how it interacts with the guard: | Site | Creates | Verdict | | --- | --- | --- | | `run-tests/run_tests.sh` | `/isaac-sim` in every image | **Fixed here**: guarded on `python.sh` | | `multi-gpu/multi_gpu_host_launcher.sh` | `/isaac-sim`, unguarded | Correct today: its only caller passes `CI_IMAGE_TAG`, the Kit image. The kit-less lane goes through `run-tests` | | `Dockerfile.base`, `Dockerfile.curobo` | `ISAACSIM_ROOT_PATH`, alongside `VIRTUAL_ENV` | Correct: `UV_PYTHON=${ISAACSIM_ROOT_PATH}/kit/python/bin/python3`, so the venv is on the bundled Python and #7405's exemption applies | | `Dockerfile.kitless` | never; asserts `test ! -e "${ISAACLAB_PATH}/_isaac_sim"` | Correct | | `cli/commands/misc.py` (`--isaacsim_source`) | source-build link | Correct: writes `.isaaclab_source_build` | | `docs/.../installation/index.rst` ×3 | user-run `ln -s` | Out of scope, see below | ## 4. Verification - `docker/test/test_container_profiles.py` gains two regression tests — one pinning the kit-less guard, one pinning that verification precedes both push steps. Each fails without its fix. 15 pass in that file, 20 with `test_carb_env_shim.py`. - `yaml.safe_load` on both YAML files, `bash -n` on the script, pre-commit on the changed files. - This PR touches `.github/actions/{ecr-build-push-pull,run-tests}/` and `build.yaml`, which the detect-changes patterns of both workflows match, so its own CI exercises both paths: the base image job takes the deps-cache-hit path (no dependency change here) and the kit-less smoke job runs. ## 5. Out of scope Three checkers implement the same "downloaded Isaac Sim vs. virtual environment" rule and #7405 updated only two, leaving user-facing divergences that this PR does not touch: - `isaaclab.bat` has neither the venv-on-bundled-Python exemption nor the `--isaacsim_source` escape, so on Windows the remedy the Linux error message recommends is still rejected. - `cli/commands/envs.py::_reject_downloaded_isaac_sim` has no exemption either, so `isaaclab -c/-u` rejects a venv created on the bundled Python that `isaaclab.sh` accepts. - The docs link a *source build* with a bare `ln -s`, which leaves no `.isaaclab_source_build` marker, so the same tree is treated as "downloaded" unless linked via `--isaacsim_source`. `isaaclab.sh` also lacks the `python.sh` precondition its Python twin (`cli/utils.py`) has, but that is deliberate: `test_launcher_rejects_downloaded_isaac_sim_with_active_environment` pins the fail-closed behaviour on a bare directory. That is why the fix here is in the CI script. ## 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 - [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] I have made corresponding changes to the documentation (the action README documents the new output) - [x] My changes generate no new warnings - [ ] Tests: CI-only change; the workflows' own runs on this PR are the test - [ ] Changelog: no source package changed - [x] I have added my name to the `CONTRIBUTORS.md` or my name already exists there
# Description Fixes two release CI failures without backporting #7405. ## Kitless multi-GPU tests Backports only the release-relevant multi-GPU test fix from #7540. PR #7466 is present on `release/3.0.0`, so `isaaclab.sh` rejects a downloaded-Isaac-Sim link alongside the kitless image's active virtual environment. The test runner currently creates `_isaac_sim -> /isaac-sim` unconditionally because runtime mounts make `/isaac-sim` exist even in the kitless image. This change creates the link only when `/isaac-sim/python.sh` exists. Kit-based test images retain their existing behavior, while the kitless multi-GPU smoke test resolves Python from `VIRTUAL_ENV`. This intentionally excludes #7540's image-invariant/cache changes and all of #7405, which was not backported to the release branch. Original PR: #7540 ## Documentation link check The automatic backport's link check failed twice because `www.ros.org` and `docs.ros.org` return HTTP 403 to the GitHub runner. Both links remain valid. The link-check workflow already excludes known crawl blockers, so this adds a single `ros.org` exclusion covering both hosts without changing the documentation destinations or weakening checks for other domains. Failed run: https://github.com/isaac-sim/IsaacLab/actions/runs/33828925399 ## Type of change - Bug fix (non-breaking change which fixes an issue) ## Release backport Not applicable: this PR directly targets `release/3.0.0`. ## Validation - Added a regression test that failed against the unmodified release branch and passes with the multi-GPU fix. - `uv run --no-project --with pytest --with pyyaml python -m pytest docker/test/test_container_profiles.py -q` - 14 passed. - `bash -n .github/actions/run-tests/run_tests.sh` - passed. - Parsed `.github/workflows/check-links.yml` with PyYAML and verified the exclusion matches both failing URLs. - Applicable file-scoped pre-commit hooks - passed. The branch-wide changelog hook is not applicable to a release backport because it compares historical release differences against `develop`. - The canonical `uv run isaaclab -f` command cannot resolve this branch's Linux/Windows-only lockfile on macOS; the equivalent file-scoped hooks were run directly. ## Checklist - [x] I have read and understood the contribution guidelines. - [x] I have run the applicable pre-commit checks. - [x] Documentation destinations remain unchanged. - [x] My changes generate no new warnings. - [x] I have added a regression test for the multi-GPU behavior. - [x] No changelog fragment is required because no source package changed. - [x] The contributors already exist in `CONTRIBUTORS.md`. --------- Co-authored-by: hujc <jichuanh@nvidia.com>
Drop two fragments left over from isaac-sim#7405, which merged: the docker and CLI changes they described are no longer part of this branch. Rename the isaaclab_newton fragment to describe the change that is actually here. The P-ADMM solver-combination fix now comes from develop, so the only remaining change in that package is the test update for Newton's new drive class names. State why the manipulation case asks for run_to_run rather than implying it matches the cartpole cases, which request the stronger gpu_to_gpu.
# Description Fix the scheduled multi-version documentation build on `release/3.0.0-beta2`. The release workflow checks out `develop` but installs only the `test` extra. Since #7405 moved Sphinx and `sphinx-multiversion` into the `dev` extra on `develop`, the build fails with `sphinx-multiversion: not found`. This changes the workflow to install the `dev` extra before running `make multi-docs`. Failed run: https://github.com/isaac-sim/IsaacLab/actions/runs/33829027682/job/100887817207 No new dependencies are introduced. ## Type of change - Bug fix (non-breaking change which fixes an issue) ## Release backport - [ ] This PR already targets `release/3.0.0-beta2`; no additional backport is requested. ## Screenshots Not applicable. ## Validation - Parsed `.github/workflows/docs.yaml` and asserted that the multi-version job installs `dev`, not `test`. - `pre-commit run --files .github/workflows/docs.yaml` passed all applicable hooks. - `uv run isaaclab -f` was attempted but dependency resolution cannot install the release branch's Linux/Windows-only `omniverseclient==2.71.1.7015` wheel on macOS ARM64. ## Checklist - [x] I have read and understood the contribution guidelines - [x] Applicable pre-commit checks pass - [x] No documentation content change is required - [x] My changes generate no new warnings - [x] Targeted workflow validation covers the fix - [x] No source package changed, so no changelog fragment is required - [x] My name is already in `CONTRIBUTORS.md`
6 participants
Summary
Docker images stop mutating Isaac Sim's shipped Python environment: the previous build
replaced 41 of its packages, this one replaces 0. All three images install from
uv.lock, so builds are reproducible and[tool.uv].override-dependenciesapply.Description
The lock is the only path that applies
[tool.uv].override-dependencies, which pip cannotread. The change is contained to the Docker build;
isaaclab.sh --installis unchanged.Isaac Sim image —
uv syncresolves into a venv at/opt/isaaclab-venv, built on Kit'sown interpreter. Isaac Sim is untouched, still reached through the existing
_isaac_simsymlink, and
isaaclab.shsourcessetup_python_env.shsoimport isaacsimkeeps working.Kit-less image —
uv syncreplaces the pip-style install, retiring the duplicatedtools/wheel_builder/uv-overrides.txt.cuRobo image — same migration. Because Isaac Sim's site-packages is no longer touched,
the image stops deleting the prebundled torch, re-bootstrapping
pipwithget-pip.py, anduninstalling
quadprog. cuRobo is still built from its pinned commit.No new extras — the images use the ones that already exist.
teleopno longer bundlesthe
isaacsimwheel, so environments that already provide Kit do not install a second,version-mismatched copy; installs that need it use
--extra teleop,isaacsim, and the docssay so wherever XR teleop appears (agreed with the teleop owner).
testis now the testsuite alone and
devadds the documentation toolchain, so the images can installtestwithout shipping Sphinx and its GPL-3.0-or-later
docutils, which the Docker licence gaterejects.
Image invariant test — the images install with
uv sync, which never runscommand_install's dangling-symlink guard, so [Fix] Stop docker installs breaking Isaac Sim extension startup (prebundle deletion, pin-pink) #6329's protection would have been lost. Apost-build step asserts no prebundled package lost its
__init__.py, bound to the digestjust built. It has already caught a real regression on a dependent branch.
Removed the
quadproguninstall (absent from the lock), thetomlinstall(
install_deps.pynow uses stdlibtomllib), and theimageio-ffmpeguninstall(excluded during the sync instead).
Newton resolves to 1.5.0 from the lock's pinned commit where the pip path floated to
release-1.5HEAD (1.5.1); regeneratinguv.lockis the lever if a newer commit is needed.uv.lockis regenerated with uv 0.9.25, the version the images pin. Newer uv renormalisesenvironment markers and rewrites ~2200 lines with no semantic change.
Type of change
Release backport
developChecklist
pre-commitchecks with./isaaclab.sh --formatsource/<pkg>/changelog.d/for every touched packageCONTRIBUTORS.mdor my name already exists there