Repository: aiidalab/aiidalab-launch (language: python)

Issue:
Allow custom volume mounts to aiidalab container
I've just started playing with `aiidalab-launch` and it works great! Much better experience than before.

However, it seems that I cannot currently utilize it since I need to have a custom volume mounted for my aiidalab application.* Would it be possible to add a profile setting to have custom volumes?

* my package depends on a quantum chemistry package ORCA, which is not open source and cannot be automatically installed. My current solution to this problem is to mount a directory with the ORCA binaries as a volume to the aiidalab container. I am all ears if there is a better approach. :-)


Discussion / hints:
@danielhollas Thanks for the feedback and for reporting the issue! I think adding a feature for mounting additional volumes would be in scope. 

For this, I would suggest to add a a `volumes` key to the [profile configuration](https://github.com/aiidalab/aiidalab-launch/blob/64dc0004b105b2341358692f32490b8631ee69e8/aiidalab_launch/profile.py#L51-L58) that uses a syntax similar or equal to those of the docker-compose file: https://docs.docker.com/compose/compose-file/#volumes

I don't have the bandwidth to work on this at the moment, but I can propose this to the team at the next meeting. In any case, would you potentially be open/interested in giving this a shot yourself?

Please note that aiidalab-launch has some function overlap with docker-compose and there is an ongoing effort (#127) that would potentially refactor aiidalab-launch to use docker-compose instead of docker, but I don't expect that to come to pass for at least another month or so.
@csadorf Thank for the info! I will give it a shot, feel free to assign this issue to me.

Fix patch:
diff --git a/README.md b/README.md
index 2fb8972..790c38f 100644
--- a/README.md
+++ b/README.md
@@ -43,6 +43,25 @@ aiidalab-launch status
 The tool allows to manage multiple profiles, e.g., with different home directories or ports.
 See `aiidalab-launch profiles --help` for more information.
 
+### Data Management
+
+By default AiiDAlab will store all of its data in a [Docker volume](https://docs.docker.com/storage/volumes/) defined in the profile configuration option `home_mount`.
+You can also provide an absolute path to the AiiDAlab home directory on the host system (so called [bind mount](https://docs.docker.com/storage/bind-mounts/)).
+If this directory does not exist, AiiDAlab launch will try to create it on startup.
+
+Additional volumes to be mounted to the the AiiDAlab container can be specified via the `extra_mounts` option using the ["docker-compose "short syntax"](https://docs.docker.com/compose/compose-file/compose-file-v3/#volumes) `source:target:mode`.
+`source` is either a volume name or an absolute path to an existing directory on the host system,
+`target` is a path within the AiiDAlab container,
+and mode is either `rw` for read-write volume (default) or `ro` for read-only volume.
+
+As an example, here's how you can mount a quantum chemistry program installed on the host system to make it accessible to AiiDA inside the AiiDAlab container:
+```python
+extra_mounts = ["/path/to/qcprogram:/opt/qcprogram:ro",]
+```
+
+Finally, AiiDAlab launch will create a dedicated volume for the local conda environment (`~/.conda`).
+That is because some conda packages are not compatible with non-linux file systems, meaning that they cannot be installed if the home directory was, for example, bound to a Mac OS-X file system on the host.
+
 ### Forward AiiDAlab from a remote server via SSH
 
 Please see [here](ssh-forward.md) for instructions on how to run AiiDAlab on a remote machine and then forward it to your local terminal.
diff --git a/aiidalab_launch/__main__.py b/aiidalab_launch/__main__.py
index a8f0c9d..42f7be8 100644
--- a/aiidalab_launch/__main__.py
+++ b/aiidalab_launch/__main__.py
@@ -53,6 +53,8 @@
 
 Home mounted: {home_mount} -> /home/{system_user}"""
 
+MSG_EXTRA_VOLUME = "Extra volume mounted: {source} -> {target} {mode}"
+
 
 LOGGING_LEVELS = {
     0: logging.ERROR,
@@ -437,6 +439,18 @@ async def _async_start(
                 ).lstrip(),
                 fg="green",
             )
+
+            for extra_mount in profile.extra_mounts:
+                source, target, mode = profile.parse_extra_mount(extra_mount)
+                click.secho(
+                    MSG_EXTRA_VOLUME.format(
+                        source=source,
+                        target=target,
+                        mode=f"({mode})" if mode else "",
+                    ).lstrip(),
+                    fg="green",
+                )
+
             if not no_browser and webbrowser_available():
                 if click.confirm(
                     "Do you want to open AiiDAlab in the browser now?", default=True
diff --git a/aiidalab_launch/application_state.py b/aiidalab_launch/application_state.py
index 3d50bbb..ecc4e5f 100644
--- a/aiidalab_launch/application_state.py
+++ b/aiidalab_launch/application_state.py
@@ -72,7 +72,7 @@ def apply_migrations(self):
             config_changed = True
 
         # No version string stored in config.
-        if self.config.version is None:
+        if self.config.version != str(parse(__version__)):
             self.config.version = str(parse(__version__))
             config_changed = True
 
diff --git a/aiidalab_launch/config.py b/aiidalab_launch/config.py
index 51c2740..14498cd 100644
--- a/aiidalab_launch/config.py
+++ b/aiidalab_launch/config.py
@@ -2,6 +2,7 @@
 # with py 3.10.
 from __future__ import annotations
 
+from copy import deepcopy
 from dataclasses import asdict, dataclass, field
 from pathlib import Path
 from uuid import uuid4
@@ -25,11 +26,16 @@ class Config:
 
     @classmethod
     def loads(cls, blob: str) -> Config:
-        config = toml.loads(blob)
-        config["profiles"] = [
-            Profile(name=name, **profile)
-            for name, profile in config.pop("profiles", dict()).items()
-        ]
+        loaded_config = toml.loads(blob)
+        config = deepcopy(loaded_config)
+        config["profiles"] = []
+        for name, profile in loaded_config.pop("profiles", dict()).items():
+            extra_mounts = (
+                set(profile.pop("extra_mounts")) if "extra_mounts" in profile else set()
+            )
+            config["profiles"].append(
+                Profile(name=name, extra_mounts=extra_mounts, **profile)
+            )
         return cls(**config)
 
     def dumps(self) -> str:
diff --git a/aiidalab_launch/instance.py b/aiidalab_launch/instance.py
index fe83d4d..81d40e6 100644
--- a/aiidalab_launch/instance.py
+++ b/aiidalab_launch/instance.py
@@ -113,10 +113,21 @@ def _home_mount(self) -> docker.types.Mount:
             type="bind" if home_mount_path.is_absolute() else "volume",
         )
 
+    def _extra_mount(self, extra_mount: str) -> docker.types.Mount:
+        source_path, target_path, mode = self.profile.parse_extra_mount(extra_mount)
+        return docker.types.Mount(
+            target=str(target_path),
+            source=str(source_path),
+            read_only=True if mode == "ro" else False,
+            type="bind" if source_path.is_absolute() else "volume",
+        )
+
     def _mounts(self) -> Generator[docker.types.Mount, None, None]:
         yield self._conda_mount()
         if self.profile.home_mount:
             yield self._home_mount()
+        for extra_mount in self.profile.extra_mounts:
+            yield self._extra_mount(extra_mount)
 
     def configuration_changes(self) -> Generator[str, None, None]:
         self._requires_container()
diff --git a/aiidalab_launch/profile.py b/aiidalab_launch/profile.py
index 0cb54ac..7a4aa05 100644
--- a/aiidalab_launch/profile.py
+++ b/aiidalab_launch/profile.py
@@ -4,13 +4,13 @@
 
 import re
 from dataclasses import asdict, dataclass, field
-from pathlib import PurePosixPath
+from pathlib import Path, PurePosixPath
 from urllib.parse import quote_plus
 
 import toml
 from docker.models.containers import Container
 
-from .util import docker_mount_for, get_docker_env
+from .util import docker_mount_for, get_docker_env, is_volume_readonly
 
 MAIN_PROFILE_NAME = "default"
 
@@ -18,9 +18,10 @@
 
 DEFAULT_PORT = 8888
 
-# Regular expression for valid container names as of Docker version 20.10.11:
-# [a-zA-Z0-9][a-zA-Z0-9_.-]
-# In addition we do not allow underscores to avoid potential issues with moving
+# Regular expression for valid container and image names as of Docker version 20.10.11:
+# [a-zA-Z0-9][a-zA-Z0-9_.-]+
+_REGEX_VALID_IMAGE_NAMES = r"[a-zA-Z0-9][a-zA-Z0-9_.-]+"
+# For profiles in addition we do not allow underscores to avoid potential issues with moving
 # to a docker-compose based implementation of aiidalab-launch in the future.
 _REGEX_VALID_PROFILE_NAMES = r"[a-zA-Z0-9][a-zA-Z0-9.-]+"
 
@@ -32,6 +33,17 @@ def _default_port() -> int:  # explicit function required to enable test patchin
 _DEFAULT_IMAGE = "aiidalab/aiidalab-docker-stack:latest"
 
 
+def _valid_volume_name(source: str) -> None:
+    # We do not allow relative paths so if the path is not absolute,
+    # we assume volume mount, whose name is restricted by Docker.
+    if not Path(source).is_absolute() and not re.fullmatch(
+        _REGEX_VALID_IMAGE_NAMES, source
+    ):
+        raise ValueError(
+            f"Invalid extra mount volume name '{source}'. Use absolute path for bind mounts."
+        )
+
+
 def _get_configured_host_port(container: Container) -> int | None:
     try:
         host_config = container.attrs["HostConfig"]
@@ -56,6 +68,7 @@ class Profile:
     system_user: str = "aiida"
     image: str = _DEFAULT_IMAGE
     home_mount: str | None = None
+    extra_mounts: set[str] = field(default_factory=set)
 
     def __post_init__(self):
         if (
@@ -70,9 +83,42 @@ def __post_init__(self):
         if self.home_mount is None:
             self.home_mount = f"{CONTAINER_PREFIX}{self.name}_home"
 
+        _valid_volume_name(self.home_mount)
+
+        # Normalize extra mount mode to be "rw" by default
+        # so that we match Docker default but are explicit.
+        for extra_mount in self.extra_mounts:
+            self.parse_extra_mount(extra_mount)
+            if len(extra_mount.split(":")) == 2:
+                self.extra_mounts.remove(extra_mount)
+                self.extra_mounts.add(f"{extra_mount}:rw")
+
     def container_name(self) -> str:
         return f"{CONTAINER_PREFIX}{self.name}"
 
+    def parse_extra_mount(
+        self, extra_mount: str
+    ) -> tuple[Path, PurePosixPath, str | None]:
+        fields = extra_mount.split(":")
+        if len(fields) < 2 or len(fields) > 3:
+            raise ValueError(f"Invalid extra mount option '{extra_mount}'")
+
+        source, target = fields[:2]
+        _valid_volume_name(source)
+
+        source_path, target_path = Path(source), PurePosixPath(target)
+        # Unlike for home_mount, we will not auto-create missing
+        # directories for extra mounts.
+        if source_path.is_absolute() and not source_path.exists():
+            raise ValueError(f"Directory '{source}' does not exist")
+
+        # By default, extra mounts are writeable
+        mode = fields[2] if len(fields) == 3 else "rw"
+        if mode not in ("ro", "rw"):
+            raise ValueError(f"Invalid extra mount mode '{mode}'")
+
+        return source_path, target_path, mode
+
     def conda_volume_name(self) -> str:
         return f"{self.container_name()}_conda"
 
@@ -88,7 +134,9 @@ def dumps(self) -> str:
 
     @classmethod
     def loads(cls, name: str, s: str) -> Profile:
-        return cls(name=name, **toml.loads(s))
+        params = toml.loads(s)
+        params["extra_mounts"] = set(params["extra_mounts"])
+        return cls(name=name, **params)
 
     @classmethod
     def from_container(cls, container: Container) -> Profile:
@@ -106,6 +154,20 @@ def from_container(cls, container: Container) -> Profile:
             else container.image.tags[0]
         )
 
+        extra_destinations: list[PurePosixPath] = [
+            PurePosixPath(mount["Destination"])
+            for mount in container.attrs["Mounts"]
+            if mount["Destination"]
+            not in (f"/home/{system_user}/.conda", f"/home/{system_user}")
+        ]
+        extra_mounts: set[str] = set()
+        for dst in extra_destinations:
+            src = docker_mount_for(container, dst)
+            if is_volume_readonly(container, dst):
+                extra_mounts.add(":".join([str(src), str(dst), "ro"]))
+            else:
+                extra_mounts.add(":".join([str(src), str(dst), "rw"]))
+
         return Profile(
             name=profile_name,
             port=_get_configured_host_port(container),
@@ -115,4 +177,5 @@ def from_container(cls, container: Container) -> Profile:
             ),
             image=image_tag,
             system_user=system_user,
+            extra_mounts=extra_mounts,
         )
diff --git a/aiidalab_launch/util.py b/aiidalab_launch/util.py
index ec5e1d8..cb8a88f 100644
--- a/aiidalab_launch/util.py
+++ b/aiidalab_launch/util.py
@@ -160,10 +160,9 @@ def confirm_with_value(value: str, text: str, abort: bool = False) -> bool:
         return False
 
 
-def docker_mount_for(
+def get_docker_mount(
     container: docker.models.containers.Container, destination: PurePosixPath
-) -> Union[Path, str]:
-    """Identify the Docker mount bind path or volume for a given destination."""
+) -> docker.types.Mount:
     try:
         mount = [
             mount
@@ -172,6 +171,21 @@ def docker_mount_for(
         ][0]
     except IndexError:
         raise ValueError(f"No mount point for {destination}.")
+    return mount
+
+
+def is_volume_readonly(
+    container: docker.models.containers.Container, destination: PurePosixPath
+) -> bool:
+    mount = get_docker_mount(container, destination)
+    return not mount["RW"]
+
+
+def docker_mount_for(
+    container: docker.models.containers.Container, destination: PurePosixPath
+) -> Union[Path, str]:
+    """Identify the Docker mount bind path or volume for a given destination."""
+    mount = get_docker_mount(container, destination)
     if mount["Type"] == "bind":
         docker_root = PurePosixPath("/host_mnt")
         docker_path = PurePosixPath(mount["Source"])


Test patch:
diff --git a/tests/conftest.py b/tests/conftest.py
index 3616695..5327532 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -87,6 +87,11 @@ def _container_prefix(_random_token, monkeypatch_session):
     yield container_prefix
 
 
+@pytest.fixture(scope="class")
+def extra_volume_name(_random_token):
+    yield f"aiidalab-launch_tests_{_random_token}_extra_volume"
+
+
 # Avoid accidentally reading or writing from the host home directory.
 @pytest.fixture(scope="class", autouse=True)
 def home_path(tmp_path_factory, monkeypatch_session):
@@ -124,7 +129,17 @@ def profile(config):
 def instance(docker_client, profile):
     instance = AiidaLabInstance(client=docker_client, profile=profile)
     yield instance
-    for op in (instance.stop, partial(instance.remove, conda=True, data=True)):
+
+    def remove_extra_mounts():
+        for extra_mount in instance.profile.extra_mounts:
+            extra_volume, _, _ = instance.profile.parse_extra_mount(extra_mount)
+            docker_client.volumes.get(str(extra_volume)).remove()
+
+    for op in (
+        instance.stop,
+        partial(instance.remove, conda=True, data=True),
+        remove_extra_mounts,
+    ):
         try:
             op()
         except (docker.errors.NotFound, RequiresContainerInstance):
diff --git a/tests/test_cli.py b/tests/test_cli.py
index 526f6a1..44fe80d 100644
--- a/tests/test_cli.py
+++ b/tests/test_cli.py
@@ -8,6 +8,7 @@
 module.
 """
 import logging
+from dataclasses import replace
 
 import docker
 import pytest
@@ -241,3 +242,44 @@ def assert_status_down():
 
         assert not get_volume(instance.profile.home_mount)
         assert not get_volume(instance.profile.conda_volume_name())
+
+
+@pytest.mark.slow
+@pytest.mark.trylast
+class TestExtraVolumes:
+    def test_extra_volumes(
+        self, instance, application_state, extra_volume_name, docker_client, caplog
+    ):
+        caplog.set_level(logging.DEBUG)
+
+        # Add extra volume to default profile.
+        config = application_state.config
+        profile = config.profiles[0]
+        assert profile.name == config.default_profile
+        profile.extra_mounts = {f"{extra_volume_name}:/opt/extra:rw"}
+        replace(
+            application_state, config=replace(config, profiles=[profile])
+        ).save_config()
+
+        # Check that extra volume is picked up.
+        runner: CliRunner = CliRunner()
+        result = runner.invoke(cli.cli, ["profiles", "show", profile.name])
+        assert result.exit_code == 0
+        assert extra_volume_name in result.output
+
+        # Start instance
+        runner: CliRunner = CliRunner()
+        result: Result = runner.invoke(
+            cli.cli, ["start", "--no-browser", "--no-pull", "--wait=300"]
+        )
+        assert extra_volume_name in result.output
+        assert result.exit_code == 0
+
+        docker_client.volumes.get(extra_volume_name)  # should not throw
+
+        # Check that instance is up.
+        result: Result = runner.invoke(cli.cli, ["status"])
+        assert result.exit_code == 0
+        assert instance.profile.container_name() in result.output
+        assert "up" in result.output
+        assert instance.url() in result.output
diff --git a/tests/test_instance.py b/tests/test_instance.py
index 03d2a6d..16ae263 100644
--- a/tests/test_instance.py
+++ b/tests/test_instance.py
@@ -76,6 +76,17 @@ async def test_instance_home_bind_mount(instance):
     assert not any(instance.configuration_changes())
 
 
+async def test_instance_extra_mount(instance, extra_volume_name):
+    instance.profile.extra_mounts = {
+        f"{extra_volume_name}:/opt/extra_volume_mount:rw",
+        f"{Path.home()}:/opt/extra_bind_mount:ro",
+    }
+    instance.create()
+    assert await instance.status() is instance.AiidaLabInstanceStatus.CREATED
+    assert instance.profile == Profile.from_container(instance.container)
+    assert not any(instance.configuration_changes())
+
+
 async def test_profile_configuration_changes(instance):
     original_profile = deepcopy(instance.profile)
 
diff --git a/tests/test_profile.py b/tests/test_profile.py
index 7b3e35b..d6ad2dd 100644
--- a/tests/test_profile.py
+++ b/tests/test_profile.py
@@ -1,4 +1,5 @@
 from dataclasses import replace
+from pathlib import Path
 
 import pytest
 
@@ -6,7 +7,30 @@
 
 VALID_PROFILE_NAMES = ["abc", "Abc", "aBC", "a0", "a-a", "a-0"]
 
-INVALID_PROFILE_NAMES = ["", ".a", "a_a", "_a"]
+INVALID_PROFILE_NAMES = ["", ".a", "a_a", "_a", "aaa_"]
+
+VALID_HOME_MOUNTS = ["{vol}", "{path}", "{path}/home"]
+
+INVALID_HOME_MOUNTS = ["@{vol}", "./{path}"]
+
+VALID_EXTRA_MOUNTS = [
+    "{path}:/opt/test",
+    "{path}:/opt/test:ro",
+    "{path}:/opt/test:rw",
+    "{vol}:/opt/test:ro",
+    "{vol}:/opt/test:ro",
+]
+
+INVALID_EXTRA_MOUNTS = [
+    "./relative/{path}:/opt/test:ro",
+    "/nonexistent/{path}:/opt/test:ro",
+    "{path}:/opt/test:invalid_mode",
+    "invalidchar@:/opt/test:ro",
+    "{vol}:/opt/test:ro:extraarg",
+    "{vol}",
+    "{path}/nonexistent:/opt/test",
+    "x:/opt/test/:ro",
+]
 
 
 def test_profile_init(profile):
@@ -24,6 +48,32 @@ def test_profile_init_invalid_names(profile, name):
         replace(profile, name=name)
 
 
+@pytest.mark.parametrize("home_mount", VALID_HOME_MOUNTS)
+def test_profile_init_valid_home_mounts(profile, extra_volume_name, home_mount):
+    home_mount = home_mount.format(path=Path.home(), vol=extra_volume_name)
+    assert replace(profile, home_mount=home_mount).home_mount == home_mount
+
+
+@pytest.mark.parametrize("home_mount", INVALID_HOME_MOUNTS)
+def test_profile_init_invalid_home_mounts(profile, extra_volume_name, home_mount):
+    home_mount = home_mount.format(path=Path.home(), vol=extra_volume_name)
+    with pytest.raises(ValueError):
+        replace(profile, home_mount=home_mount)
+
+
+@pytest.mark.parametrize("extra_mount", VALID_EXTRA_MOUNTS)
+def test_profile_init_valid_extra_mounts(profile, extra_volume_name, extra_mount):
+    extra_mounts = {extra_mount.format(path=Path.home(), vol=extra_volume_name)}
+    assert replace(profile, extra_mounts=extra_mounts).extra_mounts == extra_mounts
+
+
+@pytest.mark.parametrize("extra_mount", INVALID_EXTRA_MOUNTS)
+def test_profile_init_invalid_extra_mounts(profile, extra_volume_name, extra_mount):
+    extra_mounts = {extra_mount.format(path=Path.home(), vol=extra_volume_name)}
+    with pytest.raises(ValueError):
+        replace(profile, extra_mounts=extra_mounts)
+
+
 def test_profile_equality(profile):
     assert profile == profile
     assert profile != replace(profile, name="other")