diff --git a/.github/actions/ecr-build-push-pull/README.md b/.github/actions/ecr-build-push-pull/README.md index 2e306c978b8e..2dea04797126 100644 --- a/.github/actions/ecr-build-push-pull/README.md +++ b/.github/actions/ecr-build-push-pull/README.md @@ -16,6 +16,19 @@ ECR is also used as the BuildKit layer cache. ecr-url: (optional, complete url for ECR storage) ``` +## Verifying a freshly built image + +Pass `verify-test-path` to assert against the image before it is published: + +```yaml + verify-test-path: docker/test/test_image_invariants.py +``` + +The tests run only on a full build, with `IMAGE_TAG` set, so the caller's job needs `uv` +(`astral-sh/setup-uv`). A failure fails the action with nothing pushed, so the next run +rebuilds rather than serving the bad image from the deps cache. Exact-tag and deps-cache hits skip +them: that image passed when it was built. + ## ECR URL resolution order 1. `ecr-url` input diff --git a/.github/actions/ecr-build-push-pull/action.yml b/.github/actions/ecr-build-push-pull/action.yml index b289b4ebb7d8..7c595e7d3407 100644 --- a/.github/actions/ecr-build-push-pull/action.yml +++ b/.github/actions/ecr-build-push-pull/action.yml @@ -37,6 +37,16 @@ inputs: description: Tag used for the ECR layer cache image (e.g. "cache-base", "cache-curobo"). required: false default: 'cache' + verify-test-path: + description: > + Path to a test file or directory asserted against a freshly built image, before it is + tagged or pushed. Tests run with IMAGE_TAG set; a failure fails the action with nothing + published, so the next run rebuilds instead of inheriting the bad image from the cache. + + Not run on an exact-tag or deps-cache hit: those serve an image that already passed when it + was built. + required: false + default: '' pull-on-deps-hit: description: > Pull the image locally after a deps-cache hit. Needed by jobs that run @@ -242,6 +252,23 @@ runs: cache-to: ${{ steps.resolve-ecr.outputs.available == 'true' && format('type=registry,ref={0},mode=max', env.CACHE_IMAGE) || '' }} deps-hash: ${{ steps.deps-hash.outputs.hash }} + # Assert against the image while it is only local: the push steps below publish under both + # the commit tag and the deps tag, and a deps-cache hit later serves that image without + # rebuilding it, so anything published unverified stays unverified. + - name: Verify freshly built image + if: > + inputs.verify-test-path != '' && + steps.pull-exact.outputs.hit != 'true' && + steps.deps-cache.outputs.deps-cache-hit != 'true' + shell: bash + env: + IMAGE_TAG: ${{ inputs.image-tag }} + TEST_PATH: ${{ inputs.verify-test-path }} + run: | + set -euo pipefail + uv run --no-project --with pytest \ + python -m pytest -q "${TEST_PATH}" + - name: Tag built image with ECR-prefixed name if: > steps.resolve-ecr.outputs.available == 'true' && diff --git a/.github/actions/run-tests/run_tests.sh b/.github/actions/run-tests/run_tests.sh index 3779ab10c562..fbccbfa45dd3 100755 --- a/.github/actions/run-tests/run_tests.sh +++ b/.github/actions/run-tests/run_tests.sh @@ -320,8 +320,11 @@ run_tests() { set -e cd /workspace/isaaclab mkdir -p tests - rm _isaac_sim || true - ln -s /isaac-sim _isaac_sim + # The runtime mounts above create /isaac-sim in every image. Link it only where Kit + # lives there: in the kit-less image the link would read as a downloaded Isaac Sim, + # which isaaclab.sh refuses to combine with the image's VIRTUAL_ENV. + rm -f _isaac_sim + if [ -x /isaac-sim/python.sh ]; then ln -s /isaac-sim _isaac_sim; fi if [ -n \"\${WARP_CACHE_PATH:-}\" ]; then ./isaaclab.sh -p tools/verify_warp_cache.py fi diff --git a/.github/workflows/build.yaml b/.github/workflows/build.yaml index 2ff42e847a07..ce2791c8cf84 100644 --- a/.github/workflows/build.yaml +++ b/.github/workflows/build.yaml @@ -182,6 +182,15 @@ jobs: fetch-depth: 1 lfs: true + # The GPU runners have no uv on PATH; the invariant check below needs it. + - name: Set up uv + uses: astral-sh/setup-uv@v6 + with: + enable-cache: true + + # #6329 aborts the pip install when it strands a prebundled package's __init__.py + # (nvbugs 6343978: 14 Isaac Sim extensions fail to load). The images install with + # ``uv sync``, which never runs that guard, so assert the same invariant on the image. - name: Build and push to ECR uses: ./.github/actions/ecr-build-push-pull with: @@ -190,6 +199,7 @@ jobs: isaacsim-version: ${{ needs.config.outputs.isaacsim_image_tag }} dockerfile-path: docker/Dockerfile.base cache-tag: cache-base + verify-test-path: docker/test/test_image_invariants.py build-curobo: name: Build cuRobo Docker Image diff --git a/docker/test/test_container_profiles.py b/docker/test/test_container_profiles.py index 39d39c974829..9a17fc87aab7 100644 --- a/docker/test/test_container_profiles.py +++ b/docker/test/test_container_profiles.py @@ -15,6 +15,7 @@ from docker.utils import ContainerInterface, volume_mounts DOCKER_DIR = Path(__file__).resolve().parents[1] +REPO_ROOT = DOCKER_DIR.parent @pytest.fixture @@ -288,6 +289,41 @@ def test_kitless_compose_service_has_no_isaac_sim_mounts(): assert all("/kit/" not in mount["target"].lower() for mount in mounts) +def test_image_is_verified_before_it_is_published(): + """A published image must be a verified one. + + The push steps publish under both the commit tag and the deps tag, and a later deps-cache hit + serves that image without rebuilding it, so anything published unverified stays unverified. + """ + action = yaml.safe_load( + (REPO_ROOT / ".github" / "actions" / "ecr-build-push-pull" / "action.yml").read_text(encoding="utf-8") + ) + names = [step["name"] for step in action["runs"]["steps"] if "name" in step] + + assert names.index("Verify freshly built image") < names.index("Push to ECR") < names.index("Push deps tag") + + build = yaml.safe_load((REPO_ROOT / ".github" / "workflows" / "build.yaml").read_text(encoding="utf-8")) + (base_build,) = [ + step for step in build["jobs"]["build"]["steps"] if step.get("uses") == "./.github/actions/ecr-build-push-pull" + ] + + assert base_build["with"]["verify-test-path"] == "docker/test/test_image_invariants.py" + + +def test_run_tests_links_isaac_sim_only_where_kit_is_installed(): + """The kit-less image has no Kit under ``/isaac-sim``, which the runtime mounts create anyway. + + Linking it as ``_isaac_sim`` there reads as a downloaded Isaac Sim, which ``isaaclab.sh`` + refuses to combine with the image's ``VIRTUAL_ENV``. + """ + script = (REPO_ROOT / ".github" / "actions" / "run-tests" / "run_tests.sh").read_text(encoding="utf-8") + + link_lines = [line.strip() for line in script.splitlines() if "ln -s /isaac-sim _isaac_sim" in line] + + assert link_lines + assert all("/isaac-sim/python.sh" in line for line in link_lines), link_lines + + def test_kitless_volume_key_resolves_owned_image_paths(monkeypatch: pytest.MonkeyPatch): """The explicit kit-less volume key resolves the paths prepared by its Dockerfile.""" monkeypatch.setenv("DOCKER_ISAACLAB_PATH", "/workspace/isaaclab") diff --git a/docker/test/test_image_invariants.py b/docker/test/test_image_invariants.py new file mode 100644 index 000000000000..74aea9920244 --- /dev/null +++ b/docker/test/test_image_invariants.py @@ -0,0 +1,54 @@ +# Copyright (c) 2022-2026, The Isaac Lab Project Developers (https://github.com/isaac-sim/IsaacLab/blob/main/CONTRIBUTORS.md). +# All rights reserved. +# +# SPDX-License-Identifier: BSD-3-Clause + +"""Invariants asserted against a *built* container image. + +The image under test is named by ``IMAGE_TAG``; the tests skip when it is unset so a plain +``pytest docker/test`` stays green on a machine with no image. Run one explicitly with:: + + IMAGE_TAG=isaac-lab-base:latest pytest docker/test/test_image_invariants.py +""" + +from __future__ import annotations + +import os +import subprocess + +import pytest + +IMAGE_TAG = os.environ.get("IMAGE_TAG", "") + + +def _in_image(script: str) -> str: + """Run ``script`` with bash inside the image under test and return its stdout.""" + result = subprocess.run( + ["docker", "run", "--rm", "--entrypoint", "bash", IMAGE_TAG, "-lc", script], + capture_output=True, + text=True, + check=True, + ) + return result.stdout + + +@pytest.fixture(autouse=True) +def _require_image(): + if not IMAGE_TAG: + pytest.skip("IMAGE_TAG is unset; no built image to assert against") + + +def test_no_prebundled_package_lost_its_entry_point(): + """A dangling ``__init__.py`` in a prebundle stops Isaac Sim extensions loading. + + Isaac Sim shares prebundled packages between extensions as per-file symlinks, so deleting + or replacing one strands every symlink into it. #6329 added this invariant to the pip + install path after nvbugs 6343978, where it cost 438 error lines and 14 failed extensions. + The images now install with ``uv sync``, which never calls that code, so assert it here. + + Only ``__init__.py`` is fatal: the shipped image already carries dangling submodules and + ``.pyi`` stubs that no extension imports - 41 of them, against develop's 48 - mostly + generated protobuf stubs inside an Omniverse extension's own prebundle. + """ + broken = _in_image('find / -path "*pip_prebundle*" -xtype l -name "__init__.py" 2>/dev/null || true').strip() + assert not broken, "prebundled packages lost their entry point:\n" + broken