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 d831101c9239..07d93c98d7eb 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 554f9989ed00..b1f33e545325 100644 --- a/.github/workflows/build.yaml +++ b/.github/workflows/build.yaml @@ -207,6 +207,9 @@ jobs: 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: @@ -215,20 +218,7 @@ jobs: isaacsim-version: ${{ needs.config.outputs.isaacsim_image_tag }} dockerfile-path: docker/Dockerfile.base cache-tag: cache-base - - # #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: Verify image invariants - shell: bash - env: - IMAGE_TAG: ${{ needs.config.outputs.ci_image_tag }} - run: | - set -euo pipefail - IMAGE_DIGEST="$(docker image inspect --format '{{.Id}}' "${IMAGE_TAG}")" - export IMAGE_DIGEST - uv run --no-project --with pytest \ - python -m pytest -q docker/test/test_image_invariants.py + 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 index ffab8edf5781..74aea9920244 100644 --- a/docker/test/test_image_invariants.py +++ b/docker/test/test_image_invariants.py @@ -38,20 +38,6 @@ def _require_image(): pytest.skip("IMAGE_TAG is unset; no built image to assert against") -def test_image_under_test_is_the_one_that_was_built(): - """Fail loudly rather than assert against whatever a stale tag happens to point at.""" - expected = os.environ.get("IMAGE_DIGEST", "") - if not expected: - pytest.skip("IMAGE_DIGEST is unset; cannot bind the tag to a specific image") - actual = subprocess.run( - ["docker", "image", "inspect", "--format", "{{.Id}}", IMAGE_TAG], - capture_output=True, - text=True, - check=True, - ).stdout.strip() - assert actual == expected, f"{IMAGE_TAG} is {actual}, expected {expected}" - - def test_no_prebundled_package_lost_its_entry_point(): """A dangling ``__init__.py`` in a prebundle stops Isaac Sim extensions loading.