Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 22 additions & 6 deletions docker/run_docker.sh
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,13 @@ if [ -n "${SSH_AUTH_SOCK:-}" ] && [ -S "$SSH_AUTH_SOCK" ]; then
SSH_DOCKER_ARGS+=("-v" "$SSH_AUTH_SOCK:/ssh-agent" "--env" "SSH_AUTH_SOCK=/ssh-agent")
fi

# Isolated mode (ISAAC_AUTODATA_ISOLATED=1, e.g. CI): skip the dev-only host
# mounts ($HOME/.cache, X11) whose ownership collides with the container user.
ISOLATED=false
case "${ISAAC_AUTODATA_ISOLATED:-}" in
1 | true | TRUE | yes) ISOLATED=true ;;
esac

Comment on lines +133 to +139

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

Fail closed on invalid ISAAC_AUTODATA_ISOLATED values.

Unrecognized values currently leave ISOLATED=false, so a typo such as ture silently re-enables $HOME/.cache and X11 mounts. Reject invalid values while preserving explicit 0/false local-dev behavior.

Proposed validation
 ISOLATED=false
-case "${ISAAC_AUTODATA_ISOLATED:-}" in
-    1 | true | TRUE | yes) ISOLATED=true ;;
+value="${ISAAC_AUTODATA_ISOLATED:-}"
+case "${value,,}" in
+    1 | true | yes) ISOLATED=true ;;
+    "" | 0 | false | no) ;;
+    *) echo "Invalid ISAAC_AUTODATA_ISOLATED: ${value}" >&2; exit 2 ;;
 esac
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
# Isolated mode (ISAAC_AUTODATA_ISOLATED=1, e.g. CI): skip the dev-only host
# mounts ($HOME/.cache, X11) whose ownership collides with the container user.
ISOLATED=false
case "${ISAAC_AUTODATA_ISOLATED:-}" in
1 | true | TRUE | yes) ISOLATED=true ;;
esac
# Isolated mode (ISAAC_AUTODATA_ISOLATED=1, e.g. CI): skip the dev-only host
# mounts ($HOME/.cache, X11) whose ownership collides with the container user.
ISOLATED=false
value="${ISAAC_AUTODATA_ISOLATED:-}"
case "${value,,}" in
1 | true | yes) ISOLATED=true ;;
"" | 0 | false | no) ;;
*) echo "Invalid ISAAC_AUTODATA_ISOLATED: ${value}" >&2; exit 2 ;;
esac
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docker/run_docker.sh` around lines 133 - 139, Update the
ISAAC_AUTODATA_ISOLATED parsing case around ISOLATED so recognized false values
such as 0 and false explicitly remain non-isolated, recognized true values still
enable isolation, and any other non-empty value exits with an error instead of
defaulting to ISOLATED=false.

DOCKER_RUN_ARGS=(
"--name" "${CONTAINER_NAME}"
"--privileged"
Expand All @@ -142,10 +149,18 @@ DOCKER_RUN_ARGS=(
# Live-mount the repo: host edits are reflected in the container's editable installs.
"-v" "${REPO_ROOT}:${WORKDIR}"
$(add_volume_if_it_exists "$DATASETS_HOST_MOUNT_DIRECTORY" /datasets)
# Share the host Kit/pip cache to speed up shader warmup and reinstalls across runs.
"-v" "$HOME/.cache:/home/$(id -un)/.cache"
# X11 passthrough so "--viz kit" can open a window.
"-v" "/tmp/.X11-unix:/tmp/.X11-unix:rw"
)

if [ "$ISOLATED" = false ]; then
DOCKER_RUN_ARGS+=(
# Share the host Kit/pip cache to speed up shader warmup and reinstalls across runs.
"-v" "$HOME/.cache:/home/$(id -un)/.cache"
# X11 passthrough so "--viz kit" can open a window.
"-v" "/tmp/.X11-unix:/tmp/.X11-unix:rw"
)
fi

DOCKER_RUN_ARGS+=(
"${SSH_DOCKER_ARGS[@]}"
"--env" "DISPLAY=${DISPLAY:-}"
"--env" "ACCEPT_EULA=Y"
Expand All @@ -158,8 +173,9 @@ DOCKER_RUN_ARGS=(
"--env" "DOCKER_RUN_GROUP_NAME=$(id -gn)"
)

# Allow local X11 clients from the container (for the Kit viewer).
if command -v xhost >/dev/null 2>&1; then
# Allow local X11 clients from the container (for the Kit viewer). Skipped in
# isolated mode, where X11 is not mounted.
if [ "$ISOLATED" = false ] && command -v xhost >/dev/null 2>&1; then
xhost +local:docker >/dev/null 2>&1 || true
fi

Expand Down
82 changes: 28 additions & 54 deletions scripts/ci/run_tests.sh
Original file line number Diff line number Diff line change
Expand Up @@ -4,72 +4,46 @@
#
# SPDX-License-Identifier: Apache-2.0
#
# Run the Isaac AutoData test suite inside the repo's GPU Docker image.
# This is the single entry point shared by local runs and CI: it wraps
# ./docker/run_docker.sh so a developer reproduces a CI failure with one command:
# Run the Isaac AutoData test suite, locally or in CI, inside the repo's GPU
# Docker image with one command:
#
# ./scripts/ci/run_tests.sh
#
# Requires a GPU host with the NVIDIA container runtime and nvidia-smi (used by
# run_docker.sh -c to detect the GPU arch for the cuRobo build).
# Thin wrapper: prepares the container, then runs
# scripts/ci/run_tests_in_container.sh for the test logic.
#
# Environment overrides:
# TEST_PATH space-separated test paths (default: all)
# PYTEST_MARK pytest -m marker (single token; optional)
# FORCE_REBUILD=true force an image rebuild (nightly)
# ISAAC_AUTODATA_ISOLATED 1 (default here) drops host cache/X11 mounts
# ISAAC_AUTODATA_SUBPROCESS_TIMEOUT data-generation child timeout, seconds
#
# Requires a GPU host with the NVIDIA container runtime and nvidia-smi.

set -euo pipefail

# ---------------------------------------------------------------------------
# Configuration (override via environment)
# ---------------------------------------------------------------------------
# Test path(s) collected by pytest, relative to the repo root. May be a single
# path or several space-separated paths. Premerge scopes this to the correctness
# suites; the nightly leaves it at the default (the whole test tree).
TEST_PATH="${TEST_PATH:-isaac_autodata_tests/}"
# Optional pytest marker expression. Empty (the default) applies no marker
# filter and runs everything collected under TEST_PATH. Note: use `-` (not `:-`)
# so an explicitly empty value from a caller is honored rather than defaulted.
PYTEST_MARK="${PYTEST_MARK-}"
# Set to "true" to force an image rebuild (used by the nightly workflow).
FORCE_REBUILD="${FORCE_REBUILD:-false}"
# Per-subprocess wall-clock timeout (seconds) for the data-generation child.
# run_docker.sh gives the container a fresh environment, so this is forwarded
# explicitly on the in-container command line rather than exported on the host.
SUBPROCESS_TIMEOUT="${ISAAC_AUTODATA_SUBPROCESS_TIMEOUT:-1200}"
# Container-local cache directory. run_docker.sh bind-mounts the host's
# $HOME/.cache into the container; on the CI runner that path is root-owned (or
# auto-created as root), so the recreated non-root container user cannot write
# it -- warp fails to create ~/.cache/warp with a PermissionError. Pointing the
# cache env vars at a writable, container-local /tmp path sidesteps the mounted
# host cache entirely. The tools create these dirs themselves (makedirs).
CONTAINER_CACHE_DIR="${CONTAINER_CACHE_DIR:-/tmp/isaac_autodata_ci_cache}"
# ---------------------------------------------------------------------------

SCRIPT_DIR=$(cd -- "$(dirname -- "${BASH_SOURCE[0]}")" &>/dev/null && pwd)
REPO_ROOT=$(cd -- "${SCRIPT_DIR}/../.." &>/dev/null && pwd)
cd "${REPO_ROOT}"

# cuRobo (-c) is required for the SkillGen tests; one image covers the whole suite.
# cuRobo (-c) is required for the SkillGen tests; one image covers the suite.
RUN_DOCKER_ARGS=(-c)
if [ "${FORCE_REBUILD}" = "true" ]; then
if [ "${FORCE_REBUILD:-false}" = "true" ]; then
RUN_DOCKER_ARGS+=(-r)
fi

# Build the in-container command. `env VAR=...` forwards config into the
# container (run_docker.sh passes trailing args through verbatim, and the values
# are inherited by the data-generation subprocess pytest spawns). Call
# /isaac-sim/python.sh -m pytest explicitly so it does not depend on the
# in-container `pytest` alias expanding. XDG_CACHE_HOME/WARP_CACHE_PATH steer all
# caches away from the mounted host $HOME/.cache (see CONTAINER_CACHE_DIR above).
PYTEST_ARGS=(
env
"ISAAC_AUTODATA_SUBPROCESS_TIMEOUT=${SUBPROCESS_TIMEOUT}"
"XDG_CACHE_HOME=${CONTAINER_CACHE_DIR}"
"WARP_CACHE_PATH=${CONTAINER_CACHE_DIR}/warp"
/isaac-sim/python.sh -m pytest -sv --durations=0
)
if [ -n "${PYTEST_MARK}" ]; then
PYTEST_ARGS+=(-m "${PYTEST_MARK}")
fi
# TEST_PATH may list multiple paths; split on whitespace into separate args.
read -r -a TEST_PATHS <<< "${TEST_PATH}"
PYTEST_ARGS+=("${TEST_PATHS[@]}")
# Run the container isolated (no host $HOME/.cache or X11 bind-mounts) so CI does
# not inherit the runner's home-directory ownership. Overridable for local dev.
export ISAAC_AUTODATA_ISOLATED="${ISAAC_AUTODATA_ISOLATED:-1}"

# run_docker.sh gives the container a fresh environment and forwards its trailing
# arguments verbatim as the in-container command. Pass test paths positionally
# (space-safe) and the marker/timeout via `env`; the in-container script reads them.
read -r -a TEST_PATHS <<< "${TEST_PATH:-isaac_autodata_tests/}"

echo ">>> Running E2E tests (mark='${PYTEST_MARK:-<all>}', paths='${TEST_PATH}', rebuild=${FORCE_REBUILD})"
exec ./docker/run_docker.sh "${RUN_DOCKER_ARGS[@]}" "${PYTEST_ARGS[@]}"
exec ./docker/run_docker.sh "${RUN_DOCKER_ARGS[@]}" \
env \
"PYTEST_MARK=${PYTEST_MARK-}" \
"ISAAC_AUTODATA_SUBPROCESS_TIMEOUT=${ISAAC_AUTODATA_SUBPROCESS_TIMEOUT:-1200}" \
./scripts/ci/run_tests_in_container.sh "${TEST_PATHS[@]}"
42 changes: 42 additions & 0 deletions scripts/ci/run_tests_in_container.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
#!/bin/bash
# Copyright (c) 2026, The Isaac AutoData Project Developers.
# All rights reserved.
#
# SPDX-License-Identifier: Apache-2.0
#
# Run the Isaac AutoData test suite. Environment-independent: it assumes no host
# paths and runs anywhere the dependencies exist (the container or a conda env).
#
# Test paths are positional arguments (default: the whole tree). Behavior is
# tuned via environment variables:
# PYTEST_MARK pytest -m marker expression (optional)
# ISAAC_AUTODATA_SUBPROCESS_TIMEOUT data-generation child timeout, seconds
# ISAAC_AUTODATA_CACHE_DIR writable cache root (default /tmp/...)
# ISAAC_AUTODATA_PYTHON python launcher (default Isaac Sim's)

set -euo pipefail

# Default to the whole test tree when no paths are given.
if [ "$#" -eq 0 ]; then
set -- isaac_autodata_tests/
fi

# Run-local caches, to avoid the ownership collision on a bind-mounted host
# $HOME/.cache (warp's kernel cache lives under ~/.cache/warp).
CACHE_DIR="${ISAAC_AUTODATA_CACHE_DIR:-/tmp/isaac_autodata_cache}"
export XDG_CACHE_HOME="${CACHE_DIR}"
export WARP_CACHE_PATH="${CACHE_DIR}/warp"

# Inherited by the data-generation subprocess pytest spawns.
export ISAAC_AUTODATA_SUBPROCESS_TIMEOUT="${ISAAC_AUTODATA_SUBPROCESS_TIMEOUT:-1200}"

PYTHON="${ISAAC_AUTODATA_PYTHON:-/isaac-sim/python.sh}"

PYTEST=("${PYTHON}" -m pytest -sv --durations=0)
if [ -n "${PYTEST_MARK:-}" ]; then
PYTEST+=(-m "${PYTEST_MARK}")
fi
PYTEST+=("$@")

echo ">>> ${PYTEST[*]} (XDG_CACHE_HOME=${XDG_CACHE_HOME})"
exec "${PYTEST[@]}"
Loading