diff --git a/CONTRIBUTORS.md b/CONTRIBUTORS.md index 6a96c1bf6be..aafade12061 100644 --- a/CONTRIBUTORS.md +++ b/CONTRIBUTORS.md @@ -77,6 +77,7 @@ Guidelines for modifications: * Gary Lvov * Giulio Romualdi * Grzegorz Malczyk +* Guanpeng Long * Haoran Zhou * Harsh Patel * HoJin Jeon diff --git a/docker/container.py b/docker/container.py index ab92d816ffa..a58d1c7dcc1 100755 --- a/docker/container.py +++ b/docker/container.py @@ -48,13 +48,16 @@ def parse_cli_args() -> argparse.Namespace: ) parent_parser.add_argument( "--suffix", - nargs="?", + type=str, default=None, help=( - "Optional docker image and container name suffix. Defaults to None, in which case, the docker name" - " suffix is set to the empty string. A hyphen is inserted in between the profile and the suffix if" - ' the suffix is a nonempty string. For example, if "base" is passed to profile, and "custom" is' - " passed to suffix, then the produced docker image and container will be named ``isaac-lab-base-custom``." + "Optional docker image and container name suffix. If omitted (default), a per-user suffix" + " derived from the current username is used to avoid name collisions in multi-user environments." + " If you explicitly pass an empty string (``--suffix ''``), the old behavior (no suffix) is preserved." + " Passing ``--suffix`` without a value is not supported; omit ``--suffix`` to use the default behavior." + " A hyphen is inserted between the profile and the suffix when a nonempty suffix is used." + " For example, with profile 'base' and suffix 'custom' the container will be named" + " ``isaac-lab-base-custom``." ), ) parent_parser.add_argument( diff --git a/docker/test/test_docker.py b/docker/test/test_docker.py index 85fd66348f8..6987878bd10 100644 --- a/docker/test/test_docker.py +++ b/docker/test/test_docker.py @@ -21,7 +21,7 @@ def start_stop_docker(profile, suffix): suffix_args = ["--suffix", suffix] else: container_name = f"isaac-lab-{profile}" - suffix_args = [] + suffix_args = ["--suffix", ""] run_kwargs = { "check": False, diff --git a/docker/utils/container_interface.py b/docker/utils/container_interface.py index f8b3eb07ee2..cbd43072d77 100644 --- a/docker/utils/container_interface.py +++ b/docker/utils/container_interface.py @@ -5,7 +5,10 @@ from __future__ import annotations +import getpass +import json import os +import re import shutil import subprocess from pathlib import Path @@ -44,10 +47,10 @@ def __init__( which case a new configuration object is created by reading the configuration file at the path ``context_dir/.container.cfg``. suffix: - Optional docker image and container name suffix. Defaults to None, in which case, the docker name - suffix is set to the empty string. A hyphen is inserted in between the profile and the suffix if - the suffix is a nonempty string. For example, if "base" is passed to profile, and "custom" is - passed to suffix, then the produced docker image and container will be named ``isaac-lab-base-custom``. + Optional docker image/container name suffix. + If omitted (``None``), a username-based suffix is used to avoid collisions in multi-user setups. + If explicitly set to an empty string, legacy no-suffix behavior is preserved. + For non-empty values, a hyphen is inserted before the suffix. """ # set the context directory self.context_dir = context_dir @@ -66,12 +69,22 @@ def __init__( # but not a real profile self.profile = "base" - # set the docker image and container name suffix - if suffix is None or suffix == "": - # if no name suffix is given, default to the empty string as the name suffix + if suffix == "": + # explicit empty string -> preserve legacy behavior (no suffix) self.suffix = "" + elif suffix is None: + # no suffix provided -> default to current user to avoid name collisions in multi-user environments + try: + user = getpass.getuser() + except Exception: + user = "" + # sanitize username to be safe for docker names (replace problematic chars) + safe_user = "".join(c if c.isalnum() or c in ["-", "_"] else "_" for c in user) + if not safe_user: + safe_user = "user" + self.suffix = f"-{safe_user}" else: - # insert a hyphen before the suffix if a suffix is given + # insert a hyphen before the provided suffix self.suffix = f"-{suffix}" # set names for easier reference @@ -85,6 +98,9 @@ def __init__( self.environ = os.environ.copy() self.environ["DOCKER_NAME_SUFFIX"] = self.suffix + project_name = re.sub(r"-+", "-", f"isaac-lab{self.suffix}").rstrip("-") + self.environ["COMPOSE_PROJECT_NAME"] = project_name + # resolve the image extension through the passed yamls and envs self._resolve_image_extension(yamls, envs) # load the environment variables from the .env files @@ -166,6 +182,23 @@ def build(self): def start(self): """Build and start the Docker container using the Docker compose command.""" + # detect potential conflicting containers from other users/projects + conflicts = self._detect_conflicting_containers() + if conflicts: + print("[ERROR] Found existing containers that may conflict with this start:") + for c in conflicts: + print(f" - {c}") + print( + "\nTo avoid unintentionally recreating others' containers, choose one of the following:\n" + " * Start with an explicit --suffix (e.g. --suffix yourname) to create an isolated container.\n" + " * Start with `--suffix ''` to intentionally use the shared legacy container (not recommended).\n" + " * If you are sure, set the environment variable ISAACLAB_FORCE_START=1 to override and proceed.\n" + ) + # allow forced start via environment variable + if os.environ.get("ISAACLAB_FORCE_START") == "1": + print("[WARN] ISAACLAB_FORCE_START=1 set — proceeding despite conflicts.") + else: + raise RuntimeError("Aborting start due to potential container conflicts.") print( f"[INFO] Building the docker image and starting the container '{self.container_name}' in the" " background...\n" @@ -293,6 +326,67 @@ def config(self, output_yaml: Path | None = None): cmd = ["docker", "compose"] + self.add_yamls + self.add_profiles + self.add_env_files + ["config"] + output subprocess.run(cmd, check=False, cwd=self.context_dir, env=self.environ) + def _detect_conflicting_containers(self) -> list[str]: + """Detect existing containers in the same compose project namespace.""" + project_name = self.environ.get("COMPOSE_PROJECT_NAME", "isaac-lab") + + try: + result = subprocess.run( + [ + "docker", + "ps", + "-a", + "--filter", + f"label=com.docker.compose.project={project_name}", + "--format", + "{{.Names}}", + ], + capture_output=True, + text=True, + check=False, + ) + except Exception: + result = None + + if result is not None and result.returncode == 0: + names = [n.strip() for n in result.stdout.splitlines() if n.strip()] + return [n for n in names if n != self.container_name] + + try: + result = subprocess.run( + ["docker", "ps", "-a", "--format", "{{.Names}}"], + capture_output=True, + text=True, + check=False, + ) + except Exception: + return [] + if result.returncode != 0: + return [] + names = [n.strip() for n in result.stdout.splitlines() if n.strip()] + conflicts: list[str] = [] + + for n in names: + if n == self.container_name: + continue + + try: + out = subprocess.run( + ["docker", "inspect", "--format", "{{json .Config.Labels}}", n], + capture_output=True, + text=True, + check=False, + ).stdout + labels = json.loads(out) if out else {} + except Exception: + labels = {} + + comp_proj = labels.get("com.docker.compose.project") + if comp_proj == project_name: + conflicts.append(n) + + return conflicts + """ Helper functions. """ diff --git a/docs/source/deployment/docker.rst b/docs/source/deployment/docker.rst index 198c84a7cce..ac1aada5a59 100644 --- a/docs/source/deployment/docker.rst +++ b/docs/source/deployment/docker.rst @@ -238,21 +238,40 @@ Only one image extension can be passed at a time. The produced image and contai ``isaac-lab-${profile}``, where ``${profile}`` is the image extension name. ``suffix`` is an optional string argument to ``container.py`` that specifies a docker image and -container name suffix, which can be useful for development purposes. By default ``${suffix}`` is the empty string. -If ``${suffix}`` is a nonempty string, then the produced docker image and container will be named -``isaac-lab-${profile}-${suffix}``, where a hyphen is inserted between ``${profile}`` and ``${suffix}`` in -the name. ``suffix`` should not be used with cluster deployments. +container name suffix, which is useful for local development in multi-user environments: + +* If ``--suffix`` is omitted, Isaac Lab uses a per-user suffix derived from the current username. + This creates names such as ``isaac-lab-base-`` and isolates Docker Compose projects by user. +* Passing ``--suffix`` without a value is not supported. Omit the flag to use the default behavior. +* If ``--suffix ''`` is passed explicitly, Isaac Lab preserves the legacy no-suffix behavior + (for example, ``isaac-lab-base``). +* If ``--suffix `` is provided, Isaac Lab inserts a hyphen and uses + ``isaac-lab-${profile}-${suffix}``. + +.. note:: + + This is a breaking change from earlier releases where omitting ``--suffix`` produced containers named + ``isaac-lab-${profile}``. If you have existing legacy containers created without a suffix, explicitly + pass ``--suffix ''`` to target them when running ``stop``/``enter``/``copy``. + +``suffix`` should not be used with cluster deployments. .. code:: bash - # start base by default, named isaac-lab-base + # start base with the default user-derived suffix (for example, isaac-lab-base-gplong) ./docker/container.py start - # stop base explicitly, named isaac-lab-base + # stop base with the same default user-derived suffix ./docker/container.py stop base + + # explicitly preserve legacy behavior (no suffix), named isaac-lab-base + ./docker/container.py start base --suffix '' + # stop base explicitly, named isaac-lab-base + ./docker/container.py stop base --suffix '' + # start ros2 container named isaac-lab-ros2 - ./docker/container.py start ros2 + ./docker/container.py start ros2 --suffix '' # stop ros2 container named isaac-lab-ros2 - ./docker/container.py stop ros2 + ./docker/container.py stop ros2 --suffix '' # start base container named isaac-lab-base-custom ./docker/container.py start base --suffix custom @@ -263,6 +282,16 @@ the name. ``suffix`` should not be used with cluster deployments. # stop ros2 container named isaac-lab-ros2-custom ./docker/container.py stop ros2 --suffix custom +When starting a container, Isaac Lab checks whether another container from the same Docker Compose +project is already running. This prevents accidental recreation of containers owned by another user +or another shell session. + +If you intentionally want to proceed despite a detected conflict, set ``ISAACLAB_FORCE_START=1``: + +.. code:: bash + + ISAACLAB_FORCE_START=1 ./docker/container.py start + The passed image extension argument will build the image defined in ``Dockerfile.${image_extension}``, with the corresponding `profile`_ in the ``docker-compose.yaml`` and the envars from ``.env.${image_extension}`` in addition to the ``.env.base``, if any. diff --git a/source/isaaclab/docs/CHANGELOG.rst b/source/isaaclab/docs/CHANGELOG.rst index 0bb821cbc74..701dc6b4d78 100644 --- a/source/isaaclab/docs/CHANGELOG.rst +++ b/source/isaaclab/docs/CHANGELOG.rst @@ -34,6 +34,11 @@ Changed for warp array properties and write methods, ensuring consistent documentation between PhysX and Newton backend implementations. +* Improved Docker multi-user container isolation by making omitted ``--suffix`` default to a + user-derived value, propagating this suffix to ``COMPOSE_PROJECT_NAME``, and adding startup + conflict detection with support for explicit legacy mode (``--suffix ''``). Existing legacy + containers created without a suffix can still be targeted by passing ``--suffix ''`` explicitly. + 4.1.0 (2026-02-18) ~~~~~~~~~~~~~~~~~~