Skip to content

Commit df44b19

Browse files
authored
[CI] Skip the image invariants on a deps-cache hit and link _isaac_sim 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
1 parent 8365c57 commit df44b19

6 files changed

Lines changed: 85 additions & 30 deletions

File tree

.github/actions/ecr-build-push-pull/README.md

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,19 @@ ECR is also used as the BuildKit layer cache.
1616
ecr-url: (optional, complete url for ECR storage)
1717
```
1818
19+
## Verifying a freshly built image
20+
21+
Pass `verify-test-path` to assert against the image before it is published:
22+
23+
```yaml
24+
verify-test-path: docker/test/test_image_invariants.py
25+
```
26+
27+
The tests run only on a full build, with `IMAGE_TAG` set, so the caller's job needs `uv`
28+
(`astral-sh/setup-uv`). A failure fails the action with nothing pushed, so the next run
29+
rebuilds rather than serving the bad image from the deps cache. Exact-tag and deps-cache hits skip
30+
them: that image passed when it was built.
31+
1932
## ECR URL resolution order
2033

2134
1. `ecr-url` input

.github/actions/ecr-build-push-pull/action.yml

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,16 @@ inputs:
3737
description: Tag used for the ECR layer cache image (e.g. "cache-base", "cache-curobo").
3838
required: false
3939
default: 'cache'
40+
verify-test-path:
41+
description: >
42+
Path to a test file or directory asserted against a freshly built image, before it is
43+
tagged or pushed. Tests run with IMAGE_TAG set; a failure fails the action with nothing
44+
published, so the next run rebuilds instead of inheriting the bad image from the cache.
45+
46+
Not run on an exact-tag or deps-cache hit: those serve an image that already passed when it
47+
was built.
48+
required: false
49+
default: ''
4050
pull-on-deps-hit:
4151
description: >
4252
Pull the image locally after a deps-cache hit. Needed by jobs that run
@@ -242,6 +252,23 @@ runs:
242252
cache-to: ${{ steps.resolve-ecr.outputs.available == 'true' && format('type=registry,ref={0},mode=max', env.CACHE_IMAGE) || '' }}
243253
deps-hash: ${{ steps.deps-hash.outputs.hash }}
244254

255+
# Assert against the image while it is only local: the push steps below publish under both
256+
# the commit tag and the deps tag, and a deps-cache hit later serves that image without
257+
# rebuilding it, so anything published unverified stays unverified.
258+
- name: Verify freshly built image
259+
if: >
260+
inputs.verify-test-path != '' &&
261+
steps.pull-exact.outputs.hit != 'true' &&
262+
steps.deps-cache.outputs.deps-cache-hit != 'true'
263+
shell: bash
264+
env:
265+
IMAGE_TAG: ${{ inputs.image-tag }}
266+
TEST_PATH: ${{ inputs.verify-test-path }}
267+
run: |
268+
set -euo pipefail
269+
uv run --no-project --with pytest \
270+
python -m pytest -q "${TEST_PATH}"
271+
245272
- name: Tag built image with ECR-prefixed name
246273
if: >
247274
steps.resolve-ecr.outputs.available == 'true' &&

.github/actions/run-tests/run_tests.sh

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -320,8 +320,11 @@ run_tests() {
320320
set -e
321321
cd /workspace/isaaclab
322322
mkdir -p tests
323-
rm _isaac_sim || true
324-
ln -s /isaac-sim _isaac_sim
323+
# The runtime mounts above create /isaac-sim in every image. Link it only where Kit
324+
# lives there: in the kit-less image the link would read as a downloaded Isaac Sim,
325+
# which isaaclab.sh refuses to combine with the image's VIRTUAL_ENV.
326+
rm -f _isaac_sim
327+
if [ -x /isaac-sim/python.sh ]; then ln -s /isaac-sim _isaac_sim; fi
325328
if [ -n \"\${WARP_CACHE_PATH:-}\" ]; then
326329
./isaaclab.sh -p tools/verify_warp_cache.py
327330
fi

.github/workflows/build.yaml

Lines changed: 4 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -207,6 +207,9 @@ jobs:
207207
with:
208208
enable-cache: true
209209

210+
# #6329 aborts the pip install when it strands a prebundled package's __init__.py
211+
# (nvbugs 6343978: 14 Isaac Sim extensions fail to load). The images install with
212+
# ``uv sync``, which never runs that guard, so assert the same invariant on the image.
210213
- name: Build and push to ECR
211214
uses: ./.github/actions/ecr-build-push-pull
212215
with:
@@ -215,20 +218,7 @@ jobs:
215218
isaacsim-version: ${{ needs.config.outputs.isaacsim_image_tag }}
216219
dockerfile-path: docker/Dockerfile.base
217220
cache-tag: cache-base
218-
219-
# #6329 aborts the pip install when it strands a prebundled package's __init__.py
220-
# (nvbugs 6343978: 14 Isaac Sim extensions fail to load). The images install with
221-
# ``uv sync``, which never runs that guard, so assert the same invariant on the image.
222-
- name: Verify image invariants
223-
shell: bash
224-
env:
225-
IMAGE_TAG: ${{ needs.config.outputs.ci_image_tag }}
226-
run: |
227-
set -euo pipefail
228-
IMAGE_DIGEST="$(docker image inspect --format '{{.Id}}' "${IMAGE_TAG}")"
229-
export IMAGE_DIGEST
230-
uv run --no-project --with pytest \
231-
python -m pytest -q docker/test/test_image_invariants.py
221+
verify-test-path: docker/test/test_image_invariants.py
232222

233223
build-curobo:
234224
name: Build cuRobo Docker Image

docker/test/test_container_profiles.py

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
from docker.utils import ContainerInterface, volume_mounts
1616

1717
DOCKER_DIR = Path(__file__).resolve().parents[1]
18+
REPO_ROOT = DOCKER_DIR.parent
1819

1920

2021
@pytest.fixture
@@ -288,6 +289,41 @@ def test_kitless_compose_service_has_no_isaac_sim_mounts():
288289
assert all("/kit/" not in mount["target"].lower() for mount in mounts)
289290

290291

292+
def test_image_is_verified_before_it_is_published():
293+
"""A published image must be a verified one.
294+
295+
The push steps publish under both the commit tag and the deps tag, and a later deps-cache hit
296+
serves that image without rebuilding it, so anything published unverified stays unverified.
297+
"""
298+
action = yaml.safe_load(
299+
(REPO_ROOT / ".github" / "actions" / "ecr-build-push-pull" / "action.yml").read_text(encoding="utf-8")
300+
)
301+
names = [step["name"] for step in action["runs"]["steps"] if "name" in step]
302+
303+
assert names.index("Verify freshly built image") < names.index("Push to ECR") < names.index("Push deps tag")
304+
305+
build = yaml.safe_load((REPO_ROOT / ".github" / "workflows" / "build.yaml").read_text(encoding="utf-8"))
306+
(base_build,) = [
307+
step for step in build["jobs"]["build"]["steps"] if step.get("uses") == "./.github/actions/ecr-build-push-pull"
308+
]
309+
310+
assert base_build["with"]["verify-test-path"] == "docker/test/test_image_invariants.py"
311+
312+
313+
def test_run_tests_links_isaac_sim_only_where_kit_is_installed():
314+
"""The kit-less image has no Kit under ``/isaac-sim``, which the runtime mounts create anyway.
315+
316+
Linking it as ``_isaac_sim`` there reads as a downloaded Isaac Sim, which ``isaaclab.sh``
317+
refuses to combine with the image's ``VIRTUAL_ENV``.
318+
"""
319+
script = (REPO_ROOT / ".github" / "actions" / "run-tests" / "run_tests.sh").read_text(encoding="utf-8")
320+
321+
link_lines = [line.strip() for line in script.splitlines() if "ln -s /isaac-sim _isaac_sim" in line]
322+
323+
assert link_lines
324+
assert all("/isaac-sim/python.sh" in line for line in link_lines), link_lines
325+
326+
291327
def test_kitless_volume_key_resolves_owned_image_paths(monkeypatch: pytest.MonkeyPatch):
292328
"""The explicit kit-less volume key resolves the paths prepared by its Dockerfile."""
293329
monkeypatch.setenv("DOCKER_ISAACLAB_PATH", "/workspace/isaaclab")

docker/test/test_image_invariants.py

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -38,20 +38,6 @@ def _require_image():
3838
pytest.skip("IMAGE_TAG is unset; no built image to assert against")
3939

4040

41-
def test_image_under_test_is_the_one_that_was_built():
42-
"""Fail loudly rather than assert against whatever a stale tag happens to point at."""
43-
expected = os.environ.get("IMAGE_DIGEST", "")
44-
if not expected:
45-
pytest.skip("IMAGE_DIGEST is unset; cannot bind the tag to a specific image")
46-
actual = subprocess.run(
47-
["docker", "image", "inspect", "--format", "{{.Id}}", IMAGE_TAG],
48-
capture_output=True,
49-
text=True,
50-
check=True,
51-
).stdout.strip()
52-
assert actual == expected, f"{IMAGE_TAG} is {actual}, expected {expected}"
53-
54-
5541
def test_no_prebundled_package_lost_its_entry_point():
5642
"""A dangling ``__init__.py`` in a prebundle stops Isaac Sim extensions loading.
5743

0 commit comments

Comments
 (0)