Skip to content

Commit

Permalink
Reuse docker.types.Mount
Browse files Browse the repository at this point in the history
  • Loading branch information
danielhollas committed Feb 4, 2024
1 parent d397a5d commit dc2d221
Show file tree
Hide file tree
Showing 4 changed files with 33 additions and 65 deletions.
10 changes: 5 additions & 5 deletions aiidalab_launch/__main__.py
Original file line number Diff line number Diff line change
Expand Up @@ -473,13 +473,13 @@ async def _async_start(
)

for extra_mount in profile.extra_mounts:
mount = ExtraMount.from_string(extra_mount)
mount = ExtraMount.parse_mount_string(extra_mount)
click.secho(
MSG_EXTRA_MOUNT.format(
source=mount.source,
target=mount.target,
mode=mount.mode,
mount_type=mount.type,
source=mount["Source"],
target=mount["Target"],
mode="ro" if mount["ReadOnly"] else "rw",
mount_type=mount["Type"],
).lstrip(),
fg="green",
)
Expand Down
9 changes: 2 additions & 7 deletions aiidalab_launch/instance.py
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,7 @@ def _conda_mount(self) -> docker.types.Mount:
return docker.types.Mount(
target=f"/home/{self.profile.system_user}/.conda",
source=self.profile.conda_volume_name(),
type="volume",
)

def _home_mount(self) -> docker.types.Mount:
Expand All @@ -115,13 +116,7 @@ def _home_mount(self) -> docker.types.Mount:
)

def _extra_mount(self, extra_mount: str) -> docker.types.Mount:
mount = ExtraMount.from_string(extra_mount)
return docker.types.Mount(
target=str(mount.target),
source=str(mount.source),
read_only=True if mount.mode == "ro" else False,
type=mount.type,
)
return ExtraMount.parse_mount_string(extra_mount)

def _mounts(self) -> Generator[docker.types.Mount, None, None]:
yield self._conda_mount()
Expand Down
63 changes: 19 additions & 44 deletions aiidalab_launch/profile.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
from pathlib import Path, PurePosixPath
from urllib.parse import quote_plus

import docker
import toml
from docker.models.containers import Container

Expand All @@ -31,19 +32,7 @@ def _default_port() -> int: # explicit function required to enable test patchin
return DEFAULT_PORT


DEFAULT_IMAGE = "aiidalab/full-stack:latest"


def _get_mount_type(source: str) -> str:
# We do not allow relative paths so if the path is not absolute,
# we assume volume mount, whose name is restricted by Docker.
if Path(source).is_absolute():
return "bind"
if not re.fullmatch(_REGEX_VALID_IMAGE_NAMES, source):
raise ValueError(
f"Invalid extra mount volume name '{source}'. Use absolute path for bind mounts."
)
return "volume"
DEFAULT_IMAGE = "docker.io/aiidalab/full-stack:latest"


def _get_configured_host_port(container: Container) -> int | None:
Expand All @@ -62,34 +51,20 @@ def _get_aiidalab_default_apps(container: Container) -> list:
return []


@dataclass
class ExtraMount:
source: Path
target: PurePosixPath
mode: str # TOOD: Make this a Literal
type: str

# We extend the Mount type from Docker API
# with some extra validation to fail early if user provide wrong argument.
# https://github.com/docker/docker-py/blob/bd164f928ab82e798e30db455903578d06ba2070/docker/types/services.py#L305
class ExtraMount(docker.types.Mount):
@classmethod
def from_string(cls, mount) -> ExtraMount:
fields = mount.split(":")
if len(fields) < 2 or len(fields) > 3:
raise ValueError(f"Invalid extra mount option '{mount}'")

source, target = fields[:2]
mount_type = _get_mount_type(source)

source_path, target_path = Path(source), PurePosixPath(target)
def parse_mount_string(cls, mount) -> docker.types.Mount:
mount = super().parse_mount_string(mount)
# Unlike for home_mount, we will not auto-create missing
# directories for extra mounts.
if mount_type == "bind" 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}' in '{mount}''")

return ExtraMount(source_path, target_path, mode, mount_type)
if mount["Type"] == "bind":
source_path = Path(mount["Source"])
if not source_path.exists():
raise ValueError(f"Directory '{source_path}' does not exist!")
return mount


@dataclass
Expand All @@ -115,20 +90,20 @@ def __post_init__(self):
if self.home_mount is None:
self.home_mount = f"{CONTAINER_PREFIX}{self.name}_home"

_ = _get_mount_type(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.copy():
mount = ExtraMount.from_string(extra_mount)
if not extra_mount.endswith(mount.mode):
mount = ExtraMount.parse_mount_string(extra_mount)
mode = "ro" if mount["ReadOnly"] else "rw"
if not extra_mount.endswith(mode):
self.extra_mounts.remove(extra_mount)
self.extra_mounts.add(f"{extra_mount}:{mount.mode}")
self.extra_mounts.add(f"{extra_mount}:{mode}")

if (
self.image.split(":")[0] == "aiidalab/full-stack"
self.image.split(":")[0].endswith("aiidalab/full-stack")
and self.system_user != "jovyan"
):
# TODO: ERROR out in this case
LOGGER.warning(
"Resetting the system user may create issues for this image!"
)
Expand Down
16 changes: 7 additions & 9 deletions tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -142,10 +142,11 @@ def instance(docker_client, profile):

def remove_extra_mounts():
for extra_mount in instance.profile.extra_mounts:
mount = ExtraMount.from_string(extra_mount)
# TODO: Rmtree
if mount.type == "volume":
docker_client.volumes.get(str(mount.source)).remove()
mount = ExtraMount.parse_mount_string(extra_mount)
# TODO: Rmtree on bind mount? Maybe for safety let's just keep them in /tmp
if mount["Type"] == "volume":
docker_client.volumes.get(mount["Source"]).remove()
print(mount)

for op in (
instance.stop,
Expand All @@ -154,11 +155,8 @@ def remove_extra_mounts():
):
try:
op()
except (docker.errors.NotFound, RequiresContainerInstance) as error:
print(
f"WARNING: Issue while stopping/removing instance: {error}",
file=sys.stderr,
)
except (docker.errors.NotFound, RequiresContainerInstance):
pass
except (RuntimeError, docker.errors.APIError) as error:
print(
f"WARNING: Issue while stopping/removing instance: {error}",
Expand Down

0 comments on commit dc2d221

Please sign in to comment.