Unverified Commit b1cd175f authored by Zoe McCarthy's avatar Zoe McCarthy Committed by GitHub

Adds optional suffix parameter for docker name (#2172)

# Description


Fixes #2149 


## Type of change

- New feature (non-breaking change which adds functionality)
- This change requires a documentation update

## Screenshots


Before:

![docker_name_suffix_before](https://github.com/user-attachments/assets/bc88b41f-69ec-434e-9f81-0189f7b90112)

After:

![docker_name_suffix_after3](https://github.com/user-attachments/assets/4b2866a4-ef3d-4e05-8e27-d9ceae67831a)



## Checklist

- [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
- [x] My changes generate no new warnings
- [x] I have added tests that prove my fix is effective or that my
feature works
- [x] I have updated the changelog and the corresponding version in the
extension's `config/extension.toml` file
- [x] I have added my name to the `CONTRIBUTORS.md` or my name already
exists there

## Comments

I couldn't find any tests for the docker container building, so I'm not
sure where to add some. I'm happy to do so, and can add tests for some
of the other docker building parameters if desired. I have tested
locally for base and ros2 docker profiles with and without the new
suffix parameter. The default setting is an empty string for the suffix,
so it should be a non breaking change and most users will not notice it
being there. I did not touch the cluster docker environment deployment
since I do not have a cluster to test it with, so this new optional
parameter should not be used in the cluster deployment case.

I noticed that there was some redundant docker image building if the
profile is `base`, so I added a check to only build base explicitly if
the docker profile is not base, since the ros2 profile needs base built
in order to build upon it, which is what I assume the explicit base
building is there for.

---------
Signed-off-by: 's avatarMayank Mittal <12863862+Mayankm96@users.noreply.github.com>
Co-authored-by: 's avatarMayank Mittal <12863862+Mayankm96@users.noreply.github.com>
parent 3b8da1d3
......@@ -112,6 +112,7 @@ Guidelines for modifications:
* Yun Liu
* Zhengyu Zhang
* Ziqi Fan
* Zoe McCarthy
## Acknowledgements
......
......@@ -14,3 +14,6 @@ DOCKER_ISAACSIM_ROOT_PATH=/isaac-sim
DOCKER_ISAACLAB_PATH=/workspace/isaaclab
# Docker user directory - by default this is the root user's home directory
DOCKER_USER_HOME=/root
# Docker image and container name suffix (default "", set by the container_interface.py script)
# Example: "-custom"
DOCKER_NAME_SUFFIX=""
......@@ -9,3 +9,6 @@ RMW_IMPLEMENTATION=rmw_fastrtps_cpp
FASTRTPS_DEFAULT_PROFILES_FILE=${DOCKER_USER_HOME}/.ros/fastdds.xml
# Path to cyclonedds.xml file to use (only needed when using cyclonedds)
CYCLONEDDS_URI=${DOCKER_USER_HOME}/.ros/cyclonedds.xml
# Docker image and container name suffix (default "", set by the container_interface.py script)
# Example: "-custom"
DOCKER_NAME_SUFFIX=""
# Everything past this stage is to install
# ROS2 Humble
FROM isaac-lab-base AS ros2
# What is the docker name suffix for the base image to load? (defaults to empty string)
ARG DOCKER_NAME_SUFFIX=""
FROM isaac-lab-base${DOCKER_NAME_SUFFIX} AS ros2
# Which ROS2 apt package to install
ARG ROS2_APT_PACKAGE
......
......@@ -46,6 +46,17 @@ def parse_cli_args() -> argparse.Namespace:
" '.env.base' in their provided order."
),
)
parent_parser.add_argument(
"--suffix",
nargs="?",
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``."
),
)
# Actual command definition begins here
subparsers = parser.add_subparsers(dest="command", required=True)
......@@ -90,7 +101,11 @@ def main(args: argparse.Namespace):
# creating container interface
ci = ContainerInterface(
context_dir=Path(__file__).resolve().parent, profile=args.profile, yamls=args.files, envs=args.env_files
context_dir=Path(__file__).resolve().parent,
profile=args.profile,
yamls=args.files,
envs=args.env_files,
suffix=args.suffix,
)
print(f"[INFO] Using container profile: {ci.profile}")
......
......@@ -87,8 +87,8 @@ services:
- ISAACSIM_ROOT_PATH_ARG=${DOCKER_ISAACSIM_ROOT_PATH}
- ISAACLAB_PATH_ARG=${DOCKER_ISAACLAB_PATH}
- DOCKER_USER_HOME_ARG=${DOCKER_USER_HOME}
image: isaac-lab-base
container_name: isaac-lab-base
image: isaac-lab-base${DOCKER_NAME_SUFFIX-}
container_name: isaac-lab-base${DOCKER_NAME_SUFFIX-}
environment: *default-isaac-lab-environment
volumes: *default-isaac-lab-volumes
network_mode: host
......@@ -113,8 +113,11 @@ services:
# avoid a warning message when building only the base profile
# with the .env.base file
- ROS2_APT_PACKAGE=${ROS2_APT_PACKAGE:-NONE}
image: isaac-lab-ros2
container_name: isaac-lab-ros2
# Make sure that the correct Docker Name Suffix is being passed to the dockerfile, to know which base image to
# start from.
- DOCKER_NAME_SUFFIX=${DOCKER_NAME_SUFFIX-}
image: isaac-lab-ros2${DOCKER_NAME_SUFFIX-}
container_name: isaac-lab-ros2${DOCKER_NAME_SUFFIX-}
environment: *default-isaac-lab-environment
volumes: *default-isaac-lab-volumes
network_mode: host
......
# Copyright (c) 2022-2025, The Isaac Lab Project Developers.
# All rights reserved.
#
# SPDX-License-Identifier: BSD-3-Clause
import os
import subprocess
import unittest
from pathlib import Path
class TestDocker(unittest.TestCase):
"""Test starting and stopping of the docker container with both currently supported profiles and with and without
a suffix. This assumes that docker is installed and configured correctly so that the user can use the docker
commands from the current shell."""
def start_stop_docker(self, profile, suffix):
"""Test starting and stopping docker profile with suffix."""
environ = os.environ
context_dir = Path(__file__).resolve().parent.parent
# generate parameters for the arguments
if suffix != "":
container_name = f"isaac-lab-{profile}-{suffix}"
suffix_args = ["--suffix", suffix]
else:
container_name = f"isaac-lab-{profile}"
suffix_args = []
run_kwargs = {
"check": False,
"capture_output": True,
"text": True,
"cwd": context_dir,
"env": environ,
}
# start the container
docker_start = subprocess.run(["python", "container.py", "start", profile] + suffix_args, **run_kwargs)
self.assertEqual(docker_start.returncode, 0)
# verify that the container is running
docker_running_true = subprocess.run(["docker", "ps"], **run_kwargs)
self.assertEqual(docker_running_true.returncode, 0)
self.assertIn(container_name, docker_running_true.stdout)
# stop the container
docker_stop = subprocess.run(["python", "container.py", "stop", profile] + suffix_args, **run_kwargs)
self.assertEqual(docker_stop.returncode, 0)
# verify that the container has stopped
docker_running_false = subprocess.run(["docker", "ps"], **run_kwargs)
self.assertEqual(docker_running_false.returncode, 0)
self.assertNotIn(container_name, docker_running_false.stdout)
def test_docker_base(self):
"""Test starting and stopping docker base."""
self.start_stop_docker("base", "")
def test_docker_base_suffix(self):
"""Test starting and stopping docker base with a test suffix."""
self.start_stop_docker("base", "test")
def test_docker_ros2(self):
"""Test starting and stopping docker ros2."""
self.start_stop_docker("ros2", "")
def test_docker_ros2_suffix(self):
"""Test starting and stopping docker ros2 with a test suffix."""
self.start_stop_docker("ros2", "test")
if __name__ == "__main__":
unittest.main(verbosity=2, exit=True)
......@@ -24,6 +24,7 @@ class ContainerInterface:
yamls: list[str] | None = None,
envs: list[str] | None = None,
statefile: StateFile | None = None,
suffix: str | None = None,
):
"""Initialize the container interface with the given parameters.
......@@ -37,6 +38,10 @@ class ContainerInterface:
statefile: An instance of the :class:`Statefile` class to manage state variables. Defaults to None, in
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``.
"""
# set the context directory
self.context_dir = context_dir
......@@ -55,11 +60,21 @@ class ContainerInterface:
# but not a real profile
self.profile = "base"
self.container_name = f"isaac-lab-{self.profile}"
self.image_name = f"isaac-lab-{self.profile}:latest"
# 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
self.suffix = ""
else:
# insert a hyphen before the suffix if a suffix is given
self.suffix = f"-{suffix}"
self.container_name = f"isaac-lab-{self.profile}{self.suffix}"
self.image_name = f"isaac-lab-{self.profile}{self.suffix}:latest"
# keep the environment variables from the current environment
self.environ = os.environ
# keep the environment variables from the current environment,
# except make sure that the docker name suffix is set from the script
self.environ = os.environ.copy()
self.environ["DOCKER_NAME_SUFFIX"] = self.suffix
# resolve the image extension through the passed yamls and envs
self._resolve_image_extension(yamls, envs)
......@@ -100,22 +115,23 @@ class ContainerInterface:
" background...\n"
)
# build the image for the base profile
subprocess.run(
[
"docker",
"compose",
"--file",
"docker-compose.yaml",
"--env-file",
".env.base",
"build",
"isaac-lab-base",
],
check=False,
cwd=self.context_dir,
env=self.environ,
)
# build the image for the base profile if not running base (up will build base already if profile is base)
if self.profile != "base":
subprocess.run(
[
"docker",
"compose",
"--file",
"docker-compose.yaml",
"--env-file",
".env.base",
"build",
"isaac-lab-base",
],
check=False,
cwd=self.context_dir,
env=self.environ,
)
# build the image for the profile
subprocess.run(
......@@ -206,7 +222,7 @@ class ContainerInterface:
[
"docker",
"cp",
f"isaac-lab-{self.profile}:{container_path}/",
f"isaac-lab-{self.profile}{self.suffix}:{container_path}/",
f"{host_path}",
],
check=False,
......
......@@ -233,20 +233,35 @@ Isaac Lab Image Extensions
The produced image depends on the arguments passed to ``container.py start`` and ``container.py stop``. These
commands accept an image extension parameter as an additional argument. If no argument is passed, then this
parameter defaults to ``base``. Currently, the only valid values are (``base``, ``ros2``).
Only one image extension can be passed at a time. The produced container will be named ``isaac-lab-${profile}``,
where ``${profile}`` is the image extension name.
Only one image extension can be passed at a time. The produced image and container will be named
``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.
.. code:: bash
# start base by default
# start base by default, named isaac-lab-base
./docker/container.py start
# stop base explicitly
# stop base explicitly, named isaac-lab-base
./docker/container.py stop base
# start ros2 container
# start ros2 container named isaac-lab-ros2
./docker/container.py start ros2
# stop ros2 container
# stop ros2 container named isaac-lab-ros2
./docker/container.py stop ros2
# start base container named isaac-lab-base-custom
./docker/container.py start base --suffix custom
# stop base container named isaac-lab-base-custom
./docker/container.py stop base --suffix custom
# start ros2 container named isaac-lab-ros2-custom
./docker/container.py start ros2 --suffix custom
# stop ros2 container named isaac-lab-ros2-custom
./docker/container.py stop ros2 --suffix custom
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.
......
......@@ -7,6 +7,7 @@ Changelog
Added
^^^^^
* Added check in RecorderManager to ensure that the success indicator is only set if the termination manager is present.
* Added semantic tags in :func:`isaaclab.sim.spawners.from_files.spawn_ground_plane`.
This allows for :attr:`semantic_segmentation_mapping` to be used when using the ground plane spawner.
......@@ -122,7 +123,7 @@ Added
Changed
^^^^^^^
* Definition of render settings in :class:`~isaaclab.sim.SimulationCfg` is changed to None, which means that
* Changed default render settings in :class:`~isaaclab.sim.SimulationCfg` to None, which means that
the default settings will be used from the experience files and the double definition is removed.
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment